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

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

Merge changes Ic8bd91b3,I371c8b93,Id9ba18e7 into main

* changes:
  Prevent NPE when placing fading elements
  Move TransitionKey to Transition
  Fix interruption + overscroll bug
parents 35753ddb e3c540ad
Loading
Loading
Loading
Loading
+5 −3
Original line number Original line Diff line number Diff line
@@ -109,8 +109,7 @@ internal fun CoroutineScope.animateToScene(
                    layoutState.transitions.interruptionHandler.onInterruption(
                    layoutState.transitions.interruptionHandler.onInterruption(
                        transitionState,
                        transitionState,
                        target,
                        target,
                    )
                    ) ?: DefaultInterruptionHandler.onInterruption(transitionState, target)
                        ?: DefaultInterruptionHandler.onInterruption(transitionState, target)


                val animateFrom = interruptionResult.animateFrom
                val animateFrom = interruptionResult.animateFrom
                if (
                if (
@@ -159,6 +158,7 @@ private fun CoroutineScope.animate(
    val transition =
    val transition =
        if (reversed) {
        if (reversed) {
            OneOffTransition(
            OneOffTransition(
                key = transitionKey,
                fromScene = targetScene,
                fromScene = targetScene,
                toScene = fromScene,
                toScene = fromScene,
                currentScene = targetScene,
                currentScene = targetScene,
@@ -167,6 +167,7 @@ private fun CoroutineScope.animate(
            )
            )
        } else {
        } else {
            OneOffTransition(
            OneOffTransition(
                key = transitionKey,
                fromScene = fromScene,
                fromScene = fromScene,
                toScene = targetScene,
                toScene = targetScene,
                currentScene = targetScene,
                currentScene = targetScene,
@@ -178,7 +179,7 @@ private fun CoroutineScope.animate(
    // Change the current layout state to start this new transition. This will compute the
    // Change the current layout state to start this new transition. This will compute the
    // TransformationSpec associated to this transition, which we need to initialize the Animatable
    // TransformationSpec associated to this transition, which we need to initialize the Animatable
    // that will actually animate it.
    // that will actually animate it.
    layoutState.startTransition(transition, transitionKey, chain)
    layoutState.startTransition(transition, chain)


    // The transition now contains the transformation spec that we should use to instantiate the
    // The transition now contains the transformation spec that we should use to instantiate the
    // Animatable.
    // Animatable.
@@ -207,6 +208,7 @@ private fun CoroutineScope.animate(
}
}


private class OneOffTransition(
private class OneOffTransition(
    override val key: TransitionKey?,
    fromScene: SceneKey,
    fromScene: SceneKey,
    toScene: SceneKey,
    toScene: SceneKey,
    override val currentScene: SceneKey,
    override val currentScene: SceneKey,
+2 −2
Original line number Original line Diff line number Diff line
@@ -257,7 +257,7 @@ private class DragControllerImpl(


    fun updateTransition(newTransition: SwipeTransition, force: Boolean = false) {
    fun updateTransition(newTransition: SwipeTransition, force: Boolean = false) {
        if (isDrivingTransition || force) {
        if (isDrivingTransition || force) {
            layoutState.startTransition(newTransition, newTransition.key)
            layoutState.startTransition(newTransition)
        }
        }


        swipeTransition = newTransition
        swipeTransition = newTransition
@@ -555,7 +555,7 @@ private class SwipeTransition(
    val layoutImpl: SceneTransitionLayoutImpl,
    val layoutImpl: SceneTransitionLayoutImpl,
    val layoutState: BaseSceneTransitionLayoutState,
    val layoutState: BaseSceneTransitionLayoutState,
    val coroutineScope: CoroutineScope,
    val coroutineScope: CoroutineScope,
    val key: TransitionKey?,
    override val key: TransitionKey?,
    val _fromScene: Scene,
    val _fromScene: Scene,
    val _toScene: Scene,
    val _toScene: Scene,
    val userActionDistanceScope: UserActionDistanceScope,
    val userActionDistanceScope: UserActionDistanceScope,
+109 −26
Original line number Original line Diff line number Diff line
@@ -280,7 +280,7 @@ internal class ElementNode(
        constraints: Constraints,
        constraints: Constraints,
    ): MeasureResult {
    ): MeasureResult {
        val transitions = currentTransitions
        val transitions = currentTransitions
        val transition = elementTransition(element, transitions)
        val transition = elementTransition(layoutImpl, element, transitions)


        // If this element is not supposed to be laid out now, either because it is not part of any
        // If this element is not supposed to be laid out now, either because it is not part of any
        // ongoing transition or the other scene of its transition is overscrolling, then lay out
        // ongoing transition or the other scene of its transition is overscrolling, then lay out
@@ -318,13 +318,10 @@ internal class ElementNode(
            val targetOffsetInScene = lookaheadScopeCoordinates.localLookaheadPositionOf(coords)
            val targetOffsetInScene = lookaheadScopeCoordinates.localLookaheadPositionOf(coords)


            // No need to place the element in this scene if we don't want to draw it anyways.
            // No need to place the element in this scene if we don't want to draw it anyways.
            if (!shouldPlaceElement(layoutImpl, scene, element, transition)) {
            if (!shouldPlaceElement(layoutImpl, scene.key, element, transition)) {
                sceneState.lastOffset = Offset.Unspecified
                sceneState.lastOffset = Offset.Unspecified
                sceneState.lastScale = Scale.Unspecified
                sceneState.lastScale = Scale.Unspecified
                sceneState.lastAlpha = Element.AlphaUnspecified
                sceneState.lastAlpha = Element.AlphaUnspecified

                sceneState.clearValuesBeforeInterruption()
                sceneState.clearInterruptionDeltas()
                return
                return
            }
            }


@@ -353,7 +350,17 @@ internal class ElementNode(
                    getValueBeforeInterruption = { sceneState.offsetBeforeInterruption },
                    getValueBeforeInterruption = { sceneState.offsetBeforeInterruption },
                    setValueBeforeInterruption = { sceneState.offsetBeforeInterruption = it },
                    setValueBeforeInterruption = { sceneState.offsetBeforeInterruption = it },
                    getInterruptionDelta = { sceneState.offsetInterruptionDelta },
                    getInterruptionDelta = { sceneState.offsetInterruptionDelta },
                    setInterruptionDelta = { sceneState.offsetInterruptionDelta = it },
                    setInterruptionDelta = { delta ->
                        setPlacementInterruptionDelta(
                            element = element,
                            sceneState = sceneState,
                            transition = transition,
                            delta = delta,
                            setter = { sceneState, delta ->
                                sceneState.offsetInterruptionDelta = delta
                            },
                        )
                    },
                    diff = { a, b -> a - b },
                    diff = { a, b -> a - b },
                    add = { a, b, bProgress -> a + b * bProgress },
                    add = { a, b, bProgress -> a + b * bProgress },
                )
                )
@@ -363,7 +370,7 @@ internal class ElementNode(
            val offset = (interruptedOffset - currentOffset).round()
            val offset = (interruptedOffset - currentOffset).round()
            if (
            if (
                isElementOpaque(scene, element, transition) &&
                isElementOpaque(scene, element, transition) &&
                    interruptedAlpha(layoutImpl, transition, sceneState, alpha = 1f) == 1f
                    interruptedAlpha(layoutImpl, element, transition, sceneState, alpha = 1f) == 1f
            ) {
            ) {
                sceneState.lastAlpha = 1f
                sceneState.lastAlpha = 1f


@@ -374,13 +381,17 @@ internal class ElementNode(
            } else {
            } else {
                placeable.placeWithLayer(offset) {
                placeable.placeWithLayer(offset) {
                    // This layer might still run on its own (outside of the placement phase) even
                    // This layer might still run on its own (outside of the placement phase) even
                    // if this element is not placed anymore, so we need to double check again here
                    // if this element is not placed or composed anymore, so we need to double check
                    // before calling [elementAlpha] (which will update [SceneState.lastAlpha]). We
                    // again here before calling [elementAlpha] (which will update
                    // also need to recompute the current transition to make sure that we are using
                    // [SceneState.lastAlpha]). We also need to recompute the current transition to
                    // the current transition and not a reference to an old one. See b/343138966 for
                    // make sure that we are using the current transition and not a reference to an
                    // details.
                    // old one. See b/343138966 for details.
                    val transition = elementTransition(element, currentTransitions)
                    if (_element == null) {
                    if (!shouldPlaceElement(layoutImpl, scene, element, transition)) {
                        return@placeWithLayer
                    }

                    val transition = elementTransition(layoutImpl, element, currentTransitions)
                    if (!shouldPlaceElement(layoutImpl, scene.key, element, transition)) {
                        return@placeWithLayer
                        return@placeWithLayer
                    }
                    }


@@ -394,7 +405,7 @@ internal class ElementNode(
    override fun ContentDrawScope.draw() {
    override fun ContentDrawScope.draw() {
        element.wasDrawnInAnyScene = true
        element.wasDrawnInAnyScene = true


        val transition = elementTransition(element, currentTransitions)
        val transition = elementTransition(layoutImpl, element, currentTransitions)
        val drawScale = getDrawScale(layoutImpl, scene, element, transition, sceneState)
        val drawScale = getDrawScale(layoutImpl, scene, element, transition, sceneState)
        if (drawScale == Scale.Default) {
        if (drawScale == Scale.Default) {
            drawContent()
            drawContent()
@@ -435,6 +446,7 @@ internal class ElementNode(
 * its scenes contains the element.
 * its scenes contains the element.
 */
 */
private fun elementTransition(
private fun elementTransition(
    layoutImpl: SceneTransitionLayoutImpl,
    element: Element,
    element: Element,
    transitions: List<TransitionState.Transition>,
    transitions: List<TransitionState.Transition>,
): TransitionState.Transition? {
): TransitionState.Transition? {
@@ -448,7 +460,7 @@ private fun elementTransition(


    if (transition != previousTransition && transition != null && previousTransition != null) {
    if (transition != previousTransition && transition != null && previousTransition != null) {
        // The previous transition was interrupted by another transition.
        // The previous transition was interrupted by another transition.
        prepareInterruption(element, transition, previousTransition)
        prepareInterruption(layoutImpl, element, transition, previousTransition)
    } else if (transition == null && previousTransition != null) {
    } else if (transition == null && previousTransition != null) {
        // The transition was just finished.
        // The transition was just finished.
        element.sceneStates.values.forEach {
        element.sceneStates.values.forEach {
@@ -461,18 +473,43 @@ private fun elementTransition(
}
}


private fun prepareInterruption(
private fun prepareInterruption(
    layoutImpl: SceneTransitionLayoutImpl,
    element: Element,
    element: Element,
    transition: TransitionState.Transition,
    transition: TransitionState.Transition,
    previousTransition: TransitionState.Transition,
    previousTransition: TransitionState.Transition,
) {
) {
    val sceneStates = element.sceneStates
    val sceneStates = element.sceneStates
    sceneStates[previousTransition.fromScene]?.selfUpdateValuesBeforeInterruption()
    fun updatedSceneState(key: SceneKey): Element.SceneState? {
    sceneStates[previousTransition.toScene]?.selfUpdateValuesBeforeInterruption()
        return sceneStates[key]?.also { it.selfUpdateValuesBeforeInterruption() }
    sceneStates[transition.fromScene]?.selfUpdateValuesBeforeInterruption()
    }
    sceneStates[transition.toScene]?.selfUpdateValuesBeforeInterruption()

    val previousFromState = updatedSceneState(previousTransition.fromScene)
    val previousToState = updatedSceneState(previousTransition.toScene)
    val fromState = updatedSceneState(transition.fromScene)
    val toState = updatedSceneState(transition.toScene)


    reconcileStates(element, previousTransition)
    reconcileStates(element, previousTransition)
    reconcileStates(element, transition)
    reconcileStates(element, transition)

    // Remove the interruption values to all scenes but the scene(s) where the element will be
    // placed, to make sure that interruption deltas are computed only right after this interruption
    // is prepared.
    fun maybeCleanPlacementValuesBeforeInterruption(sceneState: Element.SceneState) {
        if (!shouldPlaceElement(layoutImpl, sceneState.scene, element, transition)) {
            sceneState.offsetBeforeInterruption = Offset.Unspecified
            sceneState.alphaBeforeInterruption = Element.AlphaUnspecified
            sceneState.scaleBeforeInterruption = Scale.Unspecified

            sceneState.offsetInterruptionDelta = Offset.Zero
            sceneState.alphaInterruptionDelta = 0f
            sceneState.scaleInterruptionDelta = Scale.Zero
        }
    }

    previousFromState?.let { maybeCleanPlacementValuesBeforeInterruption(it) }
    previousToState?.let { maybeCleanPlacementValuesBeforeInterruption(it) }
    fromState?.let { maybeCleanPlacementValuesBeforeInterruption(it) }
    toState?.let { maybeCleanPlacementValuesBeforeInterruption(it) }
}
}


/**
/**
@@ -579,9 +616,38 @@ private inline fun <T> computeInterruptedValue(
    }
    }
}
}


/**
 * Set the interruption delta of a *placement/drawing*-related value (offset, alpha, scale). This
 * ensures that the delta is also set on the other scene in the transition for shared elements, so
 * that there is no jump cut if the scene where the element is placed has changed.
 */
private inline fun <T> setPlacementInterruptionDelta(
    element: Element,
    sceneState: Element.SceneState,
    transition: TransitionState.Transition?,
    delta: T,
    setter: (Element.SceneState, T) -> Unit,
) {
    // Set the interruption delta on the current scene.
    setter(sceneState, delta)

    if (transition == null) {
        return
    }

    // If the element is shared, also set the delta on the other scene so that it is used by that
    // scene if we start overscrolling it and change the scene where the element is placed.
    val otherScene =
        if (sceneState.scene == transition.fromScene) transition.toScene else transition.fromScene
    val otherSceneState = element.sceneStates[otherScene] ?: return
    if (isSharedElementEnabled(element.key, transition)) {
        setter(otherSceneState, delta)
    }
}

private fun shouldPlaceElement(
private fun shouldPlaceElement(
    layoutImpl: SceneTransitionLayoutImpl,
    layoutImpl: SceneTransitionLayoutImpl,
    scene: Scene,
    scene: SceneKey,
    element: Element,
    element: Element,
    transition: TransitionState.Transition?,
    transition: TransitionState.Transition?,
): Boolean {
): Boolean {
@@ -592,7 +658,7 @@ private fun shouldPlaceElement(


    // Don't place the element in this scene if this scene is not part of the current element
    // Don't place the element in this scene if this scene is not part of the current element
    // transition.
    // transition.
    if (scene.key != transition.fromScene && scene.key != transition.toScene) {
    if (scene != transition.fromScene && scene != transition.toScene) {
        return false
        return false
    }
    }


@@ -610,7 +676,7 @@ private fun shouldPlaceElement(


    return shouldPlaceOrComposeSharedElement(
    return shouldPlaceOrComposeSharedElement(
        layoutImpl,
        layoutImpl,
        scene.key,
        scene,
        element.key,
        element.key,
        transition,
        transition,
    )
    )
@@ -740,13 +806,14 @@ private fun elementAlpha(
        element.sceneStates.forEach { it.value.alphaBeforeInterruption = 0f }
        element.sceneStates.forEach { it.value.alphaBeforeInterruption = 0f }
    }
    }


    val interruptedAlpha = interruptedAlpha(layoutImpl, transition, sceneState, alpha)
    val interruptedAlpha = interruptedAlpha(layoutImpl, element, transition, sceneState, alpha)
    sceneState.lastAlpha = interruptedAlpha
    sceneState.lastAlpha = interruptedAlpha
    return interruptedAlpha
    return interruptedAlpha
}
}


private fun interruptedAlpha(
private fun interruptedAlpha(
    layoutImpl: SceneTransitionLayoutImpl,
    layoutImpl: SceneTransitionLayoutImpl,
    element: Element,
    transition: TransitionState.Transition?,
    transition: TransitionState.Transition?,
    sceneState: Element.SceneState,
    sceneState: Element.SceneState,
    alpha: Float,
    alpha: Float,
@@ -760,7 +827,15 @@ private fun interruptedAlpha(
        getValueBeforeInterruption = { sceneState.alphaBeforeInterruption },
        getValueBeforeInterruption = { sceneState.alphaBeforeInterruption },
        setValueBeforeInterruption = { sceneState.alphaBeforeInterruption = it },
        setValueBeforeInterruption = { sceneState.alphaBeforeInterruption = it },
        getInterruptionDelta = { sceneState.alphaInterruptionDelta },
        getInterruptionDelta = { sceneState.alphaInterruptionDelta },
        setInterruptionDelta = { sceneState.alphaInterruptionDelta = it },
        setInterruptionDelta = { delta ->
            setPlacementInterruptionDelta(
                element = element,
                sceneState = sceneState,
                transition = transition,
                delta = delta,
                setter = { sceneState, delta -> sceneState.alphaInterruptionDelta = delta },
            )
        },
        diff = { a, b -> a - b },
        diff = { a, b -> a - b },
        add = { a, b, bProgress -> a + b * bProgress },
        add = { a, b, bProgress -> a + b * bProgress },
    )
    )
@@ -867,7 +942,15 @@ private fun ContentDrawScope.getDrawScale(
            getValueBeforeInterruption = { sceneState.scaleBeforeInterruption },
            getValueBeforeInterruption = { sceneState.scaleBeforeInterruption },
            setValueBeforeInterruption = { sceneState.scaleBeforeInterruption = it },
            setValueBeforeInterruption = { sceneState.scaleBeforeInterruption = it },
            getInterruptionDelta = { sceneState.scaleInterruptionDelta },
            getInterruptionDelta = { sceneState.scaleInterruptionDelta },
            setInterruptionDelta = { sceneState.scaleInterruptionDelta = it },
            setInterruptionDelta = { delta ->
                setPlacementInterruptionDelta(
                    element = element,
                    sceneState = sceneState,
                    transition = transition,
                    delta = delta,
                    setter = { sceneState, delta -> sceneState.scaleInterruptionDelta = delta },
                )
            },
            diff = { a, b ->
            diff = { a, b ->
                Scale(
                Scale(
                    scaleX = a.scaleX - b.scaleX,
                    scaleX = a.scaleX - b.scaleX,
+12 −7
Original line number Original line Diff line number Diff line
@@ -225,6 +225,12 @@ sealed interface TransitionState {
        /** The scene this transition is going to. Can't be the same as fromScene */
        /** The scene this transition is going to. Can't be the same as fromScene */
        val toScene: SceneKey,
        val toScene: SceneKey,
    ) : TransitionState {
    ) : TransitionState {
        /**
         * The key of this transition. This should usually be null, but it can be specified to use a
         * specific set of transformations associated to this transition.
         */
        open val key: TransitionKey? = null

        /**
        /**
         * The progress of the transition. This is usually in the `[0; 1]` range, but it can also be
         * The progress of the transition. This is usually in the `[0; 1]` range, but it can also be
         * less than `0` or greater than `1` when using transitions with a spring AnimationSpec or
         * less than `0` or greater than `1` when using transitions with a spring AnimationSpec or
@@ -455,11 +461,7 @@ internal abstract class BaseSceneTransitionLayoutState(
     *
     *
     * Important: you *must* call [finishTransition] once the transition is finished.
     * Important: you *must* call [finishTransition] once the transition is finished.
     */
     */
    internal fun startTransition(
    internal fun startTransition(transition: TransitionState.Transition, chain: Boolean = true) {
        transition: TransitionState.Transition,
        transitionKey: TransitionKey? = null,
        chain: Boolean = true,
    ) {
        checkThread()
        checkThread()


        // Compute the [TransformationSpec] when the transition starts.
        // Compute the [TransformationSpec] when the transition starts.
@@ -469,7 +471,9 @@ internal abstract class BaseSceneTransitionLayoutState(


        // Update the transition specs.
        // Update the transition specs.
        transition.transformationSpec =
        transition.transformationSpec =
            transitions.transitionSpec(fromScene, toScene, key = transitionKey).transformationSpec()
            transitions
                .transitionSpec(fromScene, toScene, key = transition.key)
                .transformationSpec()
        if (orientation != null) {
        if (orientation != null) {
            transition.updateOverscrollSpecs(
            transition.updateOverscrollSpecs(
                fromSpec = transitions.overscrollSpec(fromScene, orientation),
                fromSpec = transitions.overscrollSpec(fromScene, orientation),
@@ -568,9 +572,10 @@ internal abstract class BaseSceneTransitionLayoutState(
                    originalTransition = transitionState,
                    originalTransition = transitionState,
                    fromScene = targetCurrentScene,
                    fromScene = targetCurrentScene,
                    toScene = matchingLink.targetTo,
                    toScene = matchingLink.targetTo,
                    key = matchingLink.targetTransitionKey,
                )
                )


            stateLink.target.startTransition(linkedTransition, matchingLink.targetTransitionKey)
            stateLink.target.startTransition(linkedTransition)
            activeTransitionLinks[stateLink] = linkedTransition
            activeTransitionLinks[stateLink] = linkedTransition
        }
        }
    }
    }
+2 −0
Original line number Original line Diff line number Diff line
@@ -17,6 +17,7 @@
package com.android.compose.animation.scene.transition.link
package com.android.compose.animation.scene.transition.link


import com.android.compose.animation.scene.SceneKey
import com.android.compose.animation.scene.SceneKey
import com.android.compose.animation.scene.TransitionKey
import com.android.compose.animation.scene.TransitionState
import com.android.compose.animation.scene.TransitionState
import kotlinx.coroutines.Job
import kotlinx.coroutines.Job


@@ -25,6 +26,7 @@ internal class LinkedTransition(
    private val originalTransition: TransitionState.Transition,
    private val originalTransition: TransitionState.Transition,
    fromScene: SceneKey,
    fromScene: SceneKey,
    toScene: SceneKey,
    toScene: SceneKey,
    override val key: TransitionKey? = null,
) : TransitionState.Transition(fromScene, toScene) {
) : TransitionState.Transition(fromScene, toScene) {


    override val currentScene: SceneKey
    override val currentScene: SceneKey
Loading