Save one call to containerForRepaint() when updating layer positions
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Nov 2012 16:58:21 +0000 (16:58 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 Nov 2012 16:58:21 +0000 (16:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=101856

Reviewed by Dan Bernstein.

 RenderLayer::updateLayerPositions() has already computed the repaint container,
 but calls computeRepaintRects() which computes it again. Computing the repaint
 container involves a walk back up the layer tree, so calling it during a tree
 traversal is costly.

 Fix by passing the repaint container down into computeRepaintRects().

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateLayerPositions):
(WebCore::RenderLayer::computeRepaintRects):
(WebCore::RenderLayer::computeRepaintRectsIncludingDescendants):
(WebCore::RenderLayer::updateLayerPositionsAfterScroll):
(WebCore::RenderLayer::setHasVisibleContent):
* rendering/RenderLayer.h:
(RenderLayer):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@134174 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h

index f65da9a..3b3d0f0 100644 (file)
@@ -1,3 +1,26 @@
+2012-11-10  Simon Fraser  <simon.fraser@apple.com>
+
+        Save one call to containerForRepaint() when updating layer positions
+        https://bugs.webkit.org/show_bug.cgi?id=101856
+
+        Reviewed by Dan Bernstein.
+
+         RenderLayer::updateLayerPositions() has already computed the repaint container,
+         but calls computeRepaintRects() which computes it again. Computing the repaint
+         container involves a walk back up the layer tree, so calling it during a tree
+         traversal is costly.
+         
+         Fix by passing the repaint container down into computeRepaintRects().
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateLayerPositions):
+        (WebCore::RenderLayer::computeRepaintRects):
+        (WebCore::RenderLayer::computeRepaintRectsIncludingDescendants):
+        (WebCore::RenderLayer::updateLayerPositionsAfterScroll):
+        (WebCore::RenderLayer::setHasVisibleContent):
+        * rendering/RenderLayer.h:
+        (RenderLayer):
+
 2012-11-11  Kenichi Ishibashi  <bashi@chromium.org>
 
         WTFString::utf8() should have a mode of conversion to use replacement character
index 2a71bbe..7ba4fe8 100644 (file)
@@ -401,7 +401,8 @@ void RenderLayer::updateLayerPositions(LayoutPoint* offsetFromRoot, UpdateLayerP
         RenderLayerModelObject* repaintContainer = renderer()->containerForRepaint();
         LayoutRect oldRepaintRect = m_repaintRect;
         LayoutRect oldOutlineBox = m_outlineBox;
-        computeRepaintRects(offsetFromRoot);
+        computeRepaintRects(repaintContainer, offsetFromRoot);
+
         // FIXME: Should ASSERT that value calculated for m_outlineBox using the cached offset is the same
         // as the value not using the cached offset, but we can't due to https://bugs.webkit.org/show_bug.cgi?id=37048
         if (flags & CheckForRepaint) {
@@ -491,11 +492,10 @@ void RenderLayer::dirtyAncestorChainHasSelfPaintingLayerDescendantStatus()
     }
 }
 
-void RenderLayer::computeRepaintRects(LayoutPoint* offsetFromRoot)
+void RenderLayer::computeRepaintRects(const RenderLayerModelObject* repaintContainer, LayoutPoint* offsetFromRoot)
 {
     ASSERT(!m_visibleContentStatusDirty);
 
-    RenderLayerModelObject* repaintContainer = renderer()->containerForRepaint();
     m_repaintRect = renderer()->clippedOverflowRectForRepaint(repaintContainer);
     m_outlineBox = renderer()->outlineBoundsForRepaint(repaintContainer, offsetFromRoot);
 }
@@ -505,7 +505,8 @@ void RenderLayer::computeRepaintRectsIncludingDescendants()
 {
     // FIXME: computeRepaintRects() has to walk up the parent chain for every layer to compute the rects.
     // We should make this more efficient.
-    computeRepaintRects();
+    // FIXME: it's wrong to call this when layout is not up-to-date, which we do.
+    computeRepaintRects(renderer()->containerForRepaint());
 
     for (RenderLayer* layer = firstChild(); layer; layer = layer->nextSibling())
         layer->computeRepaintRectsIncludingDescendants();
@@ -536,12 +537,13 @@ void RenderLayer::updateLayerPositionsAfterScroll(UpdateLayerPositionsAfterScrol
 
     if ((flags & HasSeenViewportConstrainedAncestor) || renderer()->style()->hasViewportConstrainedPosition()) {
         // FIXME: Is it worth passing the offsetFromRoot around like in updateLayerPositions?
-        computeRepaintRects();
+        // FIXME: We could track the repaint container as we walk down the tree.
+        computeRepaintRects(renderer()->containerForRepaint());
         flags |= HasSeenViewportConstrainedAncestor;
     } else if ((flags & HasSeenAncestorWithOverflowClip) && !m_canSkipRepaintRectsUpdateOnScroll) {
         // If we have seen an overflow clip, we should update our repaint rects as clippedOverflowRectForRepaint
         // intersects it with our ancestor overflow clip that may have moved.
-        computeRepaintRects();
+        computeRepaintRects(renderer()->containerForRepaint());
     }
 
     if (renderer()->hasOverflowClip())
@@ -702,7 +704,7 @@ void RenderLayer::setHasVisibleContent()
 
     m_visibleContentStatusDirty = false; 
     m_hasVisibleContent = true;
-    computeRepaintRects();
+    computeRepaintRects(renderer()->containerForRepaint());
     if (!isNormalFlowOnly()) {
         // We don't collect invisible layers in z-order lists if we are not in compositing mode.
         // As we became visible, we need to dirty our stacking contexts ancestors to be properly
index 7fe448a..b41e29f 100644 (file)
@@ -699,7 +699,7 @@ private:
     void setAncestorChainHasSelfPaintingLayerDescendant();
     void dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
 
-    void computeRepaintRects(LayoutPoint* offsetFromRoot = 0);
+    void computeRepaintRects(const RenderLayerModelObject* repaintContainer, LayoutPoint* offsetFromRoot = 0);
     void computeRepaintRectsIncludingDescendants();
     void clearRepaintRects();