Changeset 273690 in webkit


Ignore:
Timestamp:
Mar 1, 2021 1:32:19 PM (17 months ago)
Author:
Martin Robinson
Message:

Scroll snapping doesn't kick in when dragging scrollbars
​https://bugs.webkit.org/show_bug.cgi?id=146696

Reviewed by Simon Fraser.

Source/WebCore:

This change adds support for scroll snap when the scrollbar thumb is
released from a drag operation. This animation is currently done with
the non-native scroll animator for all platforms, since that is also
what is used for smooth scrolling.

This change adjusts the non-native scroll animator's smoothness factor,
because it is too slow for short scroll snaps. I have verified that the
new smoothness factor is also a good selection for smooth scrolling
operations (still an experimental feature). A further change should use
"native" animations when possible for both smooth scrolling and scroll
snap after thumb drags. This is tracked by the following bug:
​https://bugs.webkit.org/show_bug.cgi?id=218857

Tests: css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html

css3/scroll-snap/scroll-snap-drag-scrollbar-thumb.html

  • platform/ScrollAnimationSmooth.cpp: Modify smoothFactorForProgrammaticScroll to

a value that is good for both scroll snapping and smooth scrolling.

  • platform/ScrollAnimator.cpp: The tick method for the non-native scroll animation

should continually update the current active snap point.
(WebCore::ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded): Split this into
two methods. One that handles a single axis and one that handles both.

  • platform/ScrollAnimator.h: Update method declarations.
  • platform/ScrollableArea.cpp:

(WebCore::ScrollableArea::doPostThumbDragSnapping): Added this method which does
axial snapping after a scrollbar thum drag.

  • platform/ScrollableArea.h:
  • platform/Scrollbar.cpp:

(WebCore::Scrollbar::mouseUp): Use the new doPostThumbDragSnapping method.

LayoutTests:

Add support for scroll snap after dragging scrollbar thumb

This change adds support for scroll snap when the scrollbar thumb is
released from a drag operation. This animation is currently done with
the non-native scroll animator for all platforms, since that is also
what is used for smooth scrolling.

This change adjusts the non-native scroll animator's smoothness factor,
because it is too slow for short scroll snaps. I have verified that the
new smoothness factor is also a good selection for smooth scrolling
operations (still an experimental feature). A further change should use
"native" animations when possible for both smooth scrolling and scroll
snap after thumb drags. This is tracked by the following bug:
​https://bugs.webkit.org/show_bug.cgi?id=218857

  • css3/scroll-snap/scroll-snap-click-scrollbar-gutter-expected.txt: Added.
  • css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html: Added.
  • css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-expected.txt: Added.
  • css3/scroll-snap/scroll-snap-drag-scrollbar-thumb.html: Added.
  • platform/ios-wk2/TestExpectations: Mark new test as failing on iOS

which has issues with scrollbars.

  • platform/ios/TestExpectations: Skip the new test on iOS which does not

support scrolling by dragging the scrollbar.

