Title: [286975] branches/safari-612-branch/Source/WebCore
- Revision
- 286975
- Author
- alanc...@apple.com
- Date
- 2021-12-13 12:56:57 -0800 (Mon, 13 Dec 2021)
Log Message
Cherry-pick r286932. rdar://problem/86385697
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):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286932 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/WebCore/ChangeLog (286974 => 286975)
--- branches/safari-612-branch/Source/WebCore/ChangeLog 2021-12-13 20:56:53 UTC (rev 286974)
+++ branches/safari-612-branch/Source/WebCore/ChangeLog 2021-12-13 20:56:57 UTC (rev 286975)
@@ -1,5 +1,69 @@
2021-12-13 Alan Coon <alanc...@apple.com>
+ Cherry-pick r286932. rdar://problem/86385697
+
+ 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):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286932 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-13 Alan Coon <alanc...@apple.com>
+
Cherry-pick r286858. rdar://problem/86423741
Use simpler idioms for std::less and std::greater possible in modern C++
Modified: branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp (286974 => 286975)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2021-12-13 20:56:53 UTC (rev 286974)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp 2021-12-13 20:56:57 UTC (rev 286975)
@@ -273,6 +273,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: branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h (286974 => 286975)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2021-12-13 20:56:53 UTC (rev 286974)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h 2021-12-13 20:56:57 UTC (rev 286975)
@@ -128,6 +128,9 @@
virtual void repositionScrollingLayers() { }
virtual void repositionRelatedLayers() { }
+
+ void updateScrollPositionAtLastDisplayRefresh();
+ std::optional<FloatPoint> scrollPositionAtLastDisplayRefresh() { return m_scrollPositionAtLastDisplayRefresh; };
void applyLayerPositions() override;
@@ -161,6 +164,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: branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp (286974 => 286975)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-13 20:56:53 UTC (rev 286974)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp 2021-12-13 20:56:57 UTC (rev 286975)
@@ -406,6 +406,8 @@
case SynchronizationState::Desynchronized:
break;
}
+
+ storeScrollPositionsAtLastDisplayRefresh();
}
void ThreadedScrollingTree::displayDidRefresh(PlatformDisplayID displayID)
@@ -428,6 +430,14 @@
});
}
+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: branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.h (286974 => 286975)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-13 20:56:53 UTC (rev 286974)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/ThreadedScrollingTree.h 2021-12-13 20:56:57 UTC (rev 286975)
@@ -94,6 +94,8 @@
void scheduleDelayedRenderingUpdateDetectionTimer(Seconds) WTF_REQUIRES_LOCK(m_treeLock);
void delayedRenderingUpdateDetectionTimerFired();
+ void storeScrollPositionsAtLastDisplayRefresh() WTF_REQUIRES_LOCK(m_treeLock);
+
Seconds frameDuration();
Seconds maxAllowableRenderingUpdateDurationForSynchronization();
Modified: branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm (286974 => 286975)
--- branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2021-12-13 20:56:53 UTC (rev 286974)
+++ branches/safari-612-branch/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm 2021-12-13 20:56:57 UTC (rev 286975)
@@ -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"
@@ -166,8 +167,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
