Skip to content
Permalink
Browse files
[GTK][WPE] CSS scroll snapping interacts badly with smooth scrolling
https://bugs.webkit.org/show_bug.cgi?id=229037

Reviewed by Simon Fraser.

Source/WebCore:

Make CSS scroll snapping behave in a predictable way when scrolling
with touchpads/screens on GTK/WPE.

Test: fast/scrolling/gtk/user-scroll-snapping-interaction.html

* platform/ScrollAnimator.h:
(WebCore::ScrollAnimator::isUserScrollInProgress const):
(WebCore::ScrollAnimator::isScrollSnapInProgress const):
* platform/ScrollingEffectsController.cpp:
(WebCore::ScrollingEffectsController::processWheelEventForKineticScrolling):
(WebCore::ScrollingEffectsController::adjustDeltaForSnappingIfNeeded):
(WebCore::ScrollingEffectsController::handleWheelEvent):
(WebCore::ScrollingEffectsController::isUserScrollInProgress const):
(WebCore::ScrollingEffectsController::isScrollSnapInProgress const):
* platform/ScrollingEffectsController.h:
* platform/mac/ScrollAnimatorMac.h:
* platform/mac/ScrollAnimatorMac.mm:
(WebCore::ScrollAnimatorMac::isUserScrollInProgress const): Deleted.
(WebCore::ScrollAnimatorMac::isScrollSnapInProgress const): Deleted.

LayoutTests:

Add a GTK-only test to verify CSS scroll snapping and kinetic/touchpad
scrolling behave correctly.

* TestExpectations:
* fast/scrolling/gtk/user-scroll-snapping-interaction-expected.txt: Added.
* fast/scrolling/gtk/user-scroll-snapping-interaction.html: Added.
* platform/gtk/TestExpectations:


Canonical link: https://commits.webkit.org/242578@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@283626 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Chris Lord committed Oct 6, 2021
1 parent 25821a9 commit fa64d812062887f686435b76ae0a562cc3f5a04e
@@ -1,3 +1,18 @@
2021-10-06 Chris Lord <clord@igalia.com>

[GTK][WPE] CSS scroll snapping interacts badly with smooth scrolling
https://bugs.webkit.org/show_bug.cgi?id=229037

Reviewed by Simon Fraser.

Add a GTK-only test to verify CSS scroll snapping and kinetic/touchpad
scrolling behave correctly.

* TestExpectations:
* fast/scrolling/gtk/user-scroll-snapping-interaction-expected.txt: Added.
* fast/scrolling/gtk/user-scroll-snapping-interaction.html: Added.
* platform/gtk/TestExpectations:

2021-10-06 Ayumi Kojima <ayumi_kojima@apple.com>

[ iOS 15 ] 2 http/tests/app-privacy-report tests are timing out.
@@ -59,6 +59,7 @@ fast/events/watchos [ Skip ]
fast/events/pointer/ios [ Skip ]
fast/events/touch/ios [ Skip ]
fast/history/ios [ Skip ]
fast/scrolling/gtk [ Skip ]
fast/scrolling/ios [ Skip ]
fast/scrolling/mac [ Skip ]
fast/scrolling/ipad [ Skip ]
@@ -0,0 +1,9 @@
PASS Scroll downwards by 25 pixels
PASS Scroll downwards by another 25 pixels
PASS End scroll snaps back to top
PASS Scroll downwards by 200 pixels
PASS End scroll should snap to next element
PASS successfullyParsed is true

TEST COMPLETE

@@ -0,0 +1,120 @@
<!-- webkit-test-runner [ useThreadedScrolling=false ] -->
<!DOCTYPE html>

<html>
<head>
<script src="../../../resources/js-test-pre.js"></script>
<title>CSS scroll snapping should not conflict with user scrolling</title>
<style>
html {
scroll-snap-type: y mandatory;
}

body {
margin: 0;
}

div {
width: 100%;
height: 250px;
scroll-snap-align: start;
}
</style>
</head>
<body>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
</body>
<script>
var expectedScrollTop = 0;
var testsPassed = 0;
var testDescription;

