Title: [259015] trunk
Revision
259015
Author
simon.fra...@apple.com
Date
2020-03-25 15:28:41 -0700 (Wed, 25 Mar 2020)

Log Message

Flashing and partly visible elements
https://bugs.webkit.org/show_bug.cgi?id=204605

Reviewed by Zalan Bujtas.

Source/WebCore:

If, during a compositing update, a layer becomes non-composited, then we repaint its
location in its new target compositing layer. However, that layer might be in the list
of BackingSharingState's layers that may paint into backing provided by some ancestor,
so they'd be in a limbo state where their repaint target was unknown. We'd erroneously
repaint in some ancestor, resulting in missing content.

Fix by having BackingSharingState track a set of layers that can't be repainted currently
because their ancestor chain contains a maybe-sharing layer, and repaint them when
the backing sharing state is resolved.

This is only an issue during RenderLayerCompositor::computeCompositingRequirements()
when the backing sharing state is being computed, so most repaints are not affected.

Test: compositing/shared-backing/repaint-into-shared-backing.html

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::BackingSharingState::isPotentialBackingSharingLayer const):
(WebCore::RenderLayerCompositor::BackingSharingState::addLayerNeedingRepaint):
(WebCore::RenderLayerCompositor::BackingSharingState::endBackingSharingSequence):
(WebCore::RenderLayerCompositor::BackingSharingState::issuePendingRepaints):
(WebCore::RenderLayerCompositor::computeCompositingRequirements):
(WebCore::RenderLayerCompositor::updateBacking):
(WebCore::RenderLayerCompositor::updateLayerCompositingState):
(WebCore::RenderLayerCompositor::layerRepaintTargetsBackingSharingLayer const):
* rendering/RenderLayerCompositor.h:

LayoutTests:

* compositing/shared-backing/repaint-into-shared-backing-expected.html: Added.
* compositing/shared-backing/repaint-into-shared-backing.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (259014 => 259015)


--- trunk/LayoutTests/ChangeLog	2020-03-25 21:57:10 UTC (rev 259014)
+++ trunk/LayoutTests/ChangeLog	2020-03-25 22:28:41 UTC (rev 259015)
@@ -1,3 +1,13 @@
+2020-03-25  Simon Fraser  <simon.fra...@apple.com>
+
+        Flashing and partly visible elements
+        https://bugs.webkit.org/show_bug.cgi?id=204605
+
+        Reviewed by Zalan Bujtas.
+
+        * compositing/shared-backing/repaint-into-shared-backing-expected.html: Added.
+        * compositing/shared-backing/repaint-into-shared-backing.html: Added.
+
 2020-03-25  Jason Lawrence  <lawrenc...@apple.com>
 
         [ Mac wk2 ] svg/as-image/svg-image-with-data-uri-background.html is flaky failing.

Added: trunk/LayoutTests/compositing/shared-backing/repaint-into-shared-backing-expected.html (0 => 259015)


--- trunk/LayoutTests/compositing/shared-backing/repaint-into-shared-backing-expected.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/shared-backing/repaint-into-shared-backing-expected.html	2020-03-25 22:28:41 UTC (rev 259015)
@@ -0,0 +1,53 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+        .container {
+            padding: 10px;
+            margin: 10px;
+            border: 2px solid black;
+            height: 400px;
+            width: 500px;
+            overflow-y: hidden;
+        }
+
+        .wrapper {
+            position: relative;
+            width: 2000px;
+            z-index: 1;
+        }
+
+        .positioned {
+            position: absolute;
+            left: 0;
+            top: 0;
+            background-color: silver;
+            z-index: 2;
+            padding: 20px;
+            width: 100%;
+            height: 250px;
+            overflow: hidden;
+        }
+
+       .inner {
+            position: relative;
+            width: 300px;
+            height: 200px;
+            background-color: green;
+            transform: translate(0px, 0px);
+        }
+    </style>
+</head>
+<body>
+<div class="container">
+    <div class="wrapper">
+        <div class="positioned">
+            &nbsp;
+            <div class="inner">
+                &nbsp;
+            </div>
+        </div>
+    </div>
+</div>
+</body>
+</html>
\ No newline at end of file

