Changeset 119736 in webkit

Timestamp:

Jun 7, 2012, 10:57:00 AM (13 years ago)

Author:

Simon Fraser

Message:

Optimize FrameView::scrollXForFixedPosition() / scrollYForFixedPosition()
​https://bugs.webkit.org/show_bug.cgi?id=88475

Reviewed by Sam Weinig.

FrameView's scrollXForFixedPosition() and scrollYForFixedPosition()
methods were often called together, but they do duplicate work,
including calling into platform widget code which might be slow.

Fix by converting scrollOffsetForFixedPosition() from being a wrapper
that just calls scrollXForFixedPosition() and scrollYForFixedPosition()
to the method that does all the work, calling just once into platform
widget code.

Changed callers to use scrollOffsetForFixedPosition() rather than make
two separate method calls.

Added ScrollView::layoutSize() and visibleSize() methods for
convenience.

Removed FrameView::scrollXForFixedPosition and FrameView::scrollYForFixedPosition
to avoid inefficient callers in future.

No new tests; refactoring only.

• page/FrameView.cpp:

(WebCore::fixedPositionScrollOffset):
(WebCore::FrameView::scrollOffsetForFixedPosition):

• page/FrameView.h: Removed scrollXForFixedPosition and scrollYForFixedPosition.
• platform/ScrollView.cpp:

(WebCore::ScrollView::layoutSize):

• platform/ScrollView.h:

(WebCore::ScrollView::visibleSize):

• rendering/RenderLayer.cpp:

(WebCore::RenderLayer::backgroundClipRect):

• rendering/RenderLayer.h:

(WebCore::ClipRect::move):

• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::requiresCompositingForPosition):

• rendering/RenderView.cpp:

(WebCore::RenderView::computeRectForRepaint):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/FrameView.cpp (modified) (1 diff)
• page/FrameView.h (modified) (1 diff)
• platform/ScrollView.cpp (modified) (1 diff)
• platform/ScrollView.h (modified) (2 diffs)
• rendering/RenderLayer.cpp (modified) (1 diff)
• rendering/RenderLayer.h (modified) (1 diff)
• rendering/RenderLayerCompositor.cpp (modified) (1 diff)
• rendering/RenderView.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-06-07  Simon Fraser  <simon.fraser@apple.com>
+
+        Optimize FrameView::scrollXForFixedPosition() / scrollYForFixedPosition()
+        https://bugs.webkit.org/show_bug.cgi?id=88475
+
+        Reviewed by Sam Weinig.
+        
+        FrameView's scrollXForFixedPosition() and scrollYForFixedPosition()
+        methods were often called together, but they do duplicate work,
+        including calling into platform widget code which might be slow.
+        
+        Fix by converting scrollOffsetForFixedPosition() from being a wrapper
+        that just calls scrollXForFixedPosition() and scrollYForFixedPosition()
+        to the method that does all the work, calling just once into platform
+        widget code.
+        
+        Changed callers to use scrollOffsetForFixedPosition() rather than make
+        two separate method calls.
+        
+        Added ScrollView::layoutSize() and visibleSize() methods for
+        convenience.
+        
+        Removed FrameView::scrollXForFixedPosition and FrameView::scrollYForFixedPosition
+        to avoid inefficient callers in future.
+
+        No new tests; refactoring only.
+
+        * page/FrameView.cpp:
+        (WebCore::fixedPositionScrollOffset):
+        (WebCore::FrameView::scrollOffsetForFixedPosition):
+        * page/FrameView.h: Removed scrollXForFixedPosition and scrollYForFixedPosition.
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::layoutSize):
+        * platform/ScrollView.h:
+        (WebCore::ScrollView::visibleSize):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::backgroundClipRect):
+        * rendering/RenderLayer.h:
+        (WebCore::ClipRect::move):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::requiresCompositingForPosition):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::computeRectForRepaint):
+
 2012-06-07  Raymes Khoury  <raymes@chromium.org>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -1373 +1373 @@
 }
 
