diff options
3 files changed, 103 insertions, 28 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 4194750c1da6..a85d9bff283e 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 @@ -531,11 +531,6 @@ private fun IntermediateMeasureScope.place( sceneValues.targetOffset = targetOffsetInScene } - // No need to place the element in this scene if we don't want to draw it anyways. - if (!shouldDrawElement(layoutImpl, scene, element)) { - return - } - val currentOffset = lookaheadScopeCoordinates.localPositionOf(coords, Offset.Zero) val lastSharedValues = element.lastSharedValues val lastValues = sceneValues.lastValues @@ -558,6 +553,13 @@ private fun IntermediateMeasureScope.place( lastSharedValues.offset = targetOffset lastValues.offset = targetOffset + // No need to place the element in this scene if we don't want to draw it anyways. Note that + // it's still important to compute the target offset and update lastValues, otherwise it + // will be out of date. + if (!shouldDrawElement(layoutImpl, scene, element)) { + return + } + val offset = (targetOffset - currentOffset).round() if (isElementOpaque(layoutImpl, element, scene, sceneValues)) { // TODO(b/291071158): Call placeWithLayer() if offset != IntOffset.Zero and size is not diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayout.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayout.kt index d455aa2ed5a0..7c31ba358dbd 100644 --- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayout.kt +++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayout.kt @@ -65,11 +65,11 @@ fun SceneTransitionLayout( SceneTransitionLayoutForTesting( currentScene, onChangeScene, + modifier, transitions, state, edgeDetector, transitionInterceptionThreshold, - modifier, onLayoutImpl = null, scenes, ) @@ -263,12 +263,12 @@ enum class SwipeDirection(val orientation: Orientation) { internal fun SceneTransitionLayoutForTesting( currentScene: SceneKey, onChangeScene: (SceneKey) -> Unit, - transitions: SceneTransitions, - state: SceneTransitionLayoutState, - edgeDetector: EdgeDetector, - transitionInterceptionThreshold: Float, - modifier: Modifier, - onLayoutImpl: ((SceneTransitionLayoutImpl) -> Unit)?, + modifier: Modifier = Modifier, + transitions: SceneTransitions = transitions {}, + state: SceneTransitionLayoutState = remember { SceneTransitionLayoutState(currentScene) }, + edgeDetector: EdgeDetector = DefaultEdgeDetector, + transitionInterceptionThreshold: Float = 0f, + onLayoutImpl: ((SceneTransitionLayoutImpl) -> Unit)? = null, scenes: SceneTransitionLayoutScope.() -> Unit, ) { val density = LocalDensity.current 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 d332910c54b1..da5a0a04ed63 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 @@ -16,6 +16,7 @@ package com.android.compose.animation.scene +import androidx.compose.animation.core.LinearEasing import androidx.compose.animation.core.tween import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.layout.Box @@ -30,16 +31,21 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.SideEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset import androidx.compose.ui.layout.intermediateLayout +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.unit.Density import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.DpOffset import androidx.compose.ui.unit.dp import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.android.compose.test.subjects.DpOffsetSubject +import com.android.compose.test.subjects.assertThat import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -259,11 +265,6 @@ class ElementTest { SceneTransitionLayoutForTesting( currentScene = currentScene, onChangeScene = { currentScene = it }, - transitions = remember { transitions {} }, - state = remember { SceneTransitionLayoutState(currentScene) }, - edgeDetector = DefaultEdgeDetector, - modifier = Modifier, - transitionInterceptionThreshold = 0f, onLayoutImpl = { nullableLayoutImpl = it }, ) { scene(TestScenes.SceneA) { /* Nothing */} @@ -429,11 +430,6 @@ class ElementTest { SceneTransitionLayoutForTesting( currentScene = TestScenes.SceneA, onChangeScene = {}, - transitions = remember { transitions {} }, - state = remember { SceneTransitionLayoutState(TestScenes.SceneA) }, - edgeDetector = DefaultEdgeDetector, - modifier = Modifier, - transitionInterceptionThreshold = 0f, onLayoutImpl = { nullableLayoutImpl = it }, ) { scene(TestScenes.SceneA) { Box(Modifier.element(key)) } @@ -484,11 +480,6 @@ class ElementTest { SceneTransitionLayoutForTesting( currentScene = TestScenes.SceneA, onChangeScene = {}, - transitions = remember { transitions {} }, - state = remember { SceneTransitionLayoutState(TestScenes.SceneA) }, - edgeDetector = DefaultEdgeDetector, - modifier = Modifier, - transitionInterceptionThreshold = 0f, onLayoutImpl = { nullableLayoutImpl = it }, ) { scene(TestScenes.SceneA) { @@ -574,4 +565,86 @@ class ElementTest { after { assertThat(fooCompositions).isEqualTo(1) } } } + + @Test + fun sharedElementOffsetIsUpdatedEvenWhenNotPlaced() { + var nullableLayoutImpl: SceneTransitionLayoutImpl? = null + var density: Density? = null + + fun layoutImpl() = nullableLayoutImpl ?: error("nullableLayoutImpl was not set") + + fun density() = density ?: error("density was not set") + + fun Offset.toDpOffset() = with(density()) { DpOffset(x.toDp(), y.toDp()) } + + fun foo() = layoutImpl().elements[TestElements.Foo] ?: error("Foo not in elements map") + + fun Element.lastSharedOffset() = lastSharedValues.offset.toDpOffset() + + fun Element.lastOffsetIn(scene: SceneKey) = + (sceneValues[scene] ?: error("$scene not in sceneValues map")) + .lastValues + .offset + .toDpOffset() + + rule.testTransition( + from = TestScenes.SceneA, + to = TestScenes.SceneB, + transitionLayout = { currentScene, onChangeScene -> + density = LocalDensity.current + + SceneTransitionLayoutForTesting( + currentScene = currentScene, + onChangeScene = onChangeScene, + onLayoutImpl = { nullableLayoutImpl = it }, + transitions = + transitions { + from(TestScenes.SceneA, to = TestScenes.SceneB) { + spec = tween(durationMillis = 4 * 16, easing = LinearEasing) + } + } + ) { + scene(TestScenes.SceneA) { Box(Modifier.element(TestElements.Foo)) } + scene(TestScenes.SceneB) { + Box(Modifier.offset(x = 40.dp, y = 80.dp).element(TestElements.Foo)) + } + } + } + ) { + val tolerance = DpOffsetSubject.DefaultTolerance + + before { + val expected = DpOffset(0.dp, 0.dp) + assertThat(foo().lastSharedOffset()).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneA)).isWithin(tolerance).of(expected) + } + + at(16) { + val expected = DpOffset(10.dp, 20.dp) + assertThat(foo().lastSharedOffset()).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneA)).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneB)).isWithin(tolerance).of(expected) + } + + at(32) { + val expected = DpOffset(20.dp, 40.dp) + assertThat(foo().lastSharedOffset()).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneA)).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneB)).isWithin(tolerance).of(expected) + } + + at(48) { + val expected = DpOffset(30.dp, 60.dp) + assertThat(foo().lastSharedOffset()).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneA)).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneB)).isWithin(tolerance).of(expected) + } + + after { + val expected = DpOffset(40.dp, 80.dp) + assertThat(foo().lastSharedOffset()).isWithin(tolerance).of(expected) + assertThat(foo().lastOffsetIn(TestScenes.SceneB)).isWithin(tolerance).of(expected) + } + } + } } |