Skip to content

Commit

Permalink
Fix unexpected ScrollView fling behavior due to Android P bug workaro…
Browse files Browse the repository at this point in the history
…und (#34233)

Summary:
Some custom logic is applied to workaround a platform bug where velocity may be incorrect on Android P. [The bug in question](https://issuetracker.google.com/issues/112385925) appears to have been fixed before Android `Q` was released, so we shouldn't *need* to apply the workaround on other versions.

As described in #34226 the workaround can adversely affect certain scroll behaviors, which can easily be reproduced when you briefly scroll one direction then quickly fling the opposite direction (see the video in the linked ticket).

This PR changes the workaround to *only* be applied on Android P, in order to avoid causing weird scroll behavior on versions that are not actually affected by the bug the workaround is working around.

## Changelog

```
[Android] [Fixed] - Fix occasionally incorrect ScrollView fling behavior
```

Pull Request resolved: #34233

Test Plan:
- Repro the strange fling behavior in the current version (See video attached in #34226)
- Verify that the string fling behavior is fixed with this patch
- Verify that fling behavior still works as expected on Android versions affected by the [original bug](https://issuetracker.google.com/issues/112385925), and those immediately following it (to verify that the bug being worked around was, in fact, fixed as expected).

Reviewed By: javache

Differential Revision: D38287277

Pulled By: ryancat

fbshipit-source-id: 2c786872c4d41655b3849bb92e02f1f16c663b41
  • Loading branch information
dhleong authored and facebook-github-bot committed Aug 2, 2022
1 parent 1bba590 commit c7c263d
Showing 1 changed file with 21 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import android.graphics.Rect;
import android.graphics.drawable.ColorDrawable;
import android.graphics.drawable.Drawable;
import android.os.Build;
import android.view.KeyEvent;
import android.view.MotionEvent;
import android.view.View;
Expand Down Expand Up @@ -480,18 +481,7 @@ public boolean getChildVisibleRect(View child, Rect r, android.graphics.Point of

@Override
public void fling(int velocityY) {
// Workaround.
// On Android P if a ScrollView is inverted, we will get a wrong sign for
// velocityY (see https://issuetracker.google.com/issues/112385925).
// At the same time, mOnScrollDispatchHelper tracks the correct velocity direction.
//
// Hence, we can use the absolute value from whatever the OS gives
// us and use the sign of what mOnScrollDispatchHelper has tracked.
float signum = Math.signum(mOnScrollDispatchHelper.getYFlingVelocity());
if (signum == 0) {
signum = Math.signum(velocityY);
}
final int correctedVelocityY = (int) (Math.abs(velocityY) * signum);
final int correctedVelocityY = correctFlingVelocityY(velocityY);

if (mPagingEnabled) {
flingAndSnap(correctedVelocityY);
Expand Down Expand Up @@ -530,6 +520,25 @@ public void fling(int velocityY) {
handlePostTouchScrolling(0, correctedVelocityY);
}

private int correctFlingVelocityY(int velocityY) {
if (Build.VERSION.SDK_INT != Build.VERSION_CODES.P) {
return velocityY;
}

// Workaround.
// On Android P if a ScrollView is inverted, we will get a wrong sign for
// velocityY (see https://issuetracker.google.com/issues/112385925).
// At the same time, mOnScrollDispatchHelper tracks the correct velocity direction.
//
// Hence, we can use the absolute value from whatever the OS gives
// us and use the sign of what mOnScrollDispatchHelper has tracked.
float signum = Math.signum(mOnScrollDispatchHelper.getYFlingVelocity());
if (signum == 0) {
signum = Math.signum(velocityY);
}
return (int) (Math.abs(velocityY) * signum);
}

private void enableFpsListener() {
if (isScrollPerfLoggingEnabled()) {
Assertions.assertNotNull(mFpsListener);
Expand Down

0 comments on commit c7c263d

Please sign in to comment.