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

Commit 8f2abe64 authored by Matt Pietal's avatar Matt Pietal
Browse files

Optimize flows to reduce jank

After some tracing, multiple flows were being recalculated
on every frame during swipe up/swipe down. Use stateIn
to share this same calculation with downstream flows.

Also, remove the shade collapse fade in callback flow. It
just really isn't needed and could produce a delay in
processing future events.

Bug: 326719188
Test: use perfetto to analyze traces
Flag: ACONFIG com.android.systemui.migrate_clocks_to_blueprint
TEAMFOOD

Change-Id: I1c71776b9cc13e964aea77dd5b7ccd91ea5261f2
parent 97ef6a07
Loading
Loading
Loading
Loading
+25 −26
Original line number Diff line number Diff line
@@ -47,9 +47,7 @@ import org.junit.runner.RunWith
@kotlinx.coroutines.ExperimentalCoroutinesApi
@android.platform.test.annotations.EnabledOnRavenwood
class KeyguardTransitionInteractorTest : SysuiTestCase() {

    val kosmos = testKosmos()

    val underTest = kosmos.keyguardTransitionInteractor
    val repository = kosmos.fakeKeyguardTransitionRepository
    val testScope = kosmos.testScope
@@ -242,7 +240,8 @@ class KeyguardTransitionInteractorTest : SysuiTestCase() {
    }

    @Test
    fun transitionValue() = runTest {
    fun transitionValue() =
        testScope.runTest {
            val startedSteps by collectValues(underTest.transitionValue(state = DOZING))

            val toSteps =
+11 −0
Original line number Diff line number Diff line
@@ -250,6 +250,17 @@ class KeyguardRootViewModelTest : SysuiTestCase() {

            keyguardRepository.topClippingBounds.value = 1000
            assertThat(topClippingBounds).isEqualTo(1000)

            // Run at least 1 transition to make sure value remains at 0
            keyguardTransitionRepository.sendTransitionSteps(
                from = KeyguardState.AOD,
                to = KeyguardState.GONE,
                testScope,
            )

            // Make sure the value hasn't changed since we're GONE
            keyguardRepository.topClippingBounds.value = 5
            assertThat(topClippingBounds).isEqualTo(1000)
        }

    @Test
+11 −13
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import com.android.systemui.SysuiTestCase
import com.android.systemui.common.shared.model.NotificationContainerBounds
import com.android.systemui.common.ui.data.repository.fakeConfigurationRepository
import com.android.systemui.coroutines.collectLastValue
import com.android.systemui.coroutines.collectValues
import com.android.systemui.flags.Flags
import com.android.systemui.flags.fakeFeatureFlagsClassic
import com.android.systemui.keyguard.data.repository.fakeKeyguardRepository
@@ -721,39 +722,36 @@ class SharedNotificationContainerViewModelTest : SysuiTestCase() {
    @Test
    fun shadeCollapseFadeIn() =
        testScope.runTest {
            val fadeIn by collectLastValue(underTest.shadeCollapseFadeIn)
            val fadeIn by collectValues(underTest.shadeCollapseFadeIn)

            // Start on lockscreen without the shade
            underTest.setShadeCollapseFadeInComplete(false)
            showLockscreen()
            assertThat(fadeIn).isEqualTo(false)
            assertThat(fadeIn[0]).isEqualTo(false)

            // ... then the shade expands
            showLockscreenWithShadeExpanded()
            assertThat(fadeIn).isEqualTo(false)
            assertThat(fadeIn[0]).isEqualTo(false)

            // ... it collapses
            showLockscreen()
            assertThat(fadeIn).isEqualTo(true)
            assertThat(fadeIn[1]).isEqualTo(true)

            // ... now send animation complete signal
            underTest.setShadeCollapseFadeInComplete(true)
            assertThat(fadeIn).isEqualTo(false)
            // ... and ensure the value goes back to false
            assertThat(fadeIn[2]).isEqualTo(false)
        }

    @Test
    fun shadeCollapseFadeIn_doesNotRunIfTransitioningToAod() =
        testScope.runTest {
            val fadeIn by collectLastValue(underTest.shadeCollapseFadeIn)
            val fadeIn by collectValues(underTest.shadeCollapseFadeIn)

            // Start on lockscreen without the shade
            underTest.setShadeCollapseFadeInComplete(false)
            showLockscreen()
            assertThat(fadeIn).isEqualTo(false)
            assertThat(fadeIn[0]).isEqualTo(false)

            // ... then the shade expands
            showLockscreenWithShadeExpanded()
            assertThat(fadeIn).isEqualTo(false)
            assertThat(fadeIn[0]).isEqualTo(false)

            // ... then user hits power to go to AOD
            keyguardTransitionRepository.sendTransitionSteps(
@@ -764,7 +762,7 @@ class SharedNotificationContainerViewModelTest : SysuiTestCase() {
            // ... followed by a shade collapse
            showLockscreen()
            // ... does not trigger a fade in
            assertThat(fadeIn).isEqualTo(false)
            assertThat(fadeIn[0]).isEqualTo(false)
        }

    private suspend fun TestScope.showLockscreen() {
+14 −5
Original line number Diff line number Diff line
@@ -57,6 +57,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.combineTransform
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.flatMapLatest
@@ -179,11 +180,19 @@ constructor(

    /** Keyguard can be clipped at the top as the shade is dragged */
    val topClippingBounds: Flow<Int?> =
        combine(configurationInteractor.onAnyConfigurationChange, repository.topClippingBounds) {
            _,
            topClippingBounds ->
            topClippingBounds
        combineTransform(
                configurationInteractor.onAnyConfigurationChange,
                keyguardTransitionInteractor
                    .transitionValue(GONE)
                    .map { it == 1f }
                    .onStart { emit(false) },
                repository.topClippingBounds
            ) { _, isGone, topClippingBounds ->
                if (!isGone) {
                    emit(topClippingBounds)
                }
            }
            .distinctUntilChanged()

    /** Last point that [KeyguardRootView] view was tapped */
    val lastRootViewTapPosition: Flow<Point?> = repository.lastRootViewTapPosition.asStateFlow()
+8 −2
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ package com.android.systemui.keyguard.domain.interactor

import com.android.keyguard.logging.KeyguardLogger
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.dagger.qualifiers.Background
import com.android.systemui.log.core.LogLevel.VERBOSE
import com.android.systemui.power.domain.interactor.PowerInteractor
import com.android.systemui.shade.domain.interactor.ShadeInteractor
@@ -34,7 +34,7 @@ private val TAG = KeyguardTransitionAuditLogger::class.simpleName!!
class KeyguardTransitionAuditLogger
@Inject
constructor(
    @Application private val scope: CoroutineScope,
    @Background private val scope: CoroutineScope,
    private val interactor: KeyguardTransitionInteractor,
    private val keyguardInteractor: KeyguardInteractor,
    private val logger: KeyguardLogger,
@@ -69,6 +69,12 @@ constructor(
            }
        }

        scope.launch {
            sharedNotificationContainerViewModel.bounds.collect {
                logger.log(TAG, VERBOSE, "Notif: bounds", it)
            }
        }

        scope.launch {
            sharedNotificationContainerViewModel.isOnLockscreenWithoutShade.collect {
                logger.log(TAG, VERBOSE, "Notif: isOnLockscreenWithoutShade", it)
Loading