Add a Size + Config Strategy.

Fixes bumptech#335. Allows us to re-use Bitmaps with configs other than ARGB_8888. Differs from AttributeStrategy in that it sorts Bitmaps by config only, and allows Bitmaps with a given config to be reconfigured to change their dimensions. In addition, we allow switching configs between the hidden GIF config and ARGB_8888 which appears not to trigger bumptech#301.
sjudd · Feb 9, 2015 · 431ccaf · 431ccaf
1 parent 7ab34f3
commit 431ccaf
Show file tree

Hide file tree

Showing 13 changed files with 326 additions and 50 deletions.
diff --git a/...ndroidTest/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeConfigStrategyTest.java b/...ndroidTest/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeConfigStrategyTest.java
@@ -0,0 +1,40 @@
+package com.bumptech.glide.load.engine.bitmap_recycle;
+
+import com.google.common.testing.EqualsTester;
+
+import android.graphics.Bitmap;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+public class SizeConfigStrategyTest {
+
+    @Mock SizeConfigStrategy.KeyPool pool;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+    }
+
+    @Test
+    public void testKeyEquals() {
+        new EqualsTester()
+                .addEqualityGroup(
+                        new SizeConfigStrategy.Key(pool, 100, Bitmap.Config.ARGB_8888),
+                        new SizeConfigStrategy.Key(pool, 100, Bitmap.Config.ARGB_8888)
+                )
+                .addEqualityGroup(
+                        new SizeConfigStrategy.Key(pool, 101, Bitmap.Config.ARGB_8888)
+                )
+                .addEqualityGroup(
+                        new SizeConfigStrategy.Key(pool, 100, Bitmap.Config.RGB_565)
+                )
+                .addEqualityGroup(
+                        new SizeConfigStrategy.Key(pool, 100, null /*config*/)
+                )
+                .testEquals();
+
+    }
+}
diff --git a/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerTest.java
@@ -49,7 +49,7 @@ public void testAlwaysArgb8888() throws FileNotFoundException {
         Downsampler downsampler = Downsampler.AT_LEAST;
         InputStream is = new FileInputStream(tempFile);
         try {
-            Bitmap result = downsampler.decode(is, mock(BitmapPool.class), 100, 100, DecodeFormat.ALWAYS_ARGB_8888);
+            Bitmap result = downsampler.decode(is, mock(BitmapPool.class), 100, 100, DecodeFormat.PREFER_ARGB_8888);
             assertEquals(Bitmap.Config.ARGB_8888, result.getConfig());
         } finally {
             try {

diff --git a/...src/androidTest/java/com/bumptech/glide/load/resource/bitmap/StreamBitmapDecoderTest.java b/...src/androidTest/java/com/bumptech/glide/load/resource/bitmap/StreamBitmapDecoderTest.java
@@ -61,7 +61,7 @@ public void testHasValidId() {
     private static class DecoderHarness {
         Downsampler downsampler = mock(Downsampler.class);
         BitmapPool bitmapPool = mock(BitmapPool.class);
-        DecodeFormat decodeFormat = DecodeFormat.ALWAYS_ARGB_8888;
+        DecodeFormat decodeFormat = DecodeFormat.PREFER_ARGB_8888;
         InputStream source = new ByteArrayInputStream(new byte[0]);
         int width = 100;
         int height = 100;

diff --git a/.../src/androidTest/java/com/bumptech/glide/load/resource/bitmap/VideoBitmapDecoderTest.java b/.../src/androidTest/java/com/bumptech/glide/load/resource/bitmap/VideoBitmapDecoderTest.java
@@ -38,7 +38,7 @@ public class VideoBitmapDecoderTest {
     @Before
     public void setup() {
         bitmapPool = mock(BitmapPool.class);
-        decodeFormat = DecodeFormat.ALWAYS_ARGB_8888;
+        decodeFormat = DecodeFormat.PREFER_ARGB_8888;
         resource = mock(ParcelFileDescriptor.class);
         factory = mock(VideoBitmapDecoder.MediaMetadataRetrieverFactory.class);
         retriever = mock(MediaMetadataRetriever.class);

diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java
@@ -1,9 +1,7 @@
 package com.bumptech.glide;
 
 import android.content.Context;
-import android.graphics.Bitmap;
 import android.os.Build;
-import android.util.Log;
 
 import com.bumptech.glide.load.DecodeFormat;
 import com.bumptech.glide.load.engine.Engine;
@@ -17,14 +15,12 @@
 import com.bumptech.glide.load.engine.cache.MemorySizeCalculator;
 import com.bumptech.glide.load.engine.executor.FifoPriorityThreadPoolExecutor;
 
-import java.util.Collections;
 import java.util.concurrent.ExecutorService;
 
 /**
  * A builder class for setting default structural classes for Glide to use.
  */
 public class GlideBuilder {
-    private static final String TAG = "Glide";
     private final Context context;
 
     private Engine engine;
@@ -159,14 +155,7 @@ public GlideBuilder setDiskCacheService(ExecutorService service) {
      * @return This builder.
      */
     public GlideBuilder setDecodeFormat(DecodeFormat decodeFormat) {
-        if (DecodeFormat.REQUIRE_ARGB_8888 && decodeFormat != DecodeFormat.ALWAYS_ARGB_8888) {
-            this.decodeFormat = DecodeFormat.ALWAYS_ARGB_8888;
-            if (Log.isLoggable(TAG, Log.WARN)) {
-                Log.w(TAG, "Unsafe to use RGB_565 on KitKat or Lollipop, ignoring setDecodeFormat");
-            }
-        } else {
-            this.decodeFormat = decodeFormat;
-        }
+        this.decodeFormat = decodeFormat;
         return this;
     }
 
@@ -189,11 +178,7 @@ Glide createGlide() {
         if (bitmapPool == null) {
             if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB) {
                 int size = calculator.getBitmapPoolSize();
-                if (DecodeFormat.REQUIRE_ARGB_8888) {
-                    bitmapPool = new LruBitmapPool(size, Collections.singleton(Bitmap.Config.ARGB_8888));
-                } else {
-                    bitmapPool = new LruBitmapPool(size);
-                }
+                bitmapPool = new LruBitmapPool(size);
             } else {
                 bitmapPool = new BitmapPoolAdapter();
             }

diff --git a/library/src/main/java/com/bumptech/glide/load/DecodeFormat.java b/library/src/main/java/com/bumptech/glide/load/DecodeFormat.java
@@ -1,7 +1,5 @@
 package com.bumptech.glide.load;
 
-import android.os.Build;
-
 /**
  * Options for setting the value of {@link android.graphics.Bitmap#getConfig()} for {@link android.graphics.Bitmap}s
  * returned by a {@link com.bumptech.glide.load.resource.bitmap.BitmapDecoder}.
@@ -17,9 +15,26 @@ public enum DecodeFormat {
     /**
      * All bitmaps returned by the {@link com.bumptech.glide.load.resource.bitmap.BitmapDecoder} should return
      * {@link android.graphics.Bitmap.Config#ARGB_8888} for {@link android.graphics.Bitmap#getConfig()}.
+     *
+     * @deprecated Use the equivalent but less misleadingly named {@link #PREFER_ARGB_8888}. Scheduled to be removed
+     * in Glide 4.0
      */
+    @Deprecated
     ALWAYS_ARGB_8888,
 
+    /**
+     * Bitmaps decoded from most image formats (other than GIFs with hidden configs), will be decoded with the
+     * ARGB_8888 config.
+     *
+     * <p>
+     *     {@link android.graphics.BitmapFactory} does not allow us to guarantee that all returned Bitmaps will
+     *     be of a requested config without resorting to expensive copying. As a result, this is a preference only.
+     *     Most GIFs, for example, will still produce {@link android.graphics.Bitmap}s with null
+     *     {@link android.graphics.Bitmap.Config}s.
+     * </p>
+     */
+    PREFER_ARGB_8888,
+
     /**
      * Bitmaps decoded from image formats that support and/or use alpha (some types of PNGs, GIFs etc) should
      * return {@link android.graphics.Bitmap.Config#ARGB_8888} for {@link android.graphics.Bitmap#getConfig()}. Bitmaps
@@ -29,10 +44,6 @@ public enum DecodeFormat {
      */
     PREFER_RGB_565;
 
-
-    /** There is a rendering issue in KitKat and L (or at least L MR1) when reusing mixed format bitmaps. See #301. */
-    public static final boolean REQUIRE_ARGB_8888 = Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT;
-
     /** The default value for DecodeFormat. */
-    public static final DecodeFormat DEFAULT = REQUIRE_ARGB_8888 ? ALWAYS_ARGB_8888 : PREFER_RGB_565;
+    public static final DecodeFormat DEFAULT = PREFER_RGB_565;
 }
diff --git a/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/LruBitmapPool.java b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/LruBitmapPool.java
@@ -213,7 +213,7 @@ private void dumpUnchecked() {
     private static LruPoolStrategy getDefaultStrategy() {
         final LruPoolStrategy strategy;
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
-            strategy = new SizeStrategy();
+            strategy = new SizeConfigStrategy();
         } else {
             strategy = new AttributeStrategy();
         }

diff --git a/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/PrettyPrintTreeMap.java b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/PrettyPrintTreeMap.java
@@ -0,0 +1,18 @@
+package com.bumptech.glide.load.engine.bitmap_recycle;
+
+import java.util.TreeMap;
+
+class PrettyPrintTreeMap<K, V> extends TreeMap<K, V> {
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append("( ");
+        for (Entry<K, V> entry : entrySet()) {
+            sb.append('{').append(entry.getKey()).append(':').append(entry.getValue()).append("}, ");
+        }
+        if (!isEmpty()) {
+            sb.replace(sb.length() - 2, sb.length(), "");
+        }
+        return sb.append(" )").toString();
+    }
+}