Attachment 371058 Details for Bug 198420 – Patch

[patch] Patch

bug-198420-20190531082234.patch (text/plain), 19.95 KB, created by Simon Fraser (smfr) on 2019-05-31 08:22:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-05-31 08:22:35 PDT

Size: 19.95 KB

patch

obsolete

>Subversion Revision: 245906
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e58e71edc2a674fc824c94dbf01e879245cc2548..3d5e763e903a07846f6b573630ca71e2eb1eacb9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2019-05-31  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Move code that sets compositing paint phases into a single function
>+        https://bugs.webkit.org/show_bug.cgi?id=198420
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        To compute the correct paint phases for the various GraphicsLayers in a RenderLayerBacking,
>+        we have to know which set of layers we've created (m_scrollContainerLayer, m_foregroundLayer etc).
>+        So move the code that sets phases into a single function which is called when that
>+        set of layers changes.
>+
>+        The test dumps paint phases for a stacking-context-composited scroller with a negative z-index child.
>+
>+        Also have GraphicsLayer::setPaintingPhase() trigger the necessary repaint when the paint phase changes.
>+
>+        Test: compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases.html
>+
>+        * platform/graphics/GraphicsLayer.cpp:
>+        (WebCore::GraphicsLayer::setPaintingPhase):
>+        * platform/graphics/GraphicsLayer.h:
>+        (WebCore::GraphicsLayer::setPaintingPhase): Deleted.
>+        * rendering/RenderLayerBacking.cpp:
>+        (WebCore::RenderLayerBacking::updateConfiguration):
>+        (WebCore::RenderLayerBacking::updateForegroundLayer):
>+        (WebCore::RenderLayerBacking::updateBackgroundLayer):
>+        (WebCore::RenderLayerBacking::updateMaskingLayer):
>+        (WebCore::RenderLayerBacking::updateScrollingLayers):
>+        (WebCore::RenderLayerBacking::updatePaintingPhases):
>+        (WebCore::RenderLayerBacking::paintingPhaseForPrimaryLayer const): Deleted.
>+        * rendering/RenderLayerBacking.h:
>+
> 2019-05-30  Simon Fraser  <simon.fraser@apple.com>
> 
>         Use an OptionSet<> for GraphicsLayerPaintingPhase
>diff --git a/Source/WebCore/platform/graphics/GraphicsLayer.cpp b/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>index 5568f4a86e334731a98f05a36ceb043dd8baa01a..a69b3064359788da9bec17fd97e6293fbe57b874 100644
>--- a/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>+++ b/Source/WebCore/platform/graphics/GraphicsLayer.cpp
>@@ -491,6 +491,15 @@ void GraphicsLayer::setBackgroundColor(const Color& color)
>     m_backgroundColor = color;
> }
> 
>+void GraphicsLayer::setPaintingPhase(OptionSet<GraphicsLayerPaintingPhase> phase)
>+{
>+    if (phase == m_paintingPhase)
>+        return;
>+
>+    setNeedsDisplay();
>+    m_paintingPhase = phase;
>+}
>+
> void GraphicsLayer::paintGraphicsLayerContents(GraphicsContext& context, const FloatRect& clip, GraphicsLayerPaintBehavior layerPaintBehavior)
> {
>     FloatSize offset = offsetFromRenderer() - toFloatSize(scrollOffset());
>diff --git a/Source/WebCore/platform/graphics/GraphicsLayer.h b/Source/WebCore/platform/graphics/GraphicsLayer.h
>index e48434ed4c49801231e6cbdf3e8dd42816a1b36b..e0d342924f7318e07af1f4bfe0bdee30c5837178 100644
>--- a/Source/WebCore/platform/graphics/GraphicsLayer.h
>+++ b/Source/WebCore/platform/graphics/GraphicsLayer.h
>@@ -415,7 +415,7 @@ public:
> 
>     // Some GraphicsLayers paint only the foreground or the background content
>     OptionSet<GraphicsLayerPaintingPhase> paintingPhase() const { return m_paintingPhase; }
>-    void setPaintingPhase(OptionSet<GraphicsLayerPaintingPhase> phase) { m_paintingPhase = phase; }
>+    void setPaintingPhase(OptionSet<GraphicsLayerPaintingPhase>);
> 
>     enum ShouldClipToLayer {
>         DoNotClipToLayer,
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index fcaf9c7c3ca746ab09f3233c18bd8a40e7ec0083..61fc70c32cc2ccf0ad82f4566cf4998e3dad7f27 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -819,7 +819,8 @@ bool RenderLayerBacking::updateConfiguration()
>             m_graphicsLayer->addChild(*flatteningLayer);
>     }
> 
>-    updateMaskingLayer(renderer().hasMask(), renderer().hasClipPath());
>+    if (updateMaskingLayer(renderer().hasMask(), renderer().hasClipPath()))
>+        layerConfigChanged = true;
> 
>     updateChildClippingStrategy(needsDescendantsClippingLayer);
> 
>@@ -888,6 +889,9 @@ bool RenderLayerBacking::updateConfiguration()
>         }
>     }
> 
>+    if (layerConfigChanged)
>+        updatePaintingPhases();
>+
>     return layerConfigChanged;
> }
> 
>@@ -1709,7 +1713,6 @@ bool RenderLayerBacking::updateForegroundLayer(bool needsForegroundLayer)
>             String layerName = m_owningLayer.name() + " (foreground)";
>             m_foregroundLayer = createGraphicsLayer(layerName);
>             m_foregroundLayer->setDrawsContent(true);
>-            m_foregroundLayer->setPaintingPhase({ GraphicsLayerPaintingPhase::Foreground });
>             layerChanged = true;
>         }
>     } else if (m_foregroundLayer) {
>@@ -1718,11 +1721,6 @@ bool RenderLayerBacking::updateForegroundLayer(bool needsForegroundLayer)
>         layerChanged = true;
>     }
> 
>-    if (layerChanged) {
>-        m_graphicsLayer->setNeedsDisplay();
>-        m_graphicsLayer->setPaintingPhase(paintingPhaseForPrimaryLayer());
>-    }
>-
>     return layerChanged;
> }
> 
>@@ -1735,7 +1733,6 @@ bool RenderLayerBacking::updateBackgroundLayer(bool needsBackgroundLayer)
>             m_backgroundLayer = createGraphicsLayer(layerName);
>             m_backgroundLayer->setDrawsContent(true);
>             m_backgroundLayer->setAnchorPoint(FloatPoint3D());
>-            m_backgroundLayer->setPaintingPhase({ GraphicsLayerPaintingPhase::Background });
>             layerChanged = true;
>         }
>         
>@@ -1759,15 +1756,12 @@ bool RenderLayerBacking::updateBackgroundLayer(bool needsBackgroundLayer)
>             m_graphicsLayer->setAppliesPageScale(true);
>         }
>     }
>-    
>-    if (layerChanged)
>-        m_graphicsLayer->setNeedsDisplay();
>-    
>+
>     return layerChanged;
> }
> 
> // Masking layer is used for masks or clip-path.
>-void RenderLayerBacking::updateMaskingLayer(bool hasMask, bool hasClipPath)
>+bool RenderLayerBacking::updateMaskingLayer(bool hasMask, bool hasClipPath)
> {
>     bool layerChanged = false;
>     if (hasMask || hasClipPath) {
>@@ -1805,8 +1799,7 @@ void RenderLayerBacking::updateMaskingLayer(bool hasMask, bool hasClipPath)
>         layerChanged = true;
>     }
> 
>-    if (layerChanged)
>-        m_graphicsLayer->setPaintingPhase(paintingPhaseForPrimaryLayer());
>+    return layerChanged;
> }
> 
> void RenderLayerBacking::updateChildClippingStrategy(bool needsDescendantsClippingLayer)
>@@ -1846,8 +1839,9 @@ bool RenderLayerBacking::updateScrollingLayers(bool needsScrollingLayers)
>         return false;
> 
>     if (!m_scrollContainerLayer) {
>-        // Outer layer which corresponds with the scroll view.
>+        // Outer layer which corresponds with the scroll view. This never paints content.
>         m_scrollContainerLayer = createGraphicsLayer("scroll container", GraphicsLayer::Type::ScrollContainer);
>+        m_scrollContainerLayer->setPaintingPhase({ });
>         m_scrollContainerLayer->setDrawsContent(false);
>         m_scrollContainerLayer->setMasksToBounds(true);
> 
>@@ -1855,11 +1849,6 @@ bool RenderLayerBacking::updateScrollingLayers(bool needsScrollingLayers)
>         m_scrolledContentsLayer = createGraphicsLayer("scrolled contents", GraphicsLayer::Type::ScrolledContents);
>         m_scrolledContentsLayer->setDrawsContent(true);
>         m_scrolledContentsLayer->setAnchorPoint({ });
>-
>-        OptionSet<GraphicsLayerPaintingPhase> paintPhases = { GraphicsLayerPaintingPhase::OverflowContents, GraphicsLayerPaintingPhase::CompositedScroll };
>-        if (!m_foregroundLayer)
>-            paintPhases.add(GraphicsLayerPaintingPhase::Foreground);
>-        m_scrolledContentsLayer->setPaintingPhase(paintPhases);
>         m_scrollContainerLayer->addChild(*m_scrolledContentsLayer);
>     } else {
>         compositor().willRemoveScrollingLayerWithBacking(m_owningLayer, *this);
>@@ -1871,9 +1860,6 @@ bool RenderLayerBacking::updateScrollingLayers(bool needsScrollingLayers)
>         GraphicsLayer::unparentAndClear(m_scrolledContentsLayer);
>     }
> 
>-    m_graphicsLayer->setPaintingPhase(paintingPhaseForPrimaryLayer());
>-    m_graphicsLayer->setNeedsDisplay(); // Because painting phases changed.
>-
>     if (m_scrollContainerLayer)
>         compositor().didAddScrollingLayer(m_owningLayer);
>     
>@@ -1939,22 +1925,6 @@ void RenderLayerBacking::setIsScrollCoordinatedWithViewportConstrainedRole(bool
>     m_graphicsLayer->setIsViewportConstrained(viewportCoordinated);
> }
> 
>-OptionSet<GraphicsLayerPaintingPhase> RenderLayerBacking::paintingPhaseForPrimaryLayer() const
>-{
>-    OptionSet<GraphicsLayerPaintingPhase> phases;
>-    if (!m_backgroundLayer)
>-        phases.add(GraphicsLayerPaintingPhase::Background);
>-    if (!m_foregroundLayer)
>-        phases.add(GraphicsLayerPaintingPhase::Foreground);
>-
>-    if (m_scrolledContentsLayer) {
>-        phases.remove(GraphicsLayerPaintingPhase::Foreground);
>-        phases.add(GraphicsLayerPaintingPhase::CompositedScroll);
>-    }
>-
>-    return phases;
>-}
>-
> float RenderLayerBacking::compositingOpacity(float rendererOpacity) const
> {
>     float finalOpacity = rendererOpacity;
>@@ -2123,6 +2093,35 @@ void RenderLayerBacking::updateRootLayerConfiguration()
>     }
> }
> 
>+void RenderLayerBacking::updatePaintingPhases()
>+{
>+    // Phases for m_childClippingMaskLayer and m_maskLayer are set elsewhere.
>+    OptionSet<GraphicsLayerPaintingPhase> primaryLayerPhases = { GraphicsLayerPaintingPhase::Background, GraphicsLayerPaintingPhase::Foreground };
>+    
>+    if (m_foregroundLayer) {
>+        OptionSet<GraphicsLayerPaintingPhase> foregroundLayerPhases { GraphicsLayerPaintingPhase::Foreground };
>+        m_foregroundLayer->setPaintingPhase(foregroundLayerPhases);
>+        primaryLayerPhases.remove(GraphicsLayerPaintingPhase::Foreground);
>+    }
>+
>+    if (m_backgroundLayer) {
>+        m_backgroundLayer->setPaintingPhase(GraphicsLayerPaintingPhase::Background);
>+        primaryLayerPhases.remove(GraphicsLayerPaintingPhase::Background);
>+    }
>+
>+    if (m_scrolledContentsLayer) {
>+        OptionSet<GraphicsLayerPaintingPhase> scrolledContentLayerPhases = { GraphicsLayerPaintingPhase::OverflowContents, GraphicsLayerPaintingPhase::CompositedScroll };
>+        if (!m_foregroundLayer)
>+            scrolledContentLayerPhases.add(GraphicsLayerPaintingPhase::Foreground);
>+        m_scrolledContentsLayer->setPaintingPhase(scrolledContentLayerPhases);
>+
>+        primaryLayerPhases.remove(GraphicsLayerPaintingPhase::Foreground);
>+        primaryLayerPhases.add(GraphicsLayerPaintingPhase::CompositedScroll);
>+    }
>+
>+    m_graphicsLayer->setPaintingPhase(primaryLayerPhases);
>+}
>+
> static bool supportsDirectlyCompositedBoxDecorations(const RenderLayerModelObject& renderer)
> {
>     if (!GraphicsLayer::supportsBackgroundColorContent())
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.h b/Source/WebCore/rendering/RenderLayerBacking.h
>index 5175f19c4a056e1877de6bc58d41ecae112ad939..0030811d6910eeac8dde883f1f10ec38c37e0bad 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.h
>+++ b/Source/WebCore/rendering/RenderLayerBacking.h
>@@ -316,7 +316,7 @@ private:
>     bool updateOverflowControlsLayers(bool needsHorizontalScrollbarLayer, bool needsVerticalScrollbarLayer, bool needsScrollCornerLayer);
>     bool updateForegroundLayer(bool needsForegroundLayer);
>     bool updateBackgroundLayer(bool needsBackgroundLayer);
>-    void updateMaskingLayer(bool hasMask, bool hasClipPath);
>+    bool updateMaskingLayer(bool hasMask, bool hasClipPath);
>     bool requiresHorizontalScrollbarLayer() const;
>     bool requiresVerticalScrollbarLayer() const;
>     bool requiresScrollCornerLayer() const;
>@@ -326,15 +326,12 @@ private:
>     void setLocationOfScrolledContents(ScrollOffset, ScrollingLayerPositionAction);
> 
>     void updateChildClippingStrategy(bool needsDescendantsClippingLayer);
>-
>     void updateMaskingLayerGeometry();
>-    
>     void updateRootLayerConfiguration();
>+    void updatePaintingPhases();
> 
>     void setBackgroundLayerPaintsFixedRootBackground(bool);
> 
>-    OptionSet<GraphicsLayerPaintingPhase> paintingPhaseForPrimaryLayer() const;
>-    
>     LayoutSize contentOffsetInCompositingLayer() const;
>     // Result is transform origin in device pixels.
>     FloatPoint3D computeTransformOriginForPainting(const LayoutRect& borderBox) const;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 78485df80530a7bd5d1b9de9fbfadb210dadf9e0..0bcd3f68d096944485c4c7ce7a10e6484fef7a07 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-05-31  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Move code that sets compositing paint phases into a single function
>+        https://bugs.webkit.org/show_bug.cgi?id=198420
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt: Added.
>+        * compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases.html: Added.
>+        * platform/ios-wk2/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt: Added.
>+        * platform/mac-wk1/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt: Added.
>+
> 2019-05-30  Justin Fan  <justin_fan@apple.com>
> 
>         [Web GPU] Vertex Buffers/Input State API updates
>diff --git a/LayoutTests/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt b/LayoutTests/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..4c7a41143b74d66465b700fe628e9c623cff982d
>--- /dev/null
>+++ b/LayoutTests/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt
>@@ -0,0 +1,61 @@
>+Scrolled contents
>+(GraphicsLayer
>+  (anchor 0.00 0.00)
>+  (bounds 800.00 600.00)
>+  (paintingPhases
>+    GraphicsLayerPaintBackground
>+    GraphicsLayerPaintForeground
>+    )
>+  (children 1
>+    (GraphicsLayer
>+      (bounds 800.00 600.00)
>+      (contentsOpaque 1)
>+      (paintingPhases
>+        GraphicsLayerPaintBackground
>+        GraphicsLayerPaintForeground
>+        )
>+      (children 1
>+        (GraphicsLayer
>+          (position 8.00 8.00)
>+          (bounds 322.00 322.00)
>+          (drawsContent 1)
>+          (paintingPhases
>+            GraphicsLayerPaintBackground
>+            GraphicsLayerPaintCompositedScroll
>+            )
>+          (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)
>+                  (anchor 0.00 0.00)
>+                  (bounds 305.00 1020.00)
>+                  (drawsContent 1)
>+                  (paintingPhases
>+                    GraphicsLayerPaintOverflowContents
>+                    GraphicsLayerPaintCompositedScroll
>+                    )
>+                  (children 1
>+                    (GraphicsLayer
>+                      (offsetFromRenderer width=1 height=1)
>+                      (bounds 305.00 1020.00)
>+                      (drawsContent 1)
>+                      (paintingPhases
>+                        GraphicsLayerPaintForeground
>+                        GraphicsLayerPaintOverflowContents
>+                        )
>+                    )
>+                  )
>+                )
>+              )
>+            )
>+          )
>+        )
>+      )
>+    )
>+  )
>+)
>+
>diff --git a/LayoutTests/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases.html b/LayoutTests/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..8cc954bf5ac1d6d47341105360347602724aef05
>--- /dev/null
>+++ b/LayoutTests/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases.html
>@@ -0,0 +1,44 @@
>+<!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;
>+        }
>+    </style>
>+    <script>
>+        if (window.testRunner)
>+            testRunner.dumpAsText();
>+
>+        window.addEventListener('load', () => {
>+            document.getElementById('layers').textContent = internals.layerTreeAsText(document, window.internals.LAYER_TREE_INCLUDES_PAINTING_PHASES);
>+        }, false);
>+    </script>
>+</head>
>+<body>
>+    <div class="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/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt b/LayoutTests/platform/ios-wk2/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..b4fb9faf3d4b95535758e0f49acd953b64ac6247
>--- /dev/null
>+++ b/LayoutTests/platform/ios-wk2/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt
>@@ -0,0 +1,61 @@
>+Scrolled contents
>+(GraphicsLayer
>+  (anchor 0.00 0.00)
>+  (bounds 800.00 600.00)
>+  (paintingPhases
>+    GraphicsLayerPaintBackground
>+    GraphicsLayerPaintForeground
>+    )
>+  (children 1
>+    (GraphicsLayer
>+      (bounds 800.00 600.00)
>+      (contentsOpaque 1)
>+      (paintingPhases
>+        GraphicsLayerPaintBackground
>+        GraphicsLayerPaintForeground
>+        )
>+      (children 1
>+        (GraphicsLayer
>+          (position 8.00 8.00)
>+          (bounds 322.00 322.00)
>+          (drawsContent 1)
>+          (paintingPhases
>+            GraphicsLayerPaintBackground
>+            GraphicsLayerPaintCompositedScroll
>+            )
>+          (children 1
>+            (GraphicsLayer
>+              (offsetFromRenderer width=1 height=1)
>+              (position 1.00 1.00)
>+              (bounds 320.00 320.00)
>+              (children 1
>+                (GraphicsLayer
>+                  (offsetFromRenderer width=1 height=1)
>+                  (anchor 0.00 0.00)
>+                  (bounds 320.00 1020.00)
>+                  (drawsContent 1)
>+                  (paintingPhases
>+                    GraphicsLayerPaintOverflowContents
>+                    GraphicsLayerPaintCompositedScroll
>+                    )
>+                  (children 1
>+                    (GraphicsLayer
>+                      (offsetFromRenderer width=1 height=1)
>+                      (bounds 320.00 1020.00)
>+                      (drawsContent 1)
>+                      (paintingPhases
>+                        GraphicsLayerPaintForeground
>+                        GraphicsLayerPaintOverflowContents
>+                        )
>+                    )
>+                  )
>+                )
>+              )
>+            )
>+          )
>+        )
>+      )
>+    )
>+  )
>+)
>+
>diff --git a/LayoutTests/platform/mac-wk1/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt b/LayoutTests/platform/mac-wk1/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..e30707a44edd62952adef07c8407e71d1744ec8e
>--- /dev/null
>+++ b/LayoutTests/platform/mac-wk1/compositing/overflow/stacking-context-composited-scroller-with-foreground-paint-phases-expected.txt
>@@ -0,0 +1,2 @@
>+Scrolled contents
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 198420: 371058 | 371062