Attachment 370978 Details for Bug 198363 – Patch

[patch] Patch

bug-198363-20190530141934.patch (text/plain), 12.74 KB, created by Simon Fraser (smfr) on 2019-05-30 14:19:34 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-05-30 14:19:34 PDT

Size: 12.74 KB

patch

obsolete

>Subversion Revision: 245863
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b9caceeec616f0f7836228566437c414f6c6a518..4afd3b1616e7a632276c0cb2b778c944f4422b57 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-05-29  Simon Fraser  <simon.fraser@apple.com>
>+
>+        [Async overflow scrolling] Flashes of missing layer backing store when scrolling an overflow
>+        https://bugs.webkit.org/show_bug.cgi?id=198363
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When the contents of an overflow:scroll did not use a tiled backing layer, GraphicsLayerCA::adjustCoverageRect()
>+        would do no coverage rect expansion for scrolling, which meant that backing store attachment for
>+        descendant layers would just use the visible rect from their scrolling ancestor which made it easy
>+        to scroll into view a layer whose backing store was not yet attached.
>+        
>+        Since this only affects non-tiled layers, re-use the generic TileController::adjustTileCoverageRect()
>+        code by moving it down to GraphicsLayer, and call it for a scrolled contents layer which does not
>+        have tiled backing.
>+        
>+        Tested by fast/scrolling/ios/reconcile-layer-position-recursive.html
>+
>+        * platform/graphics/GraphicsLayer.cpp:
>+        (WebCore::GraphicsLayer::adjustCoverageRectForMovement):
>+        * platform/graphics/GraphicsLayer.h:
>+        * platform/graphics/ca/GraphicsLayerCA.cpp:
>+        (WebCore::GraphicsLayerCA::adjustCoverageRect const):
>+        * platform/graphics/ca/TileController.cpp:
>+        (WebCore::TileController::adjustTileCoverageRect):
>+
> 2019-05-29  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r245857.
>diff --git a/Source/WebCore/platform/graphics/GraphicsLayer.cpp b/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>index 7342e44b4f2523d99664574d6cd5aeea9c2d1295..f9daf9a97bdff10a81383b8f4be22c677f69ea80 100644
>--- a/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>+++ b/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>@@ -502,6 +502,64 @@ void GraphicsLayer::paintGraphicsLayerContents(GraphicsContext& context, const F
>     client().paintContents(this, context, m_paintingPhase, clipRect, layerPaintBehavior);
> }
> 
>+FloatRect GraphicsLayer::adjustCoverageRectForMovement(const FloatRect& coverageRect, const FloatRect& previousVisibleRect, const FloatRect& currentVisibleRect)
>+{
>+    // If the old visible rect is empty, we have no information about how the visible area is changing
>+    // (maybe the layer was just created), so don't attempt to expand. Also don't attempt to expand if the rects don't overlap.
>+    if (previousVisibleRect.isEmpty() || !currentVisibleRect.intersects(previousVisibleRect))
>+        return unionRect(coverageRect, currentVisibleRect);
>+
>+    const float paddingMultiplier = 2;
>+
>+    float leftEdgeDelta = paddingMultiplier * (currentVisibleRect.x() - previousVisibleRect.x());
>+    float rightEdgeDelta = paddingMultiplier * (currentVisibleRect.maxX() - previousVisibleRect.maxX());
>+
>+    float topEdgeDelta = paddingMultiplier * (currentVisibleRect.y() - previousVisibleRect.y());
>+    float bottomEdgeDelta = paddingMultiplier * (currentVisibleRect.maxY() - previousVisibleRect.maxY());
>+    
>+    FloatRect expandedRect = currentVisibleRect;
>+
>+    // More exposed on left side.
>+    if (leftEdgeDelta < 0) {
>+        float newLeft = expandedRect.x() + leftEdgeDelta;
>+        // Pad to the left, but don't reduce padding that's already in the backing store (since we're still exposing to the left).
>+        if (newLeft < previousVisibleRect.x())
>+            expandedRect.shiftXEdgeTo(newLeft);
>+        else
>+            expandedRect.shiftXEdgeTo(previousVisibleRect.x());
>+    }
>+
>+    // More exposed on right.
>+    if (rightEdgeDelta > 0) {
>+        float newRight = expandedRect.maxX() + rightEdgeDelta;
>+        // Pad to the right, but don't reduce padding that's already in the backing store (since we're still exposing to the right).
>+        if (newRight > previousVisibleRect.maxX())
>+            expandedRect.setWidth(newRight - expandedRect.x());
>+        else
>+            expandedRect.setWidth(previousVisibleRect.maxX() - expandedRect.x());
>+    }
>+
>+    // More exposed at top.
>+    if (topEdgeDelta < 0) {
>+        float newTop = expandedRect.y() + topEdgeDelta;
>+        if (newTop < previousVisibleRect.y())
>+            expandedRect.shiftYEdgeTo(newTop);
>+        else
>+            expandedRect.shiftYEdgeTo(previousVisibleRect.y());
>+    }
>+
>+    // More exposed on bottom.
>+    if (bottomEdgeDelta > 0) {
>+        float newBottom = expandedRect.maxY() + bottomEdgeDelta;
>+        if (newBottom > previousVisibleRect.maxY())
>+            expandedRect.setHeight(newBottom - expandedRect.y());
>+        else
>+            expandedRect.setHeight(previousVisibleRect.maxY() - expandedRect.y());
>+    }
>+    
>+    return unionRect(coverageRect, expandedRect);
>+}
>+
> String GraphicsLayer::animationNameForTransition(AnimatedPropertyID property)
> {
>     // | is not a valid identifier character in CSS, so this can never conflict with a keyframe identifier.
>diff --git a/Source/WebCore/platform/graphics/GraphicsLayer.h b/Source/WebCore/platform/graphics/GraphicsLayer.h
>index 7e16d9e51fd2c9122c69283e333c9fac26f26b31..b666bf7c22cf6a980e60adef9a6643d60b2dc526 100644
>--- a/Source/WebCore/platform/graphics/GraphicsLayer.h
>+++ b/Source/WebCore/platform/graphics/GraphicsLayer.h
>@@ -576,6 +576,8 @@ public:
>     // for example to allocate new tiles.
>     virtual bool visibleRectChangeRequiresFlush(const FloatRect& /* clipRect */) const { return false; }
> 
>+    static FloatRect adjustCoverageRectForMovement(const FloatRect& coverageRect, const FloatRect& previousVisibleRect, const FloatRect& currentVisibleRect);
>+
>     // Return a string with a human readable form of the layer tree, If debug is true
>     // pointers for the layers and timing data will be included in the returned string.
>     WEBCORE_EXPORT String layerTreeAsText(LayerTreeAsTextBehavior = LayerTreeAsTextBehaviorNormal) const;
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>index a81ff694dc9594caf0e105b651a69180a6537ab4..33a33d4a7d0e533cd23807df08c789aa4f50fdc9 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>@@ -1450,6 +1450,10 @@ bool GraphicsLayerCA::adjustCoverageRect(VisibleAndCoverageRects& rects, const F
>     case Type::ScrolledContents:
>         if (m_layer->usesTiledBackingLayer())
>             coverageRect = tiledBacking()->adjustTileCoverageRectForScrolling(coverageRect, size(), oldVisibleRect, rects.visibleRect, pageScaleFactor() * deviceScaleFactor());
>+        else {
>+            // Even if we don't have tiled backing, we want to expand coverage so that contained layers get attached backing store.
>+            coverageRect = adjustCoverageRectForMovement(coverageRect, oldVisibleRect, rects.visibleRect);
>+        }
>         break;
>     case Type::Normal:
>         if (m_layer->usesTiledBackingLayer())
>diff --git a/Source/WebCore/platform/graphics/ca/TileController.cpp b/Source/WebCore/platform/graphics/ca/TileController.cpp
>index 7ffef81132b8fbadbf937c23b7e0ad3f9449c67a..75fe759a8eed32ac3ace87fbf860e12224071855 100644
>--- a/Source/WebCore/platform/graphics/ca/TileController.cpp
>+++ b/Source/WebCore/platform/graphics/ca/TileController.cpp
>@@ -28,6 +28,7 @@
> 
> #if USE(CG)
> 
>+#include "GraphicsLayer.h"
> #include "IntRect.h"
> #include "Logging.h"
> #include "PlatformCALayer.h"
>@@ -364,65 +365,11 @@ IntRect TileController::boundsAtLastRevalidateWithoutMargin() const
> 
> FloatRect TileController::adjustTileCoverageRect(const FloatRect& coverageRect, const FloatRect& previousVisibleRect, const FloatRect& currentVisibleRect, bool sizeChanged)
> {
>-    // If the old visible rect is empty, we have no information about how the visible area is changing
>-    // (maybe the layer was just created), so don't attempt to expand. Also don't attempt to expand
>-    // if the size changed or the rects don't overlap.
>-    if (previousVisibleRect.isEmpty() || sizeChanged  || !currentVisibleRect.intersects(previousVisibleRect))
>+    if (sizeChanged || MemoryPressureHandler::singleton().isUnderMemoryPressure())
>         return unionRect(coverageRect, currentVisibleRect);
> 
>-    if (MemoryPressureHandler::singleton().isUnderMemoryPressure())
>-        return unionRect(coverageRect, currentVisibleRect);
>-
>-    const float paddingMultiplier = 2;
>-
>-    float leftEdgeDelta = paddingMultiplier * (currentVisibleRect.x() - previousVisibleRect.x());
>-    float rightEdgeDelta = paddingMultiplier * (currentVisibleRect.maxX() - previousVisibleRect.maxX());
>-
>-    float topEdgeDelta = paddingMultiplier * (currentVisibleRect.y() - previousVisibleRect.y());
>-    float bottomEdgeDelta = paddingMultiplier * (currentVisibleRect.maxY() - previousVisibleRect.maxY());
>-    
>-    FloatRect expandedRect = currentVisibleRect;
>-
>-    // More exposed on left side.
>-    if (leftEdgeDelta < 0) {
>-        float newLeft = expandedRect.x() + leftEdgeDelta;
>-        // Pad to the left, but don't reduce padding that's already in the backing store (since we're still exposing to the left).
>-        if (newLeft < previousVisibleRect.x())
>-            expandedRect.shiftXEdgeTo(newLeft);
>-        else
>-            expandedRect.shiftXEdgeTo(previousVisibleRect.x());
>-    }
>-
>-    // More exposed on right.
>-    if (rightEdgeDelta > 0) {
>-        float newRight = expandedRect.maxX() + rightEdgeDelta;
>-        // Pad to the right, but don't reduce padding that's already in the backing store (since we're still exposing to the right).
>-        if (newRight > previousVisibleRect.maxX())
>-            expandedRect.setWidth(newRight - expandedRect.x());
>-        else
>-            expandedRect.setWidth(previousVisibleRect.maxX() - expandedRect.x());
>-    }
>-
>-    // More exposed at top.
>-    if (topEdgeDelta < 0) {
>-        float newTop = expandedRect.y() + topEdgeDelta;
>-        if (newTop < previousVisibleRect.y())
>-            expandedRect.shiftYEdgeTo(newTop);
>-        else
>-            expandedRect.shiftYEdgeTo(previousVisibleRect.y());
>-    }
>-
>-    // More exposed on bottom.
>-    if (bottomEdgeDelta > 0) {
>-        float newBottom = expandedRect.maxY() + bottomEdgeDelta;
>-        if (newBottom > previousVisibleRect.maxY())
>-            expandedRect.setHeight(newBottom - expandedRect.y());
>-        else
>-            expandedRect.setHeight(previousVisibleRect.maxY() - expandedRect.y());
>-    }
>-    
>-    expandedRect.intersect(boundsWithoutMargin());
>-    return unionRect(coverageRect, expandedRect);
>+    auto expandedCoverageRect = GraphicsLayer::adjustCoverageRectForMovement(coverageRect, previousVisibleRect, currentVisibleRect);
>+    return intersection(expandedCoverageRect, boundsWithoutMargin());
> }
> 
> #if !PLATFORM(IOS_FAMILY)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1e9023737da75e99b4089d4e389e40df6618e678..9836b409798e33acc3f748f4932294c676e056f2 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-05-30  Simon Fraser  <simon.fraser@apple.com>
>+
>+        [Async overflow scrolling] Flashes of missing layer backing store when scrolling an overflow
>+        https://bugs.webkit.org/show_bug.cgi?id=198363
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Reset results.
>+
>+        * fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt:
>+
> 2019-05-29  Antti Koivisto  <antti@apple.com>
> 
>         Scrolling node ordering wrong when a layer has both positioning and fixed/sticky node
>diff --git a/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt b/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt
>index fb10f4edc2fa32bb0f61038bad8afbd319fa243f..be96bc5892c6230ae1ee11543b4ad747e9e1ed9a 100644
>--- a/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt
>+++ b/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt
>@@ -39,7 +39,7 @@
>                   (bounds 300.00 510.00)
>                   (drawsContent 1)
>                   (visible rect 0.00, 13.00 300.00 x 287.00)
>-                  (coverage rect 0.00, 13.00 300.00 x 287.00)
>+                  (coverage rect 0.00, 12.00 300.00 x 288.00)
>                   (intersects coverage rect 1)
>                   (contentsScale 2.00)
>                   (children 1
>@@ -48,7 +48,7 @@
>                       (bounds 210.00 410.00)
>                       (drawsContent 1)
>                       (visible rect 0.00, 0.00 210.00 x 250.00)
>-                      (coverage rect -50.00, -37.00 300.00 x 287.00)
>+                      (coverage rect -50.00, -38.00 300.00 x 288.00)
>                       (intersects coverage rect 1)
>                       (contentsScale 2.00)
>                       (children 1

Flags:

thorton: review+
commit-queue: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 198363: 370910 | 370965 | 370970 | 370978 | 371005