Title: [240047] trunk/Source/WebCore
Revision
240047
Author
simon.fra...@apple.com
Date
2019-01-16 12:40:59 -0800 (Wed, 16 Jan 2019)

Log Message

Make didCommitChangesForLayer() explicitly about the platform layer changing because of tile/non-tile swapping
https://bugs.webkit.org/show_bug.cgi?id=193290

Reviewed by Tim Horton.

RenderLayerCompositor::didFlushChangesForLayer() triggers updates scrolling tree nodes for
the flushed layer, but it's not clear what has changed at this point.

didCommitChangesForLayer()/didFlushChangesForLayer() were added to explicitly handle the
case where the underlying platform layer for a GraphicsLayer changes because the layer swaps
between tiled and non-tiled, and structural layer changes; we need to push the new layer to
the scrolling tree because it operates on platform layers. So the only work that
didFlushChangesForLayer() should do is to update layers on scrolling tree nodes; it doesn't
need to do any geometry updating. Move towards that goal by renaming this callback to
didChangePlatformLayerForLayer() to make its function more explicit.

* platform/graphics/GraphicsLayerClient.h:
(WebCore::GraphicsLayerClient::didChangePlatformLayerForLayer):
(WebCore::GraphicsLayerClient::didCommitChangesForLayer const): Deleted.
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::flushCompositingStateForThisLayerOnly):
(WebCore::GraphicsLayerCA::recursiveCommitChanges):
(WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):
* platform/graphics/ca/GraphicsLayerCA.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::didChangePlatformLayerForLayer):
(WebCore::RenderLayerBacking::didCommitChangesForLayer const): Deleted.
* rendering/RenderLayerBacking.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::didChangePlatformLayerForLayer):
(WebCore::LegacyWebKitScrollingLayerCoordinator::didChangePlatformLayerForLayer):
(WebCore::RenderLayerCompositor::didFlushChangesForLayer): Deleted.
(WebCore::RenderLayerCompositor::didCommitChangesForLayer const): Deleted.
(WebCore::LegacyWebKitScrollingLayerCoordinator::didFlushChangesForLayer): Deleted.
* rendering/RenderLayerCompositor.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (240046 => 240047)


--- trunk/Source/WebCore/ChangeLog	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/ChangeLog	2019-01-16 20:40:59 UTC (rev 240047)
@@ -1,3 +1,41 @@
+2019-01-15  Simon Fraser  <simon.fra...@apple.com>
+
+        Make didCommitChangesForLayer() explicitly about the platform layer changing because of tile/non-tile swapping
+        https://bugs.webkit.org/show_bug.cgi?id=193290
+
+        Reviewed by Tim Horton.
+
+        RenderLayerCompositor::didFlushChangesForLayer() triggers updates scrolling tree nodes for
+        the flushed layer, but it's not clear what has changed at this point.
+
+        didCommitChangesForLayer()/didFlushChangesForLayer() were added to explicitly handle the
+        case where the underlying platform layer for a GraphicsLayer changes because the layer swaps
+        between tiled and non-tiled, and structural layer changes; we need to push the new layer to
+        the scrolling tree because it operates on platform layers. So the only work that
+        didFlushChangesForLayer() should do is to update layers on scrolling tree nodes; it doesn't
+        need to do any geometry updating. Move towards that goal by renaming this callback to
+        didChangePlatformLayerForLayer() to make its function more explicit.
+
+        * platform/graphics/GraphicsLayerClient.h:
+        (WebCore::GraphicsLayerClient::didChangePlatformLayerForLayer):
+        (WebCore::GraphicsLayerClient::didCommitChangesForLayer const): Deleted.
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::flushCompositingStateForThisLayerOnly):
+        (WebCore::GraphicsLayerCA::recursiveCommitChanges):
+        (WebCore::GraphicsLayerCA::commitLayerChangesBeforeSublayers):
+        * platform/graphics/ca/GraphicsLayerCA.h:
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::didChangePlatformLayerForLayer):
+        (WebCore::RenderLayerBacking::didCommitChangesForLayer const): Deleted.
+        * rendering/RenderLayerBacking.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::didChangePlatformLayerForLayer):
+        (WebCore::LegacyWebKitScrollingLayerCoordinator::didChangePlatformLayerForLayer):
+        (WebCore::RenderLayerCompositor::didFlushChangesForLayer): Deleted.
+        (WebCore::RenderLayerCompositor::didCommitChangesForLayer const): Deleted.
+        (WebCore::LegacyWebKitScrollingLayerCoordinator::didFlushChangesForLayer): Deleted.
+        * rendering/RenderLayerCompositor.h:
+
 2019-01-16  Chris Dumez  <cdu...@apple.com>
 
         Regression(PSON) View becomes blank after click a cross-site download link

