Fix vector/shape drawables not loading with DirectResourceLoader.

sjudd · glide-copybara-robot · commit dcd1c08b94f5 · 2023-01-06T13:14:25.000-08:00
PiperOrigin-RevId: 500212780
diff --git 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<FragmentActivity> 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
@@ -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<Boolean> 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
@@ -3,7 +3,9 @@
     package="com.bumptech.glide.instrumentation">
   <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
   <uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW" />
-  <application tools:ignore="MissingApplicationIcon" >
+  <application
+      tools:ignore="MissingApplicationIcon"
+      android:theme="@style/AppTheme">
     <activity
         android:name="com.bumptech.glide.test.GlideWithBeforeSuperOnCreateActivity"
         android:exported="false" />
diff --git 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">
   <path
-    android:fillColor="#f9b840"
+    android:fillColor="?attr/colorControlNormal"
     android:pathData="M15.5,5.5c1.1,0 2,-0.9 2,-2s-0.9,-2 -2,-2 -2,0.9
         -2,2 0.9,2 2,2zM5,12c-2.8,0 -5,2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,
         -5 -5,-5zM5,20.5c-1.9,0 -3.5,-1.6 -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,
diff --git a/instrumentation/src/main/res/drawable/vector_drawable_dark.xml b/instrumentation/src/main/res/drawable/vector_drawable_dark.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+  android:width="64dp"
+  android:height="64dp"
+  android:viewportHeight="24.0"
+  android:viewportWidth="24.0">
+  <path
+    android:fillColor="@color/colorPrimaryDark"
+    android:pathData="M15.5,5.5c1.1,0 2,-0.9 2,-2s-0.9,-2 -2,-2 -2,0.9
+        -2,2 0.9,2 2,2zM5,12c-2.8,0 -5,2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,
+        -5 -5,-5zM5,20.5c-1.9,0 -3.5,-1.6 -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,
+        1.6 3.5,3.5 -1.6,3.5 -3.5,3.5zM10.8,10.5l2.4,-2.4 0.8,0.8c1.3,1.3
+        3,2.1 5.1,2.1L19.1,9c-1.5,0 -2.7,-0.6 -3.6,-1.5l-1.9,-1.9c-0.5,-0.4
+        -1,-0.6 -1.6,-0.6s-1.1,0.2 -1.4,0.6L7.8,8.4c-0.4,0.4 -0.6,0.9 -0.6,
+        1.4 0,0.6 0.2,1.1 0.6,1.4L11,14v5h2v-6.2l-2.2,-2.3zM19,12c-2.8,0 -5,
+        2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,-5 -5,-5zM19,20.5c-1.9,0 -3.5,-1.6
+         -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,1.6 3.5,3.5 -1.6,3.5 -3.5,3.5z" />
+</vector>
diff --git a/instrumentation/src/main/res/drawable/vector_drawable_light.xml b/instrumentation/src/main/res/drawable/vector_drawable_light.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+  android:width="64dp"
+  android:height="64dp"
+  android:viewportHeight="24.0"
+  android:viewportWidth="24.0">
+  <path
+    android:fillColor="@color/colorPrimary"
+    android:pathData="M15.5,5.5c1.1,0 2,-0.9 2,-2s-0.9,-2 -2,-2 -2,0.9
+        -2,2 0.9,2 2,2zM5,12c-2.8,0 -5,2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,
+        -5 -5,-5zM5,20.5c-1.9,0 -3.5,-1.6 -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,
+        1.6 3.5,3.5 -1.6,3.5 -3.5,3.5zM10.8,10.5l2.4,-2.4 0.8,0.8c1.3,1.3
+        3,2.1 5.1,2.1L19.1,9c-1.5,0 -2.7,-0.6 -3.6,-1.5l-1.9,-1.9c-0.5,-0.4
+        -1,-0.6 -1.6,-0.6s-1.1,0.2 -1.4,0.6L7.8,8.4c-0.4,0.4 -0.6,0.9 -0.6,
+        1.4 0,0.6 0.2,1.1 0.6,1.4L11,14v5h2v-6.2l-2.2,-2.3zM19,12c-2.8,0 -5,
+        2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,-5 -5,-5zM19,20.5c-1.9,0 -3.5,-1.6
+         -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,1.6 3.5,3.5 -1.6,3.5 -3.5,3.5z" />
+</vector>
diff --git a/instrumentation/src/main/res/values/colors.xml b/instrumentation/src/main/res/values/colors.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<resources>
+  <color name="colorPrimary">#f9b840</color>
+  <color name="colorPrimaryDark">#ffffff</color>
+</resources>
diff --git a/instrumentation/src/main/res/values/styles.xml b/instrumentation/src/main/res/values/styles.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+  <style name="AppTheme" parent="Theme.AppCompat.DayNight.DarkActionBar">
+    <!-- Customize your theme here. -->
+    <item name="colorPrimary">@color/colorPrimary</item>
+    <item name="colorPrimaryDark">@color/colorPrimaryDark</item>
+  </style>
+</resources>
diff --git 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<Integer, AssetFileDescriptor> assetFileDescriptorFactory =
           DirectResourceLoader.assetFileDescriptorFactory(context);
