Layers painting into shared backing need to contribute to overlap
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 19 May 2019 14:01:15 +0000 (14:01 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 19 May 2019 14:01:15 +0000 (14:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198021

Reviewed by Zalan Bujtas.
Source/WebCore:

Layers that paint into a composited (non-root) layer get added to the overlap map so
that later layers correct overlap them; this is done via the test against currentState.compositingAncestor.

We need the same logic for layers that paint into shared backing; they need to behave
the same way in terms of how they contribute to overlap. We already had currentState.backingSharingAncestor
which was unused, but now use it for this, and correctly null it out when a layer composites.

Bug was noticed during testing, and not known to affect any websites (though it probably does).

Also move the overlap container popping into updateOverlapMap() so the two callers can
share the code, and more explicitly track whether a container was pushed.

Test: compositing/shared-backing/sharing-child-contributes-to-overlap.html

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::computeCompositingRequirements):
(WebCore::RenderLayerCompositor::traverseUnchangedSubtree):
(WebCore::RenderLayerCompositor::updateOverlapMap const):
* rendering/RenderLayerCompositor.h:

LayoutTests:

* compositing/shared-backing/sharing-child-contributes-to-overlap-expected.html: Added.
* compositing/shared-backing/sharing-child-contributes-to-overlap.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap-expected.html [new file with mode: 0644]
LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index b8f7fd9..b052f61 100644 (file)
@@ -1,3 +1,13 @@
+2019-05-19  Simon Fraser  <simon.fraser@apple.com>
+
+        Layers painting into shared backing need to contribute to overlap
+        https://bugs.webkit.org/show_bug.cgi?id=198021
+
+        Reviewed by Zalan Bujtas.
+
+        * compositing/shared-backing/sharing-child-contributes-to-overlap-expected.html: Added.
+        * compositing/shared-backing/sharing-child-contributes-to-overlap.html: Added.
+
 2019-05-18  Jiewen Tan  <jiewen_tan@apple.com>
 
         [WebAuthN] Allow authenticators that support both CTAP and U2F to try U2F if CTAP fails in authenticatorGetAssertion
diff --git a/LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap-expected.html b/LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap-expected.html
new file mode 100644 (file)
index 0000000..33bc044
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .provider {
+            position: absolute;
+            top: 20px;
+            left: 20px;
+            height: 50px;
+            width: 50px;
+            margin: 10px;
+            border: 1px solid black;
+            background-color: silver;
+        }
+        
+        .sharing {
+                       position: relative;
+            z-index: 0;
+                       top: 45px;
+                       left: 45px;
+            width: 100px;
+            height: 100px;
+            background-color: blue;
+        }
+        
+        .inner {
+            position: absolute;
+            left: 80px;
+            top: 80px;
+            width: 200px;
+            height: 200px;
+            background-color: orange;
+        }
+
+        .trigger {
+            transform: translateZ(0);
+            width: 50px;
+            height: 50px;
+            background-color: silver;
+        }
+        
+        .overlapper {
+            position: absolute;
+            top: 200px;
+            left: 200px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+            transform: translateZ(0);
+        }
+    </style>
+</head>
+<body>
+    <div class="trigger"></div>
+    <div class="provider">
+        <div class="sharing">
+            <div class="inner">
+            </div>
+        </div>
+    </div>
+    <div class="overlapper"></div>
+<pre id="layers"></pre>
+</body>
+</html>
diff --git a/LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap.html b/LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap.html
new file mode 100644 (file)
index 0000000..6743304
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>Tests that the overlap bounds for a sharing layer includes descendants.</title>
+    <style>
+        .provider {
+            position: absolute;
+            top: 20px;
+            left: 20px;
+            height: 50px;
+            width: 50px;
+            margin: 10px;
+            border: 1px solid black;
+            background-color: silver;
+        }
+        
+        .sharing {
+                       position: relative;
+            z-index: 0;
+                       top: 45px;
+                       left: 45px;
+            width: 100px;
+            height: 100px;
+            background-color: blue;
+        }
+        
+        .inner {
+            position: absolute;
+            left: 80px;
+            top: 80px;
+            width: 200px;
+            height: 200px;
+            background-color: orange;
+        }
+
+        .trigger {
+            transform: translateZ(0);
+            width: 50px;
+            height: 50px;
+            background-color: silver;
+        }
+        
+        .overlapper {
+            position: absolute;
+            top: 200px;
+            left: 200px;
+            width: 200px;
+            height: 200px;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <div class="trigger"></div>
+    <div class="provider">
+        <div class="sharing">
+            <div class="inner">
+            </div>
+        </div>
+    </div>
+    <div class="overlapper"></div>
+<pre id="layers"></pre>
+</body>
+</html>
index 5714973..33b640b 100644 (file)
@@ -1,3 +1,30 @@
+2019-05-19  Simon Fraser  <simon.fraser@apple.com>
+
+        Layers painting into shared backing need to contribute to overlap
+        https://bugs.webkit.org/show_bug.cgi?id=198021
+
+        Reviewed by Zalan Bujtas.
+        
+        Layers that paint into a composited (non-root) layer get added to the overlap map so
+        that later layers correct overlap them; this is done via the test against currentState.compositingAncestor.
+
+        We need the same logic for layers that paint into shared backing; they need to behave
+        the same way in terms of how they contribute to overlap. We already had currentState.backingSharingAncestor
+        which was unused, but now use it for this, and correctly null it out when a layer composites.
+
+        Bug was noticed during testing, and not known to affect any websites (though it probably does).
+        
+        Also move the overlap container popping into updateOverlapMap() so the two callers can
+        share the code, and more explicitly track whether a container was pushed.
+
+        Test: compositing/shared-backing/sharing-child-contributes-to-overlap.html
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
+        (WebCore::RenderLayerCompositor::traverseUnchangedSubtree):
+        (WebCore::RenderLayerCompositor::updateOverlapMap const):
+        * rendering/RenderLayerCompositor.h:
+
 2019-05-17  Joonghun Park  <pjh0718@gmail.com>
 
         Implement CSS `display: flow-root` (modern clearfix)
index 0482757..f9bf2c5 100644 (file)
@@ -879,6 +879,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
 
     IndirectCompositingReason compositingReason = compositingState.subtreeIsCompositing ? IndirectCompositingReason::Stacking : IndirectCompositingReason::None;
     bool layerPaintsIntoProvidedBacking = false;
+    bool didPushOverlapContainer = false;
 
     // If we know for sure the layer is going to be composited, don't bother looking it up in the overlap map
     if (!willBeComposited && !overlapMap.isEmpty() && compositingState.testingOverlap) {
@@ -927,15 +928,18 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
         currentState.testingOverlap = true;
         // This layer now acts as the ancestor for kids.
         currentState.compositingAncestor = &layer;
-        
+        // Compositing turns off backing sharing.
+        currentState.backingSharingAncestor = nullptr;
+
         if (layerPaintsIntoProvidedBacking) {
             layerPaintsIntoProvidedBacking = false;
             // layerPaintsIntoProvidedBacking was only true for layers that would otherwise composite because of overlap. If we can
             // no longer share, put this this indirect reason back on the layer so that requiresOwnBackingStore() sees it.
             layer.setIndirectCompositingReason(IndirectCompositingReason::Overlap);
-            LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " was sharing now will composite");
+            LOG_WITH_STREAM(Compositing, stream << "layer " << &layer << " was sharing now will composite");
         } else {
             overlapMap.pushCompositingContainer();
+            didPushOverlapContainer = true;
             LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " will composite, pushed container " << overlapMap);
         }
 
@@ -958,6 +962,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     } else if (layerPaintsIntoProvidedBacking) {
         currentState.backingSharingAncestor = &layer;
         overlapMap.pushCompositingContainer();
+        didPushOverlapContainer = true;
         LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " will share, pushed container " << overlapMap);
     }
 
