Harden light reveal transition logic
The previous code assumed a continuous order of transitions emitted by
the transition framework. Meaning if X -> LOCKSCREEN the next transition
has to be LOCKSCREEN -> Y. While this assumption is still correct, we
have seen bugs in transition code that would sometimes skip one of
these transitions. For these cases we could end up in a state where the
animation would have been skipped and all subsequent states are
revealed, therefore `willTransitionChangeEndState(it)` would block all
further calls from revealing the scrim.
The new code does not rely on this assumption and will attempt to repeat
the reveal animation if we are not in the desired reveal state and the
animation is not yet running.
Test: NONE
Bug: b/324442545 b/281655028
Flag: ACONFIG com.android.systemui.Flags.FLAG_LIGHT_REVEAL_MIGRATION STAGING
Change-Id: Iab622df064ce2e0800efac01235aa78c12621ff1
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt
index f6c0566..fb46ed9d 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt
@@ -175,6 +175,21 @@
animatorTestRule.advanceTimeBy(500L)
assertEquals(1.0f, value)
}
+
+ @Test
+ @TestableLooper.RunWithLooper(setAsMainLooper = true)
+ fun revealAmount_startingRevealTwiceWontRerunAnimator() =
+ runTest(UnconfinedTestDispatcher()) {
+ val value by collectLastValue(underTest.revealAmount)
+ underTest.startRevealAmountAnimator(true)
+ assertEquals(0.0f, value)
+ animatorTestRule.advanceTimeBy(250L)
+ assertEquals(0.5f, value)
+ underTest.startRevealAmountAnimator(true)
+ animatorTestRule.advanceTimeBy(250L)
+ assertEquals(1.0f, value)
+ }
+
@Test
@TestableLooper.RunWithLooper(setAsMainLooper = true)
fun revealAmount_emitsTo0AfterAnimationStartedReversed() =
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt
index 9b302ae..2b6e6c7 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt
@@ -22,7 +22,6 @@
import com.android.systemui.keyguard.data.fakeLightRevealScrimRepository
import com.android.systemui.keyguard.data.repository.FakeLightRevealScrimRepository
import com.android.systemui.keyguard.data.repository.fakeKeyguardTransitionRepository
-import com.android.systemui.keyguard.shared.model.KeyguardState
import com.android.systemui.keyguard.shared.model.TransitionState
import com.android.systemui.keyguard.shared.model.TransitionStep
import com.android.systemui.kosmos.testScope
@@ -33,16 +32,11 @@
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.test.UnconfinedTestDispatcher
-import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito
-import org.mockito.Mockito.anyBoolean
-import org.mockito.Mockito.never
-import org.mockito.Mockito.reset
-import org.mockito.Mockito.verify
@SmallTest
@OptIn(ExperimentalCoroutinesApi::class)
@@ -103,41 +97,4 @@
job.cancel()
}
-
- @Test
- fun lightRevealEffect_startsAnimationOnlyForDifferentStateTargets() =
- testScope.runTest {
- runCurrent()
- reset(fakeLightRevealScrimRepository)
-
- fakeKeyguardTransitionRepository.sendTransitionStep(
- TransitionStep(
- transitionState = TransitionState.STARTED,
- from = KeyguardState.OFF,
- to = KeyguardState.OFF
- )
- )
- runCurrent()
- verify(fakeLightRevealScrimRepository, never()).startRevealAmountAnimator(anyBoolean())
-
- fakeKeyguardTransitionRepository.sendTransitionStep(
- TransitionStep(
- transitionState = TransitionState.STARTED,
- from = KeyguardState.DOZING,
- to = KeyguardState.LOCKSCREEN
- )
- )
- runCurrent()
- verify(fakeLightRevealScrimRepository).startRevealAmountAnimator(true)
-
- fakeKeyguardTransitionRepository.sendTransitionStep(
- TransitionStep(
- transitionState = TransitionState.STARTED,
- from = KeyguardState.LOCKSCREEN,
- to = KeyguardState.DOZING
- )
- )
- runCurrent()
- verify(fakeLightRevealScrimRepository).startRevealAmountAnimator(false)
- }
}
diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt
index 42f14f1..9b3f13d 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt
@@ -151,9 +151,13 @@
awaitClose { revealAmountAnimator.removeUpdateListener(updateListener) }
}
+ private var willBeOrIsRevealed: Boolean? = null
+
override fun startRevealAmountAnimator(reveal: Boolean) {
+ if (reveal == willBeOrIsRevealed) return
+ willBeOrIsRevealed = reveal
if (reveal) revealAmountAnimator.start() else revealAmountAnimator.reverse()
- scrimLogger.d(TAG, "startRevealAmountAnimator, reveal", reveal)
+ scrimLogger.d(TAG, "startRevealAmountAnimator, reveal: ", reveal)
}
override val revealEffect =
diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt
index c7f262a..4d731ec 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt
@@ -21,7 +21,6 @@
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.keyguard.data.repository.LightRevealScrimRepository
import com.android.systemui.keyguard.shared.model.KeyguardState
-import com.android.systemui.keyguard.shared.model.TransitionStep
import com.android.systemui.power.domain.interactor.PowerInteractor
import com.android.systemui.power.shared.model.ScreenPowerState
import com.android.systemui.statusbar.LightRevealEffect
@@ -53,11 +52,9 @@
scope.launch {
transitionInteractor.startedKeyguardTransitionStep.collect {
scrimLogger.d(TAG, "listenForStartedKeyguardTransitionStep", it)
- if (willTransitionChangeEndState(it)) {
- lightRevealScrimRepository.startRevealAmountAnimator(
- willBeRevealedInState(it.to)
- )
- }
+ lightRevealScrimRepository.startRevealAmountAnimator(
+ willBeRevealedInState(it.to),
+ )
}
}
}
@@ -92,33 +89,25 @@
companion object {
- /**
- * Whether the transition requires a change in the reveal amount of the light reveal scrim.
- * If not, we don't care about the transition and don't need to listen to it.
- */
- fun willTransitionChangeEndState(transition: TransitionStep): Boolean {
- return willBeRevealedInState(transition.from) != willBeRevealedInState(transition.to)
+ /**
+ * Whether the light reveal scrim will be fully revealed (revealAmount = 1.0f) in the given
+ * state after the transition is complete. If false, scrim will be fully hidden.
+ */
+ private fun willBeRevealedInState(state: KeyguardState): Boolean {
+ return when (state) {
+ KeyguardState.OFF -> false
+ KeyguardState.DOZING -> false
+ KeyguardState.AOD -> false
+ KeyguardState.DREAMING -> true
+ KeyguardState.DREAMING_LOCKSCREEN_HOSTED -> true
+ KeyguardState.GLANCEABLE_HUB -> true
+ KeyguardState.ALTERNATE_BOUNCER -> true
+ KeyguardState.PRIMARY_BOUNCER -> true
+ KeyguardState.LOCKSCREEN -> true
+ KeyguardState.GONE -> true
+ KeyguardState.OCCLUDED -> true
}
-
- /**
- * Whether the light reveal scrim will be fully revealed (revealAmount = 1.0f) in the given
- * state after the transition is complete. If false, scrim will be fully hidden.
- */
- fun willBeRevealedInState(state: KeyguardState): Boolean {
- return when (state) {
- KeyguardState.OFF -> false
- KeyguardState.DOZING -> false
- KeyguardState.AOD -> false
- KeyguardState.DREAMING -> true
- KeyguardState.DREAMING_LOCKSCREEN_HOSTED -> true
- KeyguardState.GLANCEABLE_HUB -> true
- KeyguardState.ALTERNATE_BOUNCER -> true
- KeyguardState.PRIMARY_BOUNCER -> true
- KeyguardState.LOCKSCREEN -> true
- KeyguardState.GONE -> true
- KeyguardState.OCCLUDED -> true
- }
- }
+ }
val TAG = LightRevealScrimInteractor::class.simpleName!!
}