- Revision
- 164952
- Author
- [email protected]
- Date
- 2014-03-02 15:13:46 -0800 (Sun, 02 Mar 2014)
Log Message
[iOS][WK2] Pages using tiled compositing layers allocate too many tiles on zoom
https://bugs.webkit.org/show_bug.cgi?id=129471
Patch by Benjamin Poulain <[email protected]> on 2014-03-02
Reviewed by Simon Fraser.
Source/WebCore:
A few issues with TileController were causing sublayers of the root layers
to tile incorrect surfaces on zoom.
First, the exposedRect API was not updating the sublayers. The layers go correctly
into tiling mode, but the tiles cover the full document instead of the visible area.
The other major issue was the margins being applied to the coverage size in document
coordinates. Since each margin is 512px, the total coverage size after zoom was
gigantic.
To solve this, this patch switch from the exposedRect API to the generic concept
of VisibleExtentContentRect introduced for iOS WebKit1.
* WebCore.exp.in:
* platform/ScrollView.h:
Define a VisibleExtentContentRect on the scrollview itself when there is no
platformWidget().
The case with inside frame is untested due to stability issues :(.
(see <rdar://problem/16199219>)
* platform/graphics/ca/mac/TileController.mm:
(WebCore::TileController::computeTileCoverageRect):
Remove the margin from the tile coverage.
On iOS, m_tileCoverage is always zero at the moment. Previously, the tile coverage
was artificially extended by the margins. With the margins removed, I temporarily added
a factor of 1.5.
ViewUpdateDispatcher has all the information to compute a great tile coverage, I will need
a follow up patch to fix that.
* platform/ios/ScrollViewIOS.mm:
(WebCore::ScrollView::visibleExtentContentRect):
(WebCore::ScrollView::setVisibleExtentContentRect):
Source/WebKit2:
* WebProcess/WebPage/DrawingArea.h:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::updateVisibleContentRects):
A few interesting changes here:
-Now that we do not use setExposedRect, we can pass the exposed area directly
to the drawing area since everything is now in content coordinates :)
-The scale is now converted to float before being compared to the Page's scaleFactor.
The page's scalefactor being a float, the comparison was failing most of the time.
* WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h:
* WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::setVisibleExtentContentRect):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (164951 => 164952)
--- trunk/Source/WebCore/ChangeLog 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebCore/ChangeLog 2014-03-02 23:13:46 UTC (rev 164952)
@@ -1,3 +1,44 @@
+2014-03-02 Benjamin Poulain <[email protected]>
+
+ [iOS][WK2] Pages using tiled compositing layers allocate too many tiles on zoom
+ https://bugs.webkit.org/show_bug.cgi?id=129471
+
+ Reviewed by Simon Fraser.
+
+ A few issues with TileController were causing sublayers of the root layers
+ to tile incorrect surfaces on zoom.
+
+ First, the exposedRect API was not updating the sublayers. The layers go correctly
+ into tiling mode, but the tiles cover the full document instead of the visible area.
+
+ The other major issue was the margins being applied to the coverage size in document
+ coordinates. Since each margin is 512px, the total coverage size after zoom was
+ gigantic.
+
+ To solve this, this patch switch from the exposedRect API to the generic concept
+ of VisibleExtentContentRect introduced for iOS WebKit1.
+
+ * WebCore.exp.in:
+ * platform/ScrollView.h:
+ Define a VisibleExtentContentRect on the scrollview itself when there is no
+ platformWidget().
+ The case with inside frame is untested due to stability issues :(.
+ (see <rdar://problem/16199219>)
+
+ * platform/graphics/ca/mac/TileController.mm:
+ (WebCore::TileController::computeTileCoverageRect):
+ Remove the margin from the tile coverage.
+
+ On iOS, m_tileCoverage is always zero at the moment. Previously, the tile coverage
+ was artificially extended by the margins. With the margins removed, I temporarily added
+ a factor of 1.5.
+ ViewUpdateDispatcher has all the information to compute a great tile coverage, I will need
+ a follow up patch to fix that.
+
+ * platform/ios/ScrollViewIOS.mm:
+ (WebCore::ScrollView::visibleExtentContentRect):
+ (WebCore::ScrollView::setVisibleExtentContentRect):
+
2014-03-02 Darin Adler <[email protected]>
Sort Mac platform export files so they merge better
Modified: trunk/Source/WebCore/WebCore.exp.in (164951 => 164952)
--- trunk/Source/WebCore/WebCore.exp.in 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebCore/WebCore.exp.in 2014-03-02 23:13:46 UTC (rev 164952)
@@ -2376,6 +2376,7 @@
_WebThreadUnlockGuardForMail
__ZN7WebCore10FloatPointC1ERK7CGPoint
__ZN7WebCore10ScrollView15setScrollOffsetERKNS_8IntPointE
+__ZN7WebCore10ScrollView27setVisibleExtentContentRectERKNS_7IntRectE
__ZN7WebCore10XLinkNames4initEv
__ZN7WebCore10inSameLineERKNS_15VisiblePositionES2_
__ZN7WebCore11BidiContext41copyStackRemovingUnicodeEmbeddingContextsEv
Modified: trunk/Source/WebCore/platform/ScrollView.h (164951 => 164952)
--- trunk/Source/WebCore/platform/ScrollView.h 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebCore/platform/ScrollView.h 2014-03-02 23:13:46 UTC (rev 164952)
@@ -171,6 +171,7 @@
IntRect actualVisibleContentRect() const;
// This is the area that is partially or fully exposed, and may extend under overlapping UI elements.
IntRect visibleExtentContentRect() const;
+ void setVisibleExtentContentRect(const IntRect&);
void setActualScrollPosition(const IntPoint&);
TileCache* tileCache();
@@ -392,7 +393,11 @@
// whether it is safe to blit on scroll.
bool m_canBlitOnScroll;
-#if !PLATFORM(IOS)
+ // FIXME: visibleExtentContentRect is a very similar concept to fixedVisibleContentRect except it does not differentiate
+ // between exposed rect and unobscuredRects. The two attributes should eventually be merged.
+#if PLATFORM(IOS)
+ IntRect m_visibleExtentContentRect;
+#else
IntRect m_fixedVisibleContentRect;
#endif
IntSize m_scrollOffset; // FIXME: Would rather store this as a position, but we will wait to make this change until more code is shared.
Modified: trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm (164951 => 164952)
--- trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebCore/platform/graphics/ca/mac/TileController.mm 2014-03-02 23:13:46 UTC (rev 164952)
@@ -498,18 +498,28 @@
// FIXME: look at how far the document can scroll in each dimension.
float coverageHorizontalSize = visibleRect.width();
float coverageVerticalSize = visibleRect.height();
-
+
+#if PLATFORM(IOS)
+ // FIXME: ViewUpdateDispatcher could do an amazing job at computing this (<rdar://problem/16205290>).
+ // In the meantime, just default to something good enough.
+ if (!largeVisibleRectChange) {
+ coverageHorizontalSize *= 1.5;
+ coverageVerticalSize *= 1.5;
+ }
+#else
// Inflate the coverage rect so that it covers 2x of the visible width and 3x of the visible height.
// These values were chosen because it's more common to have tall pages and to scroll vertically,
// so we keep more tiles above and below the current area.
+
if (m_tileCoverage & CoverageForHorizontalScrolling && !largeVisibleRectChange)
coverageHorizontalSize *= 2;
if (m_tileCoverage & CoverageForVerticalScrolling && !largeVisibleRectChange)
coverageVerticalSize *= 3;
-
+
coverageVerticalSize += topMarginHeight() + bottomMarginHeight();
coverageHorizontalSize += leftMarginWidth() + rightMarginWidth();
+#endif
FloatRect coverageBounds = bounds();
float coverageLeft = visibleRect.x() - (coverageHorizontalSize - visibleRect.width()) / 2;
Modified: trunk/Source/WebCore/platform/ios/ScrollViewIOS.mm (164951 => 164952)
--- trunk/Source/WebCore/platform/ios/ScrollViewIOS.mm 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebCore/platform/ios/ScrollViewIOS.mm 2014-03-02 23:13:46 UTC (rev 164952)
@@ -108,24 +108,36 @@
IntRect ScrollView::visibleExtentContentRect() const
{
- NSScrollView *view = static_cast<NSScrollView *>(platformWidget());
+ if (NSScrollView *view = static_cast<NSScrollView *>(platformWidget())) {
+ CGRect r = CGRectZero;
+ BEGIN_BLOCK_OBJC_EXCEPTIONS;
+ if ([view isKindOfClass:[NSScrollView class]])
+ r = [view documentVisibleExtent];
+ else {
+ r.origin = [view visibleRect].origin;
+ r.size = [view bounds].size;
+ }
- CGRect r = CGRectZero;
- BEGIN_BLOCK_OBJC_EXCEPTIONS;
- if ([view isKindOfClass:[NSScrollView class]])
- r = [view documentVisibleExtent];
- else if (view) {
- r.origin = [view visibleRect].origin;
- r.size = [view bounds].size;
- } else {
- // FIXME: WebKit2 on iOS doesn't inform the WebProcess of the exposed area.
- return IntRect(IntPoint(), contentsSize());
+ END_BLOCK_OBJC_EXCEPTIONS;
+ return enclosingIntRect(r);
}
- END_BLOCK_OBJC_EXCEPTIONS;
- return enclosingIntRect(r);
+ const ScrollView* parent = this->parent();
+ if (!parent)
+ return m_visibleExtentContentRect;
+
+ IntRect parentViewExtentContentRect = parent->visibleExtentContentRect();
+ IntRect selfExtentContentRect = rootViewToContents(parentViewExtentContentRect);
+ selfExtentContentRect.intersect(boundsRect());
+ return selfExtentContentRect;
}
+void ScrollView::setVisibleExtentContentRect(const IntRect& rect)
+{
+ ASSERT(!platformWidget());
+ m_visibleExtentContentRect = rect;
+}
+
void ScrollView::setActualScrollPosition(const IntPoint& position)
{
NSScrollView *view = static_cast<NSScrollView *>(platformWidget());
Modified: trunk/Source/WebKit2/ChangeLog (164951 => 164952)
--- trunk/Source/WebKit2/ChangeLog 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebKit2/ChangeLog 2014-03-02 23:13:46 UTC (rev 164952)
@@ -1,3 +1,23 @@
+2014-03-02 Benjamin Poulain <[email protected]>
+
+ [iOS][WK2] Pages using tiled compositing layers allocate too many tiles on zoom
+ https://bugs.webkit.org/show_bug.cgi?id=129471
+
+ Reviewed by Simon Fraser.
+
+ * WebProcess/WebPage/DrawingArea.h:
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::updateVisibleContentRects):
+ A few interesting changes here:
+ -Now that we do not use setExposedRect, we can pass the exposed area directly
+ to the drawing area since everything is now in content coordinates :)
+ -The scale is now converted to float before being compared to the Page's scaleFactor.
+ The page's scalefactor being a float, the comparison was failing most of the time.
+
+ * WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h:
+ * WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:
+ (WebKit::RemoteLayerTreeDrawingArea::setVisibleExtentContentRect):
+
2014-03-02 Darin Adler <[email protected]>
Change public text iterator API implementations to not depend on 16-bit character pointers
Modified: trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.h (164951 => 164952)
--- trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.h 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.h 2014-03-02 23:13:46 UTC (rev 164952)
@@ -93,6 +93,9 @@
virtual WebCore::FloatRect exposedRect() const = 0;
virtual void setCustomFixedPositionRect(const WebCore::FloatRect&) = 0;
#endif
+#if PLATFORM(IOS)
+ virtual void setVisibleExtentContentRect(const WebCore::FloatRect&) = 0;
+#endif
virtual void mainFrameScrollabilityChanged(bool) { }
virtual bool supportsAsyncScrolling() { return false; }
Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (164951 => 164952)
--- trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm 2014-03-02 23:13:46 UTC (rev 164952)
@@ -1642,20 +1642,19 @@
void WebPage::updateVisibleContentRects(const VisibleContentRectUpdateInfo& visibleContentRectUpdateInfo)
{
- double boundedScale = std::min(m_viewportConfiguration.maximumScale(), std::max(m_viewportConfiguration.minimumScale(), visibleContentRectUpdateInfo.scale()));
-
- IntPoint scrollPosition = roundedIntPoint(visibleContentRectUpdateInfo.unobscuredRect().location());
-
FloatRect exposedRect = visibleContentRectUpdateInfo.exposedRect();
- exposedRect.scale(boundedScale);
- m_drawingArea->setExposedRect(exposedRect);
+ m_drawingArea->setVisibleExtentContentRect(enclosingIntRect(exposedRect));
- if (boundedScale != m_page->pageScaleFactor()) {
+ double boundedScale = std::min(m_viewportConfiguration.maximumScale(), std::max(m_viewportConfiguration.minimumScale(), visibleContentRectUpdateInfo.scale()));
+ float floatBoundedScale = boundedScale;
+ if (floatBoundedScale != m_page->pageScaleFactor()) {
m_scaleWasSetByUIProcess = true;
- m_page->setPageScaleFactor(boundedScale, scrollPosition);
+
+ IntPoint scrollPosition = roundedIntPoint(visibleContentRectUpdateInfo.unobscuredRect().location());
+ m_page->setPageScaleFactor(floatBoundedScale, scrollPosition);
if (m_drawingArea->layerTreeHost())
m_drawingArea->layerTreeHost()->deviceOrPageScaleFactorChanged();
- send(Messages::WebPageProxy::PageScaleFactorDidChange(boundedScale));
+ send(Messages::WebPageProxy::PageScaleFactorDidChange(floatBoundedScale));
}
// FIXME: we should also update the frame view from unobscured rect. Altenatively, we can have it pull the values from ScrollView.
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h (164951 => 164952)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.h 2014-03-02 23:13:46 UTC (rev 164952)
@@ -75,6 +75,10 @@
virtual void setExposedRect(const WebCore::FloatRect&) override;
virtual WebCore::FloatRect exposedRect() const override { return m_scrolledExposedRect; }
+#if PLATFORM(IOS)
+ virtual void setVisibleExtentContentRect(const WebCore::FloatRect&) override;
+#endif
+
virtual void setCustomFixedPositionRect(const WebCore::FloatRect&) override;
// WebCore::GraphicsLayerClient
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm (164951 => 164952)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm 2014-03-02 22:35:36 UTC (rev 164951)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm 2014-03-02 23:13:46 UTC (rev 164952)
@@ -246,6 +246,18 @@
updateScrolledExposedRect();
}
+#if PLATFORM(IOS)
+void RemoteLayerTreeDrawingArea::setVisibleExtentContentRect(const FloatRect& visibleExtentContentRect)
+{
+ FrameView* frameView = m_webPage->corePage()->mainFrame().view();
+ if (!frameView)
+ return;
+
+ frameView->setVisibleExtentContentRect(enclosingIntRect(visibleExtentContentRect));
+ scheduleCompositingLayerFlush();
+}
+#endif
+
void RemoteLayerTreeDrawingArea::updateScrolledExposedRect()
{
FrameView* frameView = m_webPage->corePage()->mainFrame().view();
