⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 242687 in webkit


Ignore:
Timestamp:
Mar 10, 2019, 11:03:42 AM (7 years ago)
Author:
Simon Fraser
Message:

ScrollingTree should have the final say on where layers go
https://bugs.webkit.org/show_bug.cgi?id=195507

Reviewed by Antti Koivisto.

Source/WebCore:

Main thread layer flushing can race with scrolling tree layer changes on macOS, causing
flashing as layers jump around sometimes. We go to some lengths to avoid this by trying
not to touch properties on layers that are being interacted with (scrollableArea->setIsUserScroll in
updateScrollPositionAfterAsyncScroll()), but that's fragile.

This patch adds ScrollingTree::applyScrollingTreeLayerPositions(), which enters
ScrollingTree::applyLayerPositions() on the main thread/UI process. This traverses
the tree allowing each node to run their layer positioning logic.

For macOS WK2, this is called from TiledCoreAnimationDrawingArea::flushLayers() after flushCompositingStateIncludingSubframes().
For macOS WK2 with UI-side compositing, RemoteLayerTreeDrawingAreaProxy::commitLayerTree()
calls m_webPageProxy.scrollingCoordinatorProxy()->applyScrollingTreeLayerPositions().
iOS WK2 is unchanged, using viewportChangedViaDelegatedScrolling() which does the same thing, allowing
for the dynamic viewport changes that happen when zooming on iOS.

Testing this requires infrastructure that we don't have yet.

  • page/scrolling/AsyncScrollingCoordinator.cpp:

(WebCore::AsyncScrollingCoordinator::applyScrollingTreeLayerPositions):

  • page/scrolling/AsyncScrollingCoordinator.h:
  • page/scrolling/ScrollingCoordinator.h:

(WebCore::ScrollingCoordinator::applyScrollingTreeLayerPositions):

  • page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::handleWheelEvent):
(WebCore::ScrollingTree::commitTreeState):
(WebCore::ScrollingTree::applyLayerPositions):
(WebCore::ScrollingTree::applyLayerPositionsRecursive):

  • page/scrolling/ScrollingTree.h:
  • page/scrolling/ScrollingTreeFrameHostingNode.cpp:

(WebCore::ScrollingTreeFrameHostingNode::applyLayerPositions):

  • page/scrolling/ScrollingTreeFrameHostingNode.h:
  • page/scrolling/ScrollingTreeNode.h:
  • page/scrolling/ScrollingTreeScrollingNode.cpp:

(WebCore::ScrollingTreeScrollingNode::applyLayerPositions):

  • page/scrolling/ScrollingTreeScrollingNode.h:
  • page/scrolling/cocoa/ScrollingTreeFixedNode.h:
  • page/scrolling/cocoa/ScrollingTreeFixedNode.mm:

(WebCore::ScrollingTreeFixedNode::relatedNodeScrollPositionDidChange):

  • page/scrolling/cocoa/ScrollingTreeStickyNode.h:
  • page/scrolling/cocoa/ScrollingTreeStickyNode.mm:

(WebCore::ScrollingTreeStickyNode::applyLayerPositions):
(WebCore::ScrollingTreeStickyNode::relatedNodeScrollPositionDidChange):

  • page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
  • page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:

(WebCore::ScrollingTreeFrameScrollingNodeMac::applyLayerPositions):

Source/WebKit:

  • UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:

(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):

  • UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:

(WebKit::RemoteScrollingCoordinatorProxy::applyScrollingTreeLayerPositions):

  • UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
  • WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:

(WebKit::TiledCoreAnimationDrawingArea::flushLayers):

