Refactor composited backing-sharing code

https://bugs.webkit.org/show_bug.cgi?id=197824

Reviewed by Zalan Bujtas.

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:

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 1e21209..e4b808b 100644 (file)
--- 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 Zalan Bujtas.
+
+        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-12  Youenn Fablet  <youenn@apple.com>
 
         Use clampTo in AVVideoCaptureSource::setSizeAndFrameRateWithPreset

diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
index 43d1a57..f839e46 100644 (file)
--- 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 283273c..07e8cc7 100644 (file)
--- 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 54ca466..125b329 100644 (file)
--- 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.