Extract movable contents outside of the Element class
This CL moves the Maps and objects that are used to move the content of
movable elements outside of the Element class. That way, composing a
MovableElement won't require the Element object, which will allow to
remove the last calls to Snapshot.withoutReadObservation {} and map
mutations during composition inside Modifier.element().
Test: MovableElementTest
Bug: 291071158
Flag: N/A
Change-Id: I5cd94d6685d8640650ccfd28bdbd2de1867a5373
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt
index f4138a5..d2b34ad 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Element.kt
@@ -16,10 +16,8 @@
package com.android.compose.animation.scene
-import androidx.compose.runtime.Composable
import androidx.compose.runtime.Stable
import androidx.compose.runtime.getValue
-import androidx.compose.runtime.movableContentOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshots.Snapshot
@@ -61,17 +59,6 @@
/** The mapping between a scene and the state this element has in that scene, if any. */
val sceneStates = SnapshotStateMap<SceneKey, SceneState>()
- /**
- * The movable content of this element, if this element is composed using
- * [SceneScope.MovableElement].
- */
- private var _movableContent: (@Composable (@Composable () -> Unit) -> Unit)? = null
- val movableContent: @Composable (@Composable () -> Unit) -> Unit
- get() =
- _movableContent
- ?: movableContentOf { content: @Composable () -> Unit -> content() }
- .also { _movableContent = it }
-
override fun toString(): String {
return "Element(key=$key)"
}
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MovableElement.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MovableElement.kt
index ed48c63f..9402d84 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MovableElement.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MovableElement.kt
@@ -20,8 +20,8 @@
import androidx.compose.runtime.Composable
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
+import androidx.compose.runtime.movableContentOf
import androidx.compose.runtime.remember
-import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.Modifier
import androidx.compose.ui.layout.Layout
import androidx.compose.ui.unit.IntSize
@@ -75,13 +75,9 @@
isMovable: Boolean,
) {
Box(modifier.element(layoutImpl, scene, key)) {
- // Get the Element from the map. It will always be the same and we don't want to recompose
- // every time an element is added/removed from SceneTransitionLayoutImpl.elements, so we
- // disable read observation during the look-up in that map.
- val element = Snapshot.withoutReadObservation { layoutImpl.elements.getValue(key) }
val elementScope =
- remember(layoutImpl, element, scene, contentScope, isMovable) {
- ElementScopeImpl(layoutImpl, element, scene, contentScope, isMovable)
+ remember(layoutImpl, key, scene, contentScope, isMovable) {
+ ElementScopeImpl(layoutImpl, key, scene, contentScope, isMovable)
}
elementScope.content()
@@ -90,7 +86,7 @@
private class ElementScopeImpl<ContentScope>(
private val layoutImpl: SceneTransitionLayoutImpl,
- private val element: Element,
+ private val element: ElementKey,
private val scene: Scene,
private val contentScope: ContentScope,
private val isMovable: Boolean,
@@ -105,7 +101,7 @@
return animateSharedValueAsState(
layoutImpl,
scene.key,
- element.key,
+ element,
key,
value,
lerp,
@@ -132,7 +128,12 @@
}
if (shouldComposeMovableElement) {
- element.movableContent { contentScope.content() }
+ val movableContent: MovableElementContent =
+ layoutImpl.movableContents[element]
+ ?: movableContentOf { content: @Composable () -> Unit -> content() }
+ .also { layoutImpl.movableContents[element] = it }
+
+ movableContent { contentScope.content() }
} else {
// 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,
@@ -140,7 +141,12 @@
// layout pass.
Layout { _, _ ->
// No need to measure or place anything.
- val size = placeholderContentSize(layoutImpl, scene.key, element)
+ val size =
+ placeholderContentSize(
+ layoutImpl,
+ scene.key,
+ layoutImpl.elements.getValue(element),
+ )
layout(size.width, size.height) {}
}
}
@@ -150,7 +156,7 @@
private fun shouldComposeMovableElement(
layoutImpl: SceneTransitionLayoutImpl,
scene: SceneKey,
- element: Element,
+ element: ElementKey,
): Boolean {
val transition =
layoutImpl.state.currentTransition
@@ -179,7 +185,7 @@
layoutImpl,
transition,
scene,
- element.key,
+ element,
)
}
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt
index c56202c..4da9c3d 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt
@@ -36,6 +36,8 @@
import com.android.compose.ui.util.lerp
import kotlinx.coroutines.CoroutineScope
+internal typealias MovableElementContent = @Composable (@Composable () -> Unit) -> Unit
+
@Stable
internal class SceneTransitionLayoutImpl(
internal val state: SceneTransitionLayoutStateImpl,
@@ -62,6 +64,20 @@
internal val elements = SnapshotStateMap<ElementKey, Element>()
/**
+ * The map of contents of movable elements.
+ *
+ * Note that given that this map is mutated directly during a composition, it has to be a
+ * [SnapshotStateMap] to make sure that mutations are reverted if composition is cancelled.
+ */
+ private var _movableContents: SnapshotStateMap<ElementKey, MovableElementContent>? = null
+ val movableContents: SnapshotStateMap<ElementKey, MovableElementContent>
+ get() =
+ _movableContents
+ ?: SnapshotStateMap<ElementKey, MovableElementContent>().also {
+ _movableContents = it
+ }
+
+ /**
* The different values of a shared value keyed by a a [ValueKey] and the different elements and
* scenes it is associated to.
*/