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

Commit e80f8c06 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere Committed by Android (Google) Code Review
Browse files

Merge changes I7498f56a,I833d77d5,Ia822d4a2,Ia928cc43,Ie02f4ed0 into main

* changes:
  Prevent size jumps during interruptions
  Revert local change
  Remove calls to invokeOnCompletion
  Enforce that STLState is mutated on the right thread
  Read transitions during composition instead of layout/drawing
parents 23a3bd1c 1f66aeff
Loading
Loading
Loading
Loading
+10 −8
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import androidx.compose.animation.core.AnimationVector1D
import androidx.compose.animation.core.SpringSpec
import kotlin.math.absoluteValue
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch

@@ -190,13 +191,14 @@ private fun CoroutineScope.animate(
        }

    // 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.
    transition.job =
        launch { animatable.animateTo(targetProgress, animationSpec, initialVelocity) }
            .apply {
                invokeOnCompletion {
                    // Settle the state to Idle(target). Note that this will do nothing if this
                    // transition was replaced/interrupted by another one, and this also runs if
                    // this coroutine is cancelled, i.e. if [this] coroutine scope is cancelled.
        launch(start = CoroutineStart.ATOMIC) {
            try {
                animatable.animateTo(targetProgress, animationSpec, initialVelocity)
            } finally {
                layoutState.finishTransition(transition, targetScene)
            }
        }
+7 −5
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import com.android.compose.animation.scene.TransitionState.HasOverscrollProperti
import com.android.compose.nestedscroll.PriorityNestedScrollConnection
import kotlin.math.absoluteValue
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch

@@ -684,7 +685,11 @@ private class SwipeTransition(
            val isTargetGreater = targetOffset > animatable.value
            val job =
                coroutineScope
                    .launch {
                    // Important: We start atomically to make sure that we start the coroutine even
                    // if it is cancelled right after it is launched, so that snapToScene() is
                    // correctly called. Otherwise, this transition will never be stopped and we
                    // will never settle to Idle.
                    .launch(start = CoroutineStart.ATOMIC) {
                        // TODO(b/327249191): Refactor the code so that we don't even launch a
                        // coroutine if we don't need to animate.
                        if (skipAnimation) {
@@ -726,18 +731,15 @@ private class SwipeTransition(
                            }
                        } finally {
                            bouncingScene = null
                            snapToScene(targetScene)
                        }
                    }
                    // Make sure that we settle to target scene at the end of the animation or if
                    // the animation is cancelled.
                    .apply { invokeOnCompletion { snapToScene(targetScene) } }

            OffsetAnimation(animatable, job)
        }
    }

    fun snapToScene(scene: SceneKey) {
        if (layoutState.transitionState != this) return
        cancelOffsetAnimation()
        layoutState.finishTransition(this, idleScene = scene)
    }
+61 −14
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ import androidx.compose.ui.util.lerp
import com.android.compose.animation.scene.transformation.PropertyTransformation
import com.android.compose.animation.scene.transformation.SharedElementTransformation
import com.android.compose.ui.util.lerp
import kotlin.math.roundToInt
import kotlinx.coroutines.launch

/** An element on screen, that can be composed in one or more scenes. */
@@ -81,11 +82,13 @@ internal class Element(val key: ElementKey) {

        /** The last state this element had in this scene. */
        var lastOffset = Offset.Unspecified
        var lastSize = SizeUnspecified
        var lastScale = Scale.Unspecified
        var lastAlpha = AlphaUnspecified

        /** The state of this element in this scene right before the last interruption (if any). */
        var offsetBeforeInterruption = Offset.Unspecified
        var sizeBeforeInterruption = SizeUnspecified
        var scaleBeforeInterruption = Scale.Unspecified
        var alphaBeforeInterruption = AlphaUnspecified

@@ -96,6 +99,7 @@ internal class Element(val key: ElementKey) {
         * they nicely animate from their values down to 0.
         */
        var offsetInterruptionDelta = Offset.Zero
        var sizeInterruptionDelta = IntSize.Zero
        var scaleInterruptionDelta = Scale.Zero
        var alphaInterruptionDelta = 0f

@@ -127,7 +131,14 @@ internal fun Modifier.element(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: Scene,
    key: ElementKey,
): Modifier = this.then(ElementModifier(layoutImpl, scene, key)).testTag(key.testTag)
): Modifier {
    // Make sure that we read the current transitions during composition and not during
    // layout/drawing.
    // TODO(b/341072461): Revert this and read the current transitions in ElementNode directly once
    // we can ensure that SceneTransitionLayoutImpl will compose new scenes first.
    val currentTransitions = layoutImpl.state.currentTransitions
    return then(ElementModifier(layoutImpl, currentTransitions, scene, key)).testTag(key.testTag)
}

/**
 * An element associated to [ElementNode]. Note that this element does not support updates as its
@@ -135,18 +146,20 @@ internal fun Modifier.element(
 */
private data class ElementModifier(
    private val layoutImpl: SceneTransitionLayoutImpl,
    private val currentTransitions: List<TransitionState.Transition>,
    private val scene: Scene,
    private val key: ElementKey,
) : ModifierNodeElement<ElementNode>() {
    override fun create(): ElementNode = ElementNode(layoutImpl, scene, key)
    override fun create(): ElementNode = ElementNode(layoutImpl, currentTransitions, scene, key)

    override fun update(node: ElementNode) {
        node.update(layoutImpl, scene, key)
        node.update(layoutImpl, currentTransitions, scene, key)
    }
}

internal class ElementNode(
    private var layoutImpl: SceneTransitionLayoutImpl,
    private var currentTransitions: List<TransitionState.Transition>,
    private var scene: Scene,
    private var key: ElementKey,
) : Modifier.Node(), DrawModifierNode, ApproachLayoutModifierNode {
@@ -202,10 +215,13 @@ internal class ElementNode(

    fun update(
        layoutImpl: SceneTransitionLayoutImpl,
        currentTransitions: List<TransitionState.Transition>,
        scene: Scene,
        key: ElementKey,
    ) {
        check(layoutImpl == this.layoutImpl && scene == this.scene)
        this.currentTransitions = currentTransitions

        removeNodeFromSceneState()

        val prevElement = this.element
@@ -236,7 +252,7 @@ internal class ElementNode(
        measurable: Measurable,
        constraints: Constraints,
    ): MeasureResult {
        val transitions = layoutImpl.state.currentTransitions
        val transitions = currentTransitions
        val transition = elementTransition(element, transitions)

        // If this element is not supposed to be laid out now, either because it is not part of any
@@ -251,11 +267,13 @@ internal class ElementNode(
            sceneState.lastAlpha = Element.AlphaUnspecified

            val placeable = measurable.measure(constraints)
            sceneState.lastSize = placeable.size()
            return layout(placeable.width, placeable.height) {}
        }

        val placeable =
            measure(layoutImpl, scene, element, transition, sceneState, measurable, constraints)
        sceneState.lastSize = placeable.size()
        return layout(placeable.width, placeable.height) {
            place(
                layoutImpl,
@@ -270,7 +288,7 @@ internal class ElementNode(
    }

    override fun ContentDrawScope.draw() {
        val transition = elementTransition(element, layoutImpl.state.currentTransitions)
        val transition = elementTransition(element, currentTransitions)
        val drawScale = getDrawScale(layoutImpl, scene, element, transition, sceneState)
        if (drawScale == Scale.Default) {
            drawContent()
@@ -365,12 +383,14 @@ private fun prepareInterruption(element: Element) {
    }

    val lastOffset = lastUniqueState?.lastOffset ?: Offset.Unspecified
    val lastSize = lastUniqueState?.lastSize ?: Element.SizeUnspecified
    val lastScale = lastUniqueState?.lastScale ?: Scale.Unspecified
    val lastAlpha = lastUniqueState?.lastAlpha ?: Element.AlphaUnspecified

    // Store the state of the element before the interruption and reset the deltas.
    sceneStates.forEach { sceneState ->
        sceneState.offsetBeforeInterruption = lastOffset
        sceneState.sizeBeforeInterruption = lastSize
        sceneState.scaleBeforeInterruption = lastScale
        sceneState.alphaBeforeInterruption = lastAlpha

@@ -380,6 +400,7 @@ private fun prepareInterruption(element: Element) {

private fun Element.SceneState.clearInterruptionDeltas() {
    offsetInterruptionDelta = Offset.Zero
    sizeInterruptionDelta = IntSize.Zero
    scaleInterruptionDelta = Scale.Zero
    alphaInterruptionDelta = 0f
}
@@ -615,7 +636,6 @@ private fun interruptedAlpha(
    )
}

@OptIn(ExperimentalComposeUiApi::class)
private fun ApproachMeasureScope.measure(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: Scene,
@@ -637,8 +657,6 @@ private fun ApproachMeasureScope.measure(
    // once.
    var maybePlaceable: Placeable? = null

    fun Placeable.size() = IntSize(width, height)

    val targetSize =
        computeValue(
            layoutImpl,
@@ -653,15 +671,44 @@ private fun ApproachMeasureScope.measure(
            ::lerp,
        )

    return maybePlaceable
        ?: measurable.measure(
    // The measurable was already measured, so we can't take interruptions into account here given
    // that we are not allowed to measure the same measurable twice.
    maybePlaceable?.let { placeable ->
        sceneState.sizeBeforeInterruption = Element.SizeUnspecified
        sceneState.sizeInterruptionDelta = IntSize.Zero
        return placeable
    }

    val interruptedSize =
        computeInterruptedValue(
            layoutImpl,
            transition,
            value = targetSize,
            unspecifiedValue = Element.SizeUnspecified,
            zeroValue = IntSize.Zero,
            getValueBeforeInterruption = { sceneState.sizeBeforeInterruption },
            setValueBeforeInterruption = { sceneState.sizeBeforeInterruption = it },
            getInterruptionDelta = { sceneState.sizeInterruptionDelta },
            setInterruptionDelta = { sceneState.sizeInterruptionDelta = it },
            diff = { a, b -> IntSize(a.width - b.width, a.height - b.height) },
            add = { a, b, bProgress ->
                IntSize(
                    (a.width + b.width * bProgress).roundToInt(),
                    (a.height + b.height * bProgress).roundToInt(),
                )
            },
        )

    return measurable.measure(
        Constraints.fixed(
                targetSize.width.coerceAtLeast(0),
                targetSize.height.coerceAtLeast(0),
            interruptedSize.width.coerceAtLeast(0),
            interruptedSize.height.coerceAtLeast(0),
        )
    )
}

private fun Placeable.size(): IntSize = IntSize(width, height)

private fun ContentDrawScope.getDrawScale(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: Scene,
+4 −0
Original line number Diff line number Diff line
@@ -126,6 +126,10 @@ internal class SceneTransitionLayoutImpl(
                orientation = Orientation.Vertical,
                coroutineScope = coroutineScope,
            )

        // Make sure that the state is created on the same thread (most probably the main thread)
        // than this STLImpl.
        state.checkThread()
    }

    internal fun draggableHandler(orientation: Orientation): DraggableHandlerImpl =
+63 −33
Original line number Diff line number Diff line
@@ -26,8 +26,10 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.Stable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.snapshots.SnapshotStateList
import androidx.compose.runtime.setValue
import androidx.compose.ui.util.fastAll
import androidx.compose.ui.util.fastFilter
import androidx.compose.ui.util.fastForEach
@@ -374,14 +376,17 @@ internal abstract class BaseSceneTransitionLayoutState(
    // TODO(b/290930950): Remove this flag.
    internal var enableInterruptions: Boolean,
) : SceneTransitionLayoutState {
    private val creationThread: Thread = Thread.currentThread()

    /**
     * The current [TransitionState]. This list will either be:
     * 1. A list with a single [TransitionState.Idle] element, when we are idle.
     * 2. A list with one or more [TransitionState.Transition], when we are transitioning.
     */
    @VisibleForTesting
    internal val transitionStates: MutableList<TransitionState> =
        SnapshotStateList<TransitionState>().apply { add(TransitionState.Idle(initialScene)) }
    internal var transitionStates: List<TransitionState> by
        mutableStateOf(listOf(TransitionState.Idle(initialScene)))
        private set

    override val transitionState: TransitionState
        get() = transitionStates.last()
@@ -417,6 +422,20 @@ internal abstract class BaseSceneTransitionLayoutState(
     */
    internal abstract fun CoroutineScope.onChangeScene(scene: SceneKey)

    internal fun checkThread() {
        val current = Thread.currentThread()
        if (current !== creationThread) {
            error(
                """
                    Only the original thread that created a SceneTransitionLayoutState can mutate it
                      Expected: ${creationThread.name}
                      Current: ${current.name}
                """
                    .trimIndent()
            )
        }
    }

    override fun isTransitioning(from: SceneKey?, to: SceneKey?): Boolean {
        val transition = currentTransition ?: return false
        return transition.isTransitioning(from, to)
@@ -441,6 +460,8 @@ internal abstract class BaseSceneTransitionLayoutState(
        transitionKey: TransitionKey?,
        chain: Boolean = true,
    ) {
        checkThread()

        // Compute the [TransformationSpec] when the transition starts.
        val fromScene = transition.fromScene
        val toScene = transition.toScene
@@ -465,7 +486,7 @@ internal abstract class BaseSceneTransitionLayoutState(
        if (!enableInterruptions) {
            // Set the current transition.
            check(transitionStates.size == 1)
            transitionStates[0] = transition
            transitionStates = listOf(transition)
            return
        }

@@ -473,14 +494,12 @@ internal abstract class BaseSceneTransitionLayoutState(
            is TransitionState.Idle -> {
                // Replace [Idle] by [transition].
                check(transitionStates.size == 1)
                transitionStates[0] = transition
                transitionStates = listOf(transition)
            }
            is TransitionState.Transition -> {
                // Force the current transition to finish to currentScene.
                currentState.finish().invokeOnCompletion {
                    // Make sure [finishTransition] is called at the end of the transition.
                    finishTransition(currentState, currentState.currentScene)
                }
                // Force the current transition to finish to currentScene. The transition will call
                // [finishTransition] once it's finished.
                currentState.finish()

                val tooManyTransitions = transitionStates.size >= MAX_CONCURRENT_TRANSITIONS
                val clearCurrentTransitions = !chain || tooManyTransitions
@@ -497,11 +516,11 @@ internal abstract class BaseSceneTransitionLayoutState(
                    // we end up only with the new transition after appending it.
                    check(transitionStates.size == 1)
                    check(transitionStates[0] is TransitionState.Idle)
                    transitionStates.clear()
                }

                    transitionStates = listOf(transition)
                } else {
                    // Append the new transition.
                transitionStates.add(transition)
                    transitionStates = transitionStates + transition
                }
            }
        }
    }
@@ -561,6 +580,8 @@ internal abstract class BaseSceneTransitionLayoutState(
     * nothing if [transition] was interrupted since it was started.
     */
    internal fun finishTransition(transition: TransitionState.Transition, idleScene: SceneKey) {
        checkThread()

        val existingIdleScene = finishedTransitions[transition]
        if (existingIdleScene != null) {
            // This transition was already finished.
@@ -571,6 +592,7 @@ internal abstract class BaseSceneTransitionLayoutState(
            return
        }

        val transitionStates = this.transitionStates
        if (!transitionStates.contains(transition)) {
            // This transition was already removed from transitionStates.
            return
@@ -589,23 +611,40 @@ internal abstract class BaseSceneTransitionLayoutState(
        var lastRemovedIdleScene: SceneKey? = null

        // Remove all first n finished transitions.
        while (transitionStates.isNotEmpty()) {
            val firstTransition = transitionStates[0]
            if (!finishedTransitions.contains(firstTransition)) {
        var i = 0
        val nStates = transitionStates.size
        while (i < nStates) {
            val t = transitionStates[i]
            if (!finishedTransitions.contains(t)) {
                // Stop here.
                break
            }

            // Remove the transition from the list and from the set of finished transitions.
            transitionStates.removeAt(0)
            lastRemovedIdleScene = finishedTransitions.remove(firstTransition)
            // Remove the transition from the set of finished transitions.
            lastRemovedIdleScene = finishedTransitions.remove(t)
            i++
        }

        // If all transitions are finished, we are idle.
        if (transitionStates.isEmpty()) {
        if (i == nStates) {
            check(finishedTransitions.isEmpty())
            transitionStates.add(TransitionState.Idle(checkNotNull(lastRemovedIdleScene)))
            this.transitionStates = listOf(TransitionState.Idle(checkNotNull(lastRemovedIdleScene)))
        } else if (i > 0) {
            this.transitionStates = transitionStates.subList(fromIndex = i, toIndex = nStates)
        }
    }

    fun snapToScene(scene: SceneKey) {
        checkThread()

        // Force finish all transitions.
        while (currentTransitions.isNotEmpty()) {
            val transition = transitionStates[0] as TransitionState.Transition
            finishTransition(transition, transition.currentScene)
        }

        check(transitionStates.size == 1)
        transitionStates = listOf(TransitionState.Idle(scene))
    }

    private fun finishActiveTransitionLinks(idleScene: SceneKey) {
@@ -736,6 +775,8 @@ internal class MutableSceneTransitionLayoutStateImpl(
        coroutineScope: CoroutineScope,
        transitionKey: TransitionKey?,
    ): TransitionState.Transition? {
        checkThread()

        return coroutineScope.animateToScene(
            layoutState = this@MutableSceneTransitionLayoutStateImpl,
            target = targetScene,
@@ -748,17 +789,6 @@ internal class MutableSceneTransitionLayoutStateImpl(
    override fun CoroutineScope.onChangeScene(scene: SceneKey) {
        setTargetScene(scene, coroutineScope = this)
    }

    override fun snapToScene(scene: SceneKey) {
        // Force finish all transitions.
        while (currentTransitions.isNotEmpty()) {
            val transition = transitionStates[0] as TransitionState.Transition
            finishTransition(transition, transition.currentScene)
        }

        check(transitionStates.size == 1)
        transitionStates[0] = TransitionState.Idle(scene)
    }
}

private const val TAG = "SceneTransitionLayoutState"
Loading