From 250ac363aeee12401331299741f09a84f2570e08 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Fri, 6 Jan 2023 10:54:08 -0800 Subject: [PATCH] Fix vector/shape drawables not loading with DirectResourceLoader. PiperOrigin-RevId: 500212780 --- .../java/com/bumptech/glide/DarkModeTest.java | 40 ++++++++++++ .../glide/NonBitmapDrawableResourcesTest.java | 18 +++-- instrumentation/src/main/AndroidManifest.xml | 4 +- .../src/main/res/drawable/vector_drawable.xml | 2 +- .../res/drawable/vector_drawable_dark.xml | 18 +++++ .../res/drawable/vector_drawable_light.xml | 18 +++++ .../src/main/res/values/colors.xml | 6 ++ .../src/main/res/values/styles.xml | 8 +++ .../com/bumptech/glide/RegistryFactory.java | 15 +++-- .../load/model/DirectResourceLoader.java | 65 +++++++++++++++++-- 10 files changed, 177 insertions(+), 17 deletions(-) create mode 100644 instrumentation/src/main/res/drawable/vector_drawable_dark.xml create mode 100644 instrumentation/src/main/res/drawable/vector_drawable_light.xml create mode 100644 instrumentation/src/main/res/values/colors.xml create mode 100644 instrumentation/src/main/res/values/styles.xml diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java index a8095b66da..bf87202fcf 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java @@ -8,6 +8,7 @@ import android.content.Context; import android.content.res.Resources; import android.graphics.Bitmap; +import android.graphics.Canvas; import android.graphics.drawable.BitmapDrawable; import android.graphics.drawable.Drawable; import android.net.Uri; @@ -51,6 +52,45 @@ public void before() { assumeTrue(VERSION.SDK_INT >= VERSION_CODES.Q); } + @Test + public void load_withDarkModeActivity_vectorDrawable_usesDarkModeColor() { + try (ActivityScenario scenario = darkModeActivity()) { + scenario.onActivity( + activity -> { + ViewGroup container = findContainer(activity); + ImageView imageView = newFixedSizeImageView(activity); + container.addView(imageView); + + Glide.with(activity) + .load(R.drawable.vector_drawable) + .override(Target.SIZE_ORIGINAL) + .into(imageView); + }); + + onIdle(); + scenario.onActivity( + activity -> { + ViewGroup container = findContainer(activity); + ImageView imageView = (ImageView) container.getChildAt(0); + Bitmap result = drawToBitmap(imageView.getDrawable()); + Bitmap expected = drawToBitmap(activity.getDrawable(R.drawable.vector_drawable_dark)); + assertThat(result).sameAs(expected); + }); + } + } + + private static Bitmap drawToBitmap(Drawable drawable) { + int width = drawable.getIntrinsicWidth(); + int height = drawable.getIntrinsicHeight(); + + Bitmap result = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888); + Canvas canvas = new Canvas(result); + drawable.setBounds(0, 0, width, height); + drawable.draw(canvas); + canvas.setBitmap(null); + return result; + } + @Test public void load_withDarkModeActivity_useDarkModeDrawable() { runActivityTest( diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java index 1cb24e6d09..336b13ce52 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java @@ -17,10 +17,10 @@ import android.graphics.drawable.Drawable; import android.net.Uri; import androidx.test.core.app.ApplicationProvider; -import androidx.test.ext.junit.runners.AndroidJUnit4; import com.bumptech.glide.load.resource.bitmap.RoundedCorners; import com.bumptech.glide.test.ResourceIds; import com.bumptech.glide.testutil.TearDownGlide; +import com.google.common.collect.ImmutableList; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -31,19 +31,29 @@ import org.junit.function.ThrowingRunnable; import org.junit.rules.TestName; import org.junit.runner.RunWith; -import org.mockito.MockitoAnnotations; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; -@RunWith(AndroidJUnit4.class) +@RunWith(Parameterized.class) public class NonBitmapDrawableResourcesTest { @Rule public final TestName testName = new TestName(); @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + @Parameter public boolean useDirectResourceLoader; + + @Parameters(name = "useDirectResourceLoader = {0}") + public static ImmutableList parameters() { + return ImmutableList.of(true, false); + } + private Context context; @Before public void setUp() { - MockitoAnnotations.initMocks(this); context = ApplicationProvider.getApplicationContext(); + + Glide.init(context, new GlideBuilder().useDirectResourceLoader(useDirectResourceLoader)); } @Test diff --git a/instrumentation/src/main/AndroidManifest.xml b/instrumentation/src/main/AndroidManifest.xml index 1c9610257a..4afddf23f0 100644 --- a/instrumentation/src/main/AndroidManifest.xml +++ b/instrumentation/src/main/AndroidManifest.xml @@ -3,7 +3,9 @@ package="com.bumptech.glide.instrumentation"> - + diff --git a/instrumentation/src/main/res/drawable/vector_drawable.xml b/instrumentation/src/main/res/drawable/vector_drawable.xml index 1732bb9ba5..76df008cce 100644 --- a/instrumentation/src/main/res/drawable/vector_drawable.xml +++ b/instrumentation/src/main/res/drawable/vector_drawable.xml @@ -5,7 +5,7 @@ android:viewportHeight="24.0" android:viewportWidth="24.0"> + + + diff --git a/instrumentation/src/main/res/drawable/vector_drawable_light.xml b/instrumentation/src/main/res/drawable/vector_drawable_light.xml new file mode 100644 index 0000000000..44dbc7606d --- /dev/null +++ b/instrumentation/src/main/res/drawable/vector_drawable_light.xml @@ -0,0 +1,18 @@ + + + + diff --git a/instrumentation/src/main/res/values/colors.xml b/instrumentation/src/main/res/values/colors.xml new file mode 100644 index 0000000000..b1c4e8fe09 --- /dev/null +++ b/instrumentation/src/main/res/values/colors.xml @@ -0,0 +1,6 @@ + + + + #f9b840 + #ffffff + \ No newline at end of file diff --git a/instrumentation/src/main/res/values/styles.xml b/instrumentation/src/main/res/values/styles.xml new file mode 100644 index 0000000000..2548be6a15 --- /dev/null +++ b/instrumentation/src/main/res/values/styles.xml @@ -0,0 +1,8 @@ + + + + \ No newline at end of file diff --git a/library/src/main/java/com/bumptech/glide/RegistryFactory.java b/library/src/main/java/com/bumptech/glide/RegistryFactory.java index 3ec3e961ed..f281aa048a 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/library/src/main/java/com/bumptech/glide/RegistryFactory.java @@ -279,11 +279,15 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm DirectResourceLoader.inputStreamFactory(context); ModelLoaderFactory assetFileDescriptorFactory = DirectResourceLoader.assetFileDescriptorFactory(context); + ModelLoaderFactory drawableFactory = + DirectResourceLoader.drawableFactory(context); registry .append(int.class, InputStream.class, inputStreamFactory) .append(Integer.class, InputStream.class, inputStreamFactory) .append(int.class, AssetFileDescriptor.class, assetFileDescriptorFactory) .append(Integer.class, AssetFileDescriptor.class, assetFileDescriptorFactory) + .append(int.class, Drawable.class, drawableFactory) + .append(Integer.class, Drawable.class, drawableFactory) .append(Uri.class, InputStream.class, ResourceUriLoader.newStreamFactory(context)) .append( Uri.class, @@ -292,7 +296,6 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm } else { ResourceLoader.StreamFactory resourceLoaderStreamFactory = new ResourceLoader.StreamFactory(resources); - ResourceLoader.UriFactory resourceLoaderUriFactory = new ResourceLoader.UriFactory(resources); ResourceLoader.FileDescriptorFactory resourceLoaderFileDescriptorFactory = new ResourceLoader.FileDescriptorFactory(resources); ResourceLoader.AssetFileDescriptorFactory resourceLoaderAssetFileDescriptorFactory = @@ -300,17 +303,19 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm registry .append(int.class, InputStream.class, resourceLoaderStreamFactory) - .append(int.class, ParcelFileDescriptor.class, resourceLoaderFileDescriptorFactory) .append(Integer.class, InputStream.class, resourceLoaderStreamFactory) + .append(int.class, ParcelFileDescriptor.class, resourceLoaderFileDescriptorFactory) .append(Integer.class, ParcelFileDescriptor.class, resourceLoaderFileDescriptorFactory) - .append(Integer.class, Uri.class, resourceLoaderUriFactory) .append(int.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory) .append( - Integer.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory) - .append(int.class, Uri.class, resourceLoaderUriFactory); + Integer.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory); } + // Handles resources from other applications or converting Drawable resource types to Bitmaps. + ResourceLoader.UriFactory resourceLoaderUriFactory = new ResourceLoader.UriFactory(resources); registry + .append(Integer.class, Uri.class, resourceLoaderUriFactory) + .append(int.class, Uri.class, resourceLoaderUriFactory) .append(String.class, InputStream.class, new DataUrlLoader.StreamFactory()) .append(Uri.class, InputStream.class, new DataUrlLoader.StreamFactory()) .append(String.class, InputStream.class, new StringLoader.StreamFactory()) diff --git a/library/src/main/java/com/bumptech/glide/load/model/DirectResourceLoader.java b/library/src/main/java/com/bumptech/glide/load/model/DirectResourceLoader.java index b01e6e532a..1d5eafef6a 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/DirectResourceLoader.java +++ b/library/src/main/java/com/bumptech/glide/load/model/DirectResourceLoader.java @@ -4,6 +4,7 @@ import android.content.res.AssetFileDescriptor; import android.content.res.Resources; import android.content.res.Resources.Theme; +import android.graphics.drawable.Drawable; import android.os.Build; import android.os.Build.VERSION_CODES; import androidx.annotation.NonNull; @@ -12,9 +13,9 @@ import com.bumptech.glide.load.DataSource; import com.bumptech.glide.load.Options; import com.bumptech.glide.load.data.DataFetcher; +import com.bumptech.glide.load.resource.drawable.DrawableDecoderCompat; import com.bumptech.glide.load.resource.drawable.ResourceDrawableDecoder; import com.bumptech.glide.signature.ObjectKey; -import java.io.Closeable; import java.io.IOException; import java.io.InputStream; @@ -26,8 +27,7 @@ * @param The type of data this {@code ModelLoader} will produce (e.g. {@link InputStream}, * {@link AssetFileDescriptor} etc). */ -public final class DirectResourceLoader - implements ModelLoader { +public final class DirectResourceLoader implements ModelLoader { private final Context context; private final ResourceOpener resourceOpener; @@ -41,6 +41,10 @@ public static ModelLoaderFactory assetFileDescript return new AssetFileDescriptorFactory(context); } + public static ModelLoaderFactory drawableFactory(Context context) { + return new DrawableFactory(context); + } + DirectResourceLoader(Context context, ResourceOpener resourceOpener) { this.context = context.getApplicationContext(); this.resourceOpener = resourceOpener; @@ -71,6 +75,8 @@ public boolean handles(@NonNull Integer integer) { private interface ResourceOpener { DataT open(Resources resources, int resourceId); + void close(DataT data) throws IOException; + Class getDataClass(); } @@ -89,6 +95,11 @@ public AssetFileDescriptor open(Resources resources, int resourceId) { return resources.openRawResourceFd(resourceId); } + @Override + public void close(AssetFileDescriptor data) throws IOException { + data.close(); + } + @Override public Class getDataClass() { return AssetFileDescriptor.class; @@ -125,6 +136,11 @@ public InputStream open(Resources resources, int resourceId) { return resources.openRawResource(resourceId); } + @Override + public void close(InputStream data) throws IOException { + data.close(); + } + @Override public Class getDataClass() { return InputStream.class; @@ -134,8 +150,45 @@ public Class getDataClass() { public void teardown() {} } - private static final class ResourceDataFetcher - implements DataFetcher { + /** + * Handles vectors, shapes and other resources that cannot be opened with + * Resources.openRawResource. Overlaps in functionality with {@link ResourceDrawableDecoder} and + * {@link com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder} but it's more efficient + * for simple resource loads within a single application. + */ + private static final class DrawableFactory + implements ModelLoaderFactory, ResourceOpener { + + private final Context context; + + DrawableFactory(Context context) { + this.context = context; + } + + @Override + public Drawable open(Resources resources, int resourceId) { + return DrawableDecoderCompat.getDrawable(context, resourceId, resources.newTheme()); + } + + @Override + public void close(Drawable data) throws IOException {} + + @Override + public Class getDataClass() { + return Drawable.class; + } + + @NonNull + @Override + public ModelLoader build(@NonNull MultiModelLoaderFactory multiFactory) { + return new DirectResourceLoader<>(context, this); + } + + @Override + public void teardown() {} + } + + private static final class ResourceDataFetcher implements DataFetcher { private final Resources resources; private final ResourceOpener resourceOpener; @@ -164,7 +217,7 @@ public void cleanup() { DataT local = data; if (local != null) { try { - local.close(); + resourceOpener.close(local); } catch (IOException e) { // Ignored. }