Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c43524f3 authored by Chet Haase's avatar Chet Haase
Browse files

Fix issues with delayed transitions 

Previously, there were two distinct problems with how delayed
transitions were being run:
- there would be a delay between the transition being put into
a preDrawListener (to be kicked off when that listener fired) and
being removed from the pending list. This allowed another delayed
transition to be run in parallel, which would cause conflicting/
clobbering issues with transition values on the same objects.
- there would be an extra frame delay in some cases due to how/when the
delayed transition would be started. Specfically, we would postOnAnimation()
to call a method that would then add the onPreDraw listener. This two-step
forwarding caused issues noted above.

The fix is to simply add the transition to the preDrawListener immediately, removing
the two-step problem, and also ensuring that the transition is only removed
from the pending list when it is actually started, which prevents other transitions
from starting in the meantime.

Also, added more debug logging to help chase future issues with transitions.

Change-Id: Ie2ff8e73d29f342512842e9641bd8d605e74544c
parent cd6b8e49

core/java/android/view/transition/Fade.java

+3 −1
Original line number Diff line number Diff line
@@ -32,6 +32,8 @@ import android.view.ViewGroup; 
 */

public class Fade extends Visibility {



    private static boolean DBG = Transition.DBG && false;



    private static final String LOG_TAG = "Fade";

    private static final String PROPNAME_SCREEN_X = "android:fade:screenX";

