Fix quick small scroll gesture race issues

Summary:
This diff fixes two edge case (similar to a race condition) that caused unexpected behaviors.

**Problem one**
{F680816408}

The previous fling animation is not canceled when user starts to scroll or drag. This is causing both the animation and scroll are setting the scroll position. Depends on the animation path and scroll speed, there may be cases where the [velocity calculation](https://fburl.com/code/010lsu72) ends up getting reversed values. See P467905091 as an example where you can see `mXFlingVelocity` goes back and forth from positive to negative.

It's hard to see if the wrong values are in the middle, but if that happens in the end of user gesture, the velocity for the next fling would be wrong. It shows a "bounce back" effect, and can be triggered when user makes small quick joystick scrolls in one direction.

**Problem two**

{F680821494}

There is a gap between animator's `onAnimationEnd` lifecycle method [finished](https://fburl.com/code/6baq04ne) and the `Animator#isRunning` API to return false. This is causing issues for `getPostAnimationScrollX` where we [decide to return](https://fburl.com/code/hzzugvch) the animated final value or the scroll value. User may see the `-1` value got used for the next fling start value, and the whole scroll view goes back to the beginning of scroll view and starts to fling.

This happens when the previous fling animation finishes and the animated final value is set to -1, but at the same time the next fling starts before `isRunning` returns false for the previous animation.

**Solution**
The problems are fixed by
- Do not reset animated final value to -1 in `onAnimationEnd` method
- Add `mIsFinished` states and use it to track animation finish signal, instead of using `isRunning` API
- Update logic where we decide to return the correct value for the next animation starts point. We will return previous animated final value when the animation got canceled, and user is going towards that value from the current scroll value.

Changelog:
[Android][Fixed] - Fixed edge case for quick small scrolls causing unexpected scrolling behaviors.

Reviewed By: javache

Differential Revision: D32487846

fbshipit-source-id: f1b0647656e021390e3a05de5846251a4a2647ff
This commit is contained in:
Xin Chen
2021-11-30 13:13:05 -08:00
committed by Facebook GitHub Bot
parent 79d20a1717
commit f70018b375
3 changed files with 72 additions and 12 deletions
@@ -457,6 +457,7 @@ public class ReactHorizontalScrollView extends HorizontalScrollView
ReactScrollViewHelper.emitScrollBeginDragEvent(this);
mDragging = true;
enableFpsListener();
getFlingAnimator().cancel();
return true;
}
} catch (IllegalArgumentException e) {
@@ -819,7 +820,7 @@ public class ReactHorizontalScrollView extends HorizontalScrollView
// predict where a fling would end up so we can scroll to the nearest snap offset
int maximumOffset = Math.max(0, computeHorizontalScrollRange() - getWidth());
int width = getWidth() - ViewCompat.getPaddingStart(this) - ViewCompat.getPaddingEnd(this);
Point postAnimationScroll = ReactScrollViewHelper.getPostAnimationScroll(this);
Point postAnimationScroll = ReactScrollViewHelper.getPostAnimationScroll(this, velocityX > 0);
scroller.fling(
postAnimationScroll.x, // startX
postAnimationScroll.y, // startY
@@ -846,7 +847,8 @@ public class ReactHorizontalScrollView extends HorizontalScrollView
}
double interval = (double) getSnapInterval();
double currentOffset = (double) (ReactScrollViewHelper.getPostAnimationScroll(this).x);
double currentOffset =
(double) (ReactScrollViewHelper.getPostAnimationScroll(this, velocity > 0).x);
double targetOffset = (double) predictFinalScrollPosition(velocity);
int previousPage = (int) Math.floor(currentOffset / interval);
@@ -337,6 +337,7 @@ public class ReactScrollView extends ScrollView
ReactScrollViewHelper.emitScrollBeginDragEvent(this);
mDragging = true;
enableFpsListener();
getFlingAnimator().cancel();
return true;
}
} catch (IllegalArgumentException e) {
@@ -608,7 +609,7 @@ public class ReactScrollView extends ScrollView
// predict where a fling would end up so we can scroll to the nearest snap offset
int maximumOffset = getMaxScrollY();
int height = getHeight() - getPaddingBottom() - getPaddingTop();
Point postAnimationScroll = ReactScrollViewHelper.getPostAnimationScroll(this);
Point postAnimationScroll = ReactScrollViewHelper.getPostAnimationScroll(this, velocityY > 0);
scroller.fling(
postAnimationScroll.x, // startX
postAnimationScroll.y, // startY
@@ -635,7 +636,8 @@ public class ReactScrollView extends ScrollView
*/
private void smoothScrollAndSnap(int velocity) {
double interval = (double) getSnapInterval();
double currentOffset = (double) (ReactScrollViewHelper.getPostAnimationScroll(this).y);
double currentOffset =
(double) (ReactScrollViewHelper.getPostAnimationScroll(this, velocity > 0).y);
double targetOffset = (double) predictFinalScrollPosition(velocity);
int previousPage = (int) Math.floor(currentOffset / interval);
@@ -226,6 +226,8 @@ public class ReactScrollViewHelper {
private final Point mFinalAnimatedPositionScroll = new Point();
private int mScrollAwayPaddingTop = 0;
private final Point mLastStateUpdateScroll = new Point(-1, -1);
private boolean mIsCanceled = false;
private boolean mIsFinished = true;
public ReactScrollViewScrollState(
final int layoutDirection, final ReactScrollViewScrollDirection scrollDirection) {
@@ -283,6 +285,28 @@ public class ReactScrollViewHelper {
mScrollAwayPaddingTop = scrollAwayPaddingTop;
return this;
}
/** Get true if the previous animation was canceled */
public boolean getIsCanceled() {
return mIsCanceled;
}
/** Set the state of current animation is canceled or not */
public ReactScrollViewScrollState setIsCanceled(boolean isCanceled) {
mIsCanceled = isCanceled;
return this;
}
/** Get true if previous animation was finished */
public boolean getIsFinished() {
return mIsFinished;
}
/** Set the state of current animation is finished or not */
public ReactScrollViewScrollState setIsFinished(boolean isFinished) {
mIsFinished = isFinished;
return this;
}
}
/**
@@ -326,11 +350,36 @@ public class ReactScrollViewHelper {
T extends
ViewGroup & FabricViewStateManager.HasFabricViewStateManager & HasScrollState
& HasFlingAnimator>
Point getPostAnimationScroll(final T scrollView) {
final ValueAnimator flingAnimator = scrollView.getFlingAnimator();
return flingAnimator != null && flingAnimator.isRunning()
? scrollView.getReactScrollViewScrollState().getFinalAnimatedPositionScroll()
: new Point(scrollView.getScrollX(), scrollView.getScrollY());
Point getPostAnimationScroll(final T scrollView, final boolean isPositiveVelocity) {
final ReactScrollViewScrollState scrollState = scrollView.getReactScrollViewScrollState();
final int velocityDirectionMask = isPositiveVelocity ? 1 : -1;
final Point animatedScrollPos = scrollState.getFinalAnimatedPositionScroll();
final Point currentScrollPos = new Point(scrollView.getScrollX(), scrollView.getScrollY());
boolean isMovingTowardsAnimatedValue = false;
switch (scrollState.getScrollDirection()) {
case HORIZONTAL:
isMovingTowardsAnimatedValue =
velocityDirectionMask * (animatedScrollPos.x - currentScrollPos.x) > 0;
break;
case VERTICAL:
isMovingTowardsAnimatedValue =
velocityDirectionMask * (animatedScrollPos.y - currentScrollPos.y) > 0;
break;
default:
throw new IllegalArgumentException("ScrollView has unexpected scroll direction.");
}
// When the fling animation is not finished, or it was canceled and now we are moving towards
// the final animated value, we will return the final animated value. This is because follow up
// animation should consider the "would be" animated location, so that previous quick small
// scrolls are still working.
return !scrollState.getIsFinished()
|| (scrollState.getIsCanceled() && isMovingTowardsAnimatedValue)
? animatedScrollPos
: currentScrollPos;
}
public static <
@@ -432,16 +481,23 @@ public class ReactScrollViewHelper {
.addListener(
new Animator.AnimatorListener() {
@Override
public void onAnimationStart(Animator animator) {}
public void onAnimationStart(Animator animator) {
final ReactScrollViewScrollState scrollState =
scrollView.getReactScrollViewScrollState();
scrollState.setIsCanceled(false);
scrollState.setIsFinished(false);
}
@Override
public void onAnimationEnd(Animator animator) {
scrollView.getReactScrollViewScrollState().setFinalAnimatedPositionScroll(-1, -1);
scrollView.getReactScrollViewScrollState().setIsFinished(true);
ReactScrollViewHelper.updateStateOnScroll(scrollView);
}
@Override
public void onAnimationCancel(Animator animator) {}
public void onAnimationCancel(Animator animator) {
scrollView.getReactScrollViewScrollState().setIsCanceled(true);
}
@Override
public void onAnimationRepeat(Animator animator) {}