Enable DirectResourceLoader in Glide

The new loaders respect themes, dark mode, screen sizes and other resource attributes that are not supported by contentResolver.openInputStream. We can only use them reliably with resource ids and uris that are owned by the same package as the resource, so the legacy loaders will continue to live for other packages. There are also some edge cases, like videos in raw resources, that are not supported by the direct resource loaders.

This will result in a behavior change for applications. Resources that were previously loaded with default attributes will now be loaded instead with the appropriate theme / dark mode / size etc.

PiperOrigin-RevId: 511612517

Author: sjudd

instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java

@@ -46,7 +46,7 @@ public class DarkModeTest {
 
   @Rule
   public final IdlingGlideRule idlingGlideRule =
-      IdlingGlideRule.newGlideRule(glideBuilder -> glideBuilder.useDirectResourceLoader(true));
+      IdlingGlideRule.newGlideRule(glideBuilder -> glideBuilder);
 
   @Before
   public void before() {

instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java

@@ -17,44 +17,25 @@
 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;
 import java.util.concurrent.ExecutionException;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.function.ThrowingRunnable;
 import org.junit.rules.TestName;
 import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
 
-@RunWith(Parameterized.class)
+@RunWith(AndroidJUnit4.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() {
-    context = ApplicationProvider.getApplicationContext();
-
-    Glide.init(context, new GlideBuilder().useDirectResourceLoader(useDirectResourceLoader));
-  }
+  private final Context context = ApplicationProvider.getApplicationContext();
 
   @Test
   public void load_withBitmapResourceId_asDrawable_producesNonNullDrawable()

library/src/main/java/com/bumptech/glide/GlideBuilder.java

@@ -487,23 +487,6 @@ public GlideBuilder setImageDecoderEnabledForBitmaps(boolean isEnabled) {
     return this;
   }
 
-  /**
-   * Use {@link com.bumptech.glide.load.model.DirectResourceLoader} instead of {@link
-   * com.bumptech.glide.load.model.ResourceLoader} so that we load drawables asynchronously with the
-   * correc theme (ie light / dark mode).
-   *
-   * <p>This also removes support for loading resources as resource Uris and for loading {@link
-   * android.os.ParcelFileDescriptor}s from resource ids. I think neither of those are useful but if
-   * you have a use case or seem to find some test failing with this experiment enabled, please file
-   * an issue so we can investigate.
-   *
-   * <p>This flag is experimental and will be removed without notice in a future version.
-   */
-  public GlideBuilder useDirectResourceLoader(boolean isEnabled) {
-    glideExperimentsBuilder.update(new UseDirectResourceLoader(), isEnabled);
-    return this;
-  }
-
   void setRequestManagerFactory(@Nullable RequestManagerFactory factory) {
     this.requestManagerFactory = factory;
   }

library/src/main/java/com/bumptech/glide/RegistryFactory.java

@@ -13,7 +13,6 @@
 import androidx.annotation.Nullable;
 import androidx.tracing.Trace;
 import com.bumptech.glide.GlideBuilder.EnableImageDecoderForBitmaps;
-import com.bumptech.glide.GlideBuilder.UseDirectResourceLoader;
 import com.bumptech.glide.gifdecoder.GifDecoder;
 import com.bumptech.glide.load.ImageHeaderParser;
 import com.bumptech.glide.load.ResourceDecoder;
@@ -274,48 +273,47 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
       registry.register(new ParcelFileDescriptorRewinder.Factory());
     }
 
-    if (experiments.isEnabled(UseDirectResourceLoader.class)) {
-      ModelLoaderFactory<Integer, InputStream> inputStreamFactory =
-          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,
-              AssetFileDescriptor.class,
-              ResourceUriLoader.newAssetFileDescriptorFactory(context));
-    } else {
-      ResourceLoader.StreamFactory resourceLoaderStreamFactory =
-          new ResourceLoader.StreamFactory(resources);
-      ResourceLoader.FileDescriptorFactory resourceLoaderFileDescriptorFactory =
-          new ResourceLoader.FileDescriptorFactory(resources);
-      ResourceLoader.AssetFileDescriptorFactory resourceLoaderAssetFileDescriptorFactory =
-          new ResourceLoader.AssetFileDescriptorFactory(resources);
-
-      registry
-          .append(int.class, InputStream.class, resourceLoaderStreamFactory)
-          .append(Integer.class, InputStream.class, resourceLoaderStreamFactory)
-          .append(int.class, ParcelFileDescriptor.class, resourceLoaderFileDescriptorFactory)
-          .append(Integer.class, ParcelFileDescriptor.class, resourceLoaderFileDescriptorFactory)
-          .append(int.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory)
-          .append(
-              Integer.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory);
-    }
+    // DirectResourceLoader and ResourceUriLoader handle resource IDs and Uris owned by this
+    // package.
+    ModelLoaderFactory<Integer, InputStream> directResourceLoaderStreamFactory =
+        DirectResourceLoader.inputStreamFactory(context);
+    ModelLoaderFactory<Integer, AssetFileDescriptor>
+        directResourceLoaderAssetFileDescriptorFactory =
+            DirectResourceLoader.assetFileDescriptorFactory(context);
+    ModelLoaderFactory<Integer, Drawable> directResourceLaoderDrawableFactory =
+        DirectResourceLoader.drawableFactory(context);
+    registry
+        .append(int.class, InputStream.class, directResourceLoaderStreamFactory)
+        .append(Integer.class, InputStream.class, directResourceLoaderStreamFactory)
+        .append(
+            int.class, AssetFileDescriptor.class, directResourceLoaderAssetFileDescriptorFactory)
+        .append(
+            Integer.class,
+            AssetFileDescriptor.class,
+            directResourceLoaderAssetFileDescriptorFactory)
+        .append(int.class, Drawable.class, directResourceLaoderDrawableFactory)
+        .append(Integer.class, Drawable.class, directResourceLaoderDrawableFactory)
+        .append(Uri.class, InputStream.class, ResourceUriLoader.newStreamFactory(context))
+        .append(
+            Uri.class,
+            AssetFileDescriptor.class,
+            ResourceUriLoader.newAssetFileDescriptorFactory(context));
 
-    // Handles resources from other applications or converting Drawable resource types to Bitmaps.
+    // ResourceLoader and UriLoader handle resource IDs and Uris owned by other packages.
     ResourceLoader.UriFactory resourceLoaderUriFactory = new ResourceLoader.UriFactory(resources);
+    ResourceLoader.AssetFileDescriptorFactory resourceLoaderAssetFileDescriptorFactory =
+        new ResourceLoader.AssetFileDescriptorFactory(resources);
+    ResourceLoader.StreamFactory resourceLoaderStreamFactory =
+        new ResourceLoader.StreamFactory(resources);
     registry
         .append(Integer.class, Uri.class, resourceLoaderUriFactory)
         .append(int.class, Uri.class, resourceLoaderUriFactory)
+        .append(Integer.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory)
+        .append(int.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory)
+        .append(Integer.class, InputStream.class, resourceLoaderStreamFactory)
+        .append(int.class, InputStream.class, resourceLoaderStreamFactory);
+
+    registry
         .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())
