summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Jordan Demeulenaere <jdemeulenaere@google.com> 2024-01-26 10:36:13 +0100
committer Jordan Demeulenaere <jdemeulenaere@google.com> 2024-01-26 11:07:51 +0100
commitac9016aaf7f06e98d139fb1bfcd6d022cc41f2b4 (patch)
tree2c0ac456d4251b3d98a5503dac9d048c1adb0bb7
parentfb2738a273b3d4c6c392356939c510df4f5fbf98 (diff)
Fix regression in SceneGestureHandler
This CL fixes a crash introduced in ag/25994973: if the user drags its finger by the exact same amount as the touch slop, then the overSlop will be 0f and we won't be able to compute in which direction the user is dragging. This CL makes sure that the overSlop is not 0f, unless we are intercepting a current swipe transition. Bug: 291053278 Test: SwipeToSceneTest Flag: N/A Change-Id: Ia99a388a6d177fa706cc066d4a9223b8efa919dd
-rw-r--r--packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt30
-rw-r--r--packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt41
2 files changed, 65 insertions, 6 deletions
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
index 8552aaf0ebff..436e5bf6debd 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
@@ -46,6 +46,7 @@ import androidx.compose.ui.platform.LocalViewConfiguration
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.Velocity
import androidx.compose.ui.util.fastForEach
+import kotlin.math.sign
/**
* Make an element draggable in the given [orientation].
@@ -223,12 +224,31 @@ private suspend fun PointerInputScope.detectDragGestures(
// TODO(b/291055080): Replace by await[Orientation]PointerSlopOrCancellation once
// it is public.
- when (orientation) {
- Orientation.Horizontal ->
- awaitHorizontalTouchSlopOrCancellation(down.id, onSlopReached)
- Orientation.Vertical ->
- awaitVerticalTouchSlopOrCancellation(down.id, onSlopReached)
+ val drag =
+ when (orientation) {
+ Orientation.Horizontal ->
+ awaitHorizontalTouchSlopOrCancellation(down.id, onSlopReached)
+ Orientation.Vertical ->
+ awaitVerticalTouchSlopOrCancellation(down.id, onSlopReached)
+ }
+
+ // Make sure that overSlop is not 0f. This can happen when the user drags by exactly
+ // the touch slop. However, the overSlop we pass to onDragStarted() is used to
+ // compute the direction we are dragging in, so overSlop should never be 0f unless
+ // we intercept an ongoing swipe transition (i.e. startDragImmediately() returned
+ // true).
+ if (drag != null && overSlop == 0f) {
+ val deltaOffset = drag.position - initialDown.position
+ val delta =
+ when (orientation) {
+ Orientation.Horizontal -> deltaOffset.y
+ Orientation.Vertical -> deltaOffset.y
+ }
+ check(delta != 0f)
+ overSlop = delta.sign
}
+
+ drag
}
if (drag != null) {
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
index 940335853221..560f3b7c7d53 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
@@ -74,6 +74,7 @@ class SwipeToSceneTest {
mapOf(
Swipe.Left to TestScenes.SceneB,
Swipe.Down to TestScenes.SceneC,
+ Swipe.Up to TestScenes.SceneB,
),
) {
Box(Modifier.fillMaxSize())
@@ -357,7 +358,7 @@ class SwipeToSceneTest {
// detected as a drag event.
var touchSlop = 0f
- val layoutState = MutableSceneTransitionLayoutState(TestScenes.SceneA)
+ val layoutState = layoutState()
val verticalSwipeDistance = 50.dp
assertThat(verticalSwipeDistance).isNotEqualTo(LayoutHeight)
@@ -392,4 +393,42 @@ class SwipeToSceneTest {
assertThat(transition).isNotNull()
assertThat(transition!!.progress).isEqualTo(0.5f)
}
+
+ @Test
+ fun swipeByTouchSlop() {
+ val layoutState = layoutState()
+ var touchSlop = 0f
+ rule.setContent {
+ touchSlop = LocalViewConfiguration.current.touchSlop
+ TestContent(layoutState)
+ }
+
+ // Swipe down by exactly touchSlop, so that the drag overSlop is 0f.
+ rule.onRoot().performTouchInput {
+ down(middle)
+ moveBy(Offset(0f, touchSlop), delayMillis = 1_000)
+ }
+
+ // We should still correctly compute that we are swiping down to scene C.
+ var transition = layoutState.currentTransition
+ assertThat(transition).isNotNull()
+ assertThat(transition?.toScene).isEqualTo(TestScenes.SceneC)
+
+ // Release the finger, animating back to scene A.
+ rule.onRoot().performTouchInput { up() }
+ rule.waitForIdle()
+ assertThat(layoutState.currentTransition).isNull()
+ assertThat(layoutState.transitionState.currentScene).isEqualTo(TestScenes.SceneA)
+
+ // Swipe up by exactly touchSlop, so that the drag overSlop is 0f.
+ rule.onRoot().performTouchInput {
+ down(middle)
+ moveBy(Offset(0f, -touchSlop), delayMillis = 1_000)
+ }
+
+ // We should still correctly compute that we are swiping up to scene B.
+ transition = layoutState.currentTransition
+ assertThat(transition).isNotNull()
+ assertThat(transition?.toScene).isEqualTo(TestScenes.SceneB)
+ }
}