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

Changeset 286905 in webkit


Ignore:
Timestamp:
Dec 10, 2021, 11:17:48 PM (5 years ago)
Author:
Simon Fraser
Message:

Scrolling can drop frames when CoreAnimation commits take a long time
​https://bugs.webkit.org/show_bug.cgi?id=234160
<rdar://86235740>

Reviewed by Tim Horton.

In r261985 I added a mechanism that has the scrolling thread wait for the main thread to
finish a rendering update, and, if the main thread fails to complete in time, then the
scrolling thread commits. This allows for scrolling synchronization when the main thread is
responsive, but smooth scrolling when the main thread is busy.

However, r261985 only waits for WebKit work to finish; what we really care about is whether
the main thread completes its CA commit in time (because that determines whether the scroll
shows on the screen).

So plumb through pre-/post-commit hooks from TiledCoreAnimationDrawingArea, which already
had them for inspector instrumentation. Then have ThreadedScrollingTree notify
m_stateCondition in didCompletePlatformRenderingUpdate(), instead of
didCompleteRenderingUpdate().
Source/WebCore:

Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.

  • page/Page.cpp:

(WebCore::Page::willStartPlatformRenderingUpdate):
(WebCore::Page::didCompletePlatformRenderingUpdate):

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

(WebCore::ScrollingCoordinator::willStartPlatformRenderingUpdate):
(WebCore::ScrollingCoordinator::didCompletePlatformRenderingUpdate):

  • page/scrolling/ThreadedScrollingTree.cpp:

(WebCore::ThreadedScrollingTree::didCompletePlatformRenderingUpdate):
(WebCore::ThreadedScrollingTree::didCompleteRenderingUpdate): Deleted.

  • page/scrolling/ThreadedScrollingTree.h:
  • page/scrolling/mac/ScrollingCoordinatorMac.h:
  • page/scrolling/mac/ScrollingCoordinatorMac.mm:

(WebCore::ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate):

Source/WebKit:

Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::willStartPlatformRenderingUpdate):
(WebKit::WebPage::didCompletePlatformRenderingUpdate):

  • WebProcess/WebPage/WebPage.h:
  • WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:

(WebKit::TiledCoreAnimationDrawingArea::addCommitHandlers):

