From 83b55b6a673170d162dd315a6711ff42603c74ba Mon Sep 17 00:00:00 2001 From: Mike Schneider Date: Fri, 7 Jul 2023 15:09:56 +0200 Subject: [PATCH] Change behind-scrim's tint in BOUNCER state to match GM3 surface color. This is in lieu of relying on the KeyguardSecurityContainer to paint the background with surfaceColor. It fixes an issue during KeyguardSecurityContainer's vertical translation, triggered either via a gesture or during the bouncer dismiss animation, where the wrong color tone is surfaced and the alpha fades conflict. Bug: 281965452 Test: unit tests Test: manually tested bouncer on large screen, incl theme change http://shortn/_DVOtSJtrwy Merged-In: I67149c6faa2766be6d2537f2315dd2734bdd0447 Change-Id: I67149c6faa2766be6d2537f2315dd2734bdd0447 (cherry picked from commit d71e5c8fe2db8e8b0dab26c5979c576a88243316) --- .../keyguard/KeyguardSecurityContainer.java | 2 - .../statusbar/phone/ScrimController.java | 7 +++ .../systemui/statusbar/phone/ScrimState.java | 15 ++++- .../statusbar/phone/ScrimControllerTest.java | 59 ++++++++++++++++--- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java index 841b5b3a1e82a..9ff338e8d5ab0 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java @@ -786,8 +786,6 @@ public class KeyguardSecurityContainer extends ConstraintLayout { void reloadColors() { mViewMode.reloadColors(); - setBackgroundColor(Utils.getColorAttrDefaultColor(getContext(), - com.android.internal.R.attr.materialColorSurface)); } /** Handles density or font scale changes. */ 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 25ecf1a424e04..c42a6dc5b6f2c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java @@ -1509,6 +1509,13 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump mColors.setSupportsDarkText( ColorUtils.calculateContrast(mColors.getMainColor(), Color.WHITE) > 4.5); } + + int surface = Utils.getColorAttr(mScrimBehind.getContext(), + com.android.internal.R.attr.materialColorSurface).getDefaultColor(); + for (ScrimState state : ScrimState.values()) { + state.setSurfaceColor(surface); + } + mNeedsDrawableColorUpdate = true; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimState.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimState.java index 7b2028310a84e..e3b65ab27f485 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimState.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimState.java @@ -122,11 +122,19 @@ public enum ScrimState { @Override public void prepare(ScrimState previousState) { mBehindAlpha = mClipQsScrim ? 1 : mDefaultScrimAlpha; - mBehindTint = mClipQsScrim ? Color.BLACK : Color.TRANSPARENT; + mBehindTint = mClipQsScrim ? Color.BLACK : mSurfaceColor; mNotifAlpha = mClipQsScrim ? mDefaultScrimAlpha : 0; mNotifTint = Color.TRANSPARENT; mFrontAlpha = 0f; } + + @Override + public void setSurfaceColor(int surfaceColor) { + super.setSurfaceColor(surfaceColor); + if (!mClipQsScrim) { + mBehindTint = mSurfaceColor; + } + } }, /** @@ -295,6 +303,7 @@ public enum ScrimState { int mFrontTint = Color.TRANSPARENT; int mBehindTint = Color.TRANSPARENT; int mNotifTint = Color.TRANSPARENT; + int mSurfaceColor = Color.TRANSPARENT; boolean mAnimateChange = true; float mAodFrontScrimAlpha; @@ -409,6 +418,10 @@ public enum ScrimState { mDefaultScrimAlpha = defaultScrimAlpha; } + public void setSurfaceColor(int surfaceColor) { + mSurfaceColor = surfaceColor; + } + public void setWallpaperSupportsAmbientMode(boolean wallpaperSupportsAmbientMode) { mWallpaperSupportsAmbientMode = wallpaperSupportsAmbientMode; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ScrimControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ScrimControllerTest.java index a9ed175319265..7fc02280354f3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ScrimControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ScrimControllerTest.java @@ -44,6 +44,9 @@ import static kotlinx.coroutines.flow.FlowKt.emptyFlow; import android.animation.Animator; import android.app.AlarmManager; +import android.content.Context; +import android.content.res.ColorStateList; +import android.content.res.TypedArray; import android.graphics.Color; import android.os.Handler; import android.testing.AndroidTestingRunner; @@ -121,6 +124,7 @@ public class ScrimControllerTest extends SysuiTestCase { private int mScrimVisibility; private boolean mAlwaysOnEnabled; private TestableLooper mLooper; + private Context mContext; @Mock private AlarmManager mAlarmManager; @Mock private DozeParameters mDozeParameters; @Mock private LightBarController mLightBarController; @@ -134,6 +138,7 @@ public class ScrimControllerTest extends SysuiTestCase { @Mock private PrimaryBouncerToGoneTransitionViewModel mPrimaryBouncerToGoneTransitionViewModel; @Mock private KeyguardTransitionInteractor mKeyguardTransitionInteractor; @Mock private CoroutineDispatcher mMainDispatcher; + @Mock private TypedArray mMockTypedArray; // TODO(b/204991468): Use a real PanelExpansionStateManager object once this bug is fixed. (The // event-dispatch-on-registration pattern caused some of these unit tests to fail.) @@ -182,10 +187,11 @@ public class ScrimControllerTest extends SysuiTestCase { mNumEnds = 0; mNumCancels = 0; } - }; + } private AnimatorListener mAnimatorListener = new AnimatorListener(); + private int mSurfaceColor = 0x112233; private void finishAnimationsImmediately() { // Execute code that will trigger animations. @@ -214,10 +220,17 @@ public class ScrimControllerTest extends SysuiTestCase { @Before public void setup() { MockitoAnnotations.initMocks(this); + mContext = spy(getContext()); + when(mContext.obtainStyledAttributes( + new int[]{com.android.internal.R.attr.materialColorSurface})) + .thenReturn(mMockTypedArray); - mScrimBehind = spy(new ScrimView(getContext())); - mScrimInFront = new ScrimView(getContext()); - mNotificationsScrim = new ScrimView(getContext()); + when(mMockTypedArray.getColorStateList(anyInt())) + .thenAnswer((invocation) -> ColorStateList.valueOf(mSurfaceColor)); + + mScrimBehind = spy(new ScrimView(mContext)); + mScrimInFront = new ScrimView(mContext); + mNotificationsScrim = new ScrimView(mContext); mAlwaysOnEnabled = true; mLooper = TestableLooper.get(this); DejankUtils.setImmediate(true); @@ -577,7 +590,7 @@ public class ScrimControllerTest extends SysuiTestCase { mScrimController.transitionTo(BOUNCER); finishAnimationsImmediately(); // Front scrim should be transparent - // Back scrim should be visible without tint + // Back scrim should be visible and tinted to the surface color assertScrimAlpha(Map.of( mScrimInFront, TRANSPARENT, mNotificationsScrim, TRANSPARENT, @@ -585,9 +598,31 @@ public class ScrimControllerTest extends SysuiTestCase { assertScrimTinted(Map.of( mScrimInFront, false, - mScrimBehind, false, + mScrimBehind, true, mNotificationsScrim, false )); + + assertScrimTint(mScrimBehind, mSurfaceColor); + } + + @Test + public void onThemeChange_bouncerBehindTint_isUpdatedToSurfaceColor() { + assertEquals(BOUNCER.getBehindTint(), 0x112233); + mSurfaceColor = 0x223344; + mConfigurationController.notifyThemeChanged(); + assertEquals(BOUNCER.getBehindTint(), 0x223344); + } + + @Test + public void onThemeChangeWhileClipQsScrim_bouncerBehindTint_remainsBlack() { + mScrimController.setClipsQsScrim(true); + mScrimController.transitionTo(BOUNCER); + finishAnimationsImmediately(); + + assertEquals(BOUNCER.getBehindTint(), Color.BLACK); + mSurfaceColor = 0x223344; + mConfigurationController.notifyThemeChanged(); + assertEquals(BOUNCER.getBehindTint(), Color.BLACK); } @Test @@ -619,16 +654,17 @@ public class ScrimControllerTest extends SysuiTestCase { finishAnimationsImmediately(); // Front scrim should be transparent - // Back scrim should be visible without tint + // Back scrim should be visible and has a tint of surfaceColor assertScrimAlpha(Map.of( mScrimInFront, TRANSPARENT, mNotificationsScrim, TRANSPARENT, mScrimBehind, OPAQUE)); assertScrimTinted(Map.of( mScrimInFront, false, - mScrimBehind, false, + mScrimBehind, true, mNotificationsScrim, false )); + assertScrimTint(mScrimBehind, mSurfaceColor); } @Test @@ -1809,6 +1845,13 @@ public class ScrimControllerTest extends SysuiTestCase { assertEquals(message, hasTint, scrim.getTint() != Color.TRANSPARENT); } + private void assertScrimTint(ScrimView scrim, int expectedTint) { + String message = "Tint test failed with expected scrim tint: " + + Integer.toHexString(expectedTint) + " and actual tint: " + + Integer.toHexString(scrim.getTint()) + " for scrim: " + getScrimName(scrim); + assertEquals(message, expectedTint, scrim.getTint(), 0.1); + } + private String getScrimName(ScrimView scrim) { if (scrim == mScrimInFront) { return "front";