Location:
trunk/Source
Files:
25 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r242681 r242687  
     12019-03-10  Simon Fraser  <simon.fraser@apple.com>
     2
     3        ScrollingTree should have the final say on where layers go
     4        https://bugs.webkit.org/show_bug.cgi?id=195507
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Main thread layer flushing can race with scrolling tree layer changes on macOS, causing
     9        flashing as layers jump around sometimes. We go to some lengths to avoid this by trying
     10        not to touch properties on layers that are being interacted with (scrollableArea->setIsUserScroll in
     11        updateScrollPositionAfterAsyncScroll()), but that's fragile.
     12
     13        This patch adds ScrollingTree::applyScrollingTreeLayerPositions(), which enters
     14        ScrollingTree::applyLayerPositions() on the main thread/UI process. This traverses
     15        the tree allowing each node to run their layer positioning logic.
     16
     17        For macOS WK2, this is called from TiledCoreAnimationDrawingArea::flushLayers() after flushCompositingStateIncludingSubframes().
     18        For macOS WK2 with UI-side compositing, RemoteLayerTreeDrawingAreaProxy::commitLayerTree()
     19        calls m_webPageProxy.scrollingCoordinatorProxy()->applyScrollingTreeLayerPositions().
     20        iOS WK2 is unchanged, using viewportChangedViaDelegatedScrolling() which does the same thing, allowing
     21        for the dynamic viewport changes that happen when zooming on iOS.
     22
     23        Testing this requires infrastructure that we don't have yet.
     24
     25        * page/scrolling/AsyncScrollingCoordinator.cpp:
     26        (WebCore::AsyncScrollingCoordinator::applyScrollingTreeLayerPositions):
     27        * page/scrolling/AsyncScrollingCoordinator.h:
     28        * page/scrolling/ScrollingCoordinator.h:
     29        (WebCore::ScrollingCoordinator::applyScrollingTreeLayerPositions):
     30        * page/scrolling/ScrollingTree.cpp:
     31        (WebCore::ScrollingTree::handleWheelEvent):
     32        (WebCore::ScrollingTree::commitTreeState):
     33        (WebCore::ScrollingTree::applyLayerPositions):
     34        (WebCore::ScrollingTree::applyLayerPositionsRecursive):
     35        * page/scrolling/ScrollingTree.h:
     36        * page/scrolling/ScrollingTreeFrameHostingNode.cpp:
     37        (WebCore::ScrollingTreeFrameHostingNode::applyLayerPositions):
     38        * page/scrolling/ScrollingTreeFrameHostingNode.h:
     39        * page/scrolling/ScrollingTreeNode.h:
     40        * page/scrolling/ScrollingTreeScrollingNode.cpp:
     41        (WebCore::ScrollingTreeScrollingNode::applyLayerPositions):
     42        * page/scrolling/ScrollingTreeScrollingNode.h:
     43        * page/scrolling/cocoa/ScrollingTreeFixedNode.h:
     44        * page/scrolling/cocoa/ScrollingTreeFixedNode.mm:
     45        (WebCore::ScrollingTreeFixedNode::relatedNodeScrollPositionDidChange):
     46        * page/scrolling/cocoa/ScrollingTreeStickyNode.h:
     47        * page/scrolling/cocoa/ScrollingTreeStickyNode.mm:
     48        (WebCore::ScrollingTreeStickyNode::applyLayerPositions):
     49        (WebCore::ScrollingTreeStickyNode::relatedNodeScrollPositionDidChange):
     50        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
     51        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
     52        (WebCore::ScrollingTreeFrameScrollingNodeMac::applyLayerPositions):
     53
    1542019-03-09  Andy Estes  <aestes@apple.com>
    255
  • trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

    r242465 r242687  
    229229}
    230230
     231void AsyncScrollingCoordinator::applyScrollingTreeLayerPositions()
     232{
     233    m_scrollingTree->applyLayerPositions();
     234}
     235
    231236void AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll(ScrollingNodeID nodeID, const FloatPoint& scrollPosition, const Optional<FloatPoint>& layoutViewportOrigin, bool programmaticScroll, ScrollingLayerPositionAction scrollingLayerPositionAction)
    232237{
  • trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h

    r242333 r242687  
    9898    WEBCORE_EXPORT bool requestScrollPositionUpdate(FrameView&, const IntPoint&) override;
    9999
     100    WEBCORE_EXPORT void applyScrollingTreeLayerPositions() override;
     101
    100102    WEBCORE_EXPORT ScrollingNodeID createNode(ScrollingNodeType, ScrollingNodeID newNodeID) override;
    101103    WEBCORE_EXPORT ScrollingNodeID insertNode(ScrollingNodeType, ScrollingNodeID newNodeID, ScrollingNodeID parentID, size_t childIndex) override;
  • trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h

    r242359 r242687  
    101101    virtual void frameViewRootLayerDidChange(FrameView&);
    102102
     103    // Traverses the scrolling tree, setting layer positions to represent the current scrolled state.
     104    virtual void applyScrollingTreeLayerPositions() { }
     105
    103106#if PLATFORM(COCOA)
    104107    // Dispatched by the scrolling tree during handleWheelEvent. This is required as long as scrollbars are painted on the main thread.
  • trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp

    r242670 r242687  
    107107    }
    108108
     109    LockHolder locker(m_treeMutex);
    109110    if (m_rootNode) {
    110111        auto& frameScrollingNode = downcast<ScrollingTreeFrameScrollingNode>(*m_rootNode);
     
    141142void ScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree> scrollingStateTree)
    142143{
     144    LockHolder locker(m_treeMutex);
     145
    143146    bool rootStateNodeChanged = scrollingStateTree->hasNewRootStateNode();
    144147   
     
    246249}
    247250
     251// Called from the main thread.
     252void ScrollingTree::applyLayerPositions()
     253{
     254    LockHolder locker(m_treeMutex);
     255
     256    if (!m_rootNode)
     257        return;
     258
     259    applyLayerPositionsRecursive(*m_rootNode, { }, { });
     260}
     261
     262void ScrollingTree::applyLayerPositionsRecursive(ScrollingTreeNode& currNode, FloatRect layoutViewport, FloatSize cumulativeDelta)
     263{
     264    if (is<ScrollingTreeFrameScrollingNode>(currNode)) {
     265        layoutViewport = downcast<ScrollingTreeFrameScrollingNode>(currNode).layoutViewport();
     266        cumulativeDelta = { };
     267    }
     268
     269    currNode.applyLayerPositions(layoutViewport, cumulativeDelta);
     270
     271    if (auto children = currNode.children()) {
     272        for (auto& child : *children)
     273            applyLayerPositionsRecursive(*child, layoutViewport, cumulativeDelta);
     274    }
     275}
     276
    248277ScrollingTreeNode* ScrollingTree::nodeForID(ScrollingNodeID nodeID) const
    249278{
  • trunk/Source/WebCore/page/scrolling/ScrollingTree.h

    r242670 r242687  
    6969    virtual void invalidate() { }
    7070    WEBCORE_EXPORT virtual void commitTreeState(std::unique_ptr<ScrollingStateTree>);
     71   
     72    WEBCORE_EXPORT void applyLayerPositions();
    7173
    7274    virtual Ref<ScrollingTreeNode> createScrollingTreeNode(ScrollingNodeType, ScrollingNodeID) = 0;
     
    155157    ScrollingTreeNode* nodeForID(ScrollingNodeID) const;
    156158
     159    void applyLayerPositionsRecursive(ScrollingTreeNode&, FloatRect layoutViewport, FloatSize cumulativeDelta);
     160
    157161    void notifyRelatedNodesRecursive(ScrollingTreeScrollingNode& changedNode, ScrollingTreeNode& currNode, const FloatRect& layoutViewport, FloatSize cumulativeDelta);
     162
     163    Lock m_treeMutex; // Protects the scrolling tree.
    158164
    159165    RefPtr<ScrollingTreeNode> m_rootNode;
  • trunk/Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.cpp

    r242132 r242687  
    5858}
    5959
     60void ScrollingTreeFrameHostingNode::applyLayerPositions(const FloatRect&, FloatSize&)
     61{
     62}
     63
    6064LayoutPoint ScrollingTreeFrameHostingNode::parentToLocalPoint(LayoutPoint point) const
    6165{
  • trunk/Source/WebCore/page/scrolling/ScrollingTreeFrameHostingNode.h

    r242132 r242687  
    4343
    4444    void commitStateBeforeChildren(const ScrollingStateNode&) final;
     45    void applyLayerPositions(const FloatRect&, FloatSize&) final;
    4546
    4647    const LayoutRect& parentRelativeScrollableRect() const { return m_parentRelativeScrollableRect; }
  • trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.cpp

    r242132 r242687  
    7878}
    7979
    80 void ScrollingTreeNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&, const FloatRect&, FloatSize&)
     80void ScrollingTreeNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&, const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
    8181{
     82    applyLayerPositions(layoutViewport, cumulativeDelta);
    8283}
    8384
  • trunk/Source/WebCore/page/scrolling/ScrollingTreeNode.h

    r242132 r242687  
    8686    WEBCORE_EXPORT virtual void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta);
    8787
     88    virtual void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) = 0;
     89
    8890    WEBCORE_EXPORT virtual void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const;
    8991
  • trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp

    r242601 r242687  
    180180}
    181181
     182void ScrollingTreeScrollingNode::applyLayerPositions(const FloatRect&, FloatSize&)
     183{
     184    repositionScrollingLayers();
     185    repositionRelatedLayers();
     186}
     187
    182188void ScrollingTreeScrollingNode::wasScrolledByDelegatedScrolling(const FloatPoint& position, Optional<FloatRect> overrideLayoutViewport)
    183189{
  • trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h

    r242601 r242687  
    9999    WEBCORE_EXPORT virtual void repositionRelatedLayers() { }
    100100
     101    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
     102
    101103    const FloatSize& reachableContentsSize() const { return m_reachableContentsSize; }
    102104    const LayoutRect& parentRelativeScrollableRect() const { return m_parentRelativeScrollableRect; }
  • trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.h

    r242132 r242687  
    4848
    4949    void commitStateBeforeChildren(const ScrollingStateNode&) override;
    50     void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
     50    void applyLayerPositions(const FloatRect&, FloatSize&) override;
    5151
    5252    void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
  • trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm

    r242669 r242687  
    6464}
    6565
    66 void ScrollingTreeFixedNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&, const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
     66void ScrollingTreeFixedNode::applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
    6767{
    6868    FloatPoint layerPosition = m_constraints.layerPositionForViewportRect(layoutViewport);
  • trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.h

    r242132 r242687  
    4848
    4949    void commitStateBeforeChildren(const ScrollingStateNode&) override;
    50     void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
     50    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
    5151
    5252    void dumpProperties(WTF::TextStream&, ScrollingStateTreeAsTextBehavior) const override;
  • trunk/Source/WebCore/page/scrolling/cocoa/ScrollingTreeStickyNode.mm

    r242669 r242687  
    6666}
    6767
    68 void ScrollingTreeStickyNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&, const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
     68void ScrollingTreeStickyNode::applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta)
    6969{
    7070    FloatRect constrainingRect;
  • trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.cpp

    r242132 r242687  
    5555}
    5656
    57 void ScrollingTreeFixedNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&, const FloatRect&, FloatSize&)
     57void ScrollingTreeFixedNode::applyLayerPositions(const FloatRect&, FloatSize&)
    5858{
    5959}
  • trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.h

    r242132 r242687  
    4343
    4444    void commitStateBeforeChildren(const ScrollingStateNode&) override;
    45     void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
     45    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
    4646};
    4747
  • trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.cpp

    r242132 r242687  
    5555}
    5656
    57 void ScrollingTreeStickyNode::relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode&, const FloatRect&, FloatSize&)
     57void ScrollingTreeStickyNode::applyLayerPositions(const FloatRect&, FloatSize&)
    5858{
    5959}
  • trunk/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNode.h

    r242132 r242687  
    4343
    4444    void commitStateBeforeChildren(const ScrollingStateNode&) override;
    45     void relatedNodeScrollPositionDidChange(const ScrollingTreeScrollingNode& changedNode, const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
     45    void applyLayerPositions(const FloatRect& layoutViewport, FloatSize& cumulativeDelta) override;
     46
    4647};
    4748
  • trunk/Source/WebKit/ChangeLog

    r242686 r242687  
     12019-03-10  Simon Fraser  <simon.fraser@apple.com>
     2
     3        ScrollingTree should have the final say on where layers go
     4        https://bugs.webkit.org/show_bug.cgi?id=195507
     5
     6        Reviewed by Antti Koivisto.
     7
     8        * UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
     9        (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
     10        * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:
     11        (WebKit::RemoteScrollingCoordinatorProxy::applyScrollingTreeLayerPositions):
     12        * UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h:
     13        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
     14        (WebKit::TiledCoreAnimationDrawingArea::flushLayers):
     15
    1162019-03-09  Darin Adler  <darin@apple.com>
    217
  • trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm

    r242132 r242687  
    218218
    219219#if ENABLE(ASYNC_SCROLLING)
    220 #if PLATFORM(IOS_FAMILY)
    221220    if (m_webPageProxy.scrollingCoordinatorProxy()->hasFixedOrSticky()) {
     221#if PLATFORM(IOS_FAMILY)
    222222        // If we got a new layer for a fixed or sticky node, its position from the WebProcess is probably stale. We need to re-run the "viewport" changed logic to udpate it with our UI-side state.
    223223        FloatRect layoutViewport = m_webPageProxy.computeCustomFixedPositionRect(m_webPageProxy.unobscuredContentRect(), m_webPageProxy.unobscuredContentRectRespectingInputViewBounds(), m_webPageProxy.customFixedPositionRect(), m_webPageProxy.displayedContentScale(), FrameView::LayoutViewportConstraint::Unconstrained);
    224224        m_webPageProxy.scrollingCoordinatorProxy()->viewportChangedViaDelegatedScrolling(m_webPageProxy.unobscuredContentRect().location(), layoutViewport, m_webPageProxy.displayedContentScale());
    225     }
    226 #endif
     225#else
     226        m_webPageProxy.scrollingCoordinatorProxy()->applyScrollingTreeLayerPositions();
     227#endif
     228    }
    227229
    228230    // Handle requested scroll position updates from the scrolling tree transaction after didCommitLayerTree()
  • trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp

    r242359 r242687  
    185185}
    186186
     187void RemoteScrollingCoordinatorProxy::applyScrollingTreeLayerPositions()
     188{
     189    m_scrollingTree->applyLayerPositions();
     190}
     191
    187192void RemoteScrollingCoordinatorProxy::currentSnapPointIndicesDidChange(WebCore::ScrollingNodeID nodeID, unsigned horizontal, unsigned vertical)
    188193{
  • trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.h

    r242132 r242687  
    6060    // Called externally when native views move around.
    6161    void viewportChangedViaDelegatedScrolling(const WebCore::FloatPoint& scrollPosition, const WebCore::FloatRect& layoutViewport, double scale);
     62
     63    void applyScrollingTreeLayerPositions();
    6264
    6365    void currentSnapPointIndicesDidChange(WebCore::ScrollingNodeID, unsigned horizontal, unsigned vertical);
  • trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

    r242624 r242687  
    497497
    498498#if ENABLE(ASYNC_SCROLLING)
    499         if (ScrollingCoordinator* scrollingCoordinator = m_webPage.corePage()->scrollingCoordinator())
     499        if (auto* scrollingCoordinator = m_webPage.corePage()->scrollingCoordinator()) {
    500500            scrollingCoordinator->commitTreeStateIfNeeded();
     501            scrollingCoordinator->applyScrollingTreeLayerPositions();
     502        }
    501503#endif
    502504
Note: See TracChangeset for help on using the changeset viewer.