Attachment 358718 Details for Bug 193290 – Patch

[patch] Patch

bug-193290-20190109114210.patch (text/plain), 13.13 KB, created by Simon Fraser (smfr) on 2019-01-09 11:42:11 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-01-09 11:42:11 PST

Size: 13.13 KB

patch

obsolete

>Subversion Revision: 239693
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7f29aa7d9879e468dbb575bd40d28ca9a88254eb..059bfca6e16792e3020a9e9f52d993408922a7ca 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,40 @@
>+2019-01-09  Simon Fraser  <simon.fraser@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 NOBODY (OOPS!).
>+
>+        RenderLayerCompositor::didFlushChangesForLayer() triggers updates scrolling tree nodes for the
>+        flushed layer, but it's not clear what has changed at this point.
>+
>+        didFlushChangesForLayer() was added to explicitly handle the case where the underlying platform layer
>+        for a GraphicsLayer changes because the layer swaps between tiled and non-tiled; 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 make its
>+        function more explicit.
>+
>+        * platform/graphics/GraphicsLayerClient.h:
>+        (WebCore::GraphicsLayerClient::didCommitChangesForLayer):
>+        (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):
>+        (WebCore::RenderLayerBacking::didCommitChangesForLayer const): Deleted.
>+        * rendering/RenderLayerBacking.h:
>+        * rendering/RenderLayerCompositor.cpp:
>+        (WebCore::RenderLayerCompositor::didChangePlatformLayerForLayer):
>+        (WebCore::RenderLayerCompositor::didFlushChangesForLayer): Deleted.
>+        (WebCore::RenderLayerCompositor::didCommitChangesForLayer const): Deleted.
>+        * rendering/RenderLayerCompositor.h:
>+
> 2019-01-07  David Kilzer  <ddkilzer@apple.com>
> 
>         PlatformECKey should use a std::unique_ptr
>diff --git a/Source/WebCore/platform/graphics/GraphicsLayerClient.h b/Source/WebCore/platform/graphics/GraphicsLayerClient.h
>index 25068eb43f38db4e7811acf0d0b2d9e09572b53b..5ba930ecae833d266eca5c43f349a4ab82587790 100644
>--- a/Source/WebCore/platform/graphics/GraphicsLayerClient.h
>+++ b/Source/WebCore/platform/graphics/GraphicsLayerClient.h
>@@ -101,7 +101,8 @@ public:
>     virtual void notifyFlushBeforeDisplayRefresh(const GraphicsLayer*) { }
> 
>     virtual void paintContents(const GraphicsLayer*, GraphicsContext&, GraphicsLayerPaintingPhase, const FloatRect& /* inClip */, GraphicsLayerPaintBehavior) { }
>-    virtual void didCommitChangesForLayer(const GraphicsLayer*) const { }
>+    virtual void didCommitChangesForLayer(const GraphicsLayer*) { }
>+    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.
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>index 650eb6c8e54ab7225ea8940c1d8213becdb0fd95..ecab87cd6a33ecfe37ee00b5bb2f3220579aa806 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
>@@ -1272,13 +1272,17 @@ 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 (layerTypeChanged)
>+        client().didChangePlatformLayerForLayer(this);
>+
>     if (hadChanges)
>         client().didCommitChangesForLayer(this);
> }
>@@ -1565,8 +1569,9 @@ void GraphicsLayerCA::recursiveCommitChanges(CommitState& commitState, const Tra
>         baseRelativePosition += m_position;
> 
>     bool wasRunningTransformAnimation = isRunningTransformAnimation();
>-
>-    commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition);
>+    bool layerTypeChanged = false;
>+    
>+    commitLayerChangesBeforeSublayers(childCommitState, pageScaleFactor, baseRelativePosition, layerTypeChanged);
> 
>     bool nowRunningTransformAnimation = wasRunningTransformAnimation;
>     if (m_uncommittedChanges & AnimationChanged)
>@@ -1586,7 +1591,7 @@ void GraphicsLayerCA::recursiveCommitChanges(CommitState& commitState, const Tra
> 
>     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,6 +1621,9 @@ void GraphicsLayerCA::recursiveCommitChanges(CommitState& commitState, const Tra
>     if (affectedByTransformAnimation && m_layer->layerType() == PlatformCALayer::LayerTypeTiledBackingLayer)
>         client().notifyFlushBeforeDisplayRefresh(this);
> 
>+    if (layerTypeChanged)
>+        client().didChangePlatformLayerForLayer(this);
>+
>     if (hadChanges)
>         client().didCommitChangesForLayer(this);
> 
>@@ -1712,7 +1720,7 @@ static bool isCustomBackdropLayerType(PlatformCALayer::LayerType layerType)
>     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& layerTypeChanged)
> {
>     SetForScope<bool> committingChangesChange(m_isCommittingChanges, true);
> 
>@@ -1739,8 +1747,10 @@ void GraphicsLayerCA::commitLayerChangesBeforeSublayers(CommitState& commitState
>     else if (currentLayerType == PlatformCALayer::LayerTypeTiledBackingLayer || isCustomBackdropLayerType(m_layer->layerType()))
>         neededLayerType = PlatformCALayer::LayerTypeWebLayer;
> 
>-    if (neededLayerType != m_layer->layerType())
>+    if (neededLayerType != m_layer->layerType()) {
>         changeLayerTypeTo(neededLayerType);
>+        layerTypeChanged = true;
>+    }
> 
>     // Need to handle Preserves3DChanged first, because it affects which layers subsequent properties are applied to
>     if (m_uncommittedChanges & (Preserves3DChanged | ReplicatedLayerChanged | BackdropFiltersChanged))
>diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
>index c17573ec18b312e26d4a12b057554bb3ff2f84d8..d45a449c054298329cd7d955e4014f85911ed267 100644
>--- a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
>+++ b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h
>@@ -277,7 +277,7 @@ private:
>         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;
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index be8bba0a7c86eb19e424d75b1b04051e1ae9158c..462d39e85a2a8c7fb1b9ad622a843b76b947a8cd 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -2660,9 +2660,14 @@ bool RenderLayerBacking::paintsOpaquelyAtNonIntegralScales(const GraphicsLayer*)
>     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);
>+}
>+
>+void RenderLayerBacking::didCommitChangesForLayer(const GraphicsLayer*)
>+{
>+//    compositor().didFlushChangesForLayer(m_owningLayer, layer);
> }
> 
> bool RenderLayerBacking::getCurrentTransform(const GraphicsLayer* graphicsLayer, TransformationMatrix& transform) const
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.h b/Source/WebCore/rendering/RenderLayerBacking.h
>index 8d5f43c4815d9d97ada4ed7c719b49297af86bcc..26235d24e1c10daf42cb582d7b42664f95ebd75d 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.h
>+++ b/Source/WebCore/rendering/RenderLayerBacking.h
>@@ -208,7 +208,9 @@ public:
> 
>     float pageScaleFactor() const override;
>     float zoomedOutPageScaleFactor() const override;
>-    void didCommitChangesForLayer(const GraphicsLayer*) const override;
>+
>+    void didChangePlatformLayerForLayer(const GraphicsLayer*) override;
>+    void didCommitChangesForLayer(const GraphicsLayer*) override;
>     bool getCurrentTransform(const GraphicsLayer*, TransformationMatrix&) const override;
> 
>     bool isTrackingRepaints() const override;
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.cpp b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>index a8a2583190d0a33de9a906f70bf8651b298f5e67..c582f2e1d343c5bcf6e7cb973383b7e0c35d662a 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.cpp
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp
>@@ -536,16 +536,17 @@ void RenderLayerCompositor::updateCustomLayersAfterFlush()
> #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);
> 
> #if PLATFORM(IOS_FAMILY)
>     if (m_legacyScrollingLayerCoordinator)
>-        m_legacyScrollingLayerCoordinator->didFlushChangesForLayer(layer);
>+        m_legacyScrollingLayerCoordinator->didFlushChangesForLayer(layer); // FIXME: naming.
> #endif
> 
>+    // FIXME: right?
>     auto* backing = layer.backing();
>     if (backing->backgroundLayerPaintsFixedRootBackground() && graphicsLayer == backing->backgroundLayer())
>         fixedRootBackgroundLayerChanged();
>@@ -2986,11 +2987,6 @@ float RenderLayerCompositor::contentsScaleMultiplierForNewTiles(const GraphicsLa
> #endif
> }
> 
>-void RenderLayerCompositor::didCommitChangesForLayer(const GraphicsLayer*) const
>-{
>-    // Nothing to do here yet.
>-}
>-
> bool RenderLayerCompositor::documentUsesTiledBacking() const
> {
>     auto* layer = m_renderView.layer();
>diff --git a/Source/WebCore/rendering/RenderLayerCompositor.h b/Source/WebCore/rendering/RenderLayerCompositor.h
>index 0f7a0878843db28f08f43da9707428cc4b7e05ae..293da8b97b841594ad8a2289c7e0dd71a81098ef 100644
>--- a/Source/WebCore/rendering/RenderLayerCompositor.h
>+++ b/Source/WebCore/rendering/RenderLayerCompositor.h
>@@ -159,9 +159,9 @@ public:
>     // 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 @@ public:
>     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*/);

Actions: View | Formatted Diff | Diff

Attachments on bug 193290: 358718 | 358735 | 359242 | 359253 | 359288