diff options
7 files changed, 174 insertions, 33 deletions
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt index 48edded5df18..de54e75281aa 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/interactor/SceneInteractorTest.kt @@ -575,4 +575,50 @@ class SceneInteractorTest : SysuiTestCase() { assertThat(currentScene).isNotEqualTo(disabledScene) } + + @Test + fun transitionAnimations() = + kosmos.runTest { + val isVisible by collectLastValue(underTest.isVisible) + assertThat(isVisible).isTrue() + + underTest.setVisible(false, "test") + assertThat(isVisible).isFalse() + + underTest.onTransitionAnimationStart() + // One animation is active, forced visible. + assertThat(isVisible).isTrue() + + underTest.onTransitionAnimationEnd() + // No more active animations, not forced visible. + assertThat(isVisible).isFalse() + + underTest.onTransitionAnimationStart() + // One animation is active, forced visible. + assertThat(isVisible).isTrue() + + underTest.onTransitionAnimationCancelled() + // No more active animations, not forced visible. + assertThat(isVisible).isFalse() + + underTest.setVisible(true, "test") + assertThat(isVisible).isTrue() + + underTest.onTransitionAnimationStart() + underTest.onTransitionAnimationStart() + // Two animations are active, forced visible. + assertThat(isVisible).isTrue() + + underTest.setVisible(false, "test") + // Two animations are active, forced visible. + assertThat(isVisible).isTrue() + + underTest.onTransitionAnimationEnd() + // One animation is still active, forced visible. + assertThat(isVisible).isTrue() + + underTest.onTransitionAnimationEnd() + // No more active animations, not forced visible. + assertThat(isVisible).isFalse() + } } diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/startable/SceneContainerStartableTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/startable/SceneContainerStartableTest.kt index 06dd046564df..51f056aa18da 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/startable/SceneContainerStartableTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/scene/domain/startable/SceneContainerStartableTest.kt @@ -36,6 +36,8 @@ import com.android.keyguard.AuthInteractionProperties import com.android.keyguard.keyguardUpdateMonitor import com.android.systemui.Flags import com.android.systemui.SysuiTestCase +import com.android.systemui.animation.ActivityTransitionAnimator +import com.android.systemui.animation.activityTransitionAnimator import com.android.systemui.authentication.data.repository.FakeAuthenticationRepository import com.android.systemui.authentication.data.repository.fakeAuthenticationRepository import com.android.systemui.authentication.domain.interactor.authenticationInteractor @@ -141,6 +143,7 @@ import org.mockito.Mockito.never import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.whenever @SmallTest @@ -169,6 +172,7 @@ class SceneContainerStartableTest : SysuiTestCase() { private val uiEventLoggerFake = kosmos.uiEventLoggerFake private val msdlPlayer = kosmos.fakeMSDLPlayer private val authInteractionProperties = AuthInteractionProperties() + private val mockActivityTransitionAnimator = mock<ActivityTransitionAnimator>() private lateinit var underTest: SceneContainerStartable @@ -177,6 +181,8 @@ class SceneContainerStartableTest : SysuiTestCase() { MockitoAnnotations.initMocks(this) whenever(kosmos.keyguardUpdateMonitor.isUnlockingWithBiometricAllowed(anyBoolean())) .thenReturn(true) + kosmos.activityTransitionAnimator = mockActivityTransitionAnimator + underTest = kosmos.sceneContainerStartable } @@ -2716,6 +2722,27 @@ class SceneContainerStartableTest : SysuiTestCase() { assertThat(currentOverlays).isEmpty() } + @Test + fun hydrateActivityTransitionAnimationState() = + kosmos.runTest { + underTest.start() + + val isVisible by collectLastValue(sceneInteractor.isVisible) + assertThat(isVisible).isTrue() + + sceneInteractor.setVisible(false, "reason") + assertThat(isVisible).isFalse() + + val argumentCaptor = argumentCaptor<ActivityTransitionAnimator.Listener>() + verify(mockActivityTransitionAnimator).addListener(argumentCaptor.capture()) + + val listeners = argumentCaptor.allValues + listeners.forEach { it.onTransitionAnimationStart() } + assertThat(isVisible).isTrue() + listeners.forEach { it.onTransitionAnimationEnd() } + assertThat(isVisible).isFalse() + } + private fun TestScope.emulateSceneTransition( transitionStateFlow: MutableStateFlow<ObservableTransitionState>, toScene: SceneKey, diff --git a/packages/SystemUI/src/com/android/systemui/scene/data/repository/SceneContainerRepository.kt b/packages/SystemUI/src/com/android/systemui/scene/data/repository/SceneContainerRepository.kt index d60f05e685bb..0488962fd076 100644 --- a/packages/SystemUI/src/com/android/systemui/scene/data/repository/SceneContainerRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/scene/data/repository/SceneContainerRepository.kt @@ -90,22 +90,15 @@ constructor( initialValue = defaultTransitionState, ) - fun changeScene( - toScene: SceneKey, - transitionKey: TransitionKey? = null, - ) { - dataSource.changeScene( - toScene = toScene, - transitionKey = transitionKey, - ) + /** Number of currently active transition animations. */ + val activeTransitionAnimationCount = MutableStateFlow(0) + + fun changeScene(toScene: SceneKey, transitionKey: TransitionKey? = null) { + dataSource.changeScene(toScene = toScene, transitionKey = transitionKey) } - fun snapToScene( - toScene: SceneKey, - ) { - dataSource.snapToScene( - toScene = toScene, - ) + fun snapToScene(toScene: SceneKey) { + dataSource.snapToScene(toScene = toScene) } /** @@ -116,10 +109,7 @@ constructor( * [overlay] is already shown. */ fun showOverlay(overlay: OverlayKey, transitionKey: TransitionKey? = null) { - dataSource.showOverlay( - overlay = overlay, - transitionKey = transitionKey, - ) + dataSource.showOverlay(overlay = overlay, transitionKey = transitionKey) } /** @@ -130,10 +120,7 @@ constructor( * if [overlay] is already hidden. */ fun hideOverlay(overlay: OverlayKey, transitionKey: TransitionKey? = null) { - dataSource.hideOverlay( - overlay = overlay, - transitionKey = transitionKey, - ) + dataSource.hideOverlay(overlay = overlay, transitionKey = transitionKey) } /** @@ -143,11 +130,7 @@ constructor( * This throws if [from] is not currently shown or if [to] is already shown. */ fun replaceOverlay(from: OverlayKey, to: OverlayKey, transitionKey: TransitionKey? = null) { - dataSource.replaceOverlay( - from = from, - to = to, - transitionKey = transitionKey, - ) + dataSource.replaceOverlay(from = from, to = to, transitionKey = transitionKey) } /** Sets whether the container is visible. */ diff --git a/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt b/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt index 0e6fc36fb96a..ba9dc7625769 100644 --- a/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/scene/domain/interactor/SceneInteractor.kt @@ -47,6 +47,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.update /** * Generic business logic and app state accessors for the scene framework. @@ -165,12 +166,15 @@ constructor( /** Whether the scene container is visible. */ val isVisible: StateFlow<Boolean> = - combine(repository.isVisible, repository.isRemoteUserInputOngoing) { - isVisible, - isRemoteUserInteractionOngoing -> + combine( + repository.isVisible, + repository.isRemoteUserInputOngoing, + repository.activeTransitionAnimationCount, + ) { isVisible, isRemoteUserInteractionOngoing, activeTransitionAnimationCount -> isVisibleInternal( raw = isVisible, isRemoteUserInputOngoing = isRemoteUserInteractionOngoing, + activeTransitionAnimationCount = activeTransitionAnimationCount, ) } .stateIn( @@ -436,8 +440,9 @@ constructor( private fun isVisibleInternal( raw: Boolean = repository.isVisible.value, isRemoteUserInputOngoing: Boolean = repository.isRemoteUserInputOngoing.value, + activeTransitionAnimationCount: Int = repository.activeTransitionAnimationCount.value, ): Boolean { - return raw || isRemoteUserInputOngoing + return raw || isRemoteUserInputOngoing || activeTransitionAnimationCount > 0 } /** @@ -525,4 +530,50 @@ constructor( ): Flow<Map<UserAction, UserActionResult>> { return disabledContentInteractor.filteredUserActions(unfiltered) } + + /** + * Notifies that a transition animation has started. + * + * The scene container will remain visible while any transition animation is running within it. + */ + fun onTransitionAnimationStart() { + repository.activeTransitionAnimationCount.update { current -> + (current + 1).also { + check(it < 10) { + "Number of active transition animations is too high. Something must be" + + " calling onTransitionAnimationStart too many times!" + } + } + } + } + + /** + * Notifies that a transition animation has ended. + * + * The scene container will remain visible while any transition animation is running within it. + */ + fun onTransitionAnimationEnd() { + decrementActiveTransitionAnimationCount() + } + + /** + * Notifies that a transition animation has been canceled. + * + * The scene container will remain visible while any transition animation is running within it. + */ + fun onTransitionAnimationCancelled() { + decrementActiveTransitionAnimationCount() + } + + private fun decrementActiveTransitionAnimationCount() { + repository.activeTransitionAnimationCount.update { current -> + (current - 1).also { + check(it >= 0) { + "Number of active transition animations is negative. Something must be" + + " calling onTransitionAnimationEnd or onTransitionAnimationCancelled too" + + " many times!" + } + } + } + } } diff --git a/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartable.kt b/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartable.kt index 8d8c24eae9e2..3a23a71cf7bf 100644 --- a/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartable.kt +++ b/packages/SystemUI/src/com/android/systemui/scene/domain/startable/SceneContainerStartable.kt @@ -25,6 +25,7 @@ import com.android.internal.logging.UiEventLogger import com.android.keyguard.AuthInteractionProperties import com.android.systemui.CoreStartable import com.android.systemui.Flags +import com.android.systemui.animation.ActivityTransitionAnimator import com.android.systemui.authentication.domain.interactor.AuthenticationInteractor import com.android.systemui.authentication.shared.model.AuthenticationMethodModel import com.android.systemui.bouncer.domain.interactor.AlternateBouncerInteractor @@ -146,6 +147,7 @@ constructor( private val vibratorHelper: VibratorHelper, private val msdlPlayer: MSDLPlayer, private val disabledContentInteractor: DisabledContentInteractor, + private val activityTransitionAnimator: ActivityTransitionAnimator, ) : CoreStartable { private val centralSurfaces: CentralSurfaces? get() = centralSurfacesOptLazy.get().getOrNull() @@ -169,6 +171,7 @@ constructor( handleKeyguardEnabledness() notifyKeyguardDismissCancelledCallbacks() refreshLockscreenEnabled() + hydrateActivityTransitionAnimationState() } else { sceneLogger.logFrameworkEnabled( isEnabled = false, @@ -929,6 +932,35 @@ constructor( } } + /** + * Wires the scene framework to activity transition animations that originate from anywhere. A + * subset of these may actually originate from UI inside one of the scenes in the framework. + * + * Telling the scene framework about ongoing activity transition animations is critical so the + * scene framework doesn't make its scene container invisible during a transition. + * + * As it turns out, making the scene container view invisible during a transition animation + * disrupts the animation and causes interaction jank CUJ tracking to ignore reports of the CUJ + * ending or being canceled. + */ + private fun hydrateActivityTransitionAnimationState() { + activityTransitionAnimator.addListener( + object : ActivityTransitionAnimator.Listener { + override fun onTransitionAnimationStart() { + sceneInteractor.onTransitionAnimationStart() + } + + override fun onTransitionAnimationEnd() { + sceneInteractor.onTransitionAnimationEnd() + } + + override fun onTransitionAnimationCancelled() { + sceneInteractor.onTransitionAnimationCancelled() + } + } + ) + } + companion object { private const val TAG = "SceneContainerStartable" } diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/animation/ActivityTransitionAnimatorKosmos.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/animation/ActivityTransitionAnimatorKosmos.kt index 5ac41ec6741c..f380789968f5 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/animation/ActivityTransitionAnimatorKosmos.kt +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/animation/ActivityTransitionAnimatorKosmos.kt @@ -23,11 +23,11 @@ import com.android.systemui.util.mockito.mock val Kosmos.mockActivityTransitionAnimatorController by Kosmos.Fixture { mock<ActivityTransitionAnimator.Controller>() } -val Kosmos.activityTransitionAnimator by +var Kosmos.activityTransitionAnimator by Kosmos.Fixture { ActivityTransitionAnimator( // The main thread is checked in a bunch of places inside the different transitions // animators, so we have to pass the real main executor here. - mainExecutor = testCase.context.mainExecutor, + mainExecutor = testCase.context.mainExecutor ) } diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/scene/domain/startable/SceneContainerStartableKosmos.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/scene/domain/startable/SceneContainerStartableKosmos.kt index 82b5f6332b23..72c75000ebf4 100644 --- a/packages/SystemUI/tests/utils/src/com/android/systemui/scene/domain/startable/SceneContainerStartableKosmos.kt +++ b/packages/SystemUI/tests/utils/src/com/android/systemui/scene/domain/startable/SceneContainerStartableKosmos.kt @@ -17,6 +17,7 @@ package com.android.systemui.scene.domain.startable import com.android.internal.logging.uiEventLogger +import com.android.systemui.animation.activityTransitionAnimator import com.android.systemui.authentication.domain.interactor.authenticationInteractor import com.android.systemui.bouncer.domain.interactor.alternateBouncerInteractor import com.android.systemui.bouncer.domain.interactor.bouncerInteractor @@ -85,5 +86,6 @@ val Kosmos.sceneContainerStartable by Fixture { vibratorHelper = vibratorHelper, msdlPlayer = msdlPlayer, disabledContentInteractor = disabledContentInteractor, + activityTransitionAnimator = activityTransitionAnimator, ) } |