When the view manager repaints an area, it paints the views back to front without regard to whether any of the background will actually be visible when the painting is done. A couple of examples: when an area of the browser window is repainted, we paint several layers, only to obscure all of the background layers with web content. When you select text in the URL bar in apprunner, we do lots of GIF tiling and drawing, because it's repainting the background tiled GIF under the text widget. I think fixing this could be a major performance win.

perf

Moving to M18

Viewmanager2 already contains code which optimizes for this case in nsViewManager2::OptimizeDisplayList(const nsRect& aDamageRect). It is dependent on the frame setting nsViewManager2::SetViewContentTransparency. If the frame's don't mark the view correctly the view manager will not optimize for this case. If there are frames that are not setting the ViewContentTransparency correctly we should file bugs on the owners of the frames.

Reopening to mark it INVALID instead of WONTFIX

Marking INVALID

Marking verfied invalid per last comments.

Why was this bug marked INVALID, rather than reassigned to the HTML Frames component? /be

Hope I'm not duping any bug that got closed -- sfraser says the symptom is still with us, and kmcclusk says it's not a Views bug. /be

Kevin, did you mean in general nsIFrame's should SetViewContentTransparency, or did you mean specifically HTML Frames (nsFrameFrame)? I suspect that this bug got the wrong component due to a namespace collision. :)

Eric: I didn't mean HTML frames, I meant the layout nsIFrame's in general need to use SetViewContentTransparency on their views to indicate that they are full opaque.

Changed summary from: View manager should avoid repainting background areas which are not exposed To: Opaque nsIFrame's instances need to set SetViewContentTransparency on their views to avoid repainting fully occluded background areas

Isn't nsContainerFrame::SyncViewAfterReflow doing that already?

Does anyone have a test case and a procedure for checking which frames are painting? I don't see any instrumentation in the tree.

We're seeing that large, unneccessary calls to FillRect measurably impact window repaint time on Mac. See data in 48274. How hard is it to implement this? Can we knock off a few easy cases?

This blocks 48274

One issue I worry about here is views that expand to contain their children. Right now (i.e., when I last tested, which was not all that recent) they expand on right/bottom but not left/top. See bug 13213. (The best solution here would be the reverse of the best solution there.) If a view that is larger than its frame is marked as opaque but it only paints the frame background, what happens?

