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

Commit bf11c3f7 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Compose movable elements in the scene picked by the ScenePicker (1/2)

This CL simplifies the logic of MovableElement so that we always compose
a MovableElement in the scene returned by the associated ScenePicker.

Doing this can sometimes break the animation of a movable element if it
was never compose in a scene before the transition, as explained in
b/317026105#comment1. I will add more utilities to help work around this
limitation in a follow-up CL.

Bug: 317026105
Test: PlatformComposeSceneTransitionLayoutTests
Flag: N/A
Change-Id: Icf4ab837b51cc1321ba7e1bd502bbebebd3a2eac
parent 40e53a42
Loading
Loading
Loading
Loading
+1 −13
Original line number Diff line number Diff line
@@ -16,7 +16,6 @@

package com.android.compose.animation.scene

import android.graphics.Picture
import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import androidx.compose.runtime.getValue
@@ -74,15 +73,6 @@ internal class Element(val key: ElementKey) {
                ?: movableContentOf { content: @Composable () -> Unit -> content() }
                    .also { _movableContent = it }

    /**
     * The [Picture] to which we save the last drawing commands of this element, if it is movable.
     * This is necessary because the content of this element might not be composed in the scene it
     * should currently be drawn.
     */
    private var _picture: Picture? = null
    val picture: Picture
        get() = _picture ?: Picture().also { _picture = it }

    override fun toString(): String {
        return "Element(key=$key)"
    }
@@ -325,9 +315,7 @@ internal fun shouldDrawOrComposeSharedElement(

    return scenePicker.sceneDuringTransition(
        element = element,
        fromScene = fromScene,
        toScene = toScene,
        progress = transition::progress,
        transition = transition,
        fromSceneZIndex = layoutImpl.scenes.getValue(fromScene).zIndex,
        toSceneZIndex = layoutImpl.scenes.getValue(toScene).zIndex,
    ) == scene
+54 −95
Original line number Diff line number Diff line
@@ -16,9 +16,7 @@

package com.android.compose.animation.scene

import android.util.Log
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Spacer
import androidx.compose.runtime.Composable
import androidx.compose.runtime.State
import androidx.compose.runtime.derivedStateOf
@@ -26,18 +24,9 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.drawBehind
import androidx.compose.ui.draw.drawWithCache
import androidx.compose.ui.graphics.Canvas
import androidx.compose.ui.graphics.drawscope.draw
import androidx.compose.ui.graphics.drawscope.drawIntoCanvas
import androidx.compose.ui.graphics.nativeCanvas
import androidx.compose.ui.layout.layout
import androidx.compose.ui.unit.Constraints
import androidx.compose.ui.layout.Layout
import androidx.compose.ui.unit.IntSize

private const val TAG = "MovableElement"

@Composable
internal fun Element(
    layoutImpl: SceneTransitionLayoutImpl,
@@ -124,60 +113,29 @@ private class ElementScopeImpl<ContentScope>(
            return
        }

        // The [Picture] to which we save the last drawing commands of this element. This is
        // necessary because the content of this element might not be composed in this scene, in
        // which case we still need to draw it.
        val picture = element.picture

        // Whether we should compose the movable element here. The scene picker logic to know in
        // which scene we should compose/draw a movable element might depend on the current
        // transition progress, so we put this in a derivedStateOf to prevent many recompositions
        // during the transition.
        // TODO(b/317026105): Use derivedStateOf only if the scene picker reads the progress in its
        // logic.
        val shouldComposeMovableElement by
            remember(layoutImpl, scene.key, element) {
                derivedStateOf { shouldComposeMovableElement(layoutImpl, scene.key, element) }
            }

        if (shouldComposeMovableElement) {
            Box(
                Modifier.drawWithCache {
                    val width = size.width.toInt()
                    val height = size.height.toInt()

                    onDrawWithContent {
                        // Save the draw commands into [picture] for later to draw the last content
                        // even when this movable content is not composed.
                        val pictureCanvas = Canvas(picture.beginRecording(width, height))
                        draw(this, this.layoutDirection, pictureCanvas, this.size) {
                            this@onDrawWithContent.drawContent()
                        }
                        picture.endRecording()

                        // Draw the content.
                        drawIntoCanvas { canvas -> canvas.nativeCanvas.drawPicture(picture) }
                    }
                }
            ) {
            element.movableContent { contentScope.content() }
            }
        } else {
            // If we are not composed, we draw the previous drawing commands at the same size as the
            // movable content when it was composed in this scene.
            val sceneValues = element.sceneValues.getValue(scene.key)

            Spacer(
                Modifier.layout { measurable, _ ->
                        val size =
                            sceneValues.targetSize.takeIf { it != Element.SizeUnspecified }
                                ?: IntSize.Zero
                        val placeable =
                            measurable.measure(Constraints.fixed(size.width, size.height))
                        layout(size.width, size.height) { placeable.place(0, 0) }
                    }
                    .drawBehind {
                        drawIntoCanvas { canvas -> canvas.nativeCanvas.drawPicture(picture) }
            // If we are not composed, we still need to lay out an empty space with the same *target
            // size* as its movable content, i.e. the same *size when idle*. During transitions,
            // this size will be used to interpolate the transition size, during the intermediate
            // layout pass.
            Layout { _, _ ->
                // No need to measure or place anything.
                val size = placeholderContentSize(layoutImpl, scene.key, element)
                layout(size.width, size.height) {}
            }
            )
        }
    }
}
@@ -198,54 +156,55 @@ private fun shouldComposeMovableElement(
    val fromReady = layoutImpl.isSceneReady(fromScene)
    val toReady = layoutImpl.isSceneReady(toScene)

    val otherScene =
        when (scene) {
            fromScene -> toScene
            toScene -> fromScene
            else ->
                error(
                    "shouldComposeMovableElement(scene=$scene) called with fromScene=$fromScene " +
                        "and toScene=$toScene"
                )
    if (!fromReady && !toReady) {
        // Neither of the scenes will be drawn, so where we compose it doesn't really matter. Note
        // that we could have slightly more complicated logic here to optimize for this case, but
        // it's not worth it given that readyScenes should disappear soon (b/316901148).
        return scene == toScene
    }

    val isShared = otherScene in element.sceneValues
    // If one of the scenes is not ready, compose it in the other one to make sure it is drawn.
    if (!fromReady) return scene == toScene
    if (!toReady) return scene == fromScene

    if (isShared && !toReady && !fromReady) {
        // This should usually not happen given that fromScene should be ready, but let's log a
        // warning here in case it does so it helps debugging flicker issues caused by this part of
        // the code.
        Log.w(
            TAG,
            "MovableElement $element might have to be composed for the first time in both " +
                "fromScene=$fromScene and toScene=$toScene. This will probably lead to a flicker " +
                "where the size of the element will jump from IntSize.Zero to its actual size " +
                "during the transition."
    // Always compose movable elements in the scene picked by their scene picker.
    return shouldDrawOrComposeSharedElement(
        layoutImpl,
        transition,
        scene,
        element.key,
    )
}

    // Element is not shared in this transition.
    if (!isShared) {
        return true
/**
 * Return the size of the placeholder/space that is composed when the movable content is not
 * composed in a scene.
 */
private fun placeholderContentSize(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: SceneKey,
    element: Element,
): IntSize {
    // If the content of the movable element was already composed in this scene before, use that
    // target size.
    val targetValueInScene = element.sceneValues.getValue(scene).targetSize
    if (targetValueInScene != Element.SizeUnspecified) {
        return targetValueInScene
    }

    // toScene is not ready (because we are composing it for the first time), so we compose it there
    // first. This is the most common scenario when starting a transition that has a shared movable
    // element.
    if (!toReady) {
        return scene == toScene
    }
    // This code is only run during transitions (otherwise the content would be composed and the
    // placeholder would not), so it's ok to cast the state into a Transition directly.
    val transition = layoutImpl.state.transitionState as TransitionState.Transition

    // This should usually not happen, but if we are also composing for the first time in fromScene
    // then we should compose it there only.
    if (!fromReady) {
        return scene == fromScene
    // If the content was already composed in the other scene, we use that target size assuming it
    // doesn't change between scenes.
    // TODO(b/317026105): Provide a way to give a hint size/content for cases where this is not
    // true.
    val otherScene = if (transition.fromScene == scene) transition.toScene else transition.fromScene
    val targetValueInOtherScene = element.sceneValues[otherScene]?.targetSize
    if (targetValueInOtherScene != null && targetValueInOtherScene != Element.SizeUnspecified) {
        return targetValueInOtherScene
    }

    return shouldDrawOrComposeSharedElement(
        layoutImpl,
        transition,
        scene,
        element.key,
    )
    return IntSize.Zero
}
+17 −4
Original line number Diff line number Diff line
@@ -92,6 +92,20 @@ sealed interface TransitionState {

        /** Whether user input is currently driving the transition. */
        abstract val isUserInputOngoing: Boolean

        /**
         * Whether we are transitioning. If [from] or [to] is empty, we will also check that they
         * match the scenes we are animating from and/or to.
         */
        fun isTransitioning(from: SceneKey? = null, to: SceneKey? = null): Boolean {
            return (from == null || fromScene == from) && (to == null || toScene == to)
        }

        /** Whether we are transitioning from [scene] to [other], or from [other] to [scene]. */
        fun isTransitioningBetween(scene: SceneKey, other: SceneKey): Boolean {
            return isTransitioning(from = scene, to = other) ||
                isTransitioning(from = other, to = scene)
        }
    }
}

@@ -111,13 +125,12 @@ internal class SceneTransitionLayoutStateImpl(

    override fun isTransitioning(from: SceneKey?, to: SceneKey?): Boolean {
        val transition = currentTransition ?: return false
        return (from == null || transition.fromScene == from) &&
            (to == null || transition.toScene == to)
        return transition.isTransitioning(from, to)
    }

    override fun isTransitioningBetween(scene: SceneKey, other: SceneKey): Boolean {
        return isTransitioning(from = scene, to = other) ||
            isTransitioning(from = other, to = scene)
        val transition = currentTransition ?: return false
        return transition.isTransitioningBetween(scene, other)
    }

    /** Start a new [transition], instantly interrupting any ongoing transition if there was one. */
+45 −10
Original line number Diff line number Diff line
@@ -133,25 +133,60 @@ interface TransitionBuilder : PropertyTransformationBuilder {
interface SharedElementScenePicker {
    /**
     * Return the scene in which [element] should be drawn (when using `Modifier.element(key)`) or
     * composed (when using `MovableElement(key)`) during the transition from [fromScene] to
     * [toScene].
     * composed (when using `MovableElement(key)`) during the given [transition].
     */
    fun sceneDuringTransition(
        element: ElementKey,
        fromScene: SceneKey,
        toScene: SceneKey,
        progress: () -> Float,
        transition: TransitionState.Transition,
        fromSceneZIndex: Float,
        toSceneZIndex: Float,
    ): SceneKey

    /**
     * Return [transition.fromScene] if it is in [scenes] and [transition.toScene] is not, or return
     * [transition.toScene] if it is in [scenes] and [transition.fromScene] is not, otherwise throw
     * an exception (i.e. if neither or both of fromScene and toScene are in [scenes]).
     *
     * This function can be useful when computing the scene in which a movable element should be
     * composed.
     */
    fun pickSingleSceneIn(
        scenes: Set<SceneKey>,
        transition: TransitionState.Transition,
        element: ElementKey,
    ): SceneKey {
        val fromScene = transition.fromScene
        val toScene = transition.toScene
        val fromSceneInScenes = scenes.contains(fromScene)
        val toSceneInScenes = scenes.contains(toScene)
        if (fromSceneInScenes && toSceneInScenes) {
            error(
                "Element $element can be in both $fromScene and $toScene. You should add a " +
                    "special case for this transition before calling pickSingleSceneIn()."
            )
        }

        if (!fromSceneInScenes && !toSceneInScenes) {
            error(
                "Element $element can be neither in $fromScene and $toScene. This either means " +
                    "that you should add one of them in the scenes set passed to " +
                    "pickSingleSceneIn(), or there is an internal error and this element was " +
                    "composed when it shouldn't be."
            )
        }

        return if (fromSceneInScenes) {
            fromScene
        } else {
            toScene
        }
    }
}

object DefaultSharedElementScenePicker : SharedElementScenePicker {
    override fun sceneDuringTransition(
        element: ElementKey,
        fromScene: SceneKey,
        toScene: SceneKey,
        progress: () -> Float,
        transition: TransitionState.Transition,
        fromSceneZIndex: Float,
        toSceneZIndex: Float
    ): SceneKey {
@@ -161,9 +196,9 @@ object DefaultSharedElementScenePicker : SharedElementScenePicker {
            (fromSceneZIndex > toSceneZIndex && !element.isBackground) ||
                (fromSceneZIndex < toSceneZIndex && element.isBackground)
        ) {
            fromScene
            transition.fromScene
        } else {
            toScene
            transition.toScene
        }
    }
}
+4 −6
Original line number Diff line number Diff line
@@ -149,20 +149,18 @@ class MovableElementTest {
                    object : SharedElementScenePicker {
                        override fun sceneDuringTransition(
                            element: ElementKey,
                            fromScene: SceneKey,
                            toScene: SceneKey,
                            progress: () -> Float,
                            transition: TransitionState.Transition,
                            fromSceneZIndex: Float,
                            toSceneZIndex: Float
                        ): SceneKey {
                            assertThat(fromScene).isEqualTo(TestScenes.SceneA)
                            assertThat(toScene).isEqualTo(TestScenes.SceneB)
                            assertThat(transition.fromScene).isEqualTo(TestScenes.SceneA)
                            assertThat(transition.toScene).isEqualTo(TestScenes.SceneB)
                            assertThat(fromSceneZIndex).isEqualTo(0)
                            assertThat(toSceneZIndex).isEqualTo(1)

                            // Compose Foo in Scene A if progress < 0.65f, otherwise compose it
                            // in Scene B.
                            return if (progress() < 0.65f) {
                            return if (transition.progress < 0.65f) {
                                TestScenes.SceneA
                            } else {
                                TestScenes.SceneB