Title: [135777] branches/safari-536.28-branch/Source/WebCore
Revision
135777
Author
simon.fra...@apple.com
Date
2012-11-26 15:12:49 -0800 (Mon, 26 Nov 2012)

Log Message

<rdar://problem/12751360>
Merge r135746

    2012-11-26  Simon Fraser  <simon.fra...@apple.com>

    Optimize layer updates after scrolling
    https://bugs.webkit.org/show_bug.cgi?id=102635

    Reviewed by Sam Weinig.

    updateLayerPositionsAfterScroll() previously unconditionally cleared clip
    rects, and recomputed repaint rects too often. Recomputing both of these
    can be very expensive, as they involve tree walks up to the root.

    We can optimize layer updates after document scrolling by only clearing clip
    rects, and recomputing repaint rects, if we encounter a fixed- or sticky-position
    element. For overflow scroll, we have to clear clip rects and recompute repaint rects.

    * page/FrameView.cpp:
    (WebCore::FrameView::repaintFixedElementsAfterScrolling): Call updateLayerPositionsAfterDocumentScroll().
    * rendering/RenderLayer.cpp:
    (WebCore::RenderLayer::updateLayerPositions): Call clearClipRects() because
    updateLayerPosition() no longer does.
    (WebCore::RenderLayer::updateLayerPositionsAfterDocumentScroll): Version of updateLayerPositionsAfterScroll()
    that is for document scrolls. It has no need to push layers to the geometry map.
    (WebCore::RenderLayer::updateLayerPositionsAfterOverflowScroll): Pushes layers to the geometry map,
    and calls updateLayerPositionsAfterScroll() with the IsOverflowScroll flag.
    (WebCore::RenderLayer::updateLayerPositionsAfterScroll): Set the HasChangedAncestor flag
    if our location changed, and use that as a hint to clear cached rects. Be more conservative
    than before about when to clear cached clip rects.
    (WebCore::RenderLayer::updateLayerPosition):  Move responsibility for calling
    clearClipRects() ouf of this function and into callers.
    (The one caller outside RenderLayer will be removed via bug 102624).
    Return a bool indicating whether our position changed.
    (WebCore::RenderLayer::scrollTo): Call updateLayerPositionsAfterOverflowScroll().
    (WebCore::RenderLayer::updateClipRects): Added some #ifdeffed out code that is useful
    to verify that cached clips are correct; it's too slow to leave enabled in debug builds.
    * rendering/RenderLayer.h:
    (WebCore::RenderLayer::setLocation): Change to take a LayoutPoint, rather than separate
    x and y.

Modified Paths

Diff

Modified: branches/safari-536.28-branch/Source/WebCore/ChangeLog (135776 => 135777)


--- branches/safari-536.28-branch/Source/WebCore/ChangeLog	2012-11-26 23:12:43 UTC (rev 135776)
+++ branches/safari-536.28-branch/Source/WebCore/ChangeLog	2012-11-26 23:12:49 UTC (rev 135777)
@@ -1,5 +1,48 @@
 2012-11-26  Simon Fraser  <simon.fra...@apple.com>
 
