Code cleanup

library/src/main/java/com/bumptech/glide/load/resource/bitmap/StreamBitmapDecoder.java

 package com.bumptech.glide.load.resource.bitmap;
 
+import android.content.Context;
 import android.graphics.Bitmap;
+import com.bumptech.glide.Glide;
 import com.bumptech.glide.Resource;
+import com.bumptech.glide.load.DecodeFormat;
 import com.bumptech.glide.load.ResourceDecoder;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
-import com.bumptech.glide.load.DecodeFormat;
 
 import java.io.InputStream;
 
@@ -16,6 +18,10 @@ public class StreamBitmapDecoder implements ResourceDecoder<InputStream, Bitmap>
     private DecodeFormat decodeFormat;
     private String id;
 
+    public StreamBitmapDecoder(Context context) {
+        this(Glide.get(context).getBitmapPool());
+    }
+
     public StreamBitmapDecoder(BitmapPool bitmapPool) {
         this(Downsampler.AT_LEAST, bitmapPool, DecodeFormat.PREFER_RGB_565);
     }

library/src/main/java/com/bumptech/glide/load/resource/gif/GifData.java

@@ -47,7 +47,7 @@ public class GifData {
     public GifDrawable getDrawable() {
         GifDecoder gifDecoder = new GifDecoder(bitmapPool);
         gifDecoder.setData(gifId, header, data);
-        GifFrameManager frameManager = new GifFrameManager(context, getFrameTransformation());
+        GifFrameManager frameManager = new GifFrameManager(context, gifDecoder, getFrameTransformation());
 
         GifDrawable result = new GifDrawable(gifDecoder, frameManager);
         drawables.add(result);

library/src/main/java/com/bumptech/glide/load/resource/gif/GifResource.java → library/src/main/java/com/bumptech/glide/load/resource/gif/GifDataResource.java

@@ -2,10 +2,10 @@ package com.bumptech.glide.load.resource.gif;
 
 import com.bumptech.glide.Resource;
 
-public class GifResource extends Resource<GifData> {
+public class GifDataResource extends Resource<GifData> {
     private GifData gifData;
 
-    public GifResource(GifData gifData) {
+    public GifDataResource(GifData gifData) {
         this.gifData = gifData;
     }
 

library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java

@@ -29,7 +29,7 @@ public class GifDrawable extends Drawable implements Animatable, GifFrameManager
     public void start() {
         if (!isRunning) {
             isRunning = true;
-            frameManager.getNextFrame(decoder, this);
+            frameManager.getNextFrame(this);
             invalidateSelf();
         }
     }
@@ -102,7 +102,7 @@ public class GifDrawable extends Drawable implements Animatable, GifFrameManager
             invalidateSelf();
         }
 
-        frameManager.getNextFrame(decoder, this);
+        frameManager.getNextFrame(this);
     }
 
     public void recycle() {

library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameManager.java

@@ -6,21 +6,32 @@ import android.os.Handler;
 import android.os.Looper;
 import android.os.SystemClock;
 import com.bumptech.glide.Glide;
+import com.bumptech.glide.load.ResourceDecoder;
+import com.bumptech.glide.load.ResourceEncoder;
+import com.bumptech.glide.load.SkipCache;
 import com.bumptech.glide.load.Transformation;
 import com.bumptech.glide.load.engine.cache.MemorySizeCalculator;
 import com.bumptech.glide.load.resource.NullCacheDecoder;
+import com.bumptech.glide.load.resource.bitmap.BitmapEncoder;
+import com.bumptech.glide.load.resource.bitmap.StreamBitmapDecoder;
 import com.bumptech.glide.load.resource.gif.decoder.GifDecoder;
 import com.bumptech.glide.request.target.SimpleTarget;
 
+import java.io.InputStream;
+
 class GifFrameManager {
+    // 16ms per frame = 60fps
     static final long MIN_FRAME_DELAY = 16;
     private final MemorySizeCalculator calculator;
-    private final GifFrameLoader frameLoader;
+    private final GifFrameModelLoader frameLoader;
     private final GifFrameResourceDecoder frameResourceDecoder;
-    private final NullCacheDecoder<Bitmap> cacheDecoder;
+    private final ResourceDecoder<InputStream, Bitmap> cacheDecoder;
+    private final GifDecoder decoder;
     private final Handler mainHandler;
+    private final ResourceEncoder<Bitmap> encoder;
+    private final Context context;
+
     private Transformation<Bitmap> transformation;
-    private Context context;
     private DelayTarget current;
     private DelayTarget next;
 
@@ -28,31 +39,46 @@ class GifFrameManager {
         public void onFrameRead(Bitmap frame);
     }
 
-    public GifFrameManager(Context context, Transformation<Bitmap> transformation) {
-        this(context, new Handler(Looper.getMainLooper()), transformation);
+    public GifFrameManager(Context context, GifDecoder decoder, Transformation<Bitmap> transformation) {
+        this(context, decoder, new Handler(Looper.getMainLooper()), transformation);
     }
 
-    public GifFrameManager(Context context, Handler mainHandler,
+    public GifFrameManager(Context context, GifDecoder decoder, Handler mainHandler,
             Transformation<Bitmap> transformation) {
         this.context = context;
+        this.decoder = decoder;
         this.mainHandler = mainHandler;
         this.transformation = transformation;
         calculator = new MemorySizeCalculator(context);
-        frameLoader = new GifFrameLoader();
+        frameLoader = new GifFrameModelLoader();
         frameResourceDecoder = new GifFrameResourceDecoder();
-        cacheDecoder = NullCacheDecoder.get();
+
+        if (!decoder.isTransparent()) {
+            // For non transparent gifs, we can beat the performance of our gif decoder for each frame by decoding jpegs
+            // from disk.
+            cacheDecoder = new StreamBitmapDecoder(context);
+            encoder = new BitmapEncoder(Bitmap.CompressFormat.JPEG, 70);
+        } else {
+            // For transparent gifs, we would have to encode as pngs which is actually slower than our gif decoder so we
+            // avoid writing frames to the disk cache entirely.
+            cacheDecoder = NullCacheDecoder.get();
+            encoder = SkipCache.get();
+        }
     }
 
     Transformation<Bitmap> getTransformation() {
         return transformation;
     }
 
-    public void getNextFrame(final GifDecoder decoder, FrameCallback cb) {
+    public void getNextFrame(FrameCallback cb) {
         decoder.advance();
-        boolean skipCache = decoder.getDecodedFrameByteSize() > calculator.getMemoryCacheSize() / 2;
+        // We don't want to blow out the entire memory cache with frames of gifs, so try to set some
+        // maximum size beyond which we will always just decode one frame at a time.
+        boolean skipCache = decoder.getDecodedFramesByteSizeSum() > calculator.getMemoryCacheSize() / 2;
 
         long targetTime = SystemClock.uptimeMillis() + (Math.min(MIN_FRAME_DELAY, decoder.getNextDelay()));
         next = new DelayTarget(decoder, cb, targetTime, mainHandler);
+
         Glide.with(context)
                 .using(frameLoader, GifDecoder.class)
                 .load(decoder)
@@ -60,8 +86,8 @@ class GifFrameManager {
                 .decoder(frameResourceDecoder)
                 .cacheDecoder(cacheDecoder)
                 .transform(transformation)
+                .encoder(encoder)
                 .skipMemoryCache(skipCache)
-                .skipDiskCache(true)
                 .into(next);
     }
 

library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java → library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameModelLoader.java

@@ -5,7 +5,7 @@ import com.bumptech.glide.load.data.DataFetcher;
 import com.bumptech.glide.load.model.ModelLoader;
 import com.bumptech.glide.load.resource.gif.decoder.GifDecoder;
 
-public class GifFrameLoader implements ModelLoader<GifDecoder, GifDecoder> {
+public class GifFrameModelLoader implements ModelLoader<GifDecoder, GifDecoder> {
 
     @Override
     public DataFetcher<GifDecoder> getResourceFetcher(GifDecoder model, int width, int height) {

library/src/main/java/com/bumptech/glide/load/resource/gif/GifResourceDecoder.java

@@ -7,6 +7,7 @@ import com.bumptech.glide.load.ResourceDecoder;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
 import com.bumptech.glide.load.resource.gif.decoder.GifHeader;
 import com.bumptech.glide.load.resource.gif.decoder.GifHeaderParser;
+import com.bumptech.glide.util.Util;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -30,11 +31,11 @@ public class GifResourceDecoder implements ResourceDecoder<InputStream, GifData>
     }
 
     @Override
-    public GifResource decode(InputStream source, int width, int height) throws IOException {
+    public GifDataResource decode(InputStream source, int width, int height) throws IOException {
         byte[] data = inputStreamToBytes(source);
         GifHeader header = new GifHeaderParser(data).parseHeader();
         String id = getGifId(data);
-        return new GifResource(new GifData(context, bitmapPool, id, header, data));
+        return new GifDataResource(new GifData(context, bitmapPool, id, header, data));
     }
 
     @Override
@@ -46,7 +47,7 @@ public class GifResourceDecoder implements ResourceDecoder<InputStream, GifData>
         try {
             MessageDigest digest = MessageDigest.getInstance("SHA-1");
             digest.update(data);
-            return new String(digest.digest());
+            return Util.sha256BytesToHex(digest.digest());
         } catch (NoSuchAlgorithmException e) {
             if (Log.isLoggable(TAG, Log.WARN)) {
                 Log.w(TAG, "Missing sha1 algorithm?", e);

library/src/main/java/com/bumptech/glide/load/resource/gif/decoder/GifDecoder.java

@@ -137,7 +137,7 @@ public class GifDecoder {
         return data;
     }
 
-    public int getDecodedFrameByteSize() {
+    public int getDecodedFramesByteSizeSum() {
         // 4 == ARGB_8888, 2 == RGB_565
         return header.frameCount * header.width * header.height * (header.isTransparent ? 4 : 2);
     }

library/src/main/java/com/bumptech/glide/load/resource/gifbitmap/GifBitmapWrapperTransformation.java

@@ -6,7 +6,7 @@ import com.bumptech.glide.Resource;
 import com.bumptech.glide.load.MultiTransformation;
 import com.bumptech.glide.load.Transformation;
 import com.bumptech.glide.load.resource.gif.GifData;
-import com.bumptech.glide.load.resource.gif.GifResource;
+import com.bumptech.glide.load.resource.gif.GifDataResource;
 
 public class GifBitmapWrapperTransformation implements Transformation<GifBitmapWrapper> {
     private Context context;
@@ -33,7 +33,7 @@ public class GifBitmapWrapperTransformation implements Transformation<GifBitmapW
             Transformation<Bitmap> newTransformation =
                     new MultiTransformation<Bitmap>(gifData.getFrameTransformation(), wrapped);
             gifData.setFrameTransformation(newTransformation);
-            return new GifBitmapWrapperResource(new GifBitmapWrapper(null, new GifResource(gifData)));
+            return new GifBitmapWrapperResource(new GifBitmapWrapper(null, new GifDataResource(gifData)));
         }
         return resource;
     }

library/src/test/java/com/bumptech/glide/GlideTest.java

@@ -495,10 +495,6 @@ public class GlideTest {
         Glide.with(getContext()).load(0.5f).into(target);
     }
 
-    @Test
-    public void testSomething() {
-    }
-
     @Test
     public void testNullModelDoesNotThrow() {
         String nullString = null;

library/src/test/java/com/bumptech/glide/load/resource/gif/GifResourceTest.java → library/src/test/java/com/bumptech/glide/load/resource/gif/GifDataResourceTest.java

@@ -12,14 +12,14 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @RunWith(RobolectricTestRunner.class)
-public class GifResourceTest {
-    private GifResource resource;
+public class GifDataResourceTest {
+    private GifDataResource resource;
     private GifData gifData;
 
     @Before
     public void setUp() {
         gifData = mock(GifData.class);
-        resource = new GifResource(gifData);
+        resource = new GifDataResource(gifData);
     }
 
     @Test

library/src/test/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java

@@ -66,7 +66,7 @@ public class GifDrawableTest {
     public void testRequestsNextFrameOnStart() {
         drawable.start();
 
-        verify(frameManager).getNextFrame(eq(gifDecoder), eq(drawable));
+        verify(frameManager).getNextFrame(eq(drawable));
     }
 
     @Test
@@ -81,7 +81,7 @@ public class GifDrawableTest {
         drawable.start();
         drawable.start();
 
-        verify(frameManager, times(1)).getNextFrame(eq(gifDecoder), eq(drawable));
+        verify(frameManager, times(1)).getNextFrame(eq(drawable));
     }
 
     @Test
@@ -101,7 +101,7 @@ public class GifDrawableTest {
         drawable.setIsRunning(true);
         drawable.onFrameRead(Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888));
 
-        verify(frameManager).getNextFrame(eq(gifDecoder), eq(drawable));
+        verify(frameManager).getNextFrame(eq(drawable));
     }
 
     @Test
@@ -117,7 +117,7 @@ public class GifDrawableTest {
         drawable.setIsRunning(false);
         drawable.onFrameRead(Bitmap.createBitmap(10, 100, Bitmap.Config.ARGB_8888));
 
-        verify(frameManager, never()).getNextFrame(eq(gifDecoder), eq(drawable));
+        verify(frameManager, never()).getNextFrame(eq(drawable));
     }
 
     @Test

library/src/test/java/com/bumptech/glide/load/resource/gif/GifFrameModelLoaderTest.java

+package com.bumptech.glide.load.resource.gif;
+
+import com.bumptech.glide.Priority;
+import com.bumptech.glide.load.resource.gif.decoder.GifDecoder;
+import org.junit.Before;
+import org.junit.Test;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class GifFrameModelLoaderTest {
+    private GifFrameModelLoader loader;
+    private GifDecoder decoder;
+
+    @Before
+    public void setUp() {
+        loader = new GifFrameModelLoader();
+        decoder = mock(GifDecoder.class);
+    }
+
+    @Test
+    public void testIdIncludesGifDecoderIdAndFrameIndex() {
+        String id = "asdfasd";
+        int frameIndex = 124;
+        when(decoder.getId()).thenReturn(id);
+        when(decoder.getCurrentFrameIndex()).thenReturn(frameIndex);
+
+        String loaderId = loader.getId(decoder);
+
+        assertTrue(loaderId.contains(id));
+        assertTrue(loaderId.contains(String.valueOf(frameIndex)));
+    }
+
+    @Test
+    public void testAlwaysReturnsGivenDecoderFromFetcher() throws Exception {
+        assertEquals(decoder, loader.getResourceFetcher(decoder, 100, 100).loadData(Priority.NORMAL));
+    }
+}

samples/flickr/res/layout/flickr_search_activity.xml

@@ -4,11 +4,6 @@
     android:orientation="vertical"
     android:layout_width="fill_parent"
     android:layout_height="fill_parent">
-    <ImageView
-      android:id="@+id/test"
-      android:background="@android:color/holo_red_light"
-      android:layout_width="fill_parent"
-      android:layout_height="fill_parent" />
     <LinearLayout
       android:layout_width="fill_parent"
       android:layout_height="wrap_content"

samples/flickr/src/main/java/com/bumptech/glide/samples/flickr/FlickrSearchActivity.java

@@ -13,7 +13,6 @@ import android.view.View;
 import android.view.inputmethod.EditorInfo;
 import android.widget.Button;
 import android.widget.EditText;
-import android.widget.ImageView;
 import android.widget.TextView;
 import com.actionbarsherlock.app.ActionBar;
 import com.actionbarsherlock.app.SherlockFragmentActivity;
@@ -76,19 +75,6 @@ public class FlickrSearchActivity extends SherlockFragmentActivity {
                 .penaltyLog()
                 .build());
 
-        ImageView testView = (ImageView) findViewById(R.id.test);
-//        GifResourceDecoder decoder = new GifResourceDecoder(this);
-//        Glide.with(this)
-//                .using(Glide.buildStreamModelLoader(Integer.class, this), InputStream.class)
-//                .load(R.raw.cat)
-//                .as(GifData.class)
-//                .transcode(new GifDataDrawableTranscoder(), GifDrawable.class)
-//                .decoder(decoder)
-//                .cacheDecoder(decoder)
-//                .encoder(new GifResourceEncoder())
-//                .into(testView);
-        testView.setVisibility(View.GONE);
-
         Glide.get(this).register(Photo.class, InputStream.class, new FlickrModelLoader.Factory());
 
         searching = findViewById(R.id.searching);