Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 456be2c3 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Clean up STL transitions coroutine management

This CL cleans up the scoping of the coroutines involved in the
transitions of a SceneTransitionLayoutState.

This CL changes the main API to start a transition:
MutableSTLState.startTransition() is now a suspend function which:

  1. Sets the transition as the current STLState.transitionState.
  2. Runs the suspend fun run() method of the transition.
  3. Finishes the transition when the transition is done running.

Doing this makes it easier to correctly scope a transition animation and
removes the need to call finishTransition() when a transition is done.

This CL also renames Transition.finish() to
Transition.freezeAndAnimateToCurrentState().

Bug: 362727477
Test: atest PlatformComposeSceneTransitionLayoutTests
Flag: com.android.systemui.scene_container
Change-Id: I5455dcd75b6cc9d79dcb8afb8dd6ddd6f9bfcdff
parent 51085f46
Loading
Loading
Loading
Loading
+25 −34
Original line number Diff line number Diff line
@@ -21,9 +21,7 @@ import androidx.compose.animation.core.AnimationVector1D
import androidx.compose.animation.core.SpringSpec
import com.android.compose.animation.scene.content.state.TransitionState
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch

internal fun CoroutineScope.animateContent(
    layoutState: MutableSceneTransitionLayoutStateImpl,
@@ -31,13 +29,9 @@ internal fun CoroutineScope.animateContent(
    oneOffAnimation: OneOffAnimation,
    targetProgress: Float,
    chain: Boolean = true,
) {
    // Start the transition. This will compute the TransformationSpec associated to [transition],
    // which we need to initialize the Animatable that will actually animate it.
    layoutState.startTransition(transition, chain)

    // The transition now contains the transformation spec that we should use to instantiate the
    // Animatable.
): Job {
    oneOffAnimation.onRun = {
        // Animate the progress to its target value.
        val animationSpec = transition.transformationSpec.progressSpec
        val visibilityThreshold =
            (animationSpec as? SpringSpec)?.visibilityThreshold ?: ProgressVisibilityThreshold
@@ -49,19 +43,10 @@ internal fun CoroutineScope.animateContent(
                oneOffAnimation.animatable = it
            }

    // Animate the progress to its target value.
    //
    // Important: We start atomically to make sure that we start the coroutine even if it is
    // cancelled right after it is launched, so that finishTransition() is correctly called.
    // Otherwise, this transition will never be stopped and we will never settle to Idle.
    oneOffAnimation.job =
        launch(start = CoroutineStart.ATOMIC) {
            try {
        animatable.animateTo(targetProgress, animationSpec, initialVelocity)
            } finally {
                layoutState.finishTransition(transition)
            }
    }

    return layoutState.startTransitionImmediately(animationScope = this, transition, chain)
}

internal class OneOffAnimation {
@@ -74,8 +59,8 @@ internal class OneOffAnimation {
     */
    lateinit var animatable: Animatable<Float, AnimationVector1D>

    /** The job that is animating [animatable]. */
    lateinit var job: Job
    /** The runnable to run for this animation. */
    lateinit var onRun: suspend () -> Unit

    val progress: Float
        get() = animatable.value
@@ -83,7 +68,13 @@ internal class OneOffAnimation {
    val progressVelocity: Float
        get() = animatable.velocity

    fun finish(): Job = job
    suspend fun run() {
        onRun()
    }

    fun freezeAndAnimateToCurrentState() {
        // Do nothing, the state of one-off animations never change and we directly animate to it.
    }
}

// TODO(b/290184746): Compute a good default visibility threshold that depends on the layout size
+14 −3
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package com.android.compose.animation.scene

import com.android.compose.animation.scene.content.state.TransitionState
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job

/** Trigger a one-off transition to show or hide an overlay. */
internal fun CoroutineScope.showOrHideOverlay(
@@ -120,7 +119,13 @@ private class OneOffShowOrHideOverlayTransition(
    override val isInitiatedByUserInput: Boolean = false
    override val isUserInputOngoing: Boolean = false

    override fun finish(): Job = oneOffAnimation.finish()
    override suspend fun run() {
        oneOffAnimation.run()
    }

    override fun freezeAndAnimateToCurrentState() {
        oneOffAnimation.freezeAndAnimateToCurrentState()
    }
}

private class OneOffOverlayReplacingTransition(
@@ -140,5 +145,11 @@ private class OneOffOverlayReplacingTransition(
    override val isInitiatedByUserInput: Boolean = false
    override val isUserInputOngoing: Boolean = false

    override fun finish(): Job = oneOffAnimation.finish()
    override suspend fun run() {
        oneOffAnimation.run()
    }

    override fun freezeAndAnimateToCurrentState() {
        oneOffAnimation.freezeAndAnimateToCurrentState()
    }
}
+18 −11
Original line number Diff line number Diff line
@@ -28,7 +28,7 @@ internal fun CoroutineScope.animateToScene(
    layoutState: MutableSceneTransitionLayoutStateImpl,
    target: SceneKey,
    transitionKey: TransitionKey?,
): TransitionState.Transition.ChangeScene? {
): Pair<TransitionState.Transition.ChangeScene, Job>? {
    val transitionState = layoutState.transitionState
    if (transitionState.currentScene == target) {
        // This can happen in 3 different situations, for which there isn't anything else to do:
@@ -139,7 +139,7 @@ private fun CoroutineScope.animateToScene(
    reversed: Boolean = false,
    fromScene: SceneKey = layoutState.transitionState.currentScene,
    chain: Boolean = true,
): TransitionState.Transition.ChangeScene {
): Pair<TransitionState.Transition.ChangeScene, Job> {
    val oneOffAnimation = OneOffAnimation()
    val targetProgress = if (reversed) 0f else 1f
    val transition =
@@ -165,6 +165,7 @@ private fun CoroutineScope.animateToScene(
            )
        }

    val job =
        animateContent(
            layoutState = layoutState,
            transition = transition,
@@ -173,7 +174,7 @@ private fun CoroutineScope.animateToScene(
            chain = chain,
        )

    return transition
    return transition to job
}

private class OneOffSceneTransition(
@@ -193,5 +194,11 @@ private class OneOffSceneTransition(

    override val isUserInputOngoing: Boolean = false

    override fun finish(): Job = oneOffAnimation.finish()
    override suspend fun run() {
        oneOffAnimation.run()
    }

    override fun freezeAndAnimateToCurrentState() {
        oneOffAnimation.freezeAndAnimateToCurrentState()
    }
}
+8 −24
Original line number Diff line number Diff line
@@ -28,7 +28,6 @@ import com.android.compose.animation.scene.content.state.TransitionState
import com.android.compose.animation.scene.content.state.TransitionState.HasOverscrollProperties.Companion.DistanceUnspecified
import com.android.compose.nestedscroll.PriorityNestedScrollConnection
import kotlin.math.absoluteValue
import kotlinx.coroutines.CoroutineScope

internal interface DraggableHandler {
    /**
@@ -63,7 +62,6 @@ internal interface DragController {
internal class DraggableHandlerImpl(
    internal val layoutImpl: SceneTransitionLayoutImpl,
    internal val orientation: Orientation,
    internal val coroutineScope: CoroutineScope,
) : DraggableHandler {
    internal val nestedScrollKey = Any()
    /** The [DraggableHandler] can only have one active [DragController] at a time. */
@@ -101,11 +99,6 @@ internal class DraggableHandlerImpl(

        val swipeAnimation = dragController.swipeAnimation

        // Don't intercept a transition that is finishing.
        if (swipeAnimation.isFinishing) {
            return false
        }

        // Only intercept the current transition if one of the 2 swipes results is also a transition
        // between the same pair of contents.
        val swipes = computeSwipes(startedPosition, pointersDown = 1)
@@ -140,7 +133,6 @@ internal class DraggableHandlerImpl(
            // This [transition] was already driving the animation: simply take over it.
            // Stop animating and start from the current offset.
            val oldSwipeAnimation = oldDragController.swipeAnimation
            oldSwipeAnimation.cancelOffsetAnimation()

            // We need to recompute the swipe results since this is a new gesture, and the
            // fromScene.userActions may have changed.
@@ -192,13 +184,7 @@ internal class DraggableHandlerImpl(
                else -> error("Unknown result $result ($upOrLeftResult $downOrRightResult)")
            }

        return createSwipeAnimation(
            layoutImpl,
            layoutImpl.coroutineScope,
            result,
            isUpOrLeft,
            orientation
        )
        return createSwipeAnimation(layoutImpl, result, isUpOrLeft, orientation)
    }

    private fun computeSwipes(startedPosition: Offset?, pointersDown: Int): Swipes {
@@ -279,16 +265,14 @@ private class DragControllerImpl(

    fun updateTransition(newTransition: SwipeAnimation<*>, force: Boolean = false) {
        if (force || isDrivingTransition) {
            layoutState.startTransition(newTransition.contentTransition)
            layoutState.startTransitionImmediately(
                animationScope = draggableHandler.layoutImpl.animationScope,
                newTransition.contentTransition,
                true
            )
        }

        val previous = swipeAnimation
        swipeAnimation = newTransition

        // Finish the previous transition.
        if (previous != newTransition) {
            layoutState.finishTransition(previous.contentTransition)
        }
    }

    /**
@@ -302,7 +286,7 @@ private class DragControllerImpl(
    }

    private fun <T : ContentKey> onDrag(delta: Float, swipeAnimation: SwipeAnimation<T>): Float {
        if (delta == 0f || !isDrivingTransition || swipeAnimation.isFinishing) {
        if (delta == 0f || !isDrivingTransition || swipeAnimation.isAnimatingOffset()) {
            return 0f
        }

@@ -409,7 +393,7 @@ private class DragControllerImpl(
        swipeAnimation: SwipeAnimation<T>,
    ): Float {
        // The state was changed since the drag started; don't do anything.
        if (!isDrivingTransition || swipeAnimation.isFinishing) {
        if (!isDrivingTransition || swipeAnimation.isAnimatingOffset()) {
            return 0f
        }

+21 −16
Original line number Diff line number Diff line
@@ -22,8 +22,10 @@ import androidx.compose.animation.core.AnimationSpec
import androidx.compose.foundation.gestures.Orientation
import androidx.compose.runtime.Composable
import kotlin.coroutines.cancellation.CancellationException
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch

@Composable
internal fun PredictiveBackHandler(
@@ -42,7 +44,6 @@ internal fun PredictiveBackHandler(
        val animation =
            createSwipeAnimation(
                layoutImpl,
                layoutImpl.coroutineScope,
                result.userActionCopy(
                    transitionKey = result.transitionKey ?: TransitionKey.PredictiveBack
                ),
@@ -64,7 +65,8 @@ private suspend fun <T : ContentKey> animate(
) {
    fun animateOffset(targetContent: T, spec: AnimationSpec<Float>? = null) {
        if (
            layoutImpl.state.transitionState != animation.contentTransition || animation.isFinishing
            layoutImpl.state.transitionState != animation.contentTransition ||
                animation.isAnimatingOffset()
        ) {
            return
        }
@@ -76,20 +78,23 @@ private suspend fun <T : ContentKey> animate(
        )
    }

    layoutImpl.state.startTransition(animation.contentTransition)
    coroutineScope {
        launch {
            try {
                progress.collect { backEvent -> animation.dragOffset = backEvent.progress }

                // Back gesture successful.
                animateOffset(
                    animation.toContent,
            animation.contentTransition.transformationSpec.progressSpec
                    animation.contentTransition.transformationSpec.progressSpec,
                )
            } catch (e: CancellationException) {
                // Back gesture cancelled.
        // If the back gesture is cancelled, the progress is animated back to 0f by the system.
        // Since the remaining change in progress is usually very small, the progressSpec is omitted
        // and the default spring spec used instead.
                animateOffset(animation.fromContent)
            }
        }

        // Start the transition.
        layoutImpl.state.startTransition(animation.contentTransition)
    }
}
Loading