Added: trunk/LayoutTests/compositing/shared-backing/repaint-into-shared-backing.html (0 => 259015)


--- trunk/LayoutTests/compositing/shared-backing/repaint-into-shared-backing.html	                        (rev 0)
+++ trunk/LayoutTests/compositing/shared-backing/repaint-into-shared-backing.html	2020-03-25 22:28:41 UTC (rev 259015)
@@ -0,0 +1,71 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+    <style>
+        .container {
+            padding: 10px;
+            margin: 10px;
+            border: 2px solid black;
+            height: 400px;
+            width: 500px;
+            overflow-y: hidden;
+        }
+
+        .wrapper {
+            position: relative;
+            width: 2000px;
+            z-index: 1;
+        }
+
+        .positioned {
+            position: absolute;
+            left: 0;
+            top: 0;
+            background-color: silver;
+            z-index: 2;
+            padding: 20px;
+            width: 100%;
+            height: 250px;
+            overflow: hidden;
+        }
+
+       .inner {
+            position: relative;
+            width: 300px;
+            height: 200px;
+            background-color: green;
+            transform: translateZ(0);
+        }
+        
+        body.changed .inner {
+            transform: translate(0px, 0px);
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        window.addEventListener('load', () => {
+            requestAnimationFrame(() => {
+                requestAnimationFrame(() => {
+                    document.body.classList.add('changed');
+                    if (window.testRunner)
+                        testRunner.notifyDone();
+                });
+            });
+        }, false);
+    </script>
+</head>
+<body>
+<div class="container">
+    <div class="wrapper">
+        <div class="positioned">
+            &nbsp;
+            <div class="inner">
+                &nbsp;
+            </div>
+        </div>
+    </div>
+</div>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (259014 => 259015)


--- trunk/Source/WebCore/ChangeLog	2020-03-25 21:57:10 UTC (rev 259014)
+++ trunk/Source/WebCore/ChangeLog	2020-03-25 22:28:41 UTC (rev 259015)
@@ -1,3 +1,36 @@
+2020-03-25  Simon Fraser  <simon.fra...@apple.com>
+
+        Flashing and partly visible elements
+        https://bugs.webkit.org/show_bug.cgi?id=204605
+
+        Reviewed by Zalan Bujtas.
+
+        If, during a compositing update, a layer becomes non-composited, then we repaint its
+        location in its new target compositing layer. However, that layer might be in the list
+        of BackingSharingState's layers that may paint into backing provided by some ancestor,
+        so they'd be in a limbo state where their repaint target was unknown. We'd erroneously
+        repaint in some ancestor, resulting in missing content.
+
+        Fix by having BackingSharingState track a set of layers that can't be repainted currently
+        because their ancestor chain contains a maybe-sharing layer, and repaint them when
+        the backing sharing state is resolved.
+
+        This is only an issue during RenderLayerCompositor::computeCompositingRequirements()
+        when the backing sharing state is being computed, so most repaints are not affected.
+
+        Test: compositing/shared-backing/repaint-into-shared-backing.html
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::BackingSharingState::isPotentialBackingSharingLayer const):
+        (WebCore::RenderLayerCompositor::BackingSharingState::addLayerNeedingRepaint):
+        (WebCore::RenderLayerCompositor::BackingSharingState::endBackingSharingSequence):
+        (WebCore::RenderLayerCompositor::BackingSharingState::issuePendingRepaints):
+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
+        (WebCore::RenderLayerCompositor::updateBacking):
+        (WebCore::RenderLayerCompositor::updateLayerCompositingState):
+        (WebCore::RenderLayerCompositor::layerRepaintTargetsBackingSharingLayer const):
+        * rendering/RenderLayerCompositor.h:
+
 2020-03-25  Chris Dumez  <cdu...@apple.com>
 
         Event listeners registered with 'once' option may get garbage collected too soon

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (259014 => 259015)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2020-03-25 21:57:10 UTC (rev 259014)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2020-03-25 22:28:41 UTC (rev 259015)
@@ -229,16 +229,30 @@
 
     void updateBeforeDescendantTraversal(RenderLayer&, bool willBeComposited);
     void updateAfterDescendantTraversal(RenderLayer&, RenderLayer* stackingContextAncestor);