function startScroll(startx, starty, deltaX, deltaY)
{
eventSender.mouseMoveTo(startx, starty);
eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "began", "none");
eventSender.mouseScrollByWithWheelAndMomentumPhases(-deltaX, -deltaY, "changed", "none");
}

function continueScroll(deltaX, deltaY)
{
eventSender.mouseScrollByWithWheelAndMomentumPhases(-deltaX, -deltaY, "changed", "none");
}

function endScroll(deltaX, deltaY)
{
eventSender.mouseScrollByWithWheelAndMomentumPhases(-deltaX, -deltaY, "ended", "none");
}

function triggerNextTest() {
if (testsPassed)
debug("PASS " + testDescription);

switch (testsPassed) {
case 0:
testDescription = "Scroll downwards by 25 pixels"
expectedScrollTop = 25;
startScroll(50, 50, 0, 25);
return;

case 1:
testDescription = "Scroll downwards by another 25 pixels"
expectedScrollTop = 50;
continueScroll(0, 25);
return;

case 2:
testDescription = "End scroll snaps back to top"
expectedScrollTop = 0;
endScroll(0, 0);
return;

case 3:
testDescription = "Scroll downwards by 200 pixels"
expectedScrollTop = 200;
startScroll(50, 50, 0, 200);
return;

case 4:
testDescription = "End scroll should snap to next element"
expectedScrollTop = 250;
endScroll(0, 0);
return;
}

isSuccessfullyParsed();
testRunner.notifyDone();

return;
}

function scrollEventCallback() {
if (document.scrollingElement.scrollTop == expectedScrollTop) {
++testsPassed;
if (window.testRunner)
triggerNextTest();
}
}


document.addEventListener("scroll", scrollEventCallback, false);

if (window.testRunner) {
testRunner.waitUntilDone();
}

if (window.eventSender) {
eventSender.setWheelHasPreciseDeltas(true);
triggerNextTest();
}
</script>
</html>
@@ -19,6 +19,7 @@
# Platform-specific directories.
accessibility/gtk [ Pass ]
editing/pasteboard/gtk [ Pass ]
fast/scrolling/gtk [ Pass ]
swipe [ Pass ]

ietestcenter/css3/flexbox/flexbox-layout-001.htm [ Pass ]
@@ -1,3 +1,30 @@
2021-10-06 Chris Lord <clord@igalia.com>

[GTK][WPE] CSS scroll snapping interacts badly with smooth scrolling
https://bugs.webkit.org/show_bug.cgi?id=229037

Reviewed by Simon Fraser.

Make CSS scroll snapping behave in a predictable way when scrolling
with touchpads/screens on GTK/WPE.

Test: fast/scrolling/gtk/user-scroll-snapping-interaction.html

* platform/ScrollAnimator.h:
(WebCore::ScrollAnimator::isUserScrollInProgress const):
(WebCore::ScrollAnimator::isScrollSnapInProgress const):
* platform/ScrollingEffectsController.cpp:
(WebCore::ScrollingEffectsController::processWheelEventForKineticScrolling):
(WebCore::ScrollingEffectsController::adjustDeltaForSnappingIfNeeded):
(WebCore::ScrollingEffectsController::handleWheelEvent):
(WebCore::ScrollingEffectsController::isUserScrollInProgress const):
(WebCore::ScrollingEffectsController::isScrollSnapInProgress const):
* platform/ScrollingEffectsController.h:
* platform/mac/ScrollAnimatorMac.h:
* platform/mac/ScrollAnimatorMac.mm:
(WebCore::ScrollAnimatorMac::isUserScrollInProgress const): Deleted.
(WebCore::ScrollAnimatorMac::isScrollSnapInProgress const): Deleted.

2021-10-06 Antti Koivisto <antti@apple.com>