Location:
trunk/Source
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r286904 r286905  
     12021-12-10  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Scrolling can drop frames when CoreAnimation commits take a long time
     4        https://bugs.webkit.org/show_bug.cgi?id=234160
     5        <rdar://86235740>
     6
     7        Reviewed by Tim Horton.
     8
     9        In r261985 I added a mechanism that has the scrolling thread wait for the main thread to
     10        finish a rendering update, and, if the main thread fails to complete in time, then the
     11        scrolling thread commits. This allows for scrolling synchronization when the main thread is
     12        responsive, but smooth scrolling when the main thread is busy.
     13
     14        However, r261985 only waits for WebKit work to finish; what we really care about is whether
     15        the main thread completes its CA commit in time (because that determines whether the scroll
     16        shows on the screen).
     17
     18        So plumb through pre-/post-commit hooks from TiledCoreAnimationDrawingArea, which already
     19        had them for inspector instrumentation. Then have ThreadedScrollingTree notify
     20        m_stateCondition in didCompletePlatformRenderingUpdate(), instead of
     21        didCompleteRenderingUpdate().
     22       
     23        Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     24
     25        * page/Page.cpp:
     26        (WebCore::Page::willStartPlatformRenderingUpdate):
     27        (WebCore::Page::didCompletePlatformRenderingUpdate):
     28        * page/Page.h:
     29        * page/scrolling/ScrollingCoordinator.h:
     30        (WebCore::ScrollingCoordinator::willStartPlatformRenderingUpdate):
     31        (WebCore::ScrollingCoordinator::didCompletePlatformRenderingUpdate):
     32        * page/scrolling/ThreadedScrollingTree.cpp:
     33        (WebCore::ThreadedScrollingTree::didCompletePlatformRenderingUpdate):
     34        (WebCore::ThreadedScrollingTree::didCompleteRenderingUpdate): Deleted.
     35        * page/scrolling/ThreadedScrollingTree.h:
     36        * page/scrolling/mac/ScrollingCoordinatorMac.h:
     37        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
     38        (WebCore::ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate):
     39
    1402021-12-10  Chris Dumez  <cdumez@apple.com>
    241

  • trunk/Source/WebCore/page/Page.cpp

    r286772 r286905  
    17931793}
    17941794
     1795void Page::willStartPlatformRenderingUpdate()
     1796{
     1797    // Inspector's use of "composite" is rather innacurate. On Apple platforms, the "composite" step happens
     1798    // in another process; these hooks wrap the non-WebKit CA commit time which is mostly painting-related.
     1799    m_inspectorController->willComposite(mainFrame());
     1800
     1801    if (m_scrollingCoordinator)
     1802        m_scrollingCoordinator->willStartPlatformRenderingUpdate();
     1803}
     1804
     1805void Page::didCompletePlatformRenderingUpdate()
     1806{
     1807    if (m_scrollingCoordinator)
     1808        m_scrollingCoordinator->didCompletePlatformRenderingUpdate();
     1809
     1810    m_inspectorController->didComposite(mainFrame());
     1811}
     1812
    17951813void Page::prioritizeVisibleResources()
    17961814{

  • trunk/Source/WebCore/page/Page.h

    r286767 r286905  
    621621    WEBCORE_EXPORT unsigned renderingUpdateCount() const;
    622622
     623    // A "platform rendering update" here describes the work done by the system graphics framework before work is submitted to the system compositor.
     624    // On macOS, this is a CoreAnimation commit.
     625    WEBCORE_EXPORT void willStartPlatformRenderingUpdate();
     626    WEBCORE_EXPORT void didCompletePlatformRenderingUpdate();
     627
    623628    WEBCORE_EXPORT void suspendScriptedAnimations();
    624629    WEBCORE_EXPORT void resumeScriptedAnimations();

  • trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h

    r284738 r286905  
    105105    virtual void didCompleteRenderingUpdate() { }
    106106
     107    virtual void willStartPlatformRenderingUpdate() { }
     108    virtual void didCompletePlatformRenderingUpdate() { }
     109
    107110#if ENABLE(KINETIC_SCROLLING)
    108111    // 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/ThreadedScrollingTree.cpp

    r286411 r286905  
    390390
    391391// This code allows the main thread about half a frame to complete its rendering udpate. If the main thread
    392 // is responsive (i.e. managing to render every frame), then we expect to get a didCompleteRenderingUpdate()
     392// is responsive (i.e. managing to render every frame), then we expect to get a didCompletePlatformRenderingUpdate()
    393393// within 8ms of willStartRenderingUpdate(). We time this via m_stateCondition, which blocks the scrolling
    394394// thread (with m_treeLock locked at the start and end) so that we don't handle wheel events while waiting.
    … …  
    431431void ThreadedScrollingTree::didCompleteRenderingUpdate()
    432432{
     433    // macOS needs to wait for the CA commit (the "platform rendering update").
     434#if !PLATFORM(MAC)
     435    renderingUpdateComplete();
     436#endif
     437}
     438
     439void ThreadedScrollingTree::didCompletePlatformRenderingUpdate()
     440{
     441    renderingUpdateComplete();
     442}
     443
     444void ThreadedScrollingTree::renderingUpdateComplete()
     445{
    433446    ASSERT(isMainThread());
     447
    434448    Locker locker { m_treeLock };
    435449

  • trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h

    r286410 r286905  
    6262    void didCompleteRenderingUpdate();
    6363
     64    void didCompletePlatformRenderingUpdate();
     65
    6466    Lock& treeLock() WTF_RETURNS_LOCK(m_treeLock) { return m_treeLock; }
    6567
    … …  
    9799    void displayDidRefreshOnScrollingThread();
    98100    void waitForRenderingUpdateCompletionOrTimeout() WTF_REQUIRES_LOCK(m_treeLock);
     101    void renderingUpdateComplete();
    99102
    100103    bool canUpdateLayersOnScrollingThread() const WTF_REQUIRES_LOCK(m_treeLock);

  • trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h

    r285094 r286905  
    5454    void didCompleteRenderingUpdate() final;
    5555
     56    void didCompletePlatformRenderingUpdate() final;
     57
    5658    void updateTiledScrollingIndicator();
    5759

  • trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm

    r285094 r286905  
    143143}
    144144
     145void ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate()
     146{
     147    downcast<ThreadedScrollingTree>(scrollingTree())->didCompletePlatformRenderingUpdate();
     148}
     149
    145150void ScrollingCoordinatorMac::hasNodeWithAnimatedScrollChanged(bool hasAnimatingNode)
    146151{

  • trunk/Source/WebKit/ChangeLog

    r286900 r286905  
     12021-12-10  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Scrolling can drop frames when CoreAnimation commits take a long time
     4        https://bugs.webkit.org/show_bug.cgi?id=234160
     5        <rdar://86235740>
     6
     7        Reviewed by Tim Horton.
     8
     9        In r261985 I added a mechanism that has the scrolling thread wait for the main thread to
     10        finish a rendering update, and, if the main thread fails to complete in time, then the
     11        scrolling thread commits. This allows for scrolling synchronization when the main thread is
     12        responsive, but smooth scrolling when the main thread is busy.
     13
     14        However, r261985 only waits for WebKit work to finish; what we really care about is whether
     15        the main thread completes its CA commit in time (because that determines whether the scroll
     16        shows on the screen).
     17
     18        So plumb through pre-/post-commit hooks from TiledCoreAnimationDrawingArea, which already
     19        had them for inspector instrumentation. Then have ThreadedScrollingTree notify
     20        m_stateCondition in didCompletePlatformRenderingUpdate(), instead of
     21        didCompleteRenderingUpdate().
     22
     23        Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     24
     25        * WebProcess/WebPage/WebPage.cpp:
     26        (WebKit::WebPage::willStartPlatformRenderingUpdate):
     27        (WebKit::WebPage::didCompletePlatformRenderingUpdate):
     28        * WebProcess/WebPage/WebPage.h:
     29        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
     30        (WebKit::TiledCoreAnimationDrawingArea::addCommitHandlers):
     31
    1322021-12-10  Tim Horton  <timothy_horton@apple.com>
    233

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r286825 r286905  
    42894289}
    42904290
     4291void WebPage::willStartPlatformRenderingUpdate()
     4292{
     4293    if (m_isClosed)
     4294        return;
     4295    m_page->willStartPlatformRenderingUpdate();
     4296}
     4297
     4298void WebPage::didCompletePlatformRenderingUpdate()
     4299{
     4300    if (m_isClosed)
     4301        return;
     4302    m_page->didCompletePlatformRenderingUpdate();
     4303}
     4304
    42914305void WebPage::releaseMemory(Critical)
    42924306{

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r286886 r286905  
    667667    void didUpdateRendering();
    668668
     669    // A "platform rendering update" here describes the work done by the system graphics framework before work is submitted to the system compositor.
     670    // On macOS, this is a CoreAnimation commit.
     671    void willStartPlatformRenderingUpdate();
     672    void didCompletePlatformRenderingUpdate();
     673
    669674#if PLATFORM(MAC)
    670675    void setTopOverhangImage(WebImage*);

  • trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

    r286834 r286905  
    415415
    416416    [CATransaction addCommitHandler:[retainedPage = Ref { m_webPage }] {
    417         if (Page* corePage = retainedPage->corePage()) {
    418             if (Frame* coreFrame = retainedPage->mainFrame())
    419                 corePage->inspectorController().willComposite(*coreFrame);
    420         }
     417        retainedPage->willStartPlatformRenderingUpdate();
    421418    } forPhase:kCATransactionPhasePreLayout];
    422419
    423420    [CATransaction addCommitHandler:[retainedPage = Ref { m_webPage }] {
    424         if (Page* corePage = retainedPage->corePage()) {
    425             if (Frame* coreFrame = retainedPage->mainFrame())
    426                 corePage->inspectorController().didComposite(*coreFrame);
    427         }
    428421        if (auto drawingArea = static_cast<TiledCoreAnimationDrawingArea*>(retainedPage->drawingArea()))
    429422            drawingArea->sendPendingNewlyReachedPaintingMilestones();
     423
    430424        retainedPage->setFirstFlushAfterCommit(false);
     425        retainedPage->didCompletePlatformRenderingUpdate();
    431426    } forPhase:kCATransactionPhasePostCommit];
    432427   
Note: See TracChangeset for help on using the changeset viewer.