Attachment 370011 Details for Bug 197936 – Patch

[patch] Patch

bug-197936-20190515174515.patch (text/plain), 14.11 KB, created by Simon Fraser (smfr) on 2019-05-15 17:45:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-05-15 17:45:16 PDT

Size: 14.11 KB

patch

obsolete

>Subversion Revision: 245336
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6ffd20d0cd33385506b168baf041665db49b897a..9c8791c62d82b4fe72e2a1a141e5875309a0e8b7 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2019-05-15  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Clean up code related to compositing overlap map maintenance
>+        https://bugs.webkit.org/show_bug.cgi?id=197936
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Clarify the logic around updating the overlap map:
>+
>+        When a layer becomes composited, or paints into a non-root composited layer, we add it to the overlap map
>+        after traversing descendants (since it only affets layers later in traversal).
>+
>+        If a layer became composited after traversing descendants, we need to go back and add all the descendants
>+        to the overlap map with a recursive traversal.
>+
>+        We can do all this near the end of computeCompositingRequirements/traverseUnchangedSubtree because
>+        we only check overlap when we enter this function on later layers.
>+
>+        Add a CompositingOverlap log channel and use it to log the state of the overlap map.
>+
>+        * platform/Logging.h:
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
>+        (WebCore::RenderLayerCompositor::traverseUnchangedSubtree):
>+        (WebCore::RenderLayerCompositor::addToOverlapMap const):
>+        (WebCore::RenderLayerCompositor::addDescendantsToOverlapMapRecursive const):
>+        (WebCore::RenderLayerCompositor::updateOverlapMap const):
>+        (WebCore::RenderLayerCompositor::addToOverlapMap): Deleted.
>+        (WebCore::RenderLayerCompositor::addToOverlapMapRecursive): Deleted.
>+        * rendering/RenderLayerCompositor.h:
>+
> 2019-05-15  Simon Fraser  <simon.fraser@apple.com>
> 
>         Clean up RenderLayerCompositor::computeCompositingRequirements() and traverseUnchangedSubtree()
>diff --git a/Source/WebCore/platform/Logging.h b/Source/WebCore/platform/Logging.h
>index e120a292a8325bbb80f6115666c576c44f1b24e3..27bfebb4c79358257d334fa7d423104c4d0a8496 100644
>--- a/Source/WebCore/platform/Logging.h
>+++ b/Source/WebCore/platform/Logging.h
>@@ -44,6 +44,7 @@ namespace WebCore {
>     M(Archives) \
>     M(ClipRects) \
>     M(Compositing) \
>+    M(CompositingOverlap) \
>     M(ContentFiltering) \
>     M(ContentObservation) \
>     M(DatabaseTracker) \
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index 13755c3f6981f6af4173ef6ce30ff901f6fec971..166c4119ffeefd0dd2fa3cf3d57f48d3124f6492 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -854,6 +854,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>     // We updated compositing for direct reasons in layerStyleChanged(). Here, check for compositing that can only be evaluated after layout.
>     RequiresCompositingData queryData;
>     bool willBeComposited = layer.isComposited();
>+    bool becameCompositedAfterDescendantTraversal = false;
>     if (layer.needsPostLayoutCompositingUpdate() || compositingState.fullPaintOrderTraversalRequired || compositingState.descendantsRequireCompositingUpdate) {
>         layer.setIndirectCompositingReason(RenderLayer::IndirectCompositingReason::None);
>         willBeComposited = needsToBeComposited(layer, queryData);
>@@ -918,6 +919,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>         // This layer now acts as the ancestor for kids.
>         currentState.compositingAncestor = &layer;
>         overlapMap.pushCompositingContainer();
>+        LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " will composite, pushed container " << overlapMap);
> 
>         willBeComposited = true;
>         layerPaintsIntoProvidedBacking = false;
>@@ -926,7 +928,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>     auto layerWillCompositePostDescendants = [&]() {
>         layerWillComposite();
>         currentState.subtreeIsCompositing = true;
>-        addToOverlapMapRecursive(overlapMap, layer);
>+        becameCompositedAfterDescendantTraversal = true;
>     };
> 
>     if (willBeComposited) {
>@@ -939,6 +941,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>     } else if (layerPaintsIntoProvidedBacking) {
>         currentState.backingSharingAncestor = &layer;
>         overlapMap.pushCompositingContainer();
>+        LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " will share, pushed container " << overlapMap);
>     }
> 
>     backingSharingState.updateBeforeDescendantTraversal(layer, willBeComposited);
>@@ -972,14 +975,6 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>         if (usesCompositing() && m_hasAcceleratedCompositing)
>             willBeComposited = true;
>     }
>-    
>-    // 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 (currentState.compositingAncestor && !currentState.compositingAncestor->isRenderViewLayer())
>-        addToOverlapMap(overlapMap, layer, layerExtent);
> 
> #if ENABLE(CSS_COMPOSITING)
>     bool isolatedCompositedBlending = layer.isolatesCompositedBlending();
>@@ -1052,9 +1047,14 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
>     descendantHas3DTransform |= anyDescendantHas3DTransform || layer.has3DTransform();
>     compositingState.updateWithDescendantStateAndLayer(currentState, layer, layerExtent);
> 
>+    bool layerContributesToOverlap = currentState.compositingAncestor && !currentState.compositingAncestor->isRenderViewLayer();
>+    updateOverlapMap(overlapMap, layer, layerExtent, layerContributesToOverlap, becameCompositedAfterDescendantTraversal);
>+
>     // Pop backing/overlap sharing state.
>-    if ((willBeComposited && !layer.isRenderViewLayer()) || currentState.backingSharingAncestor == &layer)
>+    if ((willBeComposited && !layer.isRenderViewLayer()) || currentState.backingSharingAncestor == &layer) {
>         overlapMap.popCompositingContainer();
>+        LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " is composited, popped container " << overlapMap);
>+    }
> 
>     backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
>     overlapMap.geometryMap().popMappingsToAncestor(ancestorLayer);
>@@ -1102,6 +1102,7 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
>         // This layer now acts as the ancestor for kids.
>         currentState.compositingAncestor = &layer;
>         overlapMap.pushCompositingContainer();
>+        LOG_WITH_STREAM(CompositingOverlap, stream << "unchangedSubtree: layer " << &layer << " will composite, pushed container " << overlapMap);
> 
>         computeExtent(overlapMap, layer, layerExtent);
>         currentState.ancestorHasTransformAnimation |= layerExtent.hasTransformAnimation;
>@@ -1129,14 +1130,6 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
>     for (auto* childLayer : layer.positiveZOrderLayers())
>         traverseUnchangedSubtree(&layer, *childLayer, overlapMap, currentState, backingSharingState, anyDescendantHas3DTransform);
> 
>-    // 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 (currentState.compositingAncestor && !currentState.compositingAncestor->isRenderViewLayer())
>-        addToOverlapMap(overlapMap, layer, layerExtent);
>-
>     // Set the flag to say that this layer has compositing children.
>     ASSERT(layer.hasCompositingDescendant() == currentState.subtreeIsCompositing);
>     ASSERT_IMPLIES(canBeComposited(layer) && clipsCompositingDescendants(layer), layerIsComposited);
>@@ -1146,8 +1139,13 @@ void RenderLayerCompositor::traverseUnchangedSubtree(RenderLayer* ancestorLayer,
>     ASSERT(!currentState.fullPaintOrderTraversalRequired);
>     compositingState.updateWithDescendantStateAndLayer(currentState, layer, layerExtent, true);
> 
>-    if ((layerIsComposited && !layer.isRenderViewLayer()) || currentState.backingSharingAncestor == &layer)
>+    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);
>+    }
> 
>     backingSharingState.updateAfterDescendantTraversal(layer, compositingState.stackingContextAncestor);
>     overlapMap.geometryMap().popMappingsToAncestor(ancestorLayer);
>@@ -1778,7 +1776,7 @@ void RenderLayerCompositor::computeExtent(const LayerOverlapMap& overlapMap, con
>     extent.extentComputed = true;
> }
> 
>-void RenderLayerCompositor::addToOverlapMap(LayerOverlapMap& overlapMap, const RenderLayer& layer, OverlapExtent& extent)
>+void RenderLayerCompositor::addToOverlapMap(LayerOverlapMap& overlapMap, const RenderLayer& layer, OverlapExtent& extent) const
> {
>     if (layer.isRenderViewLayer())
>         return;
>@@ -1794,35 +1792,57 @@ void RenderLayerCompositor::addToOverlapMap(LayerOverlapMap& overlapMap, const R
>     overlapMap.add(clipRect);
> }
> 
>-void RenderLayerCompositor::addToOverlapMapRecursive(LayerOverlapMap& overlapMap, const RenderLayer& layer, const RenderLayer* ancestorLayer)
>+void RenderLayerCompositor::addDescendantsToOverlapMapRecursive(LayerOverlapMap& overlapMap, const RenderLayer& layer, const RenderLayer* ancestorLayer) const
> {
>     if (!canBeComposited(layer))
>         return;
> 
>     // A null ancestorLayer is an indication that 'layer' has already been pushed.
>-    if (ancestorLayer)
>+    if (ancestorLayer) {
>         overlapMap.geometryMap().pushMappingsToAncestor(&layer, ancestorLayer);
>     
>-    OverlapExtent layerExtent;
>-    addToOverlapMap(overlapMap, layer, layerExtent);
>+        OverlapExtent layerExtent;
>+        addToOverlapMap(overlapMap, layer, layerExtent);
>+    }
> 
> #if !ASSERT_DISABLED
>     LayerListMutationDetector mutationChecker(const_cast<RenderLayer&>(layer));
> #endif
> 
>     for (auto* renderLayer : layer.negativeZOrderLayers())
>-        addToOverlapMapRecursive(overlapMap, *renderLayer, &layer);
>+        addDescendantsToOverlapMapRecursive(overlapMap, *renderLayer, &layer);
> 
>     for (auto* renderLayer : layer.normalFlowLayers())
>-        addToOverlapMapRecursive(overlapMap, *renderLayer, &layer);
>+        addDescendantsToOverlapMapRecursive(overlapMap, *renderLayer, &layer);
> 
>     for (auto* renderLayer : layer.positiveZOrderLayers())
>-        addToOverlapMapRecursive(overlapMap, *renderLayer, &layer);
>+        addDescendantsToOverlapMapRecursive(overlapMap, *renderLayer, &layer);
>     
>     if (ancestorLayer)
>         overlapMap.geometryMap().popMappingsToAncestor(ancestorLayer);
> }
> 
>+void RenderLayerCompositor::updateOverlapMap(LayerOverlapMap& overlapMap, const RenderLayer& layer, OverlapExtent& layerExtent, bool layerContributesToOverlap, 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) {
>+        addToOverlapMap(overlapMap, layer, layerExtent);
>+        LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " contributes to overlap, added to map " << overlapMap);
>+    }
>+
>+    if (addDescendantsToOverlap) {
>+        // If this is the first non-root layer to composite, we need to add all the descendants we already traversed to the overlap map.
>+        addDescendantsToOverlapMapRecursive(overlapMap, layer);
>+        LOG_WITH_STREAM(CompositingOverlap, stream << "layer " << &layer << " composited post descendant traversal, added recursive " << overlapMap);
>+    }
>+}
>+
> #if ENABLE(VIDEO)
> bool RenderLayerCompositor::canAccelerateVideoRendering(RenderVideo& video) const
> {
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.h b/Source/WebCore/rendering/RenderLayerCompositor.h
>index db224367db0eae4992d4bb8f06653b9404c32b88..f516b830ed52d3f8df56c7f9a06b9a5d41243244 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.h
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.h
>@@ -406,8 +406,9 @@ private:
>     void recursiveRepaintLayer(RenderLayer&);
> 
>     void computeExtent(const LayerOverlapMap&, const RenderLayer&, OverlapExtent&) const;
>-    void addToOverlapMap(LayerOverlapMap&, const RenderLayer&, OverlapExtent&);
>-    void addToOverlapMapRecursive(LayerOverlapMap&, const RenderLayer&, const RenderLayer* ancestorLayer = nullptr);
>+    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 updateCompositingLayersTimerFired();
>

Flags:

zalan: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197936: 370011 | 370022