From 716dcea6bdfaf33aecf02e822e980db1a56bb36e Mon Sep 17 00:00:00 2001 From: Josh Tsuji Date: Wed, 19 Oct 2022 15:41:08 -0400 Subject: [PATCH 1/2] Delay keyguard hiding until after occlusion animation ends. The keyguard should remain visible during the occlusion animation since the occluding activity is launching over it. The hideKeyguard call is also extremely janky and causes the animation to be nearly imperceptible through the jank. See https://b.corp.google.com/issues/250542279#comment15 for more context. Fixes: 250542279 Test: atest KeyguardViewMediatorTest Test: atest CentralSurfacesImplTest Test: launch camera, notice far less flickering Change-Id: Ic3f7e62a42829481f6809d32ee1aa4690927ece8 --- .../keyguard/KeyguardViewMediator.java | 28 ++++++++- .../statusbar/phone/CentralSurfacesImpl.java | 5 +- .../keyguard/KeyguardViewMediatorTest.java | 25 ++++++++ .../phone/CentralSurfacesImplTest.java | 60 +++++++++++++++++-- 4 files changed, 111 insertions(+), 7 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index a3632705d865d..8db18c9dd12e2 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -91,6 +91,7 @@ import android.view.animation.AnimationUtils; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import com.android.internal.jank.InteractionJankMonitor; import com.android.internal.jank.InteractionJankMonitor.Configuration; @@ -322,6 +323,12 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, // true if the keyguard is hidden by another window private boolean mOccluded = false; + /** + * Whether the {@link #mOccludeAnimationController} is currently playing the occlusion + * animation. + */ + private boolean mOccludeAnimationPlaying = false; + private boolean mWakeAndUnlocking = false; /** @@ -831,15 +838,22 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, /** * Animation launch controller for activities that occlude the keyguard. */ - private final ActivityLaunchAnimator.Controller mOccludeAnimationController = + @VisibleForTesting + final ActivityLaunchAnimator.Controller mOccludeAnimationController = new ActivityLaunchAnimator.Controller() { @Override - public void onLaunchAnimationStart(boolean isExpandingFullyAbove) {} + public void onLaunchAnimationStart(boolean isExpandingFullyAbove) { + mOccludeAnimationPlaying = true; + } @Override public void onLaunchAnimationCancelled(@Nullable Boolean newKeyguardOccludedState) { Log.d(TAG, "Occlude launch animation cancelled. Occluded state is now: " + mOccluded); + mOccludeAnimationPlaying = false; + + // Ensure keyguard state is set correctly if we're cancelled. + mCentralSurfaces.updateIsKeyguard(); } @Override @@ -848,6 +862,12 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, mCentralSurfaces.instantCollapseNotificationPanel(); } + mOccludeAnimationPlaying = false; + + // Hide the keyguard now that we're done launching the occluding activity over + // it. + mCentralSurfaces.updateIsKeyguard(); + mInteractionJankMonitor.end(CUJ_LOCKSCREEN_OCCLUSION); } @@ -1760,6 +1780,10 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, return mShowing && !mOccluded; } + public boolean isOccludeAnimationPlaying() { + return mOccludeAnimationPlaying; + } + /** * Notify us when the keyguard is occluded by another window */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java index 604e1467f9507..d881c0a75194a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -2987,7 +2987,10 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { // * When phone is unlocked: we still don't want to execute hiding of the keyguard // as the animation could prepare 'fake AOD' interface (without actually // transitioning to keyguard state) and this might reset the view states - if (!mScreenOffAnimationController.isKeyguardHideDelayed()) { + if (!mScreenOffAnimationController.isKeyguardHideDelayed() + // If we're animating occluded, there's an activity launching over the keyguard + // UI. Wait to hide it until after the animation concludes. + && !mKeyguardViewMediator.isOccludeAnimationPlaying()) { return hideKeyguardImpl(forceStateChange); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java index 4c986bffd1721..2c3ddd574b0fd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java @@ -60,6 +60,7 @@ import com.android.systemui.navigationbar.NavigationModeController; import com.android.systemui.statusbar.NotificationShadeDepthController; import com.android.systemui.statusbar.NotificationShadeWindowController; import com.android.systemui.statusbar.SysuiStatusBarStateController; +import com.android.systemui.statusbar.phone.CentralSurfaces; import com.android.systemui.statusbar.phone.DozeParameters; import com.android.systemui.statusbar.phone.ScreenOffAnimationController; import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager; @@ -112,6 +113,8 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { private FalsingCollectorFake mFalsingCollector; + private @Mock CentralSurfaces mCentralSurfaces; + @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -258,6 +261,26 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { verify(mKeyguardStateController).notifyKeyguardGoingAway(false); } + @Test + public void testUpdateIsKeyguardAfterOccludeAnimationEnds() { + mViewMediator.mOccludeAnimationController.onLaunchAnimationEnd( + false /* isExpandingFullyAbove */); + + // Since the updateIsKeyguard call is delayed during the animation, ensure it's called once + // it ends. + verify(mCentralSurfaces).updateIsKeyguard(); + } + + @Test + public void testUpdateIsKeyguardAfterOccludeAnimationIsCancelled() { + mViewMediator.mOccludeAnimationController.onLaunchAnimationCancelled( + null /* newKeyguardOccludedState */); + + // Since the updateIsKeyguard call is delayed during the animation, ensure it's called if + // it's cancelled. + verify(mCentralSurfaces).updateIsKeyguard(); + } + private void createAndStartViewMediator() { mViewMediator = new KeyguardViewMediator( mContext, @@ -287,5 +310,7 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { mNotificationShadeWindowControllerLazy, () -> mActivityLaunchAnimator); mViewMediator.start(); + + mViewMediator.registerCentralSurfaces(mCentralSurfaces, null, null, null, null, null); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java index ad497a2ec1e12..920b342a9b10b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java @@ -233,6 +233,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { @Mock private NavigationBarController mNavigationBarController; @Mock private AccessibilityFloatingMenuController mAccessibilityFloatingMenuController; @Mock private SysuiColorExtractor mColorExtractor; + private WakefulnessLifecycle mWakefulnessLifecycle; @Mock private ColorExtractor.GradientColors mGradientColors; @Mock private PulseExpansionHandler mPulseExpansionHandler; @Mock private NotificationWakeUpCoordinator mNotificationWakeUpCoordinator; @@ -363,10 +364,10 @@ public class CentralSurfacesImplTest extends SysuiTestCase { return null; }).when(mStatusBarKeyguardViewManager).addAfterKeyguardGoneRunnable(any()); - WakefulnessLifecycle wakefulnessLifecycle = + mWakefulnessLifecycle = new WakefulnessLifecycle(mContext, mIWallpaperManager, mDumpManager); - wakefulnessLifecycle.dispatchStartedWakingUp(PowerManager.WAKE_REASON_UNKNOWN); - wakefulnessLifecycle.dispatchFinishedWakingUp(); + mWakefulnessLifecycle.dispatchStartedWakingUp(PowerManager.WAKE_REASON_UNKNOWN); + mWakefulnessLifecycle.dispatchFinishedWakingUp(); when(mGradientColors.supportsDarkText()).thenReturn(true); when(mColorExtractor.getNeutralColors()).thenReturn(mGradientColors); @@ -425,7 +426,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { mBatteryController, mColorExtractor, new ScreenLifecycle(mDumpManager), - wakefulnessLifecycle, + mWakefulnessLifecycle, mStatusBarStateController, Optional.of(mBubbles), mDeviceProvisionedController, @@ -504,6 +505,8 @@ public class CentralSurfacesImplTest extends SysuiTestCase { mCentralSurfaces.mKeyguardIndicationController = mKeyguardIndicationController; mCentralSurfaces.mBarService = mBarService; mCentralSurfaces.mStackScroller = mStackScroller; + mCentralSurfaces.mGestureWakeLock = mPowerManager.newWakeLock( + PowerManager.SCREEN_BRIGHT_WAKE_LOCK, "sysui:GestureWakeLock"); mCentralSurfaces.startKeyguard(); mInitController.executePostInitTasks(); notificationLogger.setUpWithContainer(mNotificationListContainer); @@ -1068,6 +1071,55 @@ public class CentralSurfacesImplTest extends SysuiTestCase { assertThat(onDismissActionCaptor.getValue().onDismiss()).isFalse(); } + @Test + public void testKeyguardHideDelayedIfOcclusionAnimationRunning() { + // Show the keyguard and verify we've done so. + setKeyguardShowingAndOccluded(true /* showing */, false /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.KEYGUARD); + + // Request to hide the keyguard, but while the occlude animation is playing. We should delay + // this hide call, since we're playing the occlude animation over the keyguard and thus want + // it to remain visible. + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(true); + setKeyguardShowingAndOccluded(false /* showing */, true /* occluded */); + verify(mStatusBarStateController, never()).setState(StatusBarState.SHADE); + + // Once the animation ends, verify that the keyguard is actually hidden. + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(false); + setKeyguardShowingAndOccluded(false /* showing */, true /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.SHADE); + } + + @Test + public void testKeyguardHideNotDelayedIfOcclusionAnimationNotRunning() { + // Show the keyguard and verify we've done so. + setKeyguardShowingAndOccluded(true /* showing */, false /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.KEYGUARD); + + // Hide the keyguard while the occlusion animation is not running. Verify that we + // immediately hide the keyguard. + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(false); + setKeyguardShowingAndOccluded(false /* showing */, true /* occluded */); + verify(mStatusBarStateController).setState(StatusBarState.SHADE); + } + + /** + * Configures the appropriate mocks and then calls {@link CentralSurfacesImpl#updateIsKeyguard} + * to reconfigure the keyguard to reflect the requested showing/occluded states. + */ + private void setKeyguardShowingAndOccluded(boolean showing, boolean occluded) { + when(mStatusBarStateController.isKeyguardRequested()).thenReturn(showing); + when(mKeyguardStateController.isOccluded()).thenReturn(occluded); + + // If we want to show the keyguard, make sure that we think we're awake and not unlocking. + if (showing) { + when(mBiometricUnlockController.isWakeAndUnlock()).thenReturn(false); + mWakefulnessLifecycle.dispatchStartedWakingUp(PowerManager.WAKE_REASON_UNKNOWN); + } + + mCentralSurfaces.updateIsKeyguard(false /* forceStateChange */); + } + private void setDeviceState(int state) { ArgumentCaptor callbackCaptor = ArgumentCaptor.forClass(DeviceStateManager.DeviceStateCallback.class); From 55816370bd3084aeb379023d9a66352bd65f7b48 Mon Sep 17 00:00:00 2001 From: Josh Tsuji Date: Wed, 19 Oct 2022 16:04:58 -0400 Subject: [PATCH 2/2] Fix scrim turning opaque during occlusion animation. Bug: 250542279 Test: atest ScrimController Test: launch camera and observe scrim does not go opaque Change-Id: Ic62b7358be0c823eb541d2f87e2cb18dc4a9b5ec --- .../statusbar/phone/ScrimController.java | 14 +++++++- .../statusbar/phone/ScrimControllerTest.java | 35 +++++++++++++++++-- 2 files changed, 46 insertions(+), 3 deletions(-) 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 84907686835b0..cf3a48cf50001 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ScrimController.java @@ -53,6 +53,7 @@ import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dock.DockManager; import com.android.systemui.keyguard.KeyguardUnlockAnimationController; +import com.android.systemui.keyguard.KeyguardViewMediator; import com.android.systemui.scrim.ScrimView; import com.android.systemui.shade.NotificationPanelViewController; import com.android.systemui.statusbar.notification.stack.ViewState; @@ -204,6 +205,7 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump private final ScreenOffAnimationController mScreenOffAnimationController; private final KeyguardUnlockAnimationController mKeyguardUnlockAnimationController; private final StatusBarKeyguardViewManager mStatusBarKeyguardViewManager; + private KeyguardViewMediator mKeyguardViewMediator; private GradientColors mColors; private boolean mNeedsDrawableColorUpdate; @@ -273,7 +275,8 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump @Main Executor mainExecutor, ScreenOffAnimationController screenOffAnimationController, KeyguardUnlockAnimationController keyguardUnlockAnimationController, - StatusBarKeyguardViewManager statusBarKeyguardViewManager) { + StatusBarKeyguardViewManager statusBarKeyguardViewManager, + KeyguardViewMediator keyguardViewMediator) { mScrimStateListener = lightBarController::setScrimState; mDefaultScrimAlpha = BUSY_SCRIM_ALPHA; @@ -312,6 +315,8 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump } }); mColors = new GradientColors(); + + mKeyguardViewMediator = keyguardViewMediator; } /** @@ -807,6 +812,13 @@ public class ScrimController implements ViewTreeObserver.OnPreDrawListener, Dump mBehindTint, interpolatedFraction); } + + // If we're unlocked but still playing the occlude animation, remain at the keyguard + // alpha temporarily. + if (mKeyguardViewMediator.isOccludeAnimationPlaying() + || mState.mLaunchingAffordanceWithPreview) { + mNotificationsAlpha = KEYGUARD_SCRIM_ALPHA; + } } else if (mState == ScrimState.AUTH_SCRIMMED_SHADE) { float behindFraction = getInterpolatedFraction(); behindFraction = (float) Math.pow(behindFraction, 0.8f); 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 4d1a52c494abc..a5deaa45bf0f9 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 @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.phone; +import static com.android.systemui.statusbar.phone.ScrimController.KEYGUARD_SCRIM_ALPHA; import static com.android.systemui.statusbar.phone.ScrimController.OPAQUE; import static com.android.systemui.statusbar.phone.ScrimController.SEMI_TRANSPARENT; import static com.android.systemui.statusbar.phone.ScrimController.TRANSPARENT; @@ -58,6 +59,7 @@ import com.android.systemui.SysuiTestCase; import com.android.systemui.animation.ShadeInterpolation; import com.android.systemui.dock.DockManager; import com.android.systemui.keyguard.KeyguardUnlockAnimationController; +import com.android.systemui.keyguard.KeyguardViewMediator; import com.android.systemui.scrim.ScrimView; import com.android.systemui.statusbar.policy.FakeConfigurationController; import com.android.systemui.statusbar.policy.KeyguardStateController; @@ -117,6 +119,7 @@ public class ScrimControllerTest extends SysuiTestCase { // 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.) @Mock private StatusBarKeyguardViewManager mStatusBarKeyguardViewManager; + @Mock private KeyguardViewMediator mKeyguardViewMediator; private static class AnimatorListener implements Animator.AnimatorListener { private int mNumStarts; @@ -230,7 +233,8 @@ public class ScrimControllerTest extends SysuiTestCase { mDockManager, mConfigurationController, new FakeExecutor(new FakeSystemClock()), mScreenOffAnimationController, mKeyguardUnlockAnimationController, - mStatusBarKeyguardViewManager); + mStatusBarKeyguardViewManager, + mKeyguardViewMediator); mScrimController.setScrimVisibleListener(visible -> mScrimVisibility = visible); mScrimController.attachViews(mScrimBehind, mNotificationsScrim, mScrimInFront); mScrimController.setAnimatorListener(mAnimatorListener); @@ -239,6 +243,8 @@ public class ScrimControllerTest extends SysuiTestCase { mScrimController.setWallpaperSupportsAmbientMode(false); mScrimController.transitionTo(ScrimState.KEYGUARD); finishAnimationsImmediately(); + + mScrimController.setLaunchingAffordanceWithPreview(false); } @After @@ -852,7 +858,8 @@ public class ScrimControllerTest extends SysuiTestCase { mDockManager, mConfigurationController, new FakeExecutor(new FakeSystemClock()), mScreenOffAnimationController, mKeyguardUnlockAnimationController, - mStatusBarKeyguardViewManager); + mStatusBarKeyguardViewManager, + mKeyguardViewMediator); mScrimController.setScrimVisibleListener(visible -> mScrimVisibility = visible); mScrimController.attachViews(mScrimBehind, mNotificationsScrim, mScrimInFront); mScrimController.setAnimatorListener(mAnimatorListener); @@ -1592,6 +1599,30 @@ public class ScrimControllerTest extends SysuiTestCase { assertScrimAlpha(mScrimBehind, 0); } + @Test + public void keyguardAlpha_whenUnlockedForOcclusion_ifPlayingOcclusionAnimation() { + mScrimController.transitionTo(ScrimState.KEYGUARD); + + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(true); + + mScrimController.transitionTo(ScrimState.UNLOCKED); + finishAnimationsImmediately(); + + assertScrimAlpha(mNotificationsScrim, (int) (KEYGUARD_SCRIM_ALPHA * 255f)); + } + + @Test + public void keyguardAlpha_whenUnlockedForLaunch_ifLaunchingAffordance() { + mScrimController.transitionTo(ScrimState.KEYGUARD); + when(mKeyguardViewMediator.isOccludeAnimationPlaying()).thenReturn(true); + mScrimController.setLaunchingAffordanceWithPreview(true); + + mScrimController.transitionTo(ScrimState.UNLOCKED); + finishAnimationsImmediately(); + + assertScrimAlpha(mNotificationsScrim, (int) (KEYGUARD_SCRIM_ALPHA * 255f)); + } + private void assertAlphaAfterExpansion(ScrimView scrim, float expectedAlpha, float expansion) { mScrimController.setRawPanelExpansionFraction(expansion); finishAnimationsImmediately();