Attachment 369678 Details for Bug 197824 – Patch

[patch] Patch

bug-197824-20190512110923.patch (text/plain), 18.92 KB, created by Simon Fraser (smfr) on 2019-05-12 11:09:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-05-12 11:09:24 PDT

Size: 18.92 KB

patch

obsolete

>Subversion Revision: 245212
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 626c55f197498d417bd040ea9af0359c4dab5a38..a31e10da89e3bfe555adc7d08c11924cf09336b1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,38 @@
>+2019-05-12  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Refactor composited backing-sharing code
>+        https://bugs.webkit.org/show_bug.cgi?id=197824
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Clean up the backing-sharing code to share more code, and make it easier to understand.
>+        
>+        Moves more logic into member functions on BackingSharingState, which are named to make
>+        their functions clearer: startBackingSharingSequence/endBackingSharingSequence.
>+        
>+        computeCompositingRequirements() and traverseUnchangedSubtree() now just call
>+        updateBeforeDescendantTraversal/updateAfterDescendantTraversal.
>+
>+        No behavior change.
>+
>+        * rendering/RenderLayerBacking.cpp:
>+        (WebCore::RenderLayerBacking::willBeDestroyed):
>+        (WebCore::RenderLayerBacking::setBackingSharingLayers): Remove the early return, since
>+        we need to call setBackingProviderLayer() on the sharing layers in both code paths.
>+        (WebCore::RenderLayerBacking::removeBackingSharingLayer):
>+        (WebCore::RenderLayerBacking::clearBackingSharingLayers):
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::BackingSharingState::backingProviderCandidate const):
>+        (WebCore::RenderLayerCompositor::BackingSharingState::appendSharingLayer):
>+        (WebCore::RenderLayerCompositor::BackingSharingState::startBackingSharingSequence):
>+        (WebCore::RenderLayerCompositor::BackingSharingState::endBackingSharingSequence):
>+        (WebCore::RenderLayerCompositor::BackingSharingState::updateBeforeDescendantTraversal):
>+        (WebCore::RenderLayerCompositor::BackingSharingState::updateAfterDescendantTraversal):
>+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
>+        (WebCore::RenderLayerCompositor::traverseUnchangedSubtree):
>+        (WebCore::RenderLayerCompositor::BackingSharingState::resetBackingProviderCandidate): Deleted.
>+        * rendering/RenderLayerCompositor.h:
>+
> 2019-05-11  Simon Fraser  <simon.fraser@apple.com>
> 
>         Overflow scroll that becomes non-scrollable should stop being composited
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index 43d1a575ccd6c45f012b4e4ae05692b9a53c7035..f839e46e3ebd04e23439b5a940ac13d3a9bf52a1 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -260,8 +260,6 @@ void RenderLayerBacking::willBeDestroyed()
>     ASSERT(m_owningLayer.backing() == this);
>     compositor().removeFromScrollCoordinatedLayers(m_owningLayer);
> 
>-    LOG(Compositing, "RenderLayer(backing) %p willBeDestroyed", &m_owningLayer);
>-
>     clearBackingSharingLayers();
> }
> 
>@@ -282,29 +280,21 @@ static void clearBackingSharingLayerProviders(Vector<WeakPtr<RenderLayer>>& shar
> 
> void RenderLayerBacking::setBackingSharingLayers(Vector<WeakPtr<RenderLayer>>&& sharingLayers)
> {
>-    if (m_backingSharingLayers == sharingLayers) {
>-        sharingLayers.clear();
>-        return;
>-    }
>-
>     clearBackingSharingLayerProviders(m_backingSharingLayers);
>     m_backingSharingLayers = WTFMove(sharingLayers);
>+
>     for (auto& layerWeakPtr : m_backingSharingLayers)
>         layerWeakPtr->setBackingProviderLayer(&m_owningLayer);
> }
> 
> void RenderLayerBacking::removeBackingSharingLayer(RenderLayer& layer)
> {
>-    LOG(Compositing, "RenderLayer %p removeBackingSharingLayer %p", &m_owningLayer, &layer);
>-
>     layer.setBackingProviderLayer(nullptr);
>     m_backingSharingLayers.removeAll(&layer);
> }
> 
> void RenderLayerBacking::clearBackingSharingLayers()
> {
>-    LOG(Compositing, "RenderLayer %p clearBackingSharingLayers", &m_owningLayer);
>-
>     clearBackingSharingLayerProviders(m_backingSharingLayers);
>     m_backingSharingLayers.clear();
> }
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index 283273c0f97aa8a41b3df61271d284842231eec8..07e8cc79b95e4f0c3d176d5ac429bdd651f04a5e 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -284,22 +284,85 @@ struct RenderLayerCompositor::CompositingState {
> #endif
> };
> 
>-struct RenderLayerCompositor::BackingSharingState {
>-    RenderLayer* backingProviderCandidate { nullptr };
>-    RenderLayer* backingProviderStackingContext { nullptr };
>-    Vector<WeakPtr<RenderLayer>> backingSharingLayers;
>+class RenderLayerCompositor::BackingSharingState {
>+    WTF_MAKE_NONCOPYABLE(BackingSharingState);
>+public:
>+    BackingSharingState() = default;
> 
>-    void resetBackingProviderCandidate(RenderLayer* candidateLayer = nullptr, RenderLayer* candidateStackingContext = nullptr)
>+    RenderLayer* backingProviderCandidate() const { return m_backingProviderCandidate; };
>+    
>+    void appendSharingLayer(RenderLayer& layer)
>     {
>-        if (!backingSharingLayers.isEmpty()) {
>-            ASSERT(backingProviderCandidate);
>-            backingProviderCandidate->backing()->setBackingSharingLayers(WTFMove(backingSharingLayers));
>-        }
>-        backingProviderCandidate = candidateLayer;
>-        backingProviderStackingContext = candidateLayer ? candidateStackingContext : nullptr;
>+        LOG_WITH_STREAM(Compositing, stream << &layer << " appendSharingLayer " << &layer << " for backing provider " << m_backingProviderCandidate);
>+        m_backingSharingLayers.append(makeWeakPtr(layer));
>     }
>+
>+    void updateBeforeDescendantTraversal(RenderLayer&, bool willBeComposited);
>+    void updateAfterDescendantTraversal(RenderLayer&, RenderLayer* stackingContextAncestor);
>+
>+private:
>+    void layerWillBeComposited(RenderLayer&);
>+
>+    void startBackingSharingSequence(RenderLayer& candidateLayer, RenderLayer* candidateStackingContext);
>+    void endBackingSharingSequence();
>+
>+    RenderLayer* m_backingProviderCandidate { nullptr };
>+    RenderLayer* m_backingProviderStackingContext { nullptr };
>+    Vector<WeakPtr<RenderLayer>> m_backingSharingLayers;
> };
> 
>+void RenderLayerCompositor::BackingSharingState::startBackingSharingSequence(RenderLayer& candidateLayer, RenderLayer* candidateStackingContext)
>+{
>+    ASSERT(!m_backingProviderCandidate);
>+    ASSERT(m_backingSharingLayers.isEmpty());
>+
>+    m_backingProviderCandidate = &candidateLayer;
>+    m_backingProviderStackingContext = candidateStackingContext;
>+}
>+
>+void RenderLayerCompositor::BackingSharingState::endBackingSharingSequence()
>+{
>+    if (m_backingProviderCandidate) {
>+        m_backingProviderCandidate->backing()->setBackingSharingLayers(WTFMove(m_backingSharingLayers));
>+        m_backingSharingLayers.clear();
>+    }
>+    
>+    m_backingProviderCandidate = nullptr;
>+}
>+
>+void RenderLayerCompositor::BackingSharingState::updateBeforeDescendantTraversal(RenderLayer& layer, bool willBeComposited)
>+{
>+    layer.setBackingProviderLayer(nullptr);
>+
>+    // A layer that composites resets backing-sharing, since subsequent layers need to composite to overlap it.
>+    if (willBeComposited) {
>+        m_backingSharingLayers.removeAll(&layer);
>+        LOG_WITH_STREAM(Compositing, stream << "Pre-descendant compositing of " << &layer << ", ending sharing sequence for " << m_backingProviderCandidate << " with " << m_backingSharingLayers.size() << " sharing layers");
>+        endBackingSharingSequence();
>+    }
>+}
>+
>+void RenderLayerCompositor::BackingSharingState::updateAfterDescendantTraversal(RenderLayer& layer, RenderLayer* stackingContextAncestor)
>+{
>+    if (layer.isComposited()) {
>+        // If this layer is being composited, clean up sharing-related state.
>+        layer.disconnectFromBackingProviderLayer();
>+        m_backingSharingLayers.removeAll(&layer);
>+    }
>+
>+    if (m_backingProviderCandidate && &layer == m_backingProviderStackingContext) {
>+        LOG_WITH_STREAM(Compositing, stream << "End of stacking context for backing provider " << m_backingProviderCandidate << ", ending sharing sequence with " << m_backingSharingLayers.size() << " sharing layers");
>+        endBackingSharingSequence();
>+    } else if (!m_backingProviderCandidate && layer.isComposited()) {
>+        LOG_WITH_STREAM(Compositing, stream << "Post-descendant compositing of " << &layer << ", ending sharing sequence for " << m_backingProviderCandidate << " with " << m_backingSharingLayers.size() << " sharing layers");
>+        endBackingSharingSequence();
>+        startBackingSharingSequence(layer, stackingContextAncestor);
>+    }
>+    
>+    if (&layer != m_backingProviderCandidate && layer.isComposited())
>+        layer.backing()->clearBackingSharingLayers();
>+}
>+
> struct RenderLayerCompositor::OverlapExtent {
>     LayoutRect bounds;
>     bool extentComputed { false };
>@@ -880,7 +943,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>     }
> 
> #if ENABLE(TREE_DEBUGGING)
>-    LOG(Compositing, "%*p %s computeCompositingRequirements (backing provider candidate %p)", 12 + compositingState.depth * 2, &layer, layer.isNormalFlowOnly() ? "n" : "s", backingSharingState.backingProviderCandidate);
>+    LOG(Compositing, "%*p %s computeCompositingRequirements (backing provider candidate %p)", 12 + compositingState.depth * 2, &layer, layer.isNormalFlowOnly() ? "n" : "s", backingSharingState.backingProviderCandidate());
> #endif
> 
>     // FIXME: maybe we can avoid updating all remaining layers in paint order.
>@@ -891,7 +954,6 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>     layer.updateLayerListsIfNeeded();
> 
>     layer.setHasCompositingDescendant(false);
>-    layer.setBackingProviderLayer(nullptr);
> 
>     // We updated compositing for direct reasons in layerStyleChanged(). Here, check for compositing that can only be evaluated after layout.
>     RequiresCompositingData queryData;
>@@ -921,9 +983,9 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
> 
>         // If we're testing for overlap, we only need to composite if we overlap something that is already composited.
>         if (overlapMap.overlapsLayers(layerExtent.bounds)) {
>-            if (backingSharingState.backingProviderCandidate && canBeComposited(layer) && backingProviderLayerCanIncludeLayer(*backingSharingState.backingProviderCandidate, layer)) {
>-                backingSharingState.backingSharingLayers.append(makeWeakPtr(layer));
>-                LOG(Compositing, " layer %p can share with %p", &layer, backingSharingState.backingProviderCandidate);
>+            if (backingSharingState.backingProviderCandidate() && canBeComposited(layer) && backingProviderLayerCanIncludeLayer(*backingSharingState.backingProviderCandidate(), layer)) {
>+                backingSharingState.appendSharingLayer(layer);
>+                LOG(Compositing, " layer %p can share with %p", &layer, backingSharingState.backingProviderCandidate());
>                 compositingReason = RenderLayer::IndirectCompositingReason::None;
>                 layerPaintsIntoProvidedBacking = true;
>             } else
>@@ -967,10 +1029,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>         // animation running behind this layer, meaning they can rely on the overlap map testing again.
>         childState.testingOverlap = true;
>         willBeComposited = true;
>-
>         layerPaintsIntoProvidedBacking = false;
>-        layer.disconnectFromBackingProviderLayer();
>-        backingSharingState.backingSharingLayers.removeAll(&layer);
>     };
> 
>     if (willBeComposited) {
>@@ -983,15 +1042,13 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>         childState.ancestorHasTransformAnimation |= layerExtent.hasTransformAnimation;
>         // Too hard to compute animated bounds if both us and some ancestor is animating transform.
>         layerExtent.animationCausesExtentUncertainty |= layerExtent.hasTransformAnimation && compositingState.ancestorHasTransformAnimation;
>-
>-        // Compositing for any reason disables backing sharing.
>-        LOG_WITH_STREAM(Compositing, stream << &layer << " is compositing - flushing sharing to " << backingSharingState.backingProviderCandidate << " with " << backingSharingState.backingSharingLayers.size() << " sharing layers");
>-        backingSharingState.resetBackingProviderCandidate();
>     } else if (layerPaintsIntoProvidedBacking) {
>         childState.backingSharingAncestor = &layer;
>         overlapMap.pushCompositingContainer();
>     }
> 
>+    backingSharingState.updateBeforeDescendantTraversal(layer, willBeComposited);
>+
> #if !ASSERT_DISABLED
>     LayerListMutationDetector mutationChecker(layer);
> #endif
>@@ -1100,18 +1157,10 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>         layer.setChildrenNeedCompositingGeometryUpdate();
>         // The composited bounds of enclosing layers depends on which descendants are composited, so they need a geometry update.
>         layer.setNeedsCompositingGeometryUpdateOnAncestors();
>-    } else if (layer.isComposited())
>-        layer.backing()->clearBackingSharingLayers();
>-
>-    if (backingSharingState.backingProviderCandidate && &layer == backingSharingState.backingProviderStackingContext) {
>-        LOG_WITH_STREAM(Compositing, stream << &layer << " popping stacking context " << backingSharingState.backingProviderStackingContext << ", flushing candidate " << backingSharingState.backingProviderCandidate << " with " << backingSharingState.backingSharingLayers.size() << " sharing layers");
>-        backingSharingState.resetBackingProviderCandidate();
>-    } else if (!backingSharingState.backingProviderCandidate && layer.isComposited()) {
>-        LOG_WITH_STREAM(Compositing, stream << &layer << " compositing - sharing candidate " << backingSharingState.backingProviderCandidate << " with " << backingSharingState.backingSharingLayers.size() << " sharing layers");
>-        // Flush out any earlier candidate in this stacking context. This layer becomes a candidate.
>-        backingSharingState.resetBackingProviderCandidate(&layer, compositingState.stackingContextAncestor);
>     }
> 
>+    backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
>+
>     if (layer.reflectionLayer() && updateLayerCompositingState(*layer.reflectionLayer(), queryData, CompositingChangeRepaintNow))
>         layer.setNeedsCompositingLayerConnection();
> 
>@@ -1124,7 +1173,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>     }
> 
> #if ENABLE(TREE_DEBUGGING)
>-    LOG(Compositing, "%*p computeCompositingRequirements - willBeComposited %d (backing provider candidate %p)", 12 + compositingState.depth * 2, &layer, willBeComposited, backingSharingState.backingProviderCandidate);
>+    LOG(Compositing, "%*p computeCompositingRequirements - willBeComposited %d (backing provider candidate %p)", 12 + compositingState.depth * 2, &layer, willBeComposited, backingSharingState.backingProviderCandidate());
> #endif
> 
>     layer.clearCompositingRequirementsTraversalState();
>@@ -1160,9 +1209,9 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
>         computeExtent(overlapMap, layer, layerExtent);
> 
>     if (layer.paintsIntoProvidedBacking()) {
>-        ASSERT(backingSharingState.backingProviderCandidate);
>-        ASSERT(backingProviderLayerCanIncludeLayer(*backingSharingState.backingProviderCandidate, layer));
>-        backingSharingState.backingSharingLayers.append(makeWeakPtr(layer));
>+        ASSERT(backingSharingState.backingProviderCandidate());
>+        ASSERT(backingProviderLayerCanIncludeLayer(*backingSharingState.backingProviderCandidate(), layer));
>+        backingSharingState.appendSharingLayer(layer);
>     }
> 
>     CompositingState childState = compositingState.stateForPaintOrderChildren(layer);
>@@ -1182,12 +1231,10 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
>         childState.ancestorHasTransformAnimation |= layerExtent.hasTransformAnimation;
>         // Too hard to compute animated bounds if both us and some ancestor is animating transform.
>         layerExtent.animationCausesExtentUncertainty |= layerExtent.hasTransformAnimation && compositingState.ancestorHasTransformAnimation;
>-
>-        // Compositing for any reason disables backing sharing.
>-        LOG_WITH_STREAM(Compositing, stream << "tus: " << &layer << " is compositing - flushing sharing to " << backingSharingState.backingProviderCandidate << " with " << backingSharingState.backingSharingLayers.size() << " sharing layers");
>-        backingSharingState.resetBackingProviderCandidate();
>     }
> 
>+    backingSharingState.updateBeforeDescendantTraversal(layer, layerIsComposited);
>+
> #if !ASSERT_DISABLED
>     LayerListMutationDetector mutationChecker(layer);
> #endif
>@@ -1240,17 +1287,7 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
>     if ((childState.compositingAncestor == &layer && !layer.isRenderViewLayer()) || childState.backingSharingAncestor == &layer)
>         overlapMap.popCompositingContainer();
> 
>-    if (layer.isComposited())
>-        layer.backing()->clearBackingSharingLayers();
>-
>-    if (backingSharingState.backingProviderCandidate && &layer == backingSharingState.backingProviderStackingContext) {
>-        LOG_WITH_STREAM(Compositing, stream << &layer << " tus: popping stacking context " << backingSharingState.backingProviderStackingContext << ", flushing candidate " << backingSharingState.backingProviderCandidate << " with " << backingSharingState.backingSharingLayers.size() << " sharing layers");
>-        backingSharingState.resetBackingProviderCandidate();
>-    } else if (!backingSharingState.backingProviderCandidate && layer.isComposited()) {
>-        LOG_WITH_STREAM(Compositing, stream << &layer << " tus: compositing - sharing candidate " << backingSharingState.backingProviderCandidate << " with " << backingSharingState.backingSharingLayers.size() << " sharing layers");
>-        // Flush out any earlier candidate in this stacking context. This layer becomes a candidate.
>-        backingSharingState.resetBackingProviderCandidate(&layer, compositingState.stackingContextAncestor);
>-    }
>+    backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
> 
>     descendantHas3DTransform |= anyDescendantHas3DTransform || layer.has3DTransform();
> 
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.h b/Source/WebCore/rendering/RenderLayerCompositor.h
>index 54ca466ae6137737c2c77f7569907c5f239a76d1..125b3292162edcc40bada5258603433639e61618 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.h
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.h
>@@ -367,9 +367,9 @@ public:
>     unsigned compositingUpdateCount() const { return m_compositingUpdateCount; }
> 
> private:
>+    class BackingSharingState;
>     class OverlapMap;
>     struct CompositingState;
>-    struct BackingSharingState;
>     struct OverlapExtent;
> 
>     // Returns true if the policy changed.

Actions: View | Formatted Diff | Diff

Attachments on bug 197824: 369678