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

Commit 4b9cd743 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Refactor animate(Scene|Element)FooAsState (1/2)

This CL refactors animate(Scene|Element)FooAsState so that it does not
need to access the associated Element during composition. This should
allow the simplification of Modifier.element itself and remove all calls
to Snapshot.withoutReadObservation {}.

A major side effect of this CL is that reading the returned State value
during composition will now throw an exception. This is a good thing,
because animated values should never be read during composition anyways,
and before this CL we could get invalid values that were 1 frame behind.

Bug: 317026105
Test: AnimatedSharedAsStateTest
Flag: N/A
Change-Id: I20d247e9cc6a8f48e8f611421e5b3d56fc091dba
parent c49e33d0
Loading
Loading
Loading
Loading
+124 −41
Original line number Diff line number Diff line
@@ -17,15 +17,40 @@
package com.android.compose.animation.scene

import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.Stable
import androidx.compose.runtime.State
import androidx.compose.runtime.remember
import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.lerp
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.lerp
import com.android.compose.ui.util.lerp

/**
 * A [State] whose [value] is animated.
 *
 * Important: This animated value should always be ready *after* composition, e.g. during layout,
 * drawing or inside a LaunchedEffect. If you read [value] during composition, it will probably
 * throw an exception, for 2 important reasons:
 * 1. You should never read animated values during composition, because this will probably lead to
 *    bad performance.
 * 2. Given that this value depends on the target value in different scenes, its current value
 *    (depending on the current transition state) can only be computed once the full tree has been
 *    composed.
 *
 * If you don't have the choice and *have to* get the value during composition, for instance because
 * a Modifier or Composable reading this value does not have a lazy/lambda-based API, then you can
 * access [valueOrNull] and use a fallback value for the first frame where this animated value can
 * not be computed yet. Note however that doing so will be bad for performance and might lead to
 * late-by-one-frame flickers.
 */
@Stable
interface AnimatedState<out T> : State<T> {
    val valueOrNull: T?
}

