From abf87cf3316bf3535cb57723d816376bcbd70925 Mon Sep 17 00:00:00 2001 From: Peter Kalauskas Date: Fri, 7 Apr 2023 12:49:56 -0700 Subject: [PATCH] Use PropertyAnimator for CUJ_SCREEN_OFF_SHOW_AOD Previously ViewPropertyAnimator - e.g. View.animate() - and PropertyAnimator were used at the same time for animations on the same keyguard view. This made the state less predictable since one might assume calling View.animate().cancel() would cancel all the PropertyAnimators too (it does not). Since ViewPropertyAnimator was only used for animating alpha, simplify things by removing usage of View.animate() and replacing it with a new PropertyAnimator call for the View.ALPHA property. Also, - Mark KeyguardUserSwitcherController as deprecated - Throw IllegalArgumentException if KeyguardStatusView#animate() is called on a debuggable build Test: capture perfetto trace while turning screen off Test: turn screen off while on lock screen, observe that the fade and translation animations remain unchanged from home screen Bug: 262705829 Change-Id: I72d8f3986422e3c71a6f91b94e5692e929867737 --- .../android/keyguard/KeyguardStatusView.java | 12 +++ .../keyguard/KeyguardVisibilityHelper.java | 85 +++++++++++-------- .../notification/AnimatableProperty.java | 4 + .../stack/AnimationProperties.java | 23 ++++- .../UnlockedScreenOffAnimationController.kt | 51 ++++++----- .../KeyguardUserSwitcherController.java | 1 + ...nlockedScreenOffAnimationControllerTest.kt | 25 ------ 7 files changed, 110 insertions(+), 91 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardStatusView.java b/packages/SystemUI/src/com/android/keyguard/KeyguardStatusView.java index b8e196fb8787b..d8e1eb0f0860e 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardStatusView.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardStatusView.java @@ -19,10 +19,12 @@ package com.android.keyguard; import static java.util.Collections.emptySet; import android.content.Context; +import android.os.Build; import android.os.Trace; import android.util.AttributeSet; import android.view.View; import android.view.ViewGroup; +import android.view.ViewPropertyAnimator; import android.widget.GridLayout; import com.android.systemui.R; @@ -117,6 +119,16 @@ public class KeyguardStatusView extends GridLayout { } } + @Override + public ViewPropertyAnimator animate() { + if (Build.IS_DEBUGGABLE) { + throw new IllegalArgumentException( + "KeyguardStatusView does not support ViewPropertyAnimator. " + + "Use PropertyAnimator instead."); + } + return super.animate(); + } + @Override protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) { Trace.beginSection("KeyguardStatusView#onMeasure"); diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardVisibilityHelper.java b/packages/SystemUI/src/com/android/keyguard/KeyguardVisibilityHelper.java index a678edc0eb06e..651c9796140e8 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardVisibilityHelper.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardVisibilityHelper.java @@ -18,8 +18,8 @@ package com.android.keyguard; import static com.android.systemui.statusbar.StatusBarState.KEYGUARD; +import android.util.Property; import android.view.View; -import android.view.ViewPropertyAnimator; import com.android.systemui.animation.Interpolators; import com.android.systemui.plugins.log.LogBuffer; @@ -34,6 +34,8 @@ import com.android.systemui.statusbar.policy.KeyguardStateController; import com.google.errorprone.annotations.CompileTimeConstant; +import java.util.function.Consumer; + /** * Helper class for updating visibility of keyguard views based on keyguard and status bar state. * This logic is shared by both the keyguard status view and the keyguard user switcher. @@ -83,47 +85,49 @@ public class KeyguardVisibilityHelper { boolean keyguardFadingAway, boolean goingToFullShade, int oldStatusBarState) { - mView.animate().cancel(); + PropertyAnimator.cancelAnimation(mView, AnimatableProperty.ALPHA); boolean isOccluded = mKeyguardStateController.isOccluded(); mKeyguardViewVisibilityAnimating = false; if ((!keyguardFadingAway && oldStatusBarState == KEYGUARD && statusBarState != KEYGUARD) || goingToFullShade) { mKeyguardViewVisibilityAnimating = true; - mView.animate() - .alpha(0f) - .setStartDelay(0) - .setDuration(160) - .setInterpolator(Interpolators.ALPHA_OUT) - .withEndAction( - mAnimateKeyguardStatusViewGoneEndRunnable); + + AnimationProperties animProps = new AnimationProperties() + .setCustomInterpolator(View.ALPHA, Interpolators.ALPHA_OUT) + .setAnimationEndAction(mSetGoneEndAction); if (keyguardFadingAway) { - mView.animate() - .setStartDelay(mKeyguardStateController.getKeyguardFadingAwayDelay()) - .setDuration(mKeyguardStateController.getShortenedFadingAwayDuration()) - .start(); + animProps + .setDelay(mKeyguardStateController.getKeyguardFadingAwayDelay()) + .setDuration(mKeyguardStateController.getShortenedFadingAwayDuration()); log("goingToFullShade && keyguardFadingAway"); } else { + animProps.setDelay(0).setDuration(160); log("goingToFullShade && !keyguardFadingAway"); } + PropertyAnimator.setProperty( + mView, AnimatableProperty.ALPHA, 0f, animProps, true /* animate */); } else if (oldStatusBarState == StatusBarState.SHADE_LOCKED && statusBarState == KEYGUARD) { mView.setVisibility(View.VISIBLE); mKeyguardViewVisibilityAnimating = true; mView.setAlpha(0f); - mView.animate() - .alpha(1f) - .setStartDelay(0) - .setDuration(320) - .setInterpolator(Interpolators.ALPHA_IN) - .withEndAction(mAnimateKeyguardStatusViewVisibleEndRunnable); + PropertyAnimator.setProperty( + mView, AnimatableProperty.ALPHA, 1f, + new AnimationProperties() + .setDelay(0) + .setDuration(320) + .setCustomInterpolator(View.ALPHA, Interpolators.ALPHA_IN) + .setAnimationEndAction( + property -> mSetVisibleEndRunnable.run()), + true /* animate */); log("keyguardFadingAway transition w/ Y Aniamtion"); } else if (statusBarState == KEYGUARD) { if (keyguardFadingAway) { mKeyguardViewVisibilityAnimating = true; - ViewPropertyAnimator animator = mView.animate() - .alpha(0) - .setInterpolator(Interpolators.FAST_OUT_LINEAR_IN) - .withEndAction(mAnimateKeyguardStatusViewInvisibleEndRunnable); + AnimationProperties animProps = new AnimationProperties() + .setDelay(0) + .setCustomInterpolator(View.ALPHA, Interpolators.FAST_OUT_LINEAR_IN) + .setAnimationEndAction(mSetInvisibleEndAction); if (mAnimateYPos) { float target = mView.getY() - mView.getHeight() * 0.05f; int delay = 0; @@ -135,13 +139,16 @@ public class KeyguardVisibilityHelper { PropertyAnimator.setProperty(mView, AnimatableProperty.Y, target, mAnimationProperties, true /* animate */); - animator.setDuration(duration) - .setStartDelay(delay); + animProps.setDuration(duration) + .setDelay(delay); log("keyguardFadingAway transition w/ Y Aniamtion"); } else { log("keyguardFadingAway transition w/o Y Animation"); } - animator.start(); + PropertyAnimator.setProperty( + mView, AnimatableProperty.ALPHA, 0f, + animProps, + true /* animate */); } else if (mScreenOffAnimationController.shouldAnimateInKeyguard()) { log("ScreenOff transition"); mKeyguardViewVisibilityAnimating = true; @@ -149,7 +156,7 @@ public class KeyguardVisibilityHelper { // Ask the screen off animation controller to animate the keyguard visibility for us // since it may need to be cancelled due to keyguard lifecycle events. mScreenOffAnimationController.animateInKeyguard( - mView, mAnimateKeyguardStatusViewVisibleEndRunnable); + mView, mSetVisibleEndRunnable); } else { log("Direct set Visibility to VISIBLE"); mView.setVisibility(View.VISIBLE); @@ -163,19 +170,25 @@ public class KeyguardVisibilityHelper { mLastOccludedState = isOccluded; } - private final Runnable mAnimateKeyguardStatusViewInvisibleEndRunnable = () -> { - mKeyguardViewVisibilityAnimating = false; - mView.setVisibility(View.INVISIBLE); - log("Callback Set Visibility to INVISIBLE"); + private final Consumer mSetInvisibleEndAction = new Consumer<>() { + @Override + public void accept(Property property) { + mKeyguardViewVisibilityAnimating = false; + mView.setVisibility(View.INVISIBLE); + log("Callback Set Visibility to INVISIBLE"); + } }; - private final Runnable mAnimateKeyguardStatusViewGoneEndRunnable = () -> { - mKeyguardViewVisibilityAnimating = false; - mView.setVisibility(View.GONE); - log("CallbackSet Visibility to GONE"); + private final Consumer mSetGoneEndAction = new Consumer<>() { + @Override + public void accept(Property property) { + mKeyguardViewVisibilityAnimating = false; + mView.setVisibility(View.GONE); + log("CallbackSet Visibility to GONE"); + } }; - private final Runnable mAnimateKeyguardStatusViewVisibleEndRunnable = () -> { + private final Runnable mSetVisibleEndRunnable = () -> { mKeyguardViewVisibilityAnimating = false; mView.setVisibility(View.VISIBLE); log("Callback Set Visibility to VISIBLE"); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/AnimatableProperty.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/AnimatableProperty.java index ecd0c41ff82de..fcff4376811eb 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/AnimatableProperty.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/AnimatableProperty.java @@ -48,6 +48,10 @@ public abstract class AnimatableProperty { View.SCALE_Y, R.id.scale_y_animator_tag, R.id.scale_y_animator_start_value_tag, R.id.scale_y_animator_end_value_tag); + public static final AnimatableProperty ALPHA = AnimatableProperty.from( + View.ALPHA, R.id.alpha_animator_tag, R.id.alpha_animator_start_value_tag, + R.id.alpha_animator_end_value_tag); + /** * Similar to X, however this doesn't allow for any other modifications other than from this * property. When using X, it's possible that the view is laid out during the animation, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AnimationProperties.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AnimationProperties.java index 112d48c115c2b..00b9aa42ab267 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AnimationProperties.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/AnimationProperties.java @@ -32,6 +32,7 @@ public class AnimationProperties { public long duration; public long delay; private ArrayMap mInterpolatorMap; + private Consumer mAnimationCancelAction; private Consumer mAnimationEndAction; /** @@ -50,27 +51,43 @@ public class AnimationProperties { * @return a listener that will be added for a given property during its animation. */ public AnimatorListenerAdapter getAnimationFinishListener(Property property) { - if (mAnimationEndAction == null) { + if (mAnimationEndAction == null && mAnimationCancelAction == null) { return null; } + Consumer cancelAction = mAnimationCancelAction; Consumer endAction = mAnimationEndAction; return new AnimatorListenerAdapter() { private boolean mCancelled; @Override public void onAnimationCancel(Animator animation) { - mCancelled = true; + mCancelled = true; + if (cancelAction != null) { + cancelAction.accept(property); + } } @Override public void onAnimationEnd(Animator animation) { - if (!mCancelled) { + if (!mCancelled && endAction != null) { endAction.accept(property); } } }; } + /** + * Add a callback for animation cancellation. + */ + public AnimationProperties setAnimationCancelAction(Consumer listener) { + mAnimationCancelAction = listener; + return this; + } + + /** + * Add a callback for animation ending successfully. The callback will not be called when the + * animations is cancelled. + */ public AnimationProperties setAnimationEndAction(Consumer listener) { mAnimationEndAction = listener; return this; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt index 118bfc55dd4c7..0cd3401ae541c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationController.kt @@ -175,15 +175,19 @@ class UnlockedScreenOffAnimationController @Inject constructor( // We animate the Y properly separately using the PropertyAnimator, as the panel // view also needs to update the end position. PropertyAnimator.cancelAnimation(keyguardView, AnimatableProperty.Y) - PropertyAnimator.setProperty(keyguardView, AnimatableProperty.Y, currentY, + PropertyAnimator.setProperty(keyguardView, AnimatableProperty.Y, currentY, AnimationProperties().setDuration(duration.toLong()), true /* animate */) - keyguardView.animate() + // Cancel any existing CUJs before starting the animation + interactionJankMonitor.cancel(CUJ_SCREEN_OFF_SHOW_AOD) + + PropertyAnimator.setProperty( + keyguardView, AnimatableProperty.ALPHA, 1f, + AnimationProperties() + .setDelay(0) .setDuration(duration.toLong()) - .setInterpolator(Interpolators.FAST_OUT_SLOW_IN) - .alpha(1f) - .withEndAction { + .setAnimationEndAction { aodUiAnimationPlaying = false // Lock the keyguard if it was waiting for the screen off animation to end. @@ -199,30 +203,23 @@ class UnlockedScreenOffAnimationController @Inject constructor( // Done going to sleep, reset this flag. decidedToAnimateGoingToSleep = null - // We need to unset the listener. These are persistent for future animators - keyguardView.animate().setListener(null) interactionJankMonitor.end(CUJ_SCREEN_OFF_SHOW_AOD) } - .setListener(object : AnimatorListenerAdapter() { - override fun onAnimationCancel(animation: Animator?) { - // If we're cancelled, reset state flags/listeners. The end action above - // will not be called, which is what we want since that will finish the - // screen off animation and show the lockscreen, which we don't want if we - // were cancelled. - aodUiAnimationPlaying = false - decidedToAnimateGoingToSleep = null - keyguardView.animate().setListener(null) - - interactionJankMonitor.cancel(CUJ_SCREEN_OFF_SHOW_AOD) - } - - override fun onAnimationStart(animation: Animator?) { - interactionJankMonitor.begin( - mCentralSurfaces.notificationShadeWindowView, - CUJ_SCREEN_OFF_SHOW_AOD) - } - }) - .start() + .setAnimationCancelAction { + // If we're cancelled, reset state flags/listeners. The end action above + // will not be called, which is what we want since that will finish the + // screen off animation and show the lockscreen, which we don't want if we + // were cancelled. + aodUiAnimationPlaying = false + decidedToAnimateGoingToSleep = null + interactionJankMonitor.cancel(CUJ_SCREEN_OFF_SHOW_AOD) + } + .setCustomInterpolator(View.ALPHA, Interpolators.FAST_OUT_SLOW_IN), + true /* animate */) + interactionJankMonitor.begin( + mCentralSurfaces.notificationShadeWindowView, + CUJ_SCREEN_OFF_SHOW_AOD + ) } override fun onStartedWakingUp() { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardUserSwitcherController.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardUserSwitcherController.java index e9f0dcb4eb51a..928e0115287dd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardUserSwitcherController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardUserSwitcherController.java @@ -61,6 +61,7 @@ import javax.inject.Inject; * Manages the user switcher on the Keyguard. */ @KeyguardUserSwitcherScope +@Deprecated public class KeyguardUserSwitcherController extends ViewController { private static final String TAG = "KeyguardUserSwitcherController"; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt index 746c92e485b7a..02c459b11d072 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/UnlockedScreenOffAnimationControllerTest.kt @@ -16,12 +16,10 @@ package com.android.systemui.statusbar.phone -import android.animation.Animator import android.os.Handler import android.os.PowerManager import android.testing.AndroidTestingRunner import android.testing.TestableLooper.RunWithLooper -import android.view.View import androidx.test.filters.SmallTest import com.android.internal.jank.InteractionJankMonitor import com.android.systemui.SysuiTestCase @@ -39,10 +37,8 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.Mock -import org.mockito.Mockito import org.mockito.Mockito.anyLong import org.mockito.Mockito.never -import org.mockito.Mockito.spy import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.Mockito.`when` @@ -111,27 +107,6 @@ class UnlockedScreenOffAnimationControllerTest : SysuiTestCase() { controller.onStartedWakingUp() } - @Test - fun testAnimClearsEndListener() { - val keyguardView = View(context) - val animator = spy(keyguardView.animate()) - val keyguardSpy = spy(keyguardView) - Mockito.`when`(keyguardSpy.animate()).thenReturn(animator) - val listener = ArgumentCaptor.forClass(Animator.AnimatorListener::class.java) - val endAction = ArgumentCaptor.forClass(Runnable::class.java) - controller.animateInKeyguard(keyguardSpy, Runnable {}) - Mockito.verify(animator).setListener(listener.capture()) - Mockito.verify(animator).withEndAction(endAction.capture()) - - // Verify that the listener is cleared if we cancel it. - listener.value.onAnimationCancel(null) - Mockito.verify(animator).setListener(null) - - // Verify that the listener is also cleared if the end action is triggered. - endAction.value.run() - verify(animator, times(2)).setListener(null) - } - /** * The AOD UI is shown during the screen off animation, after a delay to allow the light reveal * animation to start. If the device is woken up during the screen off, we should *never* do