Fix SceneGestureHandler edge cases (64a85135) · Commits · e / os / android_frameworks_base

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneGestureHandler.kt

+88 −43

Original line number	Original line	Diff line number	Diff line
	@@ -82,12 +82,15 @@ internal class SceneGestureHandler(
	private var actionDownOrRight: UserAction? = null		private var actionDownOrRight: UserAction? = null
	private var actionUpOrLeftNoEdge: UserAction? = null		private var actionUpOrLeftNoEdge: UserAction? = null
	private var actionDownOrRightNoEdge: UserAction? = null		private var actionDownOrRightNoEdge: UserAction? = null
			private var upOrLeftScene: SceneKey? = null
			private var downOrRightScene: SceneKey? = null

	internal fun onDragStarted(pointersDown: Int, startedPosition: Offset?, overSlop: Float) {		internal fun onDragStarted(pointersDown: Int, startedPosition: Offset?, overSlop: Float) {
	if (isDrivingTransition) {		if (isDrivingTransition) {
	// This [transition] was already driving the animation: simply take over it.		// This [transition] was already driving the animation: simply take over it.
	// Stop animating and start from where the current offset.		// Stop animating and start from where the current offset.
	swipeTransition.cancelOffsetAnimation()		swipeTransition.cancelOffsetAnimation()
			updateTargetScenes(swipeTransition._fromScene)
	return		return
	}		}

	@@ -105,11 +108,8 @@ internal class SceneGestureHandler(
	val fromScene = currentScene		val fromScene = currentScene
	setCurrentActions(fromScene, startedPosition, pointersDown)		setCurrentActions(fromScene, startedPosition, pointersDown)

	if (fromScene.upOrLeft() == null && fromScene.downOrRight() == null) {		val (targetScene, distance) =
	return		findTargetSceneAndDistance(fromScene, overSlop, updateScenes = true) ?: return
	}

	val (targetScene, distance) = fromScene.findTargetSceneAndDistance(overSlop)

	updateTransition(SwipeTransition(fromScene, targetScene, distance), force = true)		updateTransition(SwipeTransition(fromScene, targetScene, distance), force = true)
	}		}
	@@ -179,16 +179,21 @@ internal class SceneGestureHandler(

	val (fromScene, acceleratedOffset) =		val (fromScene, acceleratedOffset) =
	computeFromSceneConsideringAcceleratedSwipe(swipeTransition)		computeFromSceneConsideringAcceleratedSwipe(swipeTransition)
	swipeTransition.dragOffset += acceleratedOffset

	// Compute the target scene depending on the current offset.		val isNewFromScene = fromScene.key != swipeTransition.fromScene
	val (targetScene, distance) =		val (targetScene, distance) =
	fromScene.findTargetSceneAndDistance(swipeTransition.dragOffset)		findTargetSceneAndDistance(
			fromScene,
			swipeTransition.dragOffset,
			updateScenes = isNewFromScene,
			)
			?: run {
			onDragStopped(delta, true)
			return
			}
			swipeTransition.dragOffset += acceleratedOffset

	// TODO(b/290184746): support long scroll A => B => C? especially for non fullscreen scenes		if (isNewFromScene \|\| targetScene.key != swipeTransition.toScene) {
	if (
	fromScene.key != swipeTransition.fromScene \|\| targetScene.key != swipeTransition.toScene
	) {
	updateTransition(		updateTransition(
	SwipeTransition(fromScene, targetScene, distance).apply {		SwipeTransition(fromScene, targetScene, distance).apply {
	this.dragOffset = swipeTransition.dragOffset		this.dragOffset = swipeTransition.dragOffset
	@@ -197,6 +202,11 @@ internal class SceneGestureHandler(
	}		}
	}		}

			private fun updateTargetScenes(fromScene: Scene) {
			upOrLeftScene = fromScene.upOrLeft()
			downOrRightScene = fromScene.downOrRight()
			}

	/**		/**
	* Change fromScene in the case where the user quickly swiped multiple times in the same		* Change fromScene in the case where the user quickly swiped multiple times in the same
	* direction to accelerate the transition from A => B then B => C.		* direction to accelerate the transition from A => B then B => C.
	@@ -221,30 +231,64 @@ internal class SceneGestureHandler(
	// If the offset is past the distance then let's change fromScene so that the user can swipe		// If the offset is past the distance then let's change fromScene so that the user can swipe
	// to the next screen or go back to the previous one.		// to the next screen or go back to the previous one.
	val offset = swipeTransition.dragOffset		val offset = swipeTransition.dragOffset
	return if (offset <= -absoluteDistance && fromScene.upOrLeft() == toScene.key) {		return if (offset <= -absoluteDistance && upOrLeftScene == toScene.key) {
	Pair(toScene, absoluteDistance)		Pair(toScene, absoluteDistance)
	} else if (offset >= absoluteDistance && fromScene.downOrRight() == toScene.key) {		} else if (offset >= absoluteDistance && downOrRightScene == toScene.key) {
	Pair(toScene, -absoluteDistance)		Pair(toScene, -absoluteDistance)
	} else {		} else {
	Pair(fromScene, 0f)		Pair(fromScene, 0f)
	}		}
	}		}

	// TODO(b/290184746): there are two bugs here:		/**
	// 1. if both upOrLeft and downOrRight become `null` during a transition this will crash		* Returns the target scene and distance from [fromScene] in the direction [directionOffset].
	// 2. if one of them changes during a transition, the transition will jump cut to the new target		*
	private inline fun Scene.findTargetSceneAndDistance(		* @param fromScene the scene from which we look for the target
	directionOffset: Float		* @param directionOffset signed float that indicates the direction. Positive is down or right
	): Pair<Scene, Float> {		* negative is up or left.
	val upOrLeft = upOrLeft()		* @param updateScenes whether the target scenes should be updated to the current values held in
	val downOrRight = downOrRight()		* the Scenes map. Usually we don't want to update them while doing a drag, because this could
	val absoluteDistance = getAbsoluteDistance()		* change the target scene (jump cutting) to a different scene, when some system state changed
			* the targets the background. However, an update is needed any time we calculate the targets
			* for a new fromScene.
			* @return null when there are no targets in either direction. If one direction is null and you
			* drag into the null direction this function will return the opposite direction, assuming
			* that the users intention is to start the drag into the other direction eventually. If
			* [directionOffset] is 0f and both direction are available, it will default to
			* [upOrLeftScene].
			*/
			private inline fun findTargetSceneAndDistance(
			fromScene: Scene,
			directionOffset: Float,
			updateScenes: Boolean,
			): Pair<Scene, Float>? {
			if (updateScenes) updateTargetScenes(fromScene)
			val absoluteDistance = fromScene.getAbsoluteDistance()

	// Compute the target scene depending on the current offset.		// Compute the target scene depending on the current offset.
	return if ((directionOffset < 0f && upOrLeft != null) \|\| downOrRight == null) {		return when {
	Pair(layoutImpl.scene(upOrLeft!!), -absoluteDistance)		upOrLeftScene == null && downOrRightScene == null -> null
	} else {		(directionOffset < 0f && upOrLeftScene != null) \|\| downOrRightScene == null ->
	Pair(layoutImpl.scene(downOrRight), absoluteDistance)		Pair(layoutImpl.scene(upOrLeftScene!!), -absoluteDistance)
			else -> Pair(layoutImpl.scene(downOrRightScene!!), absoluteDistance)
			}
			}

			/**
			* A strict version of [findTargetSceneAndDistance] that will return null when there is no Scene
			* in [directionOffset] direction
			*/
			private inline fun findTargetSceneAndDistanceStrict(
			fromScene: Scene,
			directionOffset: Float,
			): Pair<Scene, Float>? {
			val absoluteDistance = fromScene.getAbsoluteDistance()
			return when {
			directionOffset > 0f ->
			upOrLeftScene?.let { Pair(layoutImpl.scene(it), -absoluteDistance) }
			directionOffset < 0f ->
			downOrRightScene?.let { Pair(layoutImpl.scene(it), absoluteDistance) }
			else -> null
	}		}
	}		}

	@@ -311,20 +355,21 @@ internal class SceneGestureHandler(
	val startFromIdlePosition = swipeTransition.dragOffset == 0f		val startFromIdlePosition = swipeTransition.dragOffset == 0f

	if (startFromIdlePosition) {		if (startFromIdlePosition) {
	// If there is a next scene, we start the overscroll animation.		// If there is a target scene, we start the overscroll animation.
	val (targetScene, distance) = fromScene.findTargetSceneAndDistance(velocity)		val (targetScene, distance) =
	val isValidTarget = distance != 0f && targetScene.key != fromScene.key		findTargetSceneAndDistanceStrict(fromScene, velocity)
	if (isValidTarget) {		?: run {
			// We will not animate
			transitionState = TransitionState.Idle(fromScene.key)
			return
			}

	updateTransition(		updateTransition(
	SwipeTransition(fromScene, targetScene, distance).apply {		SwipeTransition(fromScene, targetScene, distance).apply {
	_currentScene = swipeTransition._currentScene		_currentScene = swipeTransition._currentScene
	}		}
	)		)
	animateTo(targetScene = fromScene, targetOffset = 0f)		animateTo(targetScene = fromScene, targetOffset = 0f)
	} else {
	// We will not animate
	transitionState = TransitionState.Idle(fromScene.key)
	}
	} else {		} else {
	// We were between two scenes: animate to the initial scene.		// We were between two scenes: animate to the initial scene.
	animateTo(targetScene = fromScene, targetOffset = 0f)		animateTo(targetScene = fromScene, targetOffset = 0f)

packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SceneGestureHandlerTest.kt

+72 −2

Original line number	Original line	Diff line number	Diff line
	@@ -58,16 +58,22 @@ class SceneGestureHandlerTest {
	private val layoutState: SceneTransitionLayoutState =		private val layoutState: SceneTransitionLayoutState =
	SceneTransitionLayoutState(internalCurrentScene)		SceneTransitionLayoutState(internalCurrentScene)

			val mutableUserActionsA: MutableMap<UserAction, SceneKey> =
			mutableMapOf(Swipe.Up to SceneB, Swipe.Down to SceneC)

			val mutableUserActionsB: MutableMap<UserAction, SceneKey> =
			mutableMapOf(Swipe.Up to SceneC, Swipe.Down to SceneA)

	private val scenesBuilder: SceneTransitionLayoutScope.() -> Unit = {		private val scenesBuilder: SceneTransitionLayoutScope.() -> Unit = {
	scene(		scene(
	key = SceneA,		key = SceneA,
	userActions = mapOf(Swipe.Up to SceneB, Swipe.Down to SceneC),		userActions = mutableUserActionsA,
	) {		) {
	Text("SceneA")		Text("SceneA")
	}		}
	scene(		scene(
	key = SceneB,		key = SceneB,
	userActions = mapOf(Swipe.Up to SceneC, Swipe.Down to SceneA),		userActions = mutableUserActionsB,
	) {		) {
	Text("SceneB")		Text("SceneB")
	}		}
	@@ -411,6 +417,70 @@ class SceneGestureHandlerTest {
	assertIdle(currentScene = SceneC)		assertIdle(currentScene = SceneC)
	}		}

			@Test
			fun onAccelaratedScrollBothTargetsBecomeNull_settlesToIdle() = runGestureTest {
			draggable.onDragStarted()
			draggable.onDelta(up(0.2f))

			draggable.onDelta(up(0.2f))
			draggable.onDragStopped(velocity = -velocityThreshold)
			assertTransition(currentScene = SceneB, fromScene = SceneA, toScene = SceneB)

			mutableUserActionsA.remove(Swipe.Up)
			mutableUserActionsA.remove(Swipe.Down)
			mutableUserActionsB.remove(Swipe.Up)
			mutableUserActionsB.remove(Swipe.Down)

			// start accelaratedScroll and scroll over to B -> null
			draggable.onDragStarted()
			draggable.onDelta(up(0.5f))
			draggable.onDelta(up(0.5f))

			// here onDragStopped is already triggered, but subsequent onDelta/onDragStopped calls may
			// still be called. Make sure that they don't crash or change the scene
			draggable.onDelta(up(0.5f))
			draggable.onDragStopped(0f)

			advanceUntilIdle()
			assertIdle(SceneB)

			// These events can still come in after the animation has settled
			draggable.onDelta(up(0.5f))
			draggable.onDragStopped(0f)
			assertIdle(SceneB)
			}

			@Test
			fun onDragTargetsChanged_targetStaysTheSame() = runGestureTest {
			draggable.onDragStarted(up(0.1f))
			assertTransition(fromScene = SceneA, toScene = SceneB, progress = 0.1f)

			mutableUserActionsA[Swipe.Up] = SceneC
			draggable.onDelta(up(0.1f))
			// target stays B even though UserActions changed
			assertTransition(fromScene = SceneA, toScene = SceneB, progress = 0.2f)
			draggable.onDragStopped(down(0.1f))
			advanceUntilIdle()

			// now target changed to C for new drag
			draggable.onDragStarted(up(0.1f))
			assertTransition(fromScene = SceneA, toScene = SceneC, progress = 0.1f)
			}

			@Test
			fun onDragTargetsChanged_targetsChangeWhenStartingNewDrag() = runGestureTest {
			draggable.onDragStarted(up(0.1f))
			assertTransition(fromScene = SceneA, toScene = SceneB, progress = 0.1f)

			mutableUserActionsA[Swipe.Up] = SceneC
			draggable.onDelta(up(0.1f))
			draggable.onDragStopped(down(0.1f))

			// now target changed to C for new drag that started before previous drag settled to Idle
			draggable.onDragStarted(up(0.1f))
			assertTransition(fromScene = SceneA, toScene = SceneC, progress = 0.3f)
			}

	@Test		@Test
	fun startGestureDuringAnimatingOffset_shouldImmediatelyStopTheAnimation() = runGestureTest {		fun startGestureDuringAnimatingOffset_shouldImmediatelyStopTheAnimation() = runGestureTest {
	draggable.onDragStarted()		draggable.onDragStarted()