Changeset 172659 in webkit

Timestamp:

Aug 15, 2014 4:35:21 PM (10 years ago)

Author:

commit-queue@webkit.org

Message:

Fix gliding in AxisScrollSnapAnimator for Mac
​https://bugs.webkit.org/show_bug.cgi?id=135971

Patch by Wenson Hsieh <Wenson Hsieh> on 2014-08-15
Reviewed by Dean Jackson.

Previously, momentum events that are handled immediately after finishing a glide animation cause a second, extremely
slow glide animation to trigger. To fix this, I added a new state DestinationReached. During this state, additional
momentum events that are handled after the glide animation completes will not fill up the momentum window and trigger
a second glide event.

• platform/mac/AxisScrollSnapAnimator.h:
• platform/mac/AxisScrollSnapAnimator.mm:

(WebCore::AxisScrollSnapAnimator::AxisScrollSnapAnimator):
(WebCore::AxisScrollSnapAnimator::handleWheelEvent):
(WebCore::AxisScrollSnapAnimator::scrollSnapAnimationUpdate):
(WebCore::AxisScrollSnapAnimator::endScrollSnapAnimation):
(WebCore::AxisScrollSnapAnimator::computeGlideDelta):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/mac/AxisScrollSnapAnimator.h (modified) (2 diffs)
• platform/mac/AxisScrollSnapAnimator.mm (modified) (7 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-08-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Fix gliding in AxisScrollSnapAnimator for Mac
+        https://bugs.webkit.org/show_bug.cgi?id=135971
+
+        Reviewed by Dean Jackson.
+
+        Previously, momentum events that are handled immediately after finishing a glide animation cause a second, extremely
+        slow glide animation to trigger. To fix this, I added a new state DestinationReached. During this state, additional
+        momentum events that are handled after the glide animation completes will not fill up the momentum window and trigger
+        a second glide event.
+
+        * platform/mac/AxisScrollSnapAnimator.h:
+        * platform/mac/AxisScrollSnapAnimator.mm:
+        (WebCore::AxisScrollSnapAnimator::AxisScrollSnapAnimator):
+        (WebCore::AxisScrollSnapAnimator::handleWheelEvent):
+        (WebCore::AxisScrollSnapAnimator::scrollSnapAnimationUpdate):
+        (WebCore::AxisScrollSnapAnimator::endScrollSnapAnimation):
+        (WebCore::AxisScrollSnapAnimator::computeGlideDelta):
+
 2014-08-15  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Source/WebCore/platform/mac/AxisScrollSnapAnimator.h

@@ -39 +39 @@
     Snapping,
     Gliding,
-    Idle
+    DestinationReached,
+    UserInteraction
 };
 
@@ -72 +73 @@
 private:
     void beginScrollSnapAnimation(ScrollSnapState);
-    void endScrollSnapAnimation();
+    void endScrollSnapAnimation(ScrollSnapState);
 
     float computeSnapDelta() const;

trunk/Source/WebCore/platform/mac/AxisScrollSnapAnimator.mm

@@ -87 +87 @@
     , m_snapOffsets(snapOffsets)
     , m_axis(axis)
-    , m_currentState(ScrollSnapState::Idle)
+    , m_currentState(ScrollSnapState::DestinationReached)
     , m_initialOffset(0)
     , m_targetOffset(0)
@@ -109 +109 @@
     case WheelEventStatus::UserScrollBegin:
     case WheelEventStatus::UserScrolling:
-        endScrollSnapAnimation();
+        endScrollSnapAnimation(ScrollSnapState::UserInteraction);
         break;
 
@@ -118 +118 @@
     case WheelEventStatus::InertialScrollBegin:
         // Begin tracking wheel deltas for glide prediction.
-        endScrollSnapAnimation();
+        endScrollSnapAnimation(ScrollSnapState::UserInteraction);
         pushInitialWheelDelta(wheelDelta);
         m_beginTrackingWheelDeltaOffset = m_client->scrollOffsetInAxis(m_axis);
@@ -124 +124 @@
 
     case WheelEventStatus::InertialScrolling:
-        if (m_currentState != ScrollSnapState::Gliding) {
-            // FIXME: Investigate why small wheel deltas pushed into the window without the < -1, > 1 check.
-            if (m_numWheelDeltasTracked < wheelDeltaWindowSize && (wheelDelta < -1 || wheelDelta > 1))
+        // This check for DestinationReached ensures that we don't receive another set of momentum events after ending the last glide.
+        if (m_currentState != ScrollSnapState::Gliding && m_currentState != ScrollSnapState::DestinationReached) {
+            if (m_numWheelDeltasTracked < wheelDeltaWindowSize)
                 pushInitialWheelDelta(wheelDelta);
 
@@ -156 +156 @@
     if (delta)
         m_client->immediateScrollInAxis(m_axis, delta);
-    else {
-        endScrollSnapAnimation();
-        m_currentState = ScrollSnapState::Idle;
-    }
+    else
+        endScrollSnapAnimation(ScrollSnapState::DestinationReached);
 }
 
@@ -190 +188 @@
 }
 
-void AxisScrollSnapAnimator::endScrollSnapAnimation()
-{
+void AxisScrollSnapAnimator::endScrollSnapAnimation(ScrollSnapState newState)
+{
+    ASSERT(newState == ScrollSnapState::DestinationReached || newState == ScrollSnapState::UserInteraction);
     if (m_currentState == ScrollSnapState::Gliding)
         clearInitialWheelDeltaWindow();
 
-    m_currentState = ScrollSnapState::Idle;
+    m_currentState = newState;
     m_client->stopScrollSnapTimer();
 }
@@ -246 +245 @@
 
     float progress = ((float)(offset - m_initialOffset)) / (m_targetOffset - m_initialOffset);
-    float shift = ceil(m_glideMagnitude * (1 + cos(piFloat * progress + m_glidePhaseShift)));
+    // FIXME: We might want to investigate why -m_glidePhaseShift results in the behavior we want.
+    float shift = ceil(m_glideMagnitude * (1 + cos(piFloat * progress - m_glidePhaseShift)));
     shift = m_initialOffset < m_targetOffset ? std::max<float>(shift, 1) : std::min<float>(shift, -1);
     if ((m_initialOffset < m_targetOffset && offset + shift > m_targetOffset) || (m_initialOffset > m_targetOffset && offset + shift < m_targetOffset))