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

Commit 39525b23 authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Fix dialog flicker when onDisplayChanged is received 

Adds a "distrinctUntilChanged" to the pending displayId, as
onDisplayChanged was triggering it to be recomputed, and this was
causing a flicker in the dialog. This was happening only while a video
was playing.

This makes logging for each DisplayRepository flow more clear in a perfetto trace.
Also all display events are now logged.

Flag: None
Bug: 308865451
Bug: 308789093
Test: DisplayRepositoryTest
Change-Id: I87b5d97198f1ee7e7d6983481f9cccb0634a2880
parent 61d9e422

packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayRepository.kt

+39 −30
Original line number Diff line number Diff line
@@ -23,9 +23,9 @@ import android.hardware.display.DisplayManager.EVENT_FLAG_DISPLAY_ADDED 
import android.hardware.display.DisplayManager.EVENT_FLAG_DISPLAY_CHANGED

import android.hardware.display.DisplayManager.EVENT_FLAG_DISPLAY_REMOVED

import android.os.Handler

import android.os.Trace

import android.util.Log

import android.view.Display

import com.android.app.tracing.FlowTracing.traceEach

import com.android.app.tracing.traceSection

import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow

import com.android.systemui.dagger.SysUISingleton
@@ -43,10 +43,9 @@ import kotlinx.coroutines.flow.SharingStarted 
import kotlinx.coroutines.flow.StateFlow

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.distinctUntilChanged

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.flow.filterIsInstance

import kotlinx.coroutines.flow.flowOn

import kotlinx.coroutines.flow.map

import kotlinx.coroutines.flow.onEach

import kotlinx.coroutines.flow.onStart

import kotlinx.coroutines.flow.shareIn

