UIWebView is not rendering content that comes on screen during overflow scroll
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Nov 2017 21:41:12 +0000 (21:41 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Nov 2017 21:41:12 +0000 (21:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179277
rdar://problem/34272949

Reviewed by Tim Horton

When page or overflow scrolling happens, we do a traversal of GraphicsLayers to determine
whether the exposed part of tiled layers changed in a way that requires a change in the tile
coverage. If so, we schedule a compositing layer flush.

There was no equivalent logic for computing whether the "backing store detached" state
of a layer changed (which we use to throw away backing store of layers outside the viewport),
so after scrolling an accelerated overflow:scroll which contained composited layers, we
would sometimes fail to recompute that we should re-create backing store for revealed
layers.

Fix by having GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush() determine
whether 'intersectsCoverageRect' changed, and if so trigger a flush. This requires
tracking CommitState for isViewportConstrained-ness, just like we do during commits.

Also clean up code related to computing the visible rect passed into visibleRectChangeRequiresFlush() and
flushCompositingState(); these diverged for no good reason. Also clean up the logging a little.

Not testable because UIWebView layout tests are unreliable.

* page/ios/FrameIOS.mm:
(WebCore::Frame::viewportOffsetChanged):
(WebCore::Frame::overflowScrollPositionChangedForNode):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush const):
(WebCore::GraphicsLayerCA::visibleRectChangeRequiresFlush const):
* platform/graphics/ca/GraphicsLayerCA.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::visibleRectForLayerFlushing const):
(WebCore::RenderLayerCompositor::flushPendingLayerChanges):
(WebCore::RenderLayerCompositor::didChangeVisibleRect):
* rendering/RenderLayerCompositor.h:

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

Source/WebCore/ChangeLog
Source/WebCore/page/ios/FrameIOS.mm
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index c1e297c..ca614e9 100644 (file)
@@ -1,3 +1,43 @@
+2017-11-03  Simon Fraser  <simon.fraser@apple.com>
+
+        UIWebView is not rendering content that comes on screen during overflow scroll
+        https://bugs.webkit.org/show_bug.cgi?id=179277
+        rdar://problem/34272949
+
+        Reviewed by Tim Horton
+
+        When page or overflow scrolling happens, we do a traversal of GraphicsLayers to determine
+        whether the exposed part of tiled layers changed in a way that requires a change in the tile
+        coverage. If so, we schedule a compositing layer flush.
+
+        There was no equivalent logic for computing whether the "backing store detached" state
+        of a layer changed (which we use to throw away backing store of layers outside the viewport),
+        so after scrolling an accelerated overflow:scroll which contained composited layers, we
+        would sometimes fail to recompute that we should re-create backing store for revealed
+        layers.
+
+        Fix by having GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush() determine
+        whether 'intersectsCoverageRect' changed, and if so trigger a flush. This requires
+        tracking CommitState for isViewportConstrained-ness, just like we do during commits.
+
+        Also clean up code related to computing the visible rect passed into visibleRectChangeRequiresFlush() and
+        flushCompositingState(); these diverged for no good reason. Also clean up the logging a little.
+        
+        Not testable because UIWebView layout tests are unreliable.
+
+        * page/ios/FrameIOS.mm:
+        (WebCore::Frame::viewportOffsetChanged):
+        (WebCore::Frame::overflowScrollPositionChangedForNode):
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush const):
+        (WebCore::GraphicsLayerCA::visibleRectChangeRequiresFlush const):
+        * platform/graphics/ca/GraphicsLayerCA.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::visibleRectForLayerFlushing const):
+        (WebCore::RenderLayerCompositor::flushPendingLayerChanges):
+        (WebCore::RenderLayerCompositor::didChangeVisibleRect):
+        * rendering/RenderLayerCompositor.h:
+
 2017-11-06  Chris Dumez  <cdumez@apple.com>
 
         [Service Workers] Add proper implementation for 'updatefound' event
index 52400f4..3802278 100644 (file)
@@ -47,6 +47,7 @@
 #import "HTMLObjectElement.h"
 #import "HitTestRequest.h"
 #import "HitTestResult.h"
+#import "Logging.h"
 #import "MainFrame.h"
 #import "NodeRenderStyle.h"
 #import "NodeTraversal.h"
@@ -67,6 +68,7 @@
 #import "WAKWindow.h"
 #import <runtime/JSLock.h>
 #import <wtf/BlockObjCExceptions.h>
+#import <wtf/text/TextStream.h>
 
 using namespace WebCore::HTMLNames;
 using namespace WTF::Unicode;
