Fix negative z-index layers from triggering unnecessary compositing o…

…f foreground layers. https://bugs.webkit.org/show_bug.cgi?id=244543 <rdar://99335877> Reviewed by Simon Fraser. We currently push a speculative compositing container onto the overlap stack when processing negative z-index children, in case any of them require compositing. We then pop it off it it doesn't get used. Unfortunately other mutations can happen to the overlap stack while it's pushed, and just popping the unused container isn't sufficient to restore this. This instead duplicates the overlap stack when we push a speculative compositing container, and makes any changes to both of them. Once we know which path is correct, we select the correct stack and throw away the duplicate. This fixes cases where negative z-index children weren't composited, but they ended up affecting overlaps of positive z-index children and making them composited unnecessarily. * LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling-expected.txt: Added. * LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling.html: Added. * Source/WebCore/rendering/LayerOverlapMap.cpp: (WebCore::OverlapMapContainer::isEmpty const): (WebCore::LayerOverlapMap::add): (WebCore::LayerOverlapMap::overlapsLayers const): (WebCore::LayerOverlapMap::pushCompositingContainer): (WebCore::LayerOverlapMap::pushSpeculativeCompositingContainer): (WebCore::LayerOverlapMap::confirmSpeculativeCompositingContainer): (WebCore::LayerOverlapMap::maybePopSpeculativeCompositingContainer): * Source/WebCore/rendering/LayerOverlapMap.h: * Source/WebCore/rendering/RenderLayerCompositor.cpp: (WebCore::RenderLayerCompositor::computeCompositingRequirements): Canonical link: https://commits.webkit.org/254746@main
WebKit · Sep 22, 2022 · be575e7192eed0f7fc0d3a335a1df8f91e909b30 · be575e7
1 parent c65efd2
commit be575e7192eed0f7fc0d3a335a1df8f91e909b30
Show file tree

Hide file tree

