- Revision
- 279564
- Author
- [email protected]
- Date
- 2021-07-05 02:21:05 -0700 (Mon, 05 Jul 2021)
Log Message
[css-scroll-snap] Triggering a layout during scroll causes jittery scrolling on Mac when dragging the scrollbar
https://bugs.webkit.org/show_bug.cgi?id=227478
Reviewed by Simon Fraser.
Source/WebCore:
When dragging the scrollbar thumb, wait to resnap after layout for a
given axis until the mouse button is up on the scrollbar. This prevents
the layout and the scrollbar from fighting to set the scroll position
which causes some pretty terrible jitter in this case.
Test: css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html
* platform/ScrollAnimationSmooth.cpp:
(WebCore::ScrollAnimationSmooth::updatePerAxisData): Fix an issue where the new position
was calculated incorrectly. This did not cause the animation to land on the wrong place,
but did cause a very janky animation progression when retriggering animations to the
same location.
* platform/ScrollAnimator.cpp:
(WebCore::ScrollAnimator::retargetRunningAnimation): Added this helper, which retargets a
a running animation to allow for a smooth transition during relayouts.
* platform/ScrollAnimator.h: Added new method definition.
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::resnapAfterLayout): Only snap after layout when the user is not
currently interacting with the scrollbar. Once the scrollbar isn't being interacted with,
snapping will occur already. When there is already some sort of animation in progress,
smoothly transition that animation to land on the new position instead of snapping there
immediately.
LayoutTests:
* css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts-expected.txt: Added.
* css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html: Added.
* platform/ios/TestExpectations: Skip this test on iOS since it uses a scrollbar thumb drag.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (279563 => 279564)
--- trunk/LayoutTests/ChangeLog 2021-07-05 08:25:09 UTC (rev 279563)
+++ trunk/LayoutTests/ChangeLog 2021-07-05 09:21:05 UTC (rev 279564)
@@ -1,3 +1,14 @@
+2021-07-05 Martin Robinson <[email protected]>
+
+ [css-scroll-snap] Triggering a layout during scroll causes jittery scrolling on Mac when dragging the scrollbar
+ https://bugs.webkit.org/show_bug.cgi?id=227478
+
+ Reviewed by Simon Fraser.
+
+ * css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts-expected.txt: Added.
+ * css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html: Added.
+ * platform/ios/TestExpectations: Skip this test on iOS since it uses a scrollbar thumb drag.
+
2021-07-04 Wenson Hsieh <[email protected]>
[iOS] Augment -_webView:didNotHandleTapAsMeaningfulClickAtPoint: to include meaningful taps
Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts-expected.txt (0 => 279564)
--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts-expected.txt 2021-07-05 09:21:05 UTC (rev 279564)
@@ -0,0 +1,10 @@
+250
+180
+PASS dragging the horizontal scrollbar thumb snapped
+PASS relayouts should not have snapped back
+PASS dragging the vertical scrollbar thumb snapped
+PASS relayouts should not have snapped back
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html (0 => 279564)
--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html 2021-07-05 09:21:05 UTC (rev 279564)
@@ -0,0 +1,126 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>Dragging the scrollbar thumb while triggering layout should trigger scroll snapping</title>
+ <style type="text/css">
+ /* Use customized scrollbar to avoid platform differences. */
+ ::-webkit-scrollbar {
+ width: 20px;
+ height: 20px;
+ }
+ ::-webkit-scrollbar-button {
+ height: 20px;
+ width: 20px;
+ background-color: blue;
+ }
+
+ ::-webkit-scrollbar-track-piece {
+ background-color: gray;
+ }
+
+ ::-webkit-scrollbar-thumb {
+ height: 20px;
+ width: 20px;
+ background-color: black;
+ }
+
+ body {
+ margin: 0px;
+ }
+
+ .container {
+ height: 200px;
+ width: 200px;
+ overflow: auto;
+ scroll-snap-type: both mandatory;
+ }
+
+ .horizontal-drawer {
+ height: 100%;
+ width: 500px;
+ }
+
+ .block {
+ height: 100%;
+ width: 250px;
+ scroll-snap-align: start;
+ }
+ </style>
+ <script src=""
+ <script src=""
+ <script>
+ window.jsTestIsAsync = true;
+
+ async function onLoad()
+ {
+ let sawZero = false;
+ let horizontalContainer = document.getElementById("horizontal-container");
+ horizontalContainer.addEventListener('scroll', () => {
+ document.getElementById('horizontal-block').innerText = horizontalContainer.scrollLeft;
+ sawZero ||= horizontalContainer.scrollLeft == 0;
+ })
+
+ let verticalContainer = document.getElementById("vertical-container");
+ verticalContainer.addEventListener('scroll', () => {
+ document.getElementById('vertical-block').innerText = verticalContainer.scrollTop;
+ sawZero ||= verticalContainer.scrollTop == 0;
+ })
+
+ if (window.eventSender == undefined) {
+ document.getElementById('console').innerText = "Dragging the scrollbar thumb should trigger scroll snapping.";
+ return;
+ }
+ try {
+ eventSender.mouseMoveTo(20, 190);
+ eventSender.mouseDown();
+
+ // Pause and move to trigger relayouts.
+ eventSender.mouseMoveTo(40, 190);
+ await UIHelper.delayFor(1);
+ eventSender.mouseMoveTo(50, 190);
+ await UIHelper.delayFor(1);
+
+ eventSender.mouseMoveTo(80, 190);
+ eventSender.mouseUp();
+
+ await UIHelper.waitForTargetScrollAnimationToSettle(horizontalContainer);
+ expectTrue(horizontalContainer.scrollLeft == 250, "dragging the horizontal scrollbar thumb snapped");
+ expectTrue(sawZero === false, "relayouts should not have snapped back");
+
+ sawZero = false;
+ eventSender.mouseMoveTo(190, 220);
+ eventSender.mouseDown();
+
+ // Pause and move to trigger relayouts.
+ eventSender.mouseMoveTo(190, 250);
+ await UIHelper.delayFor(1);
+ eventSender.mouseMoveTo(190, 255);
+ await UIHelper.delayFor(1);
+
+ eventSender.mouseMoveTo(190, 270);
+ eventSender.mouseUp();
+ await UIHelper.waitForTargetScrollAnimationToSettle(verticalContainer);
+ expectTrue(verticalContainer.scrollTop == 180, "dragging the vertical scrollbar thumb snapped");
+ expectTrue(sawZero === false, "relayouts should not have snapped back");
+ } catch (e) {
+ console.log(e);
+ } finally {
+ finishJSTest();
+ }
+ }
+ </script>
+ </head>
+ <body _onload_="onLoad();">
+ <div id="horizontal-container" class="container">
+ <div class="horizontal-drawer">
+ <div class="block" id="horizontal-block" style="float: left; background: #80475E"></div>
+ <div class="block" style="float: left; background: #CC5A71"></div>
+ </div>
+ </div>
+ <div id="vertical-container" class="container">
+ <div class="block" id="vertical-block" style="background: #80475E"></div>
+ <div class="block" style="background: #CC5A71"></div>
+ </div>
+ <p id="console"></p>
+ </body>
+</html>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (279563 => 279564)
--- trunk/LayoutTests/platform/ios/TestExpectations 2021-07-05 08:25:09 UTC (rev 279563)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2021-07-05 09:21:05 UTC (rev 279564)
@@ -2287,6 +2287,7 @@
# iOS does not allow you to scroll by dragging the scrollbar thumb.
webkit.org/b/157201 fast/scrolling/rtl-drag-vertical-scroller.html [ Failure ]
webkit.org/b/157201 css3/scroll-snap/scroll-snap-drag-scrollbar-thumb.html [ Failure ]
+webkit.org/b/157201 css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html [ Failure ]
# DumpRenderTree doesn't support logging calls to runOpenPanel and iOS's WebKitTestRunner doesn't support eventSender.mouseDown.
fast/forms/file/open-file-panel.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (279563 => 279564)
--- trunk/Source/WebCore/ChangeLog 2021-07-05 08:25:09 UTC (rev 279563)
+++ trunk/Source/WebCore/ChangeLog 2021-07-05 09:21:05 UTC (rev 279564)
@@ -1,3 +1,33 @@
+2021-07-05 Martin Robinson <[email protected]>
+
+ [css-scroll-snap] Triggering a layout during scroll causes jittery scrolling on Mac when dragging the scrollbar
+ https://bugs.webkit.org/show_bug.cgi?id=227478
+
+ Reviewed by Simon Fraser.
+
+ When dragging the scrollbar thumb, wait to resnap after layout for a
+ given axis until the mouse button is up on the scrollbar. This prevents
+ the layout and the scrollbar from fighting to set the scroll position
+ which causes some pretty terrible jitter in this case.
+
+ Test: css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html
+
+ * platform/ScrollAnimationSmooth.cpp:
+ (WebCore::ScrollAnimationSmooth::updatePerAxisData): Fix an issue where the new position
+ was calculated incorrectly. This did not cause the animation to land on the wrong place,
+ but did cause a very janky animation progression when retriggering animations to the
+ same location.
+ * platform/ScrollAnimator.cpp:
+ (WebCore::ScrollAnimator::retargetRunningAnimation): Added this helper, which retargets a
+ a running animation to allow for a smooth transition during relayouts.
+ * platform/ScrollAnimator.h: Added new method definition.
+ * platform/ScrollableArea.cpp:
+ (WebCore::ScrollableArea::resnapAfterLayout): Only snap after layout when the user is not
+ currently interacting with the scrollbar. Once the scrollbar isn't being interacted with,
+ snapping will occur already. When there is already some sort of animation in progress,
+ smoothly transition that animation to land on the new position instead of snapping there
+ immediately.
+
2021-07-04 Alexey Shvayka <[email protected]>
[WebIDL] Simplify generation of runtime conditionally read-write attributes
Modified: trunk/Source/WebCore/platform/ScrollAnimationSmooth.cpp (279563 => 279564)
--- trunk/Source/WebCore/platform/ScrollAnimationSmooth.cpp 2021-07-05 08:25:09 UTC (rev 279563)
+++ trunk/Source/WebCore/platform/ScrollAnimationSmooth.cpp 2021-07-05 09:21:05 UTC (rev 279564)
@@ -293,13 +293,13 @@
data.desiredPosition = data.currentPosition;
data.startTime = { };
}
- float newPosition = data.desiredPosition + delta;
+ float newPosition = data.currentPosition + delta;
newPosition = std::max(std::min(newPosition, maxScrollPosition), minScrollPosition);
-
if (newPosition == data.desiredPosition)
return false;
+
Seconds animationTime, repeatMinimumSustainTime, attackTime, releaseTime, maximumCoastTime;
Curve coastTimeCurve;
getAnimationParametersForGranularity(granularity, animationTime, repeatMinimumSustainTime, attackTime, releaseTime, coastTimeCurve, maximumCoastTime);
Modified: trunk/Source/WebCore/platform/ScrollAnimator.cpp (279563 => 279564)
--- trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-07-05 08:25:09 UTC (rev 279563)
+++ trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-07-05 09:21:05 UTC (rev 279564)
@@ -146,6 +146,13 @@
return true;
}
+void ScrollAnimator::retargetRunningAnimation(const FloatPoint& newPosition)
+{
+ ASSERT(scrollableArea().currentScrollBehaviorStatus() == ScrollBehaviorStatus::InNonNativeAnimation);
+ ASSERT(m_scrollAnimation->isActive());
+ m_scrollAnimation->scroll(newPosition);
+}
+
FloatPoint ScrollAnimator::offsetFromPosition(const FloatPoint& position)
{
return ScrollableArea::scrollOffsetFromPosition(position, toFloatSize(m_scrollableArea.scrollOrigin()));
Modified: trunk/Source/WebCore/platform/ScrollAnimator.h (279563 => 279564)
--- trunk/Source/WebCore/platform/ScrollAnimator.h 2021-07-05 08:25:09 UTC (rev 279563)
+++ trunk/Source/WebCore/platform/ScrollAnimator.h 2021-07-05 09:21:05 UTC (rev 279564)
@@ -72,6 +72,7 @@
bool scrollToOffsetWithoutAnimation(const FloatPoint&, ScrollClamping = ScrollClamping::Clamped);
virtual bool scrollToPositionWithoutAnimation(const FloatPoint& position, ScrollClamping = ScrollClamping::Clamped);
+ virtual void retargetRunningAnimation(const FloatPoint&);
bool scrollToOffsetWithAnimation(const FloatPoint&);
virtual bool scrollToPositionWithAnimation(const FloatPoint&);
Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (279563 => 279564)
--- trunk/Source/WebCore/platform/ScrollableArea.cpp 2021-07-05 08:25:09 UTC (rev 279563)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp 2021-07-05 09:21:05 UTC (rev 279564)
@@ -529,19 +529,28 @@
auto currentOffset = scrollOffset();
auto correctedOffset = currentOffset;
- const auto& horizontal = info->horizontalSnapOffsets;
- auto activeHorizontalIndex = currentHorizontalSnapPointIndex();
- if (activeHorizontalIndex)
- correctedOffset.setX(horizontal[*activeHorizontalIndex].offset.toInt());
- const auto& vertical = info->verticalSnapOffsets;
- auto activeVerticalIndex = currentVerticalSnapPointIndex();
- if (activeVerticalIndex)
- correctedOffset.setY(vertical[*activeVerticalIndex].offset.toInt());
+ if (!horizontalScrollbar() || horizontalScrollbar()->pressedPart() == ScrollbarPart::NoPart) {
+ const auto& horizontal = info->horizontalSnapOffsets;
+ auto activeHorizontalIndex = currentHorizontalSnapPointIndex();
+ if (activeHorizontalIndex)
+ correctedOffset.setX(horizontal[*activeHorizontalIndex].offset.toInt());
+ }
+ if (!verticalScrollbar() || verticalScrollbar()->pressedPart() == ScrollbarPart::NoPart) {
+ const auto& vertical = info->verticalSnapOffsets;
+ auto activeVerticalIndex = currentVerticalSnapPointIndex();
+ if (activeVerticalIndex)
+ correctedOffset.setY(vertical[*activeVerticalIndex].offset.toInt());
+ }
+
if (correctedOffset != currentOffset) {
LOG_WITH_STREAM(ScrollSnap, stream << " adjusting offset from " << currentOffset << " to " << correctedOffset);
- scrollToOffsetWithoutAnimation(correctedOffset);
+ auto position = scrollPositionFromOffset(correctedOffset);
+ if (currentScrollBehaviorStatus() == ScrollBehaviorStatus::NotInAnimation)
+ scrollToOffsetWithoutAnimation(correctedOffset);
+ else
+ scrollAnimator->retargetRunningAnimation(position);
}
}