[LFC] Layout box should own its children
@@ -93,9 +93,10 @@ class ScrollAnimator : private ScrollingEffectsControllerClient {

virtual void cancelAnimations();

virtual bool isUserScrollInProgress() const { return false; }
virtual bool isRubberBandInProgress() const { return false; }
virtual bool isScrollSnapInProgress() const { return false; }

bool isUserScrollInProgress() const { return m_scrollController.isUserScrollInProgress(); }
bool isScrollSnapInProgress() const { return m_scrollController.isScrollSnapInProgress(); }

void contentsSizeChanged();

@@ -240,11 +240,16 @@ bool ScrollingEffectsController::processWheelEventForKineticScrolling(const Plat
if (!event.isEndOfNonMomentumScroll() && !event.isTransitioningToMomentumScroll())
return false;

m_inScrollGesture = false;

if (m_currentAnimation && !is<ScrollAnimationKinetic>(m_currentAnimation.get())) {
m_currentAnimation->stop();
m_currentAnimation = nullptr;
}

if (usesScrollSnap())
return false;

if (!m_currentAnimation)
m_currentAnimation = makeUnique<ScrollAnimationKinetic>(*this);

@@ -267,9 +272,29 @@ bool ScrollingEffectsController::processWheelEventForKineticScrolling(const Plat
}
#endif

void ScrollingEffectsController::adjustDeltaForSnappingIfNeeded(float& deltaX, float& deltaY)
{
if (snapOffsetsInfo() && !snapOffsetsInfo()->isEmpty()) {
float scale = m_client.pageScaleFactor();
auto scrollOffset = m_client.scrollOffset();
auto extents = m_client.scrollExtents();

auto originalOffset = LayoutPoint(scrollOffset.x() / scale, scrollOffset.y() / scale);
auto newOffset = LayoutPoint((scrollOffset.x() + deltaX) / scale, (scrollOffset.y() + deltaY) / scale);

auto offsetX = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Horizontal, LayoutSize(extents.contentsSize), newOffset, deltaX, originalOffset.x()).first;
auto offsetY = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Vertical, LayoutSize(extents.contentsSize), newOffset, deltaY, originalOffset.y()).first;

deltaX = (offsetX - originalOffset.x()) * scale;
deltaY = (offsetY - originalOffset.y()) * scale;
}
}

bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
{
#if ENABLE(KINETIC_SCROLLING)
m_inScrollGesture = wheelEvent.hasPreciseScrollingDeltas() && !wheelEvent.isEndOfNonMomentumScroll() && !wheelEvent.isTransitioningToMomentumScroll();

if (processWheelEventForKineticScrolling(wheelEvent))
return true;
#endif
@@ -288,9 +313,6 @@ bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& whee
|| (deltaY > 0 && scrollOffset.y() <= minPosition.y()))
deltaY = 0;

if (!deltaX && !deltaY)
return false;

if (wheelEvent.granularity() == ScrollByPageWheelEvent) {
if (deltaX) {
bool negative = deltaX < 0;
@@ -309,20 +331,14 @@ bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& whee
deltaX = -deltaX;
deltaY = -deltaY;

if (snapOffsetsInfo() && !snapOffsetsInfo()->isEmpty()) {
float scale = m_client.pageScaleFactor();
auto originalOffset = LayoutPoint(scrollOffset.x() / scale, scrollOffset.y() / scale);
auto newOffset = LayoutPoint((scrollOffset.x() + deltaX) / scale, (scrollOffset.y() + deltaY) / scale);

auto offsetX = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Horizontal, LayoutSize(extents.contentsSize), newOffset, deltaX, originalOffset.x()).first;
auto offsetY = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Vertical, LayoutSize(extents.contentsSize), newOffset, deltaY, originalOffset.y()).first;
if (!m_inScrollGesture)
adjustDeltaForSnappingIfNeeded(deltaX, deltaY);

deltaX = (offsetX - originalOffset.x()) * scale;
deltaY = (offsetY - originalOffset.y()) * scale;
}
if (!deltaX && !deltaY)
return false;

