From d71e5c8fe2db8e8b0dab26c5979c576a88243316 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 Change-Id: I67149c6faa2766be6d2537f2315dd2734bdd0447 --- .../keyguard/KeyguardSecurityContainer.java | 2 - .../statusbar/phone/ScrimController.java | 9 ++- .../systemui/statusbar/phone/ScrimState.java | 15 ++++- .../statusbar/phone/ScrimControllerTest.java | 61 ++++++++++++++++--- 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java index 6853f815a1ffd..42a4e7202c824 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java @@ -793,8 +793,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 c16e13cd4af3f..e4e912e1ceec3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java @@ -52,6 +52,7 @@ import com.android.systemui.DejankUtils; import com.android.systemui.Dumpable; import com.android.systemui.R; import com.android.systemui.animation.ShadeInterpolation; +import com.android.systemui.bouncer.shared.constants.KeyguardBouncerConstants; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dock.DockManager; @@ -59,7 +60,6 @@ import com.android.systemui.flags.FeatureFlags; import com.android.systemui.flags.Flags; import com.android.systemui.keyguard.KeyguardUnlockAnimationController; import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor; -import com.android.systemui.bouncer.shared.constants.KeyguardBouncerConstants; import com.android.systemui.keyguard.shared.model.ScrimAlpha; import com.android.systemui.keyguard.shared.model.TransitionState; import com.android.systemui.keyguard.shared.model.TransitionStep; @@ -1492,6 +1492,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 ab801588575dc..bbc75c9eeae85 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; @@ -59,11 +62,11 @@ import com.android.keyguard.KeyguardUpdateMonitor; import com.android.systemui.DejankUtils; import com.android.systemui.SysuiTestCase; import com.android.systemui.animation.ShadeInterpolation; +import com.android.systemui.bouncer.shared.constants.KeyguardBouncerConstants; import com.android.systemui.dock.DockManager; import com.android.systemui.flags.FeatureFlags; import com.android.systemui.keyguard.KeyguardUnlockAnimationController; import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor; -import com.android.systemui.bouncer.shared.constants.KeyguardBouncerConstants; import com.android.systemui.keyguard.shared.model.KeyguardState; import com.android.systemui.keyguard.shared.model.TransitionState; import com.android.systemui.keyguard.shared.model.TransitionStep; @@ -120,6 +123,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; @@ -133,6 +137,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.) @@ -181,10 +186,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. @@ -213,10 +219,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); @@ -576,7 +589,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, @@ -584,9 +597,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 @@ -618,16 +653,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 @@ -1764,6 +1800,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";