From 0dea0b2a7c3751cca864d8e9734bb4fd5282eb9a Mon Sep 17 00:00:00 2001 From: Hawkwood Glazier Date: Mon, 25 Apr 2022 17:45:47 +0000 Subject: [PATCH] Fix corner clipping on UMO This also includes a change to match POR for when album art is missing. Additionally, I animated the scrim transition since I was editing that code anyway. Test: Manual Fixes: 226275320 Fixes: 230382916 Change-Id: I1d339012c78241f064815b4540cdb344747d609f --- .../drawable/qs_media_outline_album_bg.xml | 21 +++++ .../drawable/qs_media_outline_layout_bg.xml | 21 +++++ .../SystemUI/res/drawable/qs_media_scrim.xml | 1 - .../res/layout/media_session_view.xml | 5 +- .../systemui/media/ColorSchemeTransition.kt | 84 +++++++++---------- .../systemui/media/MediaControlPanel.java | 27 ++---- .../android/systemui/media/MediaViewHolder.kt | 18 ---- .../media/ColorSchemeTransitionTest.kt | 10 +-- .../systemui/media/MediaControlPanelTest.kt | 7 ++ 9 files changed, 102 insertions(+), 92 deletions(-) create mode 100644 packages/SystemUI/res/drawable/qs_media_outline_album_bg.xml create mode 100644 packages/SystemUI/res/drawable/qs_media_outline_layout_bg.xml diff --git a/packages/SystemUI/res/drawable/qs_media_outline_album_bg.xml b/packages/SystemUI/res/drawable/qs_media_outline_album_bg.xml new file mode 100644 index 0000000000000..ecd3876356bc0 --- /dev/null +++ b/packages/SystemUI/res/drawable/qs_media_outline_album_bg.xml @@ -0,0 +1,21 @@ + + + + + + diff --git a/packages/SystemUI/res/drawable/qs_media_outline_layout_bg.xml b/packages/SystemUI/res/drawable/qs_media_outline_layout_bg.xml new file mode 100644 index 0000000000000..4ba45c47f7fda --- /dev/null +++ b/packages/SystemUI/res/drawable/qs_media_outline_layout_bg.xml @@ -0,0 +1,21 @@ + + + + + + diff --git a/packages/SystemUI/res/drawable/qs_media_scrim.xml b/packages/SystemUI/res/drawable/qs_media_scrim.xml index 2ec319c6e253f..159414d63afc3 100644 --- a/packages/SystemUI/res/drawable/qs_media_scrim.xml +++ b/packages/SystemUI/res/drawable/qs_media_scrim.xml @@ -16,7 +16,6 @@ --> - diff --git a/packages/SystemUI/src/com/android/systemui/media/ColorSchemeTransition.kt b/packages/SystemUI/src/com/android/systemui/media/ColorSchemeTransition.kt index f93ce377938be..6e8423801a5d3 100644 --- a/packages/SystemUI/src/com/android/systemui/media/ColorSchemeTransition.kt +++ b/packages/SystemUI/src/com/android/systemui/media/ColorSchemeTransition.kt @@ -21,6 +21,7 @@ import android.animation.ValueAnimator.AnimatorUpdateListener import android.animation.ValueAnimator import android.content.Context import android.content.res.ColorStateList +import android.graphics.Color import android.graphics.drawable.GradientDrawable import android.graphics.drawable.RippleDrawable import com.android.internal.R @@ -37,13 +38,6 @@ interface ColorTransition { fun updateColorScheme(scheme: ColorScheme?) } -/** A generic implementation of [ColorTransition] so that we can define a factory method. */ -open class GenericColorTransition( - private val applyTheme: (ColorScheme?) -> Unit -) : ColorTransition { - override fun updateColorScheme(scheme: ColorScheme?) = applyTheme(scheme) -} - /** * A [ColorTransition] that animates between two specific colors. * It uses a ValueAnimator to execute the animation and interpolate between the source color and @@ -96,7 +90,6 @@ open class AnimatingColorTransition( typealias AnimatingColorTransitionFactory = (Int, (ColorScheme) -> Int, (Int) -> Unit) -> AnimatingColorTransition -typealias GenericColorTransitionFactory = ((ColorScheme?) -> Unit) -> GenericColorTransition /** * ColorSchemeTransition constructs a ColorTransition for each color in the scheme @@ -105,23 +98,22 @@ typealias GenericColorTransitionFactory = ((ColorScheme?) -> Unit) -> GenericCol */ class ColorSchemeTransition internal constructor( private val context: Context, - mediaViewHolder: MediaViewHolder, - animatingColorTransitionFactory: AnimatingColorTransitionFactory, - genericColorTransitionFactory: GenericColorTransitionFactory + private val mediaViewHolder: MediaViewHolder, + animatingColorTransitionFactory: AnimatingColorTransitionFactory ) { constructor(context: Context, mediaViewHolder: MediaViewHolder) : - this(context, mediaViewHolder, ::AnimatingColorTransition, ::GenericColorTransition) + this(context, mediaViewHolder, ::AnimatingColorTransition) + private var isGradientEnabled = true val bgColor = context.getColor(com.android.systemui.R.color.material_dynamic_secondary95) - val surfaceColor = animatingColorTransitionFactory( bgColor, ::surfaceFromScheme ) { surfaceColor -> val colorList = ColorStateList.valueOf(surfaceColor) - mediaViewHolder.player.backgroundTintList = colorList mediaViewHolder.seamlessIcon.imageTintList = colorList mediaViewHolder.seamlessText.setTextColor(surfaceColor) + mediaViewHolder.albumView.backgroundTintList = colorList mediaViewHolder.gutsViewHolder.setSurfaceColor(surfaceColor) } @@ -181,36 +173,15 @@ class ColorSchemeTransition internal constructor( mediaViewHolder.seekBar.progressBackgroundTintList = ColorStateList.valueOf(textTertiary) } - // Note: This background gradient currently doesn't animate between colors. - val backgroundGradient = genericColorTransitionFactory { scheme -> - val defaultTintColor = ColorStateList.valueOf(bgColor) - if (scheme == null) { - mediaViewHolder.albumView.foregroundTintList = defaultTintColor - mediaViewHolder.albumView.backgroundTintList = defaultTintColor - return@genericColorTransitionFactory - } + val bgGradientStart = animatingColorTransitionFactory( + bgColor, + albumGradientPicker(::backgroundStartFromScheme, 0.25f) + ) { _ -> updateAlbumGradient() } - // If there's no album art, just hide the gradient so we show the solid background. - val showGradient = mediaViewHolder.albumView.drawable != null - val startColor = getColorWithAlpha( - backgroundStartFromScheme(scheme), - alpha = if (showGradient) .25f else 0f - ) - val endColor = getColorWithAlpha( - backgroundEndFromScheme(scheme), - alpha = if (showGradient) .90f else 0f - ) - val gradientColors = intArrayOf(startColor, endColor) - - val foregroundGradient = mediaViewHolder.albumView.foreground?.mutate() - if (foregroundGradient is GradientDrawable) { - foregroundGradient.colors = gradientColors - } - val backgroundGradient = mediaViewHolder.albumView.background?.mutate() - if (backgroundGradient is GradientDrawable) { - backgroundGradient.colors = gradientColors - } - } + val bgGradientEnd = animatingColorTransitionFactory( + bgColor, + albumGradientPicker(::backgroundEndFromScheme, 0.9f) + ) { _ -> updateAlbumGradient() } val colorTransitions = arrayOf( surfaceColor, @@ -220,14 +191,37 @@ class ColorSchemeTransition internal constructor( textPrimaryInverse, textSecondary, textTertiary, - backgroundGradient + bgGradientStart, + bgGradientEnd ) + private fun updateAlbumGradient() { + val gradient = mediaViewHolder.albumView.foreground?.mutate() + if (gradient is GradientDrawable) { + gradient.colors = intArrayOf( + bgGradientStart?.currentColor ?: 0, + bgGradientEnd?.currentColor ?: 0) + } + } + + private fun albumGradientPicker( + inner: (ColorScheme) -> Int, + targetAlpha: Float + ): (ColorScheme) -> Int { + return { scheme -> + if (isGradientEnabled) + getColorWithAlpha(inner(scheme), targetAlpha) + else + Color.TRANSPARENT + } + } + private fun loadDefaultColor(id: Int): Int { return Utils.getColorAttr(context, id).defaultColor } - fun updateColorScheme(colorScheme: ColorScheme?) { + fun updateColorScheme(colorScheme: ColorScheme?, enableGradient: Boolean) { + isGradientEnabled = enableGradient colorTransitions.forEach { it.updateColorScheme(colorScheme) } } } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java index 20417aff024c5..06e6c974e2e0d 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java +++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java @@ -340,6 +340,10 @@ public class MediaControlPanel { } }); + // AlbumView uses a hardware layer so that clipping of the foreground is handled + // with clipping the album art. Otherwise album art shows through at the edges. + mMediaViewHolder.getAlbumView().setLayerType(View.LAYER_TYPE_HARDWARE, null); + TextView titleText = mMediaViewHolder.getTitleText(); TextView artistText = mMediaViewHolder.getArtistText(); AnimatorSet enter = loadAnimator(R.anim.media_metadata_enter, @@ -570,8 +574,8 @@ public class MediaControlPanel { } // Capture width & height from views in foreground for artwork scaling in background - int width = mMediaViewHolder.getPlayer().getWidth(); - int height = mMediaViewHolder.getPlayer().getHeight(); + int width = mMediaViewHolder.getAlbumView().getMeasuredWidth(); + int height = mMediaViewHolder.getAlbumView().getMeasuredHeight(); // WallpaperColors.fromBitmap takes a good amount of time. We do that work // on the background executor to avoid stalling animations on the UI Thread. @@ -609,7 +613,6 @@ public class MediaControlPanel { // Bind the album view to the artwork or a transition drawable ImageView albumView = mMediaViewHolder.getAlbumView(); albumView.setPadding(0, 0, 0, 0); - albumView.setClipToOutline(true); if (updateBackground || (!mIsArtworkBound && isArtworkBound)) { if (mPrevArtwork == null) { albumView.setImageDrawable(artwork); @@ -627,7 +630,7 @@ public class MediaControlPanel { } // Transition Colors to current color scheme - mColorSchemeTransition.updateColorScheme(colorScheme); + mColorSchemeTransition.updateColorScheme(colorScheme, mIsArtworkBound); // App icon - use notification icon ImageView appIconView = mMediaViewHolder.getAppIcon(); @@ -894,7 +897,7 @@ public class MediaControlPanel { InteractionJankMonitor.CUJ_SHADE_APP_LAUNCH_FROM_MEDIA_PLAYER) { @Override protected float getCurrentTopCornerRadius() { - return ((IlluminationDrawable) player.getBackground()).getCornerRadius(); + return mContext.getResources().getDimension(R.dimen.notification_corner_radius); } @Override @@ -902,20 +905,6 @@ public class MediaControlPanel { // TODO(b/184121838): Make IlluminationDrawable support top and bottom radius. return getCurrentTopCornerRadius(); } - - @Override - protected void setBackgroundCornerRadius(Drawable background, float topCornerRadius, - float bottomCornerRadius) { - // TODO(b/184121838): Make IlluminationDrawable support top and bottom radius. - float radius = Math.min(topCornerRadius, bottomCornerRadius); - ((IlluminationDrawable) background).setCornerRadiusOverride(radius); - } - - @Override - public void onLaunchAnimationEnd(boolean isExpandingFullyAbove) { - super.onLaunchAnimationEnd(isExpandingFullyAbove); - ((IlluminationDrawable) player.getBackground()).setCornerRadiusOverride(null); - } }; } diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt b/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt index 5c93cdaeb0dae..fc9515c050e5c 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaViewHolder.kt @@ -69,23 +69,6 @@ class MediaViewHolder constructor(itemView: View) { val actionsTopBarrier = itemView.requireViewById(R.id.media_action_barrier_top) - init { - (player.background as IlluminationDrawable).let { - it.registerLightSource(seamless) - it.registerLightSource(gutsViewHolder.cancel) - it.registerLightSource(gutsViewHolder.dismiss) - it.registerLightSource(gutsViewHolder.settings) - it.registerLightSource(actionPlayPause) - it.registerLightSource(actionNext) - it.registerLightSource(actionPrev) - it.registerLightSource(action0) - it.registerLightSource(action1) - it.registerLightSource(action2) - it.registerLightSource(action3) - it.registerLightSource(action4) - } - } - fun getAction(id: Int): ImageButton { return when (id) { R.id.actionPlayPause -> actionPlayPause @@ -161,7 +144,6 @@ class MediaViewHolder constructor(itemView: View) { R.id.media_scrubbing_total_time ) - // Buttons used for notification-based actions val genericButtonIds = setOf( R.id.action0, diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/ColorSchemeTransitionTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/ColorSchemeTransitionTest.kt index 5463977ec9aea..ea841b7fd7a13 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/ColorSchemeTransitionTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/ColorSchemeTransitionTest.kt @@ -50,14 +50,12 @@ class ColorSchemeTransitionTest : SysuiTestCase() { private lateinit var colorSchemeTransition: ColorSchemeTransition @Mock private lateinit var mockAnimatingTransition: AnimatingColorTransition - @Mock private lateinit var mockGenericTransition: GenericColorTransition @Mock private lateinit var valueAnimator: ValueAnimator @Mock private lateinit var colorScheme: ColorScheme @Mock private lateinit var extractColor: ExtractCB @Mock private lateinit var applyColor: ApplyCB private lateinit var animatingColorTransitionFactory: AnimatingColorTransitionFactory - private lateinit var genericColorTransitionFactory: GenericColorTransitionFactory @Mock private lateinit var mediaViewHolder: MediaViewHolder @JvmField @Rule val mockitoRule = MockitoJUnit.rule() @@ -65,11 +63,10 @@ class ColorSchemeTransitionTest : SysuiTestCase() { @Before fun setUp() { animatingColorTransitionFactory = { _, _, _ -> mockAnimatingTransition } - genericColorTransitionFactory = { _ -> mockGenericTransition } whenever(extractColor.invoke(colorScheme)).thenReturn(TARGET_COLOR) colorSchemeTransition = ColorSchemeTransition( - context, mediaViewHolder, animatingColorTransitionFactory, genericColorTransitionFactory + context, mediaViewHolder, animatingColorTransitionFactory ) colorTransition = object : AnimatingColorTransition( @@ -148,8 +145,7 @@ class ColorSchemeTransitionTest : SysuiTestCase() { @Test fun testColorSchemeTransition_update() { - colorSchemeTransition.updateColorScheme(colorScheme) - verify(mockAnimatingTransition, times(7)).updateColorScheme(colorScheme) - verify(mockGenericTransition).updateColorScheme(colorScheme) + colorSchemeTransition.updateColorScheme(colorScheme, true) + verify(mockAnimatingTransition, times(9)).updateColorScheme(colorScheme) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt index 94254da0d7deb..5a9256bfcafc2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt @@ -534,6 +534,13 @@ public class MediaControlPanelTest : SysuiTestCase() { verify(expandedSet).setVisibility(R.id.actionNext, ConstraintSet.INVISIBLE) } + @Test + fun bindAlbumView_testHardwareAfterAttach() { + player.attachPlayer(viewHolder) + + verify(albumView).setLayerType(View.LAYER_TYPE_HARDWARE, null) + } + @Test fun bindAlbumView_setAfterExecutors() { val bmp = Bitmap.createBitmap(10, 10, Bitmap.Config.ARGB_8888)