+      ModelLoaderFactory<Integer, Drawable> 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,25 +296,26 @@ 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 =
           new ResourceLoader.AssetFileDescriptorFactory(resources);
 
       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<String>())
         .append(Uri.class, InputStream.class, new DataUrlLoader.StreamFactory<Uri>())
         .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
@@ -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 <DataT> The type of data this {@code ModelLoader} will produce (e.g. {@link InputStream},
  *     {@link AssetFileDescriptor} etc).
  */
-public final class DirectResourceLoader<DataT extends Closeable>
-    implements ModelLoader<Integer, DataT> {
+public final class DirectResourceLoader<DataT> implements ModelLoader<Integer, DataT> {
 
   private final Context context;
   private final ResourceOpener<DataT> resourceOpener;
@@ -41,6 +41,10 @@ public static ModelLoaderFactory<Integer, AssetFileDescriptor> assetFileDescript
     return new AssetFileDescriptorFactory(context);
   }
 
+  public static ModelLoaderFactory<Integer, Drawable> drawableFactory(Context context) {
+    return new DrawableFactory(context);
+  }
+
   DirectResourceLoader(Context context, ResourceOpener<DataT> resourceOpener) {
     this.context = context.getApplicationContext();
     this.resourceOpener = resourceOpener;
@@ -71,6 +75,8 @@ public boolean handles(@NonNull Integer integer) {
   private interface ResourceOpener<DataT> {
     DataT open(Resources resources, int resourceId);
 
+    void close(DataT data) throws IOException;
+
     Class<DataT> 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<AssetFileDescriptor> 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<InputStream> getDataClass() {
       return InputStream.class;
@@ -134,8 +150,45 @@ public Class<InputStream> getDataClass() {
     public void teardown() {}
   }
 
-  private static final class ResourceDataFetcher<DataT extends Closeable>
-      implements DataFetcher<DataT> {
+  /**
+   * 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<Integer, Drawable>, ResourceOpener<Drawable> {
+
+    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<Drawable> getDataClass() {
+      return Drawable.class;
+    }
+
+    @NonNull
+    @Override
+    public ModelLoader<Integer, Drawable> build(@NonNull MultiModelLoaderFactory multiFactory) {
+      return new DirectResourceLoader<>(context, this);
+    }
+
+    @Override
+    public void teardown() {}
+  }
+
+  private static final class ResourceDataFetcher<DataT> implements DataFetcher<DataT> {
 
     private final Resources resources;
     private final ResourceOpener<DataT> resourceOpener;
@@ -164,7 +217,7 @@ public void cleanup() {
       DataT local = data;
       if (local != null) {
         try {
-          local.close();
+          resourceOpener.close(local);
         } catch (IOException e) {
           // Ignored.
         }