Showing 9 changed files with 244 additions and 8 deletions.
diff --git a/LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling-expected.txt b/LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling-expected.txt
@@ -0,0 +1,24 @@
+
+(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)
+          (preserves3D 1)
+          (children 1
+            (GraphicsLayer
+              (bounds 792.00 540.00)
+              (drawsContent 1)
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
diff --git a/...ests/compositing/layer-creation/no-compositing-for-negative-z-sibling-nested-expected.txt b/...ests/compositing/layer-creation/no-compositing-for-negative-z-sibling-nested-expected.txt
@@ -0,0 +1,31 @@
+
+(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)
+          (preserves3D 1)
+          (children 1
+            (GraphicsLayer
+              (bounds 204.00 121.00)
+              (children 1
+                (GraphicsLayer
+                  (anchor 0.00 0.00)
+                  (bounds 204.00 108.00)
+                  (drawsContent 1)
+                  (transform [5.00 0.00 0.00 0.00] [0.00 5.00 0.00 0.00] [0.00 0.00 1.00 0.00] [0.00 0.00 0.00 1.00])
+                )
+              )
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling-nested.html b/LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling-nested.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <title>test</title>
+  <script>
+    if (window.testRunner) {
+      testRunner.waitUntilDone();
+      testRunner.dumpAsText();
+    }
+
+    function doTest()
+    {
+      document.getElementById('results').innerText = window.internals.layerTreeAsText(document);
+      if (window.testRunner) {
+        testRunner.notifyDone();
+      }
+    }
+
+    window.addEventListener('load', doTest, false);
+  </script>
+</head>
+
+<body style="position:fixed">
+  <div id="container" style="transform-origin: left top; transform: scale(5); ">
+    <div id="negative" style="position: relative; z-index: -1;">
+        <div style="position: relative; z-index:-1"></div>
+    </div>
+    <div id="positive" style="position: relative">
+      <iframe srcdoc="Hello, world" style="width: 200px; height: 100px;"></iframe>
+    </div>
+  </div>
+<pre id="results"></pre>
+</body>
+
+</html>
+
diff --git a/LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling.html b/LayoutTests/compositing/layer-creation/no-compositing-for-negative-z-sibling.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <title>test</title>
+  <script>
+    if (window.testRunner) {
+      testRunner.waitUntilDone();
+      testRunner.dumpAsText();
+    }
+
+    function doTest()
+    {
+      document.getElementById('results').innerText = window.internals.layerTreeAsText(document);
+      if (window.testRunner) {
+        testRunner.notifyDone();
+      }
+    }
+
+    window.addEventListener('load', doTest, false);
+  </script>
+</head>
+
+<body style="position:fixed">
+  <div id="container" style="transform-origin: left top; transform: scale(5); ">
+    <div id="negative" style="position: relative; z-index: -1;"></div>
+    <div id="positive" style="position: relative">
+      <iframe srcdoc="Hello, world" style="width: 200px; height: 100px;"></iframe>
+    </div>
+  </div>
+<pre id="results"></pre>
+</body>
+
+</html>
+
diff --git a/...orm/ios-wk2/compositing/layer-creation/no-compositing-for-negative-z-sibling-expected.txt b/...orm/ios-wk2/compositing/layer-creation/no-compositing-for-negative-z-sibling-expected.txt
@@ -0,0 +1,24 @@
+
+(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)
+          (preserves3D 1)
+          (children 1
+            (GraphicsLayer
+              (bounds 792.00 545.00)
+              (drawsContent 1)
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
diff --git a/...-wk2/compositing/layer-creation/no-compositing-for-negative-z-sibling-nested-expected.txt b/...-wk2/compositing/layer-creation/no-compositing-for-negative-z-sibling-nested-expected.txt
@@ -0,0 +1,31 @@
+
+(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)
+          (preserves3D 1)
+          (children 1
+            (GraphicsLayer
+              (bounds 204.00 122.00)
+              (children 1
+                (GraphicsLayer
+                  (anchor 0.00 0.00)
+                  (bounds 204.00 109.00)
+                  (drawsContent 1)
+                  (transform [5.00 0.00 0.00 0.00] [0.00 5.00 0.00 0.00] [0.00 0.00 1.00 0.00] [0.00 0.00 0.00 1.00])
+                )
+              )
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
diff --git a/Source/WebCore/rendering/LayerOverlapMap.cpp b/Source/WebCore/rendering/LayerOverlapMap.cpp
@@ -84,6 +84,8 @@ class OverlapMapContainer {
     void add(const RenderLayer&, const LayoutRect& bounds, const Vector<LayerOverlapMap::LayerAndBounds>& enclosingClippingLayers);
     bool overlapsLayers(const RenderLayer&, const LayoutRect& bounds, const Vector<LayerOverlapMap::LayerAndBounds>& enclosingClippingLayers) const;
     void append(std::unique_ptr<OverlapMapContainer>&&);
+
+    bool isEmpty() const;
 
     String dump(unsigned) const;
 
@@ -167,6 +169,11 @@ class OverlapMapContainer {
     const RenderLayer& m_scopeLayer;
 };
 
+bool OverlapMapContainer::isEmpty() const
+{
+    return m_rootScope.rectList.rects.isEmpty() && m_rootScope.children.isEmpty();
+}
+
 void OverlapMapContainer::add(const RenderLayer&, const LayoutRect& bounds, const Vector<LayerOverlapMap::LayerAndBounds>& enclosingClippingLayers)
 {
     auto* layerScope = ensureClippingScopeForLayers(enclosingClippingLayers);
@@ -291,6 +298,10 @@ void LayerOverlapMap::add(const RenderLayer& layer, const LayoutRect& bounds, co
     ASSERT(m_overlapStack.size() >= 2);
     auto& container = m_overlapStack[m_overlapStack.size() - 2];
     container->add(layer, bounds, enclosingClippingLayers);
+    if (m_speculativeOverlapStack.size()) {
+        ASSERT(m_speculativeOverlapStack.size() >= 2);
+        m_speculativeOverlapStack[m_speculativeOverlapStack.size() - 2]->add(layer, bounds, enclosingClippingLayers);
+    }
 
     LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " contributes to overlap in the scope of layer " << &container->scopeLayer() << ", added to map " << *this);
 
@@ -299,11 +310,15 @@ void LayerOverlapMap::add(const RenderLayer& layer, const LayoutRect& bounds, co
 
 bool LayerOverlapMap::overlapsLayers(const RenderLayer& layer, const LayoutRect& bounds, const Vector<LayerAndBounds>& enclosingClippingLayers) const
 {
-    return m_overlapStack.last()->overlapsLayers(layer, bounds, enclosingClippingLayers);
+    if (m_speculativeOverlapStack.isEmpty())
+        return m_overlapStack.last()->overlapsLayers(layer, bounds, enclosingClippingLayers);
+    ASSERT(m_speculativeOverlapStack.last()->isEmpty());
+    return false;
 }
 
 void LayerOverlapMap::pushCompositingContainer(const RenderLayer& layer)
 {
+    confirmSpeculativeCompositingContainer();
     m_overlapStack.append(makeUnique<OverlapMapContainer>(m_rootLayer, layer));
 }
 
@@ -314,6 +329,38 @@ void LayerOverlapMap::popCompositingContainer(const RenderLayer& layer)
     m_overlapStack.removeLast();
 }
 
+void LayerOverlapMap::pushSpeculativeCompositingContainer(const RenderLayer& layer)
+{
+    // Create a duplicate copy of the overlap stack, push the container to one,
+    // and make all future add calls apply to both stacks.  
+    // If we end up needing the compositing container, then we copy across the speculative
+    // stack to replace the main, otherwise we throw the speculative stack away.
+    // If we already have a speculative stack (and we've recursed into this), then just force
+    // a real compositing container on the outer, since otherwise we'd have to start
+    // tracking four possible outcomes.
+    confirmSpeculativeCompositingContainer();
+    for (auto& container : m_overlapStack)
+        m_speculativeOverlapStack.append(makeUnique<OverlapMapContainer>(*container));
+    m_speculativeOverlapStack.append(makeUnique<OverlapMapContainer>(m_rootLayer, layer));
+}
+
+void LayerOverlapMap::confirmSpeculativeCompositingContainer()
+{
+    if (m_speculativeOverlapStack.size()) {
+        m_overlapStack.clear();
+        m_overlapStack.swap(m_speculativeOverlapStack);
+    }
+}
+
+bool LayerOverlapMap::maybePopSpeculativeCompositingContainer()
+{
+    if (m_speculativeOverlapStack.size()) {
+        m_speculativeOverlapStack.clear();
+        return true;
+    }
+    return false;
+}
+
 static TextStream& operator<<(TextStream& ts, const OverlapMapContainer& container)
 {
     ts << container.dump(ts.indent());

diff --git a/Source/WebCore/rendering/LayerOverlapMap.h b/Source/WebCore/rendering/LayerOverlapMap.h
@@ -56,13 +56,18 @@ class LayerOverlapMap {
     void pushCompositingContainer(const RenderLayer&);
     void popCompositingContainer(const RenderLayer&);
 
+    void pushSpeculativeCompositingContainer(const RenderLayer&);
+    void confirmSpeculativeCompositingContainer();
+    bool maybePopSpeculativeCompositingContainer();
+
     const RenderGeometryMap& geometryMap() const { return m_geometryMap; }
     RenderGeometryMap& geometryMap() { return m_geometryMap; }
 
     const Vector<std::unique_ptr<OverlapMapContainer>>& overlapStack() const { return m_overlapStack; }
 
 private:
     Vector<std::unique_ptr<OverlapMapContainer>> m_overlapStack;
+    Vector<std::unique_ptr<OverlapMapContainer>> m_speculativeOverlapStack;
     RenderGeometryMap m_geometryMap;
     const RenderLayer& m_rootLayer;
     bool m_isEmpty { true };

diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
@@ -1107,13 +1107,11 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
         // Speculatively push this layer onto the overlap map.
         bool didSpeculativelyPushOverlapContainer = false;
         if (!didPushOverlapContainer) {
-            overlapMap.pushCompositingContainer(layer);
+            overlapMap.pushSpeculativeCompositingContainer(layer);
             didPushOverlapContainer = true;
             didSpeculativelyPushOverlapContainer = true;
         }
 
-        bool compositeForNegativeZOrderDescendant = false;
-
         for (auto* childLayer : layer.negativeZOrderLayers()) {
             computeCompositingRequirements(&layer, *childLayer, overlapMap, currentState, backingSharingState, anyDescendantHas3DTransform);
 
@@ -1122,13 +1120,17 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
             if (!willBeComposited && currentState.subtreeIsCompositing) {
                 layer.setIndirectCompositingReason(IndirectCompositingReason::BackgroundLayer);
                 layerWillComposite();
-                compositeForNegativeZOrderDescendant = true;
+                overlapMap.confirmSpeculativeCompositingContainer();
             }
         }
 
-        if (!compositeForNegativeZOrderDescendant && didSpeculativelyPushOverlapContainer) {
-            overlapMap.popCompositingContainer(layer);
-            didPushOverlapContainer = false;
+        if (didSpeculativelyPushOverlapContainer) {
+            if (overlapMap.maybePopSpeculativeCompositingContainer())
+                didPushOverlapContainer = false;
+            else if (!willBeComposited) {
+                layer.setIndirectCompositingReason(IndirectCompositingReason::BackgroundLayer);
+                layerWillComposite();
+            }
         }
     }