Starting new transition cancels running transition

The behavior of running a transition is janky and unpredictable,
when there is already a transition running on the same scene root.
Usually, the new transition simply jumps to the end values, or
jumps to the start values for that transition and animates from
there.

A better approach is to cancel any running transition first, the
start the new transition from that point.

Even better would be to blend old/new transitions, or at least adjust
the animation timing according to where/when the previous transition
stopped. In the meantime, this fix is at least better than the
previous approach of ignoring running transitions.

Change-Id: I4f5fabb55f6454f1e9d66589a9a7c36f9fc013fb

3 files changed, 53 insertions, 6 deletions

diff --git a/core/java/android/view/transition/Transition.java b/core/java/android/view/transition/Transition.java
index 2e3cdee26c77..8f2bc5ac7ee8 100644
--- a/core/java/android/view/transition/Transition.java
+++ b/core/java/android/view/transition/Transition.java
@@ -78,6 +78,10 @@ public abstract class Transition {
     private ArrayList<TransitionValues> mPlayStartValuesList = new ArrayList<TransitionValues>();
     private ArrayList<TransitionValues> mPlayEndValuesList = new ArrayList<TransitionValues>();
 
+    // Track all animators in use in case the transition gets canceled and needs to
+    // cancel running animators
+    private ArrayList<Animator> mCurrentAnimators = new ArrayList<Animator>();
+
     // Number of per-target instances of this Transition currently running. This count is
     // determined by calls to startTransition() and endTransition()
     int mNumInstances = 0;
@@ -401,13 +405,30 @@ public abstract class Transition {
             TransitionValues start = mPlayStartValuesList.get(i);
             TransitionValues end = mPlayEndValuesList.get(i);
             startTransition();
-            animate(play(sceneRoot, start, end));
+            runAnimator(play(sceneRoot, start, end));
         }
         mPlayStartValuesList.clear();
         mPlayEndValuesList.clear();
         endTransition();
     }
 
+    private void runAnimator(Animator animator) {
+        if (animator != null) {
+            // TODO: could be a single listener instance for all of them since it uses the param
+            animator.addListener(new AnimatorListenerAdapter() {
+                @Override
+                public void onAnimationStart(Animator animation) {
+                    mCurrentAnimators.add(animation);
+                }
+                @Override
+                public void onAnimationEnd(Animator animation) {
+                    mCurrentAnimators.remove(animation);
+                }
+            });
+            animate(animator);
+        }
+    }
+
     /**
      * Captures the current scene of values for the properties that this
      * transition monitors. These values can be either the start or end
@@ -668,11 +689,6 @@ public abstract class Transition {
             }
             animator.addListener(new AnimatorListenerAdapter() {
                 @Override
-                public void onAnimationCancel(Animator animation) {
-                    cancelTransition();
-                }
-
-                @Override
                 public void onAnimationEnd(Animator animation) {
                     endTransition();
                     animation.removeListener(this);
@@ -771,6 +787,7 @@ public abstract class Transition {
             mEndValues.clear();
             mEndIdValues.clear();
             mEndItemIdValues.clear();
+            mCurrentAnimators.clear();
         }
     }
 
@@ -781,6 +798,11 @@ public abstract class Transition {
     protected void cancelTransition() {
         // TODO: how does this work with instances?
         // TODO: this doesn't actually do *anything* yet
+        int numAnimators = mCurrentAnimators.size();
+        for (int i = 0; i < numAnimators; ++i) {
+            Animator animator = mCurrentAnimators.get(i);
+            animator.cancel();
+        }
         onTransitionCancel();
         if (mListeners != null && mListeners.size() > 0) {
             ArrayList<TransitionListener> tmpListeners =
diff --git a/core/java/android/view/transition/TransitionGroup.java b/core/java/android/view/transition/TransitionGroup.java
index 8d222d8a073d..4ebb53f68d8d 100644
--- a/core/java/android/view/transition/TransitionGroup.java
+++ b/core/java/android/view/transition/TransitionGroup.java
@@ -303,6 +303,15 @@ public class TransitionGroup extends Transition {
     }
 
     @Override
+    protected void cancelTransition() {
+        super.cancelTransition();
+        int numTransitions = mTransitions.size();
+        for (int i = 0; i < numTransitions; ++i) {
+            mTransitions.get(i).cancelTransition();
+        }
+    }
+
+    @Override
     String toString(String indent) {
         String result = super.toString(indent);
         for (int i = 0; i < mTransitions.size(); ++i) {
diff --git a/core/java/android/view/transition/TransitionManager.java b/core/java/android/view/transition/TransitionManager.java
index 5a1991cdcf90..17f0d089aeb2 100644
--- a/core/java/android/view/transition/TransitionManager.java
+++ b/core/java/android/view/transition/TransitionManager.java
@@ -15,6 +15,7 @@
  */
 package android.view.transition;
 
+import android.util.ArrayMap;
 import android.view.ViewGroup;
 import android.view.ViewTreeObserver;
 
@@ -40,6 +41,8 @@ public class TransitionManager {
     HashMap<Scene, Transition> mSceneTransitions = new HashMap<Scene, Transition>();
     HashMap<Scene, HashMap<Scene, Transition>> mScenePairTransitions =
             new HashMap<Scene, HashMap<Scene, Transition>>();
+    static ArrayMap<ViewGroup, Transition> sRunningTransitions =
+            new ArrayMap<ViewGroup, Transition>();
 
         /**
      * Sets the transition to be used for any scene change for which no
@@ -141,6 +144,11 @@ public class TransitionManager {
 
         final ViewGroup sceneRoot = scene.getSceneRoot();
 
+        Transition runningTransition = sRunningTransitions.get(sceneRoot);
+        if (runningTransition != null) {
+            runningTransition.cancelTransition();
+        }
+
         // Capture current values
         if (transition != null) {
             transition.captureValues(sceneRoot, true);
@@ -159,6 +167,14 @@ public class TransitionManager {
             observer.addOnPreDrawListener(new ViewTreeObserver.OnPreDrawListener() {
                 public boolean onPreDraw() {
                     sceneRoot.getViewTreeObserver().removeOnPreDrawListener(this);
+                    // Add to running list, handle end to remove it
+                    sRunningTransitions.put(sceneRoot, transition);
+                    transition.addListener(new Transition.TransitionListenerAdapter() {
+                        @Override
+                        public void onTransitionEnd(Transition transition) {
+                            sRunningTransitions.remove(sceneRoot);
+                        }
+                    });
                     transition.captureValues(sceneRoot, false);
                     transition.play(sceneRoot);
                     return true;