From fd41f0f638bcca0a47de57562fe48c5f4601fec8 Mon Sep 17 00:00:00 2001 From: George Mount Date: Wed, 15 Feb 2023 15:38:48 -0800 Subject: [PATCH] Call pause() and resume() on child animators. Fixes: 269529338 When AnimatorSet was paused or resumed, the child animators were not being paused or resumed. This CL makes fixeds that. This CL also updates the iteration over children so that temporary allocations are reduced. A cached array of items are saved and reused for the next iteration when possible. Test: new tests (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:218395a67afc6a16a53b5bef8d7c6fa764bed9b4) Merged-In: I40fa2115099116cfe9320b0587948f08a8650f91 Change-Id: I40fa2115099116cfe9320b0587948f08a8650f91 --- core/java/android/animation/Animator.java | 115 ++++++-- core/java/android/animation/AnimatorSet.java | 49 ++-- .../java/android/animation/ValueAnimator.java | 45 +--- .../animation/AnimatorSetCallsTest.java | 251 ++++++++++++++++++ 4 files changed, 378 insertions(+), 82 deletions(-) create mode 100644 core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java diff --git a/core/java/android/animation/Animator.java b/core/java/android/animation/Animator.java index d0ce701334146..21ba403d332ac 100644 --- a/core/java/android/animation/Animator.java +++ b/core/java/android/animation/Animator.java @@ -71,6 +71,13 @@ public abstract class Animator implements Cloneable { */ private static long sBackgroundPauseDelay = 1000; + /** + * A cache of the values in a list. Used so that when calling the list, we have a copy + * of it in case the list is modified while iterating. The array can be reused to avoid + * allocation on every notification. + */ + private Object[] mCachedList; + /** * Sets the duration for delaying pausing animators when apps go into the background. * Used by AnimationHandler when requested to pause animators. @@ -160,14 +167,7 @@ public abstract class Animator implements Cloneable { public void pause() { if (isStarted() && !mPaused) { mPaused = true; - if (mPauseListeners != null) { - ArrayList tmpListeners = - (ArrayList) mPauseListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationPause(this); - } - } + notifyPauseListeners(AnimatorCaller.ON_PAUSE); } } @@ -184,14 +184,7 @@ public abstract class Animator implements Cloneable { public void resume() { if (mPaused) { mPaused = false; - if (mPauseListeners != null) { - ArrayList tmpListeners = - (ArrayList) mPauseListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationResume(this); - } - } + notifyPauseListeners(AnimatorCaller.ON_RESUME); } } @@ -450,6 +443,7 @@ public abstract class Animator implements Cloneable { if (mPauseListeners != null) { anim.mPauseListeners = new ArrayList(mPauseListeners); } + anim.mCachedList = null; return anim; } catch (CloneNotSupportedException e) { throw new AssertionError(); @@ -590,6 +584,70 @@ public abstract class Animator implements Cloneable { } } + /** + * Calls notification for each AnimatorListener. + * + * @param notification The notification method to call on each listener. + * @param isReverse When this is used with start/end, this is the isReverse parameter. For + * other calls, this is ignored. + */ + void notifyListeners( + AnimatorCaller notification, + boolean isReverse + ) { + callOnList(mListeners, notification, this, isReverse); + } + + /** + * Call pause/resume on each AnimatorPauseListener. + * + * @param notification Either ON_PAUSE or ON_RESUME to call onPause or onResume on each + * listener. + */ + void notifyPauseListeners(AnimatorCaller notification) { + callOnList(mPauseListeners, notification, this, false); + } + + /** + * Calls call for every item in list with animator and + * isReverse as parameters. + * + * @param list The list of items to make calls on. + * @param call The method to call for each item in list. + * @param animator The animator parameter of call. + * @param isReverse The isReverse parameter of call. + * @param The item type of list + * @param The Animator type of animator. + */ + void callOnList( + ArrayList list, + AnimatorCaller call, + A animator, + boolean isReverse + ) { + int size = list == null ? 0 : list.size(); + if (size > 0) { + // Try to reuse mCacheList to store the items of list. + Object[] array; + if (mCachedList == null || mCachedList.length < size) { + array = new Object[size]; + } else { + array = mCachedList; + // Clear it in case there is some reentrancy + mCachedList = null; + } + list.toArray(array); + for (int i = 0; i < size; i++) { + //noinspection unchecked + T item = (T) array[i]; + call.call(item, animator, isReverse); + array[i] = null; + } + // Store it for the next call so we can reuse this array, if needed. + mCachedList = array; + } + } + /** *

An animation listener receives notifications from an animation. * Notifications indicate animation related events, such as the end or the @@ -748,4 +806,29 @@ public abstract class Animator implements Cloneable { return clone; } } + + /** + * Internally used by {@link #callOnList(ArrayList, AnimatorCaller, Object, boolean)} to + * make a call on all children of a list. This can be for start, stop, pause, cancel, update, + * etc notifications. + * + * @param The type of listener to make the call on + * @param The type of animator that is passed as a parameter + */ + interface AnimatorCaller { + void call(T listener, A animator, boolean isReverse); + + AnimatorCaller ON_START = AnimatorListener::onAnimationStart; + AnimatorCaller ON_END = AnimatorListener::onAnimationEnd; + AnimatorCaller ON_CANCEL = + (listener, animator, isReverse) -> listener.onAnimationCancel(animator); + AnimatorCaller ON_REPEAT = + (listener, animator, isReverse) -> listener.onAnimationRepeat(animator); + AnimatorCaller ON_PAUSE = + (listener, animator, isReverse) -> listener.onAnimationPause(animator); + AnimatorCaller ON_RESUME = + (listener, animator, isReverse) -> listener.onAnimationResume(animator); + AnimatorCaller ON_UPDATE = + (listener, animator, isReverse) -> listener.onAnimationUpdate(animator); + } } diff --git a/core/java/android/animation/AnimatorSet.java b/core/java/android/animation/AnimatorSet.java index a700efb7234b1..09eec9d25a4bd 100644 --- a/core/java/android/animation/AnimatorSet.java +++ b/core/java/android/animation/AnimatorSet.java @@ -32,6 +32,7 @@ import java.util.Collection; import java.util.Comparator; import java.util.HashMap; import java.util.List; +import java.util.function.Consumer; /** * This class plays a set of {@link Animator} objects in the specified order. Animations @@ -424,24 +425,28 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim throw new AndroidRuntimeException("Animators may only be run on Looper threads"); } if (isStarted()) { - ArrayList tmpListeners = null; - if (mListeners != null) { - tmpListeners = (ArrayList) mListeners.clone(); - int size = tmpListeners.size(); - for (int i = 0; i < size; i++) { - tmpListeners.get(i).onAnimationCancel(this); - } - } - ArrayList playingSet = new ArrayList<>(mPlayingSet); - int setSize = playingSet.size(); - for (int i = 0; i < setSize; i++) { - playingSet.get(i).mAnimation.cancel(); - } + notifyListeners(AnimatorCaller.ON_CANCEL, false); + callOnPlayingSet(Animator::cancel); mPlayingSet.clear(); endAnimation(); } } + /** + * Calls consumer on every Animator of mPlayingSet. + * + * @param consumer The method to call on every Animator of mPlayingSet. + */ + private void callOnPlayingSet(Consumer consumer) { + final ArrayList list = mPlayingSet; + final int size = list.size(); + //noinspection ForLoopReplaceableByForEach + for (int i = 0; i < size; i++) { + final Animator animator = list.get(i).mAnimation; + consumer.accept(animator); + } + } + // Force all the animations to end when the duration scale is 0. private void forceToEnd() { if (mEndCanBeCalled) { @@ -662,6 +667,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim super.pause(); if (!previouslyPaused && mPaused) { mPauseTime = -1; + callOnPlayingSet(Animator::pause); } } @@ -676,6 +682,7 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim if (mPauseTime >= 0) { addAnimationCallback(0); } + callOnPlayingSet(Animator::resume); } } @@ -751,26 +758,14 @@ public final class AnimatorSet extends Animator implements AnimationHandler.Anim private void notifyStartListeners(boolean inReverse) { if (mListeners != null && !mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - AnimatorListener listener = tmpListeners.get(i); - listener.onAnimationStart(this, inReverse); - } + notifyListeners(AnimatorCaller.ON_START, inReverse); } mStartListenersCalled = true; } private void notifyEndListeners(boolean inReverse) { if (mListeners != null && mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - AnimatorListener listener = tmpListeners.get(i); - listener.onAnimationEnd(this, inReverse); - } + notifyListeners(AnimatorCaller.ON_END, inReverse); } mStartListenersCalled = false; } diff --git a/core/java/android/animation/ValueAnimator.java b/core/java/android/animation/ValueAnimator.java index 7009725aa32bc..46d9847c2a94e 100644 --- a/core/java/android/animation/ValueAnimator.java +++ b/core/java/android/animation/ValueAnimator.java @@ -1110,24 +1110,14 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio private void notifyStartListeners(boolean isReversing) { if (mListeners != null && !mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationStart(this, isReversing); - } + notifyListeners(AnimatorCaller.ON_START, isReversing); } mStartListenersCalled = true; } private void notifyEndListeners(boolean isReversing) { if (mListeners != null && mStartListenersCalled) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - int numListeners = tmpListeners.size(); - for (int i = 0; i < numListeners; ++i) { - tmpListeners.get(i).onAnimationEnd(this, isReversing); - } + notifyListeners(AnimatorCaller.ON_END, isReversing); } mStartListenersCalled = false; } @@ -1224,15 +1214,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio // If it's not yet running, then start listeners weren't called. Call them now. notifyStartListeners(mReversing); } - int listenersSize = mListeners.size(); - if (listenersSize > 0) { - ArrayList tmpListeners = - (ArrayList) mListeners.clone(); - for (int i = 0; i < listenersSize; i++) { - AnimatorListener listener = tmpListeners.get(i); - listener.onAnimationCancel(this); - } - } + notifyListeners(AnimatorCaller.ON_CANCEL, false); } endAnimation(); @@ -1435,12 +1417,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio done = true; } else if (newIteration && !lastIterationFinished) { // Time to repeat - if (mListeners != null) { - int numListeners = mListeners.size(); - for (int i = 0; i < numListeners; ++i) { - mListeners.get(i).onAnimationRepeat(this); - } - } + notifyListeners(AnimatorCaller.ON_REPEAT, false); } else if (lastIterationFinished) { done = true; } @@ -1494,12 +1471,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio lastIteration = Math.min(lastIteration, mRepeatCount); if (iteration != lastIteration) { - if (mListeners != null) { - int numListeners = mListeners.size(); - for (int i = 0; i < numListeners; ++i) { - mListeners.get(i).onAnimationRepeat(this); - } - } + notifyListeners(AnimatorCaller.ON_REPEAT, false); } } @@ -1697,12 +1669,7 @@ public class ValueAnimator extends Animator implements AnimationHandler.Animatio for (int i = 0; i < numValues; ++i) { mValues[i].calculateValue(fraction); } - if (mUpdateListeners != null) { - int numListeners = mUpdateListeners.size(); - for (int i = 0; i < numListeners; ++i) { - mUpdateListeners.get(i).onAnimationUpdate(this); - } - } + callOnList(mUpdateListeners, AnimatorCaller.ON_UPDATE, this, false); } @Override diff --git a/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java new file mode 100644 index 0000000000000..fda08ff1e6773 --- /dev/null +++ b/core/tests/coretests/src/android/animation/AnimatorSetCallsTest.java @@ -0,0 +1,251 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.animation; + +import static org.junit.Assert.assertEquals; + +import android.util.PollingCheck; +import android.view.View; + +import androidx.test.ext.junit.rules.ActivityScenarioRule; +import androidx.test.filters.MediumTest; + +import com.android.frameworks.coretests.R; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +@MediumTest +public class AnimatorSetCallsTest { + @Rule + public final ActivityScenarioRule mRule = + new ActivityScenarioRule<>(AnimatorSetActivity.class); + + private AnimatorSetActivity mActivity; + private AnimatorSet mSet1; + private CountListener mListener1; + private CountListener mListener2; + private CountListener mListener3; + + @Before + public void setUp() throws Exception { + mRule.getScenario().onActivity((activity) -> { + mActivity = activity; + View square = mActivity.findViewById(R.id.square1); + + mSet1 = new AnimatorSet(); + mListener1 = new CountListener(); + mSet1.addListener(mListener1); + mSet1.addPauseListener(mListener1); + + AnimatorSet set2 = new AnimatorSet(); + mListener2 = new CountListener(); + set2.addListener(mListener2); + set2.addPauseListener(mListener2); + + ObjectAnimator anim = ObjectAnimator.ofFloat(square, "translationX", 0f, 100f); + mListener3 = new CountListener(); + anim.addListener(mListener3); + anim.addPauseListener(mListener3); + anim.setDuration(1); + + set2.play(anim); + mSet1.play(set2); + }); + } + + @Test + public void startEndCalledOnChildren() { + mRule.getScenario().onActivity((a) -> mSet1.start()); + waitForOnUiThread(() -> mListener1.endForward > 0); + + // only startForward and endForward should have been called once + mListener1.assertValues( + 1, 0, 1, 0, 0, 0, 0, 0 + ); + mListener2.assertValues( + 1, 0, 1, 0, 0, 0, 0, 0 + ); + mListener3.assertValues( + 1, 0, 1, 0, 0, 0, 0, 0 + ); + } + + @Test + public void cancelCalledOnChildren() { + mRule.getScenario().onActivity((a) -> { + mSet1.start(); + mSet1.cancel(); + }); + waitForOnUiThread(() -> mListener1.endForward > 0); + + // only startForward and endForward should have been called once + mListener1.assertValues( + 1, 0, 1, 0, 1, 0, 0, 0 + ); + mListener2.assertValues( + 1, 0, 1, 0, 1, 0, 0, 0 + ); + mListener3.assertValues( + 1, 0, 1, 0, 1, 0, 0, 0 + ); + } + + @Test + public void startEndReversedCalledOnChildren() { + mRule.getScenario().onActivity((a) -> mSet1.reverse()); + waitForOnUiThread(() -> mListener1.endReverse > 0); + + // only startForward and endForward should have been called once + mListener1.assertValues( + 0, 1, 0, 1, 0, 0, 0, 0 + ); + mListener2.assertValues( + 0, 1, 0, 1, 0, 0, 0, 0 + ); + mListener3.assertValues( + 0, 1, 0, 1, 0, 0, 0, 0 + ); + } + + @Test + public void pauseResumeCalledOnChildren() { + mRule.getScenario().onActivity((a) -> { + mSet1.start(); + mSet1.pause(); + }); + waitForOnUiThread(() -> mListener1.pause > 0); + + // only startForward and pause should have been called once + mListener1.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener2.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + mListener3.assertValues( + 1, 0, 0, 0, 0, 0, 1, 0 + ); + + mRule.getScenario().onActivity((a) -> mSet1.resume()); + waitForOnUiThread(() -> mListener1.endForward > 0); + + // resume and endForward should have been called once + mListener1.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + mListener2.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + mListener3.assertValues( + 1, 0, 1, 0, 0, 0, 1, 1 + ); + } + + private void waitForOnUiThread(PollingCheck.PollingCheckCondition condition) { + final boolean[] value = new boolean[1]; + PollingCheck.waitFor(() -> { + mActivity.runOnUiThread(() -> value[0] = condition.canProceed()); + return value[0]; + }); + } + + private static class CountListener implements Animator.AnimatorListener, + Animator.AnimatorPauseListener { + public int startNoParam; + public int endNoParam; + public int startReverse; + public int startForward; + public int endForward; + public int endReverse; + public int cancel; + public int repeat; + public int pause; + public int resume; + + public void assertValues( + int startForward, + int startReverse, + int endForward, + int endReverse, + int cancel, + int repeat, + 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); + } + + @Override + public void onAnimationStart(Animator animation, boolean isReverse) { + if (isReverse) { + startReverse++; + } else { + startForward++; + } + } + + @Override + public void onAnimationEnd(Animator animation, boolean isReverse) { + if (isReverse) { + endReverse++; + } else { + endForward++; + } + } + + @Override + public void onAnimationStart(Animator animation) { + startNoParam++; + } + + @Override + public void onAnimationEnd(Animator animation) { + endNoParam++; + } + + @Override + public void onAnimationCancel(Animator animation) { + cancel++; + } + + @Override + public void onAnimationRepeat(Animator animation) { + repeat++; + } + + @Override + public void onAnimationPause(Animator animation) { + pause++; + } + + @Override + public void onAnimationResume(Animator animation) { + resume++; + } + } +}