Allow jumping to previous/next page with horizontal swipe gesture#603
Allow jumping to previous/next page with horizontal swipe gesture#603ggtlvkma356 wants to merge 2 commits intoGrapheneOS:mainfrom
Conversation
| float screenDensity = getResources().getDisplayMetrics().density; | ||
| int swipeThreshold = (int) (SWIPE_THRESHOLD * screenDensity); | ||
| int swipeVelocityThreshold = (int) (SWIPE_VELOCITY_THRESHOLD * screenDensity); |
There was a problem hiding this comment.
We should precompute swipeThreshold and swipeVelocityThreshold instead of calculating it every fling
There was a problem hiding this comment.
I changed it to precomputed, also now the thresholds use system-defined values.
72e8d25 to
54e3738
Compare
| if (scaleDetector.isInProgress()) { | ||
| return false; | ||
| } | ||
|
|
||
| // Ignore multi-touch | ||
| if (e1 != null && (e1.getPointerCount() > 1 || e2.getPointerCount() > 1)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Android delivers touch events one at a time. When the user lifts their fingers from a pinch, this always happens as two separate events, one per finger (physically impossible to lift both fingers in the exact same instant)
The first finger to leave the screen produces a ACTION_POINTER_UP event. At this point, the ScaleGestureDetector sees that only one finger remains, so it ends the scale gesture and sets isInProgress() to false. however, GestureDetector does nothing special on ACTION_POINTER_UP; it never checks for flings on this event type. It only checks for flings when the last finger leaves the screen.
The second (last) finger to leave produces a different ACTION_UP event. Now GestureDetector wakes up: it checks the remaining finger's velocity, and if it's fast enough, fires onFling. But the scale already ended on the previous event. By the time onFling runs and checks isInProgress(), the answer is always false, as the scale ended one event ago.
This means the isInProgress() guard only works if both fingers are still touching the screen when onFling fires. But that never happens because onFling requires the last finger to lift (ACTION_UP), and the scale requires at least two fingers to stay active. The scale always ends first.
The same two-event split explains why the pointer count check also doesn't work as intended. The e1 event (the original ACTION_DOWN) was recorded when the first finger touched so that's one pointer. The e2 event (the ACTION_UP) is recorded when the last finger lifts so that's also one pointer, because the other finger already left on the earlier ACTION_POINTER_UP. GestureDetector never updates e1 when a second finger arrives, and e2 only ever sees the one finger that remains. So both e1.getPointerCount() and e2.getPointerCount() are 1, and the multi-touch guard sees a gesture that looks entirely single-finger.
Can add these logs to GestureHelper to confirm this:
@Override
public boolean onFling(@Nullable MotionEvent e1, @NonNull MotionEvent e2,
float velocityX, float velocityY) {
Log.d(TAG, "onFling: isInProgress=" + scaleDetector.isInProgress()
+ " e1.pointers=" + (e1 != null ? e1.getPointerCount() : "null")
+ " e2.pointers=" + e2.getPointerCount()
+ " vX=" + velocityX + " vY=" + velocityY);gestureView.setOnTouchListener((view, motionEvent) -> {
final int action = motionEvent.getActionMasked();
final boolean scaleInProgress = scaleDetector.isInProgress();
if (action == MotionEvent.ACTION_DOWN) {
Log.d(TAG, "ACTION_DOWN: pointers=" + motionEvent.getPointerCount());
} else if (action == MotionEvent.ACTION_POINTER_DOWN) {
Log.d(TAG, "ACTION_POINTER_DOWN: pointers=" + motionEvent.getPointerCount() + " isInProgress=" + scaleInProgress);
} else if (action == MotionEvent.ACTION_POINTER_UP) {
Log.d(TAG, "ACTION_POINTER_UP: pointers=" + motionEvent.getPointerCount() + " isInProgress=" + scaleInProgress);
} else if (action == MotionEvent.ACTION_UP) {
Log.d(TAG, "ACTION_UP: pointers=" + motionEvent.getPointerCount() + " isInProgress=" + scaleInProgress);
}10:51:17 is when I put both fingers down, 10:51:20 is when I released both fingers during a zoom gesture at the side. In terms of onFling, e1 happens at 10:51:17.887 (only 1 pointer), and e2 happens at 10:51:20.992.
2026-04-07 10:51:17.887 7592-7592 GestureHelper app.grapheneos.pdfviewer.debug D ACTION_DOWN: pointers=1 // This is e1 from onFling
2026-04-07 10:51:17.893 7592-7592 GestureHelper app.grapheneos.pdfviewer.debug D ACTION_POINTER_DOWN: pointers=2 isInProgress=false
2026-04-07 10:51:20.991 7592-7592 GestureHelper app.grapheneos.pdfviewer.debug D ACTION_POINTER_UP: pointers=2 isInProgress=true // Scale ends after this event
2026-04-07 10:51:20.992 7592-7592 GestureHelper app.grapheneos.pdfviewer.debug D ACTION_UP: pointers=1 isInProgress=false // This is e2, i.e., what triggers the fling
2026-04-07 10:51:20.992 7592-7592 GestureHelper app.grapheneos.pdfviewer.debug D onFling: isInProgress=false e1.pointers=1 e2.pointers=1 vX=145.1329 vY=67.527855
We could add a mWasScaling to GestureHelper inside of gestureView.setOnTouchListener and set that to true when scaleDetector.isInProgress(). This mWasScaling can replace both the scaleDetector and pointer count checks. Then only reset mWasScaling to false when user taps again, i.e., ACTION_DOWN is received
| private int swipeThreshold; | ||
| private int swipeVelocityThreshold; |
There was a problem hiding this comment.
nit: these should be prefixed with m
There was a problem hiding this comment.
Actually I saw the other PR, this is probably okay
There was a problem hiding this comment.
Yeah this is based on the assumption that it will be on top of #616.
|
|
||
| private void initializeGestures() { | ||
| ViewConfiguration vc = ViewConfiguration.get(this); | ||
| swipeThreshold = vc.getScaledTouchSlop() * 4; |
There was a problem hiding this comment.
I think it's a bit too sensitive, maybe 6 would work better
This is an improved version of #383, which lacks edge detection, in which case page will change regardless if we are in the middle of a page or at its edge.
The swipe is horizontal only.
closes #41.