Modified: trunk/Source/WebCore/platform/graphics/GraphicsLayerClient.h (240046 => 240047)


--- trunk/Source/WebCore/platform/graphics/GraphicsLayerClient.h	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/platform/graphics/GraphicsLayerClient.h	2019-01-16 20:40:59 UTC (rev 240047)
@@ -101,7 +101,7 @@
     virtual void notifyFlushBeforeDisplayRefresh(const GraphicsLayer*) { }
 
     virtual void paintContents(const GraphicsLayer*, GraphicsContext&, GraphicsLayerPaintingPhase, const FloatRect& /* inClip */, GraphicsLayerPaintBehavior) { }
-    virtual void didCommitChangesForLayer(const GraphicsLayer*) const { }
+    virtual void didChangePlatformLayerForLayer(const GraphicsLayer*) { }
 
     // Provides current transform (taking transform-origin and animations into account). Input matrix has been
     // initialized to identity already. Returns false if the layer has no transform.

Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (240046 => 240047)


--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2019-01-16 20:40:59 UTC (rev 240047)
@@ -1271,16 +1271,16 @@
 void GraphicsLayerCA::flushCompositingStateForThisLayerOnly()
 {
     float pageScaleFactor;
-    bool hadChanges = m_uncommittedChanges;
+    bool layerTypeChanged = false;
     
     CommitState commitState;
 
     FloatPoint offset = computePositionRelativeToBase(pageScaleFactor);
-    commitLayerChangesBeforeSublayers(commitState, pageScaleFactor, offset);
+    commitLayerChangesBeforeSublayers(commitState, pageScaleFactor, offset, layerTypeChanged);
     commitLayerChangesAfterSublayers(commitState);
 
-    if (hadChanges)
-        client().didCommitChangesForLayer(this);
+    if (layerTypeChanged)
+        client().didChangePlatformLayerForLayer(this);
 }
 
 static inline bool accumulatesTransform(const GraphicsLayerCA& layer)
@@ -1565,9 +1565,10 @@
         baseRelativePosition += m_position;
 
     bool wasRunningTransformAnimation = isRunningTransformAnimation();
+    bool layerTypeChanged = false;
+    
+    commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition, layerTypeChanged);
 
-    commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition);
-
     bool nowRunningTransformAnimation = wasRunningTransformAnimation;
     if (m_uncommittedChanges & AnimationChanged)
         nowRunningTransformAnimation = isRunningTransformAnimation();
@@ -1586,7 +1587,7 @@
 
     if (GraphicsLayerCA* maskLayer = downcast<GraphicsLayerCA>(m_maskLayer.get())) {
         maskLayer->setVisibleAndCoverageRects(rects, m_isViewportConstrained || commitState.ancestorIsViewportConstrained);
-        maskLayer->commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition);
+        maskLayer->commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition, layerTypeChanged);
     }
 
     bool hasDescendantsWithRunningTransformAnimations = false;
@@ -1616,8 +1617,8 @@
     if (affectedByTransformAnimation && m_layer->layerType() == PlatformCALayer::LayerTypeTiledBackingLayer)
         client().notifyFlushBeforeDisplayRefresh(this);
 
-    if (hadChanges)
-        client().didCommitChangesForLayer(this);
+    if (layerTypeChanged)
+        client().didChangePlatformLayerForLayer(this);
 
     if (usesDisplayListDrawing() && m_drawsContent && (!m_hasEverPainted || hadDirtyRects)) {
         TraceScope tracingScope(DisplayListRecordStart, DisplayListRecordEnd);
@@ -1712,7 +1713,7 @@
     return layerType == PlatformCALayer::LayerTypeLightSystemBackdropLayer || layerType == PlatformCALayer::LayerTypeDarkSystemBackdropLayer;
 }
 