@@ -338,16 +336,15 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
           new QMediaStoreUriLoader.FileDescriptorFactory(context));
     }
     registry
-        .append(
-            Uri.class, InputStream.class, new UriLoader.StreamFactory(contentResolver, experiments))
+        .append(Uri.class, InputStream.class, new UriLoader.StreamFactory(contentResolver))
         .append(
             Uri.class,
             ParcelFileDescriptor.class,
-            new UriLoader.FileDescriptorFactory(contentResolver, experiments))
+            new UriLoader.FileDescriptorFactory(contentResolver))
         .append(
             Uri.class,
             AssetFileDescriptor.class,
-            new UriLoader.AssetFileDescriptorFactory(contentResolver, experiments))
+            new UriLoader.AssetFileDescriptorFactory(contentResolver))
         .append(Uri.class, InputStream.class, new UrlUriLoader.StreamFactory())
         .append(URL.class, InputStream.class, new UrlLoader.StreamFactory())
         .append(Uri.class, File.class, new MediaStoreFileLoader.Factory(context))

library/src/main/java/com/bumptech/glide/load/model/DirectResourceLoader.java

@@ -24,6 +24,10 @@
  * Resources#openRawResourceFd(int)} using the theme from {@link ResourceDrawableDecoder#THEME} when
  * it's available.
  *
+ * <p>Resource ids from other packages are handled by {@link ResourceLoader} via {@link
+ * ResourceDrawableDecoder} and {@link
+ * com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder}.
+ *
  * @param <DataT> The type of data this {@code ModelLoader} will produce (e.g. {@link InputStream},
  *     {@link AssetFileDescriptor} etc).
  */

library/src/main/java/com/bumptech/glide/load/model/ResourceLoader.java

@@ -15,6 +15,14 @@
  * A model loader for handling Android resource files. Model must be an Android resource id in the
  * package of the given context.
  *
+ * <p>This class should always be less preferred than {@link DirectResourceLoader} because {@link
+ * DirectResourceLoader} is more efficient for {@code Drawables} owned by this package. This class
+ * only handles passing through {@link Uri}s to {@link
+ * com.bumptech.glide.load.resource.drawable.ResourceDrawableDecoder} and {@link
+ * com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder}. Those classes can handle assets
+ * from other applications, but are not as efficient as {@link DirectResourceLoader} for assets
+ * owned by this package.
+ *
  * @param <Data> The type of data that will be loaded for the given android resource.
  */
 public class ResourceLoader<Data> implements ModelLoader<Integer, Data> {
@@ -82,7 +90,14 @@ public void teardown() {
     }
   }
 
-  /** Factory for loading {@link ParcelFileDescriptor}s from Android resource ids. */
+  /**
+   * Factory for loading {@link ParcelFileDescriptor}s from Android resource ids.
+   *
+   * @deprecated This class is unused by Glide. {@link AssetFileDescriptorFactory} should be
+   *     preferred because it's not possible to reliably load a simple {@link
+   *     java.io.FileDescriptor} for resources.
+   */
+  @Deprecated
   public static class FileDescriptorFactory
       implements ModelLoaderFactory<Integer, ParcelFileDescriptor> {
 

library/src/main/java/com/bumptech/glide/load/model/ResourceUriLoader.java

@@ -15,11 +15,19 @@
 /**
  * Converts Resource Uris to resource ids if the resource Uri points to a resource in this package.
  *
+ * <p>This class works by parsing Uris into resource ids, then delegating the resource ID load to
+ * other {@link ModelLoader}s, typically {@link DirectResourceLoader}.
+ *
  * <p>This class really shouldn't need to exist. If you need to load resources, just pass in the
  * integer resource id directly using {@link com.bumptech.glide.RequestManager#load(Integer)}
  * instead. It'll be more correct in terms of caching and more efficient to load. The only reason
  * we're supporting this case is for backwards compatibility.
  *
+ * <p>Because this class explicitly only handles resource Uris that are from the application's
+ * package, resource uris from other packages are handled by {@link UriLoader}. {@link UriLoader} is
+ * even less preferred because it can only handle certain resources from raw resources and it will
+ * not apply appropriate theming, RTL or night mode attributes.
+ *
  * @param <DataT> The type of data produced, e.g. {@link InputStream} or {@link
  *     AssetFileDescriptor}.
  */