From 8f93973b7f81201ab0771f76c6c77e1022cfffbd Mon Sep 17 00:00:00 2001 From: George Mount Date: Fri, 24 Feb 2023 10:17:14 -0800 Subject: [PATCH] AnimatorSet sends pause/resume for seeked animators Fixes: 270709315 This CL makes it so animators that are part of an AnimatorSet that has been seeked will send onAnimationPause() and onAnimationResume() when the AnimatorSet is paused and resumed. Also made AnimatorSetActivityTest#testIsRunning() more robust by checking state on the UI thread. Test: new test and ran previous animator tests Change-Id: I20fed31c626f9bf1d665169a61a37ea1307d4790 --- core/java/android/animation/Animator.java | 26 +++- core/java/android/animation/AnimatorSet.java | 36 ++--- .../java/android/animation/ValueAnimator.java | 24 +-- .../animation/AnimatorSetActivityTest.java | 8 +- .../animation/AnimatorSetCallsTest.java | 144 ++++++++++++++++-- 5 files changed, 174 insertions(+), 64 deletions(-) diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index 322d23b55d5d1..de7b80f2d7615 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -78,6 +78,13 @@ public abstract class Animator implements Cloneable { */ private Object[] mCachedList; + /** + * Tracks whether we've notified listeners of the onAnimationStart() event. This can be + * complex to keep track of since we notify listeners at different times depending on + * startDelay and whether start() was called before end(). + */ + boolean mStartListenersCalled = false; + /** * Sets the duration for delaying pausing animators when apps go into the background. * Used by AnimationHandler when requested to pause animators. @@ -165,7 +172,9 @@ public abstract class Animator implements Cloneable { * @see AnimatorPauseListener */ public void pause() { - if (isStarted() && !mPaused) { + // We only want to pause started Animators or animators that setCurrentPlayTime() + // have been called on. mStartListenerCalled will be true if seek has happened. + if ((isStarted() || mStartListenersCalled) && !mPaused) { mPaused = true; notifyPauseListeners(AnimatorCaller.ON_PAUSE); } @@ -444,6 +453,7 @@ public abstract class Animator implements Cloneable { anim.mPauseListeners = new ArrayList(mPauseListeners); } anim.mCachedList = null; + anim.mStartListenersCalled = false; return anim; } catch (CloneNotSupportedException e) { throw new AssertionError(); @@ -608,6 +618,20 @@ public abstract class Animator implements Cloneable { callOnList(mPauseListeners, notification, this, false); } + void notifyStartListeners(boolean isReversing) { + if (mListeners != null && !mStartListenersCalled) { + notifyListeners(AnimatorCaller.ON_START, isReversing); + } + mStartListenersCalled = true; + } + + void notifyEndListeners(boolean isReversing) { + if (mListeners != null && mStartListenersCalled) { + notifyListeners(AnimatorCaller.ON_END, isReversing); + } + mStartListenersCalled = false; + } + /** * Calls call for every item in list with animator and * isReverse as parameters. diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java index 09eec9d25a4bd..ffdb9a21b39c8 100644 --- a/core/java/android/animation/AnimatorSet.java +++ b/core/java/android/animation/AnimatorSet.java @@ -189,11 +189,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim */ private long[] mChildStartAndStopTimes; - /** - * Tracks whether we've notified listeners of the onAnimationStart() event. - */ - private boolean mStartListenersCalled; - // This is to work around a bug in b/34736819. This needs to be removed once app team // fixes their side. private AnimatorListenerAdapter mAnimationEndListener = new AnimatorListenerAdapter() { @@ -424,7 +419,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim if (Looper.myLooper() == null) { throw new AndroidRuntimeException("Animators may only be run on Looper threads"); } - if (isStarted()) { + if (isStarted() || mStartListenersCalled) { notifyListeners(AnimatorCaller.ON_CANCEL, false); callOnPlayingSet(Animator::cancel); mPlayingSet.clear(); @@ -756,20 +751,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim } } - private void notifyStartListeners(boolean inReverse) { - if (mListeners != null && !mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_START, inReverse); - } - mStartListenersCalled = true; - } - - private void notifyEndListeners(boolean inReverse) { - if (mListeners != null && mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_END, inReverse); - } - mStartListenersCalled = false; - } - // Returns true if set is empty or contains nothing but animator sets with no start delay. private static boolean isEmptySet(AnimatorSet set) { if (set.getStartDelay() > 0) { @@ -936,12 +917,18 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim lastPlayTime - node.mStartTime, notify ); + if (notify) { + mPlayingSet.remove(node); + } } else if (start <= currentPlayTime && currentPlayTime <= end) { animator.animateSkipToEnds( currentPlayTime - node.mStartTime, lastPlayTime - node.mStartTime, notify ); + if (notify && !mPlayingSet.contains(node)) { + mPlayingSet.add(node); + } } } } @@ -969,12 +956,18 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim lastPlayTime - node.mStartTime, notify ); + if (notify) { + mPlayingSet.remove(node); + } } else if (start <= currentPlayTime && currentPlayTime <= end) { animator.animateSkipToEnds( currentPlayTime - node.mStartTime, lastPlayTime - node.mStartTime, notify ); + if (notify && !mPlayingSet.contains(node)) { + mPlayingSet.add(node); + } } } } @@ -1115,8 +1108,8 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim mSeekState.setPlayTime(0, mReversing); } } - animateBasedOnPlayTime(playTime, lastPlayTime, mReversing, true); mSeekState.setPlayTime(playTime, mReversing); + animateBasedOnPlayTime(playTime, lastPlayTime, mReversing, true); } /** @@ -1498,7 +1491,6 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim anim.mNodeMap = new ArrayMap(); anim.mNodes = new ArrayList(nodeCount); anim.mEvents = new ArrayList(); - anim.mStartListenersCalled = false; anim.mAnimationEndListener = new AnimatorListenerAdapter() { @Override public void onAnimationEnd(Animator animation) { diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index 719f596949bb2..d77e3222ad569 100644 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -198,13 +198,6 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio */ private boolean mStarted = false; - /** - * Tracks whether we've notified listeners of the onAnimationStart() event. This can be - * complex to keep track of since we notify listeners at different times depending on - * startDelay and whether start() was called before end(). - */ - private boolean mStartListenersCalled = false; - /** * Flag that denotes whether the animation is set up and ready to go. Used to * set up animation that has not yet been started. @@ -1108,20 +1101,6 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio } } - private void notifyStartListeners(boolean isReversing) { - if (mListeners != null && !mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_START, isReversing); - } - mStartListenersCalled = true; - } - - private void notifyEndListeners(boolean isReversing) { - if (mListeners != null && mStartListenersCalled) { - notifyListeners(AnimatorCaller.ON_END, isReversing); - } - mStartListenersCalled = false; - } - /** * Start the animation playing. This version of start() takes a boolean flag that indicates * whether the animation should play in reverse. The flag is usually false, but may be set @@ -1209,7 +1188,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio // Only cancel if the animation is actually running or has been started and is about // to run // Only notify listeners if the animator has actually started - if ((mStarted || mRunning) && mListeners != null) { + if ((mStarted || mRunning || mStartListenersCalled) && mListeners != null) { if (!mRunning) { // If it's not yet running, then start listeners weren't called. Call them now. notifyStartListeners(mReversing); @@ -1687,7 +1666,6 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio anim.mRunning = false; anim.mPaused = false; anim.mResumed = false; - anim.mStartListenersCalled = false; anim.mStartTime = -1; anim.mStartTimeCommitted = false; anim.mAnimationEndRequested = false; diff --git a/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java b/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java index 7a1de0c5d4fe4..a7538701807a3 100644 --- a/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java +++ b/core/tests/coretests/src/android/animation/AnimatorSetActivityTest.java @@ -435,9 +435,11 @@ public class AnimatorSetActivityTest { mActivityRule.runOnUiThread(s::start); while (!listener.endIsCalled) { - boolean passedStartDelay = a1.isStarted() || a2.isStarted() || a3.isStarted() || - a4.isStarted() || a5.isStarted(); - assertEquals(passedStartDelay, s.isRunning()); + mActivityRule.runOnUiThread(() -> { + boolean passedStartDelay = a1.isStarted() || a2.isStarted() || a3.isStarted() + || a4.isStarted() || a5.isStarted(); + assertEquals(passedStartDelay, s.isRunning()); + }); Thread.sleep(50); } assertFalse(s.isRunning()); diff --git a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java index 22da0aa4597e6..41a66771986fa 100644 --- a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java +++ b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java @@ -40,6 +40,7 @@ public class AnimatorSetCallsTest { private AnimatorSetActivity mActivity; private AnimatorSet mSet1; + private AnimatorSet mSet2; private ObjectAnimator mAnimator; private CountListener mListener1; private CountListener mListener2; @@ -56,10 +57,10 @@ public class AnimatorSetCallsTest { mSet1.addListener(mListener1); mSet1.addPauseListener(mListener1); - AnimatorSet set2 = new AnimatorSet(); + mSet2 = new AnimatorSet(); mListener2 = new CountListener(); - set2.addListener(mListener2); - set2.addPauseListener(mListener2); + mSet2.addListener(mListener2); + mSet2.addPauseListener(mListener2); mAnimator = ObjectAnimator.ofFloat(square, "translationX", 0f, 100f); mListener3 = new CountListener(); @@ -67,8 +68,8 @@ public class AnimatorSetCallsTest { mAnimator.addPauseListener(mListener3); mAnimator.setDuration(1); - set2.play(mAnimator); - mSet1.play(set2); + mSet2.play(mAnimator); + mSet1.play(mSet2); }); } @@ -175,6 +176,7 @@ public class AnimatorSetCallsTest { assertEquals(1, updateValues.size()); assertEquals(0f, updateValues.get(0), 0f); } + @Test public void updateOnlyWhileRunning() { ArrayList updateValues = new ArrayList<>(); @@ -207,6 +209,118 @@ public class AnimatorSetCallsTest { } } + @Test + public void pauseResumeSeekingAnimators() { + ValueAnimator animator2 = ValueAnimator.ofFloat(0f, 1f); + mSet2.play(animator2).after(mAnimator); + mSet2.setStartDelay(100); + mSet1.setStartDelay(100); + mAnimator.setDuration(100); + + mActivity.runOnUiThread(() -> { + mSet1.setCurrentPlayTime(0); + mSet1.pause(); + + // only startForward and pause should have been called once + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener2.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + mListener2.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 0, 0, 0, 0, 0, 0, 0, 0 + ); + + mSet1.setCurrentPlayTime(200); + + // resume and endForward should have been called once + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 0, 0 + ); + + mSet1.pause(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 2, 1 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 2, 2 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 1, 1 + ); + + // now go to animator2 + mSet1.setCurrentPlayTime(400); + mSet1.pause(); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 3, 3 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 2, 2 + ); + mListener3.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + + // now go back to mAnimator + mSet1.setCurrentPlayTime(250); + mSet1.pause(); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 4, 4 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 3, 3 + ); + mListener3.assertValues( + 1, 1, 1, 0, 0, 0, 2, 2 + ); + + // now go back to before mSet2 was being run + mSet1.setCurrentPlayTime(1); + mSet1.pause(); + mSet1.resume(); + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 5, 5 + ); + mListener2.assertValues( + 1, 0, 0, 1, 0, 0, 3, 3 + ); + mListener3.assertValues( + 1, 1, 1, 1, 0, 0, 2, 2 + ); + }); + } + private void waitForOnUiThread(PollingCheck.PollingCheckCondition condition) { final boolean[] value = new boolean[1]; PollingCheck.waitFor(() -> { @@ -238,16 +352,16 @@ public class AnimatorSetCallsTest { int pause, int resume ) { - assertEquals(0, startNoParam); - assertEquals(0, endNoParam); - assertEquals(startForward, this.startForward); - assertEquals(startReverse, this.startReverse); - assertEquals(endForward, this.endForward); - assertEquals(endReverse, this.endReverse); - assertEquals(cancel, this.cancel); - assertEquals(repeat, this.repeat); - assertEquals(pause, this.pause); - assertEquals(resume, this.resume); + assertEquals("onAnimationStart() without direction", 0, startNoParam); + assertEquals("onAnimationEnd() without direction", 0, endNoParam); + assertEquals("onAnimationStart(forward)", startForward, this.startForward); + assertEquals("onAnimationStart(reverse)", startReverse, this.startReverse); + assertEquals("onAnimationEnd(forward)", endForward, this.endForward); + assertEquals("onAnimationEnd(reverse)", endReverse, this.endReverse); + assertEquals("onAnimationCancel()", cancel, this.cancel); + assertEquals("onAnimationRepeat()", repeat, this.repeat); + assertEquals("onAnimationPause()", pause, this.pause); + assertEquals("onAnimationResume()", resume, this.resume); } @Override