+        <rdar://problem/12751360>
+        Merge r135746
+
+    2012-11-26  Simon Fraser  <simon.fra...@apple.com>
+    
+            Optimize layer updates after scrolling
+            https://bugs.webkit.org/show_bug.cgi?id=102635
+    
+            Reviewed by Sam Weinig.
+    
+            updateLayerPositionsAfterScroll() previously unconditionally cleared clip
+            rects, and recomputed repaint rects too often. Recomputing both of these
+            can be very expensive, as they involve tree walks up to the root.
+            
+            We can optimize layer updates after document scrolling by only clearing clip
+            rects, and recomputing repaint rects, if we encounter a fixed- or sticky-position
+            element. For overflow scroll, we have to clear clip rects and recompute repaint rects.
+    
+            * page/FrameView.cpp:
+            (WebCore::FrameView::repaintFixedElementsAfterScrolling): Call updateLayerPositionsAfterDocumentScroll().
+            * rendering/RenderLayer.cpp:
+            (WebCore::RenderLayer::updateLayerPositions): Call clearClipRects() because
+            updateLayerPosition() no longer does.
+            (WebCore::RenderLayer::updateLayerPositionsAfterDocumentScroll): Version of updateLayerPositionsAfterScroll()
+            that is for document scrolls. It has no need to push layers to the geometry map.
+            (WebCore::RenderLayer::updateLayerPositionsAfterOverflowScroll): Pushes layers to the geometry map,
+            and calls updateLayerPositionsAfterScroll() with the IsOverflowScroll flag.
+            (WebCore::RenderLayer::updateLayerPositionsAfterScroll): Set the HasChangedAncestor flag
+            if our location changed, and use that as a hint to clear cached rects. Be more conservative
+            than before about when to clear cached clip rects.
+            (WebCore::RenderLayer::updateLayerPosition):  Move responsibility for calling
+            clearClipRects() ouf of this function and into callers.
+            (The one caller outside RenderLayer will be removed via bug 102624).
+            Return a bool indicating whether our position changed.
+            (WebCore::RenderLayer::scrollTo): Call updateLayerPositionsAfterOverflowScroll().
+            (WebCore::RenderLayer::updateClipRects): Added some #ifdeffed out code that is useful
+            to verify that cached clips are correct; it's too slow to leave enabled in debug builds.
+            * rendering/RenderLayer.h:
+            (WebCore::RenderLayer::setLocation): Change to take a LayoutPoint, rather than separate
+            x and y.
+
+2012-11-26  Simon Fraser  <simon.fra...@apple.com>
+
         <rdar://problem/12753059>
         Merge r135025
 

Modified: branches/safari-536.28-branch/Source/WebCore/page/FrameView.cpp (135776 => 135777)


--- branches/safari-536.28-branch/Source/WebCore/page/FrameView.cpp	2012-11-26 23:12:43 UTC (rev 135776)
+++ branches/safari-536.28-branch/Source/WebCore/page/FrameView.cpp	2012-11-26 23:12:49 UTC (rev 135777)
@@ -1761,7 +1761,7 @@
     if (!m_nestedLayoutCount && hasFixedObjects()) {
         if (RenderView* root = rootRenderer(this)) {
             root->updateWidgetPositions();
-            root->layer()->updateLayerPositionsAfterScroll();
+            root->layer()->updateLayerPositionsAfterDocumentScroll();
         }
     }
 }

Modified: branches/safari-536.28-branch/Source/WebCore/rendering/RenderLayer.cpp (135776 => 135777)


--- branches/safari-536.28-branch/Source/WebCore/rendering/RenderLayer.cpp	2012-11-26 23:12:43 UTC (rev 135776)
+++ branches/safari-536.28-branch/Source/WebCore/rendering/RenderLayer.cpp	2012-11-26 23:12:49 UTC (rev 135777)
@@ -329,6 +329,9 @@
                            // to our parent layer.
     if (geometryMap)
         geometryMap->pushMappingsToAncestor(this, parent());
+
+    // Clear our cached clip rect information.
+    clearClipRects();
     
     if (hasOverflowControls()) {
         LayoutPoint offsetFromRoot;
@@ -451,13 +454,24 @@
     m_outlineBox = IntRect();
 }
 
-void RenderLayer::updateLayerPositionsAfterScroll()
+void RenderLayer::updateLayerPositionsAfterDocumentScroll()
 {
+    ASSERT(this == renderer()->view()->layer());
+
     RenderGeometryMap geometryMap;
+    updateLayerPositionsAfterScroll(&geometryMap);
+}
+
+void RenderLayer::updateLayerPositionsAfterOverflowScroll()
+{
+    RenderGeometryMap geometryMap;
     RenderView* view = renderer()->view();
     if (this != view->layer())
         geometryMap.pushMappingsToAncestor(parent(), 0);
-    updateLayerPositionsAfterScroll(&geometryMap);
+
+    // FIXME: why is it OK to not check the ancestors of this layer in order to
+    // initialize the HasSeenViewportConstrainedAncestor and HasSeenAncestorWithOverflowClip flags?
+    updateLayerPositionsAfterScroll(&geometryMap, IsOverflowScroll);
 }
 
 void RenderLayer::updateLayerPositionsAfterScroll(RenderGeometryMap* geometryMap, UpdateLayerPositionsAfterScrollFlags flags)
@@ -472,25 +486,32 @@
     if (!m_hasVisibleDescendant && !m_hasVisibleContent)
         return;
 
