diff --git a/library/src/androidTest/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeConfigStrategyTest.java b/library/src/androidTest/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeConfigStrategyTest.java new file mode 100644 index 0000000000..23da6135ff --- /dev/null +++ b/library/src/androidTest/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(); + + } +} \ No newline at end of file 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 index 3347531ab5..2ec1dc152e 100644 --- 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/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/StreamBitmapDecoderTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/StreamBitmapDecoderTest.java index a6a9d4d3ac..7f2dfb5ade 100644 --- a/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/StreamBitmapDecoderTest.java +++ b/library/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/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/VideoBitmapDecoderTest.java b/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/VideoBitmapDecoderTest.java index 744b0e9b2b..edb54aeb90 100644 --- a/library/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/VideoBitmapDecoderTest.java +++ b/library/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 index 3208a13448..31fcc98f43 100644 --- 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 index b6bea2f639..ac28a63530 100644 --- 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. + * + *

+ * {@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. + *

+ */ + 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 index a8b7a751c2..0a00f53004 100644 --- 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 new file mode 100644 index 0000000000..c12e4e34eb --- /dev/null +++ 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 extends TreeMap { + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("( "); + for (Entry 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(); + } +} diff --git a/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeConfigStrategy.java b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeConfigStrategy.java new file mode 100644 index 0000000000..a8d5fce88d --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeConfigStrategy.java @@ -0,0 +1,238 @@ +package com.bumptech.glide.load.engine.bitmap_recycle; + +import android.annotation.TargetApi; +import android.graphics.Bitmap; +import android.os.Build; + +import com.bumptech.glide.util.Util; + +import java.util.HashMap; +import java.util.Map; +import java.util.NavigableMap; +import java.util.TreeMap; + +/** + * Keys {@link android.graphics.Bitmap Bitmaps} using both {@link android.graphics.Bitmap#getAllocationByteCount()} and + * the {@link android.graphics.Bitmap.Config} returned from {@link android.graphics.Bitmap#getConfig()}. + * + *

+ * Using both the config and the byte size allows us to safely re-use a greater variety of + * {@link android.graphics.Bitmap Bitmaps}, which increases the hit rate of the pool and therefore the performance + * of applications. This class works around #301 by only allowing re-use of {@link android.graphics.Bitmap Bitmaps} + * with a matching number of bytes per pixel. + *

+ */ +@TargetApi(Build.VERSION_CODES.KITKAT) +public class SizeConfigStrategy implements LruPoolStrategy { + private static final int MAX_SIZE_MULTIPLE = 8; + private static final Bitmap.Config[] ARGB_8888_IN_CONFIGS = new Bitmap.Config[] { + Bitmap.Config.ARGB_8888, + // The value returned by Bitmaps with the hidden Bitmap config. + null, + }; + // We probably could allow ARGB_4444 and RGB_565 to decode into each other, but ARGB_4444 is deprecated and we'd + // rather be safe. + private static final Bitmap.Config[] RGB_565_IN_CONFIGS = new Bitmap.Config[] { + Bitmap.Config.RGB_565 + }; + private static final Bitmap.Config[] ARGB_4444_IN_CONFIGS = new Bitmap.Config[] { + Bitmap.Config.ARGB_4444 + }; + private static final Bitmap.Config[] ALPHA_8_IN_CONFIGS = new Bitmap.Config[] { + Bitmap.Config.ALPHA_8 + }; + + private final KeyPool keyPool = new KeyPool(); + private final GroupedLinkedMap groupedMap = new GroupedLinkedMap(); + private final Map> sortedSizes = + new HashMap>(); + + @Override + public void put(Bitmap bitmap) { + int size = Util.getBitmapByteSize(bitmap); + Key key = keyPool.get(size, bitmap.getConfig()); + + groupedMap.put(key, bitmap); + + NavigableMap sizes = getSizesForConfig(bitmap.getConfig()); + Integer current = sizes.get(key.size); + sizes.put(key.size, current == null ? 1 : current + 1); + } + + @Override + public Bitmap get(int width, int height, Bitmap.Config config) { + int size = Util.getBitmapByteSize(width, height, config); + Key targetKey = keyPool.get(size, config); + Key bestKey = findBestKey(targetKey, size, config); + + Bitmap result = groupedMap.get(bestKey); + if (result != null) { + // Decrement must be called before reconfigure. + decrementBitmapOfSize(Util.getBitmapByteSize(result), result.getConfig()); + result.reconfigure(width, height, + result.getConfig() != null ? result.getConfig() : Bitmap.Config.ARGB_8888); + } + return result; + } + + private Key findBestKey(Key key, int size, Bitmap.Config config) { + Key result = key; + for (Bitmap.Config possibleConfig : getInConfigs(config)) { + NavigableMap sizesForPossibleConfig = getSizesForConfig(possibleConfig); + Integer possibleSize = sizesForPossibleConfig.ceilingKey(size); + if (possibleSize != null && possibleSize <= size * MAX_SIZE_MULTIPLE) { + if (possibleSize != size + || (possibleConfig == null ? config != null : !possibleConfig.equals(config))) { + keyPool.offer(key); + result = keyPool.get(possibleSize, possibleConfig); + } + break; + } + } + return result; + } + + @Override + public Bitmap removeLast() { + Bitmap removed = groupedMap.removeLast(); + if (removed != null) { + int removedSize = Util.getBitmapByteSize(removed); + decrementBitmapOfSize(removedSize, removed.getConfig()); + } + return removed; + } + + private void decrementBitmapOfSize(Integer size, Bitmap.Config config) { + NavigableMap sizes = getSizesForConfig(config); + Integer current = sizes.get(size); + if (current == 1) { + sizes.remove(size); + } else { + sizes.put(size, current - 1); + } + } + + private NavigableMap getSizesForConfig(Bitmap.Config config) { + NavigableMap sizes = sortedSizes.get(config); + if (sizes == null) { + sizes = new TreeMap(); + sortedSizes.put(config, sizes); + } + return sizes; + } + + @Override + public String logBitmap(Bitmap bitmap) { + int size = Util.getBitmapByteSize(bitmap); + return getBitmapString(size, bitmap.getConfig()); + } + + @Override + public String logBitmap(int width, int height, Bitmap.Config config) { + int size = Util.getBitmapByteSize(width, height, config); + return getBitmapString(size, config); + } + + @Override + public int getSize(Bitmap bitmap) { + return Util.getBitmapByteSize(bitmap); + } + + @Override + public String toString() { + StringBuilder sb = new StringBuilder() + .append("SizeConfigStrategy{groupedMap=") + .append(groupedMap) + .append(", sortedSizes=("); + for (Map.Entry> entry : sortedSizes.entrySet()) { + sb.append(entry.getKey()).append('[').append(entry.getValue()).append("], "); + } + if (!sortedSizes.isEmpty()) { + sb.replace(sb.length() - 2, sb.length(), ""); + } + return sb.append(")}").toString(); + } + + // Visible for testing. + static class KeyPool extends BaseKeyPool { + + public Key get(int size, Bitmap.Config config) { + Key result = get(); + result.init(size, config); + return result; + } + + @Override + protected Key create() { + return new Key(this); + } + } + + // Visible for testing. + static final class Key implements Poolable { + private final KeyPool pool; + + private int size; + private Bitmap.Config config; + + public Key(KeyPool pool) { + this.pool = pool; + } + + // Visible for testing. + Key(KeyPool pool, int size, Bitmap.Config config) { + this(pool); + init(size, config); + } + + public void init(int size, Bitmap.Config config) { + this.size = size; + this.config = config; + } + + @Override + public void offer() { + pool.offer(this); + } + + @Override + public String toString() { + return getBitmapString(size, config); + } + + @Override + public boolean equals(Object o) { + if (o instanceof Key) { + Key other = (Key) o; + return size == other.size && (config == null ? other.config == null : config.equals(other.config)); + } + return false; + } + + @Override + public int hashCode() { + int result = size; + result = 31 * result + (config != null ? config.hashCode() : 0); + return result; + } + } + + private static String getBitmapString(int size, Bitmap.Config config) { + return "[" + size + "](" + config + ")"; + } + + private static Bitmap.Config[] getInConfigs(Bitmap.Config requested) { + switch (requested) { + case ARGB_8888: + return ARGB_8888_IN_CONFIGS; + case RGB_565: + return RGB_565_IN_CONFIGS; + case ARGB_4444: + return ARGB_4444_IN_CONFIGS; + case ALPHA_8: + return ALPHA_8_IN_CONFIGS; + default: + return new Bitmap.Config[] { requested }; + } + } +} diff --git a/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeStrategy.java b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeStrategy.java index 2716d337dd..fd7a28c5d1 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeStrategy.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/SizeStrategy.java @@ -93,24 +93,6 @@ public String toString() { + " SortedSizes" + sortedSizes; } - private static class PrettyPrintTreeMap extends TreeMap { - @Override - public String toString() { - StringBuilder sb = new StringBuilder(); - sb.append("( "); - for (Entry entry : entrySet()) { - sb.append('{').append(entry.getKey()).append(':').append(entry.getValue()).append("}, "); - } - final String result; - if (!isEmpty()) { - result = sb.substring(0, sb.length() - 2); - } else { - result = sb.toString(); - } - return result + " )"; - } - } - private static String getBitmapString(Bitmap bitmap) { int size = Util.getBitmapByteSize(bitmap); return getBitmapString(size); diff --git a/library/src/main/java/com/bumptech/glide/load/engine/prefill/BitmapPreFiller.java b/library/src/main/java/com/bumptech/glide/load/engine/prefill/BitmapPreFiller.java index f2e9accc14..e9ab19b2a0 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/prefill/BitmapPreFiller.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/prefill/BitmapPreFiller.java @@ -40,7 +40,8 @@ public void preFill(PreFillType.Builder... bitmapAttributeBuilders) { for (int i = 0; i < bitmapAttributeBuilders.length; i++) { PreFillType.Builder builder = bitmapAttributeBuilders[i]; if (builder.getConfig() == null) { - builder.setConfig(defaultFormat == DecodeFormat.ALWAYS_ARGB_8888 + builder.setConfig( + defaultFormat == DecodeFormat.ALWAYS_ARGB_8888 || defaultFormat == DecodeFormat.PREFER_ARGB_8888 ? Bitmap.Config.ARGB_8888 : Bitmap.Config.RGB_565); } bitmapAttributes[i] = builder.build(); diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java index d24f2d1775..56eca774ac 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java @@ -247,7 +247,8 @@ private static boolean shouldUsePool(InputStream is) { private static Bitmap.Config getConfig(InputStream is, DecodeFormat format) { // Changing configs can cause skewing on 4.1, see issue #128. - if (format == DecodeFormat.ALWAYS_ARGB_8888 || Build.VERSION.SDK_INT == Build.VERSION_CODES.JELLY_BEAN) { + if (format == DecodeFormat.ALWAYS_ARGB_8888 || format == DecodeFormat.PREFER_ARGB_8888 + || Build.VERSION.SDK_INT == Build.VERSION_CODES.JELLY_BEAN) { return Bitmap.Config.ARGB_8888; } diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java b/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java index adc6eea5bb..721006c7ba 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java @@ -71,8 +71,8 @@ public void onDestroy() { @Override public void onTrimMemory(int level) { - // If an activity is re-created, onTrimMemory may be called before a manager is ever set. - // See #329. + // If an activity is re-created, onTrimMemory may be called before a manager is ever set. + // See #329. if (requestManager != null) { requestManager.onTrimMemory(level); }