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

Commit 9142b53a authored by Michael Mikhail's avatar Michael Mikhail
Browse files

Don't reuse colored and animatable drawables 

Play/pause button drawables are changing color and state with each new
media loaded. Changing the color after animation occurs cause the other
drawable of the prev/next UMO in carousel to change as we cannot mutate
a mutated drawable. It is better not to reuse these kind of drawables
instead and keep reusing the other static drawables.

Flag: com.android.systemui.media_controls_drawables_reuse
Bug: 358402034
Test: atest MediaDataProcessorTest
Change-Id: I97808b006ca1647559db8439779f86ffc8c17494
parent 083b275a

packages/SystemUI/src/com/android/systemui/media/controls/domain/pipeline/MediaActions.kt

+7 −6
Original line number Diff line number Diff line
@@ -59,13 +59,14 @@ fun createActionsFromState( 
    val playOrPause =

        if (isConnectingState(state.state)) {

            // Spinner needs to be animating to render anything. Start it here.

            val drawable = MediaControlDrawables.getProgress(context)

            val drawable =

                context.getDrawable(com.android.internal.R.drawable.progress_small_material)

            (drawable as Animatable).start()

            MediaAction(

                drawable,

                null, // no action to perform when clicked

                context.getString(R.string.controls_media_button_connecting),

                MediaControlDrawables.getConnecting(context),

                context.getDrawable(R.drawable.ic_media_connecting_container),

                // Specify a rebind id to prevent the spinner from restarting on later binds.

                com.android.internal.R.drawable.progress_small_material

            )
@@ -153,18 +154,18 @@ private fun getStandardAction( 
    return when (action) {

        PlaybackState.ACTION_PLAY -> {

            MediaAction(

                MediaControlDrawables.getPlayIcon(context),

                context.getDrawable(R.drawable.ic_media_play),

                { controller.transportControls.play() },

                context.getString(R.string.controls_media_button_play),

                MediaControlDrawables.getPlayBackground(context)

                context.getDrawable(R.drawable.ic_media_play_container)

            )

        }

        PlaybackState.ACTION_PAUSE -> {

            MediaAction(

                MediaControlDrawables.getPauseIcon(context),

                context.getDrawable(R.drawable.ic_media_pause),

                { controller.transportControls.pause() },

                context.getString(R.string.controls_media_button_pause),

                MediaControlDrawables.getPauseBackground(context)

                context.getDrawable(R.drawable.ic_media_pause_container)

            )

        }

        PlaybackState.ACTION_SKIP_TO_PREVIOUS -> {

packages/SystemUI/src/com/android/systemui/media/controls/domain/pipeline/MediaDataProcessor.kt

+1 −2
Original line number Diff line number Diff line
@@ -71,7 +71,6 @@ import com.android.systemui.media.controls.data.repository.MediaDataRepository 
import com.android.systemui.media.controls.domain.pipeline.MediaDataManager.Companion.isMediaNotification

import com.android.systemui.media.controls.domain.pipeline.interactor.MediaCarouselInteractor

import com.android.systemui.media.controls.domain.resume.ResumeMediaBrowser

import com.android.systemui.media.controls.shared.MediaControlDrawables

import com.android.systemui.media.controls.shared.model.EXTRA_KEY_TRIGGER_SOURCE

import com.android.systemui.media.controls.shared.model.EXTRA_VALUE_TRIGGER_PERIODIC

import com.android.systemui.media.controls.shared.model.MediaAction
@@ -1229,7 +1228,7 @@ class MediaDataProcessor( 
                .loadDrawable(context),

            action,

            context.getString(R.string.controls_media_resume),

            MediaControlDrawables.getPlayBackground(context)

            context.getDrawable(R.drawable.ic_media_play_container)

        )

    }

packages/SystemUI/src/com/android/systemui/media/controls/shared/MediaControlDrawables.kt

+0 −98
Original line number Diff line number Diff line
@@ -17,20 +17,12 @@ 
package com.android.systemui.media.controls.shared



import android.content.Context

import android.graphics.drawable.AnimatedVectorDrawable

import android.graphics.drawable.Drawable

import com.android.systemui.Flags.mediaControlsDrawablesReuse

import com.android.systemui.res.R



object MediaControlDrawables {



    // Play/Pause Button drawables.

    private var progress: Drawable? = null

    private var connecting: Drawable? = null

    private var playIcon: AnimatedVectorDrawable? = null

    private var playBackground: AnimatedVectorDrawable? = null

    private var pauseIcon: AnimatedVectorDrawable? = null

    private var pauseBackground: AnimatedVectorDrawable? = null

    // Prev button.

    private var prevIcon: Drawable? = null

    // Next button.
@@ -40,81 +32,6 @@ object MediaControlDrawables { 
    private var antenna: Drawable? = null

    private var groupDevice: Drawable? = null

    private var homeDevices: Drawable? = null

    // Guts drawables.

    private var outline: Drawable? = null

    private var solid: Drawable? = null



    fun getProgress(context: Context): Drawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(com.android.internal.R.drawable.progress_small_material)

        }

        return progress?.mutate()

            ?: context.getDrawable(com.android.internal.R.drawable.progress_small_material).also {

                progress = it

            }

    }



    fun getConnecting(context: Context): Drawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(R.drawable.ic_media_connecting_container)

        }

        return connecting?.mutate()

            ?: context.getDrawable(R.drawable.ic_media_connecting_container).also {

                connecting = it

            }

    }



    fun getPlayIcon(context: Context): AnimatedVectorDrawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(R.drawable.ic_media_play) as AnimatedVectorDrawable?

        }

        return playIcon?.let {

            it.reset()

            it.mutate() as AnimatedVectorDrawable

        }

            ?: (context.getDrawable(R.drawable.ic_media_play) as AnimatedVectorDrawable?).also {

                playIcon = it

            }

    }



    fun getPlayBackground(context: Context): AnimatedVectorDrawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(R.drawable.ic_media_play_container)

                as AnimatedVectorDrawable?

        }

        return playBackground?.let {

            it.reset()

            it.mutate() as AnimatedVectorDrawable

        }

            ?: (context.getDrawable(R.drawable.ic_media_play_container) as AnimatedVectorDrawable?)

                .also { playBackground = it }

    }



    fun getPauseIcon(context: Context): AnimatedVectorDrawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(R.drawable.ic_media_pause) as AnimatedVectorDrawable?

        }

        return pauseIcon?.let {

            it.reset()

            it.mutate() as AnimatedVectorDrawable

        }

            ?: (context.getDrawable(R.drawable.ic_media_pause) as AnimatedVectorDrawable?).also {

                pauseIcon = it

            }

    }



    fun getPauseBackground(context: Context): AnimatedVectorDrawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(R.drawable.ic_media_pause_container)

                as AnimatedVectorDrawable?

        }

        return pauseBackground?.let {

            it.reset()

            it.mutate() as AnimatedVectorDrawable

        }

            ?: (context.getDrawable(R.drawable.ic_media_pause_container) as AnimatedVectorDrawable?)

                .also { pauseBackground = it }

    }



    fun getNextIcon(context: Context): Drawable? {

        if (!mediaControlsDrawablesReuse()) {
@@ -165,19 +82,4 @@ object MediaControlDrawables { 
        return homeDevices

            ?: context.getDrawable(R.drawable.ic_media_home_devices).also { homeDevices = it }

    }



    fun getOutline(context: Context): Drawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(R.drawable.qs_media_outline_button)

        }

        return outline

            ?: context.getDrawable(R.drawable.qs_media_outline_button).also { outline = it }

    }



    fun getSolid(context: Context): Drawable? {

        if (!mediaControlsDrawablesReuse()) {

            return context.getDrawable(R.drawable.qs_media_solid_button)

        }

        return solid ?: context.getDrawable(R.drawable.qs_media_solid_button).also { solid = it }

    }

}