    private static final String PROPNAME_SCREEN_Y = "android:fade:screenY";
@@ -121,7 +123,7 @@ public class Fade extends Visibility { 
        View view;

        View startView = (startValues != null) ? startValues.view : null;

        View endView = (endValues != null) ? endValues.view : null;

        if (Transition.DBG) {

        if (DBG) {

            Log.d(LOG_TAG, "Fade.predisappear: startView, startVis, endView, endVis = " +

                        startView + ", " + startVisibility + ", " + endView + ", " + endVisibility);

        }

core/java/android/view/transition/Transition.java

+77 −31
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.animation.Animator; 
import android.animation.AnimatorListenerAdapter;

import android.animation.TimeInterpolator;

import android.util.ArrayMap;

import android.util.Log;

import android.util.LongSparseArray;

import android.util.Pair;

import android.util.SparseArray;
@@ -84,11 +85,14 @@ public abstract class Transition implements Cloneable { 
    int mNumInstances = 0;





    /**

     * The set of listeners to be sent transition lifecycle events.

     */



    // The set of listeners to be sent transition lifecycle events.

    ArrayList<TransitionListener> mListeners = null;



    // The set of animators collected from calls to play(), to be run in runAnimations()

    ArrayMap<Pair<TransitionValues, TransitionValues>, Animator> mAnimatorMap =

            new ArrayMap<Pair<TransitionValues, TransitionValues>, Animator>();



    /**

     * Constructs a Transition object with no target objects. A transition with

     * no targets defaults to running on all target objects in the scene hierarchy
@@ -203,6 +207,9 @@ public abstract class Transition implements Cloneable { 
     */

    protected void play(ViewGroup sceneRoot, TransitionValuesMaps startValues,

            TransitionValuesMaps endValues) {

        if (DBG) {

            Log.d(LOG_TAG, "play() for " + this);

        }

        mPlayStartValuesList.clear();

        mPlayEndValuesList.clear();

        ArrayMap<View, TransitionValues> endCopy =
@@ -312,6 +319,29 @@ public abstract class Transition implements Cloneable { 
        for (int i = 0; i < startValuesList.size(); ++i) {

            TransitionValues start = startValuesList.get(i);

            TransitionValues end = endValuesList.get(i);

            // Only bother trying to animate with values that differ between start/end

            if (start != null || end != null) {

                if (start == null || !start.equals(end)) {

                    if (DBG) {

                        View view = (end != null) ? end.view : start.view;

                        Log.d(LOG_TAG, "  differing start/end values for view " +

                                view);

                        if (start == null || end == null) {

                            if (start == null) {

                                Log.d(LOG_TAG, "    " + ((start == null) ?

                                        "start null, end non-null" : "start non-null, end null"));

                            }

                        } else {

                            for (String key : start.values.keySet()) {

                                Object startValue = start.values.get(key);

                                Object endValue = end.values.get(key);

                                if (startValue != endValue && !startValue.equals(endValue)) {

                                    Log.d(LOG_TAG, "    " + key + ": start(" + startValue +

                                            "), end(" + endValue +")");

                                }

                            }

                        }

                    }

                    // TODO: what to do about targetIds and itemIds?

                    Animator animator = play(sceneRoot, start, end);

                    if (animator != null) {
@@ -320,11 +350,13 @@ public abstract class Transition implements Cloneable { 
                        mPlayStartValuesList.add(start);

                        mPlayEndValuesList.add(end);

                    }

                } else if (DBG) {

                    View view = (end != null) ? end.view : start.view;

                    Log.d(LOG_TAG, "  No change for view " + view);

                }

            }

        }

    }



    ArrayMap<Pair<TransitionValues, TransitionValues>, Animator> mAnimatorMap =

            new ArrayMap<Pair<TransitionValues, TransitionValues>, Animator>();



    /**

     * Internal utility method for checking whether a given view/id
@@ -364,14 +396,20 @@ public abstract class Transition implements Cloneable { 
     * @hide

     */

    protected void runAnimations() {



        if (DBG && mPlayStartValuesList.size() > 0) {

            Log.d(LOG_TAG, "runAnimations (" + mPlayStartValuesList.size() + ") on " + this);

        }

        startTransition();

        // Now walk the list of TransitionValues, calling play for each pair

        for (int i = 0; i < mPlayStartValuesList.size(); ++i) {

            TransitionValues start = mPlayStartValuesList.get(i);

            TransitionValues end = mPlayEndValuesList.get(i);

            Animator anim = mAnimatorMap.get(new Pair(start, end));

            if (DBG) {

                Log.d(LOG_TAG, "  anim: " + anim);

            }

            startTransition();

            runAnimator(mAnimatorMap.get(new Pair(start, end)));

            runAnimator(anim);

        }

        mPlayStartValuesList.clear();

        mPlayEndValuesList.clear();
@@ -871,9 +909,16 @@ public abstract class Transition implements Cloneable { 
    String toString(String indent) {

        String result = indent + getClass().getSimpleName() + "@" +

                Integer.toHexString(hashCode()) + ": ";

        if (mDuration != -1) {

            result += "dur(" + mDuration + ") ";

        }

        if (mStartDelay != -1) {

            result += "dly(" + mStartDelay + ") ";

        }

        if (mInterpolator != null) {

            result += "interp(" + mInterpolator + ") ";

        }

        if (mTargetIds != null || mTargets != null) {

            result += "tgts(";

            if (mTargetIds != null) {

                for (int i = 0; i < mTargetIds.length; ++i) {
@@ -892,6 +937,7 @@ public abstract class Transition implements Cloneable { 
                }

            }

            result += ")";

        }

        return result;

    }

core/java/android/view/transition/TransitionManager.java

+10 −9
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package android.view.transition;



import android.util.ArrayMap;

import android.util.Log;

import android.view.ViewGroup;

import android.view.ViewTreeObserver;
@@ -36,6 +37,8 @@ import java.util.ArrayList; 
public class TransitionManager {

    // TODO: how to handle enter/exit?



    private static String LOG_TAG = "TransitionManager";



    private static final Transition sDefaultTransition = new AutoTransition();

    private Transition mDefaultTransition = new AutoTransition();
@@ -164,6 +167,7 @@ public class TransitionManager { 
            observer.addOnPreDrawListener(new ViewTreeObserver.OnPreDrawListener() {

                public boolean onPreDraw() {

                    sceneRoot.getViewTreeObserver().removeOnPreDrawListener(this);

                    sPendingTransitions.remove(sceneRoot);

                    // Add to running list, handle end to remove it

                    sRunningTransitions.put(sceneRoot, transition);

                    transition.addListener(new Transition.TransitionListenerAdapter() {
@@ -316,8 +320,11 @@ public class TransitionManager { 
     * value of null causes the TransitionManager to use the default transition.

     */

    public static void beginDelayedTransition(final ViewGroup sceneRoot, Transition transition) {



        if (!sPendingTransitions.contains(sceneRoot)) {

        if (!sPendingTransitions.contains(sceneRoot) && sceneRoot.hasLayout()) {

            if (Transition.DBG) {

                Log.d(LOG_TAG, "beginDelayedTransition: root, transition = " +

                        sceneRoot + ", " + transition);

            }

            sPendingTransitions.add(sceneRoot);

            if (transition == null) {

                transition = sDefaultTransition;
@@ -325,13 +332,7 @@ public class TransitionManager { 
            final Transition finalTransition = transition.clone();

            sceneChangeSetup(sceneRoot, transition);

            sceneRoot.setCurrentScene(null);

            sceneRoot.postOnAnimation(new Runnable() {

                @Override

                public void run() {

                    sPendingTransitions.remove(sceneRoot);

            sceneChangeRunTransition(sceneRoot, finalTransition);

        }

            });

        }

    }

}

core/java/android/view/transition/TransitionValues.java

+17 −0
Original line number Diff line number Diff line
@@ -52,6 +52,23 @@ public class TransitionValues { 
     */

    public final Map<String, Object> values = new ArrayMap<String, Object>();



    @Override

    public boolean equals(Object other) {

        if (other instanceof TransitionValues) {

            if (view == ((TransitionValues) other).view) {

                if (values.equals(((TransitionValues) other).values)) {

                    return true;

                }

            }

        }

        return false;

    }



    @Override

    public int hashCode() {

        return 31*view.hashCode() + values.hashCode();

    }



    @Override

    public String toString() {

        String returnValue = "TransitionValues@" + Integer.toHexString(hashCode()) + ":\n";