@@ -708,6 +710,8 @@ static bool anyFrameHasTiledLayers(Frame* rootFrame)
 
 void Frame::viewportOffsetChanged(ViewportOffsetChangeType changeType)
 {
+    LOG_WITH_STREAM(Scrolling, stream << "Frame::viewportOffsetChanged - " << (changeType == IncrementalScrollOffset ? "incremental" : "completed"));
+
     if (changeType == IncrementalScrollOffset) {
         if (anyFrameHasTiledLayers(this)) {
             if (RenderView* root = contentRenderer())
@@ -731,6 +735,8 @@ bool Frame::containsTiledBackingLayers() const
 
 void Frame::overflowScrollPositionChangedForNode(const IntPoint& position, Node* node, bool isUserScroll)
 {
+    LOG_WITH_STREAM(Scrolling, stream << "Frame::overflowScrollPositionChangedForNode " << node << " position " << position);
+
     RenderObject* renderer = node->renderer();
     if (!renderer || !renderer->hasLayer())
         return;
index 6e15f04..dafb2a0 100644 (file)
@@ -1235,15 +1235,23 @@ static inline bool accumulatesTransform(const GraphicsLayerCA& layer)
     return !layer.masksToBounds() && (layer.preserves3D() || (layer.parent() && layer.parent()->preserves3D()));
 }
 
-bool GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush(const TransformState& state) const
+bool GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush(const CommitState& commitState, const TransformState& state) const
 {
     TransformState localState = state;
-    
+    CommitState childCommitState = commitState;
+
     // This may be called at times when layout has not been updated, so we want to avoid calling out to the client
     // for animating transforms.
     VisibleAndCoverageRects rects = computeVisibleAndCoverageRect(localState, accumulatesTransform(*this), 0);
     adjustCoverageRect(rects, m_visibleRect);
 
+    auto bounds = FloatRect(m_boundsOrigin, size());
+
+    bool isViewportConstrained = m_isViewportConstrained || commitState.ancestorIsViewportConstrained;
+    bool intersectsCoverageRect = isViewportConstrained || rects.coverageRect.intersects(bounds);
+    if (intersectsCoverageRect != m_intersectsCoverageRect)
+        return true;
+
     if (rects.coverageRect != m_coverageRect) {
         if (TiledBacking* tiledBacking = this->tiledBacking()) {
             if (tiledBacking->tilesWouldChangeForCoverageRect(rects.coverageRect))
@@ -1251,9 +1259,11 @@ bool GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush(const TransformSta
         }
     }
 
+    childCommitState.ancestorIsViewportConstrained |= m_isViewportConstrained;
+
     if (m_maskLayer) {
         GraphicsLayerCA& maskLayerCA = downcast<GraphicsLayerCA>(*m_maskLayer);
-        if (maskLayerCA.recursiveVisibleRectChangeRequiresFlush(localState))
+        if (maskLayerCA.recursiveVisibleRectChangeRequiresFlush(childCommitState, localState))
             return true;
     }
 
@@ -1262,12 +1272,12 @@ bool GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush(const TransformSta
     
     for (size_t i = 0; i < numChildren; ++i) {
         GraphicsLayerCA& currentChild = downcast<GraphicsLayerCA>(*childLayers[i]);
-        if (currentChild.recursiveVisibleRectChangeRequiresFlush(localState))
+        if (currentChild.recursiveVisibleRectChangeRequiresFlush(childCommitState, localState))
             return true;
     }
 
     if (m_replicaLayer)
-        if (downcast<GraphicsLayerCA>(*m_replicaLayer).recursiveVisibleRectChangeRequiresFlush(localState))
+        if (downcast<GraphicsLayerCA>(*m_replicaLayer).recursiveVisibleRectChangeRequiresFlush(childCommitState, localState))
             return true;
     
     return false;
@@ -1276,7 +1286,8 @@ bool GraphicsLayerCA::recursiveVisibleRectChangeRequiresFlush(const TransformSta
 bool GraphicsLayerCA::visibleRectChangeRequiresFlush(const FloatRect& clipRect) const
 {
     TransformState state(TransformState::UnapplyInverseTransformDirection, FloatQuad(clipRect));
-    return recursiveVisibleRectChangeRequiresFlush(state);
+    CommitState commitState;
+    return recursiveVisibleRectChangeRequiresFlush(commitState, state);
 }
 
 TiledBacking* GraphicsLayerCA::tiledBacking() const
index 9860780..22df585 100644 (file)
@@ -314,7 +314,7 @@ private:
     
     static FloatRect adjustTiledLayerVisibleRect(TiledBacking*, const FloatRect& oldVisibleRect, const FloatRect& newVisibleRect, const FloatSize& oldSize, const FloatSize& newSize);
 
-    bool recursiveVisibleRectChangeRequiresFlush(const TransformState&) const;
+    bool recursiveVisibleRectChangeRequiresFlush(const CommitState&, const TransformState&) const;
     
     bool isPageTiledBackingLayer() const { return type() == Type::PageTiledBacking; }
 
index f9f4c4f..18bd410 100644 (file)
@@ -412,6 +412,22 @@ void RenderLayerCompositor::scheduleLayerFlush(bool canThrottle)
     scheduleLayerFlushNow();
 }
 
+FloatRect RenderLayerCompositor::visibleRectForLayerFlushing() const
+{
+    const FrameView& frameView = m_renderView.frameView();
+#if PLATFORM(IOS)
+    return frameView.exposedContentRect();
+#else
+    // Having a m_clipLayer indicates that we're doing scrolling via GraphicsLayers.
+    FloatRect visibleRect = m_clipLayer ? FloatRect({ }, frameView.sizeForVisibleContent()) : frameView.visibleContentRect();
+
+    if (frameView.viewExposedRect())
+        visibleRect.intersect(frameView.viewExposedRect().value());
+
+    return visibleRect;
+#endif
+}
+
 void RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot)
 {
     // FrameView::flushCompositingStateIncludingSubframes() flushes each subframe,
@@ -436,23 +452,9 @@ void RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot)
     m_flushingLayers = true;
 
     if (auto* rootLayer = rootGraphicsLayer()) {
-#if PLATFORM(IOS)
-        FloatRect exposedRect = frameView.exposedContentRect();
-        LOG_WITH_STREAM(Compositing, stream << "\nRenderLayerCompositor " << this << " flushPendingLayerChanges (is root " << isFlushRoot << ") exposedRect " << exposedRect);
-
-        // FIXME: Use optimized flushes.
-        rootLayer->flushCompositingState(exposedRect);
-#else
-        // Having a m_clipLayer indicates that we're doing scrolling via GraphicsLayers.
-        FloatRect visibleRect = m_clipLayer ? FloatRect({ 0, 0 }, frameView.sizeForVisibleContent()) : frameView.visibleContentRect();
-
-        if (frameView.viewExposedRect())
-            visibleRect.intersect(frameView.viewExposedRect().value());
-
+        FloatRect visibleRect = visibleRectForLayerFlushing();
         LOG_WITH_STREAM(Compositing,  stream << "\nRenderLayerCompositor " << this << " flushPendingLayerChanges (is root " << isFlushRoot << ") visible rect " << visibleRect);
         rootLayer->flushCompositingState(visibleRect);
-        LOG_WITH_STREAM(Compositing,  stream << "RenderLayerCompositor " << this << " flush complete\n");
-#endif
     }
     
     ASSERT(m_flushingLayers);
@@ -559,16 +561,11 @@ void RenderLayerCompositor::didChangeVisibleRect()
     if (!rootLayer)
         return;
 
-    const FrameView& frameView = m_renderView.frameView();
-
-#if PLATFORM(IOS)
-    IntRect visibleRect = enclosingIntRect(frameView.exposedContentRect());
-#else
-    IntRect visibleRect = m_clipLayer ? IntRect(IntPoint(), frameView.contentsSize()) : frameView.visibleContentRect();
-#endif
-    if (!rootLayer->visibleRectChangeRequiresFlush(visibleRect))
-        return;
-    scheduleLayerFlushNow();
+    FloatRect visibleRect = visibleRectForLayerFlushing();
+    bool requiresFlush = rootLayer->visibleRectChangeRequiresFlush(visibleRect);
+    LOG_WITH_STREAM(Compositing, stream << "RenderLayerCompositor::didChangeVisibleRect " << visibleRect << " requiresFlush " << requiresFlush);
+    if (requiresFlush)
+        scheduleLayerFlushNow();
 }
 
 void RenderLayerCompositor::notifyFlushBeforeDisplayRefresh(const GraphicsLayer*)
index ac26b22..024d6d0 100644 (file)
@@ -407,6 +407,8 @@ private:
     bool isFlushingLayers() const { return m_flushingLayers; }
     void updateScrollCoordinatedLayersAfterFlushIncludingSubframes();
     void updateScrollCoordinatedLayersAfterFlush();
+
+    FloatRect visibleRectForLayerFlushing() const;
     
     Page& page() const;