Attachment 370204 Details for Bug 198021 – Patch

[patch] Patch

bug-198021-20190518165305.patch (text/plain), 17.08 KB, created by Simon Fraser (smfr) on 2019-05-18 16:53:06 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-05-18 16:53:06 PDT

Size: 17.08 KB

patch

obsolete

>Subversion Revision: 245498
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 571497342240f02fbf2324b49c75c68f57e010e2..5fc88b929affee1aecd9fe93bdbb6591c261e27f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-05-18  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 NOBODY (OOPS!).
>+        
>+        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)
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index 04827571a46b0dad316d0476162805b7f3bd97aa..f9bf2c5674fede227954cf754e6ea0de86bb1a32 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -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)
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.h b/Source/WebCore/rendering/RenderLayerCompositor.h
>index 3bc23590931605366fd704900d0c54a1ccd959c3..7f66d1866dcc2b66b1112ab8487083eff2ca07df 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.h
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.h
>@@ -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();
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 60a6b0287a30be8951b073278b63679113dd8c7a..0e935ad1a44d28ab37297a7203b763f1d790211c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-05-18  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 NOBODY (OOPS!).
>+
>+        * compositing/shared-backing/sharing-child-contributes-to-overlap-expected.html: Added.
>+        * compositing/shared-backing/sharing-child-contributes-to-overlap.html: Added.
>+
> 2019-05-17  Joonghun Park  <pjh0718@gmail.com>
> 
>         Implement CSS `display: flow-root` (modern clearfix)
>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
>index 0000000000000000000000000000000000000000..33bc0442a68851fb9f741f1a6dcd361c33d4ea20
>--- /dev/null
>+++ b/LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap-expected.html
>@@ -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
>index 0000000000000000000000000000000000000000..6743304adc4c42b92c8bdaaad35bac552fcacef4
>--- /dev/null
>+++ b/LayoutTests/compositing/shared-backing/sharing-child-contributes-to-overlap.html
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 198021: 370204