Title: [282741] trunk/Source/WebCore
Revision
282741
Author
simon.fra...@apple.com
Date
2021-09-19 18:15:37 -0700 (Sun, 19 Sep 2021)

Log Message

Have ScrollAnimation work in terms of offsets, not positions
https://bugs.webkit.org/show_bug.cgi?id=230450

Reviewed by Antti Koivisto.

Scroll positions can have negative values in RTL content. It's simpler for ScrollAnimation
to work on zero-based scroll offsets. ScrollAnimator handles the mapping from these to
ScrollPositions.

Also share a bit of setCurrentPosition() code. This required adding an early return in
ScrollableArea::setScrollPositionFromAnimation() to avoid re-entrant calls during scrolling
tree commit, which would cause deadlocks on the scrolling tree lock, which happened when a
ScrollAnimation set a fractional offset.

* platform/KeyboardScrollingAnimator.cpp:
(WebCore::KeyboardScrollingAnimator::scrollableDirectionsFromPosition const): Clarify offset vs. position.
(WebCore::KeyboardScrollingAnimator::updateKeyboardScrollPosition):
(WebCore::KeyboardScrollingAnimator::scrollableDirectionsFromOffset const): Deleted.
* platform/KeyboardScrollingAnimator.h:
* platform/ScrollAnimation.h:
* platform/ScrollAnimationKinetic.cpp:
(WebCore::ScrollAnimationKinetic::PerAxisData::PerAxisData):
(WebCore::ScrollAnimationKinetic::PerAxisData::animateScroll):
(WebCore::ScrollAnimationKinetic::startAnimatedScrollWithInitialVelocity):
(WebCore::ScrollAnimationKinetic::animationTimerFired):
* platform/ScrollAnimationKinetic.h:
* platform/ScrollAnimationSmooth.cpp:
(WebCore::ScrollAnimationSmooth::startAnimatedScroll):
(WebCore::ScrollAnimationSmooth::startAnimatedScrollToDestination):
(WebCore::ScrollAnimationSmooth::retargetActiveAnimation):
(WebCore::ScrollAnimationSmooth::startOrRetargetAnimation):
(WebCore::ScrollAnimationSmooth::updateScrollExtents):
(WebCore::ScrollAnimationSmooth::animateScroll):
(WebCore::ScrollAnimationSmooth::animationTimerFired):
* platform/ScrollAnimationSmooth.h:
* platform/ScrollAnimator.cpp:
(WebCore::ScrollAnimator::scroll):
(WebCore::ScrollAnimator::scrollToPositionWithoutAnimation):
(WebCore::ScrollAnimator::scrollToPositionWithAnimation):
(WebCore::ScrollAnimator::offsetFromPosition const):
(WebCore::ScrollAnimator::positionFromOffset const):
(WebCore::ScrollAnimator::setCurrentPosition):
(WebCore::ScrollAnimator::notifyPositionChanged):
(WebCore::ScrollAnimator::scrollAnimationDidUpdate):
(WebCore::ScrollAnimator::scrollExtentsForAnimation):
(WebCore::ScrollAnimator::offsetFromPosition): Deleted.
(WebCore::ScrollAnimator::positionFromOffset): Deleted.
* platform/ScrollAnimator.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::setScrollOffset):
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::setScrollPositionFromAnimation):
* platform/ScrollableArea.h:
(WebCore::ScrollableArea::minimumScrollOffset const): minimumScrollOffset is always 0,0 but add this function for symmetry.
* platform/mac/ScrollAnimatorMac.mm:
(WebCore::ScrollAnimatorMac::immediateScrollBy):
* rendering/RenderLayerScrollableArea.cpp:
(WebCore::RenderLayerScrollableArea::clampScrollOffset const):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (282740 => 282741)