Looking at about:blank in viewer, the Viewport frame and ScrollPort frame are marking themselves as opaque. The canvas frame is not (it probably isn't in this case, but I doubt it ever does -- it might be tricky though), and the scrollbar and slider frames are not (they probably could be rather easily).

Actually, the canvas frame's view seems to be marked opaque when the doc has a background, and the sliders seem to be marked opaque when they have nonzero dimensions. That still leaves the scrollbars, and any XUL stuff that doesn't show up in viewer...

Views coupled to frames that have "overflowing" children are always marked as transparent. http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsContainerFrame.cp p#533 But this isn't very common, as far as I know.

Where are you guys getting the information that the views are to blame? It's quite easy to create a page where there's only one view but you have a pile of stacked frames which paint the same rectangle over and over. The view manager can't help you there.

hey, we're grasping at straws ;) you're right, though. the navigator window only has 3 or 4 views (window, url bar, content area, maybe sidebar) so this probably isn't a panacea towards squashing overzealous painting.

Agreed.

Well, we could always create more views. For example, we could create a view for a frame whenever that view would be opaque. That would basically solve the problem if the view manager does its job right, but it could be costly in other ways. Views could certainly use some fat-trimming for one thing. By the way, now may be a good time to mention that I've experimented with using the view manager's Refresh(nsIRegion) code path instead of the Refresh(nsRect) that's currently always used. Works great in Windows, once I cleaned up the Refresh(nsIRegion) code, which was pretty rotten. I can entirely avoid painting any views that do not intersect the damage region (even if they intersect its bounding box). It also gets things into place so that we could pass a paint region down to nsFrame::Paint, which could have a major impact on bugs like this. Performance aside, the main reason for the patch was to reduce code duplication in nsViewManager. It's just not worth having a separate Refresh(nsRect) function. Oh, I also had to fix Windows' nsWindow implementation, which passes uninitialized garbage in nsPaintEvent::region, not even courteous enough to just pass nsnull :-).

Here's what I see when I expose a rectangular chunk of the bottom right part of the "about:blank" Mozilla window: First, Windows issues an update for the chrome window. Most of it is covered by the subwindow for HTML document so the paint event only covers the chrome at the bottom: > ROC: PaintRegion=(0,8249,7335,556)+ > ROC: Painting view (bounds=0,0,9735,8805), paintrect=0,8250,7335,555 > Painting frame background: 0,0,9735,8805 > Painting frame background: 0,0,9735,8805 > Painting frame background: 0,0,9735,300 > Painting frame background: 0,0,1440,195 > Painting frame background: 0,0,9735,255 Notice the stacked frames painting over each other. Notice also that nsCSSRendering::PaintBackground, on the "solid color" path, paints the entire border area regardless of the actual dirty rect. This would be trivial to fix and would really help this case. Then Windows issues an update for the HTML document's widget: > ROC: PaintRegion=(0,794,7335,6091)+ > ROC: Painting view (bounds=0,0,9735,6885), paintrect=0,794,7335,6091 > Painting frame background: 0,0,9735,6885 > ROC: Painting view (bounds=0,0,9735,6885), paintrect=0,794,7335,6091 Nothing much going on here. Conclusion: this is not a view manager problem. Even a perfect view manager can do nothing better than what's happening here. I did this test with my view manager, but I'm pretty sure it's no smarter than nsViewManager2 for this simple case.

Er, I mean the bottom left chunk of the window. I tried the obvious patch to nsCSSRendering::PaintBackground. It works. Here's the result: > ROC: PaintRegion=(0,8249,4710,556)+ > ROC: Painting view (bounds=0,0,9735,8805), paintrect=0,8250,4710,555 > Painting frame background: 0,8250,4710,555 > Painting frame background: 0,8250,4710,555 > Painting frame background: 0,0,4710,300 > Painting frame background: 0,0,1440,195 > Painting frame background: 0,0,4710,255 > ROC: PaintRegion=(0,1124,4710,5761)+ > ROC: Painting view (bounds=0,0,9735,6885), paintrect=0,1124,4710,5761 > Painting frame background: 0,1124,4710,5761 > ROC: Painting view (bounds=0,0,9735,6885), paintrect=0,1124,4710,5761 Tasty! Of course, life's not perfect. If the damage region includes the top of the chrome as well as the bottom, then we paint the entire bounding box, which means we paint a rect that is mostly behind the HTML content area. To fix that we either have to have frames paint general damage regions, or we could have the view manager break up the region into rectangles and request frame paint over a set of separate rectangles if (sum of areas of rectangles)/(area of bounding box) is lower than some threshold. I think the latter approach, plus making views for big opaque frames, would be a fairly easy and near optimal solution to the whole problem.

Hmm, maybe this rectangle information is misleading, because the rendering context should be clipping to the damage area anyway.

The viewmanager is setting the cliprect on the rendering context to match the damaged rect. The code is at line 655 in nsViewManager2. rect is the damaged rect passed in to nsViewManager2::Refresh nsRect trect = *rect; PRBool result; localcx->SetClipRect(trect, nsClipCombine_kReplace, result); All the fillrect's will be clipped to the damaged rect, unless a frame explicitly resets the clipping rect.

i think the main problem we have with slow drawing (and that's not the fault of this bug per se) is that we're relying too heavily on this clip rect and overdrawing by a factor of 10.

Setting the cliprect should be pretty efficient. I could be wrong, but I doubt that we would get a big speed up by trying to do a fillrect to match the damaged rect. I think the real problem is that the frames always paint back to front with the parent frame always painting it's background even though a child or other descendant will completely cover the area painted by the parent frame.

that's a better way of saying what i was trying to ;)

Relying on clipping may be only slightly less efficient than redrawing only the dirty rect in some cases (e.g. a FillRect), but in others, it can be measurably slower (e.g. image refreshes, which require blitting). But I think Kevin is correct, in that we redraw background frames way too much, when they are obscured by foremost ones. That's what this bug was originally about, until someone suggested that the view manager would solve this ;)

this is a blocker for a nsbeta3+ bug, nominating as such.

This is not currently a view manager problem, but I suggest we MAKE it a view manager problem by forcing opaque frames to have views. I think the philosophy of rendering is (or should be) that nsViewManager does all the hard work of worrying about clipping, opacity, z-ordering etc. Just as they do now, frames should just paint themselves and their children and use a damage rectangle to skip unnecessary work when that's easy to do. Therefore we should not try to smarten the frame painting code, we should just use more views. If views are too heavyweight or the view manager can't handle the burden, then we fix views and the view manager. I don't say that lightly; I'm willing to do the work. What say you?

Forcing opaque frames to have views should work OK with the current nsViewManager2. If we need to we can tune things by only making views for opaque frames larger than a certain size.

Another thing we can do in the view manager is to set the clip region to the exact visible dirty area of a view when we paint the view. Right now we set it to the bounding box of the visible dirty area (well actually, nsViewManager2 is a bit dumber than that, but it tries). The bounding box loses in some common situations though, like when you have one view containing a slightly inset view; we always paint the entire area of the outer view. It's easy to compute the exact visible dirty region, but there are some issues: setting and using a lot of complex clip regions could be expensive on some toolkits*, and nsRenderingContextWin::SetClipRegion is horribly broken (probably true in other toolkits too). Still, it's the only thing standing in the way of pretty much optimal view repainting (in terms of pixel coverage). *But using them should not be so bad, since all toolkits have to be able to paint windows which are partially obscured by other windows and may thus have complex visible regions. Given that the Mac got this right in 1984, everyone else should have it figured out by now. I hear setting clip regions is slow on X though.

It's not worth grafting heavy duty clip region stuff onto the current nsViewManager2 however. I'll work on it in the context of nsViewManager. I think the only piece of the puzzle we can do right away is to make opaque frames have views. I'll take a look.

one _big_ reason against making opaque frames have views is that there is code all over the place that calls GetNearestEnclosingView() and expects that view to be something in particular (eg, tree cells call that looking for their parent scrolling view). If the frame suddenly got itself, lots of things would break. I think the best thing is to make the frame drawing code smarter.

There are lots of ways that a frame can get a view. For example, the frame might have z-index set on it. How can any assumptions about GetNearestEnclosingView be safe?

I think creating view's for opaque frame's is the correct long term solution. The logic for intelligent rendering should be centralized within the view module, and the frames should only do very simple rendering.

Creating views for opaque frames was an easy little change, and produced the desired rendering improvement, but as Mr. Pinkerton suggested, it has broken something (scrollbar display, at least). I will keep working on it. Understandably you're looking for something quick and low risk for now, but I don't think that "making frame painting smarter" fits that bill...

Are views lightweight enough for us to do this? What are the bloat implications of having many more views per document?

Looks like about 100 bytes per view. With the opaqueness change, starting Mozilla, pulling open all the menus, surfing to Slashdot, and opening Preferences, I get up to 166 views. Similar workload without the change gives about 70 views. So that's no big deal. Now, if we're looking at a table with 10,000 cells each with a background color, then we're talking about real memory. But if this is a worry, then we can easily come up with throttling heuristics (e.g. only create views for frames that cover more than (e.g.) 2500 pixels, or put a limit on the number of "discretionary" views per view manager).

The scrollbar thing is because nsSliderFrame repositions its thumb but does not reposition the thumb's view (because it assumes there isn't one). Easy fix, but it exposes another problem with mouse capturing. Haven't found any problems related to GetNearestEnclosingView or similar functions, yet. Will it be OK if I file bugs against these broken assumptions? They should probably be cleaned up anyway.

are you trying trees? mail? bookmarks? sidebar? xpmenus w/ cascading submenus?

Trying all of those at once, I managed to get up to 473 views. I think we're more likely to run into problems with pathological pages than with the application stuff.

There seem to be some issues with mouse capturing. nsFrame::CaptureMouse sets up capturing to the nearest enclosing view. This seems irrational since it means, for example, that whether or not a block's background image is "fixed" will seriously affect the scope of mouse capturing. It's an interesting question to decide what exactly the scope of mouse capturing should be. It seems that capturing to the nearest scrolling view would be a good choice. Unfortunately that triggers some more bugs. Ick...

Well, I modified mouse capturing so that it captures to the nearest enclosing scrolling view. Works nicely, in fact, selection is much more useful now on complex pages. I also made changes to nsSliderFrame so that scrollbars work correctly when their thumb has a view. I haven't run into any other bugs involving assumptions about views not being present.

Oh, here's another big one: bug 45092. Comboboxes assume that their child frames do not have views. This is actually false for translucent comboboxes. All such assumptions are broken and will have to be fixed, whether or not we decide to make views for all opaque frames.

Marking nsbeta3-. Not enough time to fix before beta3.

Set milestone to mozilla0.9

Inconsistency alert: this bug is marked "Future" but blocks bug 48274 which is targeted at mozilla1.0 and "wanted for 0.9.1/Mojo beta".

The dependency is false. Removing.

It is? How do we know that?

Pinkerton says "copybits isn't really the problem i thought it was"

Adding back dependency from bug 48274. CopyBits is not used during frame redraw (which is the expensive part that this bug addresses). CopyBits is used to blit pixels from the offscreen buffer to the screen (the view manager's double buffering). The slow OS calls that result from this bug are over-zealous calls to FillRect, to paint frame background colors (see bug 69010).

Changing platform

try to get a better handle if painting performance is taking a lot of time. kevin is going to provide us a special build, or a patch, to turn off painting, and jrgm will do some measurements.

One example of why we need this: Typing in the URL bar causes us to do work tiling images which are under the url bar, since everything from the bottom up gets redrawn every time you type, or select.

turning off painting and running jrgm's tests shows painting takes 5%-7% of total page load time on an optimized Linux build.

Does the patch remove all work right down to the compositor, inclusive? (Sorry, I don't know the code enough to tell from the patch.)

"Does the patch remove all work right down to the compositor, inclusive?" Yes. It removes all of the processing of the NS_PAINT message. This includes all of the work done by the ViewManager, frame painting, calls to the rendering context, copying the offscreen to the window etc.

my guess is that this percentage will be higher on mac. Can someone try it?

Page loads aren't the only performance measurement worth making. It would also be good to look at the paint overhead of typing into the URL bar, or the mail composer, or form fields.

If I hadn't done those builds, run and watched those tests myself, I'd be suspicious of those numbers. But that is what the answer definitely is. I'll leave it to larger minds to ponder what it means. [p.s. should have note: those are both optimized builds].

Doh, I also should have mentioned that I did 5 cycles through the test for each run. So, on those charts, the average of those 5 times is the dotted line, and the diamond markers are the individual times per page, which gives a sense of the scatter in the results. [Note that on Mac, with painting off, this scatter is almost entirely gone (but again, I haven't a clue what that really means]. I'll see if I can sneak in a linux run (I need to go steal somebody else's build, since I can't do a compile on the server that is serving these tests).

There must be something wrong in the results on win2k. I don't know anything to the low-level implementation on win2k and I don't know much about jrgm's tests either, but if we have two threads - one thread that gets the data from the server, does the layout, waits a couple of seconds and asks for more data; another thread that does the painting and handles user interactions - I guess we would end up measuring only the time spent in first thread.

I hope I'm wrong. If not it would mean that jrgm's tests never measured the time spent in rendering on Windows.

Hrmm, that threading model doesn't sound like the way I understand it. Necko fetches data from the network on one or more threads, and then posts PLEvents to the main thread's event queue to indicate to listeners (e.g. the parser) that data is available. Everything else (parsing, reflow, drawing) happens on the main thread. In addition, these page-load tests are measuring wall-clock time, albeit dependent on the calling of an 'onload' handler. Maybe some event prioritization stuff on Windows affects when this onload handler is called, causing it to fire before we've actually painted the page content.

If you beat the paint suppression timer, then yes, the onload will fire before you've painted, and then the page will paint. If you lose to the paint suppression timer, then the page will paint prior to the onload firing. What is probably happening here is that Win32's times are so much faster than the other two compared platforms that Win32 on nearly every page is beating the paint suppression timer. A great way to verify this claim is to run jrgm's tests with a delay of 0. If you never paint a page, then you know you beat the timer. If you do see a page paint, then you know that page wasn't finished loading when the timer fired.

Effectively, for any page where you beat the timer, the cost of painting will be 0%. If you run jrgm's tests with a 500ms delay, on many pages, the cost of the painting is part of the 500ms delay, rather than part of the actual time. This will work on all platforms... it's just a matter of getting all platforms fast enough so that they can start coming in under the timer. :)

I'm also assuming jrgm's tests are still patched not to do a setTimeout when the delay is 0.

> In addition, these page-load tests are measuring wall-clock time, albeit > dependent on the calling of an 'onload' handler. Just wanted to affirm this. The test doesn't care about anything but the time between a call 'window.location.href="page2.html"' and when the onload handler is fired for 'page2.html'. (With the caveat/pitfall that if onload lies to me, well, it lies to me, and that's the time I get). > A great way to verify this claim is to run jrgm's tests with a delay of 0. > If you never paint a page, then you know you beat the timer. If you do see > a page paint, then you know that page wasn't finished loading when the timer > fired. I ran these again, content already cached again, and watched the pages load. Mac OS9, 450MHz, 256MB page times: avg 2.2s, max 6.0s, painted: ~33 of 40 win2k, 500MHz, 128MB, page times: avg 1.2s, max 3.5s, painted: 0 of 40 So, all painting was effectively suppressed already for the windows test. > I'm also assuming jrgm's tests are still patched not to do a setTimeout when > the delay is 0. Yes, on jrgm.mcom.com's server. (Just added now to cowtools.mcom.com server).

Changed summary to describe the real issue.

*** Bug 7179 has been marked as a duplicate of this bug. ***

Wouldn't it be possible to write some code that allows a frame to query its children, and ask whether they are opaque? If they are, and they cover the entire area of their parent frame, then we know that we don't have to paint the parent frame.

I still claim the best thing to do is to give views to opaque frames and let the view manager take care of it.

Moving to Mozilla1.1 Engineers are overloaded with higher priority bugs.

Bulk moving Mozilla1.2 bugs to future-P2. I will pull from this list when scheduling post Mozilla1.0 work.

Bulk moving Mozilla1.1 bugs to future-P2. I will pull from this list when scheduling post Mozilla1.0 work.

This performance work should wait until after Mozilla1.0. The frame's were designed to paint from front to back and changing this or adding additional views to control painting would be very risky. nsbeta1-.

I agree with the retargeting... You identified two main risks with the "use views" approach, and I agree that they're real issues, but I argue that they have to be resolved anyway: -- "increasing the number of views may increase footprint and hurt performance." Raw speed is not an issue; the view manager is smarter and faster than the frame system because it can keep more information around in the views. Memory footprint is a possible concern but the measurements I did for this bug a while ago show that the memory used by extra views would be insignificant. Views aren't big and they can even be shrunk considerably with a little bit of effort. -- "creating views for frames that never had them could break things" Yep, it sure does break things. But there are many ways to create views for arbitrary frames, e.g. by setting -moz-opacity, thus all the bugs caused by unexpected views have to be fixed sooner or later anyway. Nonetheless this alone is sufficient reason to push this out into the future.

*** Bug 69010 has been marked as a duplicate of this bug. ***

batch: adding topembed per Gecko2 document http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html

Any chance that the layout developers outside the Netscape firewall could have a look at that document?

Changing from topembed to embed, as this is not technically blocking a major embedding customer.

Bulk adding topembed keyword. Gecko/embedding needed.

removing topembed again, still a good thing to have but not critical

nsbeta1-

This has been defined as a topembed bug for Gecko2 in comment #86 How much effect will this have on page loading?

I think Kevin won't work on that anymore - anyone else willing to take this one?

Marking all tracking bugs which haven't been updated since 2014 as INCOMPLETE. If this bug is still relevant, please reopen it and move it into a bugzilla component related to the work being tracked. The Core: Tracking component will no longer be used.