Bug 7259 – Pages in bugzilla scroll slowly when obsolete attachments are present

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

opacity basically has two major performance problems.

The first problem can be traced to mustExamineRenderer in RenderLayer.cpp, which basically gives up if you're an inlineFlow, meaning even when inline flows are totally off-screen, we still begin/end transparency layers for them (and try to paint them).

The second problem is that there's no clip set when we begin/end transparency layers, and so they're always enormous.  We need to try to set up a clip to limit the size.

I think a good way to fix mustExamineRenderer is to completely eliminate it and to also eliminate layer bounds.  We should replace the concept of layer bounds with an actual "bounding box" IntRect that should be specifically used for hit testing and for painting of the layer.  Then places that really cared about the use of x/w/width/height of the render object could use that instead of x/y/width/height vals in the layer.

[Dave Hyatt]

To solve the second problem, we can use this boundingBox concept but also have a function to get the boundingBox of a layer and all its child layers.  Then when starting and ending a transparency layer we can set up a clip that is that bounding box (which I believe will reduce the size of the offscreen buffer created by CGBeginTransparencyLayer).

Fixing problem 1 will make scrolling performance good again on Bugzilla when the transparent layers are off screen.

Fixing problem 2 will make the scrolling performance good even when the transparent layers are on screen.

[Dave Hyatt]

Created attachment 7054 [details]
Patch to address problem #1.

This patch addresses the first issue and makes scrolling zippy again when no transparency layers are on-screen.

[Dave Hyatt]

Created attachment 7055 [details]
Complete patch.

[Maciej Stachowiak]

Comment on attachment 7055 [details]
Complete patch.

r=me, but I have some style comments:

+    // Always examine the canvas and the root.
+    // FIXME: We need to 

Is that the comment you meant to write?


+IntRect RenderLayer::absoluteBoundingBox() const

This function is tricky, it might be easier to read if you broke out the inline and non-inline cases.

+        result.setX(m_x + result.x());
+        result.setY(m_y + result.y());

I think this would read better using IntRect::move


+    return IntRect(absX + result.x() - m_x,
+                   absY + result.y() - m_y,
+                   result.width(),
+                   result.height());

another case for using move() IMO

[Dave Hyatt]

Fixed.

[Eric Seidel (no email)]

Would be great to see this in a software update... :)