Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cdd2eaed authored by Andreas Miko's avatar Andreas Miko
Browse files

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
parent 2794bccf

packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepositoryTest.kt

+15 −0
Original line number Diff line number Diff line
@@ -175,6 +175,21 @@ class LightRevealScrimRepositoryTest : SysuiTestCase() { 
            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() =

packages/SystemUI/multivalentTests/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractorTest.kt

+0 −43
Original line number Diff line number Diff line
@@ -22,7 +22,6 @@ import com.android.systemui.SysuiTestCase 
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.ExperimentalCoroutinesApi 
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 @@ class LightRevealScrimInteractorTest : SysuiTestCase() { 


            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)

        }

}

packages/SystemUI/src/com/android/systemui/keyguard/data/repository/LightRevealScrimRepository.kt

+5 −1
Original line number Diff line number Diff line
@@ -151,9 +151,13 @@ constructor( 
        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 =

packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/LightRevealScrimInteractor.kt

+21 −32
Original line number Diff line number Diff line
@@ -21,7 +21,6 @@ import com.android.systemui.dagger.SysUISingleton 
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,14 +52,12 @@ constructor( 
        scope.launch {

            transitionInteractor.startedKeyguardTransitionStep.collect {

                scrimLogger.d(TAG, "listenForStartedKeyguardTransitionStep", it)

                if (willTransitionChangeEndState(it)) {

                lightRevealScrimRepository.startRevealAmountAnimator(

                        willBeRevealedInState(it.to)

                    willBeRevealedInState(it.to),

                )

            }

        }

    }

    }



    /**

     * Whenever a keyguard transition starts, sample the latest reveal effect from the repository
@@ -92,19 +89,11 @@ constructor( 


    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.

     */

        fun willBeRevealedInState(state: KeyguardState): Boolean {

    private fun willBeRevealedInState(state: KeyguardState): Boolean {

        return when (state) {

            KeyguardState.OFF -> false

            KeyguardState.DOZING -> false