Modified: trunk/Source/WebCore/ChangeLog (276238 => 276239)
--- trunk/Source/WebCore/ChangeLog 2021-04-19 08:16:42 UTC (rev 276238)
+++ trunk/Source/WebCore/ChangeLog 2021-04-19 08:23:34 UTC (rev 276239)
@@ -1,3 +1,32 @@
+2021-04-19 Martin Robinson <[email protected]>
+
+ [css-scroll-snap] Properly support fractional scroll steps in WebCore::ScrollAnimator::scroll
+ https://bugs.webkit.org/show_bug.cgi?id=224176
+
+ Reviewed by Simon Fraser.
+
+ Stop using the ScrollableArea's position to calculate scroll snap positions in ScrollAnimator::snap.
+ The position stored in ScrollAnimator is a floating point position, while the one stored in ScrollableArea
+ is an integer position. This currently isn't an issue, because all callers of ScrollAnimator::scroll
+ use integer scroll offsets, but this will allow this function to be used in the future for precise
+ scrolling delta.
+
+ No new tests. This doesn't change any behavior, since all callers currently use
+ integer scroll offsets, but a future change will make use of this fix.
+
+ * platform/ScrollAnimator.cpp:
+ (WebCore::ScrollAnimator::scroll): Get the current scroll position from the ScrollAnimator,
+ which stores it in floating point. Also, only call into the scroll snap code if we actually
+ have scroll offsets.
+ (WebCore::ScrollAnimator::offsetFromPosition): Added this helper.
+ (WebCore::ScrollAnimator::positionFromOffset): Ditto.
+ (WebCore::ScrollAnimator::deltaFromStep): Return a delta instead of a position so this helper
+ can be used with offsets or positions.
+ (WebCore::ScrollAnimator::positionFromStep): Deleted.
+ * platform/ScrollAnimator.h:
+ * platform/mac/ScrollAnimatorMac.mm:
+ (WebCore::ScrollAnimatorMac::scroll): Use the new helper.j
+
2021-04-19 Philippe Normand <[email protected]>
[WPE][GTK] Enable AVIF decoder as experimental feature and unskip tests
Modified: trunk/Source/WebCore/platform/ScrollAnimator.cpp (276238 => 276239)
--- trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-04-19 08:16:42 UTC (rev 276238)
+++ trunk/Source/WebCore/platform/ScrollAnimator.cpp 2021-04-19 08:23:34 UTC (rev 276239)
@@ -83,16 +83,23 @@
bool ScrollAnimator::scroll(ScrollbarOrientation orientation, ScrollGranularity granularity, float step, float multiplier, ScrollBehavior behavior)
{
+ auto delta = deltaFromStep(orientation, step, multiplier);
#if ENABLE(CSS_SCROLL_SNAP)
if (behavior == ScrollBehavior::DoDirectionalSnapping) {
- auto newOffset = ScrollableArea::scrollOffsetFromPosition(positionFromStep(orientation, step, multiplier), toFloatSize(m_scrollableArea.scrollOrigin()));
- auto currentOffset = m_scrollableArea.scrollOffset();
- if (orientation == HorizontalScrollbar) {
+ if (!m_scrollController.usesScrollSnap())
+ return scroll(orientation, granularity, step, multiplier);
+
+ auto currentOffset = offsetFromPosition(currentPosition());
+ auto newOffset = currentOffset + delta;
+ if (orientation == HorizontalScrollbar)
newOffset.setX(m_scrollController.adjustScrollDestination(ScrollEventAxis::Horizontal, newOffset.x(), multiplier, currentOffset.x()));
- return scroll(HorizontalScrollbar, granularity, newOffset.x() - currentOffset.x(), 1.0);
- }
- newOffset.setY(m_scrollController.adjustScrollDestination(ScrollEventAxis::Vertical, newOffset.y(), multiplier, currentOffset.y()));
- return scroll(VerticalScrollbar, granularity, newOffset.y() - currentOffset.y(), 1.0);
+ else
+ newOffset.setY(m_scrollController.adjustScrollDestination(ScrollEventAxis::Vertical, newOffset.y(), multiplier, currentOffset.y()));
+ auto newDelta = newOffset - currentOffset;
+
+ if (orientation == HorizontalScrollbar)
+ return scroll(HorizontalScrollbar, granularity, newDelta.width(), 1.0);
+ return scroll(VerticalScrollbar, granularity, newDelta.height(), 1.0);
}
#else
UNUSED_PARAM(granularity);
@@ -99,7 +106,7 @@
UNUSED_PARAM(behavior);
#endif
- return scrollToPositionWithoutAnimation(positionFromStep(orientation, step, multiplier));
+ return scrollToPositionWithoutAnimation(currentPosition() + delta);
}
bool ScrollAnimator::scrollToOffsetWithoutAnimation(const FloatPoint& offset, ScrollClamping clamping)
@@ -140,14 +147,24 @@
return true;
}
-FloatPoint ScrollAnimator::positionFromStep(ScrollbarOrientation orientation, float step, float multiplier)
+FloatPoint ScrollAnimator::offsetFromPosition(const FloatPoint& position)
{
+ return ScrollableArea::scrollOffsetFromPosition(position, toFloatSize(m_scrollableArea.scrollOrigin()));
+}
+
+FloatPoint ScrollAnimator::positionFromOffset(const FloatPoint& offset)
+{
+ return ScrollableArea::scrollPositionFromOffset(offset, toFloatSize(m_scrollableArea.scrollOrigin()));
+}
+
+FloatSize ScrollAnimator::deltaFromStep(ScrollbarOrientation orientation, float step, float multiplier)
+{
FloatSize delta;
if (orientation == HorizontalScrollbar)
delta.setWidth(step * multiplier);
else
delta.setHeight(step * multiplier);
- return this->currentPosition() + delta;
+ return delta;
}
#if ENABLE(CSS_SCROLL_SNAP)
@@ -212,7 +229,7 @@
deltaY = -deltaY;
}
if (e.hasPreciseScrollingDeltas())
- scrollToPositionWithoutAnimation(positionFromStep(VerticalScrollbar, verticalScrollbar->pixelStep(), -deltaY));
+ scrollToPositionWithoutAnimation(currentPosition() + deltaFromStep(VerticalScrollbar, verticalScrollbar->pixelStep(), -deltaY));
else
scroll(VerticalScrollbar, granularity, verticalScrollbar->pixelStep(), -deltaY);
}
@@ -225,7 +242,7 @@
deltaX = -deltaX;
}
if (e.hasPreciseScrollingDeltas())
- scrollToPositionWithoutAnimation(positionFromStep(HorizontalScrollbar, horizontalScrollbar->pixelStep(), -deltaX));
+ scrollToPositionWithoutAnimation(currentPosition() + deltaFromStep(HorizontalScrollbar, horizontalScrollbar->pixelStep(), -deltaX));
else
scroll(HorizontalScrollbar, granularity, horizontalScrollbar->pixelStep(), -deltaX);
}
Modified: trunk/Source/WebCore/platform/ScrollAnimator.h (276238 => 276239)
--- trunk/Source/WebCore/platform/ScrollAnimator.h 2021-04-19 08:16:42 UTC (rev 276238)
+++ trunk/Source/WebCore/platform/ScrollAnimator.h 2021-04-19 08:23:34 UTC (rev 276239)
@@ -173,8 +173,11 @@
protected:
virtual void notifyPositionChanged(const FloatSize& delta);
void updateActiveScrollSnapIndexForOffset();
- FloatPoint positionFromStep(ScrollbarOrientation, float step, float multiplier);
+ FloatPoint offsetFromPosition(const FloatPoint& position);
+ FloatPoint positionFromOffset(const FloatPoint& offset);
+ FloatSize deltaFromStep(ScrollbarOrientation, float step, float multiplier);
+
ScrollableArea& m_scrollableArea;
RefPtr<WheelEventTestMonitor> m_wheelEventTestMonitor;
#if ENABLE(CSS_SCROLL_SNAP) || ENABLE(RUBBER_BANDING)
Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (276238 => 276239)
--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2021-04-19 08:16:42 UTC (rev 276238)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2021-04-19 08:23:34 UTC (rev 276239)
@@ -761,7 +761,7 @@
return ScrollAnimator::scroll(orientation, granularity, step, multiplier, behavior);
bool shouldAnimate = scrollAnimationEnabledForSystem() && m_scrollableArea.scrollAnimatorEnabled() && granularity != ScrollByPixel;
- FloatPoint newPosition = positionFromStep(orientation, step, multiplier);
+ FloatPoint newPosition = this->currentPosition() + deltaFromStep(orientation, step, multiplier);
newPosition = newPosition.constrainedBetween(scrollableArea().minimumScrollPosition(), scrollableArea().maximumScrollPosition());
LOG_WITH_STREAM(Scrolling, stream << "ScrollAnimatorMac::scroll from " << currentPosition() << " to " << newPosition);
