Replace transitions to avoid unnecessary interruption handling
This CL adds a Transition.replacedTransition so that a transition can be
replaced without triggering the interruption support. This is for
instance used when intercepting a swipe transition.
Bug: 290930950
Test: atest ElementTest
Test: atest DraggableHandlerTest
Flag: com.android.systemui.scene_container
Change-Id: I864f217b1eb62abc970323daf076f1c0af71c764
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/AnimateToScene.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/AnimateToScene.kt
index c2dd803..ea740a8 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/AnimateToScene.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/AnimateToScene.kt
@@ -48,8 +48,15 @@
}
return when (transitionState) {
- is TransitionState.Idle ->
- animate(layoutState, target, transitionKey, isInitiatedByUserInput = false)
+ is TransitionState.Idle -> {
+ animate(
+ layoutState,
+ target,
+ transitionKey,
+ isInitiatedByUserInput = false,
+ replacedTransition = null,
+ )
+ }
is TransitionState.Transition -> {
val isInitiatedByUserInput = transitionState.isInitiatedByUserInput
@@ -79,6 +86,7 @@
isInitiatedByUserInput,
initialProgress = progress,
initialVelocity = transitionState.progressVelocity,
+ replacedTransition = transitionState,
)
}
} else if (transitionState.fromScene == target) {
@@ -101,6 +109,7 @@
initialProgress = progress,
initialVelocity = transitionState.progressVelocity,
reversed = true,
+ replacedTransition = transitionState,
)
}
} else {
@@ -137,6 +146,7 @@
isInitiatedByUserInput,
fromScene = animateFrom,
chain = chain,
+ replacedTransition = null,
)
}
}
@@ -148,6 +158,7 @@
targetScene: SceneKey,
transitionKey: TransitionKey?,
isInitiatedByUserInput: Boolean,
+ replacedTransition: TransitionState.Transition?,
initialProgress: Float = 0f,
initialVelocity: Float = 0f,
reversed: Boolean = false,
@@ -164,6 +175,7 @@
currentScene = targetScene,
isInitiatedByUserInput = isInitiatedByUserInput,
isUserInputOngoing = false,
+ replacedTransition = replacedTransition,
)
} else {
OneOffTransition(
@@ -173,6 +185,7 @@
currentScene = targetScene,
isInitiatedByUserInput = isInitiatedByUserInput,
isUserInputOngoing = false,
+ replacedTransition = replacedTransition,
)
}
@@ -214,7 +227,8 @@
override val currentScene: SceneKey,
override val isInitiatedByUserInput: Boolean,
override val isUserInputOngoing: Boolean,
-) : TransitionState.Transition(fromScene, toScene) {
+ replacedTransition: TransitionState.Transition?,
+) : TransitionState.Transition(fromScene, toScene, replacedTransition) {
/**
* The animatable used to animate this transition.
*
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt
index da968ac..e313aa5 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt
@@ -37,7 +37,7 @@
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
-interface DraggableHandler {
+internal interface DraggableHandler {
/**
* Start a drag in the given [startedPosition], with the given [overSlop] and number of
* [pointersDown].
@@ -51,7 +51,7 @@
* The [DragController] provides control over the transition between two scenes through the [onDrag]
* and [onStop] methods.
*/
-interface DragController {
+internal interface DragController {
/** Drag the current scene by [delta] pixels. */
fun onDrag(delta: Float)
@@ -537,6 +537,7 @@
orientation = orientation,
isUpOrLeft = isUpOrLeft,
requiresFullDistanceSwipe = result.requiresFullDistanceSwipe,
+ replacedTransition = null,
)
}
@@ -553,6 +554,7 @@
isUpOrLeft = old.isUpOrLeft,
lastDistance = old.lastDistance,
requiresFullDistanceSwipe = old.requiresFullDistanceSwipe,
+ replacedTransition = old,
)
.apply {
_currentScene = old._currentScene
@@ -571,9 +573,10 @@
override val orientation: Orientation,
override val isUpOrLeft: Boolean,
val requiresFullDistanceSwipe: Boolean,
+ replacedTransition: SwipeTransition?,
var lastDistance: Float = DistanceUnspecified,
) :
- TransitionState.Transition(_fromScene.key, _toScene.key),
+ TransitionState.Transition(_fromScene.key, _toScene.key, replacedTransition),
TransitionState.HasOverscrollProperties {
var _currentScene by mutableStateOf(_fromScene)
override val currentScene: SceneKey
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt
index d4f1ad1..69124c1 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt
@@ -495,6 +495,10 @@
transition: TransitionState.Transition,
previousTransition: TransitionState.Transition,
) {
+ if (transition.replacedTransition == previousTransition) {
+ return
+ }
+
val sceneStates = element.sceneStates
fun updatedSceneState(key: SceneKey): Element.SceneState? {
return sceneStates[key]?.also { it.selfUpdateValuesBeforeInterruption() }
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt
index a8df6f4..5b4fbf0 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt
@@ -224,6 +224,9 @@
/** The scene this transition is going to. Can't be the same as fromScene */
val toScene: SceneKey,
+
+ /** The transition that `this` transition is replacing, if any. */
+ internal val replacedTransition: Transition? = null,
) : TransitionState {
/**
* The key of this transition. This should usually be null, but it can be specified to use a
@@ -279,6 +282,11 @@
init {
check(fromScene != toScene)
+ check(
+ replacedTransition == null ||
+ (replacedTransition.fromScene == fromScene &&
+ replacedTransition.toScene == toScene)
+ )
}
/**
@@ -321,6 +329,10 @@
return 0f
}
+ if (replacedTransition != null) {
+ return replacedTransition.interruptionProgress(layoutImpl)
+ }
+
fun create(): Animatable<Float, AnimationVector1D> {
val animatable = Animatable(1f, visibilityThreshold = ProgressVisibilityThreshold)
layoutImpl.coroutineScope.launch {
@@ -521,6 +533,10 @@
check(transitionStates.size == 1)
check(transitionStates[0] is TransitionState.Idle)
transitionStates = listOf(transition)
+ } else if (currentState == transition.replacedTransition) {
+ // Replace the transition.
+ transitionStates =
+ transitionStates.subList(0, transitionStates.lastIndex) + transition
} else {
// Append the new transition.
transitionStates = transitionStates + transition
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt
index c738ad3..55a342c 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt
@@ -1233,4 +1233,17 @@
advanceUntilIdle()
assertIdle(SceneB)
}
+
+ @Test
+ fun interceptingTransitionReplacesCurrentTransition() = runGestureTest {
+ val controller = onDragStarted(overSlop = up(fractionOfScreen = 0.5f))
+ val transition = assertThat(layoutState.transitionState).isTransition()
+ controller.onDragStopped(velocity = 0f)
+
+ // Intercept the transition.
+ onDragStartedImmediately()
+ val newTransition = assertThat(layoutState.transitionState).isTransition()
+ assertThat(newTransition).isNotSameInstanceAs(transition)
+ assertThat(newTransition.replacedTransition).isSameInstanceAs(transition)
+ }
}
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt
index 7c20a97..9646bc2 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt
@@ -2088,4 +2088,42 @@
rule.onNode(isElement(TestElements.Foo, SceneA)).assertPositionInRootIsEqualTo(20.dp, 30.dp)
rule.onNode(isElement(TestElements.Foo, SceneB)).assertIsNotDisplayed()
}
+
+ @Test
+ fun replacedTransitionDoesNotTriggerInterruption() = runTest {
+ val state = rule.runOnIdle { MutableSceneTransitionLayoutStateImpl(SceneA) }
+
+ @Composable
+ fun SceneScope.Foo(modifier: Modifier = Modifier) {
+ Box(modifier.element(TestElements.Foo).size(10.dp))
+ }
+
+ rule.setContent {
+ SceneTransitionLayout(state) {
+ scene(SceneA) { Foo() }
+ scene(SceneB) { Foo(Modifier.offset(40.dp, 60.dp)) }
+ }
+ }
+
+ // Start A => B at 50%.
+ val aToB1 =
+ transition(from = SceneA, to = SceneB, progress = { 0.5f }, onFinish = neverFinish())
+ rule.runOnUiThread { state.startTransition(aToB1) }
+ rule.onNode(isElement(TestElements.Foo, SceneA)).assertIsNotDisplayed()
+ rule.onNode(isElement(TestElements.Foo, SceneB)).assertPositionInRootIsEqualTo(20.dp, 30.dp)
+
+ // Replace A => B by another A => B at 100%. Even with interruption progress at 100%, Foo
+ // should be at (40dp, 60dp) given that aToB1 was replaced by aToB2.
+ val aToB2 =
+ transition(
+ from = SceneA,
+ to = SceneB,
+ progress = { 1f },
+ interruptionProgress = { 1f },
+ replacedTransition = aToB1,
+ )
+ rule.runOnUiThread { state.startTransition(aToB2) }
+ rule.onNode(isElement(TestElements.Foo, SceneA)).assertIsNotDisplayed()
+ rule.onNode(isElement(TestElements.Foo, SceneB)).assertPositionInRootIsEqualTo(40.dp, 60.dp)
+ }
}
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/InterruptionHandlerTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/InterruptionHandlerTest.kt
index 09d1a82..3552d3d 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/InterruptionHandlerTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/InterruptionHandlerTest.kt
@@ -131,10 +131,6 @@
assertThat(state.currentTransitions)
.comparingElementsUsing(FromToCurrentTriple)
.containsExactly(
- // Initial transition A to B. This transition will never be consumed by anyone given
- // that it has the same (from, to) pair as the next transition.
- Triple(SceneA, SceneB, SceneB),
-
// Initial transition reversed, B back to A.
Triple(SceneA, SceneB, SceneA),
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/Transition.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/Transition.kt
index 322b035..65f4f9e 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/Transition.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/Transition.kt
@@ -37,8 +37,11 @@
bouncingScene: SceneKey? = null,
orientation: Orientation = Orientation.Horizontal,
onFinish: ((TransitionState.Transition) -> Job)? = null,
+ replacedTransition: TransitionState.Transition? = null,
): TransitionState.Transition {
- return object : TransitionState.Transition(from, to), TransitionState.HasOverscrollProperties {
+ return object :
+ TransitionState.Transition(from, to, replacedTransition),
+ TransitionState.HasOverscrollProperties {
override val currentScene: SceneKey
get() = current()