-int FrameView::scrollXForFixedPosition() const
-{
-    int visibleContentWidth = visibleContentRect().width();
-    int maxX = contentsWidth() - visibleContentWidth;
-
-    if (maxX == 0)
+static int fixedPositionScrollOffset(int scrollPosition, int maxValue, int scrollOrigin, float dragFactor)
+{
+    if (!maxValue)
         return 0;
 
-    int x = scrollX();
-
-    if (!scrollOrigin().x()) {
-        if (x < 0)
-            x = 0;
-        else if (x > maxX)
-            x = maxX;
+    if (!scrollOrigin) {
+        if (scrollPosition < 0)
+            scrollPosition = 0;
+        else if (scrollPosition > maxValue)
+            scrollPosition = maxValue;
     } else {
-        if (x > 0)
-            x = 0;
-        else if (x < -maxX)
-            x = -maxX;
-    }
-
-    if (!m_frame)
-        return x;
-
-    float frameScaleFactor = m_frame->frameScaleFactor();
-
-    // When the page is scaled, the scaled "viewport" with respect to which fixed object are positioned
-    // doesn't move as fast as the content view, so that when the content is scrolled all the way to the
-    // end, the bottom of the scaled "viewport" touches the bottom of the real viewport.
-    float dragFactor = fixedElementsLayoutRelativeToFrame() ? 1 : (contentsWidth() - visibleContentWidth * frameScaleFactor) / maxX;
-
-    return x * dragFactor / frameScaleFactor;
-}
-
-int FrameView::scrollYForFixedPosition() const
-{
-    int visibleContentHeight = visibleContentRect().height();
-
-    int maxY = contentsHeight() - visibleContentHeight;
-    if (maxY == 0)
-        return 0;
-
-    int y = scrollY();
-
-    if (!scrollOrigin().y()) {
-        if (y < 0)
-            y = 0;
-        else if (y > maxY)
-            y = maxY;
-    } else {
-        if (y > 0)
-            y = 0;
-        else if (y < -maxY)
-            y = -maxY;
-    }
-
-    if (!m_frame)
-        return y;
-
-    float frameScaleFactor = m_frame->frameScaleFactor();
-    float dragFactor = fixedElementsLayoutRelativeToFrame() ? 1 : (contentsHeight() - visibleContentHeight * frameScaleFactor) / maxY;
-    return y * dragFactor / frameScaleFactor;
+        if (scrollPosition > 0)
+            scrollPosition = 0;
+        else if (scrollPosition < -maxValue)
+            scrollPosition = -maxValue;
+    }
+    
+    return scrollPosition * dragFactor;
 }
 
 IntSize FrameView::scrollOffsetForFixedPosition() const
 {
-    return IntSize(scrollXForFixedPosition(), scrollYForFixedPosition());
+    IntRect visibleContentRect = this->visibleContentRect();
+    IntSize contentsSize = this->contentsSize();
+    IntPoint scrollPosition = this->scrollPosition();
+    IntPoint scrollOrigin = this->scrollOrigin();
+    
+    IntSize maxOffset(contentsSize.width() - visibleContentRect.width(), contentsSize.height() - visibleContentRect.height());
+    
+    float frameScaleFactor = m_frame ? m_frame->frameScaleFactor() : 1;
+
+    FloatSize dragFactor = fixedElementsLayoutRelativeToFrame() ? FloatSize(1, 1) : FloatSize(
+        (contentsSize.width() - visibleContentRect.width() * frameScaleFactor) / maxOffset.width(),
+        (contentsSize.height() - visibleContentRect.height() * frameScaleFactor) / maxOffset.height());
+
+    int x = fixedPositionScrollOffset(scrollPosition.x(), maxOffset.width(), scrollOrigin.x(), dragFactor.width() / frameScaleFactor);
+    int y = fixedPositionScrollOffset(scrollPosition.y(), maxOffset.height(), scrollOrigin.y(), dragFactor.height() / frameScaleFactor);
+
+    return IntSize(x, y);
 }
 

trunk/Source/WebCore/page/FrameView.h

@@ -201 +201 @@
     // Functions for querying the current scrolled position, negating the effects of overhang
     // and adjusting for page scale.
-    int scrollXForFixedPosition() const;
-    int scrollYForFixedPosition() const;
     IntSize scrollOffsetForFixedPosition() const;
 

trunk/Source/WebCore/platform/ScrollView.cpp

@@ -253 +253 @@
 #endif
 
+IntSize ScrollView::layoutSize() const
+{
+    return m_fixedLayoutSize.isEmpty() || !m_useFixedLayout ? visibleSize() : m_fixedLayoutSize;
+}
+
 int ScrollView::layoutWidth() const
 {

trunk/Source/WebCore/platform/ScrollView.h

@@ -147 +147 @@
     virtual void setFixedVisibleContentRect(const IntRect& visibleContentRect) { m_fixedVisibleContentRect = visibleContentRect; }
     IntRect fixedVisibleContentRect() const { return m_fixedVisibleContentRect; }
+    IntSize visibleSize() const { return visibleContentRect().size(); }
     int visibleWidth() const { return visibleContentRect().width(); }
     int visibleHeight() const { return visibleContentRect().height(); }
@@ -152 +153 @@
     // Functions for getting/setting the size webkit should use to layout the contents. By default this is the same as the visible
     // content size. Explicitly setting a layout size value will cause webkit to layout the contents using this size instead.
+    IntSize layoutSize() const;
     int layoutWidth() const;
     int layoutHeight() const;

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -3903 +3903 @@
     // Note: infinite clipRects should not be scrolled here, otherwise they will accidentally no longer be considered infinite.
     if (parentRects.fixed() && rootLayer->renderer() == view && backgroundClipRect != PaintInfo::infiniteRect())
-        backgroundClipRect.move(view->frameView()->scrollXForFixedPosition(), view->frameView()->scrollYForFixedPosition());
+        backgroundClipRect.move(view->frameView()->scrollOffsetForFixedPosition());
 
     return backgroundClipRect;

trunk/Source/WebCore/rendering/RenderLayer.h

@@ -112 +112 @@
     }
     void move(LayoutUnit x, LayoutUnit y) { m_rect.move(x, y); }
+    void move(const LayoutSize& size) { m_rect.move(size); }
 
     bool isEmpty() const { return m_rect.isEmpty(); }

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -1805 +1805 @@
     // Fixed position elements that are invisible in the current view don't get their own layer.
     FrameView* frameView = m_renderView->frameView();
-    if (frameView && !layer->absoluteBoundingBox().intersects(IntRect(frameView->scrollXForFixedPosition(), frameView->scrollYForFixedPosition(), frameView->layoutWidth(), frameView->layoutHeight())))
+    if (frameView && !layer->absoluteBoundingBox().intersects(IntRect(IntPoint(frameView->scrollOffsetForFixedPosition()), frameView->layoutSize())))
         return false;
 

trunk/Source/WebCore/rendering/RenderView.cpp

@@ -391 +391 @@
 
     if (fixed && m_frameView)
-        rect.move(m_frameView->scrollXForFixedPosition(), m_frameView->scrollYForFixedPosition());
+        rect.move(m_frameView->scrollOffsetForFixedPosition());
         
     // Apply our transform if we have one (because of full page zooming).