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

Changeset 286977 in webkit


Ignore:
Timestamp:
Dec 13, 2021, 12:57:05 PM (5 years ago)
Author:
Alan Coon
Message:

Cherry-pick r286905. rdar://problem/86235740

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):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@286905 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-612-branch/Source
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-612-branch/Source/WebCore/ChangeLog

    r286975 r286977  
     12021-12-13  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r286905. rdar://problem/86235740
     4
     5    Scrolling can drop frames when CoreAnimation commits take a long time
     6    https://bugs.webkit.org/show_bug.cgi?id=234160
     7    <rdar://86235740>
     8   
     9    Reviewed by Tim Horton.
     10   
     11    In r261985 I added a mechanism that has the scrolling thread wait for the main thread to
     12    finish a rendering update, and, if the main thread fails to complete in time, then the
     13    scrolling thread commits. This allows for scrolling synchronization when the main thread is
     14    responsive, but smooth scrolling when the main thread is busy.
     15   
     16    However, r261985 only waits for WebKit work to finish; what we really care about is whether
     17    the main thread completes its CA commit in time (because that determines whether the scroll
     18    shows on the screen).
     19   
     20    So plumb through pre-/post-commit hooks from TiledCoreAnimationDrawingArea, which already
     21    had them for inspector instrumentation. Then have ThreadedScrollingTree notify
     22    m_stateCondition in didCompletePlatformRenderingUpdate(), instead of
     23    didCompleteRenderingUpdate().
     24    Source/WebCore:
     25   
     26    Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     27   
     28    * page/Page.cpp:
     29    (WebCore::Page::willStartPlatformRenderingUpdate):
     30    (WebCore::Page::didCompletePlatformRenderingUpdate):
     31    * page/Page.h:
     32    * page/scrolling/ScrollingCoordinator.h:
     33    (WebCore::ScrollingCoordinator::willStartPlatformRenderingUpdate):
     34    (WebCore::ScrollingCoordinator::didCompletePlatformRenderingUpdate):
     35    * page/scrolling/ThreadedScrollingTree.cpp:
     36    (WebCore::ThreadedScrollingTree::didCompletePlatformRenderingUpdate):
     37    (WebCore::ThreadedScrollingTree::didCompleteRenderingUpdate): Deleted.
     38    * page/scrolling/ThreadedScrollingTree.h:
     39    * page/scrolling/mac/ScrollingCoordinatorMac.h:
     40    * page/scrolling/mac/ScrollingCoordinatorMac.mm:
     41    (WebCore::ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate):
     42   
     43    Source/WebKit:
     44   
     45    Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     46   
     47    * WebProcess/WebPage/WebPage.cpp:
     48    (WebKit::WebPage::willStartPlatformRenderingUpdate):
     49    (WebKit::WebPage::didCompletePlatformRenderingUpdate):
     50    * WebProcess/WebPage/WebPage.h:
     51    * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
     52    (WebKit::TiledCoreAnimationDrawingArea::addCommitHandlers):
     53   
     54    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286905 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     55
     56    2021-12-10  Simon Fraser  <simon.fraser@apple.com>
     57
     58            Scrolling can drop frames when CoreAnimation commits take a long time
     59            https://bugs.webkit.org/show_bug.cgi?id=234160
     60            <rdar://86235740>
     61
     62            Reviewed by Tim Horton.
     63
     64            In r261985 I added a mechanism that has the scrolling thread wait for the main thread to
     65            finish a rendering update, and, if the main thread fails to complete in time, then the
     66            scrolling thread commits. This allows for scrolling synchronization when the main thread is
     67            responsive, but smooth scrolling when the main thread is busy.
     68
     69            However, r261985 only waits for WebKit work to finish; what we really care about is whether
     70            the main thread completes its CA commit in time (because that determines whether the scroll
     71            shows on the screen).
     72
     73            So plumb through pre-/post-commit hooks from TiledCoreAnimationDrawingArea, which already
     74            had them for inspector instrumentation. Then have ThreadedScrollingTree notify
     75            m_stateCondition in didCompletePlatformRenderingUpdate(), instead of
     76            didCompleteRenderingUpdate().
     77
     78            Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     79
     80            * page/Page.cpp:
     81            (WebCore::Page::willStartPlatformRenderingUpdate):
     82            (WebCore::Page::didCompletePlatformRenderingUpdate):
     83            * page/Page.h:
     84            * page/scrolling/ScrollingCoordinator.h:
     85            (WebCore::ScrollingCoordinator::willStartPlatformRenderingUpdate):
     86            (WebCore::ScrollingCoordinator::didCompletePlatformRenderingUpdate):
     87            * page/scrolling/ThreadedScrollingTree.cpp:
     88            (WebCore::ThreadedScrollingTree::didCompletePlatformRenderingUpdate):
     89            (WebCore::ThreadedScrollingTree::didCompleteRenderingUpdate): Deleted.
     90            * page/scrolling/ThreadedScrollingTree.h:
     91            * page/scrolling/mac/ScrollingCoordinatorMac.h:
     92            * page/scrolling/mac/ScrollingCoordinatorMac.mm:
     93            (WebCore::ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate):
     94
    1952021-12-13  Alan Coon  <alancoon@apple.com>
    296

  • branches/safari-612-branch/Source/WebCore/page/Page.cpp

    r284351 r286977  
    17661766}
    17671767
     1768void Page::willStartPlatformRenderingUpdate()
     1769{
     1770    // Inspector's use of "composite" is rather innacurate. On Apple platforms, the "composite" step happens
     1771    // in another process; these hooks wrap the non-WebKit CA commit time which is mostly painting-related.
     1772    m_inspectorController->willComposite(mainFrame());
     1773
     1774    if (m_scrollingCoordinator)
     1775        m_scrollingCoordinator->willStartPlatformRenderingUpdate();
     1776}
     1777
     1778void Page::didCompletePlatformRenderingUpdate()
     1779{
     1780    if (m_scrollingCoordinator)
     1781        m_scrollingCoordinator->didCompletePlatformRenderingUpdate();
     1782
     1783    m_inspectorController->didComposite(mainFrame());
     1784}
     1785
    17681786void Page::prioritizeVisibleResources()
    17691787{

  • branches/safari-612-branch/Source/WebCore/page/Page.h

    r284351 r286977  
    585585    WEBCORE_EXPORT unsigned renderingUpdateCount() const;
    586586
     587    // A "platform rendering update" here describes the work done by the system graphics framework before work is submitted to the system compositor.
     588    // On macOS, this is a CoreAnimation commit.
     589    WEBCORE_EXPORT void willStartPlatformRenderingUpdate();
     590    WEBCORE_EXPORT void didCompletePlatformRenderingUpdate();
     591
    587592    WEBCORE_EXPORT void suspendScriptedAnimations();
    588593    WEBCORE_EXPORT void resumeScriptedAnimations();

  • branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingCoordinator.h

    r278419 r286977  
    103103    virtual void didCompleteRenderingUpdate() { }
    104104
     105    virtual void willStartPlatformRenderingUpdate() { }
     106    virtual void didCompletePlatformRenderingUpdate() { }
     107
    105108#if ENABLE(KINETIC_SCROLLING)
    106109    // Dispatched by the scrolling tree during handleWheelEvent. This is required as long as scrollbars are painted on the main thread.

  • branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp

    r286975 r286977  
    311311
    312312// This code allows the main thread about half a frame to complete its rendering udpate. If the main thread
    313 // is responsive (i.e. managing to render every frame), then we expect to get a didCompleteRenderingUpdate()
     313// is responsive (i.e. managing to render every frame), then we expect to get a didCompletePlatformRenderingUpdate()
    314314// within 8ms of willStartRenderingUpdate(). We time this via m_stateCondition, which blocks the scrolling
    315315// thread (with m_treeLock locked at the start and end) so that we don't handle wheel events while waiting.
    … …  
    352352void ThreadedScrollingTree::didCompleteRenderingUpdate()
    353353{
     354    // macOS needs to wait for the CA commit (the "platform rendering update").
     355#if !PLATFORM(MAC)
     356    renderingUpdateComplete();
     357#endif
     358}
     359
     360void ThreadedScrollingTree::didCompletePlatformRenderingUpdate()
     361{
     362    renderingUpdateComplete();
     363}
     364
     365void ThreadedScrollingTree::renderingUpdateComplete()
     366{
    354367    ASSERT(isMainThread());
     368
    355369    Locker locker { m_treeLock };
    356370

  • branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.h

    r286975 r286977  
    6161    void didCompleteRenderingUpdate();
    6262
     63    void didCompletePlatformRenderingUpdate();
     64
    6365    Lock& treeLock() WTF_RETURNS_LOCK(m_treeLock) { return m_treeLock; }
    6466
    … …  
    8991    void displayDidRefreshOnScrollingThread();
    9092    void waitForRenderingUpdateCompletionOrTimeout() WTF_REQUIRES_LOCK(m_treeLock);
     93    void renderingUpdateComplete();
    9194
    9295    bool canUpdateLayersOnScrollingThread() const WTF_REQUIRES_LOCK(m_treeLock);

  • branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h

    r278253 r286977  
    5151    void didCompleteRenderingUpdate() final;
    5252
     53    void didCompletePlatformRenderingUpdate() final;
     54
    5355    void updateTiledScrollingIndicator();
    5456

  • branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm

    r278253 r286977  
    136136}
    137137
     138void ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate()
     139{
     140    downcast<ThreadedScrollingTree>(scrollingTree())->didCompletePlatformRenderingUpdate();
     141}
     142
    138143void ScrollingCoordinatorMac::updateTiledScrollingIndicator()
    139144{

  • branches/safari-612-branch/Source/WebKit/ChangeLog

    r286976 r286977  
     12021-12-13  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r286905. rdar://problem/86235740
     4
     5    Scrolling can drop frames when CoreAnimation commits take a long time
     6    https://bugs.webkit.org/show_bug.cgi?id=234160
     7    <rdar://86235740>
     8   
     9    Reviewed by Tim Horton.
     10   
     11    In r261985 I added a mechanism that has the scrolling thread wait for the main thread to
     12    finish a rendering update, and, if the main thread fails to complete in time, then the
     13    scrolling thread commits. This allows for scrolling synchronization when the main thread is
     14    responsive, but smooth scrolling when the main thread is busy.
     15   
     16    However, r261985 only waits for WebKit work to finish; what we really care about is whether
     17    the main thread completes its CA commit in time (because that determines whether the scroll
     18    shows on the screen).
     19   
     20    So plumb through pre-/post-commit hooks from TiledCoreAnimationDrawingArea, which already
     21    had them for inspector instrumentation. Then have ThreadedScrollingTree notify
     22    m_stateCondition in didCompletePlatformRenderingUpdate(), instead of
     23    didCompleteRenderingUpdate().
     24    Source/WebCore:
     25   
     26    Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     27   
     28    * page/Page.cpp:
     29    (WebCore::Page::willStartPlatformRenderingUpdate):
     30    (WebCore::Page::didCompletePlatformRenderingUpdate):
     31    * page/Page.h:
     32    * page/scrolling/ScrollingCoordinator.h:
     33    (WebCore::ScrollingCoordinator::willStartPlatformRenderingUpdate):
     34    (WebCore::ScrollingCoordinator::didCompletePlatformRenderingUpdate):
     35    * page/scrolling/ThreadedScrollingTree.cpp:
     36    (WebCore::ThreadedScrollingTree::didCompletePlatformRenderingUpdate):
     37    (WebCore::ThreadedScrollingTree::didCompleteRenderingUpdate): Deleted.
     38    * page/scrolling/ThreadedScrollingTree.h:
     39    * page/scrolling/mac/ScrollingCoordinatorMac.h:
     40    * page/scrolling/mac/ScrollingCoordinatorMac.mm:
     41    (WebCore::ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate):
     42   
     43    Source/WebKit:
     44   
     45    Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     46   
     47    * WebProcess/WebPage/WebPage.cpp:
     48    (WebKit::WebPage::willStartPlatformRenderingUpdate):
     49    (WebKit::WebPage::didCompletePlatformRenderingUpdate):
     50    * WebProcess/WebPage/WebPage.h:
     51    * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
     52    (WebKit::TiledCoreAnimationDrawingArea::addCommitHandlers):
     53   
     54    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286905 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     55
     56    2021-12-10  Simon Fraser  <simon.fraser@apple.com>
     57
     58            Scrolling can drop frames when CoreAnimation commits take a long time
     59            https://bugs.webkit.org/show_bug.cgi?id=234160
     60            <rdar://86235740>
     61
     62            Reviewed by Tim Horton.
     63
     64            In r261985 I added a mechanism that has the scrolling thread wait for the main thread to
     65            finish a rendering update, and, if the main thread fails to complete in time, then the
     66            scrolling thread commits. This allows for scrolling synchronization when the main thread is
     67            responsive, but smooth scrolling when the main thread is busy.
     68
     69            However, r261985 only waits for WebKit work to finish; what we really care about is whether
     70            the main thread completes its CA commit in time (because that determines whether the scroll
     71            shows on the screen).
     72
     73            So plumb through pre-/post-commit hooks from TiledCoreAnimationDrawingArea, which already
     74            had them for inspector instrumentation. Then have ThreadedScrollingTree notify
     75            m_stateCondition in didCompletePlatformRenderingUpdate(), instead of
     76            didCompleteRenderingUpdate().
     77
     78            Also, now we can call the inspector hooks from Page, rather than out in TiledCoreAnimationDrawingArea.
     79
     80            * WebProcess/WebPage/WebPage.cpp:
     81            (WebKit::WebPage::willStartPlatformRenderingUpdate):
     82            (WebKit::WebPage::didCompletePlatformRenderingUpdate):
     83            * WebProcess/WebPage/WebPage.h:
     84            * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
     85            (WebKit::TiledCoreAnimationDrawingArea::addCommitHandlers):
     86
    1872021-12-13  Alan Coon  <alancoon@apple.com>
    288

  • branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r286683 r286977  
    42454245}
    42464246
     4247void WebPage::willStartPlatformRenderingUpdate()
     4248{
     4249    if (m_isClosed)
     4250        return;
     4251    m_page->willStartPlatformRenderingUpdate();
     4252}
     4253
     4254void WebPage::didCompletePlatformRenderingUpdate()
     4255{
     4256    if (m_isClosed)
     4257        return;
     4258    m_page->didCompletePlatformRenderingUpdate();
     4259}
     4260
    42474261void WebPage::releaseMemory(Critical)
    42484262{

  • branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.h

    r286683 r286977  
    660660    void didUpdateRendering();
    661661
     662    // A "platform rendering update" here describes the work done by the system graphics framework before work is submitted to the system compositor.
     663    // On macOS, this is a CoreAnimation commit.
     664    void willStartPlatformRenderingUpdate();
     665    void didCompletePlatformRenderingUpdate();
     666
    662667#if PLATFORM(MAC)
    663668    void setTopOverhangImage(WebImage*);

  • branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

    r286976 r286977  
    423423
    424424    [CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
    425         if (Page* corePage = retainedPage->corePage()) {
    426             if (Frame* coreFrame = retainedPage->mainFrame())
    427                 corePage->inspectorController().willComposite(*coreFrame);
    428         }
     425        retainedPage->willStartPlatformRenderingUpdate();
    429426    } forPhase:kCATransactionPhasePreLayout];
    430427
    431428    [CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
    432         if (Page* corePage = retainedPage->corePage()) {
    433             if (Frame* coreFrame = retainedPage->mainFrame())
    434                 corePage->inspectorController().didComposite(*coreFrame);
    435         }
    436429        if (auto drawingArea = static_cast<TiledCoreAnimationDrawingArea*>(retainedPage->drawingArea()))
    437430            drawingArea->sendPendingNewlyReachedPaintingMilestones();
     431
    438432        retainedPage->setFirstFlushAfterCommit(false);
     433        retainedPage->didCompletePlatformRenderingUpdate();
    439434    } forPhase:kCATransactionPhasePostCommit];
    440435   
Note: See TracChangeset for help on using the changeset viewer.