diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/DrawableTransformationTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/DrawableTransformationTest.java index b84fa37a46..1057c81dbf 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/DrawableTransformationTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/DrawableTransformationTest.java @@ -17,6 +17,8 @@ import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool; import com.bumptech.glide.load.resource.bitmap.TransformationUtils; import com.bumptech.glide.request.RequestOptions; +import com.bumptech.glide.test.BitmapSubject; +import com.bumptech.glide.test.GlideApp; import java.util.concurrent.ExecutionException; import org.junit.After; import org.junit.Before; @@ -125,4 +127,83 @@ public void load_withColorDrawable_sizeOriginal_requiredTransform_fails() .submit() .get(); } + + @Test + public void load_withBitmapDrawable_andDoNothingTransformation_doesNotRecycleBitmap() + throws ExecutionException, InterruptedException { + Bitmap bitmap = Bitmap.createBitmap(100, 200, Config.ARGB_8888); + BitmapDrawable drawable = new BitmapDrawable(context.getResources(), bitmap); + + Drawable result = GlideApp.with(context) + .load(drawable) + .fitCenter() + .override(bitmap.getWidth(), bitmap.getHeight()) + .submit() + .get(); + + BitmapSubject.assertThat(result).isNotRecycled(); + } + + @Test + public void load_withBitmapDrawable_andFunctionalTransformation_doesNotRecycleBitmap() + throws ExecutionException, InterruptedException { + Bitmap bitmap = Bitmap.createBitmap(100, 200, Config.ARGB_8888); + BitmapDrawable drawable = new BitmapDrawable(context.getResources(), bitmap); + + Drawable result = GlideApp.with(context) + .load(drawable) + .fitCenter() + .override(bitmap.getWidth() / 2, bitmap.getHeight() / 2) + .submit() + .get(); + + BitmapSubject.assertThat(result).isNotRecycled(); + } + + @Test + public void load_withColorDrawable_fixedSize_unitBitmapTransform_recyclesIntermediates() + throws ExecutionException, InterruptedException { + Drawable colorDrawable = new ColorDrawable(Color.RED); + + int width = 100; + int height = 200; + + GlideApp.with(context) + .load(colorDrawable) + .fitCenter() + .override(width, height) + .submit() + .get(); + + BitmapPool bitmapPool = Glide.get(context).getBitmapPool(); + // Make sure we didn't put the same Bitmap twice. + Bitmap first = bitmapPool.get(width, height, Config.ARGB_8888); + Bitmap second = bitmapPool.get(width, height, Config.ARGB_8888); + + assertThat(first).isNotSameAs(second); + } + @Test + public void load_withColorDrawable_fixedSize_functionalBitmapTransform_doesNotRecycleOutput() + throws ExecutionException, InterruptedException { + Drawable colorDrawable = new ColorDrawable(Color.RED); + + int width = 100; + int height = 200; + + Drawable result = GlideApp.with(context) + .load(colorDrawable) + .circleCrop() + .override(width, height) + .submit() + .get(); + + BitmapSubject.assertThat(result).isNotRecycled(); + + BitmapPool bitmapPool = Glide.get(context).getBitmapPool(); + // Make sure we didn't put the same Bitmap twice. + Bitmap first = bitmapPool.get(width, height, Config.ARGB_8888); + Bitmap second = bitmapPool.get(width, height, Config.ARGB_8888); + + assertThat(first).isNotSameAs(second); + } } diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/test/BitmapSubject.java b/instrumentation/src/androidTest/java/com/bumptech/glide/test/BitmapSubject.java index 6128cbff7d..fd98416040 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/test/BitmapSubject.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/test/BitmapSubject.java @@ -15,7 +15,7 @@ /** * Truth assertions for comparing {@link Bitmap}s. */ -final class BitmapSubject extends Subject { +public final class BitmapSubject extends Subject { private static final SubjectFactory FACTORY = new SubjectFactory() { @@ -30,14 +30,14 @@ private BitmapSubject(FailureStrategy failureStrategy, super(failureStrategy, subject); } - static BitmapSubject assertThat(Drawable drawable) { + public static BitmapSubject assertThat(Drawable drawable) { if (!(drawable instanceof BitmapDrawable)) { throw new IllegalArgumentException("Not a BitmapDrawable: " + drawable); } return assertThat(((BitmapDrawable) drawable).getBitmap()); } - static BitmapSubject assertThat(Bitmap bitmap) { + public static BitmapSubject assertThat(Bitmap bitmap) { return Truth.assertAbout(FACTORY).that(bitmap); } @@ -54,34 +54,40 @@ private static String getDisplayString(Bitmap bitmap) { + ">"; } - void sameAs(@DrawableRes int resourceId) { + public void sameAs(@DrawableRes int resourceId) { Context context = InstrumentationRegistry.getTargetContext(); Drawable drawable = ResourcesCompat.getDrawable(context.getResources(), resourceId, context.getTheme()); sameAs(drawable); } - void isMutable() { + public void isMutable() { if (!getSubject().isMutable()) { fail("is mutable"); } } - void isImmutable() { + public void isImmutable() { if (getSubject().isMutable()) { fail("is immutable"); } } + public void isNotRecycled() { + if (getSubject().isRecycled()) { + fail("is not recycled"); + } + } + @SuppressWarnings("unchecked") - void sameAs(Drawable other) { + public void sameAs(Drawable other) { if (!(other instanceof BitmapDrawable)) { fail("Not a BitmapDrawable"); } sameAs(((BitmapDrawable) other).getBitmap()); } - void sameAs(Bitmap other) { + public void sameAs(Bitmap other) { if (!getSubject().sameAs(other)) { fail("is the same as " + getDisplayString(other)); } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableToBitmapConverter.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableToBitmapConverter.java index 9f62571936..2ed84484dd 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableToBitmapConverter.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableToBitmapConverter.java @@ -15,7 +15,12 @@ final class DrawableToBitmapConverter { private static final String TAG = "DrawableToBitmap"; - private static final BitmapPool NO_BITMAP_POOL = new BitmapPoolAdapter(); + private static final BitmapPool NO_RECYCLE_BITMAP_POOL = new BitmapPoolAdapter() { + @Override + public void put(Bitmap bitmap) { + // Avoid calling super to avoid recycling the given Bitmap. + } + }; private DrawableToBitmapConverter() { // Utility class. } @@ -34,7 +39,7 @@ static Resource convert(BitmapPool bitmapPool, Drawable drawable, int wi isRecycleable = true; } - BitmapPool toUse = isRecycleable ? bitmapPool : NO_BITMAP_POOL; + BitmapPool toUse = isRecycleable ? bitmapPool : NO_RECYCLE_BITMAP_POOL; return BitmapResource.obtain(result, toUse); } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformation.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformation.java index 2038ac3c0a..54b7abc515 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformation.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformation.java @@ -61,7 +61,6 @@ public Resource transform(Context context, Resource resource if (transformedBitmapResource.equals(bitmapResourceToTransform)) { transformedBitmapResource.recycle(); - bitmapResourceToTransform.recycle(); return resource; } else { return newDrawableResource(context, transformedBitmapResource.get()); diff --git a/library/src/test/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformationTest.java b/library/src/test/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformationTest.java new file mode 100644 index 0000000000..f0a39f65ed --- /dev/null +++ b/library/src/test/java/com/bumptech/glide/load/resource/bitmap/DrawableTransformationTest.java @@ -0,0 +1,154 @@ +package com.bumptech.glide.load.resource.bitmap; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.graphics.Bitmap; +import android.graphics.Color; +import android.graphics.drawable.BitmapDrawable; +import android.graphics.drawable.ColorDrawable; +import android.graphics.drawable.Drawable; +import com.bumptech.glide.Glide; +import com.bumptech.glide.GlideBuilder; +import com.bumptech.glide.load.Transformation; +import com.bumptech.glide.load.engine.Resource; +import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool; +import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPoolAdapter; +import com.bumptech.glide.load.resource.SimpleResource; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE, sdk = 18) +public class DrawableTransformationTest { + + private BitmapPool bitmapPool; + @Mock private Transformation bitmapTransformation; + private DrawableTransformation transformation; + private Context context; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + transformation = new DrawableTransformation(bitmapTransformation, /*isRequired=*/ true); + context = RuntimeEnvironment.application; + bitmapPool = new BitmapPoolAdapter(); + Glide.init(new GlideBuilder() + .setBitmapPool(bitmapPool) + .build(context)); + } + + @After + public void tearDown() { + Glide.tearDown(); + } + + @Test + public void transform_withBitmapDrawable_andUnitBitmapTransformation_doesNotRecycle() { + when( + bitmapTransformation + .transform( + any(Context.class), anyBitmapResource(), anyInt(), anyInt())) + .thenAnswer(new ReturnGivenResource()); + + Bitmap bitmap = Bitmap.createBitmap(100, 200, Bitmap.Config.ARGB_8888); + BitmapDrawable drawable = new BitmapDrawable(context.getResources(), bitmap); + @SuppressWarnings("unchecked") + Resource input = + (Resource) (Resource) new BitmapDrawableResource(drawable, bitmapPool); + transformation.transform(context, input, /*outWidth=*/ 100, /*outHeight=*/ 200); + + assertThat(bitmap.isRecycled()).isFalse(); + } + + @Test + public void transform_withBitmapDrawable_andFunctionalBitmapTransformation_doesNotRecycle() { + when(bitmapTransformation.transform( + any(Context.class), anyBitmapResource(), anyInt(), anyInt())) + .thenAnswer(new Answer>() { + @Override + public Resource answer(InvocationOnMock invocationOnMock) throws Throwable { + return BitmapResource.obtain( + Bitmap.createBitmap(200, 200, Bitmap.Config.ARGB_8888), bitmapPool); + } + }); + Bitmap bitmap = Bitmap.createBitmap(100, 200, Bitmap.Config.ARGB_8888); + BitmapDrawable drawable = new BitmapDrawable(context.getResources(), bitmap); + @SuppressWarnings("unchecked") + Resource input = + (Resource) (Resource) new BitmapDrawableResource(drawable, bitmapPool); + transformation.transform(context, input, /*outWidth=*/ 100, /*outHeight=*/ 200); + + assertThat(bitmap.isRecycled()).isFalse(); + } + + @Test + public void transform_withColorDrawable_andUnitBitmapTransformation_recycles() { + bitmapPool = mock(BitmapPool.class); + Glide.tearDown(); + Glide.init(new GlideBuilder().setBitmapPool(bitmapPool).build(context)); + when( + bitmapTransformation + .transform( + any(Context.class), anyBitmapResource(), anyInt(), anyInt())) + .thenAnswer(new ReturnGivenResource()); + + ColorDrawable colorDrawable = new ColorDrawable(Color.RED); + final Resource input = new SimpleResource(colorDrawable); + + doAnswer(new Answer() { + @Override + public Void answer(InvocationOnMock invocationOnMock) throws Throwable { + Bitmap bitmap = (Bitmap) invocationOnMock.getArguments()[0]; + assertThat(bitmap.getWidth()).isEqualTo(100); + assertThat(bitmap.getHeight()).isEqualTo(200); + return null; + } + }).when(bitmapPool).put(any(Bitmap.class)); + when(bitmapPool.get(anyInt(), anyInt(), any(Bitmap.Config.class))) + .thenAnswer(new Answer() { + @Override + public Bitmap answer(InvocationOnMock invocationOnMock) throws Throwable { + int width = (Integer) invocationOnMock.getArguments()[0]; + int height = (Integer) invocationOnMock.getArguments()[1]; + Bitmap.Config config = (Bitmap.Config) invocationOnMock.getArguments()[2]; + return Bitmap.createBitmap(width, height, config); + } + }); + + transformation.transform(context, input, /*outWidth=*/ 100, /*outHeight=*/ 200); + + verify(bitmapPool).put(isA(Bitmap.class)); + } + + @SuppressWarnings("unchecked") + private static Resource anyBitmapResource() { + return any(Resource.class); + } + + private static final class ReturnGivenResource implements Answer> { + + @Override + public Resource answer(InvocationOnMock invocationOnMock) throws Throwable { + @SuppressWarnings("unchecked") + Resource input = (Resource) invocationOnMock.getArguments()[1]; + return input; + } + } +}