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

Commit fbec530e authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Fix NSSL background flicker during dual shade display transition 

The NSSL background flickered when moving the dual shade between displays.
This was primarily due to NSSL dimensions not updating while the shade was
briefly invisible during the transition.

This CL addresses this by:
- Avoiding collapsing/expanding the shade if the same section remains active,
  allowing new dimensions to apply immediately to the visible window.
- For different sections (e.g., QS vs. notifications), the shade is
  collapsed, then reparented. A frame wait after configuration change
  ensures the correct section expands without flicker, adding one frame of latency.

A potential remaining issue with NSSL background dimension updates from
DisplayStateRepository (due to onDisplayChanged event timing) will be
addressed in a subsequent CL.

Refactored ShadeDisplaysWaitInteractor.kt for use in multiple classes
(covered by existing tests).

Bug: 362719719
Bug: 417956803
Test: ShadeDisplaysInteractorTest, ShadeDisplayChangeLatencyTrackerTest
Flag: com.android.systemui.shade_window_goes_around
Change-Id: I8034f96d2f9d2e7ab3cf5d84d89a0f601c51d2f1
parent 234100e4

packages/SystemUI/src/com/android/systemui/shade/ShadeDisplayChangeLatencyTracker.kt

+8 −24
Original line number Diff line number Diff line
@@ -16,22 +16,16 @@ 
package com.android.systemui.shade



import android.util.Log

import com.android.app.tracing.coroutines.TrackTracer

import com.android.internal.util.LatencyTracker

import com.android.systemui.common.ui.data.repository.ConfigurationRepository

import com.android.systemui.common.ui.view.ChoreographerUtils

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.scene.ui.view.WindowRootView

import com.android.systemui.shade.data.repository.ShadeDisplaysRepository

import com.android.systemui.shade.domain.interactor.ShadeDisplaysWaitInteractor

import java.util.concurrent.CancellationException

import javax.inject.Inject

import kotlin.time.Duration.Companion.seconds

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.Job

import kotlinx.coroutines.flow.StateFlow

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.flow.first

import kotlinx.coroutines.launch

import kotlinx.coroutines.withTimeout
@@ -49,18 +43,11 @@ import kotlinx.coroutines.withTimeout 
class ShadeDisplayChangeLatencyTracker

@Inject