-void GraphicsLayerCA::commitLayerChangesBeforeSublayers(CommitState& commitState, float pageScaleFactor, const FloatPoint& positionRelativeToBase)
+void GraphicsLayerCA::commitLayerChangesBeforeSublayers(CommitState& commitState, float pageScaleFactor, const FloatPoint& positionRelativeToBase, bool& layerChanged)
 {
     SetForScope<bool> committingChangesChange(m_isCommittingChanges, true);
 
@@ -1739,12 +1740,16 @@
     else if (currentLayerType == PlatformCALayer::LayerTypeTiledBackingLayer || isCustomBackdropLayerType(m_layer->layerType()))
         neededLayerType = PlatformCALayer::LayerTypeWebLayer;
 
-    if (neededLayerType != m_layer->layerType())
+    if (neededLayerType != m_layer->layerType()) {
         changeLayerTypeTo(neededLayerType);
+        layerChanged = true;
+    }
 
     // Need to handle Preserves3DChanged first, because it affects which layers subsequent properties are applied to
-    if (m_uncommittedChanges & (Preserves3DChanged | ReplicatedLayerChanged | BackdropFiltersChanged))
-        updateStructuralLayer();
+    if (m_uncommittedChanges & (Preserves3DChanged | ReplicatedLayerChanged | BackdropFiltersChanged)) {
+        if (updateStructuralLayer())
+            layerChanged = true;
+    }
 
     if (m_uncommittedChanges & GeometryChanged)
         updateGeometry(pageScaleFactor, positionRelativeToBase);
@@ -2265,12 +2270,12 @@
     m_layer->setShapeWindRule(m_shapeLayerWindRule);
 }
 
-void GraphicsLayerCA::updateStructuralLayer()
+bool GraphicsLayerCA::updateStructuralLayer()
 {
-    ensureStructuralLayer(structuralLayerPurpose());
+    return ensureStructuralLayer(structuralLayerPurpose());
 }
 
-void GraphicsLayerCA::ensureStructuralLayer(StructuralLayerPurpose purpose)
+bool GraphicsLayerCA::ensureStructuralLayer(StructuralLayerPurpose purpose)
 {
     const LayerChangeFlags structuralLayerChangeFlags = NameChanged
         | GeometryChanged
@@ -2283,6 +2288,8 @@
         | MaskLayerChanged
         | OpacityChanged;
 
+    bool structuralLayerChanged = false;
+
     if (purpose == NoStructuralLayer) {
         if (m_structuralLayer) {
             // Replace the transformLayer in the parent with this layer.
@@ -2297,14 +2304,13 @@
 
             // Release the structural layer.
             m_structuralLayer = nullptr;
+            structuralLayerChanged = true;
 
             addUncommittedChanges(structuralLayerChangeFlags);
         }
-        return;
+        return structuralLayerChanged;
     }
 
-    bool structuralLayerChanged = false;
-    
     if (purpose == StructuralLayerForPreserves3D) {
         if (m_structuralLayer && m_structuralLayer->layerType() != PlatformCALayer::LayerTypeTransformLayer)
             m_structuralLayer = nullptr;
@@ -2324,7 +2330,7 @@
     }
     
     if (!structuralLayerChanged)
-        return;
+        return false;
     
     addUncommittedChanges(structuralLayerChangeFlags);
 
@@ -2349,6 +2355,7 @@
     }
 
     moveAnimations(m_layer.get(), m_structuralLayer.get());
+    return true;
 }
 
 GraphicsLayerCA::StructuralLayerPurpose GraphicsLayerCA::structuralLayerPurpose() const

Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (240046 => 240047)


--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h	2019-01-16 20:40:59 UTC (rev 240047)
@@ -277,7 +277,7 @@
         return m_animations && m_animations->runningAnimations.contains(animationName);
     }
 
-    void commitLayerChangesBeforeSublayers(CommitState&, float pageScaleFactor, const FloatPoint& positionRelativeToBase);
+    void commitLayerChangesBeforeSublayers(CommitState&, float pageScaleFactor, const FloatPoint& positionRelativeToBase, bool& layerTypeChanged);
     void commitLayerChangesAfterSublayers(CommitState&);
 
     FloatPoint computePositionRelativeToBase(float& pageScale) const;
@@ -402,7 +402,7 @@
     void updateContentsVisibility();
     void updateContentsOpaque(float pageScaleFactor);
     void updateBackfaceVisibility();
-    void updateStructuralLayer();
+    bool updateStructuralLayer();
     void updateDrawsContent();
     void updateCoverage(const CommitState&);
     void updateBackgroundColor();
@@ -443,7 +443,7 @@
         StructuralLayerForReplicaFlattening,
         StructuralLayerForBackdrop
     };
-    void ensureStructuralLayer(StructuralLayerPurpose);
+    bool ensureStructuralLayer(StructuralLayerPurpose);
     StructuralLayerPurpose structuralLayerPurpose() const;
     
     void ensureLayerAnimations();

Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (240046 => 240047)


--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2019-01-16 20:40:59 UTC (rev 240047)
@@ -2660,9 +2660,9 @@
     return m_isMainFrameRenderViewLayer;
 }
 