+    
+    bool isPotentialBackingSharingLayer(const RenderLayer& layer) const
+    {
+        return m_backingSharingLayers.contains(&layer);
+    }
 
+    // Add a layer that would repaint into a layer in m_backingSharingLayers.
+    // That repaint has to wait until we've set the provider's backing-sharing layers.
+    void addLayerNeedingRepaint(RenderLayer& layer)
+    {
+        m_layersPendingRepaint.add(layer);
+    }
+
 private:
     void layerWillBeComposited(RenderLayer&);
 
     void startBackingSharingSequence(RenderLayer& candidateLayer, RenderLayer* candidateStackingContext);
     void endBackingSharingSequence();
+    void issuePendingRepaints();
 
     RenderLayer* m_backingProviderCandidate { nullptr };
     RenderLayer* m_backingProviderStackingContext { nullptr };
     Vector<WeakPtr<RenderLayer>> m_backingSharingLayers;
+    WeakHashSet<RenderLayer> m_layersPendingRepaint;
 };
 
 void RenderLayerCompositor::BackingSharingState::startBackingSharingSequence(RenderLayer& candidateLayer, RenderLayer* candidateStackingContext)
@@ -255,6 +269,7 @@
     if (m_backingProviderCandidate) {
         m_backingProviderCandidate->backing()->setBackingSharingLayers(WTFMove(m_backingSharingLayers));
         m_backingSharingLayers.clear();
+        issuePendingRepaints();
     }
     
     m_backingProviderCandidate = nullptr;
@@ -292,6 +307,16 @@
         layer.backing()->clearBackingSharingLayers();
 }
 
+void RenderLayerCompositor::BackingSharingState::issuePendingRepaints()
+{
+    for (auto& layer : m_layersPendingRepaint) {
+        LOG_WITH_STREAM(Compositing, stream << "Issuing postponed repaint of layer " << &layer);
+        layer.compositor().repaintOnCompositingChange(layer);
+    }
+    
+    m_layersPendingRepaint.clear();
+}
+
 #if !LOG_DISABLED || ENABLE(TREE_DEBUGGING)
 static inline bool compositingLogEnabled()
 {
@@ -1049,7 +1074,7 @@
 
     // Create or destroy backing here. However, we can't update geometry because layers above us may become composited
     // during post-order traversal (e.g. for clipping).
-    if (updateBacking(layer, queryData, willBeComposited ? BackingRequired::Yes : BackingRequired::No)) {
+    if (updateBacking(layer, queryData, &backingSharingState, willBeComposited ? BackingRequired::Yes : BackingRequired::No)) {
         layer.setNeedsCompositingLayerConnection();
         // Child layers need to get a geometry update to recompute their position.
         layer.setChildrenNeedCompositingGeometryUpdate();
@@ -1058,7 +1083,7 @@
     }
 
     // Update layer state bits.
-    if (layer.reflectionLayer() && updateLayerCompositingState(*layer.reflectionLayer(), &layer, queryData))
+    if (layer.reflectionLayer() && updateLayerCompositingState(*layer.reflectionLayer(), &layer, queryData, backingSharingState))
         layer.setNeedsCompositingLayerConnection();
     
     // FIXME: clarify needsCompositingPaintOrderChildrenUpdate. If a composited layer gets a new ancestor, it needs geometry computations.
@@ -1589,7 +1614,7 @@
     m_rootContentsLayer->setMasksToBounds(!m_renderView.settings().backgroundShouldExtendBeyondPage());
 }
 
-bool RenderLayerCompositor::updateBacking(RenderLayer& layer, RequiresCompositingData& queryData, BackingRequired backingRequired)
+bool RenderLayerCompositor::updateBacking(RenderLayer& layer, RequiresCompositingData& queryData, BackingSharingState* backingSharingState, BackingRequired backingRequired)
 {
     bool layerChanged = false;
     if (backingRequired == BackingRequired::Unknown)
@@ -1599,6 +1624,14 @@
         requiresCompositingForPosition(rendererForCompositingTests(layer), layer, queryData);
     }
 
+    auto repaintLayer = [&](RenderLayer& layer) {
+        if (backingSharingState && layerRepaintTargetsBackingSharingLayer(layer, *backingSharingState)) {
+            LOG_WITH_STREAM(Compositing, stream << "Layer " << &layer << " needs to repaint into potential backing-sharing layer, postponing repaint");
+            backingSharingState->addLayerNeedingRepaint(layer);
+        } else
+            repaintOnCompositingChange(layer);
+    };
+
     if (backingRequired == BackingRequired::Yes) {
         layer.disconnectFromBackingProviderLayer();
 
@@ -1606,7 +1639,7 @@
         
         if (!layer.backing()) {
             // If we need to repaint, do so before making backing
-            repaintOnCompositingChange(layer);
+            repaintLayer(layer);
 
             layer.ensureBacking();
 
@@ -1656,7 +1689,7 @@
             layer.computeRepaintRectsIncludingDescendants();
 
             // If we need to repaint, do so now that we've removed the backing
-            repaintOnCompositingChange(layer);
+            repaintLayer(layer);
         }
     }
     
@@ -1696,9 +1729,9 @@
     return layerChanged;
 }
 