Location:
trunk
Files:
4 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r273687 r273690  
     12021-03-01  Martin Robinson  <mrobinson@webkit.org>
     2
     3        Scroll snapping doesn't kick in when dragging scrollbars
     4        https://bugs.webkit.org/show_bug.cgi?id=146696
     5
     6        Reviewed by Simon Fraser.
     7
     8        Add support for scroll snap after dragging scrollbar thumb
     9
     10        This change adds support for scroll snap when the scrollbar thumb is
     11        released from a drag operation. This animation is currently done with
     12        the non-native scroll animator for all platforms, since that is also
     13        what is used for smooth scrolling.
     14
     15        This change adjusts the non-native scroll animator's smoothness factor,
     16        because it is too slow for short scroll snaps. I have verified that the
     17        new smoothness factor is also a good selection for smooth scrolling
     18        operations (still an experimental feature). A further change should use
     19        "native" animations when possible for both smooth scrolling and scroll
     20        snap after thumb drags. This is tracked by the following bug:
     21        https://bugs.webkit.org/show_bug.cgi?id=218857
     22
     23        * css3/scroll-snap/scroll-snap-click-scrollbar-gutter-expected.txt: Added.
     24        * css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html: Added.
     25        * css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-expected.txt: Added.
     26        * css3/scroll-snap/scroll-snap-drag-scrollbar-thumb.html: Added.
     27        * platform/ios-wk2/TestExpectations: Mark new test as failing on iOS
     28        which has issues with scrollbars.
     29        * platform/ios/TestExpectations: Skip the new test on iOS which does not
     30        support scrolling by dragging the scrollbar.
     31
    1322021-03-01  Truitt Savell  <tsavell@apple.com>
    233

  • trunk/LayoutTests/platform/ios-wk2/TestExpectations

    r273559 r273690  
    138138css3/scroll-snap/scroll-padding-mainframe-paging.html [ Failure ]
    139139css3/scroll-snap/scroll-padding-overflow-paging.html [ Failure ]
     140css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html [ Failure ]
    140141
    141142# SVG tests that time out (these require EventSender)

  • trunk/LayoutTests/platform/ios/TestExpectations

    r273684 r273690  
    23492349# iOS does not allow you to scroll by dragging the scrollbar thumb.
    23502350webkit.org/b/157201 fast/scrolling/rtl-drag-vertical-scroller.html [ Failure ]
     2351webkit.org/b/157201 css3/scroll-snap/scroll-snap-drag-scrollbar-thumb.html [ Failure ]
    23512352
    23522353# DumpRenderTree doesn't support logging calls to runOpenPanel and iOS's WebKitTestRunner doesn't support eventSender.mouseDown.

  • trunk/Source/WebCore/ChangeLog

    r273688 r273690  
     12021-03-01  Martin Robinson  <mrobinson@webkit.org>
     2
     3        Scroll snapping doesn't kick in when dragging scrollbars
     4        https://bugs.webkit.org/show_bug.cgi?id=146696
     5
     6        Reviewed by Simon Fraser.
     7
     8        This change adds support for scroll snap when the scrollbar thumb is
     9        released from a drag operation. This animation is currently done with
     10        the non-native scroll animator for all platforms, since that is also
     11        what is used for smooth scrolling.
     12
     13        This change adjusts the non-native scroll animator's smoothness factor,
     14        because it is too slow for short scroll snaps. I have verified that the
     15        new smoothness factor is also a good selection for smooth scrolling
     16        operations (still an experimental feature). A further change should use
     17        "native" animations when possible for both smooth scrolling and scroll
     18        snap after thumb drags. This is tracked by the following bug:
     19        https://bugs.webkit.org/show_bug.cgi?id=218857
     20
     21        Tests: css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html
     22               css3/scroll-snap/scroll-snap-drag-scrollbar-thumb.html
     23
     24        * platform/ScrollAnimationSmooth.cpp: Modify smoothFactorForProgrammaticScroll to
     25        a value that is good for both scroll snapping and smooth scrolling.
     26        * platform/ScrollAnimator.cpp: The tick method for the non-native scroll animation
     27        should continually update the current active snap point.
     28        (WebCore::ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded): Split this into
     29        two methods. One that handles a single axis and one that handles both.
     30        * platform/ScrollAnimator.h: Update method declarations.
     31        * platform/ScrollableArea.cpp:
     32        (WebCore::ScrollableArea::doPostThumbDragSnapping): Added this method which does
     33        axial snapping after a scrollbar thum drag.
     34        * platform/ScrollableArea.h:
     35        * platform/Scrollbar.cpp:
     36        (WebCore::Scrollbar::mouseUp): Use the new doPostThumbDragSnapping method.
     37
    1382021-03-01  Sam Weinig  <weinig@apple.com>
    239

  • trunk/Source/WebCore/platform/ScrollAnimationSmooth.cpp

    r268522 r273690  
    4141static const Seconds tickTime = 1_s / frameRate;
    4242static const Seconds minimumTimerInterval { 1_ms };
    43 static const double smoothFactorForProgrammaticScroll = 5;
     43static const double smoothFactorForProgrammaticScroll = 1;
    4444
    4545ScrollAnimationSmooth::PerAxisData::PerAxisData(ScrollbarOrientation orientation, const FloatPoint& position, ScrollExtentsCallback& extentsCallback)

  • trunk/Source/WebCore/platform/ScrollAnimator.cpp

    r273275 r273690  
    6767            m_currentPosition = WTFMove(position);
    6868            notifyPositionChanged(delta);
     69            updateActiveScrollSnapIndexForOffset();
    6970        },
    7071        [this] {
    … …  
    363364        return offset;
    364365
    365     Optional<float> originalXOffset, originalYOffset;
    366     FloatSize velocity;
    367     if (method == ScrollSnapPointSelectionMethod::Directional) {
    368         FloatSize scrollOrigin = toFloatSize(m_scrollableArea.scrollOrigin());
    369         auto currentOffset = ScrollableArea::scrollOffsetFromPosition(this->currentPosition(), scrollOrigin);
    370         originalXOffset = currentOffset.x();
    371         originalYOffset = currentOffset.y();
    372         velocity = { offset.x() - currentOffset.x(), offset.y() - currentOffset.y() };
    373     }
    374 
    375366    FloatPoint newOffset = offset;
    376     newOffset.setX(m_scrollController.adjustScrollDestination(ScrollEventAxis::Horizontal, newOffset.x(), velocity.width(), originalXOffset));
    377     newOffset.setY(m_scrollController.adjustScrollDestination(ScrollEventAxis::Vertical, newOffset.y(), velocity.height(), originalYOffset));
     367    newOffset.setX(adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Horizontal, newOffset.x(), method));
     368    newOffset.setY(adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Vertical, newOffset.y(), method));
    378369    return newOffset;
    379370#else
    … …  
    383374}
    384375
     376float ScrollAnimator::adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis axis, float newOffset, ScrollSnapPointSelectionMethod method)
     377{
     378#if ENABLE(CSS_SCROLL_SNAP)
     379    if (!m_scrollController.usesScrollSnap())
     380        return newOffset;
     381
     382    Optional<float> originalOffset;
     383    float velocity = 0.;
     384    if (method == ScrollSnapPointSelectionMethod::Directional) {
     385        FloatSize scrollOrigin = toFloatSize(m_scrollableArea.scrollOrigin());
     386        auto currentOffset = ScrollableArea::scrollOffsetFromPosition(this->currentPosition(), scrollOrigin);
     387        originalOffset = axis == ScrollEventAxis::Horizontal ? currentOffset.x() : currentOffset.y();
     388        velocity = newOffset - (axis == ScrollEventAxis::Horizontal ? currentOffset.x() : currentOffset.y());
     389    }
     390
     391    return m_scrollController.adjustScrollDestination(axis, newOffset, velocity, originalOffset);
     392#else
     393    UNUSED_PARAM(method);
     394    UNUSED_PARAM(axis);
     395    return newOffset;
     396#endif
     397}
     398
    385399
    386400} // namespace WebCore

  • trunk/Source/WebCore/platform/ScrollAnimator.h

    r273275 r273690  
    143143
    144144    FloatPoint adjustScrollOffsetForSnappingIfNeeded(const FloatPoint& offset, ScrollSnapPointSelectionMethod);
     145    float adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis, float newOffset, ScrollSnapPointSelectionMethod);
    145146
    146147#if ENABLE(CSS_SCROLL_SNAP) || ENABLE(RUBBER_BANDING)

  • trunk/Source/WebCore/platform/ScrollableArea.cpp

    r273275 r273690  
    558558    }
    559559}
     560
     561void ScrollableArea::doPostThumbMoveSnapping(ScrollbarOrientation orientation)
     562{
     563    if (!usesScrollSnap())
     564        return;
     565
     566    auto currentOffset = scrollOffset();
     567    auto newOffset = currentOffset;
     568    if (orientation == HorizontalScrollbar)
     569        newOffset.setX(m_scrollAnimator->adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Horizontal, currentOffset.x(), ScrollSnapPointSelectionMethod::Closest));
     570    else
     571        newOffset.setY(m_scrollAnimator->adjustScrollOffsetForSnappingIfNeeded(ScrollEventAxis::Vertical, currentOffset.y(), ScrollSnapPointSelectionMethod::Closest));
     572    if (newOffset == currentOffset)
     573        return;
     574
     575    scrollAnimator().scrollToOffsetWithAnimation(newOffset);
     576}
    560577#else
    561578void ScrollableArea::updateScrollSnapState()
     579{
     580}
     581
     582void ScrollableArea::doPostThumbMoveSnapping(ScrollbarOrientation)
    562583{
    563584}

  • trunk/Source/WebCore/platform/ScrollableArea.h

    r273275 r273690  
    104104
    105105    void updateScrollSnapState();
     106    void doPostThumbMoveSnapping(ScrollbarOrientation);
    106107
    107108#if ENABLE(TOUCH_EVENTS)

  • trunk/Source/WebCore/platform/Scrollbar.cpp

    r259761 r273690  
    357357bool Scrollbar::mouseUp(const PlatformMouseEvent& mouseEvent)
    358358{
     359    auto previouslyPressedPart = m_pressedPart;
    359360    setPressedPart(NoPart);
    360361    m_pressedPos = 0;
    … …  
    369370    if (part == NoPart)
    370371        m_scrollableArea.mouseExitedScrollbar(this);
     372
     373    if (previouslyPressedPart == ThumbPart)
     374        m_scrollableArea.doPostThumbMoveSnapping(m_orientation);
    371375
    372376    return true;
Note: See TracChangeset for help on using the changeset viewer.