packages/SystemUI/src/com/android/systemui/media/controls/ui/viewmodel/MediaControlViewModel.kt

+2 −3
Original line number Diff line number Diff line
@@ -30,7 +30,6 @@ import com.android.systemui.common.shared.model.Icon 
import com.android.systemui.dagger.qualifiers.Application

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

import com.android.systemui.media.controls.domain.pipeline.interactor.MediaControlInteractor

import com.android.systemui.media.controls.shared.MediaControlDrawables

import com.android.systemui.media.controls.shared.model.MediaAction

import com.android.systemui.media.controls.shared.model.MediaButton

import com.android.systemui.media.controls.shared.model.MediaControlModel
@@ -285,9 +284,9 @@ class MediaControlViewModel( 
            },

            cancelTextBackground =

                if (model.isDismissible) {

                    MediaControlDrawables.getOutline(applicationContext)

                    applicationContext.getDrawable(R.drawable.qs_media_outline_button)

                } else {

                    MediaControlDrawables.getSolid(applicationContext)

                    applicationContext.getDrawable(R.drawable.qs_media_solid_button)

                },

            onSettingsClicked = {

                logger.logLongPressSettings(model.uid, model.packageName, model.instanceId)

packages/SystemUI/tests/src/com/android/systemui/media/controls/domain/pipeline/MediaDataProcessorTest.kt

+0 −9
Original line number Diff line number Diff line
@@ -1835,10 +1835,6 @@ class MediaDataProcessorTest(flags: FlagsParameterization) : SysuiTestCase() { 


        assertThat(userEntries).hasSize(1)

        val secondSemanticActions = userEntries?.values?.toList()?.get(0)?.semanticActions!!

        assertThat(secondSemanticActions.playOrPause?.icon)

            .isEqualTo(firstSemanticActions.playOrPause?.icon)

        assertThat(secondSemanticActions.playOrPause?.background)

            .isEqualTo(firstSemanticActions.playOrPause?.background)

        assertThat(secondSemanticActions.nextOrCustom?.icon)

            .isEqualTo(firstSemanticActions.nextOrCustom?.icon)

        assertThat(secondSemanticActions.prevOrCustom?.icon)
@@ -1873,11 +1869,6 @@ class MediaDataProcessorTest(flags: FlagsParameterization) : SysuiTestCase() { 


        assertThat(userEntries).hasSize(1)

        val secondSemanticActions = userEntries?.values?.toList()?.get(0)?.semanticActions!!



        assertThat(secondSemanticActions.playOrPause?.icon)

            .isNotEqualTo(firstSemanticActions.playOrPause?.icon)

        assertThat(secondSemanticActions.playOrPause?.background)

            .isNotEqualTo(firstSemanticActions.playOrPause?.background)

        assertThat(secondSemanticActions.nextOrCustom?.icon)

            .isNotEqualTo(firstSemanticActions.nextOrCustom?.icon)

        assertThat(secondSemanticActions.prevOrCustom?.icon)