Let animation factories be used for multiple loads

library/src/androidTest/java/com/bumptech/glide/request/animation/DrawableCrossFadeViewAnimationTest.java

@@ -4,10 +4,7 @@ import android.graphics.Color;
 import android.graphics.drawable.ColorDrawable;
 import android.graphics.drawable.Drawable;
 import android.graphics.drawable.TransitionDrawable;
-import android.view.animation.AlphaAnimation;
-import android.view.animation.Animation;
 import android.widget.ImageView;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -17,6 +14,7 @@ import static com.bumptech.glide.request.animation.GlideAnimation.ViewAdapter;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -34,7 +32,7 @@ public class DrawableCrossFadeViewAnimationTest {
     public void testStartsDefaultAnimationIfNoPreviousDrawableIsNotSet() {
         when(harness.adapter.getView()).thenReturn(harness.view);
         harness.animation.animate(harness.current, harness.adapter);
-        verify(harness.view).startAnimation(any(Animation.class));
+        verify(harness.defaultAnimation).animate(eq(harness.current), eq(harness.adapter));
     }
 
     @Test
@@ -69,7 +67,7 @@ public class DrawableCrossFadeViewAnimationTest {
         Drawable current = new ColorDrawable(Color.GRAY);
         ViewAdapter adapter = mock(ViewAdapter.class);
         ImageView view = mock(ImageView.class);
-        Animation defaultAnimation = new AlphaAnimation(0f, 1f);
+        GlideAnimation<Drawable> defaultAnimation = mock(GlideAnimation.class);
         int duration = 200;
         DrawableCrossFadeViewAnimation<Drawable> animation = new DrawableCrossFadeViewAnimation<Drawable>(
                 defaultAnimation, duration);

library/src/androidTest/java/com/bumptech/glide/request/animation/ViewAnimationTest.java

 package com.bumptech.glide.request.animation;
 
-import android.view.animation.AlphaAnimation;
+import android.view.animation.Animation;
 import android.widget.ImageView;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -17,19 +16,19 @@ import static org.mockito.Mockito.when;
 
 @RunWith(RobolectricTestRunner.class)
 public class ViewAnimationTest {
-    private AlphaAnimation animation;
-    private ViewAnimation viewAnimation;
+    private ViewAnimation<Object> viewAnimation;
     private ViewAdapter adapter;
     private ImageView view;
+    private ViewAnimation.AnimationFactory animationFactory;
 
     @SuppressWarnings("unchecked")
     @Before
     public void setUp() {
-        animation = new AlphaAnimation(0f, 1f);
+        animationFactory = mock(ViewAnimation.AnimationFactory.class);
         view = mock(ImageView.class);
         adapter = mock(ViewAdapter.class);
         when(adapter.getView()).thenReturn(view);
-        viewAnimation = new ViewAnimation(animation);
+        viewAnimation = new ViewAnimation<Object>(animationFactory);
     }
 
     @Test
@@ -46,7 +45,10 @@ public class ViewAnimationTest {
 
     @Test
     public void testStartsAnimationOnAnimate() {
+        Animation animation = mock(Animation.class);
+        when(animationFactory.build()).thenReturn(animation);
         viewAnimation.animate(null, adapter);
+        verify(view).clearAnimation();
         verify(view).startAnimation(eq(animation));
     }
 }

library/src/main/java/com/bumptech/glide/BitmapRequestBuilder.java

@@ -331,6 +331,8 @@ public class BitmapRequestBuilder<ModelType, TranscodeType>
     /**
      * {@inheritDoc}
      */
+    @Deprecated
+    @SuppressWarnings("deprecation")
     @Override
     public BitmapRequestBuilder<ModelType, TranscodeType> animate(Animation animation) {
         super.animate(animation);

library/src/main/java/com/bumptech/glide/DrawableOptions.java

@@ -10,7 +10,6 @@ interface DrawableOptions {
      *
      * @see #crossFade(int)
      * @see #crossFade(int, int)
-     * @see #crossFade(android.view.animation.Animation, int)
      *
      * @return This request builder.
      */
@@ -22,7 +21,6 @@ interface DrawableOptions {
      *
      * @see #crossFade()
      * @see #crossFade(int, int)
-     * @see #crossFade(android.view.animation.Animation, int)
      *
      * @param duration The duration of the cross fade and initial fade in.
      * @return This request builder.
@@ -39,13 +37,17 @@ interface DrawableOptions {
      * @see #crossFade(int)
      * @see #crossFade(int, int)
      *
+     * @deprecated If this builder is used for multiple loads, using this method will result in multiple view's being
+     * asked to start an animation using a single {@link android.view.animation.Animation} object which results in
+     * views animating repeatedly. Use {@link #crossFade(int, int)}} instead, or be sure to call this method once
+     * per call to {@link com.bumptech.glide.GenericRequestBuilder#load(Object)} to avoid re-using animation objects.
      * @param animation The Animation to use if no placeholder is set.
      * @param duration The duration of the cross fade animation.
      * @return This request builder.
      */
+    @Deprecated
     GenericRequestBuilder<?, ?, ?, ?> crossFade(Animation animation, int duration);
 
-
     /**
      * Applies a cross fade transformation that des from the placeholder to the loaded
      * {@link android.graphics.drawable.Drawable}. If no placeholder is set, the Drawable will instead be animated in
@@ -53,7 +55,6 @@ interface DrawableOptions {
      *
      * @see #crossFade()
      * @see #crossFade(int)
-     * @see #crossFade(android.view.animation.Animation, int)
      *
      * @param animationId The id of the Animation to use if no placeholder is set.
      * @param duration The duration of the cross fade animation.

library/src/main/java/com/bumptech/glide/DrawableRequestBuilder.java

@@ -257,6 +257,7 @@ public class DrawableRequestBuilder<ModelType>
     /**
      * {@inheritDoc}
      */
+    @Deprecated
     public DrawableRequestBuilder<ModelType> crossFade(Animation animation, int duration) {
         super.animate(new DrawableCrossFadeViewAnimation.DrawableCrossFadeFactory<GlideDrawable>(animation, duration));
         return this;
@@ -301,6 +302,8 @@ public class DrawableRequestBuilder<ModelType>
     /**
      * {@inheritDoc}
      */
+    @Deprecated
+    @SuppressWarnings("deprecation")
     @Override
     public DrawableRequestBuilder<ModelType> animate(Animation animation) {
         super.animate(animation);

library/src/main/java/com/bumptech/glide/GenericRequestBuilder.java

@@ -366,9 +366,7 @@ public class GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeT
      * @param animationId The resource id of the animation to run
      * @return This request builder.
      */
-    // This is safe because the view animation doesn't care about the resource type it receives.
-    public GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeType> animate(
-            int animationId) {
+    public GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeType> animate(int animationId) {
         return animate(new ViewAnimation.ViewAnimationFactory<TranscodeType>(context, animationId));
     }
 
@@ -376,12 +374,18 @@ public class GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeT
      * Sets an animation to run on the wrapped target when a resource load finishes. Will only be run if the resource
      * was loaded asynchronously (ie was not in the memory cache)
      *
+     * @see #animate(int)
+     * @see #animate(com.bumptech.glide.request.animation.ViewPropertyAnimation.Animator)
+     *
+     * @deprecated If this builder is used for multiple loads, using this method will result in multiple view's being
+     * asked to start an animation using a single {@link android.view.animation.Animation} object which results in
+     * views animating repeatedly. Use {@link #animate(int)} or
+     * {@link #animate(com.bumptech.glide.request.animation.ViewPropertyAnimation.Animator)}.
      * @param animation The animation to run
      * @return This request builder.
      */
-    // This is safe because the view animation doesn't care about the resource type it receives.
-    public GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeType> animate(
-            Animation animation) {
+    @Deprecated
+    public GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeType> animate(Animation animation) {
         return animate(new ViewAnimation.ViewAnimationFactory<TranscodeType>(animation));
     }
 
@@ -393,7 +397,6 @@ public class GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeT
      * @param animator The {@link com.bumptech.glide.request.animation.ViewPropertyAnimation.Animator} to run.
      * @return This request builder.
      */
-    // This is safe because the view property animation doesn't care about the resource type it receives.
     public GenericRequestBuilder<ModelType, DataType, ResourceType, TranscodeType> animate(
             ViewPropertyAnimation.Animator animator) {
         return animate(new ViewPropertyAnimation.ViewPropertyAnimationFactory<TranscodeType>(animator));

library/src/main/java/com/bumptech/glide/GifRequestBuilder.java

@@ -245,6 +245,7 @@ public class GifRequestBuilder<ModelType>
     /**
      * {@inheritDoc}
      */
+    @Deprecated
     @Override
     public GifRequestBuilder<ModelType> crossFade(Animation animation, int duration) {
         super.animate(new DrawableCrossFadeViewAnimation.DrawableCrossFadeFactory<GifDrawable>(animation, duration));
@@ -282,6 +283,8 @@ public class GifRequestBuilder<ModelType>
     /**
      * {@inheritDoc}
      */
+    @Deprecated
+    @SuppressWarnings("deprecation")
     @Override
     public GifRequestBuilder<ModelType> animate(Animation animation) {
         super.animate(animation);

library/src/main/java/com/bumptech/glide/request/animation/DrawableCrossFadeViewAnimation.java

@@ -3,10 +3,8 @@ package com.bumptech.glide.request.animation;
 import android.content.Context;
 import android.graphics.drawable.Drawable;
 import android.graphics.drawable.TransitionDrawable;
-import android.view.View;
 import android.view.animation.AlphaAnimation;
 import android.view.animation.Animation;
-import android.view.animation.AnimationUtils;
 
 /**
  * A cross fade {@link GlideAnimation} for {@link android.graphics.drawable.Drawable}s
@@ -19,15 +17,9 @@ import android.view.animation.AnimationUtils;
 public class DrawableCrossFadeViewAnimation<T extends Drawable> implements GlideAnimation<T> {
     // 150 ms.
     public static final int DEFAULT_DURATION = 300;
-    private final Animation defaultAnimation;
+    private final GlideAnimation<T> defaultAnimation;
     private final int duration;
 
-    private static Animation getDefaultAnimation() {
-        AlphaAnimation animation = new AlphaAnimation(0f, 1f);
-        animation.setDuration(DEFAULT_DURATION / 2);
-        return animation;
-    }
-
     /**
      * A factory class that produces a new {@link GlideAnimation} that varies depending
      * on whether or not the drawable was loaded from the memory cache and whether or not the drawable is the first
@@ -41,28 +33,28 @@ public class DrawableCrossFadeViewAnimation<T extends Drawable> implements Glide
      * </p>
      */
     public static class DrawableCrossFadeFactory<T extends Drawable> implements GlideAnimationFactory<T> {
-        private Context context;
-        private int defaultAnimationId;
-        private Animation defaultAnimation;
-        private int duration;
+        private final ViewAnimation.ViewAnimationFactory<T> animationFactory;
+        private final int duration;
         private DrawableCrossFadeViewAnimation<T> animation;
 
         public DrawableCrossFadeFactory() {
-            this(getDefaultAnimation(), DEFAULT_DURATION);
+            this(DEFAULT_DURATION);
         }
 
         public DrawableCrossFadeFactory(int duration) {
-            this(getDefaultAnimation(), duration);
+            this(new ViewAnimation.ViewAnimationFactory<T>(new DefaultAnimationFactory()), duration);
         }
 
         public DrawableCrossFadeFactory(Context context, int defaultAnimationId, int duration) {
-            this.context = context;
-            this.defaultAnimationId = defaultAnimationId;
-            this.duration = duration;
+            this(new ViewAnimation.ViewAnimationFactory<T>(context, defaultAnimationId), duration);
         }
 
         public DrawableCrossFadeFactory(Animation defaultAnimation, int duration) {
-            this.defaultAnimation = defaultAnimation;
+            this(new ViewAnimation.ViewAnimationFactory<T>(defaultAnimation), duration);
+        }
+
+        DrawableCrossFadeFactory(ViewAnimation.ViewAnimationFactory<T> animationFactory, int duration) {
+            this.animationFactory = animationFactory;
             this.duration = duration;
         }
 
@@ -73,9 +65,7 @@ public class DrawableCrossFadeViewAnimation<T extends Drawable> implements Glide
             }
 
             if (animation == null) {
-                if (defaultAnimation == null) {
-                    defaultAnimation = AnimationUtils.loadAnimation(context, defaultAnimationId);
-                }
+                GlideAnimation<T> defaultAnimation = animationFactory.build(false, isFirstResource);
                 animation = new DrawableCrossFadeViewAnimation<T>(defaultAnimation, duration);
             }
 
@@ -86,12 +76,10 @@ public class DrawableCrossFadeViewAnimation<T extends Drawable> implements Glide
     /**
      * Constructor that takes a default animation and a duration in milliseconds that the cross fade animation should
      * last.
-     * @param defaultAnimation The default animation that will run if there is nothing to cross fade from when a new
-     *                         {@link android.graphics.drawable.Drawable} is set.
      * @param duration The duration that the cross fade animation should run if there is something to cross fade from
      *                 when a new {@link android.graphics.drawable.Drawable} is set.
      */
-    public DrawableCrossFadeViewAnimation(Animation defaultAnimation, int duration) {
+    public DrawableCrossFadeViewAnimation(GlideAnimation<T> defaultAnimation, int duration) {
         this.defaultAnimation = defaultAnimation;
         this.duration = duration;
     }
@@ -119,11 +107,18 @@ public class DrawableCrossFadeViewAnimation<T extends Drawable> implements Glide
             adapter.setDrawable(transitionDrawable);
             return true;
         } else {
-            View view = adapter.getView();
-            if (view != null) {
-                view.startAnimation(defaultAnimation);
-            }
+            defaultAnimation.animate(current, adapter);
             return false;
         }
     }
+
+    private static class DefaultAnimationFactory implements ViewAnimation.AnimationFactory {
+
+        @Override
+        public Animation build() {
+            AlphaAnimation animation = new AlphaAnimation(0f, 1f);
+            animation.setDuration(DEFAULT_DURATION / 2);
+            return animation;
+        }
+    }
 }

library/src/main/java/com/bumptech/glide/request/animation/ViewAnimation.java

@@ -14,14 +14,13 @@ import android.view.animation.AnimationUtils;
  */
 public class ViewAnimation<R> implements GlideAnimation<R> {
 
-    private final Animation animation;
+    private final AnimationFactory animationFactory;
 
     /**
      * Constructs a new ViewAnimation that will start the given {@link android.view.animation.Animation}.
-     * @param animation The animation to use.
      */
-    public ViewAnimation(Animation animation) {
-        this.animation = animation;
+    ViewAnimation(AnimationFactory animationFactory) {
+        this.animationFactory = animationFactory;
     }
 
     /**
@@ -39,7 +38,7 @@ public class ViewAnimation<R> implements GlideAnimation<R> {
         View view = adapter.getView();
         if (view != null) {
             view.clearAnimation();
-
+            Animation animation = animationFactory.build();
             view.startAnimation(animation);
         }
 
@@ -50,18 +49,19 @@ public class ViewAnimation<R> implements GlideAnimation<R> {
      * A {@link com.bumptech.glide.request.animation.GlideAnimationFactory} that produces ViewAnimations.
      */
     public static class ViewAnimationFactory<R> implements GlideAnimationFactory<R> {
-        private Animation animation;
-        private Context context;
-        private int animationId;
+        private final AnimationFactory animationFactory;
         private GlideAnimation<R> glideAnimation;
 
         public ViewAnimationFactory(Animation animation) {
-            this.animation = animation;
+            this(new ConcreteAnimationFactory(animation));
         }
 
         public ViewAnimationFactory(Context context, int animationId) {
-            this.context = context;
-            this.animationId = animationId;
+            this(new ResourceAnimationFactory(context, animationId));
+        }
+
+        ViewAnimationFactory(AnimationFactory animationFactory) {
+            this.animationFactory = animationFactory;
         }
 
         /**
@@ -80,13 +80,42 @@ public class ViewAnimation<R> implements GlideAnimation<R> {
             }
 
             if (glideAnimation == null) {
-                if (animation == null) {
-                    animation = AnimationUtils.loadAnimation(context, animationId);
-                }
-                glideAnimation = new ViewAnimation<R>(animation);
+                glideAnimation = new ViewAnimation<R>(animationFactory);
             }
 
             return glideAnimation;
         }
     }
+
+    private static class ConcreteAnimationFactory implements AnimationFactory {
+        private final Animation animation;
+
+        public ConcreteAnimationFactory(Animation animation) {
+            this.animation = animation;
+        }
+
+        @Override
+        public Animation build() {
+            return animation;
+        }
+    }
+
+    private static class ResourceAnimationFactory implements AnimationFactory {
+        private final Context context;
+        private final int animationId;
+
+        public ResourceAnimationFactory(Context context, int animationId) {
+            this.context = context.getApplicationContext();
+            this.animationId = animationId;
+        }
+
+        @Override
+        public Animation build() {
+            return AnimationUtils.loadAnimation(context, animationId);
+        }
+    }
+
+    interface AnimationFactory {
+        Animation build();
+    }
 }