@@ -1064,17 +1069,11 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     // Compute state passed to the caller.
     descendantHas3DTransform |= anyDescendantHas3DTransform || layer.has3DTransform();
     compositingState.updateWithDescendantStateAndLayer(currentState, layer, layerExtent);
+    backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
 
-    bool layerContributesToOverlap = currentState.compositingAncestor && !currentState.compositingAncestor->isRenderViewLayer();
-    updateOverlapMap(overlapMap, layer, layerExtent, layerContributesToOverlap, becameCompositedAfterDescendantTraversal && !descendantsAddedToOverlap);
-
-    // Pop backing/overlap sharing state.
-    if ((willBeComposited && !layer.isRenderViewLayer()) || currentState.backingSharingAncestor == &layer) {
-        overlapMap.popCompositingContainer();
-        LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " is composited, popped container " << overlapMap);
-    }
+    bool layerContributesToOverlap = (currentState.compositingAncestor && !currentState.compositingAncestor->isRenderViewLayer()) || currentState.backingSharingAncestor;
+    updateOverlapMap(overlapMap, layer, layerExtent, didPushOverlapContainer, layerContributesToOverlap, becameCompositedAfterDescendantTraversal && !descendantsAddedToOverlap);
 
-    backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
     overlapMap.geometryMap().popMappingsToAncestor(ancestorLayer);
 
     LOG_WITH_STREAM(Compositing, stream << TextStream::Repeat(compositingState.depth * 2, ' ') << &layer << " computeCompositingRequirements - willBeComposited " << willBeComposited << " (backing provider candidate " << backingSharingState.backingProviderCandidate() << ")");
