Attachment 371129 Details for Bug 196508 – Patch

[patch] Patch

bug-196508-20190601162518.patch (text/plain), 20.89 KB, created by Simon Fraser (smfr) on 2019-06-01 16:25:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-06-01 16:25:19 PDT

Size: 20.89 KB

patch

obsolete

>Subversion Revision: 246008
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b5568f427968d781e3e831424191e0fcf736f6a7..70e691a8c3f3e59ba881c0b7bc36da67728f478c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-06-01  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Async overflow scroll on iOS paints slowly if it has a negative z-index child
>+        https://bugs.webkit.org/show_bug.cgi?id=196508
>+        rdar://problem/49532709
>+
>+        Reviewed by Dean Jackson.
>+        
>+        If a RenderLayerBacking had a foreground layer and a scrolled contents layer, every geometry
>+        update would change the size and offsetFromRenderer of the foreground layer between two
>+        states, triggering repaint.
>+
>+        Fix by updating the fore- and background-layers last (nothing elese has dependencies
>+        on their geometry), and using GraphicsLayer::DontSetNeedsDisplay as we do for the
>+        scrolled contents layer.
>+        
>+        The test also revealed a bug where the shapeMaskLayer would get incorrect geometry when scrollbars
>+        were visible, because it would be squished by setting the wrong bounds, so fix that.
>+
>+        Test: compositing/repaint/scroller-with-foreground-layer-repaints.html
>+
>+        * platform/graphics/ca/GraphicsLayerCA.cpp:
>+        (WebCore::GraphicsLayerCA::updateClippingStrategy):
>+        * rendering/RenderLayerBacking.cpp:
>+        (WebCore::RenderLayerBacking::updateGeometry):
>+
> 2019-06-01  Simon Fraser  <simon.fraser@apple.com>
> 
>         [Async overflow scroll] Flashing content when scrolling async overflow with a negative z-index child
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>index 33a33d4a7d0e533cd23807df08c789aa4f50fdc9..3ca5c5ca1778289e587810f66dc7a7d7ac2e5ccd 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>@@ -2598,18 +2598,18 @@ void GraphicsLayerCA::updateClippingStrategy(PlatformCALayer& clippingLayer, Ref
> 
>     if (!shapeMaskLayer) {
>         shapeMaskLayer = createPlatformCALayer(PlatformCALayer::LayerTypeShapeLayer, this);
>-        shapeMaskLayer->setAnchorPoint(FloatPoint3D());
>+        shapeMaskLayer->setAnchorPoint({ });
>         shapeMaskLayer->setName("shape mask");
>     }
>     
>-    shapeMaskLayer->setPosition(FloatPoint());
>-    shapeMaskLayer->setBounds(clippingLayer.bounds());
>+    shapeMaskLayer->setPosition(roundedRect.rect().location() - offsetFromRenderer());
>+    FloatRect shapeBounds({ }, roundedRect.rect().size());
>+    shapeMaskLayer->setBounds(shapeBounds);
>+    FloatRoundedRect offsetRoundedRect(shapeBounds, roundedRect.radii());
>+    shapeMaskLayer->setShapeRoundedRect(offsetRoundedRect);
> 
>     clippingLayer.setCornerRadius(0);
>     clippingLayer.setMask(shapeMaskLayer.get());
>-    
>-    FloatRoundedRect offsetRoundedRect(clippingLayer.bounds(), roundedRect.radii());
>-    shapeMaskLayer->setShapeRoundedRect(offsetRoundedRect);
> }
> 
> void GraphicsLayerCA::updateContentsRects()
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index 19dfbb12cce6a4bc88e262edf2ea8a3b7e544ad3..59bb7607704e7f5735b7bdacaa6b3fc5adeb8f9d 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -1204,39 +1204,6 @@ void RenderLayerBacking::updateGeometry()
>             m_contentsContainmentLayer->setAnchorPoint(FloatPoint3D(0.5, 0.5, 0));
>     }
> 
>-    if (m_foregroundLayer) {
>-        FloatPoint foregroundPosition;
>-        FloatSize foregroundSize = primaryGraphicsLayerRect.size();
>-        FloatSize foregroundOffset = m_graphicsLayer->offsetFromRenderer();
>-        if (hasClippingLayer()) {
>-            // If we have a clipping layer (which clips descendants), then the foreground layer is a child of it,
>-            // so that it gets correctly sorted with children. In that case, position relative to the clipping layer.
>-            foregroundSize = FloatSize(clippingBox.size());
>-            foregroundOffset = toFloatSize(clippingBox.location());
>-        }
>-
>-        m_foregroundLayer->setPosition(foregroundPosition);
>-        m_foregroundLayer->setSize(foregroundSize);
>-        m_foregroundLayer->setOffsetFromRenderer(foregroundOffset);
>-    }
>-
>-    if (m_backgroundLayer) {
>-        FloatPoint backgroundPosition;
>-        FloatSize backgroundSize = primaryGraphicsLayerRect.size();
>-        if (backgroundLayerPaintsFixedRootBackground()) {
>-            const FrameView& frameView = renderer().view().frameView();
>-            backgroundPosition = frameView.scrollPositionForFixedPosition();
>-            backgroundSize = frameView.layoutSize();
>-        } else {
>-            auto boundingBox = renderer().objectBoundingBox();
>-            backgroundPosition = boundingBox.location();
>-            backgroundSize = boundingBox.size();
>-        }
>-        m_backgroundLayer->setPosition(backgroundPosition);
>-        m_backgroundLayer->setSize(backgroundSize);
>-        m_backgroundLayer->setOffsetFromRenderer(m_graphicsLayer->offsetFromRenderer());
>-    }
>-
>     if (m_owningLayer.reflectionLayer() && m_owningLayer.reflectionLayer()->isComposited()) {
>         auto* reflectionBacking = m_owningLayer.reflectionLayer()->backing();
>         reflectionBacking->updateGeometry();
>@@ -1282,15 +1249,50 @@ void RenderLayerBacking::updateGeometry()
>         m_scrolledContentsLayer->setSize(scrollSize);
>         m_scrolledContentsLayer->setScrollOffset(scrollOffset, GraphicsLayer::DontSetNeedsDisplay);
>         m_scrolledContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBoxIncludingScrollbar.location()), GraphicsLayer::DontSetNeedsDisplay);
>-
>-        if (m_foregroundLayer) {
>-            m_foregroundLayer->setSize(m_scrolledContentsLayer->size());
>-            m_foregroundLayer->setOffsetFromRenderer(m_scrolledContentsLayer->offsetFromRenderer() - toLayoutSize(m_scrolledContentsLayer->scrollOffset()));
>-        }
>         
>         adjustTiledBackingCoverage();
>     }
> 
>+    if (m_foregroundLayer) {
>+        FloatSize foregroundSize;
>+        FloatSize foregroundOffset;
>+        GraphicsLayer::ShouldSetNeedsDisplay needsDisplayOnOffsetChange = GraphicsLayer::SetNeedsDisplay;
>+        if (m_scrolledContentsLayer) {
>+            foregroundSize = m_scrolledContentsLayer->size();
>+            foregroundOffset = m_scrolledContentsLayer->offsetFromRenderer() - toLayoutSize(m_scrolledContentsLayer->scrollOffset());
>+            needsDisplayOnOffsetChange = GraphicsLayer::DontSetNeedsDisplay;
>+        } else if (hasClippingLayer()) {
>+            // If we have a clipping layer (which clips descendants), then the foreground layer is a child of it,
>+            // so that it gets correctly sorted with children. In that case, position relative to the clipping layer.
>+            foregroundSize = FloatSize(clippingBox.size());
>+            foregroundOffset = toFloatSize(clippingBox.location());
>+        } else {
>+            foregroundSize = primaryGraphicsLayerRect.size();
>+            foregroundOffset = m_graphicsLayer->offsetFromRenderer();
>+        }
>+
>+        m_foregroundLayer->setPosition({ });
>+        m_foregroundLayer->setSize(foregroundSize);
>+        m_foregroundLayer->setOffsetFromRenderer(foregroundOffset, needsDisplayOnOffsetChange);
>+    }
>+
>+    if (m_backgroundLayer) {
>+        FloatPoint backgroundPosition;
>+        FloatSize backgroundSize = primaryGraphicsLayerRect.size();
>+        if (backgroundLayerPaintsFixedRootBackground()) {
>+            const FrameView& frameView = renderer().view().frameView();
>+            backgroundPosition = frameView.scrollPositionForFixedPosition();
>+            backgroundSize = frameView.layoutSize();
>+        } else {
>+            auto boundingBox = renderer().objectBoundingBox();
>+            backgroundPosition = boundingBox.location();
>+            backgroundSize = boundingBox.size();
>+        }
>+        m_backgroundLayer->setPosition(backgroundPosition);
>+        m_backgroundLayer->setSize(backgroundSize);
>+        m_backgroundLayer->setOffsetFromRenderer(m_graphicsLayer->offsetFromRenderer());
>+    }
>+
>     // If this layer was created just for clipping or to apply perspective, it doesn't need its own backing store.
>     LayoutRect ancestorCompositedBounds = compositedAncestor ? compositedAncestor->backing()->compositedBounds() : LayoutRect();
>     setRequiresOwnBackingStore(compositor().requiresOwnBackingStore(m_owningLayer, compositedAncestor,
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 682672a0580f2ff8b48fd4d0e043add33895259a..7b32dd7707699e8d80397ee3458fb504be2c7c24 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-06-01  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Async overflow scroll on iOS paints slowly if it has a negative z-index child
>+        https://bugs.webkit.org/show_bug.cgi?id=196508
>+        rdar://problem/49532709
>+
>+        Reviewed by Dean Jackson.
>+
>+        * compositing/geometry/scroller-with-clipping-and-foreground-layers-expected.html: Added.
>+        * compositing/geometry/scroller-with-clipping-and-foreground-layers.html: Added.
>+        * compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt: Added.
>+        * compositing/repaint/scroller-with-foreground-layer-repaints.html: Added.
>+        * platform/ios-wk2/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt: Added.
>+        * platform/mac-wk1/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt: Added.
>+
> 2019-06-01  Simon Fraser  <simon.fraser@apple.com>
> 
>         [Async overflow scroll] Flashing content when scrolling async overflow with a negative z-index child
>diff --git a/LayoutTests/compositing/geometry/scroller-with-clipping-and-foreground-layers-expected.html b/LayoutTests/compositing/geometry/scroller-with-clipping-and-foreground-layers-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..4908f17855d77f0c2de04c9b27c390efb1ffb935
>--- /dev/null
>+++ b/LayoutTests/compositing/geometry/scroller-with-clipping-and-foreground-layers-expected.html
>@@ -0,0 +1,63 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <style>
>+        body {
>+            margin: 0;
>+        }
>+        
>+        #scroller {
>+            position: relative;
>+            z-index: 0;
>+            width: 400px;
>+            height: 400px;
>+            margin: 20px;
>+            overflow: scroll;
>+            padding: 20px;
>+            border: 30px solid orange;
>+            box-sizing: border-box;
>+            border-radius: 80px 200px;
>+            background-color: silver;
>+        }
>+
>+        .negative {
>+            position: absolute;
>+            top: 20px;
>+            left: 30px;
>+            z-index: 1;
>+            padding: 20px;
>+            background-color: gray;
>+            transform: translateZ(0);
>+        }
>+        
>+        .contents {
>+            height: 1000px;
>+            background: linear-gradient(green, blue);
>+        }
>+        
>+        .masker {
>+            position: absolute;
>+            top: 49px;
>+            left: 49px;
>+            width: 342px;
>+            height: 342px;
>+            box-sizing: border-box;
>+            border: 3px solid gray;
>+            border-radius: 49px 170px;
>+        }
>+    </style>
>+    <script>
>+        window.addEventListener('load', () => {
>+            scroller.scrollTo(0, 250);
>+        }, false);
>+    </script>
>+</head>
>+<body>
>+    <div id="scroller">
>+        <div class="negative"></div>
>+        <div class="contents"></div>
>+    </div>
>+    <div class="masker"></div>
>+</body>
>+</html>
>+
>diff --git a/LayoutTests/compositing/geometry/scroller-with-clipping-and-foreground-layers.html b/LayoutTests/compositing/geometry/scroller-with-clipping-and-foreground-layers.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5fa0f7eb258b88e410d1b18a862ebefd2649dfc1
>--- /dev/null
>+++ b/LayoutTests/compositing/geometry/scroller-with-clipping-and-foreground-layers.html
>@@ -0,0 +1,63 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <style>
>+        body {
>+            margin: 0;
>+        }
>+        
>+        #scroller {
>+            position: relative;
>+            z-index: 0;
>+            width: 400px;
>+            height: 400px;
>+            margin: 20px;
>+            overflow: scroll;
>+            padding: 20px;
>+            border: 30px solid orange;
>+            box-sizing: border-box;
>+            border-radius: 80px 200px;
>+            background-color: silver;
>+        }
>+
>+        .negative {
>+            position: absolute;
>+            top: 20px;
>+            left: 30px;
>+            z-index: -1;
>+            padding: 20px;
>+            background-color: gray;
>+            transform: translateZ(0);
>+        }
>+        
>+        .contents {
>+            height: 1000px;
>+            background: linear-gradient(green, blue);
>+        }
>+        
>+        .masker {
>+            position: absolute;
>+            top: 49px;
>+            left: 49px;
>+            width: 342px;
>+            height: 342px;
>+            box-sizing: border-box;
>+            border: 3px solid gray;
>+            border-radius: 49px 170px;
>+        }
>+    </style>
>+    <script>
>+        window.addEventListener('load', () => {
>+            scroller.scrollTo(0, 250);
>+        }, false);
>+    </script>
>+</head>
>+<body>
>+    <div id="scroller">
>+        <div class="negative"></div>
>+        <div class="contents"></div>
>+    </div>
>+    <div class="masker"></div>
>+</body>
>+</html>
>+
>diff --git a/LayoutTests/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt b/LayoutTests/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..871c0c63d02c6ae3d9f164986b61cefee6d0c655
>--- /dev/null
>+++ b/LayoutTests/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt
>@@ -0,0 +1,54 @@
>+Scrolled contents
>+(GraphicsLayer
>+  (anchor 0.00 0.00)
>+  (bounds 800.00 600.00)
>+  (children 1
>+    (GraphicsLayer
>+      (bounds 800.00 600.00)
>+      (contentsOpaque 1)
>+      (children 1
>+        (GraphicsLayer
>+          (position 8.00 8.00)
>+          (bounds 322.00 322.00)
>+          (drawsContent 1)
>+          (repaint rects
>+            (rect 306.00 1.00 15.00 104.00)
>+            (rect 306.00 60.00 15.00 91.00)
>+            (rect 306.00 105.00 15.00 201.00)
>+          )
>+          (children 1
>+            (GraphicsLayer
>+              (offsetFromRenderer width=1 height=1)
>+              (position 1.00 1.00)
>+              (bounds 305.00 305.00)
>+              (children 1
>+                (GraphicsLayer
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,200))
>+                  (position 0.00 -200.00)
>+                  (anchor 0.00 0.00)
>+                  (bounds 305.00 1020.00)
>+                  (drawsContent 1)
>+                  (children 2
>+                    (GraphicsLayer
>+                      (position 30.00 50.00)
>+                      (bounds 102.00 102.00)
>+                      (contentsOpaque 1)
>+                      (drawsContent 1)
>+                    )
>+                    (GraphicsLayer
>+                      (offsetFromRenderer width=1 height=-199)
>+                      (bounds 305.00 1020.00)
>+                      (drawsContent 1)
>+                    )
>+                  )
>+                )
>+              )
>+            )
>+          )
>+        )
>+      )
>+    )
>+  )
>+)
>+
>diff --git a/LayoutTests/compositing/repaint/scroller-with-foreground-layer-repaints.html b/LayoutTests/compositing/repaint/scroller-with-foreground-layer-repaints.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..3dc8e0a2c4c68469ab66024ce8f3fb9ba13f99b7
>--- /dev/null
>+++ b/LayoutTests/compositing/repaint/scroller-with-foreground-layer-repaints.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html> <html>
>+<head>
>+    <style>
>+        #scroller {
>+            position: relative;
>+            z-index: 0;
>+            width: 300px;
>+            height: 300px;
>+            border: 1px solid black;
>+            overflow: scroll;
>+            padding: 10px;
>+        }
>+
>+        .contents {
>+            height: 1000px;
>+        }
>+        
>+        .negative {
>+            position: absolute;
>+            z-index: -1;
>+            top: 50px;
>+            left: 30px;
>+            width: 100px;
>+            height: 100px;
>+            background-color: blue;
>+            border: 1px solid green;
>+            transform: translateZ(0);
>+        }
>+    </style>
>+    <script>
>+        if (window.testRunner) {
>+            testRunner.dumpAsText();
>+            testRunner.waitUntilDone();
>+        }
>+
>+        window.addEventListener('load', () => {
>+            setTimeout(() => {
>+                if (window.internals)
>+                    internals.startTrackingRepaints();
>+            
>+                scroller.scrollTo(0, 200);
>+                document.getElementById('layers').textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_REPAINT_RECTS);
>+
>+                if (window.testRunner)
>+                    testRunner.notifyDone();
>+            }, 0);
>+        }, false);
>+    </script>
>+</head>
>+<body>
>+    <div id="scroller">
>+        <div class="negative"></div>
>+        <div class="contents">Scrolled contents</div>
>+    </div>
>+<pre id="layers"></pre>
>+</body>
>+</html>
>diff --git a/LayoutTests/platform/ios-wk2/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt b/LayoutTests/platform/ios-wk2/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..7d9457667e94a0054d64dc1d3110a3b6e5bfef2a
>--- /dev/null
>+++ b/LayoutTests/platform/ios-wk2/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt
>@@ -0,0 +1,49 @@
>+Scrolled contents
>+(GraphicsLayer
>+  (anchor 0.00 0.00)
>+  (bounds 800.00 600.00)
>+  (children 1
>+    (GraphicsLayer
>+      (bounds 800.00 600.00)
>+      (contentsOpaque 1)
>+      (children 1
>+        (GraphicsLayer
>+          (position 8.00 8.00)
>+          (bounds 322.00 322.00)
>+          (drawsContent 1)
>+          (children 1
>+            (GraphicsLayer
>+              (offsetFromRenderer width=1 height=1)
>+              (position 1.00 1.00)
>+              (bounds origin 0.00 200.00)
>+              (bounds 320.00 320.00)
>+              (children 1
>+                (GraphicsLayer
>+                  (offsetFromRenderer width=1 height=1)
>+                  (scrollOffset (0,200))
>+                  (anchor 0.00 0.00)
>+                  (bounds 320.00 1020.00)
>+                  (drawsContent 1)
>+                  (children 2
>+                    (GraphicsLayer
>+                      (position 30.00 50.00)
>+                      (bounds 102.00 102.00)
>+                      (contentsOpaque 1)
>+                      (drawsContent 1)
>+                    )
>+                    (GraphicsLayer
>+                      (offsetFromRenderer width=1 height=-199)
>+                      (bounds 320.00 1020.00)
>+                      (drawsContent 1)
>+                    )
>+                  )
>+                )
>+              )
>+            )
>+          )
>+        )
>+      )
>+    )
>+  )
>+)
>+
>diff --git a/LayoutTests/platform/mac-wk1/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt b/LayoutTests/platform/mac-wk1/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..d619fd7bfb0cc60a255115f8157f5077bb4e16a6
>--- /dev/null
>+++ b/LayoutTests/platform/mac-wk1/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt
>@@ -0,0 +1,48 @@
>+Scrolled contents
>+(GraphicsLayer
>+  (anchor 0.00 0.00)
>+  (bounds 800.00 600.00)
>+  (children 1
>+    (GraphicsLayer
>+      (bounds 800.00 600.00)
>+      (contentsOpaque 1)
>+      (children 1
>+        (GraphicsLayer
>+          (position 8.00 8.00)
>+          (bounds 322.00 322.00)
>+          (drawsContent 1)
>+          (repaint rects
>+            (rect 0.00 0.00 322.00 322.00)
>+            (rect 306.00 1.00 15.00 104.00)
>+            (rect 306.00 60.00 15.00 91.00)
>+            (rect 306.00 105.00 15.00 201.00)
>+          )
>+          (children 1
>+            (GraphicsLayer
>+              (offsetFromRenderer width=1 height=1)
>+              (position 1.00 1.00)
>+              (bounds 305.00 305.00)
>+              (children 2
>+                (GraphicsLayer
>+                  (position 30.00 -150.00)
>+                  (bounds 102.00 102.00)
>+                  (contentsOpaque 1)
>+                  (drawsContent 1)
>+                )
>+                (GraphicsLayer
>+                  (offsetFromRenderer width=1 height=1)
>+                  (bounds 305.00 305.00)
>+                  (drawsContent 1)
>+                  (repaint rects
>+                    (rect 0.00 0.00 305.00 305.00)
>+                  )
>+                )
>+              )
>+            )
>+          )
>+        )
>+      )
>+    )
>+  )
>+)
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 196508: 366531 | 371119 | 371129