-    updateLayerPosition();
+    bool positionChanged = updateLayerPosition();
+    if (positionChanged)
+        flags |= HasChangedAncestor;
 
     if (geometryMap)
         geometryMap->pushMappingsToAncestor(this, parent());
 
-    if ((flags & HasSeenViewportConstrainedAncestor) || renderer()->style()->position() == FixedPosition) {
-        // FIXME: Is it worth passing the offsetFromRoot around like in updateLayerPositions?
-        // FIXME: We could track the repaint container as we walk down the tree.
-        computeRepaintRects(renderer()->containerForRepaint(), geometryMap);
+    if (flags & HasChangedAncestor || flags & HasSeenViewportConstrainedAncestor || flags & IsOverflowScroll)
+        clearClipRects();
+
+    if (renderer()->style()->position() == FixedPosition)
         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(renderer()->containerForRepaint(), geometryMap);
-    }
 
     if (renderer()->hasOverflowClip())
         flags |= HasSeenAncestorWithOverflowClip;
 
+    if (flags & HasSeenViewportConstrainedAncestor
+        || (flags & IsOverflowScroll && flags & HasSeenAncestorWithOverflowClip && !m_canSkipRepaintRectsUpdateOnScroll)) {
+        // FIXME: We could track the repaint container as we walk down the tree.
+        computeRepaintRects(renderer()->containerForRepaint(), geometryMap);
+    } else {
+        // Check that our cached rects are correct.
+        ASSERT(m_repaintRect == renderer()->clippedOverflowRectForRepaint(renderer()->containerForRepaint()));
+        ASSERT(m_outlineBox == renderer()->outlineBoundsForRepaint(renderer()->containerForRepaint(), geometryMap));
+    }
+    
     for (RenderLayer* child = firstChild(); child; child = child->nextSibling())
         child->updateLayerPositionsAfterScroll(geometryMap, flags);
 
@@ -752,7 +773,7 @@
     return has3DTransform();
 }
 
-void RenderLayer::updateLayerPosition()
+bool RenderLayer::updateLayerPosition()
 {
     LayoutPoint localPoint;
     LayoutSize inlineBoundingBoxOffset; // We don't put this into the RenderLayer x/y for inlines, so we need to subtract it out when done.
@@ -768,9 +789,6 @@
         localPoint += box->topLeftLocationOffset();
     }
 
-    // Clear our cached clip rect information.
-    clearClipRects();
- 
     if (!renderer()->isPositioned() && renderer()->parent()) {
         // We must adjust our position by walking up the render tree looking for the
         // nearest enclosing object with a layer.
@@ -813,9 +831,12 @@
         LayoutSize scrollOffset = parent()->scrolledContentOffset();
         localPoint -= scrollOffset;
     }
-        
+
+    bool positionOrOffsetChanged = false;
     if (renderer()->isRelPositioned()) {
-        m_relativeOffset = renderer()->relativePositionOffset();
+        LayoutSize newOffset = renderer()->relativePositionOffset();
+        positionOrOffsetChanged = newOffset != m_relativeOffset;
+        m_relativeOffset = newOffset;
         localPoint.move(m_relativeOffset);
     } else {
         m_relativeOffset = LayoutSize();
@@ -823,7 +844,10 @@
 
     // FIXME: We'd really like to just get rid of the concept of a layer rectangle and rely on the renderers.
     localPoint -= inlineBoundingBoxOffset;
-    setLocation(localPoint.x(), localPoint.y());
+    
+    positionOrOffsetChanged |= location() != localPoint;
+    setLocation(localPoint);
+    return positionOrOffsetChanged;
 }
 
 TransformationMatrix RenderLayer::perspectiveTransform() const
@@ -1565,26 +1589,27 @@
         return;
     m_scrollOffset = newScrollOffset;
 
-    // Update the positions of our child layers (if needed as only fixed layers should be impacted by a scroll).
-    // We don't update compositing layers, because we need to do a deep update from the compositing ancestor.
-    updateLayerPositionsAfterScroll();
-
     RenderView* view = renderer()->view();
     
     // We should have a RenderView if we're trying to scroll.
     ASSERT(view);
