diff options
7 files changed, 234 insertions, 36 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 9d9b0a9fe496..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 @@ -16,6 +16,7 @@ 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 @@ -66,12 +67,21 @@ internal class Element(val key: ElementKey) { * The movable content of this element, if this element is composed using * [SceneScope.MovableElement]. */ - val movableContent by - // This is only accessed from the composition (main) thread, so no need to use the default - // lock of lazy {} to synchronize. - lazy(mode = LazyThreadSafetyMode.NONE) { - movableContentOf { content: @Composable () -> Unit -> content() } - } + private var _movableContent: (@Composable (@Composable () -> Unit) -> Unit)? = null + val movableContent: @Composable (@Composable () -> Unit) -> Unit + get() = + _movableContent + ?: 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)" @@ -521,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 @@ -548,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/MovableElement.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MovableElement.kt index 306f27626e19..49df2f6b6062 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 @@ -16,7 +16,6 @@ package com.android.compose.animation.scene -import android.graphics.Picture import android.util.Log import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Spacer @@ -60,7 +59,7 @@ internal fun MovableElement( // 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 = remember { Picture() } + 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 diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Scene.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Scene.kt index 6a7a3a00d4fe..30e50a972230 100644 --- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Scene.kt +++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/Scene.kt @@ -34,6 +34,7 @@ import androidx.compose.ui.layout.intermediateLayout import androidx.compose.ui.platform.testTag import androidx.compose.ui.unit.IntSize import androidx.compose.ui.zIndex +import com.android.compose.animation.scene.modifiers.noResizeDuringTransitions /** A scene in a [SceneTransitionLayout]. */ @Stable @@ -152,4 +153,8 @@ private class SceneScopeImpl( bounds: ElementKey, shape: Shape ): Modifier = punchHole(layoutImpl, element, bounds, shape) + + override fun Modifier.noResizeDuringTransitions(): Modifier { + return noResizeDuringTransitions(layoutState = layoutImpl.state) + } } 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 63fe9e98764d..5eb339e4a5e4 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, ) @@ -205,6 +205,12 @@ interface SceneScope { * the result. */ fun Modifier.punchHole(element: ElementKey, bounds: ElementKey, shape: Shape): Modifier + + /** + * Don't resize during transitions. This can for instance be used to make sure that scrollable + * lists keep a constant size during transitions even if its elements are growing/shrinking. + */ + fun Modifier.noResizeDuringTransitions(): Modifier } // TODO(b/291053742): Add animateSharedValueAsState(targetValue) without any ValueKey and ElementKey @@ -257,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/src/com/android/compose/animation/scene/modifiers/Size.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/modifiers/Size.kt new file mode 100644 index 000000000000..bd36cb8655ac --- /dev/null +++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/modifiers/Size.kt @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.compose.animation.scene.modifiers + +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.intermediateLayout +import androidx.compose.ui.unit.Constraints +import com.android.compose.animation.scene.SceneTransitionLayoutState + +@OptIn(ExperimentalComposeUiApi::class) +internal fun Modifier.noResizeDuringTransitions(layoutState: SceneTransitionLayoutState): Modifier { + return intermediateLayout { measurable, constraints -> + if (layoutState.currentTransition == null) { + return@intermediateLayout measurable.measure(constraints).run { + layout(width, height) { place(0, 0) } + } + } + + // Make sure that this layout node has the same size than when we are at rest. + val sizeAtRest = lookaheadSize + measurable.measure(Constraints.fixed(sizeAtRest.width, sizeAtRest.height)).run { + layout(width, height) { place(0, 0) } + } + } +} 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) + } + } + } } diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/modifiers/SizeTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/modifiers/SizeTest.kt new file mode 100644 index 000000000000..2c159d15fa66 --- /dev/null +++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/modifiers/SizeTest.kt @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.compose.animation.scene.modifiers + +import androidx.compose.animation.core.LinearEasing +import androidx.compose.animation.core.tween +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.unit.dp +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.android.compose.animation.scene.TestElements +import com.android.compose.animation.scene.element +import com.android.compose.animation.scene.testTransition +import com.android.compose.test.assertSizeIsEqualTo +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class SizeTest { + @get:Rule val rule = createComposeRule() + + @Test + fun noResizeDuringTransitions() { + // The tag for the parent of the shared Foo element. + val parentTag = "parent" + + rule.testTransition( + fromSceneContent = { Box(Modifier.element(TestElements.Foo).size(100.dp)) }, + toSceneContent = { + // Don't resize the parent of Foo during transitions so that it's always the same + // size as when there is no transition (200dp). + Box(Modifier.noResizeDuringTransitions().testTag(parentTag)) { + Box(Modifier.element(TestElements.Foo).size(200.dp)) + } + }, + transition = { spec = tween(durationMillis = 4 * 16, easing = LinearEasing) }, + ) { + at(16) { rule.onNodeWithTag(parentTag).assertSizeIsEqualTo(200.dp, 200.dp) } + at(32) { rule.onNodeWithTag(parentTag).assertSizeIsEqualTo(200.dp, 200.dp) } + at(48) { rule.onNodeWithTag(parentTag).assertSizeIsEqualTo(200.dp, 200.dp) } + after { rule.onNodeWithTag(parentTag).assertSizeIsEqualTo(200.dp, 200.dp) } + } + } +} |