import kotlinx.coroutines.flow.stateIn
@@ -97,7 +96,6 @@ constructor( 
    @Application applicationScope: CoroutineScope,

    @Background backgroundCoroutineDispatcher: CoroutineDispatcher

) : DisplayRepository {

    // Displays are enabled only after receiving them in [onDisplayAdded]

    private val allDisplayEvents: Flow<DisplayEvent> =

        conflatedCallbackFlow {

                val callback =
@@ -124,16 +122,22 @@ constructor( 
                awaitClose { displayManager.unregisterDisplayListener(callback) }

            }

            .onStart { emit(DisplayEvent.Changed(Display.DEFAULT_DISPLAY)) }

            .debugLog("allDisplayEvents")

            .flowOn(backgroundCoroutineDispatcher)



    override val displayChangeEvent: Flow<Int> =

        allDisplayEvents.filter { it is DisplayEvent.Changed }.map { it.displayId }

        allDisplayEvents.filterIsInstance<DisplayEvent.Changed>().map { event -> event.displayId }



    override val displayAdditionEvent: Flow<Display?> =

        allDisplayEvents

            .filter { it is DisplayEvent.Added }

            .map { displayManager.getDisplay(it.displayId) }

        allDisplayEvents.filterIsInstance<DisplayEvent.Added>().map {

            displayManager.getDisplay(it.displayId)

        }



    /**

     * Represents displays that went though the [DisplayListener.onDisplayAdded] callback.

     *

     * Those are commonly the ones provided by [DisplayManager.getDisplays] by default.

     */

    private val enabledDisplays =

        allDisplayEvents

            .map { getDisplays() }
@@ -143,9 +147,7 @@ constructor( 
    override val displays: Flow<Set<Display>> = enabledDisplays



    private fun getDisplays(): Set<Display> =

        traceSection("DisplayRepository#getDisplays()") {

            displayManager.displays?.toSet() ?: emptySet()

        }

        traceSection("$TAG#getDisplays()") { displayManager.displays?.toSet() ?: emptySet() }



    /** Propagate to the listeners only enabled displays */

    private val enabledDisplayIds: Flow<Set<Int>> =
@@ -153,7 +155,8 @@ constructor( 
            .map { enabledDisplaysSet -> enabledDisplaysSet.map { it.displayId }.toSet() }

            .debugLog("enabledDisplayIds")



    private val ignoredDisplayIds = MutableStateFlow<Set<Int>>(emptySet())

    private val _ignoredDisplayIds = MutableStateFlow<Set<Int>>(emptySet())

    private val ignoredDisplayIds: Flow<Set<Int>> = _ignoredDisplayIds.debugLog("ignoredDisplayIds")



    private fun getInitialConnectedDisplays(): Set<Int> =

        displayManager
@@ -177,7 +180,7 @@ constructor( 
                                Log.d(TAG, "display with id=$id connected.")

                            }

                            connectedIds += id

                            ignoredDisplayIds.value -= id

                            _ignoredDisplayIds.value -= id

                            trySend(connectedIds.toSet())

                        }
@@ -186,7 +189,7 @@ constructor( 
                            if (DEBUG) {

                                Log.d(TAG, "display with id=$id disconnected.")

                            }

                            ignoredDisplayIds.value -= id

                            _ignoredDisplayIds.value -= id

                            trySend(connectedIds.toSet())

                        }

                    }
@@ -214,17 +217,23 @@ constructor( 
    private val connectedExternalDisplayIds: Flow<Set<Int>> =

        connectedDisplayIds

            .map { connectedDisplayIds ->

                traceSection("$TAG#filteringExternalDisplays") {

                    connectedDisplayIds

                    .filter { id -> displayManager.getDisplay(id)?.type == Display.TYPE_EXTERNAL }

                        .filter { id -> getDisplayType(id) == Display.TYPE_EXTERNAL }

                        .toSet()

                }

            }

            .flowOn(backgroundCoroutineDispatcher)

            .debugLog("connectedExternalDisplayIds")



    private fun getDisplayType(displayId: Int): Int? =

        traceSection("$TAG#getDisplayType") { displayManager.getDisplay(displayId)?.type }



    /**

     * Pending displays are the ones connected, but not enabled and not ignored. A connected display

     * is ignored after the user makes the decision to use it or not. For now, the initial decision

     * from the user is final and not reversible.

     * Pending displays are the ones connected, but not enabled and not ignored.

     *

     * A connected display is ignored after the user makes the decision to use it or not. For now,

     * the initial decision from the user is final and not reversible.

     */

    private val pendingDisplayIds: Flow<Set<Int>> =

        combine(enabledDisplayIds, connectedExternalDisplayIds, ignoredDisplayIds) {
@@ -241,12 +250,16 @@ constructor( 
                }

                connectedExternalDisplayIds - enabledDisplaysIds - ignoredDisplayIds

            }

            .debugLog("pendingDisplayIds")

            .debugLog("allPendingDisplayIds")



    /** Which display id should be enabled among the pending ones. */

    private val pendingDisplayId: Flow<Int?> =

        pendingDisplayIds.map { it.maxOrNull() }.distinctUntilChanged().debugLog("pendingDisplayId")



    override val pendingDisplay: Flow<DisplayRepository.PendingDisplay?> =

        pendingDisplayIds

            .map { pendingDisplayIds ->

                val id = pendingDisplayIds.maxOrNull() ?: return@map null

        pendingDisplayId

            .map { displayId ->

                val id = displayId ?: return@map null

                object : DisplayRepository.PendingDisplay {

                    override val id = id
@@ -263,7 +276,7 @@ constructor( 


                    override suspend fun ignore() {

                        traceSection("DisplayRepository#ignore($id)") {

                            ignoredDisplayIds.value += id

                            _ignoredDisplayIds.value += id

                        }

                    }
@@ -282,11 +295,7 @@ constructor( 


    private fun <T> Flow<T>.debugLog(flowName: String): Flow<T> {

        return if (DEBUG) {

            this.onEach {

                Log.d(TAG, "$flowName: $it")

                Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_APP, "$TAG#$flowName", 0)

                Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_APP, "$TAG#$flowName", "$it", 0)

            }

            traceEach(flowName, logcat = true)

        } else {

            this

        }

packages/SystemUI/tests/src/com/android/systemui/display/data/repository/DisplayRepositoryTest.kt

+30 −0
Original line number Diff line number Diff line
@@ -379,6 +379,32 @@ class DisplayRepositoryTest : SysuiTestCase() { 
            assertThat(pendingDisplay).isNull()

        }



    @Test

    fun pendingDisplay_afterConfigChanged_doesNotChange() =

        testScope.runTest {

            val pendingDisplay by lastPendingDisplay()



            sendOnDisplayConnected(1, TYPE_EXTERNAL)

            val initialPendingDisplay: DisplayRepository.PendingDisplay? = pendingDisplay

            assertThat(pendingDisplay).isNotNull()

            sendOnDisplayChanged(1)



            assertThat(initialPendingDisplay).isEqualTo(pendingDisplay)

        }



    @Test

    fun pendingDisplay_afterNewHigherDisplayConnected_changes() =

        testScope.runTest {

            val pendingDisplay by lastPendingDisplay()



            sendOnDisplayConnected(1, TYPE_EXTERNAL)

            val initialPendingDisplay: DisplayRepository.PendingDisplay? = pendingDisplay

            assertThat(pendingDisplay).isNotNull()

            sendOnDisplayConnected(2, TYPE_EXTERNAL)



            assertThat(initialPendingDisplay).isNotEqualTo(pendingDisplay)

        }



    @Test

    fun onPendingDisplay_OneInternalAndOneExternalDisplay_internalIgnored() =

        testScope.runTest {
@@ -466,6 +492,10 @@ class DisplayRepositoryTest : SysuiTestCase() { 
        connectedDisplayListener.value.onDisplayConnected(id)

    }



    private fun sendOnDisplayChanged(id: Int) {

        connectedDisplayListener.value.onDisplayChanged(id)

    }



    private fun setDisplays(displays: List<Display>) {

        whenever(displayManager.displays).thenReturn(displays.toTypedArray())

        displays.forEach { display ->