-    if (view) {
+
+    // Update the positions of our child layers (if needed as only fixed layers should be impacted by a scroll).
+    // We don't update compositing layers, because we need to do a deep update from the compositing ancestor.
+    bool inLayout = view ? view->frameView()->isInLayout() : false;
+    if (!inLayout) {
+        // If we're in the middle of layout, we'll just update layers once layout has finished.
+        updateLayerPositionsAfterOverflowScroll();
+        if (view) {
+            // Update regions, scrolling may change the clip of a particular region.
 #if ENABLE(DASHBOARD_SUPPORT)
-        // Update dashboard regions, scrolling may change the clip of a
-        // particular region.
-        view->frameView()->updateDashboardRegions();
+            view->frameView()->updateDashboardRegions();
 #endif
-
-        view->updateWidgetPositions();
+            view->updateWidgetPositions();
+        }
+        updateCompositingLayersAfterScroll();
     }
 
-    updateCompositingLayersAfterScroll();
-
     RenderBoxModelObject* repaintContainer = renderer()->containerForRepaint();
     Frame* frame = renderer()->frame();
     if (frame) {
@@ -3829,6 +3854,15 @@
     if (m_clipRectsCache && m_clipRectsCache->m_clipRects[clipRectsType]) {
         ASSERT(rootLayer == m_clipRectsCache->m_clipRectsRoot[clipRectsType]);
         ASSERT(m_clipRectsCache->m_scrollbarRelevancy[clipRectsType] == relevancy);
+        
+#ifdef CHECK_CACHED_CLIP_RECTS
+        // This code is useful to check cached clip rects, but is too expensive to leave enabled in debug builds by default.
+        ClipRectsContext tempContext(clipRectsContext);
+        tempContext.clipRectsType = TemporaryClipRects;
+        ClipRects clipRects;
+        calculateClipRects(tempContext, clipRects);
+        ASSERT(clipRects == *m_clipRectsCache->m_clipRects[clipRectsType].get());
+#endif
         return; // We have the correct cached value.
     }
     

Modified: branches/safari-536.28-branch/Source/WebCore/rendering/RenderLayer.h (135776 => 135777)


--- branches/safari-536.28-branch/Source/WebCore/rendering/RenderLayer.h	2012-11-26 23:12:43 UTC (rev 135776)
+++ branches/safari-536.28-branch/Source/WebCore/rendering/RenderLayer.h	2012-11-26 23:12:49 UTC (rev 135777)
@@ -300,7 +300,7 @@
     }
     
     const LayoutPoint& location() const { return m_topLeft; }
-    void setLocation(LayoutUnit x, LayoutUnit y) { m_topLeft = LayoutPoint(x, y); }
+    void setLocation(const LayoutPoint& p) { m_topLeft = p; }
 
     const IntSize& size() const { return m_layerSize; }
     void setSize(const IntSize& size) { m_layerSize = size; }
@@ -382,7 +382,8 @@
 
     bool canRender3DTransforms() const;
 
-    void updateLayerPosition();
+    // Returns true if the position changed.
+    bool updateLayerPosition();
 
     enum UpdateLayerPositionsFlag {
         CheckForRepaint = 1,
@@ -394,8 +395,10 @@
     static const UpdateLayerPositionsFlags defaultFlags = CheckForRepaint | IsCompositingUpdateRoot | UpdateCompositingLayers;
 
     void updateLayerPositionsAfterLayout(const RenderLayer* rootLayer, UpdateLayerPositionsFlags);
-    void updateLayerPositionsAfterScroll();
 
+    void updateLayerPositionsAfterOverflowScroll();
+    void updateLayerPositionsAfterDocumentScroll();
+    
     void updateTransform();
 
     void relativePositionOffset(LayoutUnit& relX, LayoutUnit& relY) const { relX += m_relativeOffset.width(); relY += m_relativeOffset.height(); }
@@ -661,8 +664,10 @@
 
     enum UpdateLayerPositionsAfterScrollFlag {
         NoFlag = 0,
-        HasSeenViewportConstrainedAncestor = 1 << 0,
-        HasSeenAncestorWithOverflowClip = 1 << 1
+        IsOverflowScroll = 1 << 0,
+        HasSeenViewportConstrainedAncestor = 1 << 1,
+        HasSeenAncestorWithOverflowClip = 1 << 2,
+        HasChangedAncestor = 1 << 3
     };
     typedef unsigned UpdateLayerPositionsAfterScrollFlags;
     void updateLayerPositionsAfterScroll(RenderGeometryMap*, UpdateLayerPositionsAfterScrollFlags = NoFlag);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to