-void RenderLayerBacking::didCommitChangesForLayer(const GraphicsLayer* layer) const
+void RenderLayerBacking::didChangePlatformLayerForLayer(const GraphicsLayer* layer)
 {
-    compositor().didFlushChangesForLayer(m_owningLayer, layer);
+    compositor().didChangePlatformLayerForLayer(m_owningLayer, layer);
 }
 
 bool RenderLayerBacking::getCurrentTransform(const GraphicsLayer* graphicsLayer, TransformationMatrix& transform) const

Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.h (240046 => 240047)


--- trunk/Source/WebCore/rendering/RenderLayerBacking.h	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.h	2019-01-16 20:40:59 UTC (rev 240047)
@@ -208,7 +208,8 @@
 
     float pageScaleFactor() const override;
     float zoomedOutPageScaleFactor() const override;
-    void didCommitChangesForLayer(const GraphicsLayer*) const override;
+
+    void didChangePlatformLayerForLayer(const GraphicsLayer*) override;
     bool getCurrentTransform(const GraphicsLayer*, TransformationMatrix&) const override;
 
     bool isTrackingRepaints() const override;

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (240046 => 240047)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2019-01-16 20:40:59 UTC (rev 240047)
@@ -536,7 +536,7 @@
 #endif
 }
 
-void RenderLayerCompositor::didFlushChangesForLayer(RenderLayer& layer, const GraphicsLayer* graphicsLayer)
+void RenderLayerCompositor::didChangePlatformLayerForLayer(RenderLayer& layer, const GraphicsLayer* graphicsLayer)
 {
     if (m_scrollCoordinatedLayers.contains(&layer))
         m_scrollCoordinatedLayersNeedingUpdate.add(&layer);
@@ -543,7 +543,7 @@
 
 #if PLATFORM(IOS_FAMILY)
     if (m_legacyScrollingLayerCoordinator)
-        m_legacyScrollingLayerCoordinator->didFlushChangesForLayer(layer);
+        m_legacyScrollingLayerCoordinator->didChangePlatformLayerForLayer(layer);
 #endif
 
     auto* backing = layer.backing();
@@ -2985,11 +2985,6 @@
 #endif
 }
 
-void RenderLayerCompositor::didCommitChangesForLayer(const GraphicsLayer*) const
-{
-    // Nothing to do here yet.
-}
-
 bool RenderLayerCompositor::documentUsesTiledBacking() const
 {
     auto* layer = m_renderView.layer();
@@ -4230,7 +4225,7 @@
     m_chromeClient.removeScrollingLayer(layer.renderer().element(), scrollingLayer, contentsLayer);
 }
 
-void LegacyWebKitScrollingLayerCoordinator::didFlushChangesForLayer(RenderLayer& layer)
+void LegacyWebKitScrollingLayerCoordinator::didChangePlatformLayerForLayer(RenderLayer& layer)
 {
     if (m_scrollingLayers.contains(&layer))
         m_scrollingLayersNeedingUpdate.add(&layer);

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (240046 => 240047)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2019-01-16 20:40:02 UTC (rev 240046)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2019-01-16 20:40:59 UTC (rev 240047)
@@ -107,7 +107,7 @@
     void addScrollingLayer(RenderLayer&);
     void removeScrollingLayer(RenderLayer&, RenderLayerBacking&);
 
-    void didFlushChangesForLayer(RenderLayer&);
+    void didChangePlatformLayerForLayer(RenderLayer&);
 
 private:
     void updateScrollingLayer(RenderLayer&);
@@ -159,9 +159,9 @@
     // at specific times.
     void scheduleLayerFlush(bool canThrottle);
     void flushPendingLayerChanges(bool isFlushRoot = true);
-    
+
     // Called when the GraphicsLayer for the given RenderLayer has flushed changes inside of flushPendingLayerChanges().
-    void didFlushChangesForLayer(RenderLayer&, const GraphicsLayer*);
+    void didChangePlatformLayerForLayer(RenderLayer&, const GraphicsLayer*);
 
     // Called when something outside WebKit affects the visible rect (e.g. delegated scrolling). Might schedule a layer flush.
     void didChangeVisibleRect();
@@ -289,8 +289,7 @@
     float contentsScaleMultiplierForNewTiles(const GraphicsLayer*) const override;
     float pageScaleFactor() const override;
     float zoomedOutPageScaleFactor() const override;
-
-    void didCommitChangesForLayer(const GraphicsLayer*) const override;
+    void didChangePlatformLayerForLayer(const GraphicsLayer*) override { }
     void notifyFlushBeforeDisplayRefresh(const GraphicsLayer*) override;
 
     void layerTiledBackingUsageChanged(const GraphicsLayer*, bool /*usingTiledBacking*/);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to