@@ -1093,6 +1092,8 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
     layer.updateLayerListsIfNeeded();
 
     bool layerIsComposited = layer.isComposited();
+    bool layerPaintsIntoProvidedBacking = false;
+    bool didPushOverlapContainer = false;
 
     OverlapExtent layerExtent;
     if (layerIsComposited && !layer.isRenderViewLayer())
@@ -1109,6 +1110,7 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
         ASSERT(backingSharingState.backingProviderCandidate());
         ASSERT(backingProviderLayerCanIncludeLayer(*backingSharingState.backingProviderCandidate(), layer));
         backingSharingState.appendSharingLayer(layer);
+        layerPaintsIntoProvidedBacking = true;
     }
 
     CompositingState currentState = compositingState.stateForPaintOrderChildren(layer);
@@ -1119,13 +1121,20 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
         currentState.testingOverlap = true;
         // This layer now acts as the ancestor for kids.
         currentState.compositingAncestor = &layer;
+        currentState.backingSharingAncestor = nullptr;
         overlapMap.pushCompositingContainer();
+        didPushOverlapContainer = true;
         LOG_WITH_STREAM(CompositingOverlap, stream << "unchangedSubtree: layer " << &layer << " will composite, pushed container " << overlapMap);
 
         computeExtent(overlapMap, layer, layerExtent);
         currentState.ancestorHasTransformAnimation |= layerExtent.hasTransformAnimation;
         // Too hard to compute animated bounds if both us and some ancestor is animating transform.
         layerExtent.animationCausesExtentUncertainty |= layerExtent.hasTransformAnimation && compositingState.ancestorHasTransformAnimation;
+    } else if (layerPaintsIntoProvidedBacking) {
+        overlapMap.pushCompositingContainer();
+        currentState.backingSharingAncestor = &layer;
+        didPushOverlapContainer = true;
+        LOG_WITH_STREAM(CompositingOverlap, stream << "unchangedSubtree: layer " << &layer << " will share, pushed container " << overlapMap);
     }
 
     backingSharingState.updateBeforeDescendantTraversal(layer, layerIsComposited);
@@ -1156,16 +1165,11 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
 
     ASSERT(!currentState.fullPaintOrderTraversalRequired);
     compositingState.updateWithDescendantStateAndLayer(currentState, layer, layerExtent, true);