--- trunk/Source/WebCore/ChangeLog	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/ChangeLog	2021-09-20 01:15:37 UTC (rev 282741)
@@ -1,3 +1,64 @@
+2021-09-19  Simon Fraser  <simon.fra...@apple.com>
+
+        Have ScrollAnimation work in terms of offsets, not positions
+        https://bugs.webkit.org/show_bug.cgi?id=230450
+
+        Reviewed by Antti Koivisto.
+        
+        Scroll positions can have negative values in RTL content. It's simpler for ScrollAnimation
+        to work on zero-based scroll offsets. ScrollAnimator handles the mapping from these to
+        ScrollPositions.
+
+        Also share a bit of setCurrentPosition() code. This required adding an early return in
+        ScrollableArea::setScrollPositionFromAnimation() to avoid re-entrant calls during scrolling
+        tree commit, which would cause deadlocks on the scrolling tree lock, which happened when a
+        ScrollAnimation set a fractional offset.
+
+        * platform/KeyboardScrollingAnimator.cpp:
+        (WebCore::KeyboardScrollingAnimator::scrollableDirectionsFromPosition const): Clarify offset vs. position.
+        (WebCore::KeyboardScrollingAnimator::updateKeyboardScrollPosition):
+        (WebCore::KeyboardScrollingAnimator::scrollableDirectionsFromOffset const): Deleted.
+        * platform/KeyboardScrollingAnimator.h:
+        * platform/ScrollAnimation.h:
+        * platform/ScrollAnimationKinetic.cpp:
+        (WebCore::ScrollAnimationKinetic::PerAxisData::PerAxisData):
+        (WebCore::ScrollAnimationKinetic::PerAxisData::animateScroll):
+        (WebCore::ScrollAnimationKinetic::startAnimatedScrollWithInitialVelocity):
+        (WebCore::ScrollAnimationKinetic::animationTimerFired):
+        * platform/ScrollAnimationKinetic.h:
+        * platform/ScrollAnimationSmooth.cpp:
+        (WebCore::ScrollAnimationSmooth::startAnimatedScroll):
+        (WebCore::ScrollAnimationSmooth::startAnimatedScrollToDestination):
+        (WebCore::ScrollAnimationSmooth::retargetActiveAnimation):
+        (WebCore::ScrollAnimationSmooth::startOrRetargetAnimation):
+        (WebCore::ScrollAnimationSmooth::updateScrollExtents):
+        (WebCore::ScrollAnimationSmooth::animateScroll):
+        (WebCore::ScrollAnimationSmooth::animationTimerFired):
+        * platform/ScrollAnimationSmooth.h:
+        * platform/ScrollAnimator.cpp:
+        (WebCore::ScrollAnimator::scroll):
+        (WebCore::ScrollAnimator::scrollToPositionWithoutAnimation):
+        (WebCore::ScrollAnimator::scrollToPositionWithAnimation):
+        (WebCore::ScrollAnimator::offsetFromPosition const):
+        (WebCore::ScrollAnimator::positionFromOffset const):
+        (WebCore::ScrollAnimator::setCurrentPosition):
+        (WebCore::ScrollAnimator::notifyPositionChanged):
+        (WebCore::ScrollAnimator::scrollAnimationDidUpdate):
+        (WebCore::ScrollAnimator::scrollExtentsForAnimation):
+        (WebCore::ScrollAnimator::offsetFromPosition): Deleted.
+        (WebCore::ScrollAnimator::positionFromOffset): Deleted.
+        * platform/ScrollAnimator.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::setScrollOffset):
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::setScrollPositionFromAnimation):
+        * platform/ScrollableArea.h:
+        (WebCore::ScrollableArea::minimumScrollOffset const): minimumScrollOffset is always 0,0 but add this function for symmetry.
+        * platform/mac/ScrollAnimatorMac.mm:
+        (WebCore::ScrollAnimatorMac::immediateScrollBy):
+        * rendering/RenderLayerScrollableArea.cpp:
+        (WebCore::RenderLayerScrollableArea::clampScrollOffset const):
+
 2021-09-19  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Teach `WebKit::createShareableBitmap` to snapshot video elements

Modified: trunk/Source/WebCore/platform/KeyboardScrollingAnimator.cpp (282740 => 282741)


--- trunk/Source/WebCore/platform/KeyboardScrollingAnimator.cpp	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/KeyboardScrollingAnimator.cpp	2021-09-20 01:15:37 UTC (rev 282741)
@@ -39,7 +39,7 @@
 {
 }
 
-RectEdges<bool> KeyboardScrollingAnimator::scrollableDirectionsFromOffset(FloatPoint offset) const
+RectEdges<bool> KeyboardScrollingAnimator::scrollableDirectionsFromPosition(FloatPoint position) const
 {
     auto minimumScrollPosition = m_scrollAnimator.scrollableArea().minimumScrollPosition();
     auto maximumScrollPosition = m_scrollAnimator.scrollableArea().maximumScrollPosition();
@@ -46,10 +46,10 @@
 
     RectEdges<bool> edges;
 
-    edges.setTop(offset.y() > minimumScrollPosition.y());
-    edges.setBottom(offset.y() < maximumScrollPosition.y());
-    edges.setLeft(offset.x() > minimumScrollPosition.x());
-    edges.setRight(offset.x() < maximumScrollPosition.x());
+    edges.setTop(position.y() > minimumScrollPosition.y());
+    edges.setBottom(position.y() < maximumScrollPosition.y());
+    edges.setLeft(position.x() > minimumScrollPosition.x());
+    edges.setRight(position.x() < maximumScrollPosition.x());
 
     return edges;
 }