-bool RenderLayerCompositor::updateLayerCompositingState(RenderLayer& layer, const RenderLayer* compositingAncestor, RequiresCompositingData& queryData)
+bool RenderLayerCompositor::updateLayerCompositingState(RenderLayer& layer, const RenderLayer* compositingAncestor, RequiresCompositingData& queryData, BackingSharingState& backingSharingState)
 {
-    bool layerChanged = updateBacking(layer, queryData);
+    bool layerChanged = updateBacking(layer, queryData, &backingSharingState);
 
     // See if we need content or clipping layers. Methods called here should assume
     // that the compositing state of descendant layers has not been updated yet.
@@ -2226,6 +2259,25 @@
         recursiveRepaintLayer(*renderLayer);
 }
 
+bool RenderLayerCompositor::layerRepaintTargetsBackingSharingLayer(RenderLayer& layer, BackingSharingState& sharingState) const
+{
+    if (!sharingState.backingProviderCandidate())
+        return false;
+
+    for (const RenderLayer* currLayer = &layer; currLayer; currLayer = currLayer->paintOrderParent()) {
+        if (compositedWithOwnBackingStore(*currLayer))
+            return false;
+        
+        if (currLayer->paintsIntoProvidedBacking())
+            return false;
+
+        if (sharingState.isPotentialBackingSharingLayer(*currLayer))
+            return true;
+    }
+
+    return false;
+}
+
 RenderLayer& RenderLayerCompositor::rootRenderLayer() const
 {
     return *m_renderView.layer();

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (259014 => 259015)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2020-03-25 21:57:10 UTC (rev 259014)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2020-03-25 22:28:41 UTC (rev 259015)
@@ -206,9 +206,6 @@
         bool reevaluateAfterLayout { false };
     };
 
-    // Update the compositing state of the given layer. Returns true if that state changed.
-    bool updateLayerCompositingState(RenderLayer&, const RenderLayer* compositingAncestor, RequiresCompositingData&);
-
     // Whether layer's backing needs a graphics layer to do clipping by an ancestor (non-stacking-context parent with overflow).
     bool clippedByAncestor(RenderLayer&, const RenderLayer* compositingAncestor) const;
 
@@ -414,12 +411,14 @@
 
     // Make or destroy the backing for this layer; returns true if backing changed.
     enum class BackingRequired { No, Yes, Unknown };
-    bool updateBacking(RenderLayer&, RequiresCompositingData&, BackingRequired = BackingRequired::Unknown);
+    bool updateBacking(RenderLayer&, RequiresCompositingData&, BackingSharingState* = nullptr, BackingRequired = BackingRequired::Unknown);
+    bool updateLayerCompositingState(RenderLayer&, const RenderLayer* compositingAncestor, RequiresCompositingData&, BackingSharingState&);
 
     void clearBackingForLayerIncludingDescendants(RenderLayer&);
 
     // Repaint this and its child layers.
     void recursiveRepaintLayer(RenderLayer&);
+    bool layerRepaintTargetsBackingSharingLayer(RenderLayer&, BackingSharingState&) const;
 
     void computeExtent(const LayerOverlapMap&, const RenderLayer&, OverlapExtent&) const;
     void addToOverlapMap(LayerOverlapMap&, const RenderLayer&, OverlapExtent&) const;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to