diff options
4 files changed, 103 insertions, 10 deletions
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 980982a30926..0bb07603c069 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 @@ -39,6 +39,8 @@ import androidx.compose.ui.layout.MeasureScope import androidx.compose.ui.layout.Placeable import androidx.compose.ui.node.DrawModifierNode import androidx.compose.ui.node.ModifierNodeElement +import androidx.compose.ui.node.TraversableNode +import androidx.compose.ui.node.traverseDescendants import androidx.compose.ui.platform.testTag import androidx.compose.ui.unit.Constraints import androidx.compose.ui.unit.IntSize @@ -165,7 +167,7 @@ internal class ElementNode( private var currentTransitions: List<TransitionState.Transition>, private var scene: Scene, private var key: ElementKey, -) : Modifier.Node(), DrawModifierNode, ApproachLayoutModifierNode { +) : Modifier.Node(), DrawModifierNode, ApproachLayoutModifierNode, TraversableNode { private var _element: Element? = null private val element: Element get() = _element!! @@ -174,6 +176,8 @@ internal class ElementNode( private val sceneState: Element.SceneState get() = _sceneState!! + override val traverseKey: Any = ElementTraverseKey + override fun onAttach() { super.onAttach() updateElementAndSceneValues() @@ -289,9 +293,7 @@ internal class ElementNode( val isOtherSceneOverscrolling = overscrollScene != null && overscrollScene != scene.key val isNotPartOfAnyOngoingTransitions = transitions.isNotEmpty() && transition == null if (isNotPartOfAnyOngoingTransitions || isOtherSceneOverscrolling) { - sceneState.lastOffset = Offset.Unspecified - sceneState.lastScale = Scale.Unspecified - sceneState.lastAlpha = Element.AlphaUnspecified + recursivelyClearPlacementValues() val placeable = measurable.measure(constraints) sceneState.lastSize = placeable.size() @@ -319,9 +321,7 @@ internal class ElementNode( // No need to place the element in this scene if we don't want to draw it anyways. if (!shouldPlaceElement(layoutImpl, scene.key, element, transition)) { - sceneState.lastOffset = Offset.Unspecified - sceneState.lastScale = Scale.Unspecified - sceneState.lastAlpha = Element.AlphaUnspecified + recursivelyClearPlacementValues() return } @@ -402,6 +402,25 @@ internal class ElementNode( } } + /** + * Recursively clear the last placement values on this node and all descendants ElementNodes. + * This should be called when this node is not placed anymore, so that we correctly clear values + * for the descendants for which approachMeasure() won't be called. + */ + private fun recursivelyClearPlacementValues() { + fun Element.SceneState.clearLastPlacementValues() { + lastOffset = Offset.Unspecified + lastScale = Scale.Unspecified + lastAlpha = Element.AlphaUnspecified + } + + sceneState.clearLastPlacementValues() + traverseDescendants(ElementTraverseKey) { node -> + (node as ElementNode).sceneState.clearLastPlacementValues() + TraversableNode.Companion.TraverseDescendantsAction.ContinueTraversal + } + } + override fun ContentDrawScope.draw() { element.wasDrawnInAnyScene = true @@ -421,6 +440,8 @@ internal class ElementNode( } companion object { + private val ElementTraverseKey = Any() + private fun maybePruneMaps( layoutImpl: SceneTransitionLayoutImpl, element: Element, diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt index 6a178c8b0c25..a8df6f4ae8b1 100644 --- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt +++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutState.kt @@ -768,7 +768,7 @@ internal class HoistedSceneTransitionLayoutState( /** A [MutableSceneTransitionLayoutState] that holds the value for the current scene. */ internal class MutableSceneTransitionLayoutStateImpl( initialScene: SceneKey, - override var transitions: SceneTransitions, + override var transitions: SceneTransitions = transitions {}, private val canChangeScene: (SceneKey) -> Boolean = { true }, stateLinks: List<StateLink> = emptyList(), enableInterruptions: Boolean = DEFAULT_INTERRUPTIONS_ENABLED, diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt index 41cacb4c71fc..45880bc3134a 100644 --- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt +++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/ElementTest.kt @@ -1719,4 +1719,76 @@ class ElementTest { rule.onNode(isElement(TestElements.Foo, SceneB)).assertIsNotDisplayed() rule.onNode(isElement(TestElements.Foo, SceneC)).assertPositionInRootIsEqualTo(40.dp, 40.dp) } + + @Test + fun lastPlacementValuesAreClearedOnNestedElements() { + val state = rule.runOnIdle { MutableSceneTransitionLayoutStateImpl(SceneA) } + + @Composable + fun SceneScope.NestedFooBar() { + Box(Modifier.element(TestElements.Foo)) { + Box(Modifier.element(TestElements.Bar).size(10.dp)) + } + } + + lateinit var layoutImpl: SceneTransitionLayoutImpl + rule.setContent { + SceneTransitionLayoutForTesting(state, onLayoutImpl = { layoutImpl = it }) { + scene(SceneA) { NestedFooBar() } + scene(SceneB) { NestedFooBar() } + } + } + + // Idle on A: composed and placed only in B. + rule.onNode(isElement(TestElements.Foo, SceneA)).assertIsDisplayed() + rule.onNode(isElement(TestElements.Bar, SceneA)).assertIsDisplayed() + rule.onNode(isElement(TestElements.Foo, SceneB)).assertDoesNotExist() + rule.onNode(isElement(TestElements.Bar, SceneB)).assertDoesNotExist() + + assertThat(layoutImpl.elements).containsKey(TestElements.Foo) + assertThat(layoutImpl.elements).containsKey(TestElements.Bar) + val foo = layoutImpl.elements.getValue(TestElements.Foo) + val bar = layoutImpl.elements.getValue(TestElements.Bar) + + assertThat(foo.sceneStates).containsKey(SceneA) + assertThat(bar.sceneStates).containsKey(SceneA) + assertThat(foo.sceneStates).doesNotContainKey(SceneB) + assertThat(bar.sceneStates).doesNotContainKey(SceneB) + + val fooInA = foo.sceneStates.getValue(SceneA) + val barInA = bar.sceneStates.getValue(SceneA) + assertThat(fooInA.lastOffset).isNotEqualTo(Offset.Unspecified) + assertThat(fooInA.lastAlpha).isNotEqualTo(Element.AlphaUnspecified) + assertThat(fooInA.lastScale).isNotEqualTo(Scale.Unspecified) + + assertThat(barInA.lastOffset).isNotEqualTo(Offset.Unspecified) + assertThat(barInA.lastAlpha).isNotEqualTo(Element.AlphaUnspecified) + assertThat(barInA.lastScale).isNotEqualTo(Scale.Unspecified) + + // A => B: composed in both and placed only in B. + rule.runOnUiThread { state.startTransition(transition(from = SceneA, to = SceneB)) } + rule.onNode(isElement(TestElements.Foo, SceneA)).assertExists().assertIsNotDisplayed() + rule.onNode(isElement(TestElements.Bar, SceneA)).assertExists().assertIsNotDisplayed() + rule.onNode(isElement(TestElements.Foo, SceneB)).assertIsDisplayed() + rule.onNode(isElement(TestElements.Bar, SceneB)).assertIsDisplayed() + + assertThat(foo.sceneStates).containsKey(SceneB) + assertThat(bar.sceneStates).containsKey(SceneB) + + val fooInB = foo.sceneStates.getValue(SceneB) + val barInB = bar.sceneStates.getValue(SceneB) + assertThat(fooInA.lastOffset).isEqualTo(Offset.Unspecified) + assertThat(fooInA.lastAlpha).isEqualTo(Element.AlphaUnspecified) + assertThat(fooInA.lastScale).isEqualTo(Scale.Unspecified) + assertThat(fooInB.lastOffset).isNotEqualTo(Offset.Unspecified) + assertThat(fooInB.lastAlpha).isNotEqualTo(Element.AlphaUnspecified) + assertThat(fooInB.lastScale).isNotEqualTo(Scale.Unspecified) + + assertThat(barInA.lastOffset).isEqualTo(Offset.Unspecified) + assertThat(barInA.lastAlpha).isEqualTo(Element.AlphaUnspecified) + assertThat(barInA.lastScale).isEqualTo(Scale.Unspecified) + assertThat(barInB.lastOffset).isNotEqualTo(Offset.Unspecified) + assertThat(barInB.lastAlpha).isNotEqualTo(Element.AlphaUnspecified) + assertThat(barInB.lastScale).isNotEqualTo(Scale.Unspecified) + } } diff --git a/packages/SystemUI/compose/scene/tests/utils/src/com/android/compose/animation/scene/TestMatchers.kt b/packages/SystemUI/compose/scene/tests/utils/src/com/android/compose/animation/scene/TestMatchers.kt index e743c7885c14..6d063a0418d6 100644 --- a/packages/SystemUI/compose/scene/tests/utils/src/com/android/compose/animation/scene/TestMatchers.kt +++ b/packages/SystemUI/compose/scene/tests/utils/src/com/android/compose/animation/scene/TestMatchers.kt @@ -17,7 +17,7 @@ package com.android.compose.animation.scene import androidx.compose.ui.test.SemanticsMatcher -import androidx.compose.ui.test.hasParent +import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasTestTag /** A [SemanticsMatcher] that matches [element], optionally restricted to scene [scene]. */ @@ -25,6 +25,6 @@ fun isElement(element: ElementKey, scene: SceneKey? = null): SemanticsMatcher { return if (scene == null) { hasTestTag(element.testTag) } else { - hasTestTag(element.testTag) and hasParent(hasTestTag(scene.testTag)) + hasTestTag(element.testTag) and hasAnyAncestor(hasTestTag(scene.testTag)) } } |