@@ -91,7 +91,7 @@
     KeyboardScrollParameters params = KeyboardScrollParameters::parameters();
 
     if (m_currentKeyboardScroll) {
-        auto scrollableDirections = scrollableDirectionsFromOffset(m_scrollAnimator.currentPosition());
+        auto scrollableDirections = scrollableDirectionsFromPosition(m_scrollAnimator.currentPosition());
         auto direction = m_currentKeyboardScroll->direction;
 
         if (scrollableDirections.at(boxSideForDirection(direction))) {

Modified: trunk/Source/WebCore/platform/KeyboardScrollingAnimator.h (282740 => 282741)


--- trunk/Source/WebCore/platform/KeyboardScrollingAnimator.h	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/KeyboardScrollingAnimator.h	2021-09-20 01:15:37 UTC (rev 282741)
@@ -25,8 +25,8 @@
 
 #pragma once
 
-#include "KeyboardEvent.h"
-#include "KeyboardScroll.h"
+#include "KeyboardEvent.h" // FIXME: This is a layering violation.
+#include "KeyboardScroll.h" // FIXME: This is a layering violation.
 #include "RectEdges.h"
 #include "ScrollAnimator.h"
 
@@ -44,7 +44,7 @@
 
 private:
     void stopKeyboardScrollAnimation();
-    RectEdges<bool> scrollableDirectionsFromOffset(FloatPoint) const;
+    RectEdges<bool> scrollableDirectionsFromPosition(FloatPoint) const;
     std::optional<KeyboardScroll> keyboardScrollForKeyboardEvent(KeyboardEvent&) const;
     float scrollDistance(ScrollDirection, ScrollGranularity) const;
 

Modified: trunk/Source/WebCore/platform/ScrollAnimation.h (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollAnimation.h	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollAnimation.h	2021-09-20 01:15:37 UTC (rev 282741)
@@ -35,8 +35,8 @@
 class ScrollAnimation;
 
 struct ScrollExtents {
-    ScrollPosition minimumScrollPosition;
-    ScrollPosition maximumScrollPosition;
+    ScrollOffset minimumScrollOffset;
+    ScrollOffset maximumScrollOffset;
     IntSize visibleSize;
 };
 
@@ -44,7 +44,7 @@
 public:
     virtual ~ScrollAnimationClient() = default;
 
-    virtual void scrollAnimationDidUpdate(ScrollAnimation&, const FloatPoint& currentPosition) = 0;
+    virtual void scrollAnimationDidUpdate(ScrollAnimation&, const FloatPoint& currentOffset) = 0;
     virtual void scrollAnimationDidEnd(ScrollAnimation&) = 0;
     virtual ScrollExtents scrollExtentsForAnimation(ScrollAnimation&) = 0;
 };
@@ -57,7 +57,7 @@
     { }
     virtual ~ScrollAnimation() = default;
 
-    virtual bool retargetActiveAnimation(const FloatPoint& newDestination) = 0;
+    virtual bool retargetActiveAnimation(const FloatPoint& newDestinationOffset) = 0;
     virtual void stop() = 0;
     virtual bool isActive() const = 0;
     virtual void updateScrollExtents() { };

Modified: trunk/Source/WebCore/platform/ScrollAnimationKinetic.cpp (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollAnimationKinetic.cpp	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollAnimationKinetic.cpp	2021-09-20 01:15:37 UTC (rev 282741)
@@ -77,36 +77,36 @@
 
 namespace WebCore {
 
-ScrollAnimationKinetic::PerAxisData::PerAxisData(double lower, double upper, double initialPosition, double initialVelocity)
+ScrollAnimationKinetic::PerAxisData::PerAxisData(double lower, double upper, double initialOffset, double initialVelocity)
     : m_lower(lower)
     , m_upper(upper)
-    , m_coef1(initialVelocity / decelFriction + initialPosition)
+    , m_coef1(initialVelocity / decelFriction + initialOffset)
     , m_coef2(-initialVelocity / decelFriction)
-    , m_position(clampTo(initialPosition, lower, upper))
-    , m_velocity(initialPosition < lower || initialPosition > upper ? 0 : initialVelocity)
+    , m_offset(clampTo(initialOffset, lower, upper))
+    , m_velocity(initialOffset < lower || initialOffset > upper ? 0 : initialVelocity)
 {
 }
 
 bool ScrollAnimationKinetic::PerAxisData::animateScroll(Seconds timeDelta)
 {
-    auto lastPosition = m_position;
+    auto lastOffset = m_offset;
     auto lastTime = m_elapsedTime;
     m_elapsedTime += timeDelta;
 
     double exponentialPart = exp(-decelFriction * m_elapsedTime.value());
-    m_position = m_coef1 + m_coef2 * exponentialPart;
+    m_offset = m_coef1 + m_coef2 * exponentialPart;
     m_velocity = -decelFriction * m_coef2 * exponentialPart;
 
-    if (m_position < m_lower) {
-        m_velocity = m_lower - m_position;
-        m_position = m_lower;
-    } else if (m_position > m_upper) {
-        m_velocity = m_upper - m_position;
-        m_position = m_upper;
+    if (m_offset < m_lower) {
+        m_velocity = m_lower - m_offset;
+        m_offset = m_lower;
+    } else if (m_offset > m_upper) {
+        m_velocity = m_upper - m_offset;
+        m_offset = m_upper;
     }
 
-    if (fabs(m_velocity) < 1 || (lastTime && fabs(m_position - lastPosition) < 1)) {
-        m_position = round(m_position);
+    if (fabs(m_velocity) < 1 || (lastTime && fabs(m_offset - lastOffset) < 1)) {
+        m_offset = round(m_offset);
         m_velocity = 0;
     }
 
@@ -169,12 +169,12 @@
     return FloatPoint(accumDelta.x() * -1 / (last - first).value(), accumDelta.y() * -1 / (last - first).value());
 }
 
-bool ScrollAnimationKinetic::startAnimatedScrollWithInitialVelocity(const FloatPoint& initialPosition, const FloatPoint& velocity, bool mayHScroll, bool mayVScroll)
+bool ScrollAnimationKinetic::startAnimatedScrollWithInitialVelocity(const FloatPoint& initialOffset, const FloatPoint& velocity, bool mayHScroll, bool mayVScroll)
 {
     stop();
 
     if (!velocity.x() && !velocity.y()) {
-        m_position = initialPosition;
+        m_offset = initialOffset;
         m_horizontalData = std::nullopt;
         m_verticalData = std::nullopt;
         return false;
@@ -199,20 +199,20 @@
     };
 
     if (mayHScroll) {
-        m_horizontalData = PerAxisData(extents.minimumScrollPosition.x(),
-            extents.maximumScrollPosition.x(),
-            initialPosition.x(), accumulateVelocity(velocity.x(), m_horizontalData));
+        m_horizontalData = PerAxisData(extents.minimumScrollOffset.x(),
+            extents.maximumScrollOffset.x(),
+            initialOffset.x(), accumulateVelocity(velocity.x(), m_horizontalData));
     } else
         m_horizontalData = std::nullopt;
 
     if (mayVScroll) {
-        m_verticalData = PerAxisData(extents.minimumScrollPosition.y(),
-            extents.maximumScrollPosition.y(),
-            initialPosition.y(), accumulateVelocity(velocity.y(), m_verticalData));
+        m_verticalData = PerAxisData(extents.minimumScrollOffset.y(),
+            extents.maximumScrollOffset.y(),
+            initialOffset.y(), accumulateVelocity(velocity.y(), m_verticalData));
     } else
         m_verticalData = std::nullopt;
 
-    m_position = initialPosition;
+    m_offset = initialOffset;
     m_startTime = MonotonicTime::now() - tickTime / 2.;
     animationTimerFired();
     return true;
@@ -241,10 +241,10 @@
     if (m_horizontalData || m_verticalData)
         m_animationTimer.startOneShot(std::max(minimumTimerInterval, delta));
 
-    double x = m_horizontalData ? m_horizontalData.value().position() : m_position.x();
-    double y = m_verticalData ? m_verticalData.value().position() : m_position.y();
-    m_position = FloatPoint(x, y);
-    m_client.scrollAnimationDidUpdate(*this, FloatPoint(m_position));
+    double x = m_horizontalData ? m_horizontalData.value().offset() : m_offset.x();
+    double y = m_verticalData ? m_verticalData.value().offset() : m_offset.y();
+    m_offset = FloatPoint(x, y);
+    m_client.scrollAnimationDidUpdate(*this, m_offset);
 }
 
 Seconds ScrollAnimationKinetic::deltaToNextFrame()

Modified: trunk/Source/WebCore/platform/ScrollAnimationKinetic.h (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollAnimationKinetic.h	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollAnimationKinetic.h	2021-09-20 01:15:37 UTC (rev 282741)
@@ -38,9 +38,9 @@
 private:
     class PerAxisData {
     public:
-        PerAxisData(double lower, double upper, double initialPosition, double initialVelocity);
+        PerAxisData(double lower, double upper, double initialOffset, double initialVelocity);
 
-        double position() { return m_position; }
+        double offset() { return m_offset; }
         double velocity() { return m_velocity; }
 
         bool animateScroll(Seconds timeDelta);
@@ -53,7 +53,7 @@
         double m_coef2 { 0 };
 
         Seconds m_elapsedTime;
-        double m_position { 0 };
+        double m_offset { 0 };
         double m_velocity { 0 };
     };
 
@@ -61,9 +61,9 @@
     ScrollAnimationKinetic(ScrollAnimationClient&);
     virtual ~ScrollAnimationKinetic();
 
-    bool startAnimatedScrollWithInitialVelocity(const FloatPoint& initialPosition, const FloatPoint& velocity, bool mayHScroll, bool mayVScroll);
+    bool startAnimatedScrollWithInitialVelocity(const FloatPoint& initialOffset, const FloatPoint& velocity, bool mayHScroll, bool mayVScroll);
 
-    bool retargetActiveAnimation(const FloatPoint& newDestination) final;
+    bool retargetActiveAnimation(const FloatPoint& newOffset) final;
     void stop() final;
     bool isActive() const final;
 
@@ -81,7 +81,7 @@
 
     MonotonicTime m_startTime;
     RunLoop::Timer<ScrollAnimationKinetic> m_animationTimer;
-    FloatPoint m_position;
+    FloatPoint m_offset;
     Vector<PlatformWheelEvent> m_scrollHistory;
 };
 

Modified: trunk/Source/WebCore/platform/ScrollAnimationSmooth.cpp (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollAnimationSmooth.cpp	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollAnimationSmooth.cpp	2021-09-20 01:15:37 UTC (rev 282741)
@@ -54,47 +54,47 @@
 
 ScrollAnimationSmooth::~ScrollAnimationSmooth() = default;
 
-bool ScrollAnimationSmooth::startAnimatedScroll(ScrollbarOrientation orientation, ScrollGranularity, const FloatPoint& fromPosition, float step, float multiplier)
+bool ScrollAnimationSmooth::startAnimatedScroll(ScrollbarOrientation orientation, ScrollGranularity, const FloatPoint& fromOffset, float step, float multiplier)
 {
-    m_startPosition = fromPosition;
-    auto destinationPosition = fromPosition;
+    m_startOffset = fromOffset;
+    auto destinationOffset = fromOffset;
     switch (orientation) {
     case HorizontalScrollbar:
-        destinationPosition.setX(destinationPosition.x() + step * multiplier);
+        destinationOffset.setX(destinationOffset.x() + step * multiplier);
         break;
     case VerticalScrollbar:
-        destinationPosition.setY(destinationPosition.y() + step * multiplier);
+        destinationOffset.setY(destinationOffset.y() + step * multiplier);
         break;
     }
 
-    m_duration = durationFromDistance(destinationPosition - m_startPosition);
+    m_duration = durationFromDistance(destinationOffset - m_startOffset);
 
     auto extents = m_client.scrollExtentsForAnimation(*this);
-    return startOrRetargetAnimation(extents, destinationPosition);
+    return startOrRetargetAnimation(extents, destinationOffset);
 }
 
-bool ScrollAnimationSmooth::startAnimatedScrollToDestination(const FloatPoint& fromPosition, const FloatPoint& destinationPosition)
+bool ScrollAnimationSmooth::startAnimatedScrollToDestination(const FloatPoint& fromOffset, const FloatPoint& destinationOffset)
 {
-    m_startPosition = fromPosition;
-    m_duration = durationFromDistance(destinationPosition - m_startPosition);
+    m_startOffset = fromOffset;
+    m_duration = durationFromDistance(destinationOffset - m_startOffset);
 
     auto extents = m_client.scrollExtentsForAnimation(*this);
-    return startOrRetargetAnimation(extents, destinationPosition);
+    return startOrRetargetAnimation(extents, destinationOffset);
 }
 
-bool ScrollAnimationSmooth::retargetActiveAnimation(const FloatPoint& newDestination)
+bool ScrollAnimationSmooth::retargetActiveAnimation(const FloatPoint& newOffset)
 {
     if (!isActive())
         return false;
 
     auto extents = m_client.scrollExtentsForAnimation(*this);
-    return startOrRetargetAnimation(extents, newDestination);
+    return startOrRetargetAnimation(extents, newOffset);
 }
 
-bool ScrollAnimationSmooth::startOrRetargetAnimation(const ScrollExtents& extents, const FloatPoint& destinationPosition)
+bool ScrollAnimationSmooth::startOrRetargetAnimation(const ScrollExtents& extents, const FloatPoint& destinationOffset)
 {
-    m_destinationPosition = destinationPosition.constrainedBetween(extents.minimumScrollPosition, extents.maximumScrollPosition);
-    bool needToScroll = m_startPosition != m_destinationPosition;
+    m_destinationOffset = destinationOffset.constrainedBetween(extents.minimumScrollOffset, extents.maximumScrollOffset);
+    bool needToScroll = m_startOffset != m_destinationOffset;
 
     if (needToScroll && !isActive()) {
         m_startTime = MonotonicTime::now();
@@ -112,8 +112,8 @@
 void ScrollAnimationSmooth::updateScrollExtents()
 {
     auto extents = m_client.scrollExtentsForAnimation(*this);
-    // FIXME: Ideally fix up m_startPosition so m_currentPosition doesn't go backwards.
-    m_destinationPosition = m_destinationPosition.constrainedBetween(extents.minimumScrollPosition, extents.maximumScrollPosition);
+    // FIXME: Ideally fix up m_startOffset so m_currentOffset doesn't go backwards.
+    m_destinationOffset = m_destinationOffset.constrainedBetween(extents.minimumScrollOffset, extents.maximumScrollOffset);
 }
 
 Seconds ScrollAnimationSmooth::durationFromDistance(const FloatSize& delta) const
@@ -135,9 +135,9 @@
     double fractionComplete = (currentTime - m_startTime) / m_duration;
     double progress = m_easeInOutTimingFunction->transformTime(fractionComplete, m_duration.value());
 
-    m_currentPosition = {
-        linearInterpolation(progress, m_startPosition.x(), m_destinationPosition.x()),
-        linearInterpolation(progress, m_startPosition.y(), m_destinationPosition.y()),
+    m_currentOffset = {
+        linearInterpolation(progress, m_startOffset.x(), m_destinationOffset.x()),
+        linearInterpolation(progress, m_startOffset.y(), m_destinationOffset.y()),
     };
 
     return currentTime < endTime;
@@ -153,7 +153,7 @@
     if (continueAnimation)
         startNextTimer(std::max(minimumTimerInterval, deltaToNextFrame));
 
-    m_client.scrollAnimationDidUpdate(*this, m_currentPosition);
+    m_client.scrollAnimationDidUpdate(*this, m_currentOffset);
     if (!continueAnimation)
         m_client.scrollAnimationDidEnd(*this);
 }

Modified: trunk/Source/WebCore/platform/ScrollAnimationSmooth.h (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollAnimationSmooth.h	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollAnimationSmooth.h	2021-09-20 01:15:37 UTC (rev 282741)
@@ -40,10 +40,10 @@
     ScrollAnimationSmooth(ScrollAnimationClient&);
     virtual ~ScrollAnimationSmooth();
 
-    bool startAnimatedScroll(ScrollbarOrientation, ScrollGranularity, const FloatPoint& fromPosition, float step, float multiplier);
-    bool startAnimatedScrollToDestination(const FloatPoint& fromPosition, const FloatPoint& destinationPosition);
+    bool startAnimatedScroll(ScrollbarOrientation, ScrollGranularity, const FloatPoint& fromOffset, float step, float multiplier);
+    bool startAnimatedScrollToDestination(const FloatPoint& fromOffset, const FloatPoint& destinationOffset);
 
-    bool retargetActiveAnimation(const FloatPoint& newDestination) final;
+    bool retargetActiveAnimation(const FloatPoint& newOffset) final;
     void stop() final;
     void updateScrollExtents() final;
     bool isActive() const final;
@@ -50,7 +50,7 @@
 
 
 private:
-    bool startOrRetargetAnimation(const ScrollExtents&, const FloatPoint& destination);
+    bool startOrRetargetAnimation(const ScrollExtents&, const FloatPoint& destinationOffset);
 
     void requestAnimationTimerFired();
     void startNextTimer(Seconds delay);
@@ -63,9 +63,9 @@
     MonotonicTime m_startTime;
     Seconds m_duration;
 
-    FloatPoint m_startPosition;
-    FloatPoint m_destinationPosition;
-    FloatPoint m_currentPosition;
+    FloatPoint m_startOffset;
+    FloatPoint m_destinationOffset;
+    FloatPoint m_currentOffset;
     
     // FIXME: Should not have timer here, and instead use serviceAnimation().
     RunLoop::Timer<ScrollAnimationSmooth> m_animationTimer;

Modified: trunk/Source/WebCore/platform/ScrollAnimator.cpp (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollAnimator.cpp	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollAnimator.cpp	2021-09-20 01:15:37 UTC (rev 282741)
@@ -88,7 +88,7 @@
 
 #if ENABLE(SMOOTH_SCROLLING) && !PLATFORM(IOS_FAMILY)
     if (m_scrollableArea.scrollAnimatorEnabled() && platformAllowsScrollAnimation() && !behavior.contains(ScrollBehavior::NeverAnimate))
-        return m_scrollAnimation->startAnimatedScroll(orientation, granularity, m_currentPosition, step, multiplier);
+        return m_scrollAnimation->startAnimatedScroll(orientation, granularity, offsetFromPosition(m_currentPosition), step, multiplier);
 #endif
 
     return scrollToPositionWithoutAnimation(currentPosition() + delta);
@@ -106,9 +106,7 @@
 
     m_scrollAnimation->stop();
 
-    m_currentPosition = adjustedPosition;
-    notifyPositionChanged(adjustedPosition - currentPosition);
-    updateActiveScrollSnapIndexForOffset();
+    setCurrentPosition(adjustedPosition, NotifyScrollableArea::Yes);
     return true;
 }
 
@@ -118,7 +116,7 @@
     if (!positionChanged && !scrollableArea().scrollOriginChanged())
         return false;
 
-    m_scrollAnimation->startAnimatedScrollToDestination(m_currentPosition, newPosition);
+    m_scrollAnimation->startAnimatedScrollToDestination(offsetFromPosition(m_currentPosition), offsetFromPosition(newPosition));
     scrollableArea().setScrollBehaviorStatus(ScrollBehaviorStatus::InNonNativeAnimation);
     return true;
 }
@@ -130,12 +128,12 @@
     m_scrollAnimation->retargetActiveAnimation(newPosition);
 }
 
-FloatPoint ScrollAnimator::offsetFromPosition(const FloatPoint& position)
+FloatPoint ScrollAnimator::offsetFromPosition(const FloatPoint& position) const
 {
     return ScrollableArea::scrollOffsetFromPosition(position, toFloatSize(m_scrollableArea.scrollOrigin()));
 }
 
-FloatPoint ScrollAnimator::positionFromOffset(const FloatPoint& offset)
+FloatPoint ScrollAnimator::positionFromOffset(const FloatPoint& offset) const
 {
     return ScrollableArea::scrollPositionFromOffset(offset, toFloatSize(m_scrollableArea.scrollOrigin()));
 }
@@ -237,9 +235,16 @@
 }
 #endif
 
-void ScrollAnimator::setCurrentPosition(const FloatPoint& position)
+void ScrollAnimator::setCurrentPosition(const FloatPoint& position, NotifyScrollableArea notify)
 {
+    // FIXME: An early return here if the position is not changing triggers test failures because of adjustForIOSCaretWhenScrolling()
+    // code in RenderLayerScrollableArea. We can early return when webkit.org/b/230454 is fixed.
+    auto delta = position - m_currentPosition;
     m_currentPosition = position;
+    
+    if (notify == NotifyScrollableArea::Yes)
+        notifyPositionChanged(delta);
+    
     updateActiveScrollSnapIndexForOffset();
 }
 
@@ -251,7 +256,8 @@
 void ScrollAnimator::notifyPositionChanged(const FloatSize& delta)
 {
     UNUSED_PARAM(delta);
-    m_scrollableArea.setScrollPositionFromAnimation(roundedIntPoint(currentPosition()));
+    
+    m_scrollableArea.setScrollPositionFromAnimation(roundedIntPoint(m_currentPosition));
     m_scrollController.scrollPositionChanged();
 }
 
@@ -270,6 +276,7 @@
     return m_scrollableArea.scrollOffsetFromPosition(roundedIntPoint(currentPosition()));
 }
 
+// FIXME: Unused.
 void ScrollAnimator::immediateScrollOnAxis(ScrollEventAxis axis, float delta)
 {
     FloatSize deltaSize;
@@ -381,12 +388,9 @@
     return m_scrollController.adjustScrollDestination(axis, newOffset, velocityInScrollAxis, originalOffset);
 }
 
-void ScrollAnimator::scrollAnimationDidUpdate(ScrollAnimation&, const FloatPoint& position)
+void ScrollAnimator::scrollAnimationDidUpdate(ScrollAnimation&, const FloatPoint& currentOffset)
 {
-    FloatSize delta = position - m_currentPosition;
-    m_currentPosition = position;
-    notifyPositionChanged(delta);
-    updateActiveScrollSnapIndexForOffset();
+    setCurrentPosition(positionFromOffset(currentOffset), NotifyScrollableArea::Yes);
 }
 
 void ScrollAnimator::scrollAnimationDidEnd(ScrollAnimation&)
@@ -397,8 +401,8 @@
 ScrollExtents ScrollAnimator::scrollExtentsForAnimation(ScrollAnimation&)
 {
     return {
-        m_scrollableArea.minimumScrollPosition(),
-        m_scrollableArea.maximumScrollPosition(),
+        m_scrollableArea.minimumScrollOffset(),
+        m_scrollableArea.maximumScrollOffset(),
         m_scrollableArea.visibleSize()
     };
 }

Modified: trunk/Source/WebCore/platform/ScrollAnimator.h (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollAnimator.h	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollAnimator.h	2021-09-20 01:15:37 UTC (rev 282741)
@@ -97,13 +97,16 @@
     virtual bool isScrollSnapInProgress() const { return false; }
 
     // ScrollAnimationClient
-    void scrollAnimationDidUpdate(ScrollAnimation&, const FloatPoint& currentPosition) override;
+    void scrollAnimationDidUpdate(ScrollAnimation&, const FloatPoint& currentOffset) override;
     void scrollAnimationDidEnd(ScrollAnimation&) override;
     ScrollExtents scrollExtentsForAnimation(ScrollAnimation&) override;
 
     void contentsSizeChanged() const;
 
-    void setCurrentPosition(const FloatPoint&);
+    enum NotifyScrollableArea : bool {
+        No, Yes
+    };
+    void setCurrentPosition(const FloatPoint&, NotifyScrollableArea = NotifyScrollableArea::No);
     const FloatPoint& currentPosition() const { return m_currentPosition; }
 
     KeyboardScrollingAnimator *keyboardScrollingAnimator() const override { return m_keyboardScrollingAnimator.get(); }
@@ -145,10 +148,11 @@
 
     void updateActiveScrollSnapIndexForOffset();
 
-    FloatPoint offsetFromPosition(const FloatPoint& position);
-    FloatPoint positionFromOffset(const FloatPoint& offset);
-    FloatSize deltaFromStep(ScrollbarOrientation, float step, float multiplier);
+    FloatPoint offsetFromPosition(const FloatPoint& position) const;
+    FloatPoint positionFromOffset(const FloatPoint& offset) const;
 
+    static FloatSize deltaFromStep(ScrollbarOrientation, float step, float multiplier);
+
     ScrollableArea& m_scrollableArea;
     RefPtr<WheelEventTestMonitor> m_wheelEventTestMonitor;
     ScrollingEffectsController m_scrollController;

Modified: trunk/Source/WebCore/platform/ScrollView.cpp (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollView.cpp	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollView.cpp	2021-09-20 01:15:37 UTC (rev 282741)
@@ -438,7 +438,7 @@
 
     IntPoint constrainedOffset = offset;
     if (constrainsScrollingToContentEdge())
-        constrainedOffset = constrainedOffset.constrainedBetween(IntPoint(), maximumScrollOffset());
+        constrainedOffset = constrainedOffset.constrainedBetween(minimumScrollOffset(), maximumScrollOffset());
 
     scrollTo(scrollPositionFromOffset(constrainedOffset));
 }

Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollableArea.cpp	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp	2021-09-20 01:15:37 UTC (rev 282741)
@@ -240,6 +240,7 @@
 
 void ScrollableArea::setScrollPositionFromAnimation(const ScrollPosition& position)
 {
+    // An early return here if the position hasn't changed breaks an iOS RTL scrolling test: webkit.org/b/230450.
     auto scrollType = currentScrollType();
     auto clamping = scrollType == ScrollType::User ? ScrollClamping::Unclamped : ScrollClamping::Clamped;
     if (requestScrollPositionUpdate(position, scrollType, clamping))

Modified: trunk/Source/WebCore/platform/ScrollableArea.h (282740 => 282741)


--- trunk/Source/WebCore/platform/ScrollableArea.h	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/ScrollableArea.h	2021-09-20 01:15:37 UTC (rev 282741)
@@ -227,6 +227,7 @@
 
     WEBCORE_EXPORT ScrollOffset scrollOffset() const;
 
+    ScrollOffset minimumScrollOffset() const { return { }; }
     ScrollOffset maximumScrollOffset() const;
 
     WEBCORE_EXPORT ScrollPosition scrollPositionFromOffset(ScrollOffset) const;

Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (282740 => 282741)


--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2021-09-20 01:15:37 UTC (rev 282741)
@@ -286,10 +286,7 @@
     if (newPosition == currentPosition)
         return;
 
-    FloatSize adjustedDelta = newPosition - currentPosition;
-    m_currentPosition = newPosition;
-    notifyPositionChanged(adjustedDelta);
-    updateActiveScrollSnapIndexForOffset();
+    setCurrentPosition(newPosition, NotifyScrollableArea::Yes);
 }
 #endif
 

Modified: trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp (282740 => 282741)


--- trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp	2021-09-20 00:10:37 UTC (rev 282740)
+++ trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp	2021-09-20 01:15:37 UTC (rev 282741)
@@ -240,7 +240,7 @@
 
 ScrollOffset RenderLayerScrollableArea::clampScrollOffset(const ScrollOffset& scrollOffset) const
 {
-    return scrollOffset.constrainedBetween(IntPoint(), maximumScrollOffset());
+    return scrollOffset.constrainedBetween(minimumScrollOffset(), maximumScrollOffset());
 }
 
 bool RenderLayerScrollableArea::requestScrollPositionUpdate(const ScrollPosition& position, ScrollType scrollType, ScrollClamping clamping)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to