- Revision
- 119736
- Author
- simon.fra...@apple.com
- Date
- 2012-06-07 10:57:00 -0700 (Thu, 07 Jun 2012)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (119735 => 119736)
--- trunk/Source/WebCore/ChangeLog 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/ChangeLog 2012-06-07 17:57:00 UTC (rev 119736)
@@ -1,3 +1,47 @@
+2012-06-07 Simon Fraser <simon.fra...@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 <ray...@chromium.org>
Incorrect rect-based hit-test result for culled-inline elements
Modified: trunk/Source/WebCore/page/FrameView.cpp (119735 => 119736)
--- trunk/Source/WebCore/page/FrameView.cpp 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/page/FrameView.cpp 2012-06-07 17:57:00 UTC (rev 119736)
@@ -1372,76 +1372,47 @@
}
}
-int FrameView::scrollXForFixedPosition() const
+static int fixedPositionScrollOffset(int scrollPosition, int maxValue, int scrollOrigin, float dragFactor)
{
- int visibleContentWidth = visibleContentRect().width();
- int maxX = contentsWidth() - visibleContentWidth;
-
- if (maxX == 0)
+ 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 (scrollPosition > 0)
+ scrollPosition = 0;
+ else if (scrollPosition < -maxValue)
+ scrollPosition = -maxValue;
}
-
- 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;
+
+ return scrollPosition * dragFactor;
}
-int FrameView::scrollYForFixedPosition() const
+IntSize FrameView::scrollOffsetForFixedPosition() const
{
- int visibleContentHeight = visibleContentRect().height();
+ 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;
- int maxY = contentsHeight() - visibleContentHeight;
- if (maxY == 0)
- return 0;
+ FloatSize dragFactor = fixedElementsLayoutRelativeToFrame() ? FloatSize(1, 1) : FloatSize(
+ (contentsSize.width() - visibleContentRect.width() * frameScaleFactor) / maxOffset.width(),
+ (contentsSize.height() - visibleContentRect.height() * frameScaleFactor) / maxOffset.height());
- int y = scrollY();
+ int x = fixedPositionScrollOffset(scrollPosition.x(), maxOffset.width(), scrollOrigin.x(), dragFactor.width() / frameScaleFactor);
+ int y = fixedPositionScrollOffset(scrollPosition.y(), maxOffset.height(), scrollOrigin.y(), dragFactor.height() / frameScaleFactor);
- 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;
+ return IntSize(x, y);
}
-IntSize FrameView::scrollOffsetForFixedPosition() const
-{
- return IntSize(scrollXForFixedPosition(), scrollYForFixedPosition());
-}
-
bool FrameView::fixedElementsLayoutRelativeToFrame() const
{
ASSERT(m_frame);
Modified: trunk/Source/WebCore/page/FrameView.h (119735 => 119736)
--- trunk/Source/WebCore/page/FrameView.h 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/page/FrameView.h 2012-06-07 17:57:00 UTC (rev 119736)
@@ -200,8 +200,6 @@
// 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;
bool fixedElementsLayoutRelativeToFrame() const;
Modified: trunk/Source/WebCore/platform/ScrollView.cpp (119735 => 119736)
--- trunk/Source/WebCore/platform/ScrollView.cpp 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/platform/ScrollView.cpp 2012-06-07 17:57:00 UTC (rev 119736)
@@ -252,6 +252,11 @@
}
#endif
+IntSize ScrollView::layoutSize() const
+{
+ return m_fixedLayoutSize.isEmpty() || !m_useFixedLayout ? visibleSize() : m_fixedLayoutSize;
+}
+
int ScrollView::layoutWidth() const
{
return m_fixedLayoutSize.isEmpty() || !m_useFixedLayout ? visibleWidth() : m_fixedLayoutSize.width();
Modified: trunk/Source/WebCore/platform/ScrollView.h (119735 => 119736)
--- trunk/Source/WebCore/platform/ScrollView.h 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/platform/ScrollView.h 2012-06-07 17:57:00 UTC (rev 119736)
@@ -146,11 +146,13 @@
virtual IntRect visibleContentRect(bool includeScrollbars = false) const;
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(); }
// 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;
IntSize fixedLayoutSize() const;
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (119735 => 119736)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-06-07 17:57:00 UTC (rev 119736)
@@ -3902,7 +3902,7 @@
// 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;
}
Modified: trunk/Source/WebCore/rendering/RenderLayer.h (119735 => 119736)
--- trunk/Source/WebCore/rendering/RenderLayer.h 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/rendering/RenderLayer.h 2012-06-07 17:57:00 UTC (rev 119736)
@@ -111,6 +111,7 @@
m_hasRadius = true;
}
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(); }
bool intersects(const LayoutRect& rect) { return m_rect.intersects(rect); }
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (119735 => 119736)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2012-06-07 17:57:00 UTC (rev 119736)
@@ -1804,7 +1804,7 @@
// 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;
return true;
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (119735 => 119736)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2012-06-07 17:44:50 UTC (rev 119735)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2012-06-07 17:57:00 UTC (rev 119736)
@@ -390,7 +390,7 @@
}
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).
if (!repaintContainer && m_layer && m_layer->transform())
