Title: [286932] trunk/Source/WebCore
- Revision
- 286932
- Author
- simon.fra...@apple.com
- Date
- 2021-12-12 18:42:35 -0800 (Sun, 12 Dec 2021)
Log Message
Ensure that the scrolling thread always commits layer position changes to reduce scrolling stutters
https://bugs.webkit.org/show_bug.cgi?id=234213
Reviewed by Tim Horton.
On a page where CA commits on the main thread take a long time (e.g. because of expensive
painting), it's possible that the main thread has updated the scrolling layer position, and
then the scrolling thread detects that the commit is taking a long time and attempts to
trigger its own commit, but because the layer position property doesn't change, no commit
occurs.
Work around this by setting the layer position to 0,0 and back when we're on the scrolling
thread. Only do this if the scroll position changed since the last display refresh to avoid
triggering redundant commits.
Ideally we'd traverse the scrolling tree and do this for every scrolling node, but scrolling
trees can get large so for now just apply this to the root node.
* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::updateScrollPositionAtLastDisplayRefresh):
* page/scrolling/ScrollingTreeScrollingNode.h:
* page/scrolling/ThreadedScrollingTree.cpp:
(WebCore::ThreadedScrollingTree::displayDidRefreshOnScrollingThread):
(WebCore::ThreadedScrollingTree::storeScrollPositionsAtLastDisplayRefresh):
* page/scrolling/ThreadedScrollingTree.h:
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
(WebCore::ScrollingTreeFrameScrollingNodeMac::repositionScrollingLayers):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (286931 => 286932)
--- trunk/Source/WebCore/ChangeLog 2021-12-13 01:46:09 UTC (rev 286931)
+++ trunk/Source/WebCore/ChangeLog 2021-12-13 02:42:35 UTC (rev 286932)
@@ -1,3 +1,33 @@
+2021-12-12 Simon Fraser <simon.fra...@apple.com>
+
+ Ensure that the scrolling thread always commits layer position changes to reduce scrolling stutters
+ https://bugs.webkit.org/show_bug.cgi?id=234213
+
+ Reviewed by Tim Horton.
+
+ On a page where CA commits on the main thread take a long time (e.g. because of expensive
+ painting), it's possible that the main thread has updated the scrolling layer position, and
+ then the scrolling thread detects that the commit is taking a long time and attempts to
+ trigger its own commit, but because the layer position property doesn't change, no commit
+ occurs.
+
+ Work around this by setting the layer position to 0,0 and back when we're on the scrolling
+ thread. Only do this if the scroll position changed since the last display refresh to avoid
+ triggering redundant commits.
+
+ Ideally we'd traverse the scrolling tree and do this for every scrolling node, but scrolling
+ trees can get large so for now just apply this to the root node.
+
+ * page/scrolling/ScrollingTreeScrollingNode.cpp:
+ (WebCore::ScrollingTreeScrollingNode::updateScrollPositionAtLastDisplayRefresh):
+ * page/scrolling/ScrollingTreeScrollingNode.h:
+ * page/scrolling/ThreadedScrollingTree.cpp:
+ (WebCore::ThreadedScrollingTree::displayDidRefreshOnScrollingThread):
+ (WebCore::ThreadedScrollingTree::storeScrollPositionsAtLastDisplayRefresh):
+ * page/scrolling/ThreadedScrollingTree.h:
+ * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+ (WebCore::ScrollingTreeFrameScrollingNodeMac::repositionScrollingLayers):
+
2021-12-12 Alan Bujtas <za...@apple.com>
[LFC][IFC] Add partial unicode-bidi support on inline level boxes
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp (286931 => 286932)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2021-12-13 01:46:09 UTC (rev 286931)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2021-12-13 02:42:35 UTC (rev 286932)
@@ -303,6 +303,11 @@
scrollingTree().scrollingTreeNodeDidScroll(*this, action);
}
+void ScrollingTreeScrollingNode::updateScrollPositionAtLastDisplayRefresh()
+{
+ m_scrollPositionAtLastDisplayRefresh = m_currentScrollPosition;
+}
+
bool ScrollingTreeScrollingNode::scrollPositionAndLayoutViewportMatch(const FloatPoint& position, std::optional<FloatRect>)
{
return position == m_currentScrollPosition;
Modified: trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h (286931 => 286932)
--- trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2021-12-13 01:46:09 UTC (rev 286931)
+++ trunk/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2021-12-13 02:42:35 UTC (rev 286932)
@@ -141,6 +141,9 @@
virtual void repositionScrollingLayers() { }
virtual void repositionRelatedLayers() { }
+
+ void updateScrollPositionAtLastDisplayRefresh();
+ std::optional<FloatPoint> scrollPositionAtLastDisplayRefresh() { return m_scrollPositionAtLastDisplayRefresh; };
void applyLayerPositions() override;
@@ -174,6 +177,7 @@
std::optional<unsigned> m_currentHorizontalSnapPointIndex;
std::optional<unsigned> m_currentVerticalSnapPointIndex;
ScrollableAreaParameters m_scrollableAreaParameters;
+ std::optional<FloatPoint> m_scrollPositionAtLastDisplayRefresh;
#if ENABLE(SCROLLING_THREAD)
OptionSet<SynchronousScrollingReason> m_synchronousScrollingReasons;
#endif
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (286931 => 286932)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-13 01:46:09 UTC (rev 286931)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-13 02:42:35 UTC (rev 286932)
@@ -500,6 +500,8 @@
case SynchronizationState::Desynchronized:
break;
}
+
+ storeScrollPositionsAtLastDisplayRefresh();
}
void ThreadedScrollingTree::displayDidRefresh(PlatformDisplayID displayID)
@@ -525,6 +527,14 @@
m_nodesWithPendingScrollAnimations.remove(nodeID);
}
+void ThreadedScrollingTree::storeScrollPositionsAtLastDisplayRefresh()
+{
+ // Ideally this would be a tree walk for every scrolling node, but scrolling trees can get big so for now just do this for the root;
+ // ScrollingTreeFrameScrollingNodeMac::repositionScrollingLayers() uses the state to know whether it should force a commit.
+ if (auto* rootNode = this->rootNode())
+ rootNode->updateScrollPositionAtLastDisplayRefresh();
+}
+
} // namespace WebCore
#endif // ENABLE(ASYNC_SCROLLING) && ENABLE(SCROLLING_THREAD)
Modified: trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h (286931 => 286932)
--- trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-13 01:46:09 UTC (rev 286931)
+++ trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-13 02:42:35 UTC (rev 286932)
@@ -107,6 +107,8 @@
void hasNodeWithAnimatedScrollChanged(bool) final;
+ void storeScrollPositionsAtLastDisplayRefresh() WTF_REQUIRES_LOCK(m_treeLock);
+
void serviceScrollAnimations(MonotonicTime) WTF_REQUIRES_LOCK(m_treeLock);
Seconds frameDuration();
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm (286931 => 286932)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2021-12-13 01:46:09 UTC (rev 286931)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2021-12-13 02:42:35 UTC (rev 286932)
@@ -35,6 +35,7 @@
#import "ScrollableArea.h"
#import "ScrollingCoordinator.h"
#import "ScrollingStateTree.h"
+#import "ScrollingThread.h"
#import "ScrollingTree.h"
#import "TileController.h"
#import "WebCoreCALayerExtras.h"
@@ -174,8 +175,19 @@
void ScrollingTreeFrameScrollingNodeMac::repositionScrollingLayers()
{
BEGIN_BLOCK_OBJC_EXCEPTIONS
+
+ auto* layer = static_cast<CALayer*>(scrolledContentsLayer());
+ if (ScrollingThread::isCurrentThread()) {
+ // If we're committing on the scrolling thread, it means that ThreadedScrollingTree is in "desynchronized" mode.
+ // The main thread may already have set the same layer position, but here we need to trigger a scrolling thread commit to
+ // ensure that the scroll happens even when the main thread commit is taking a long time. So make sure the layer property changes
+ // when there has been a scroll position change.
+ if (scrollPositionAtLastDisplayRefresh() && scrollPositionAtLastDisplayRefresh().value() != currentScrollPosition())
+ layer.position = CGPointZero;
+ }
+
// We use scroll position here because the root content layer is offset to account for scrollOrigin (see FrameView::positionForRootContentLayer).
- static_cast<CALayer*>(scrolledContentsLayer()).position = -currentScrollPosition();
+ layer.position = -currentScrollPosition();
END_BLOCK_OBJC_EXCEPTIONS
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