+    backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
 
-    bool layerContributesToOverlap = currentState.compositingAncestor && !currentState.compositingAncestor->isRenderViewLayer();
-    updateOverlapMap(overlapMap, layer, layerExtent, layerContributesToOverlap);
-
-    if ((layerIsComposited && !layer.isRenderViewLayer()) || currentState.backingSharingAncestor == &layer) {
-        overlapMap.popCompositingContainer();
-        LOG_WITH_STREAM(CompositingOverlap, stream << "unchangedSubtree: layer " << &layer << " is composited, popped container " << overlapMap);
-    }
+    bool layerContributesToOverlap = (currentState.compositingAncestor && !currentState.compositingAncestor->isRenderViewLayer()) || currentState.backingSharingAncestor;
+    updateOverlapMap(overlapMap, layer, layerExtent, didPushOverlapContainer, layerContributesToOverlap);
 
-    backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
     overlapMap.geometryMap().popMappingsToAncestor(ancestorLayer);
 
     ASSERT(!layer.needsCompositingRequirementsTraversal());
@@ -1840,16 +1844,9 @@ void RenderLayerCompositor::addDescendantsToOverlapMapRecursive(LayerOverlapMap&
         overlapMap.geometryMap().popMappingsToAncestor(ancestorLayer);
 }
 
-void RenderLayerCompositor::updateOverlapMap(LayerOverlapMap& overlapMap, const RenderLayer& layer, OverlapExtent& layerExtent, bool layerContributesToOverlap, bool addDescendantsToOverlap) const
+void RenderLayerCompositor::updateOverlapMap(LayerOverlapMap& overlapMap, const RenderLayer& layer, OverlapExtent& layerExtent, bool didPushContainer, bool addLayerToOverlap, bool addDescendantsToOverlap) const
 {
-    ASSERT_IMPLIES(addDescendantsToOverlap, layerContributesToOverlap);
-
-    // All layers (even ones that aren't being composited) need to get added to
-    // the overlap map. Layers that do not composite will draw into their
-    // compositing ancestor's backing, and so are still considered for overlap.
-    // FIXME: When layerExtent has taken animation bounds into account, we also know that the bounds
-    // include descendants, so we don't need to add them all to the overlap map.
-    if (layerContributesToOverlap) {
+    if (addLayerToOverlap) {
         addToOverlapMap(overlapMap, layer, layerExtent);
         LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " contributes to overlap, added to map " << overlapMap);
     }
@@ -1859,6 +1856,11 @@ void RenderLayerCompositor::updateOverlapMap(LayerOverlapMap& overlapMap, const
         addDescendantsToOverlapMapRecursive(overlapMap, layer);
         LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " composited post descendant traversal, added recursive " << overlapMap);
     }
+
+    if (didPushContainer) {
+        overlapMap.popCompositingContainer();
+        LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " is composited or shared, popped container " << overlapMap);
+    }
 }
 
 #if ENABLE(VIDEO)
index 3bc2359..7f66d18 100644 (file)
@@ -408,7 +408,7 @@ private:
     void computeExtent(const LayerOverlapMap&, const RenderLayer&, OverlapExtent&) const;
     void addToOverlapMap(LayerOverlapMap&, const RenderLayer&, OverlapExtent&) const;
     void addDescendantsToOverlapMapRecursive(LayerOverlapMap&, const RenderLayer&, const RenderLayer* ancestorLayer = nullptr) const;
-    void updateOverlapMap(LayerOverlapMap&, const RenderLayer&, OverlapExtent&, bool layerContributesToOverlap, bool addDescendantsToOverlap = false) const;
+    void updateOverlapMap(LayerOverlapMap&, const RenderLayer&, OverlapExtent&, bool didPushContainer, bool addLayerToOverlap, bool addDescendantsToOverlap = false) const;
 
     void updateCompositingLayersTimerFired();