Update scrolling logic in Android horizontal tab switcher

The Android horizontal tab switcher currently feels very "slippery." This is
largely because we currently have a bug in the logic that gives flings a "boost"
to the next tab where, if you start dragging a tab over so that it's already the
centered tab, we apply the boost anyway, so you actually end up scrolling by two
tabs. The minimum scroll distance to move over by one tab is also currently
fairly large, which makes it easier to run into the bugging fling boost
behavior.

This CL fixes these two issues. I'm also introducing some non-linearity into the
scroll distance function. After this CL, the velocity range to scroll by one tab
is fairly large, then the velocity range to scroll by two tabs is somewhat
smaller, and then the ranges to scroll by 3 through 11 tabs are fairly small,
and then the range to scroll by 12 tabs is fairly large again.

Bug: 849417
Change-Id: I9f3d442a191091a126410526c9097c445eb9fe40
Reviewed-on: https://chromium-review.googlesource.com/1109212
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570102}

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/NonOverlappingStack.java

@@ -91,7 +91,21 @@ public class NonOverlappingStack extends Stack {
      * Multiplier for adjusting the scrolling friction from the amount provided by
      * ViewConfiguration.
      */
-    private static final float FRICTION_MULTIPLIER = 0.2f;
+    private static final float FRICTION_MULTIPLIER = 0.6f;
+
+    /**
+     * For short scrolls of duration less than this (in milliseconds), we assume the user wants to
+     * scroll over to the next tab. If the scroll is longer in duration, we assume they're
+     * reconsidering their scroll, so we leave them on the current tab (unless they drag over far
+     * enough to center a new tab).
+     */
+    private static final int SCROLL_BOOST_TIMEOUT_MS = 250;
+
+    /**
+     * The minimum fraction of a tab the user has to scroll over by before we apply the boost to
+     * scroll them to the next tab.
+     */
+    private static final float SCROLL_BOOST_THRESHOLD = 0.05f;
 
     /**
      * Used to prevent mScrollOffset from being changed as a result of clamping during the switch
@@ -106,6 +120,11 @@ public class NonOverlappingStack extends Stack {
      */
     private boolean mSwitchedAway;
 
+    /** Time at which the last touch down event occurred. */
+    private long mLastTouchDownTime;
+    /** Index of the tab that was centered when the last touch down event occurred. */
+    private int mCenteredTabAtTouchDown;
+
     /**
      * @param layout The parent layout.
      */
@@ -179,6 +198,14 @@ public class NonOverlappingStack extends Stack {
         updateScrollSnap();
     }
 
+    @Override
+    public void onDown(long time) {
+        super.onDown(time);
+        mLastTouchDownTime = time;
+        mCenteredTabAtTouchDown = getCenteredTabIndex();
+        mScroller.setCenteredYSnapIndexAtTouchDown(mCenteredTabAtTouchDown);
+    }
+
     @Override
     public void onLongPress(long time, float x, float y) {
         // Ignore long presses
@@ -191,13 +218,26 @@ public class NonOverlappingStack extends Stack {
 
     @Override
     protected void springBack(long time) {
-        if (mScroller.isFinished()) {
-            int newTarget = Math.round(mScrollTarget / mSpacing) * mSpacing;
-            mScroller.springBack(0, (int) mScrollTarget, 0, 0, newTarget, newTarget, time);
+        if (!mScroller.isFinished()) return;
+
+        int offsetAtTouchDown = -mCenteredTabAtTouchDown * mSpacing;
+        float scrollFractionToNextTab = (offsetAtTouchDown - mScrollOffset) / mSpacing;
+
+        int newCenteredTab;
+        // Make quick, short scrolls go over to the next tab (if a scroll is short but not quick, we
+        // assume the user might have decided to stay on the current tab).
+        if (time < mLastTouchDownTime + SCROLL_BOOST_TIMEOUT_MS
+                && Math.abs(scrollFractionToNextTab) > SCROLL_BOOST_THRESHOLD) {
+            newCenteredTab = mCenteredTabAtTouchDown + (int) Math.signum(scrollFractionToNextTab);
+        } else {
+            newCenteredTab = getCenteredTabIndex();
+        }
+
+        int newTarget = -newCenteredTab * mSpacing;
+        mScroller.flingYTo((int) mScrollTarget, newTarget, time);
         setScrollTarget(newTarget, false);
         mLayout.requestUpdate();
     }
-    }
 
     @Override
     protected float getSpacingScreen() {
@@ -273,6 +313,11 @@ public class NonOverlappingStack extends Stack {
         return 0;
     }
 
+    @Override
+    protected boolean allowOverscroll() {
+        return false;
+    }
+
     @Override
     protected int computeSpacing(int layoutTabCount) {
         return (int) Math.round(

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/StackScroller.java

@@ -9,6 +9,8 @@ import android.hardware.SensorManager;
 import android.util.Log;
 import android.view.ViewConfiguration;
 
+import org.chromium.chrome.browser.util.MathUtils;
+
 /**
  * This class is vastly copied from {@link android.widget.OverScroller} but decouples the time
  * from the app time so it can be specified manually.
@@ -59,6 +61,28 @@ public class StackScroller {
         mScrollerY.setSnapDistance(snapDistance);
     }
 
+    /**
+     * This method should be called when a touch down event is received if snapping is enabled in
+     * the X direction.
+     *
+     * @param index What multiple of the snap distance (i.e. it can be multiplied by the snap
+     *              distance) we were closest to when a touch down event was received.
+     */
+    public final void setCenteredXSnapIndexAtTouchDown(int index) {
+        mScrollerX.setCenteredSnapIndexAtTouchDown(index);
+    }
+
+    /**
+     * This method should be called when a touch down event is received if snapping is enabled in
+     * the Y direction.
+     *
+     * @param index What multiple of the snap distance (i.e. it can be multiplied by the snap
+     *              distance) we were closest to when a touch down event was received.
+     */
+    public final void setCenteredYSnapIndexAtTouchDown(int index) {
+        mScrollerY.setCenteredSnapIndexAtTouchDown(index);
+    }
+
     /**
      *
      * Returns whether the scroller has finished scrolling.
@@ -275,6 +299,28 @@ public class StackScroller {
         mScrollerY.fling(startY, velocityY, minY, maxY, overY, time);
     }
 
+    /**
+     * Tells the X scroller to animate a fling to the specified position.
+     *
+     * @param startX The initial position for the animation.
+     * @param finalX The end position for the animation.
+     * @param time The start time to use for the animation.
+     */
+    public void flingXTo(int startX, int finalX, long time) {
+        mScrollerX.flingTo(startX, finalX, time);
+    }
+
+    /**
+     * Tells the Y scroller to animate a fling to the specified position.
+     *
+     * @param startY The initial position for the animation.
+     * @param finalY The end position for the animation.
+     * @param time The start time to use for the animation.
+     */
+    public void flingYTo(int startY, int finalY, long time) {
+        mScrollerY.flingTo(startY, finalY, time);
+    }
+
     /**
      * Stops the animation. Contrary to {@link #forceFinished(boolean)},
      * aborting the animating causes the scroller to move to the final x and y
@@ -328,6 +374,9 @@ public class StackScroller {
         private final float mFlingFriction = ViewConfiguration.getScrollFriction();
         private float mFrictionMultiplier = 1.f;
 
+        private int mCenteredSnapIndexAtTouchDown;
+        private long mLastMaxFlingTime;
+
         // If this is non-zero, we enable logic to force the ending scroll position to be an integer
         // multiple of this number.
         private int mSnapDistance;
@@ -356,6 +405,28 @@ public class StackScroller {
         private static final int CUBIC = 1;
         private static final int BALLISTIC = 2;
 
+        // The following parameters are only used when snapping is enabled (mSnapDistance != 0).
+
+        // Maximum number of snapped positions to scroll over for a call to fling().
+        private static final int MAX_SNAP_SCROLL = 12;
+
+        // Minimum fling velocity to scroll away from the currently-snapped position..
+        private static final int SINGLE_SNAP_MIN_VELOCITY = 100;
+        // Minimum fling velocity to scroll two snap postions instead of one.
+        private static final int DOUBLE_SNAP_MIN_VELOCITY = 1800;
+        // Minimum fling velocity to scroll three snap positions instead of one.
+        private static final int TRIPLE_SNAP_MIN_VELOCITY = 2500;
+        // Minimum fling velocity to scroll by MAX_SNAP_SCROLL positions.
+        private static final int MAX_SNAP_SCROLL_MIN_VELOCITY = 5000;
+
+        // If we receive a fling within this many milliseconds of receiving a previous fling that
+        // caused us to do a maximum distance scroll (and a few other sanity checks hold), we lower
+        // the velocity threshold for the new fling to also do a maximum velocity scroll;
+        private static final int REPEATED_FLING_TIMEOUT = 1500;
+        // Minimum velocity for a "repeated fling" (see previous comment) to trigger a maximum
+        // velocity scroll;
+        private static final int REPEATED_FLING_VELOCITY_THRESHOLD = 1000;
+
         static {
             float xMin = 0.0f;
             float yMin = 0.0f;
@@ -415,6 +486,10 @@ public class StackScroller {
             mSnapDistance = snapDistance;
         }
 
+        void setCenteredSnapIndexAtTouchDown(int centeredSnapDistanceAtTouchDown) {
+            mCenteredSnapIndexAtTouchDown = centeredSnapDistanceAtTouchDown;
+        }
+
         void updateScroll(float q) {
             mCurrentPosition = mStart + Math.round(q * (mFinal - mStart));
         }
@@ -486,7 +561,6 @@ public class StackScroller {
             } else if (start > max) {
                 startSpringback(start, max, 0);
             }
-
             return !mFinished;
         }
 
@@ -504,7 +578,26 @@ public class StackScroller {
             mDuration = (int) (1000.0 * Math.sqrt(-2.0 * delta / mDeceleration));
         }
 
+        int computeSnapScrollDistance(int velocity) {
+            if (Math.abs(velocity) < SINGLE_SNAP_MIN_VELOCITY) return 0;
+            if (Math.abs(velocity) < DOUBLE_SNAP_MIN_VELOCITY) return 1;
+            if (Math.abs(velocity) < TRIPLE_SNAP_MIN_VELOCITY) return 2;
+            if (Math.abs(velocity) >= MAX_SNAP_SCROLL_MIN_VELOCITY) return MAX_SNAP_SCROLL;
+
+            // For fling velocities between TRIPLE_SNAP_MIN_VELOCITY and
+            // MAX_SNAP_SCROLL_MIN_VELOCITY, we do linear interpolation to decide how many snap
+            // positions to scroll by.
+            float increment = (MAX_SNAP_SCROLL_MIN_VELOCITY - TRIPLE_SNAP_MIN_VELOCITY)
+                    / (MAX_SNAP_SCROLL - 3);
+            return (int) ((Math.abs(velocity) - TRIPLE_SNAP_MIN_VELOCITY) / increment) + 3;
+        }
+
         void fling(int start, int velocity, int min, int max, int over, long time) {
+            if (mSnapDistance != 0) {
+                doSnapScroll(start, velocity, min, max, time);
+                return;
+            }
+
             mOver = over;
             mFinished = false;
             mCurrVelocity = mVelocity = velocity;
@@ -521,27 +614,8 @@ public class StackScroller {
             double totalDistance = 0.0;
 
             if (velocity != 0) {
-                totalDistance = getSplineFlingDistance(velocity);
-
-                if (mSnapDistance != 0) {
-                    final double signedDistance = totalDistance *= Math.signum(velocity);
-                    final double newPositionPreSnapping = mCurrentPosition + signedDistance;
-                    double newPositionPostSnapping =
-                            Math.round(newPositionPreSnapping / mSnapDistance) * mSnapDistance;
-
-                    if (Math.round(newPositionPostSnapping / mSnapDistance)
-                            == Math.round((double) mCurrentPosition / mSnapDistance)) {
-                        // If we would snap to the current tab, give a boost to move over to the
-                        // next one.
-                        newPositionPostSnapping += Math.signum(velocity) * mSnapDistance;
-                    }
-
-                    totalDistance = Math.abs(newPositionPostSnapping - mCurrentPosition);
-                    velocity = Math.round(
-                            Math.signum(velocity) * getSplineFlingDistanceInverse(totalDistance));
-                }
-
                 mDuration = mSplineDuration = getSplineFlingDuration(velocity);
+                totalDistance = getSplineFlingDistance(velocity);
             }
 
             mSplineDistance = (int) (totalDistance * Math.signum(velocity));
@@ -559,6 +633,56 @@ public class StackScroller {
             }
         }
 
+        private void doSnapScroll(int start, int velocity, int min, int max, long time) {
+            boolean sameDirection = (Math.signum(velocity) == Math.signum(mCurrVelocity));
+
+            int numTabsToScroll = computeSnapScrollDistance(velocity);
+            if (numTabsToScroll == MAX_SNAP_SCROLL
+                    || (time < mLastMaxFlingTime + REPEATED_FLING_TIMEOUT && sameDirection
+                               && Math.abs(velocity) > REPEATED_FLING_VELOCITY_THRESHOLD)) {
+                // After receiving one "max speed" fling, give a boost to subsequent flings to make
+                // it easier to scroll by a large number of tabs.
+                mLastMaxFlingTime = time;
+                numTabsToScroll = MAX_SNAP_SCROLL;
+            }
+
+            int newCenteredTab =
+                    mCenteredSnapIndexAtTouchDown - (int) Math.signum(velocity) * numTabsToScroll;
+            double newPositionPostSnapping = -newCenteredTab * mSnapDistance;
+
+            double newPositionPostClamping =
+                    MathUtils.clamp((float) newPositionPostSnapping, min, max);
+            if (newPositionPostSnapping == mCurrentPosition) {
+                // Don't apply the repeated fling boost right after a fling that didn't actually
+                // scroll anything.
+                mLastMaxFlingTime = 0;
+                return;
+            }
+
+            flingTo(start, (int) newPositionPostSnapping, time);
+        }
+
+        /**
+         * Animates a fling to the specified position.
+         *
+         * @param startPosition The initial position for the animation.
+         * @param finalPosition The end position for the animation.
+         * @param time The start time to use for the animation.
+         */
+        void flingTo(int startPosition, int finalPosition, long time) {
+            mCurrentPosition = mStart = startPosition;
+            mFinal = finalPosition;
+            mStartTime = time;
+            mSplineDistance = finalPosition - startPosition;
+            mFinished = false;
+            mOver = 0;
+            mState = SPLINE;
+
+            mCurrVelocity = (int) (Math.signum(mSplineDistance)
+                    * getSplineFlingDistanceInverse(Math.abs(mSplineDistance)));
+            mDuration = mSplineDuration = getSplineFlingDuration((int) mCurrVelocity);
+        }
+
         private double getSplineDeceleration(int velocity) {
             return Math.log(INFLEXION * Math.abs(velocity) / (getFriction() * mPhysicalCoeff));
         }