constructor(

    private val shadeRootView: WindowRootView,

    @ShadeDisplayAware private val configurationRepository: ConfigurationRepository,

    private val latencyTracker: LatencyTracker,

    @Background private val bgScope: CoroutineScope,

    private val choreographerUtils: ChoreographerUtils,

    private val waitInteractor: ShadeDisplaysWaitInteractor,

) {



    /**

     * We need to keep this always up to date eagerly to avoid delays receiving the new display ID.

     */

    private val onMovedToDisplayFlow: StateFlow<Int> = configurationRepository.onMovedToDisplay



    private var previousJob: Job? = null



    /**
@@ -84,7 +71,7 @@ constructor( 
        try {

            latencyTracker.onActionStart(SHADE_MOVE_ACTION)

            waitForOnMovedToDisplayDispatchedToView(displayId)

            waitUntilNextDoFrameDone()

            waitUntilNextDoFrameDone(displayId)

            latencyTracker.onActionEnd(SHADE_MOVE_ACTION)

        } catch (e: Exception) {

            val reason =
@@ -101,20 +88,17 @@ constructor( 
    }



    private suspend fun waitForOnMovedToDisplayDispatchedToView(newDisplayId: Int) {

        t.traceAsync({ "waitForOnMovedToDisplayDispatchedToView(newDisplayId=$newDisplayId)" }) {

            withTimeout(TIMEOUT) { onMovedToDisplayFlow.filter { it == newDisplayId }.first() }

            t.instant { "onMovedToDisplay received with $newDisplayId" }

        withTimeout(TIMEOUT) {

            waitInteractor.waitForOnMovedToDisplayDispatchedToView(newDisplayId, TAG)

        }

    }



    private suspend fun waitUntilNextDoFrameDone(): Unit =

        t.traceAsync("waitUntilNextDoFrameDone") {

            withTimeout(TIMEOUT) { choreographerUtils.waitUntilNextDoFrameDone(shadeRootView) }

    private suspend fun waitUntilNextDoFrameDone(newDisplayId: Int) {

        withTimeout(TIMEOUT) { waitInteractor.waitForNextDoFrameDone(newDisplayId, TAG) }

    }



    private companion object {

        const val TAG = "ShadeDisplayLatency"

        val t = TrackTracer(trackName = TAG, trackGroup = "shade")

        val TIMEOUT = 3.seconds

        const val SHADE_MOVE_ACTION = LatencyTracker.ACTION_SHADE_WINDOW_DISPLAY_CHANGE

    }

packages/SystemUI/src/com/android/systemui/shade/domain/interactor/ShadeDisplaysInteractor.kt

+52 −34
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@ import android.window.WindowContext 
import androidx.annotation.UiThread

import com.android.app.tracing.coroutines.launchTraced

import com.android.systemui.CoreStartable

import com.android.systemui.common.ui.data.repository.ConfigurationRepository

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.dagger.qualifiers.Main
@@ -38,9 +37,9 @@ import com.android.systemui.shade.ShadeTraceLogger.traceReparenting 
import com.android.systemui.shade.data.repository.MutableShadeDisplaysRepository

import com.android.systemui.shade.data.repository.ShadeDisplaysRepository

import com.android.systemui.shade.display.ShadeExpansionIntent

import com.android.systemui.shade.domain.interactor.ShadeExpandedStateInteractor.ShadeElement

import com.android.systemui.shade.shared.flag.ShadeWindowGoesAround

import com.android.systemui.statusbar.notification.domain.interactor.ActiveNotificationsInteractor

import com.android.systemui.statusbar.notification.row.NotificationRebindingTracker

import com.android.systemui.statusbar.notification.stack.NotificationStackRebindingHider

import com.android.systemui.statusbar.phone.ConfigurationForwarder

import com.android.window.flags.Flags
@@ -50,8 +49,6 @@ import kotlin.time.Duration.Companion.seconds 
import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.flow.StateFlow

import kotlinx.coroutines.flow.collectLatest

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.flow.first

import kotlinx.coroutines.withContext

import kotlinx.coroutines.withTimeoutOrNull
@@ -73,17 +70,16 @@ class ShadeDisplaysInteractorImpl 
constructor(

    private val shadePositionRepository: MutableShadeDisplaysRepository,

    @ShadeDisplayAware private val shadeContext: WindowContext,

    @ShadeDisplayAware private val configurationRepository: ConfigurationRepository,

    @Background private val bgScope: CoroutineScope,

    @Main private val mainThreadContext: CoroutineContext,

    private val shadeDisplayChangeLatencyTracker: ShadeDisplayChangeLatencyTracker,

    private val shadeExpandedInteractor: ShadeExpandedStateInteractor,

    private val shadeExpansionIntent: ShadeExpansionIntent,

    private val activeNotificationsInteractor: ActiveNotificationsInteractor,

    private val notificationRebindingTracker: NotificationRebindingTracker,

    private val notificationStackRebindingHider: NotificationStackRebindingHider,

    @ShadeDisplayAware private val configForwarder: ConfigurationForwarder,

    @ShadeDisplayLog private val logBuffer: LogBuffer,

    private val waitInteractor: ShadeDisplaysWaitInteractor,

) : ShadeDisplaysInteractor, CoreStartable {



    private val hasActiveNotifications: Boolean
@@ -168,8 +164,28 @@ constructor( 
    }



    private suspend fun collapseAndExpandShadeIfNeeded(newDisplayId: Int, reparent: () -> Unit) {

        val previouslyExpandedElement = shadeExpandedInteractor.currentlyExpandedElement.value

        val previouslyExpandedElement: ShadeElement? =

            shadeExpandedInteractor.currentlyExpandedElement.value



        // The next expanded element depends on the reason why the shade is changing window. e.g. if

        // the trigger was a status bar swipe, based on the swipe location we might want to open

        // quick settings or notifications next (in case of dual shade)

        val nextExpandedElement =

            shadeExpansionIntent.consumeExpansionIntent() ?: previouslyExpandedElement



        // We first collapse the shade only if the element to show after the reparenting is

        // different (e.g. if notifications where visible, but now the user is expanding quick

        // settings in a different display, with dual shade enabled).

        // If we didn't do this, there would be some flicker where the previous element appear for

        // some time.

        val needsToCollapseThenExpand = previouslyExpandedElement != nextExpandedElement



        if (needsToCollapseThenExpand) {

            // We could also consider adding an API to collapse/hide the previous instantaneously in

            // the future.

            previouslyExpandedElement?.collapse(reason = COLLAPSE_EXPAND_REASON)

        }



        val notificationStackHidden =

            if (!hasActiveNotifications) {

                // This covers the case the previous move was cancelled before setting the
@@ -187,11 +203,13 @@ constructor( 


        reparent()



        val elementToExpand =

            shadeExpansionIntent.consumeExpansionIntent() ?: previouslyExpandedElement

        // If the user was trying to expand a specific shade element, let's make sure to expand

        // that one. Otherwise, we can just re-expand the previous expanded element.

        elementToExpand?.expand(COLLAPSE_EXPAND_REASON)

        if (needsToCollapseThenExpand) {

            waitForOnMovedToDisplayDispatchedToView(newDisplayId)

            // Let's make sure a frame has been drawn with the new configuration before expanding

            // the shade again, otherwise we might end up having a flicker.

            waitForNextFrameDrawn(newDisplayId)

            nextExpandedElement?.expand(reason = COLLAPSE_EXPAND_REASON)

        }

        if (notificationStackHidden) {

            if (hasActiveNotifications) {

                // "onMovedToDisplay" is what synchronously triggers the rebinding of views: we need
@@ -205,33 +223,29 @@ constructor( 


    private suspend fun waitForOnMovedToDisplayDispatchedToView(newDisplayId: Int) {

        withContext(bgScope.coroutineContext) {

            t.traceAsync({

                "waitForOnMovedToDisplayDispatchedToView(newDisplayId=$newDisplayId)"

            }) {

                withTimeoutOrNull(TIMEOUT) {

                    configurationRepository.onMovedToDisplay.filter { it == newDisplayId }.first()

                    t.instant { "onMovedToDisplay received with $newDisplayId" }

            withTimeoutOrNull(WAIT_TIMEOUT) {

                waitInteractor.waitForOnMovedToDisplayDispatchedToView(newDisplayId, TAG)

            }

                ?: errorLog(

                    "Timed out while waiting for onMovedToDisplay to be dispatched to " +

                            "the shade root view in ShadeDisplaysInteractor"

                        "the shade root view"

                )

        }

    }

    }



    private suspend fun waitForNotificationsRebinding() {

        // here we don't need to wait for rebinding to appear (e.g. going > 0), as it already

        // happened synchronously when the new configuration was received by ViewConfigCoordinator.

        t.traceAsync("waiting for notifications rebinding to finish") {

            withTimeoutOrNull(TIMEOUT) {

                notificationRebindingTracker.rebindingInProgressCount.first { it == 0 }

            } ?: errorLog("Timed out while waiting for inflations to finish")

    private suspend fun waitForNextFrameDrawn(newDisplayId: Int) {

        withContext(bgScope.coroutineContext) {

            withTimeoutOrNull(WAIT_TIMEOUT) {

                waitInteractor.waitForNextDoFrameDone(newDisplayId, TAG)

            } ?: errorLog("Timed out while waiting for the next frame to be drawn.")

        }

    }



    private fun errorLog(s: String) {

        logBuffer.log(TAG, LogLevel.ERROR, s)

    private suspend fun waitForNotificationsRebinding() {

        withContext(bgScope.coroutineContext) {

            withTimeoutOrNull(WAIT_TIMEOUT) { waitInteractor.waitForNotificationsRebinding(TAG) }

                ?: errorLog("Timed out while waiting for inflations to finish")

        }

    }



    private fun checkContextDisplayMatchesExpected(destinationId: Int) {
@@ -246,6 +260,10 @@ constructor( 
        }

    }



    private fun errorLog(s: String) {

        logBuffer.log(TAG, LogLevel.ERROR, s)

    }



    @UiThread

    private fun reparentToDisplayId(id: Int) {

        t.traceSyncAndAsync({ "reparentToDisplayId(id=$id)" }) {
@@ -254,8 +272,8 @@ constructor( 
    }



    private companion object {

        val WAIT_TIMEOUT = 1.seconds

        const val TAG = "ShadeDisplaysInteractor"

        const val COLLAPSE_EXPAND_REASON = "Shade window move"

        val TIMEOUT = 1.seconds

    }

}

packages/SystemUI/src/com/android/systemui/shade/domain/interactor/ShadeDisplaysWaitInteractor.kt

0 → 100644
+83 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2025 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.systemui.shade.domain.interactor



import com.android.app.tracing.coroutines.TrackTracer

import com.android.systemui.common.ui.data.repository.ConfigurationRepository

import com.android.systemui.common.ui.view.ChoreographerUtils

import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.scene.ui.view.WindowRootView

import com.android.systemui.shade.ShadeDisplayAware

import com.android.systemui.statusbar.notification.row.NotificationRebindingTracker

import javax.inject.Inject

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.flow.first



/** Provides a way to wait for specific events related to the shade window. */

@SysUISingleton

class ShadeDisplaysWaitInteractor

@Inject

constructor(

    private val choreographerUtils: ChoreographerUtils,

    private val shadeRootView: WindowRootView,

    private val notificationRebindingTracker: NotificationRebindingTracker,

    @ShadeDisplayAware private val configurationRepository: ConfigurationRepository,

) {



    private val t = TrackTracer(trackName = "ShadeDisplaysWaitInteractor", trackGroup = "shade")



    /**

     * Suspends until the latest `onMovedToDisplay` received by the shade window has a display id

     * matching [newDisplayId].

     *

     * The caller is supposed to wrap this with [withTimeout], if needed.

     */

    suspend fun waitForOnMovedToDisplayDispatchedToView(newDisplayId: Int, logKey: String) {

        t.traceAsync({

            "$logKey#waitForOnMovedToDisplayDispatchedToView(newDisplayId=$newDisplayId)"

        }) {

            configurationRepository.onMovedToDisplay.filter { it == newDisplayId }.first()

            t.instant { "$logKey#onMovedToDisplay received with $newDisplayId" }

        }

    }



    /**

     * Suspends until the next doFrame is done. See [ChoreographerUtils] for details of this

     * behaviour (which has some corner cases).

     *

     * The caller is supposed to wrap this with [withTimeout], if needed.

     */

    suspend fun waitForNextDoFrameDone(newDisplayId: Int, logKey: String) {

        t.traceAsync({ "$logKey#waitForNextFrameDrawn(newDisplayId=$newDisplayId)" }) {

            choreographerUtils.waitUntilNextDoFrameDone(shadeRootView)

        }

    }



    /**

     * Waits for notifications inflations to be done. This assumes that there is at least one

     * notification being re-inflated already (otherwise it returns immediately).

     *

     * The caller is supposed to wrap this with [withTimeout], if needed.

     */

    suspend fun waitForNotificationsRebinding(logKey: String) {

        // here we don't need to wait for rebinding to appear (e.g. going > 0), as it already

        // happened synchronously when the new configuration was received by ViewConfigCoordinator.

        t.traceAsync("$logKey#waiting for notifications rebinding to finish") {

            notificationRebindingTracker.rebindingInProgressCount.first { it == 0 }

        }

    }

}

packages/SystemUI/tests/utils/src/com/android/systemui/shade/ShadeDisplayChangeLatencyTrackerKosmos.kt

+2 −7
Original line number Diff line number Diff line
@@ -17,20 +17,15 @@ 
package com.android.systemui.shade



import com.android.internal.logging.latencyTracker

import com.android.systemui.common.ui.data.repository.configurationRepository

import com.android.systemui.common.ui.view.fakeChoreographerUtils

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.Kosmos.Fixture

import com.android.systemui.kosmos.testScope

import com.android.systemui.scene.ui.view.mockShadeRootView

import java.util.Optional

import com.android.systemui.shade.domain.interactor.shadeDisplaysWaitInteractor



val Kosmos.shadeDisplayChangeLatencyTracker by Fixture {

    ShadeDisplayChangeLatencyTracker(

        mockShadeRootView,

        configurationRepository,

        latencyTracker,

        testScope.backgroundScope,

        fakeChoreographerUtils,

        shadeDisplaysWaitInteractor,

    )

}

packages/SystemUI/tests/utils/src/com/android/systemui/shade/domain/interactor/ShadeDisplaysInteractorKosmos.kt

+1 −4
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package com.android.systemui.shade.domain.interactor 


import android.content.mockedContext

import android.window.WindowContext

import com.android.systemui.common.ui.data.repository.configurationRepository

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.testScope

import com.android.systemui.log.logcatLogBuffer
@@ -27,7 +26,6 @@ import com.android.systemui.shade.ShadeWindowLayoutParams 
import com.android.systemui.shade.data.repository.fakeShadeDisplaysRepository

import com.android.systemui.shade.data.repository.shadeExpansionIntent

import com.android.systemui.statusbar.notification.domain.interactor.activeNotificationsInteractor

import com.android.systemui.statusbar.notification.row.notificationRebindingTracker

import com.android.systemui.statusbar.notification.stack.notificationStackRebindingHider

import com.android.systemui.statusbar.phone.systemUIDialogManager

import com.android.systemui.statusbar.policy.configurationController
@@ -52,17 +50,16 @@ val Kosmos.shadeDisplaysInteractor by 
        ShadeDisplaysInteractorImpl(

            fakeShadeDisplaysRepository,

            mockedWindowContext,

            configurationRepository,

            testScope.backgroundScope,

            testScope.backgroundScope.coroutineContext,

            mockedShadeDisplayChangeLatencyTracker,

            shadeExpandedStateInteractor,

            shadeExpansionIntent,

            activeNotificationsInteractor,

            notificationRebindingTracker,

            notificationStackRebindingHider,

            configurationController,

            logcatLogBuffer("ShadeDisplaysInteractor"),

            shadeDisplaysWaitInteractor,

        )

    }