I just tried taking out the innerHTML updates on the testcase for bug 424715 and discovered (with my various layout patches to make the O(N^2) behavior go away) that painting takes a lot less time that way (15s runtime instead of 65s). I thought this might have been due to us simplifying paint regions, but I tried changing MAX_RECTS_IN_REGION in nsChildView.mm to 1000 and changed the SimplifyOutward calls in nsViewManager::UpdateWidgetArea and AccumulateIntersectionsIntoDirtyRegion to use 1000 as well, and that seems to have no effect. It'd be nice to figure out why we're painting as much as we are here.

The damage region we receive for the paints is the entire window, which is pretty disturbing.

And that's what we get from the system...

I lied, here is the sort of thing we get at each frame: Breakpoint 3, nsChildView::Invalidate (this=0xc91b1c0, aRect=@0xbfffbde4, aIsSynchronous=0) at /Users/roc/mozilla-checkin/widget/src/cocoa/nsChildView.mm:1811 1811 NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT; $527 = (const nsIntRect & volatile) @0xbfffbde4: { x = 769, y = 0, width = 383, height = 15 } Current language: auto; currently objective-c++ Warning: the current language does not match this frame. Breakpoint 3, nsChildView::Invalidate (this=0xc91b1c0, aRect=@0xbfffbd64, aIsSynchronous=0) at /Users/roc/mozilla-checkin/widget/src/cocoa/nsChildView.mm:1811 1811 NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT; $528 = (const nsIntRect & volatile) @0xbfffbd64: { x = 769, y = 0, width = 383, height = 15 } Breakpoint 3, nsChildView::Invalidate (this=0xc91b1c0, aRect=@0xbfffbf34, aIsSynchronous=0) at /Users/roc/mozilla-checkin/widget/src/cocoa/nsChildView.mm:1811 1811 NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT; $529 = (const nsIntRect & volatile) @0xbfffbf34: { x = 769, y = 0, width = 383, height = 15 } Breakpoint 3, nsChildView::Invalidate (this=0xc91b1c0, aRect=@0xbfffbf34, aIsSynchronous=0) at /Users/roc/mozilla-checkin/widget/src/cocoa/nsChildView.mm:1811 1811 NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT; $530 = (const nsIntRect & volatile) @0xbfffbf34: { x = 769, y = 14, width = 383, height = 1 } Breakpoint 3, nsChildView::Invalidate (this=0xc91b1c0, aRect=@0xbfffbf34, aIsSynchronous=0) at /Users/roc/mozilla-checkin/widget/src/cocoa/nsChildView.mm:1811 1811 NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT; $531 = (const nsIntRect & volatile) @0xbfffbf34: { x = 454, y = 233, width = 244, height = 10 } Breakpoint 2, nsViewManager::Refresh (this=0xfcf5430, aView=0xd843bf0, aContext=0xe5729e0, aRegion=0xe5734c0, aUpdateFlags=1) at /Users/roc/mozilla-checkin/view/src/nsViewManager.cpp:436 436 if (damageRegion.IsEmpty()) { $532 = { mRectCount = 1, mCurRect = 0x10c2c5c, mRectListHead = { <nsRegion::nsRectFast> = { <nsRect> = { x = 0, y = 0, width = 0, height = 0 }, <No data fields>}, members of nsRegion::RgnRect: prev = 0x10c2c5c, next = 0x10c2c5c }, mBoundRect = { <nsRect> = { x = 46140, y = 0, width = 22980, height = 900 }, <No data fields>} } Current language: auto; currently c++ $533 = { <nsRegion::nsRectFast> = { <nsRect> = { x = 46140, y = 0, width = 22980, height = 900 }, <No data fields>}, members of nsRegion::RgnRect: prev = 0xbfffc0a8, next = 0xbfffc0a8 } $534 = { <nsRegion::nsRectFast> = { <nsRect> = { x = 0, y = 0, width = 0, height = 0 }, <No data fields>}, members of nsRegion::RgnRect: prev = 0x10c2c5c, next = 0x10c2c5c } Breakpoint 2, nsViewManager::Refresh (this=0xfcf5430, aView=0xd843bf0, aContext=0xe0a48f0, aRegion=0xe572960, aUpdateFlags=1) at /Users/roc/mozilla-checkin/view/src/nsViewManager.cpp:436 436 if (damageRegion.IsEmpty()) { $535 = { mRectCount = 2, mCurRect = 0x10c2aac, mRectListHead = { <nsRegion::nsRectFast> = { <nsRect> = { x = 0, y = 2147483647, width = 0, height = 0 }, <No data fields>}, members of nsRegion::RgnRect: prev = 0x10c2aac, next = 0x10c2c14 }, mBoundRect = { <nsRect> = { x = 27240, y = 0, width = 41880, height = 14580 }, <No data fields>} } $536 = { <nsRegion::nsRectFast> = { <nsRect> = { x = 46140, y = 0, width = 22980, height = 900 }, <No data fields>}, members of nsRegion::RgnRect: prev = 0xbfffaf38, next = 0x10c2aac } $537 = { <nsRegion::nsRectFast> = { <nsRect> = { x = 27240, y = 13980, width = 14640, height = 600 }, <No data fields>}, members of nsRegion::RgnRect: prev = 0x10c2c14, next = 0xbfffaf38 }

We get 4 invalidates associated with the text, followed by 1 invalidate of the pixel strip, which is 10 pixels high. Then we get two refreshes, the first one is just for the text, then again for the text plus pixel strip. Here's why we see two refreshes here ... there's a pending paint event for the invalidated text and a pending reflow for the abs-pos elements in the pixel strip (and possibly also a pending reflow for the text, but that doesn't matter). The repaint event fires and we get into nsViewManager::DispatchEvent which flushes reflows, which triggers another invalidate which leads to the second refresh. I'm not sure why the bounding box for the reflowed stuff is 10 pixels high, need to look into that. Dumping display lists, it's also clear that we're painting every single abs-pos pixel-rect in the bounding-box of the dirty region, even though we optimize the display list which should be throwing out elements that aren't in the region, looking into that now.

I think we're failing to optimize here because we're subtracting the opaque pixel rects from the region until the region gets too complex, and then it fluffs out to its bounding-box. If I remove the code that subtracts the bounding box from the region, we only paint a reasonable subset of the abs-pos elements. (However, my build does have the patches from bug 513082 in it, so something slightly different may be going on on trunk).

Hmm. I thought I had hacked most of the fluff-out things to not do it (per comment 0), but maybe I missed one?

I think the 10-pixels-high dirty rect is just 3 rows of pixels plus 1 pixel of slop due to the abs-pos rects not being pixel-aligned, so nothing particularly wrong there.

(In reply to comment #6) > Hmm. I thought I had hacked most of the fluff-out things to not do it (per > comment 0), but maybe I missed one? Maybe, I dunno.

This fixes it for me. Seems like the right thing to do, because it ensures that we never increase the area of the visible region.

Comment on attachment 400909 [details] [diff] [review] patch This is obviously good, but doesn't make the testcase better on its own for me...

Maybe it's the combination with the patches for bug 513082. Hopefully that will get reviewed and we can get all this on trunk and see how we stand.

Sure, let's try that.

There's another reason for redrawing the entire window: focus rings. Bug and patch coming soon.

Comment on attachment 400909 [details] [diff] [review] patch This is a simple safe fix that we should probably take on 1.9.2.

Comment on attachment 400909 [details] [diff] [review] patch a1.9.2=dbaron