- Revision
- 286977
- Author
- [email protected]
- Date
- 2021-12-13 12:57:05 -0800 (Mon, 13 Dec 2021)
Log 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
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/WebCore/ChangeLog (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/ChangeLog 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/ChangeLog 2021-12-13 20:57:05 UTC (rev 286977)
@@ -1,5 +1,99 @@
2021-12-13 Alan Coon <[email protected]>
+ 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
+
+ 2021-12-10 Simon Fraser <[email protected]>
+
+ 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().
+
+ 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):
+
+2021-12-13 Alan Coon <[email protected]>
+
Cherry-pick r286932. rdar://problem/86385697
Ensure that the scrolling thread always commits layer position changes to reduce scrolling stutters
Modified: branches/safari-612-branch/Source/WebCore/page/Page.cpp (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/page/Page.cpp 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/page/Page.cpp 2021-12-13 20:57:05 UTC (rev 286977)
@@ -1765,6 +1765,24 @@
}
}
+void Page::willStartPlatformRenderingUpdate()
+{
+ // Inspector's use of "composite" is rather innacurate. On Apple platforms, the "composite" step happens
+ // in another process; these hooks wrap the non-WebKit CA commit time which is mostly painting-related.
+ m_inspectorController->willComposite(mainFrame());
+
+ if (m_scrollingCoordinator)
+ m_scrollingCoordinator->willStartPlatformRenderingUpdate();
+}
+
+void Page::didCompletePlatformRenderingUpdate()
+{
+ if (m_scrollingCoordinator)
+ m_scrollingCoordinator->didCompletePlatformRenderingUpdate();
+
+ m_inspectorController->didComposite(mainFrame());
+}
+
void Page::prioritizeVisibleResources()
{
if (loadSchedulingMode() == LoadSchedulingMode::Direct)
Modified: branches/safari-612-branch/Source/WebCore/page/Page.h (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/page/Page.h 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/page/Page.h 2021-12-13 20:57:05 UTC (rev 286977)
@@ -584,6 +584,11 @@
WEBCORE_EXPORT void startTrackingRenderingUpdates();
WEBCORE_EXPORT unsigned renderingUpdateCount() const;
+ // A "platform rendering update" here describes the work done by the system graphics framework before work is submitted to the system compositor.
+ // On macOS, this is a CoreAnimation commit.
+ WEBCORE_EXPORT void willStartPlatformRenderingUpdate();
+ WEBCORE_EXPORT void didCompletePlatformRenderingUpdate();
+
WEBCORE_EXPORT void suspendScriptedAnimations();
WEBCORE_EXPORT void resumeScriptedAnimations();
bool scriptedAnimationsSuspended() const { return m_scriptedAnimationsSuspended; }
Modified: branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingCoordinator.h (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2021-12-13 20:57:05 UTC (rev 286977)
@@ -102,6 +102,9 @@
virtual void willStartRenderingUpdate() { }
virtual void didCompleteRenderingUpdate() { }
+ virtual void willStartPlatformRenderingUpdate() { }
+ virtual void didCompletePlatformRenderingUpdate() { }
+
#if ENABLE(KINETIC_SCROLLING)
// Dispatched by the scrolling tree during handleWheelEvent. This is required as long as scrollbars are painted on the main thread.
virtual void handleWheelEventPhase(ScrollingNodeID, PlatformWheelEventPhase) { }
Modified: branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-13 20:57:05 UTC (rev 286977)
@@ -310,7 +310,7 @@
}
// This code allows the main thread about half a frame to complete its rendering udpate. If the main thread
-// is responsive (i.e. managing to render every frame), then we expect to get a didCompleteRenderingUpdate()
+// is responsive (i.e. managing to render every frame), then we expect to get a didCompletePlatformRenderingUpdate()
// within 8ms of willStartRenderingUpdate(). We time this via m_stateCondition, which blocks the scrolling
// thread (with m_treeLock locked at the start and end) so that we don't handle wheel events while waiting.
// If the condition times out, we know the main thread is being slow, and allow the scrolling thread to
@@ -351,7 +351,21 @@
void ThreadedScrollingTree::didCompleteRenderingUpdate()
{
+ // macOS needs to wait for the CA commit (the "platform rendering update").
+#if !PLATFORM(MAC)
+ renderingUpdateComplete();
+#endif
+}
+
+void ThreadedScrollingTree::didCompletePlatformRenderingUpdate()
+{
+ renderingUpdateComplete();
+}
+
+void ThreadedScrollingTree::renderingUpdateComplete()
+{
ASSERT(isMainThread());
+
Locker locker { m_treeLock };
if (m_state == SynchronizationState::InRenderingUpdate)
Modified: branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.h (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-13 20:57:05 UTC (rev 286977)
@@ -60,6 +60,8 @@
void willStartRenderingUpdate();
void didCompleteRenderingUpdate();
+ void didCompletePlatformRenderingUpdate();
+
Lock& treeLock() WTF_RETURNS_LOCK(m_treeLock) { return m_treeLock; }
bool scrollAnimatorEnabled() const { return m_scrollAnimatorEnabled; }
@@ -88,6 +90,7 @@
void displayDidRefreshOnScrollingThread();
void waitForRenderingUpdateCompletionOrTimeout() WTF_REQUIRES_LOCK(m_treeLock);
+ void renderingUpdateComplete();
bool canUpdateLayersOnScrollingThread() const WTF_REQUIRES_LOCK(m_treeLock);
Modified: branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h 2021-12-13 20:57:05 UTC (rev 286977)
@@ -50,6 +50,8 @@
void willStartRenderingUpdate() final;
void didCompleteRenderingUpdate() final;
+ void didCompletePlatformRenderingUpdate() final;
+
void updateTiledScrollingIndicator();
void startMonitoringWheelEvents(bool clearLatchingState) final;
Modified: branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (286976 => 286977)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2021-12-13 20:57:05 UTC (rev 286977)
@@ -135,6 +135,11 @@
downcast<ThreadedScrollingTree>(scrollingTree())->didCompleteRenderingUpdate();
}
+void ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate()
+{
+ downcast<ThreadedScrollingTree>(scrollingTree())->didCompletePlatformRenderingUpdate();
+}
+
void ScrollingCoordinatorMac::updateTiledScrollingIndicator()
{
FrameView* frameView = m_page->mainFrame().view();
Modified: branches/safari-612-branch/Source/WebKit/ChangeLog (286976 => 286977)
--- branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebKit/ChangeLog 2021-12-13 20:57:05 UTC (rev 286977)
@@ -1,5 +1,91 @@
2021-12-13 Alan Coon <[email protected]>
+ 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
+
+ 2021-12-10 Simon Fraser <[email protected]>
+
+ 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().
+
+ 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):
+
+2021-12-13 Alan Coon <[email protected]>
+
Cherry-pick r286834. rdar://problem/86235740
Inspector "composite" events are all very short
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp (286976 => 286977)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-12-13 20:57:05 UTC (rev 286977)
@@ -4244,6 +4244,20 @@
#endif
}
+void WebPage::willStartPlatformRenderingUpdate()
+{
+ if (m_isClosed)
+ return;
+ m_page->willStartPlatformRenderingUpdate();
+}
+
+void WebPage::didCompletePlatformRenderingUpdate()
+{
+ if (m_isClosed)
+ return;
+ m_page->didCompletePlatformRenderingUpdate();
+}
+
void WebPage::releaseMemory(Critical)
{
#if ENABLE(GPU_PROCESS)
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.h (286976 => 286977)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-12-13 20:57:05 UTC (rev 286977)
@@ -659,6 +659,11 @@
void didUpdateRendering();
+ // A "platform rendering update" here describes the work done by the system graphics framework before work is submitted to the system compositor.
+ // On macOS, this is a CoreAnimation commit.
+ void willStartPlatformRenderingUpdate();
+ void didCompletePlatformRenderingUpdate();
+
#if PLATFORM(MAC)
void setTopOverhangImage(WebImage*);
void setBottomOverhangImage(WebImage*);
Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm (286976 => 286977)
--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm 2021-12-13 20:57:00 UTC (rev 286976)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm 2021-12-13 20:57:05 UTC (rev 286977)
@@ -422,20 +422,15 @@
return;
[CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
- if (Page* corePage = retainedPage->corePage()) {
- if (Frame* coreFrame = retainedPage->mainFrame())
- corePage->inspectorController().willComposite(*coreFrame);
- }
+ retainedPage->willStartPlatformRenderingUpdate();
} forPhase:kCATransactionPhasePreLayout];
[CATransaction addCommitHandler:[retainedPage = makeRefPtr(&m_webPage)] {
- if (Page* corePage = retainedPage->corePage()) {
- if (Frame* coreFrame = retainedPage->mainFrame())
- corePage->inspectorController().didComposite(*coreFrame);
- }
if (auto drawingArea = static_cast<TiledCoreAnimationDrawingArea*>(retainedPage->drawingArea()))
drawingArea->sendPendingNewlyReachedPaintingMilestones();
+
retainedPage->setFirstFlushAfterCommit(false);
+ retainedPage->didCompletePlatformRenderingUpdate();
} forPhase:kCATransactionPhasePostCommit];
m_webPage.setFirstFlushAfterCommit(true);