#if ENABLE(SMOOTH_SCROLLING)
if (m_client.scrollAnimationEnabled() && !wheelEvent.hasPreciseScrollingDeltas()) {
if (m_client.scrollAnimationEnabled() && !m_inScrollGesture) {
if (is<ScrollAnimationSmooth>(m_currentAnimation.get())) {
auto lastDestinationOffset = downcast<ScrollAnimationSmooth>(*m_currentAnimation).destinationOffset();
retargetAnimatedScroll(lastDestinationOffset + FloatSize { deltaX, deltaY });
@@ -336,6 +352,22 @@ bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& whee

return true;
}

bool ScrollingEffectsController::isUserScrollInProgress() const
{
return m_inScrollGesture;
}

bool ScrollingEffectsController::isScrollSnapInProgress() const
{
if (!usesScrollSnap())
return false;

if (m_inScrollGesture || (m_currentAnimation && m_currentAnimation->isActive()))
return true;

return false;
}
#endif

void ScrollingEffectsController::updateActiveScrollSnapIndexForClientOffset()
@@ -163,12 +163,12 @@ class ScrollingEffectsController : public ScrollAnimationClient {
// Returns true if handled.
bool handleWheelEvent(const PlatformWheelEvent&);

#if PLATFORM(MAC)
static FloatSize wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent&);

bool isScrollSnapInProgress() const;
bool isUserScrollInProgress() const;

#if PLATFORM(MAC)
static FloatSize wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent&);

// Returns true if handled.
bool processWheelEventForScrollSnap(const PlatformWheelEvent&);

@@ -216,6 +216,8 @@ class ScrollingEffectsController : public ScrollAnimationClient {
void scrollAnimationDidEnd(ScrollAnimation&) final;
ScrollExtents scrollExtentsForAnimation(ScrollAnimation&) final;

void adjustDeltaForSnappingIfNeeded(float& deltaX, float& deltaY);

#if ENABLE(KINETIC_SCROLLING) && !PLATFORM(MAC)
// Returns true if handled.
bool processWheelEventForKineticScrolling(const PlatformWheelEvent&);
@@ -234,14 +236,14 @@ class ScrollingEffectsController : public ScrollAnimationClient {
bool m_isAnimatingRubberBand { false };
bool m_isAnimatingScrollSnap { false };
bool m_isAnimatingKeyboardScrolling { false };
bool m_inScrollGesture { false };

#if PLATFORM(MAC)
WallTime m_lastMomentumScrollTimestamp;
FloatSize m_unappliedOverscrollDelta;
FloatSize m_stretchScrollForce;
FloatSize m_momentumVelocity;

bool m_inScrollGesture { false };
bool m_momentumScrollInProgress { false };
bool m_ignoreMomentumScrolls { false };
bool m_isRubberBanding { false };
@@ -49,10 +49,8 @@ class ScrollAnimatorMac final : public ScrollAnimator {
bool platformAllowsScrollAnimation() const;

void handleWheelEventPhase(PlatformWheelEventPhase) final;

bool isUserScrollInProgress() const final;

bool isRubberBandInProgress() const final;
bool isScrollSnapInProgress() const final;

bool processWheelEventForScrollSnap(const PlatformWheelEvent&) final;

@@ -73,21 +73,11 @@ static bool rubberBandingEnabledForSystem()
return enabled;
}

bool ScrollAnimatorMac::isUserScrollInProgress() const
{
return m_scrollController.isUserScrollInProgress();
}

bool ScrollAnimatorMac::isRubberBandInProgress() const
{
return m_scrollController.isRubberBandInProgress();
}

bool ScrollAnimatorMac::isScrollSnapInProgress() const
{
return m_scrollController.isScrollSnapInProgress();
}

void ScrollAnimatorMac::handleWheelEventPhase(PlatformWheelEventPhase phase)
{
LOG_WITH_STREAM(OverlayScrollbars, stream << "ScrollAnimatorMac " << this << " scrollableArea " << m_scrollableArea << " handleWheelEventPhase " << phase);

0 comments on commit fa64d81

Please sign in to comment.