Flashing and partly visible elements
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Mar 2020 22:28:41 +0000 (22:28 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Mar 2020 22:28:41 +0000 (22:28 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/compositing/shared-backing/repaint-into-shared-backing-expected.html [new file with mode: 0644]
LayoutTests/compositing/shared-backing/repaint-into-shared-backing.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index 9aa2a42..06021f1 100644 (file)
@@ -1,3 +1,13 @@
+2020-03-25  Simon Fraser  <simon.fraser@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  <lawrence.j@apple.com>
 
         [ Mac wk2 ] svg/as-image/svg-image-with-data-uri-background.html is flaky failing.
diff --git a/LayoutTests/compositing/shared-backing/repaint-into-shared-backing-expected.html b/LayoutTests/compositing/shared-backing/repaint-into-shared-backing-expected.html
new file mode 100644 (file)
index 0000000..ffefa3d
--- /dev/null
@@ -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
diff --git a/LayoutTests/compositing/shared-backing/repaint-into-shared-backing.html b/LayoutTests/compositing/shared-backing/repaint-into-shared-backing.html
new file mode 100644 (file)
index 0000000..356665b
--- /dev/null
@@ -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
index 71cd891..8d586fe 100644 (file)
@@ -1,3 +1,36 @@
+2020-03-25  Simon Fraser  <simon.fraser@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  <cdumez@apple.com>
 
         Event listeners registered with 'once' option may get garbage collected too soon
index ef4099a..3a038ce 100644 (file)
@@ -229,16 +229,30 @@ public:
 
     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 @@ void RenderLayerCompositor::BackingSharingState::endBackingSharingSequence()
     if (m_backingProviderCandidate) {
         m_backingProviderCandidate->backing()->setBackingSharingLayers(WTFMove(m_backingSharingLayers));
         m_backingSharingLayers.clear();
+        issuePendingRepaints();
     }
     
     m_backingProviderCandidate = nullptr;
@@ -292,6 +307,16 @@ void RenderLayerCompositor::BackingSharingState::updateAfterDescendantTraversal(
         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 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
 
     // 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 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     }
 
     // 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 @@ void RenderLayerCompositor::updateRootContentLayerClipping()
     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 @@ bool RenderLayerCompositor::updateBacking(RenderLayer& layer, RequiresCompositin
         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 @@ bool RenderLayerCompositor::updateBacking(RenderLayer& layer, RequiresCompositin
         
         if (!layer.backing()) {
             // If we need to repaint, do so before making backing
-            repaintOnCompositingChange(layer);
+            repaintLayer(layer);
 
             layer.ensureBacking();
 
@@ -1656,7 +1689,7 @@ bool RenderLayerCompositor::updateBacking(RenderLayer& layer, RequiresCompositin
             layer.computeRepaintRectsIncludingDescendants();
 
             // If we need to repaint, do so now that we've removed the backing
-            repaintOnCompositingChange(layer);
+            repaintLayer(layer);
         }
     }
     
@@ -1696,9 +1729,9 @@ bool RenderLayerCompositor::updateBacking(RenderLayer& layer, RequiresCompositin
     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 @@ void RenderLayerCompositor::recursiveRepaintLayer(RenderLayer& layer)
         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();
index ece258a..946dfc4 100644 (file)
@@ -206,9 +206,6 @@ public:
         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 @@ private:
 
     // 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;