Fix using incorrect target for Drawables.

library/src/androidTest/java/com/bumptech/glide/request/target/DrawableImageViewTargetTest.java → library/src/androidTest/java/com/bumptech/glide/request/target/GlideDrawableImageViewTargetTest.java

@@ -25,12 +25,12 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @RunWith(RobolectricTestRunner.class)
-public class DrawableImageViewTargetTest {
+public class GlideDrawableImageViewTargetTest {
 
     @Test
     public void testSetsDrawableOnViewInSetResource() {
         ImageView view = new ImageView(Robolectric.application);
-        DrawableImageViewTarget target = new DrawableImageViewTarget(view);
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(view);
         GlideDrawable expected = new MockAnimatedDrawable();
 
         target.setResource(expected);
@@ -43,7 +43,7 @@ public class DrawableImageViewTargetTest {
         ImageView mockView = mock(ImageView.class);
         when(mockView.getWidth()).thenReturn(100);
         when(mockView.getHeight()).thenReturn(100);
-        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(mockView);
         GlideDrawable drawable = new MockAnimatedDrawable() {
             @Override
             public int getIntrinsicHeight() {
@@ -75,7 +75,7 @@ public class DrawableImageViewTargetTest {
         ImageView mockView = mock(ImageView.class);
         when(mockView.getWidth()).thenReturn(100);
         when(mockView.getHeight()).thenReturn(100);
-        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(mockView);
         GlideDrawable drawable = new MockAnimatedDrawable() {
             @Override
             public int getIntrinsicHeight() {
@@ -105,7 +105,7 @@ public class DrawableImageViewTargetTest {
         ImageView mockView = mock(ImageView.class);
         when(mockView.getWidth()).thenReturn(100);
         when(mockView.getHeight()).thenReturn(100);
-        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(mockView);
         GlideDrawable drawable = new MockAnimatedDrawable() {
             @Override
             public int getIntrinsicHeight() {
@@ -135,7 +135,7 @@ public class DrawableImageViewTargetTest {
         ImageView mockView = mock(ImageView.class);
         when(mockView.getWidth()).thenReturn(100);
         when(mockView.getHeight()).thenReturn(150);
-        DrawableImageViewTarget target = new DrawableImageViewTarget(mockView);
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(mockView);
         GlideDrawable drawable = new MockAnimatedDrawable() {
             @Override
             public int getIntrinsicHeight() {
@@ -163,7 +163,7 @@ public class DrawableImageViewTargetTest {
     @Test
     public void testStartsAnimatableDrawablesInOnReasourceReady() {
         MockAnimatedDrawable drawable = new MockAnimatedDrawable();
-        DrawableImageViewTarget target = new DrawableImageViewTarget(new ImageView(Robolectric.application));
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(new ImageView(Robolectric.application));
         target.onResourceReady(drawable, null);
 
         assertTrue(drawable.isStarted);
@@ -172,7 +172,7 @@ public class DrawableImageViewTargetTest {
     @Test
     public void testStartsAnimatableDrawablesOnStart() {
         MockAnimatedDrawable drawable = new MockAnimatedDrawable();
-        DrawableImageViewTarget target = new DrawableImageViewTarget(new ImageView(Robolectric.application));
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(new ImageView(Robolectric.application));
         target.onResourceReady(drawable, null);
         target.onStop();
         target.onStart();
@@ -182,14 +182,14 @@ public class DrawableImageViewTargetTest {
 
     @Test
     public void testDoesNotStartNullDrawablesOnStart() {
-        DrawableImageViewTarget target = new DrawableImageViewTarget(new ImageView(Robolectric.application));
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(new ImageView(Robolectric.application));
         target.onStart();
     }
 
     @Test
     public void testStopsAnimatedDrawablesOnStop() {
         MockAnimatedDrawable drawable = new MockAnimatedDrawable();
-        DrawableImageViewTarget target = new DrawableImageViewTarget(new ImageView(Robolectric.application));
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(new ImageView(Robolectric.application));
         target.onResourceReady(drawable, null);
         target.onStop();
 
@@ -198,7 +198,7 @@ public class DrawableImageViewTargetTest {
 
     @Test
     public void testDoesNotStopNullDrawablesOnStop() {
-        DrawableImageViewTarget target = new DrawableImageViewTarget(new ImageView(Robolectric.application));
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(new ImageView(Robolectric.application));
         target.onStop();
     }
 
@@ -206,7 +206,7 @@ public class DrawableImageViewTargetTest {
     public void testSetsLoopCountOnDrawable() {
         int maxLoopCount = 6;
         MockAnimatedDrawable drawable = new MockAnimatedDrawable();
-        DrawableImageViewTarget target = new DrawableImageViewTarget(new ImageView(Robolectric.application),
+        GlideDrawableImageViewTarget target = new GlideDrawableImageViewTarget(new ImageView(Robolectric.application),
                 maxLoopCount);
         target.onResourceReady(drawable, null);
         assertEquals(maxLoopCount, drawable.loopCount);

library/src/androidTest/java/com/bumptech/glide/request/target/ImageViewTargetFactoryTest.java

 package com.bumptech.glide.request.target;
 
 import android.graphics.Bitmap;
+import android.graphics.Color;
 import android.graphics.drawable.BitmapDrawable;
+import android.graphics.drawable.ColorDrawable;
 import android.graphics.drawable.Drawable;
 import android.widget.ImageView;
+import com.bumptech.glide.load.resource.bitmap.GlideBitmapDrawable;
+import com.bumptech.glide.load.resource.drawable.GlideDrawable;
+import com.bumptech.glide.load.resource.gif.GifDrawable;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.robolectric.Robolectric;
 import org.robolectric.RobolectricTestRunner;
 
-import static junit.framework.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
 
 @RunWith(RobolectricTestRunner.class)
 public class ImageViewTargetFactoryTest {
@@ -25,17 +31,51 @@ public class ImageViewTargetFactoryTest {
 
     @Test
     public void testReturnsTargetForBitmaps() {
-        assertNotNull(factory.buildTarget(view, Bitmap.class));
+        Bitmap bitmap = Bitmap.createBitmap(50, 50, Bitmap.Config.ARGB_8888);
+        Target<Bitmap> target = factory.buildTarget(view, Bitmap.class);
+        target.onResourceReady(bitmap, null);
+        assertTrue(target instanceof BitmapImageViewTarget);
+    }
+
+    @Test
+    public void testReturnsTargetForGlideDrawables() {
+        GlideDrawable glideDrawable = mock(GlideDrawable.class);
+        Target<GlideDrawable> target = factory.buildTarget(view, GlideDrawable.class);
+        target.onResourceReady(glideDrawable, null);
+        assertTrue(target instanceof GlideDrawableImageViewTarget);
+    }
+
+    @Test
+    public void testReturnsTargetForGifDrawables() {
+        GifDrawable gifDrawable = mock(GifDrawable.class);
+        Target target = factory.buildTarget(view, GifDrawable.class);
+        target.onResourceReady(gifDrawable, null);
+        assertTrue(target instanceof GlideDrawableImageViewTarget);
+    }
+
+    @Test
+    public void testReturnsTargetForGlideBitmapDrawables() {
+        GlideBitmapDrawable drawable = mock(GlideBitmapDrawable.class);
+        Target target = factory.buildTarget(view, GlideBitmapDrawable.class);
+        target.onResourceReady(drawable, null);
+        assertTrue(target instanceof GlideDrawableImageViewTarget);
     }
 
     @Test
     public void testReturnsTargetForBitmapDrawables() {
-        assertNotNull(factory.buildTarget(view, BitmapDrawable.class));
+        BitmapDrawable drawable = new BitmapDrawable(Robolectric.application.getResources(),
+                Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_4444));
+
+        Target target = factory.buildTarget(view, BitmapDrawable.class);
+        target.onResourceReady(drawable, null);
+        assertTrue(target instanceof DrawableImageViewTarget);
     }
 
     @Test
     public void testReturnsTargetForDrawables() {
-        assertNotNull(factory.buildTarget(view, Drawable.class));
+        Target<Drawable> target = factory.buildTarget(view, Drawable.class);
+        target.onResourceReady(new ColorDrawable(Color.RED), null);
+        assertTrue(target instanceof DrawableImageViewTarget);
     }
 
     @Test(expected = IllegalArgumentException.class)

library/src/main/java/com/bumptech/glide/request/target/BitmapImageViewTarget.java

@@ -7,7 +7,7 @@ import android.widget.ImageView;
  * A {@link com.bumptech.glide.request.target.Target} that can display an {@link android.graphics.Bitmap} in an
  * {@link android.widget.ImageView}.
  *
- * @see com.bumptech.glide.request.target.DrawableImageViewTarget
+ * @see GlideDrawableImageViewTarget
  */
 public class BitmapImageViewTarget extends ImageViewTarget<Bitmap> {
     private final ImageView view;

library/src/main/java/com/bumptech/glide/request/target/DrawableImageViewTarget.java

 package com.bumptech.glide.request.target;
 
+import android.graphics.drawable.Drawable;
 import android.widget.ImageView;
 
-import com.bumptech.glide.load.resource.drawable.GlideDrawable;
-import com.bumptech.glide.request.animation.GlideAnimation;
-
 /**
- * A {@link com.bumptech.glide.request.target.Target} that can display an {@link android.graphics.drawable.Drawable} in
- * an {@link android.widget.ImageView}.
+ * A target for display {@link Drawable} objects in {@link ImageView}s.
  */
-public class DrawableImageViewTarget extends ImageViewTarget<GlideDrawable> {
-    private static final float SQUARE_RATIO_MARGIN = 0.05f;
-    private final ImageView view;
-    private int maxLoopCount;
-    private GlideDrawable resource;
-
-    /**
-     * Constructor for an {@link com.bumptech.glide.request.target.Target} that can display an
-     * {@link com.bumptech.glide.load.resource.drawable.GlideDrawable} in an {@link android.widget.ImageView}.
-     *
-     * @param view The view to display the drawable in.
-     */
+public class DrawableImageViewTarget extends ImageViewTarget<Drawable> {
     public DrawableImageViewTarget(ImageView view) {
-        this(view, GlideDrawable.LOOP_FOREVER);
-    }
-
-    /**
-     * Constructor for an {@link com.bumptech.glide.request.target.Target} that can display an
-     * {@link com.bumptech.glide.load.resource.drawable.GlideDrawable} in an {@link android.widget.ImageView}.
-     *
-     * @param view The view to display the drawable in.
-     * @param maxLoopCount A value to pass to to {@link com.bumptech.glide.load.resource.drawable.GlideDrawable}s
-     *                     indicating how many times they should repeat their animation (if they have one). See
-     *                     {@link com.bumptech.glide.load.resource.drawable.GlideDrawable#setLoopCount(int)}.
-     */
-    public DrawableImageViewTarget(ImageView view, int maxLoopCount) {
         super(view);
-        this.view = view;
-        this.maxLoopCount = maxLoopCount;
-    }
-
-    /**
-     * {@inheritDoc}
-     * If no {@link com.bumptech.glide.request.animation.GlideAnimation} is given or if the animation does not set the
-     * {@link android.graphics.drawable.Drawable} on the view, the drawable is set using
-     * {@link android.widget.ImageView#setImageDrawable(android.graphics.drawable.Drawable)}.
-     *
-     * @param resource {@inheritDoc}
-     * @param animation {@inheritDoc}
-     */
-    @Override
-    public void onResourceReady(GlideDrawable resource, GlideAnimation<? super GlideDrawable> animation) {
-        if (!resource.isAnimated()) {
-            //TODO: Try to generalize this to other sizes/shapes.
-            // This is a dirty hack that tries to make loading square thumbnails and then square full images less costly
-            // by forcing both the smaller thumb and the larger version to have exactly the same intrinsic dimensions.
-            // If a drawable is replaced in an ImageView by another drawable with different intrinsic dimensions,
-            // the ImageView requests a layout. Scrolling rapidly while replacing thumbs with larger images triggers
-            // lots of these calls and causes significant amounts of jank.
-            float viewRatio = view.getWidth() / (float) view.getHeight();
-            float drawableRatio = resource.getIntrinsicWidth() / (float) resource.getIntrinsicHeight();
-            if (Math.abs(viewRatio - 1f) <= SQUARE_RATIO_MARGIN
-                    && Math.abs(drawableRatio - 1f) <= SQUARE_RATIO_MARGIN) {
-                resource = new SquaringDrawable(resource, view.getWidth());
-            }
-        }
-        super.onResourceReady(resource, animation);
-        this.resource = resource;
-        resource.setLoopCount(maxLoopCount);
-        resource.start();
-    }
-
-    /**
-     * Sets the drawable on the view using
-     * {@link android.widget.ImageView#setImageDrawable(android.graphics.drawable.Drawable)}.
-     *
-     * @param resource The {@link android.graphics.drawable.Drawable} to display in the view.
-     */
-    @Override
-    protected void setResource(GlideDrawable resource) {
-        view.setImageDrawable(resource);
-    }
-
-    @Override
-    public void onStart() {
-        if (resource != null) {
-            resource.start();
-        }
     }
 
     @Override
-    public void onStop() {
-        if (resource != null) {
-            resource.stop();
-        }
+    protected void setResource(Drawable resource) {
+       view.setImageDrawable(resource);
     }
 }

library/src/main/java/com/bumptech/glide/request/target/GlideDrawableImageViewTarget.java

+package com.bumptech.glide.request.target;
+
+import android.widget.ImageView;
+
+import com.bumptech.glide.load.resource.drawable.GlideDrawable;
+import com.bumptech.glide.request.animation.GlideAnimation;
+
+/**
+ * A {@link com.bumptech.glide.request.target.Target} that can display an {@link android.graphics.drawable.Drawable} in
+ * an {@link android.widget.ImageView}.
+ */
+public class GlideDrawableImageViewTarget extends ImageViewTarget<GlideDrawable> {
+    private static final float SQUARE_RATIO_MARGIN = 0.05f;
+    private final ImageView view;
+    private int maxLoopCount;
+    private GlideDrawable resource;
+
+    /**
+     * Constructor for an {@link com.bumptech.glide.request.target.Target} that can display an
+     * {@link com.bumptech.glide.load.resource.drawable.GlideDrawable} in an {@link android.widget.ImageView}.
+     *
+     * @param view The view to display the drawable in.
+     */
+    public GlideDrawableImageViewTarget(ImageView view) {
+        this(view, GlideDrawable.LOOP_FOREVER);
+    }
+
+    /**
+     * Constructor for an {@link com.bumptech.glide.request.target.Target} that can display an
+     * {@link com.bumptech.glide.load.resource.drawable.GlideDrawable} in an {@link android.widget.ImageView}.
+     *
+     * @param view The view to display the drawable in.
+     * @param maxLoopCount A value to pass to to {@link com.bumptech.glide.load.resource.drawable.GlideDrawable}s
+     *                     indicating how many times they should repeat their animation (if they have one). See
+     *                     {@link com.bumptech.glide.load.resource.drawable.GlideDrawable#setLoopCount(int)}.
+     */
+    public GlideDrawableImageViewTarget(ImageView view, int maxLoopCount) {
+        super(view);
+        this.view = view;
+        this.maxLoopCount = maxLoopCount;
+    }
+
+    /**
+     * {@inheritDoc}
+     * If no {@link com.bumptech.glide.request.animation.GlideAnimation} is given or if the animation does not set the
+     * {@link android.graphics.drawable.Drawable} on the view, the drawable is set using
+     * {@link android.widget.ImageView#setImageDrawable(android.graphics.drawable.Drawable)}.
+     *
+     * @param resource {@inheritDoc}
+     * @param animation {@inheritDoc}
+     */
+    @Override
+    public void onResourceReady(GlideDrawable resource, GlideAnimation<? super GlideDrawable> animation) {
+        if (!resource.isAnimated()) {
+            //TODO: Try to generalize this to other sizes/shapes.
+            // This is a dirty hack that tries to make loading square thumbnails and then square full images less costly
+            // by forcing both the smaller thumb and the larger version to have exactly the same intrinsic dimensions.
+            // If a drawable is replaced in an ImageView by another drawable with different intrinsic dimensions,
+            // the ImageView requests a layout. Scrolling rapidly while replacing thumbs with larger images triggers
+            // lots of these calls and causes significant amounts of jank.
+            float viewRatio = view.getWidth() / (float) view.getHeight();
+            float drawableRatio = resource.getIntrinsicWidth() / (float) resource.getIntrinsicHeight();
+            if (Math.abs(viewRatio - 1f) <= SQUARE_RATIO_MARGIN
+                    && Math.abs(drawableRatio - 1f) <= SQUARE_RATIO_MARGIN) {
+                resource = new SquaringDrawable(resource, view.getWidth());
+            }
+        }
+        super.onResourceReady(resource, animation);
+        this.resource = resource;
+        resource.setLoopCount(maxLoopCount);
+        resource.start();
+    }
+
+    /**
+     * Sets the drawable on the view using
+     * {@link android.widget.ImageView#setImageDrawable(android.graphics.drawable.Drawable)}.
+     *
+     * @param resource The {@link android.graphics.drawable.Drawable} to display in the view.
+     */
+    @Override
+    protected void setResource(GlideDrawable resource) {
+        view.setImageDrawable(resource);
+    }
+
+    @Override
+    public void onStart() {
+        if (resource != null) {
+            resource.start();
+        }
+    }
+
+    @Override
+    public void onStop() {
+        if (resource != null) {
+            resource.stop();
+        }
+    }
+}

library/src/main/java/com/bumptech/glide/request/target/ImageViewTargetFactory.java

@@ -3,6 +3,9 @@ package com.bumptech.glide.request.target;
 import android.graphics.Bitmap;
 import android.graphics.drawable.Drawable;
 import android.widget.ImageView;
+import com.bumptech.glide.load.resource.bitmap.GlideBitmapDrawable;
+import com.bumptech.glide.load.resource.drawable.GlideDrawable;
+import com.bumptech.glide.load.resource.gif.GifDrawable;
 
 /**
  * A factory responsible for producing the correct type of {@link com.bumptech.glide.request.target.Target} for a given
@@ -12,7 +15,11 @@ public class ImageViewTargetFactory {
 
     @SuppressWarnings("unchecked")
     public <Z> Target<Z> buildTarget(ImageView view, Class<Z> clazz) {
-        if (Bitmap.class.equals(clazz)) {
+        if (GlideDrawable.class.equals(clazz)
+                || GlideBitmapDrawable.class.equals(clazz)
+                || GifDrawable.class.equals(clazz)) {
+            return (Target<Z>) new GlideDrawableImageViewTarget(view);
+        } else if (Bitmap.class.equals(clazz)) {
             return (Target<Z>) new BitmapImageViewTarget(view);
         } else if (Drawable.class.isAssignableFrom(clazz)) {
             return (Target<Z>) new DrawableImageViewTarget(view);