From c0823b6c0b2f5216c4f742120cbf30de3b6bc3ef Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Fri, 15 Jul 2022 22:41:02 +0000 Subject: [PATCH] Fix LightBarController's dark/light logic for the Navigation Bar The old logic failed to account for the notification shade's white scrim in light theme appearing underneath the navigation bar and didn't correctly keep the navigation bar in light mode for that state, making for white buttons on a nearly white background. However, when accounting for the new possibility of an override to light mode, a pair of bug was discovered that cancelled each other out but made intermediate logic very confusing to follow. It's still confusing, but now it's more correct. * GradientColors.supportsDarkText() was set backward by the ScrimController. This has been fixed. * Clarified in the docs for the fields that mNavigationLight and related fields use "Light" to mean the theme / background, which means that the button color would be dark. These locals were interpreted incorrectly in some places, cancelling out the above incorrectness. * We never had light background scrims before, but we do now, so the other main thing this logical change does is introduce the ability for the scrim to override the theme to light (thus forcing the buttons to be dark). * GlobalActionsDialogLite needed to be updated to let the LightBarController know when it was visible, since that would require light status bars. * Finally, I added a ton of debug logging (off by default) and more info in the dumps that allows validating the progression of the logic for any state. Bug: 195622153 Test: atest LightBarControllerTest Flag: NEW_LIGHT_BAR_LOGIC Change-Id: I380a8d03fdebace7dacba5f6c0b610540acc0c5a --- .../GlobalActionsDialogLite.java | 10 ++ .../shade/QuickSettingsController.java | 7 + .../statusbar/phone/LightBarController.java | 163 +++++++++++++++--- .../statusbar/phone/ScrimController.java | 22 ++- .../GlobalActionsDialogLiteTest.java | 3 + ...tificationPanelViewControllerBaseTest.java | 3 + .../shade/QuickSettingsControllerTest.java | 3 + .../phone/LightBarControllerTest.java | 81 +++++++++ .../LightBarControllerWithNewLogicTest.kt | 28 +++ 9 files changed, 295 insertions(+), 25 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerWithNewLogicTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java index d3b6fc2370845..f64ed6078ddb1 100644 --- a/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java +++ b/packages/SystemUI/src/com/android/systemui/globalactions/GlobalActionsDialogLite.java @@ -128,6 +128,7 @@ import com.android.systemui.settings.UserTracker; import com.android.systemui.statusbar.NotificationShadeWindowController; import com.android.systemui.statusbar.VibratorHelper; import com.android.systemui.statusbar.phone.CentralSurfaces; +import com.android.systemui.statusbar.phone.LightBarController; import com.android.systemui.statusbar.phone.SystemUIDialog; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.KeyguardStateController; @@ -240,6 +241,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene private final ScreenshotHelper mScreenshotHelper; private final SysuiColorExtractor mSysuiColorExtractor; private final IStatusBarService mStatusBarService; + protected final LightBarController mLightBarController; protected final NotificationShadeWindowController mNotificationShadeWindowController; private final IWindowManager mIWindowManager; private final Executor mBackgroundExecutor; @@ -349,6 +351,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene MetricsLogger metricsLogger, SysuiColorExtractor colorExtractor, IStatusBarService statusBarService, + LightBarController lightBarController, NotificationShadeWindowController notificationShadeWindowController, IWindowManager iWindowManager, @Background Executor backgroundExecutor, @@ -381,6 +384,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene mUiEventLogger = uiEventLogger; mSysuiColorExtractor = colorExtractor; mStatusBarService = statusBarService; + mLightBarController = lightBarController; mNotificationShadeWindowController = notificationShadeWindowController; mIWindowManager = iWindowManager; mBackgroundExecutor = backgroundExecutor; @@ -694,6 +698,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene ActionsDialogLite dialog = new ActionsDialogLite(mContext, com.android.systemui.R.style.Theme_SystemUI_Dialog_GlobalActionsLite, mAdapter, mOverflowAdapter, mSysuiColorExtractor, mStatusBarService, + mLightBarController, mNotificationShadeWindowController, this::onRefresh, mKeyguardShowing, mPowerAdapter, mUiEventLogger, mCentralSurfacesOptional, mKeyguardUpdateMonitor, mLockPatternUtils); @@ -2192,6 +2197,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene protected final SysuiColorExtractor mColorExtractor; private boolean mKeyguardShowing; protected float mScrimAlpha; + protected final LightBarController mLightBarController; protected final NotificationShadeWindowController mNotificationShadeWindowController; private ListPopupWindow mOverflowPopup; private Dialog mPowerOptionsDialog; @@ -2267,6 +2273,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene ActionsDialogLite(Context context, int themeRes, MyAdapter adapter, MyOverflowAdapter overflowAdapter, SysuiColorExtractor sysuiColorExtractor, IStatusBarService statusBarService, + LightBarController lightBarController, NotificationShadeWindowController notificationShadeWindowController, Runnable onRefreshCallback, boolean keyguardShowing, MyPowerOptionsAdapter powerAdapter, UiEventLogger uiEventLogger, @@ -2282,6 +2289,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene mPowerOptionsAdapter = powerAdapter; mColorExtractor = sysuiColorExtractor; mStatusBarService = statusBarService; + mLightBarController = lightBarController; mNotificationShadeWindowController = notificationShadeWindowController; mOnRefreshCallback = onRefreshCallback; mKeyguardShowing = keyguardShowing; @@ -2474,6 +2482,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene @Override protected void start() { mGlobalActionsLayout.updateList(); + mLightBarController.setGlobalActionsVisible(true); if (mBackgroundDrawable instanceof ScrimDrawable) { mColorExtractor.addOnColorsChangedListener(this); @@ -2504,6 +2513,7 @@ public class GlobalActionsDialogLite implements DialogInterface.OnDismissListene @Override protected void stop() { + mLightBarController.setGlobalActionsVisible(false); mColorExtractor.removeOnColorsChangedListener(this); } diff --git a/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java b/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java index ef14d1cb7f63d..bfed6642b7b4d 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/QuickSettingsController.java @@ -84,6 +84,7 @@ import com.android.systemui.statusbar.notification.stack.NotificationStackScroll import com.android.systemui.statusbar.notification.stack.StackStateAnimator; import com.android.systemui.statusbar.phone.KeyguardBypassController; import com.android.systemui.statusbar.phone.KeyguardStatusBarView; +import com.android.systemui.statusbar.phone.LightBarController; import com.android.systemui.statusbar.phone.LockscreenGestureLogger; import com.android.systemui.statusbar.phone.ScrimController; import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager; @@ -115,6 +116,7 @@ public class QuickSettingsController { private final PulseExpansionHandler mPulseExpansionHandler; private final ShadeExpansionStateManager mShadeExpansionStateManager; private final StatusBarKeyguardViewManager mStatusBarKeyguardViewManager; + private final LightBarController mLightBarController; private final NotificationStackScrollLayoutController mNotificationStackScrollLayoutController; private final LockscreenShadeTransitionController mLockscreenShadeTransitionController; private final NotificationShadeDepthController mDepthController; @@ -300,6 +302,7 @@ public class QuickSettingsController { NotificationRemoteInputManager remoteInputManager, ShadeExpansionStateManager shadeExpansionStateManager, StatusBarKeyguardViewManager statusBarKeyguardViewManager, + LightBarController lightBarController, NotificationStackScrollLayoutController notificationStackScrollLayoutController, LockscreenShadeTransitionController lockscreenShadeTransitionController, NotificationShadeDepthController notificationShadeDepthController, @@ -340,6 +343,7 @@ public class QuickSettingsController { mRemoteInputManager = remoteInputManager; mShadeExpansionStateManager = shadeExpansionStateManager; mStatusBarKeyguardViewManager = statusBarKeyguardViewManager; + mLightBarController = lightBarController; mNotificationStackScrollLayoutController = notificationStackScrollLayoutController; mLockscreenShadeTransitionController = lockscreenShadeTransitionController; mDepthController = notificationShadeDepthController; @@ -1009,6 +1013,9 @@ public class QuickSettingsController { mShadeHeaderController.setShadeExpandedFraction(shadeExpandedFraction); mShadeHeaderController.setQsExpandedFraction(qsExpansionFraction); mShadeHeaderController.setQsVisible(mVisible); + + // Update the light bar + mLightBarController.setQsExpanded(mFullyExpanded); } float getLockscreenShadeDragProgress() { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java index 534edb97bc5d0..a058bf88e2e4b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java @@ -25,6 +25,7 @@ import static com.android.systemui.statusbar.phone.BarTransitions.MODE_TRANSPARE import android.annotation.ColorInt; import android.content.Context; import android.graphics.Rect; +import android.util.Log; import android.view.InsetsFlags; import android.view.ViewDebug; import android.view.WindowInsetsController.Appearance; @@ -35,13 +36,17 @@ import com.android.systemui.Dumpable; import com.android.systemui.R; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dump.DumpManager; +import com.android.systemui.flags.FeatureFlags; +import com.android.systemui.flags.Flags; import com.android.systemui.navigationbar.NavigationModeController; import com.android.systemui.plugins.DarkIconDispatcher; import com.android.systemui.settings.DisplayTracker; import com.android.systemui.statusbar.policy.BatteryController; +import com.android.systemui.util.Compile; import java.io.PrintWriter; import java.util.ArrayList; +import java.util.Date; import javax.inject.Inject; @@ -51,10 +56,14 @@ import javax.inject.Inject; @SysUISingleton public class LightBarController implements BatteryController.BatteryStateChangeCallback, Dumpable { + private static final String TAG = "LightBarController"; + private static final boolean DEBUG = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.DEBUG); + private static final float NAV_BAR_INVERSION_SCRIM_ALPHA_THRESHOLD = 0.1f; private final SysuiDarkIconDispatcher mStatusBarIconController; private final BatteryController mBatteryController; + private final boolean mUseNewLightBarLogic; private BiometricUnlockController mBiometricUnlockController; private LightBarTransitionsController mNavigationBarController; @@ -67,13 +76,17 @@ public class LightBarController implements BatteryController.BatteryStateChangeC private final int mLightIconColor; /** - * Whether the navigation bar should be light factoring in already how much alpha the scrim has + * Whether the navigation bar should be light factoring in already how much alpha the scrim has. + * "Light" refers to the background color of the navigation bar, so when this is true, + * it's referring to a state where the navigation bar icons are tinted dark. */ private boolean mNavigationLight; /** - * Whether the flags indicate that a light status bar is requested. This doesn't factor in the - * scrim alpha yet. + * Whether the flags indicate that a light navigation bar is requested. + * "Light" refers to the background color of the navigation bar, so when this is true, + * it's referring to a state where the navigation bar icons would be tinted dark. + * This doesn't factor in the scrim alpha yet. */ private boolean mHasLightNavigationBar; @@ -82,22 +95,34 @@ public class LightBarController implements BatteryController.BatteryStateChangeC * {@link #mNavigationLight} {@code false}. */ private boolean mForceDarkForScrim; + /** + * {@code true} if {@link #mHasLightNavigationBar} should be ignored and forcefully make + * {@link #mNavigationLight} {@code true}. + */ + private boolean mForceLightForScrim; private boolean mQsCustomizing; + private boolean mQsExpanded; + private boolean mGlobalActionsVisible; private boolean mDirectReplying; private boolean mNavbarColorManagedByIme; private boolean mIsCustomizingForBackNav; + private String mLastSetScrimStateLog; + private String mLastNavigationBarAppearanceChangedLog; + @Inject public LightBarController( Context ctx, DarkIconDispatcher darkIconDispatcher, BatteryController batteryController, NavigationModeController navModeController, + FeatureFlags featureFlags, DumpManager dumpManager, DisplayTracker displayTracker) { + mUseNewLightBarLogic = featureFlags.isEnabled(Flags.NEW_LIGHT_BAR_LOGIC); mDarkIconColor = ctx.getColor(R.color.dark_mode_icon_color_single_tone); mLightIconColor = ctx.getColor(R.color.light_mode_icon_color_single_tone); mStatusBarIconController = (SysuiDarkIconDispatcher) darkIconDispatcher; @@ -159,9 +184,42 @@ public class LightBarController implements BatteryController.BatteryStateChangeC final boolean last = mNavigationLight; mHasLightNavigationBar = isLight(appearance, navigationBarMode, APPEARANCE_LIGHT_NAVIGATION_BARS); - mNavigationLight = mHasLightNavigationBar - && (mDirectReplying && mNavbarColorManagedByIme || !mForceDarkForScrim) - && !mQsCustomizing; + if (mUseNewLightBarLogic) { + final boolean ignoreScrimForce = mDirectReplying && mNavbarColorManagedByIme; + final boolean darkForScrim = mForceDarkForScrim && !ignoreScrimForce; + final boolean lightForScrim = mForceLightForScrim && !ignoreScrimForce; + final boolean darkForQs = mQsCustomizing || mQsExpanded || mGlobalActionsVisible; + mNavigationLight = + ((mHasLightNavigationBar && !darkForScrim) || lightForScrim) && !darkForQs; + mLastNavigationBarAppearanceChangedLog = "onNavigationBarAppearanceChanged()" + + " appearance=" + appearance + + " nbModeChanged=" + nbModeChanged + + " navigationBarMode=" + navigationBarMode + + " navbarColorManagedByIme=" + navbarColorManagedByIme + + " mHasLightNavigationBar=" + mHasLightNavigationBar + + " ignoreScrimForce=" + ignoreScrimForce + + " darkForScrim=" + darkForScrim + + " lightForScrim=" + lightForScrim + + " darkForQs=" + darkForQs + + " mNavigationLight=" + mNavigationLight + + " last=" + last + + " timestamp=" + new Date(); + if (DEBUG) Log.d(TAG, mLastNavigationBarAppearanceChangedLog); + } else { + mNavigationLight = mHasLightNavigationBar + && (mDirectReplying && mNavbarColorManagedByIme || !mForceDarkForScrim) + && !mQsCustomizing; + mLastNavigationBarAppearanceChangedLog = "onNavigationBarAppearanceChanged()" + + " appearance=" + appearance + + " nbModeChanged=" + nbModeChanged + + " navigationBarMode=" + navigationBarMode + + " navbarColorManagedByIme=" + navbarColorManagedByIme + + " mHasLightNavigationBar=" + mHasLightNavigationBar + + " mNavigationLight=" + mNavigationLight + + " last=" + last + + " timestamp=" + new Date(); + if (DEBUG) Log.d(TAG, mLastNavigationBarAppearanceChangedLog); + } if (mNavigationLight != last) { updateNavigation(); } @@ -188,6 +246,20 @@ public class LightBarController implements BatteryController.BatteryStateChangeC reevaluate(); } + /** Set if Quick Settings is fully expanded, which affects notification scrim visibility */ + public void setQsExpanded(boolean expanded) { + if (mQsExpanded == expanded) return; + mQsExpanded = expanded; + reevaluate(); + } + + /** Set if Global Actions dialog is visible, which requires dark mode (light buttons) */ + public void setGlobalActionsVisible(boolean visible) { + if (mGlobalActionsVisible == visible) return; + mGlobalActionsVisible = visible; + reevaluate(); + } + /** * Controls the light status bar temporarily for back navigation. * @param appearance the custmoized appearance. @@ -225,16 +297,52 @@ public class LightBarController implements BatteryController.BatteryStateChangeC public void setScrimState(ScrimState scrimState, float scrimBehindAlpha, GradientColors scrimInFrontColor) { - boolean forceDarkForScrimLast = mForceDarkForScrim; - // For BOUNCER/BOUNCER_SCRIMMED cases, we assume that alpha is always below threshold. - // This enables IMEs to control the navigation bar color. - // For other cases, scrim should be able to veto the light navigation bar. - mForceDarkForScrim = scrimState != ScrimState.BOUNCER - && scrimState != ScrimState.BOUNCER_SCRIMMED - && scrimBehindAlpha >= NAV_BAR_INVERSION_SCRIM_ALPHA_THRESHOLD - && !scrimInFrontColor.supportsDarkText(); - if (mHasLightNavigationBar && (mForceDarkForScrim != forceDarkForScrimLast)) { - reevaluate(); + if (mUseNewLightBarLogic) { + boolean forceDarkForScrimLast = mForceDarkForScrim; + boolean forceLightForScrimLast = mForceLightForScrim; + final boolean forceForScrim = + scrimBehindAlpha >= NAV_BAR_INVERSION_SCRIM_ALPHA_THRESHOLD; + final boolean scrimColorIsLight = scrimInFrontColor.supportsDarkText(); + + mForceDarkForScrim = forceForScrim && !scrimColorIsLight; + mForceLightForScrim = forceForScrim && scrimColorIsLight; + if (mHasLightNavigationBar) { + if (mForceDarkForScrim != forceDarkForScrimLast) reevaluate(); + } else { + if (mForceLightForScrim != forceLightForScrimLast) reevaluate(); + } + mLastSetScrimStateLog = "setScrimState()" + + " scrimState=" + scrimState + + " scrimBehindAlpha=" + scrimBehindAlpha + + " scrimInFrontColor=" + scrimInFrontColor + + " forceForScrim=" + forceForScrim + + " scrimColorIsLight=" + scrimColorIsLight + + " mHasLightNavigationBar=" + mHasLightNavigationBar + + " mForceDarkForScrim=" + mForceDarkForScrim + + " mForceLightForScrim=" + mForceLightForScrim + + " timestamp=" + new Date(); + if (DEBUG) Log.d(TAG, mLastSetScrimStateLog); + } else { + boolean forceDarkForScrimLast = mForceDarkForScrim; + // For BOUNCER/BOUNCER_SCRIMMED cases, we assume that alpha is always below threshold. + // This enables IMEs to control the navigation bar color. + // For other cases, scrim should be able to veto the light navigation bar. + // NOTE: this was also wrong for S and has been removed in the new logic. + mForceDarkForScrim = scrimState != ScrimState.BOUNCER + && scrimState != ScrimState.BOUNCER_SCRIMMED + && scrimBehindAlpha >= NAV_BAR_INVERSION_SCRIM_ALPHA_THRESHOLD + && !scrimInFrontColor.supportsDarkText(); + if (mHasLightNavigationBar && (mForceDarkForScrim != forceDarkForScrimLast)) { + reevaluate(); + } + mLastSetScrimStateLog = "setScrimState()" + + " scrimState=" + scrimState + + " scrimBehindAlpha=" + scrimBehindAlpha + + " scrimInFrontColor=" + scrimInFrontColor + + " mHasLightNavigationBar=" + mHasLightNavigationBar + + " mForceDarkForScrim=" + mForceDarkForScrim + + " timestamp=" + new Date(); + if (DEBUG) Log.d(TAG, mLastSetScrimStateLog); } } @@ -309,16 +417,24 @@ public class LightBarController implements BatteryController.BatteryStateChangeC pw.print(mAppearanceRegions[i].toString()); pw.print(" isLight="); pw.println(isLight); } - pw.print(" mNavigationLight="); pw.print(mNavigationLight); + pw.print(" mNavigationLight="); pw.println(mNavigationLight); pw.print(" mHasLightNavigationBar="); pw.println(mHasLightNavigationBar); - + pw.println(); pw.print(" mStatusBarMode="); pw.print(mStatusBarMode); pw.print(" mNavigationBarMode="); pw.println(mNavigationBarMode); - - pw.print(" mForceDarkForScrim="); pw.print(mForceDarkForScrim); - pw.print(" mQsCustomizing="); pw.print(mQsCustomizing); + pw.println(); + pw.print(" mForceDarkForScrim="); pw.println(mForceDarkForScrim); + pw.print(" mForceLightForScrim="); pw.println(mForceLightForScrim); + pw.println(); + pw.print(" mQsCustomizing="); pw.println(mQsCustomizing); + pw.print(" mQsExpanded="); pw.println(mQsExpanded); + pw.print(" mGlobalActionsVisible="); pw.println(mGlobalActionsVisible); pw.print(" mDirectReplying="); pw.println(mDirectReplying); pw.print(" mNavbarColorManagedByIme="); pw.println(mNavbarColorManagedByIme); + pw.println(); + pw.println(" Recent Calculation Logs:"); + pw.print(" "); pw.println(mLastSetScrimStateLog); + pw.print(" "); pw.println(mLastNavigationBarAppearanceChangedLog); pw.println(); @@ -344,6 +460,7 @@ public class LightBarController implements BatteryController.BatteryStateChangeC private final DarkIconDispatcher mDarkIconDispatcher; private final BatteryController mBatteryController; private final NavigationModeController mNavModeController; + private final FeatureFlags mFeatureFlags; private final DumpManager mDumpManager; private final DisplayTracker mDisplayTracker; @@ -352,12 +469,14 @@ public class LightBarController implements BatteryController.BatteryStateChangeC DarkIconDispatcher darkIconDispatcher, BatteryController batteryController, NavigationModeController navModeController, + FeatureFlags featureFlags, DumpManager dumpManager, DisplayTracker displayTracker) { mDarkIconDispatcher = darkIconDispatcher; mBatteryController = batteryController; mNavModeController = navModeController; + mFeatureFlags = featureFlags; mDumpManager = dumpManager; mDisplayTracker = displayTracker; } @@ -365,7 +484,7 @@ public class LightBarController implements BatteryController.BatteryStateChangeC /** Create an {@link LightBarController} */ public LightBarController create(Context context) { return new LightBarController(context, mDarkIconDispatcher, mBatteryController, - mNavModeController, mDumpManager, mDisplayTracker); + mNavModeController, mFeatureFlags, mDumpManager, mDisplayTracker); } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java index 51c56a021825d..fdb772bc6c132 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java @@ -42,6 +42,7 @@ import androidx.annotation.Nullable; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.colorextraction.ColorExtractor.GradientColors; import com.android.internal.graphics.ColorUtils; +import com.android.internal.util.ContrastColorUtil; import com.android.internal.util.function.TriConsumer; import com.android.keyguard.BouncerPanelExpansionCalculator; import com.android.keyguard.KeyguardUpdateMonitor; @@ -249,6 +250,7 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump private final TriConsumer mScrimStateListener; private final LargeScreenShadeInterpolator mLargeScreenShadeInterpolator; private final FeatureFlags mFeatureFlags; + private final boolean mUseNewLightBarLogic; private Consumer mScrimVisibleListener; private boolean mBlankScreen; private boolean mScreenBlankingCallbackCalled; @@ -306,6 +308,7 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump mScrimStateListener = lightBarController::setScrimState; mLargeScreenShadeInterpolator = largeScreenShadeInterpolator; mFeatureFlags = featureFlags; + mUseNewLightBarLogic = featureFlags.isEnabled(Flags.NEW_LIGHT_BAR_LOGIC); mDefaultScrimAlpha = BUSY_SCRIM_ALPHA; mKeyguardStateController = keyguardStateController; @@ -1159,7 +1162,13 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump if (mClipsQsScrim && mQsBottomVisible) { alpha = mNotificationsAlpha; } - mScrimStateListener.accept(mState, alpha, mScrimInFront.getColors()); + if (mUseNewLightBarLogic) { + mScrimStateListener.accept(mState, alpha, mColors); + } else { + // NOTE: This wasn't wrong, but it implied that each scrim might have different colors, + // when in fact they all share the same GradientColors instance, which we own. + mScrimStateListener.accept(mState, alpha, mScrimInFront.getColors()); + } } private void dispatchScrimsVisible() { @@ -1487,8 +1496,15 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump int accent = Utils.getColorAccent(mScrimBehind.getContext()).getDefaultColor(); mColors.setMainColor(background); mColors.setSecondaryColor(accent); - mColors.setSupportsDarkText( - ColorUtils.calculateContrast(mColors.getMainColor(), Color.WHITE) > 4.5); + if (mUseNewLightBarLogic) { + final boolean isBackgroundLight = !ContrastColorUtil.isColorDark(background); + mColors.setSupportsDarkText(isBackgroundLight); + } else { + // NOTE: This was totally backward, but LightBarController was flipping it back. + // There may be other consumers of this which would struggle though + mColors.setSupportsDarkText( + ColorUtils.calculateContrast(mColors.getMainColor(), Color.WHITE) > 4.5); + } mNeedsDrawableColorUpdate = true; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java index 8795ac013bd3b..c9ee1e8ef5b96 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsDialogLiteTest.java @@ -70,6 +70,7 @@ import com.android.systemui.settings.UserTracker; import com.android.systemui.statusbar.NotificationShadeWindowController; import com.android.systemui.statusbar.VibratorHelper; import com.android.systemui.statusbar.phone.CentralSurfaces; +import com.android.systemui.statusbar.phone.LightBarController; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.systemui.telephony.TelephonyListenerManager; @@ -115,6 +116,7 @@ public class GlobalActionsDialogLiteTest extends SysuiTestCase { @Mock private MetricsLogger mMetricsLogger; @Mock private SysuiColorExtractor mColorExtractor; @Mock private IStatusBarService mStatusBarService; + @Mock private LightBarController mLightBarController; @Mock private NotificationShadeWindowController mNotificationShadeWindowController; @Mock private IWindowManager mWindowManager; @Mock private Executor mBackgroundExecutor; @@ -166,6 +168,7 @@ public class GlobalActionsDialogLiteTest extends SysuiTestCase { mMetricsLogger, mColorExtractor, mStatusBarService, + mLightBarController, mNotificationShadeWindowController, mWindowManager, mBackgroundExecutor, diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerBaseTest.java b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerBaseTest.java index f870631bd72bd..e0af81aacf5f8 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerBaseTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/NotificationPanelViewControllerBaseTest.java @@ -150,6 +150,7 @@ import com.android.systemui.statusbar.phone.KeyguardBottomAreaViewController; import com.android.systemui.statusbar.phone.KeyguardBypassController; import com.android.systemui.statusbar.phone.KeyguardStatusBarView; import com.android.systemui.statusbar.phone.KeyguardStatusBarViewController; +import com.android.systemui.statusbar.phone.LightBarController; import com.android.systemui.statusbar.phone.LockscreenGestureLogger; import com.android.systemui.statusbar.phone.ScreenOffAnimationController; import com.android.systemui.statusbar.phone.ScrimController; @@ -238,6 +239,7 @@ public class NotificationPanelViewControllerBaseTest extends SysuiTestCase { @Mock protected KeyguardStatusBarViewComponent mKeyguardStatusBarViewComponent; @Mock protected KeyguardClockSwitchController mKeyguardClockSwitchController; @Mock protected KeyguardStatusBarViewController mKeyguardStatusBarViewController; + @Mock protected LightBarController mLightBarController; @Mock protected NotificationStackScrollLayoutController mNotificationStackScrollLayoutController; @Mock protected NotificationShadeDepthController mNotificationShadeDepthController; @@ -651,6 +653,7 @@ public class NotificationPanelViewControllerBaseTest extends SysuiTestCase { mNotificationRemoteInputManager, mShadeExpansionStateManager, mStatusBarKeyguardViewManager, + mLightBarController, mNotificationStackScrollLayoutController, mLockscreenShadeTransitionController, mNotificationShadeDepthController, diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/QuickSettingsControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/shade/QuickSettingsControllerTest.java index 908f7cbf48015..f68c3a8a0a3f0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/QuickSettingsControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/QuickSettingsControllerTest.java @@ -82,6 +82,7 @@ import com.android.systemui.statusbar.notification.stack.NotificationStackScroll import com.android.systemui.statusbar.phone.KeyguardBottomAreaView; import com.android.systemui.statusbar.phone.KeyguardBypassController; import com.android.systemui.statusbar.phone.KeyguardStatusBarView; +import com.android.systemui.statusbar.phone.LightBarController; import com.android.systemui.statusbar.phone.LockscreenGestureLogger; import com.android.systemui.statusbar.phone.ScrimController; import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager; @@ -131,6 +132,7 @@ public class QuickSettingsControllerTest extends SysuiTestCase { @Mock private PulseExpansionHandler mPulseExpansionHandler; @Mock private NotificationRemoteInputManager mNotificationRemoteInputManager; @Mock private StatusBarKeyguardViewManager mStatusBarKeyguardViewManager; + @Mock private LightBarController mLightBarController; @Mock private NotificationStackScrollLayoutController mNotificationStackScrollLayoutController; @Mock private LockscreenShadeTransitionController mLockscreenShadeTransitionController; @Mock private NotificationShadeDepthController mNotificationShadeDepthController; @@ -220,6 +222,7 @@ public class QuickSettingsControllerTest extends SysuiTestCase { mNotificationRemoteInputManager, mShadeExpansionStateManager, mStatusBarKeyguardViewManager, + mLightBarController, mNotificationStackScrollLayoutController, mLockscreenShadeTransitionController, mNotificationShadeDepthController, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java index 529519a6246e5..a501556280271 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java @@ -22,21 +22,31 @@ import static com.android.systemui.statusbar.phone.BarTransitions.MODE_TRANSPARE import static junit.framework.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.graphics.Color; import android.graphics.Rect; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; +import androidx.annotation.ColorInt; import androidx.test.filters.SmallTest; +import com.android.internal.colorextraction.ColorExtractor.GradientColors; +import com.android.internal.util.ContrastColorUtil; import com.android.internal.view.AppearanceRegion; import com.android.systemui.SysuiTestCase; import com.android.systemui.dump.DumpManager; +import com.android.systemui.flags.FakeFeatureFlags; +import com.android.systemui.flags.Flags; import com.android.systemui.navigationbar.NavigationModeController; import com.android.systemui.settings.FakeDisplayTracker; import com.android.systemui.statusbar.policy.BatteryController; @@ -53,13 +63,25 @@ import java.util.ArrayList; @TestableLooper.RunWithLooper public class LightBarControllerTest extends SysuiTestCase { + private static final GradientColors COLORS_LIGHT = makeColors(Color.WHITE); + private static final GradientColors COLORS_DARK = makeColors(Color.BLACK); + private final FakeFeatureFlags mFeatureFlags = new FakeFeatureFlags(); private LightBarTransitionsController mLightBarTransitionsController; + private LightBarTransitionsController mNavBarController; private SysuiDarkIconDispatcher mStatusBarIconController; private LightBarController mLightBarController; + /** Allow testing with NEW_LIGHT_BAR_LOGIC flag in different states */ + protected boolean testNewLightBarLogic() { + return false; + } + @Before public void setup() { + mFeatureFlags.set(Flags.NEW_LIGHT_BAR_LOGIC, testNewLightBarLogic()); mStatusBarIconController = mock(SysuiDarkIconDispatcher.class); + mNavBarController = mock(LightBarTransitionsController.class); + when(mNavBarController.supportsIconTintForNavMode(anyInt())).thenReturn(true); mLightBarTransitionsController = mock(LightBarTransitionsController.class); when(mStatusBarIconController.getTransitionsController()).thenReturn( mLightBarTransitionsController); @@ -68,10 +90,19 @@ public class LightBarControllerTest extends SysuiTestCase { mStatusBarIconController, mock(BatteryController.class), mock(NavigationModeController.class), + mFeatureFlags, mock(DumpManager.class), new FakeDisplayTracker(mContext)); } + private static GradientColors makeColors(@ColorInt int bgColor) { + GradientColors colors = new GradientColors(); + colors.setMainColor(bgColor); + colors.setSecondaryColor(bgColor); + colors.setSupportsDarkText(!ContrastColorUtil.isColorDark(bgColor)); + return colors; + } + @Test public void testOnStatusBarAppearanceChanged_multipleStacks_allStacksLight() { final Rect firstBounds = new Rect(0, 0, 1, 1); @@ -177,4 +208,54 @@ public class LightBarControllerTest extends SysuiTestCase { false /* navbarColorManagedByIme */); verify(mLightBarTransitionsController).setIconsDark(eq(false), anyBoolean()); } + + @Test + public void validateNavBarChangesUpdateIcons() { + assumeTrue(testNewLightBarLogic()); // Only run in the new suite + + // On the launcher in dark mode buttons are light + mLightBarController.setScrimState(ScrimState.UNLOCKED, 0f, COLORS_DARK); + mLightBarController.onNavigationBarAppearanceChanged( + 0, /* nbModeChanged = */ true, + MODE_TRANSPARENT, /* navbarColorManagedByIme = */ false); + verifyNavBarIconsUnchanged(); // no changes yet; not attached + + // Initial state is set when controller is set + mLightBarController.setNavigationBar(mNavBarController); + verifyNavBarIconsDarkSetTo(false); + + // Changing the color of the transparent scrim has no effect + mLightBarController.setScrimState(ScrimState.UNLOCKED, 0f, COLORS_LIGHT); + verifyNavBarIconsUnchanged(); // still light + + // Showing the notification shade with white scrim requires dark icons + mLightBarController.setScrimState(ScrimState.UNLOCKED, 1f, COLORS_LIGHT); + verifyNavBarIconsDarkSetTo(true); + + // Expanded QS always provides a black background, so icons become light again + mLightBarController.setQsExpanded(true); + verifyNavBarIconsDarkSetTo(false); + + // Tapping the QS tile to change to dark theme has no effect in this state + mLightBarController.setScrimState(ScrimState.UNLOCKED, 1f, COLORS_DARK); + verifyNavBarIconsUnchanged(); // still light + + // collapsing QS in dark mode doesn't affect button color + mLightBarController.setQsExpanded(false); + verifyNavBarIconsUnchanged(); // still light + + // Closing the shade has no affect + mLightBarController.setScrimState(ScrimState.UNLOCKED, 0f, COLORS_DARK); + verifyNavBarIconsUnchanged(); // still light + } + + private void verifyNavBarIconsUnchanged() { + verify(mNavBarController, never()).setIconsDark(anyBoolean(), anyBoolean()); + } + + private void verifyNavBarIconsDarkSetTo(boolean iconsDark) { + verify(mNavBarController).setIconsDark(eq(iconsDark), anyBoolean()); + verify(mNavBarController, never()).setIconsDark(eq(!iconsDark), anyBoolean()); + clearInvocations(mNavBarController); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerWithNewLogicTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerWithNewLogicTest.kt new file mode 100644 index 0000000000000..d9c2cfa5ea7fd --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerWithNewLogicTest.kt @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2023 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.statusbar.phone + +import androidx.test.filters.SmallTest +import com.android.systemui.flags.Flags.NEW_LIGHT_BAR_LOGIC + +/** + * This file only needs to live as long as [NEW_LIGHT_BAR_LOGIC] does. When we delete that flag, we + * can roll this back into the old test. + */ +@SmallTest +class LightBarControllerWithNewLogicTest : LightBarControllerTest() { + override fun testNewLightBarLogic(): Boolean = true +}