/**
 * Animate a scene Int value.
 *
@@ -36,7 +61,7 @@ fun SceneScope.animateSceneIntAsState(
    value: Int,
    key: ValueKey,
    canOverflow: Boolean = true,
): State<Int> {
): AnimatedState<Int> {
    return animateSceneValueAsState(value, key, ::lerp, canOverflow)
}

@@ -50,7 +75,7 @@ fun ElementScope<*>.animateElementIntAsState(
    value: Int,
    key: ValueKey,
    canOverflow: Boolean = true,
): State<Int> {
): AnimatedState<Int> {
    return animateElementValueAsState(value, key, ::lerp, canOverflow)
}

@@ -64,7 +89,7 @@ fun SceneScope.animateSceneFloatAsState(
    value: Float,
    key: ValueKey,
    canOverflow: Boolean = true,
): State<Float> {
): AnimatedState<Float> {
    return animateSceneValueAsState(value, key, ::lerp, canOverflow)
}

@@ -78,7 +103,7 @@ fun ElementScope<*>.animateElementFloatAsState(
    value: Float,
    key: ValueKey,
    canOverflow: Boolean = true,
): State<Float> {
): AnimatedState<Float> {
    return animateElementValueAsState(value, key, ::lerp, canOverflow)
}

@@ -92,7 +117,7 @@ fun SceneScope.animateSceneDpAsState(
    value: Dp,
    key: ValueKey,
    canOverflow: Boolean = true,
): State<Dp> {
): AnimatedState<Dp> {
    return animateSceneValueAsState(value, key, ::lerp, canOverflow)
}

@@ -106,7 +131,7 @@ fun ElementScope<*>.animateElementDpAsState(
    value: Dp,
    key: ValueKey,
    canOverflow: Boolean = true,
): State<Dp> {
): AnimatedState<Dp> {
    return animateElementValueAsState(value, key, ::lerp, canOverflow)
}

@@ -119,7 +144,7 @@ fun ElementScope<*>.animateElementDpAsState(
fun SceneScope.animateSceneColorAsState(
    value: Color,
    key: ValueKey,
): State<Color> {
): AnimatedState<Color> {
    return animateSceneValueAsState(value, key, ::lerp, canOverflow = false)
}

@@ -132,7 +157,7 @@ fun SceneScope.animateSceneColorAsState(
fun ElementScope<*>.animateElementColorAsState(
    value: Color,
    key: ValueKey,
): State<Color> {
): AnimatedState<Color> {
    return animateElementValueAsState(value, key, ::lerp, canOverflow = false)
}

@@ -145,38 +170,79 @@ internal fun <T> animateSharedValueAsState(
    value: T,
    lerp: (T, T, Float) -> T,
    canOverflow: Boolean,
): State<T> {
    val sharedValue =
        Snapshot.withoutReadObservation {
            val sharedValues =
                element?.sceneValues?.getValue(scene.key)?.sharedValues ?: scene.sharedValues
            sharedValues.getOrPut(key) { Element.SharedValue(key, value) } as Element.SharedValue<T>
): AnimatedState<T> {
    // Create the associated SharedValue object that holds the current value.
    DisposableEffect(scene, element, key) {
        val sharedValues = sharedValues(scene, element)
        sharedValues[key] = Element.SharedValue(key, value)
        onDispose { sharedValues.remove(key) }
    }

    if (value != sharedValue.value) {
        sharedValue.value = value
    }
    // Update the current value. Note that side effects run after disposable effects, so we know
    // that the SharedValue object was created at this point.
    SideEffect { sharedValue<T>(scene, element, key).value = value }

    return remember(layoutImpl, element, sharedValue, lerp, canOverflow) {
        object : State<T> {
    val sceneKey = scene.key
    return remember(layoutImpl, sceneKey, element, lerp, canOverflow) {
        object : AnimatedState<T> {
            override val value: T
                get() = computeValue(layoutImpl, element, sharedValue, lerp, canOverflow)
                get() = value(layoutImpl, sceneKey, element, key, lerp, canOverflow)

            override val valueOrNull: T?
                get() = valueOrNull(layoutImpl, sceneKey, element, key, lerp, canOverflow)
        }
    }
}

private fun <T> computeValue(
private fun sharedValues(
    scene: Scene,
    element: Element?,
): MutableMap<ValueKey, Element.SharedValue<*>> {
    return element?.sceneValues?.getValue(scene.key)?.sharedValues ?: scene.sharedValues
}

private fun <T> sharedValueOrNull(
    scene: Scene,
    element: Element?,
    key: ValueKey,
): Element.SharedValue<T>? {
    val sharedValue = sharedValues(scene, element)[key] ?: return null
    return sharedValue as Element.SharedValue<T>
}

private fun <T> sharedValue(
    scene: Scene,
    element: Element?,
    key: ValueKey,
): Element.SharedValue<T> {
    return sharedValueOrNull(scene, element, key) ?: error(valueReadTooEarlyMessage(key))
}

private fun valueReadTooEarlyMessage(key: ValueKey) =
    "Animated value $key was read before its target values were set. This probably " +
        "means that you are reading it during composition, which you should not do. See the " +
        "documentation of AnimatedState for more information."

private fun <T> value(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: SceneKey,
    element: Element?,
    sharedValue: Element.SharedValue<T>,
    key: ValueKey,
    lerp: (T, T, Float) -> T,
    canOverflow: Boolean,
): T {
    val transition = layoutImpl.state.currentTransition
    if (transition == null || !layoutImpl.isTransitionReady(transition)) {
        return sharedValue.value
    return valueOrNull(layoutImpl, scene, element, key, lerp, canOverflow)
        ?: error(valueReadTooEarlyMessage(key))
}

private fun <T> valueOrNull(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: SceneKey,
    element: Element?,
    key: ValueKey,
    lerp: (T, T, Float) -> T,
    canOverflow: Boolean,
): T? {
    fun sceneValue(scene: SceneKey): Element.SharedValue<T>? {
        val sharedValues =
            if (element == null) {
@@ -185,21 +251,38 @@ private fun <T> computeValue(
                element.sceneValues[scene]?.sharedValues
            }
                ?: return null
        val value = sharedValues[sharedValue.key] ?: return null
        val value = sharedValues[key] ?: return null
        return value as Element.SharedValue<T>
    }

    return when (val transition = layoutImpl.state.transitionState) {
        is TransitionState.Idle -> sceneValue(transition.currentScene)?.value
        is TransitionState.Transition -> {
            // Note: no need to check for transition ready here given that all target values are
            // defined during composition, we should already have the correct values to interpolate
            // between here.
            val fromValue = sceneValue(transition.fromScene)
            val toValue = sceneValue(transition.toScene)
    return if (fromValue != null && toValue != null) {
            if (fromValue != null && toValue != null) {
                val from = fromValue.value
                val to = toValue.value
                if (from == to) {
                    // Optimization: avoid reading progress if the values are the same, so we don't
                    // relayout/redraw for nothing.
                    from
                } else {
                    val progress =
            if (canOverflow) transition.progress else transition.progress.coerceIn(0f, 1f)
        lerp(fromValue.value, toValue.value, progress)
                        if (canOverflow) transition.progress
                        else transition.progress.coerceIn(0f, 1f)
                    lerp(from, to, progress)
                }
            } else if (fromValue != null) {
                fromValue.value
    } else if (toValue != null) {
        toValue.value
    } else {
        sharedValue.value
            } else toValue?.value
        }
    }
    // TODO(b/311600838): Remove this. We should not have to fallback to the current scene value,
    // but we have to because code of removed nodes can still run if they are placed with a graphics
    // layer.
    ?: sceneValue(scene)?.value
}
+5 −1
Original line number Diff line number Diff line
@@ -100,7 +100,11 @@ internal class Element(val key: ElementKey) {
        var targetSize by mutableStateOf(SizeUnspecified)
        var targetOffset by mutableStateOf(Offset.Unspecified)

        val sharedValues = SnapshotStateMap<ValueKey, SharedValue<*>>()
        private var _sharedValues: MutableMap<ValueKey, SharedValue<*>>? = null
        val sharedValues: MutableMap<ValueKey, SharedValue<*>>
            get() =
                _sharedValues
                    ?: SnapshotStateMap<ValueKey, SharedValue<*>>().also { _sharedValues = it }

        /**
         * The attached [ElementNode] a Modifier.element() for a given element and scene. During
+1 −2
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package com.android.compose.animation.scene

import androidx.compose.foundation.layout.Box
import androidx.compose.runtime.Composable
import androidx.compose.runtime.State
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
@@ -102,7 +101,7 @@ private class ElementScopeImpl<ContentScope>(
        key: ValueKey,
        lerp: (start: T, stop: T, fraction: Float) -> T,
        canOverflow: Boolean
    ): State<T> {
    ): AnimatedState<T> {
        return animateSharedValueAsState(layoutImpl, scene, element, key, value, lerp, canOverflow)
    }

+6 −3
Original line number Diff line number Diff line
@@ -20,7 +20,6 @@ import androidx.compose.foundation.gestures.Orientation
import androidx.compose.foundation.layout.Box
import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import androidx.compose.runtime.State
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableFloatStateOf
import androidx.compose.runtime.mutableStateOf
@@ -52,7 +51,11 @@ internal class Scene(
    var targetSize by mutableStateOf(IntSize.Zero)

    /** The shared values in this scene that are not tied to a specific element. */
    val sharedValues = SnapshotStateMap<ValueKey, Element.SharedValue<*>>()
    private var _sharedValues: MutableMap<ValueKey, Element.SharedValue<*>>? = null
    val sharedValues: MutableMap<ValueKey, Element.SharedValue<*>>
        get() =
            _sharedValues
                ?: SnapshotStateMap<ValueKey, Element.SharedValue<*>>().also { _sharedValues = it }

    @Composable
    @OptIn(ExperimentalComposeUiApi::class)
@@ -110,7 +113,7 @@ internal class SceneScopeImpl(
        key: ValueKey,
        lerp: (T, T, Float) -> T,
        canOverflow: Boolean
    ): State<T> {
    ): AnimatedState<T> {
        return animateSharedValueAsState(
            layoutImpl = layoutImpl,
            scene = scene,
+2 −3
Original line number Diff line number Diff line
@@ -22,7 +22,6 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.Stable
import androidx.compose.runtime.State
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.Modifier
@@ -245,7 +244,7 @@ interface SceneScope : BaseSceneScope {
        key: ValueKey,
        lerp: (start: T, stop: T, fraction: Float) -> T,
        canOverflow: Boolean,
    ): State<T>
    ): AnimatedState<T>
}

@Stable
@@ -272,7 +271,7 @@ interface ElementScope<ContentScope> {
        key: ValueKey,
        lerp: (start: T, stop: T, fraction: Float) -> T,
        canOverflow: Boolean,
    ): State<T>
    ): AnimatedState<T>

    /**
     * The content of this element.
Loading