From 389edcd7c56b886abe2df23eba38cb8aa40082f0 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Thu, 11 May 2017 19:16:44 -0700 Subject: [PATCH] Fixed an issue where the media notification wouldn't have contrast Because we were relying on the output to go in the right direction already, this could be wrong. We're now only following the given lightness direction if it is even possible to satisfy contrast. Test: runtest -x core/tests/coretests/src/android/app/NotificationTest.java Change-Id: I06d934a6b5c328ebbf0cf707030b0d707ccb5ab4 Fixes: 38182819 --- core/java/android/app/Notification.java | 56 ++++++++++++--- .../internal/util/NotificationColorUtil.java | 8 +++ .../src/android/app/NotificationTest.java | 69 +++++++++++++++++++ .../MediaNotificationProcessor.java | 3 +- 4 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 core/tests/coretests/src/android/app/NotificationTest.java diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 4294eab222aa2..4a076945bee46 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -16,6 +16,8 @@ package android.app; +import static com.android.internal.util.NotificationColorUtil.satisfiesTextContrast; + import android.annotation.ColorInt; import android.annotation.DrawableRes; import android.annotation.IntDef; @@ -42,7 +44,6 @@ import android.media.PlayerBase; import android.media.session.MediaSession; import android.net.Uri; import android.os.BadParcelableException; -import android.os.Binder; import android.os.Build; import android.os.Bundle; import android.os.IBinder; @@ -72,6 +73,7 @@ import android.widget.ProgressBar; import android.widget.RemoteViews; import com.android.internal.R; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; import com.android.internal.util.NotificationColorUtil; import com.android.internal.util.Preconditions; @@ -2670,6 +2672,19 @@ public class Notification implements Parcelable private static final boolean USE_ONLY_TITLE_IN_LOW_PRIORITY_SUMMARY = SystemProperties.getBoolean("notifications.only_title", true); + /** + * The lightness difference that has to be added to the primary text color to obtain the + * secondary text color when the background is light. + */ + private static final int LIGHTNESS_TEXT_DIFFERENCE_LIGHT = 20; + + /** + * The lightness difference that has to be added to the primary text color to obtain the + * secondary text color when the background is dark. + * A bit less then the above value, since it looks better on dark backgrounds. + */ + private static final int LIGHTNESS_TEXT_DIFFERENCE_DARK = -10; + private Context mContext; private Notification mN; private Bundle mUserExtras = new Bundle(); @@ -3835,11 +3850,26 @@ public class Notification implements Parcelable contentView.setTextColor(id, mPrimaryTextColor); } - private int getPrimaryTextColor() { + /** + * @return the primary text color + * @hide + */ + @VisibleForTesting + public int getPrimaryTextColor() { ensureColors(); return mPrimaryTextColor; } + /** + * @return the secondary text color + * @hide + */ + @VisibleForTesting + public int getSecondaryTextColor() { + ensureColors(); + return mSecondaryTextColor; + } + private int getActionBarColor() { ensureColors(); return mActionBarColor; @@ -3879,16 +3909,21 @@ public class Notification implements Parcelable double textLum = NotificationColorUtil.calculateLuminance(mForegroundColor); double contrast = NotificationColorUtil.calculateContrast(mForegroundColor, backgroundColor); - boolean textDark = backLum > textLum; + // We only respect the given colors if worst case Black or White still has + // contrast + boolean backgroundLight = backLum > textLum + && satisfiesTextContrast(backgroundColor, Color.BLACK) + || backLum <= textLum + && !satisfiesTextContrast(backgroundColor, Color.WHITE); if (contrast < 4.5f) { - if (textDark) { + if (backgroundLight) { mSecondaryTextColor = NotificationColorUtil.findContrastColor( mForegroundColor, backgroundColor, true /* findFG */, 4.5f); mPrimaryTextColor = NotificationColorUtil.changeColorLightness( - mSecondaryTextColor, -20); + mSecondaryTextColor, -LIGHTNESS_TEXT_DIFFERENCE_LIGHT); } else { mSecondaryTextColor = NotificationColorUtil.findContrastColorAgainstDark( @@ -3897,16 +3932,17 @@ public class Notification implements Parcelable true /* findFG */, 4.5f); mPrimaryTextColor = NotificationColorUtil.changeColorLightness( - mSecondaryTextColor, 10); + mSecondaryTextColor, -LIGHTNESS_TEXT_DIFFERENCE_DARK); } } else { mPrimaryTextColor = mForegroundColor; mSecondaryTextColor = NotificationColorUtil.changeColorLightness( - mPrimaryTextColor, textDark ? 10 : -20); + mPrimaryTextColor, backgroundLight ? LIGHTNESS_TEXT_DIFFERENCE_LIGHT + : LIGHTNESS_TEXT_DIFFERENCE_DARK); if (NotificationColorUtil.calculateContrast(mSecondaryTextColor, backgroundColor) < 4.5f) { // oh well the secondary is not good enough - if (textDark) { + if (backgroundLight) { mSecondaryTextColor = NotificationColorUtil.findContrastColor( mSecondaryTextColor, backgroundColor, @@ -3921,7 +3957,9 @@ public class Notification implements Parcelable 4.5f); } mPrimaryTextColor = NotificationColorUtil.changeColorLightness( - mSecondaryTextColor, textDark ? -20 : 10); + mSecondaryTextColor, backgroundLight + ? -LIGHTNESS_TEXT_DIFFERENCE_LIGHT + : -LIGHTNESS_TEXT_DIFFERENCE_DARK); } } } diff --git a/core/java/com/android/internal/util/NotificationColorUtil.java b/core/java/com/android/internal/util/NotificationColorUtil.java index cd41f9e9f9020..1ba92bfb6a773 100644 --- a/core/java/com/android/internal/util/NotificationColorUtil.java +++ b/core/java/com/android/internal/util/NotificationColorUtil.java @@ -533,6 +533,10 @@ public class NotificationColorUtil { return ColorUtilsFromCompat.calculateContrast(foregroundColor, backgroundColor); } + public static boolean satisfiesTextContrast(int backgroundColor, int foregroundColor) { + return NotificationColorUtil.calculateContrast(backgroundColor, foregroundColor) >= 4.5; + } + /** * Composite two potentially translucent colors over each other and returns the result. */ @@ -540,6 +544,10 @@ public class NotificationColorUtil { return ColorUtilsFromCompat.compositeColors(foreground, background); } + public static boolean isColorLight(int backgroundColor) { + return calculateLuminance(backgroundColor) > 0.5f; + } + /** * Framework copy of functions needed from android.support.v4.graphics.ColorUtils. */ diff --git a/core/tests/coretests/src/android/app/NotificationTest.java b/core/tests/coretests/src/android/app/NotificationTest.java new file mode 100644 index 0000000000000..ac1ac195b7251 --- /dev/null +++ b/core/tests/coretests/src/android/app/NotificationTest.java @@ -0,0 +1,69 @@ +/* + * Copyright (C) 2017 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 android.app; + +import static com.android.internal.util.NotificationColorUtil.satisfiesTextContrast; + +import static org.junit.Assert.assertTrue; + +import android.content.Context; +import android.media.session.MediaSession; +import android.support.test.InstrumentationRegistry; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import com.android.internal.util.NotificationColorUtil; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class NotificationTest { + + private Context mContext; + + @Before + public void setUp() { + mContext = InstrumentationRegistry.getContext(); + } + + @Test + public void testColorSatisfiedWhenBgDarkTextDarker() { + Notification.Builder builder = getMediaNotification(); + builder.build(); + + // An initial guess where the foreground color is actually darker than an already dark bg + int backgroundColor = 0xff585868; + int initialForegroundColor = 0xff505868; + builder.setColorPalette(backgroundColor, initialForegroundColor); + int primaryTextColor = builder.getPrimaryTextColor(); + assertTrue(satisfiesTextContrast(primaryTextColor, backgroundColor)); + int secondaryTextColor = builder.getSecondaryTextColor(); + assertTrue(satisfiesTextContrast(secondaryTextColor, backgroundColor)); + } + + private Notification.Builder getMediaNotification() { + MediaSession session = new MediaSession(mContext, "test"); + return new Notification.Builder(mContext, "color") + .setSmallIcon(com.android.internal.R.drawable.emergency_icon) + .setContentTitle("Title") + .setContentText("Text") + .setStyle(new Notification.MediaStyle().setMediaSession(session.getSessionToken())); + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java index 52c053f411261..d0a6fcf19cae4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java @@ -27,6 +27,7 @@ import android.support.v4.graphics.ColorUtils; import android.support.v7.graphics.Palette; import android.util.LayoutDirection; +import com.android.internal.util.NotificationColorUtil; import com.android.systemui.R; import java.util.List; @@ -110,7 +111,7 @@ public class MediaNotificationProcessor { paletteBuilder.addFilter(mBlackWhiteFilter); palette = paletteBuilder.generate(); int foregroundColor; - if (ColorUtils.calculateLuminance(backgroundColor) > 0.5) { + if (NotificationColorUtil.isColorLight(backgroundColor)) { Palette.Swatch first = palette.getDarkVibrantSwatch(); Palette.Swatch second = palette.getVibrantSwatch(); if (first != null && second != null) {