- Revision
- 286905
- Author
- simon.fra...@apple.com
- Date
- 2021-12-10 23:17:48 -0800 (Fri, 10 Dec 2021)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (286904 => 286905)
--- trunk/Source/WebCore/ChangeLog 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/ChangeLog 2021-12-11 07:17:48 UTC (rev 286905)
@@ -1,3 +1,42 @@
+2021-12-10 Simon Fraser <simon.fra...@apple.com>
+
+ 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-10 Chris Dumez <cdu...@apple.com>
Implement AbortSignal.throwIfAborted
Modified: trunk/Source/WebCore/page/Page.cpp (286904 => 286905)
--- trunk/Source/WebCore/page/Page.cpp 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/page/Page.cpp 2021-12-11 07:17:48 UTC (rev 286905)
@@ -1792,6 +1792,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: trunk/Source/WebCore/page/Page.h (286904 => 286905)
--- trunk/Source/WebCore/page/Page.h 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/page/Page.h 2021-12-11 07:17:48 UTC (rev 286905)
@@ -620,6 +620,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: trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h (286904 => 286905)
--- trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h 2021-12-11 07:17:48 UTC (rev 286905)
@@ -104,6 +104,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: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (286904 => 286905)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-11 07:17:48 UTC (rev 286905)
@@ -389,7 +389,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
@@ -430,7 +430,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: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h (286904 => 286905)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-11 07:17:48 UTC (rev 286905)
@@ -61,6 +61,8 @@
void willStartRenderingUpdate();
void didCompleteRenderingUpdate();
+ void didCompletePlatformRenderingUpdate();
+
Lock& treeLock() WTF_RETURNS_LOCK(m_treeLock) { return m_treeLock; }
bool scrollAnimatorEnabled() const { return m_scrollAnimatorEnabled; }
@@ -96,6 +98,7 @@
void displayDidRefreshOnScrollingThread();
void waitForRenderingUpdateCompletionOrTimeout() WTF_REQUIRES_LOCK(m_treeLock);
+ void renderingUpdateComplete();
bool canUpdateLayersOnScrollingThread() const WTF_REQUIRES_LOCK(m_treeLock);
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h (286904 => 286905)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h 2021-12-11 07:17:48 UTC (rev 286905)
@@ -53,6 +53,8 @@
void willStartRenderingUpdate() final;
void didCompleteRenderingUpdate() final;
+ void didCompletePlatformRenderingUpdate() final;
+
void updateTiledScrollingIndicator();
void startMonitoringWheelEvents(bool clearLatchingState) final;
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm (286904 => 286905)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm 2021-12-11 07:17:48 UTC (rev 286905)
@@ -142,6 +142,11 @@
scheduleRenderingUpdate();
}
+void ScrollingCoordinatorMac::didCompletePlatformRenderingUpdate()
+{
+ downcast<ThreadedScrollingTree>(scrollingTree())->didCompletePlatformRenderingUpdate();
+}
+
void ScrollingCoordinatorMac::hasNodeWithAnimatedScrollChanged(bool hasAnimatingNode)
{
// This is necessary to trigger a rendering update, after which the code in
Modified: trunk/Source/WebKit/ChangeLog (286904 => 286905)
--- trunk/Source/WebKit/ChangeLog 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebKit/ChangeLog 2021-12-11 07:17:48 UTC (rev 286905)
@@ -1,3 +1,34 @@
+2021-12-10 Simon Fraser <simon.fra...@apple.com>
+
+ 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-10 Tim Horton <timothy_hor...@apple.com>
Momentum Event Dispatcher: Magic Mouse doesn't use momentum event dispatcher
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (286904 => 286905)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-12-11 07:17:48 UTC (rev 286905)
@@ -4288,6 +4288,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: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (286904 => 286905)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-12-11 07:17:48 UTC (rev 286905)
@@ -666,6 +666,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: trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm (286904 => 286905)
--- trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm 2021-12-11 07:00:12 UTC (rev 286904)
+++ trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm 2021-12-11 07:17:48 UTC (rev 286905)
@@ -414,20 +414,15 @@
return;
[CATransaction addCommitHandler:[retainedPage = Ref { m_webPage }] {
- if (Page* corePage = retainedPage->corePage()) {
- if (Frame* coreFrame = retainedPage->mainFrame())
- corePage->inspectorController().willComposite(*coreFrame);
- }
+ retainedPage->willStartPlatformRenderingUpdate();
} forPhase:kCATransactionPhasePreLayout];
[CATransaction addCommitHandler:[retainedPage = Ref { 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);
