diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java index 6d1e1a17a0..12bd0528d7 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java +++ b/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() { diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java index 336b13ce52..4239cc677f 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/NonBitmapDrawableResourcesTest.java +++ b/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 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() diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index 54cf8a91f7..c6f7c7fdd1 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/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). - * - *

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. - * - *

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; } diff --git a/library/src/main/java/com/bumptech/glide/RegistryFactory.java b/library/src/main/java/com/bumptech/glide/RegistryFactory.java index 26e238eb3b..dfe8a34ee6 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/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 inputStreamFactory = - 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, - 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 directResourceLoaderStreamFactory = + DirectResourceLoader.inputStreamFactory(context); + ModelLoaderFactory + directResourceLoaderAssetFileDescriptorFactory = + DirectResourceLoader.assetFileDescriptorFactory(context); + ModelLoaderFactory 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()) .append(Uri.class, InputStream.class, new DataUrlLoader.StreamFactory()) .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)) 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 ce25662432..4a356d161a 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 @@ -24,6 +24,10 @@ * Resources#openRawResourceFd(int)} using the theme from {@link ResourceDrawableDecoder#THEME} when * it's available. * + *

Resource ids from other packages are handled by {@link ResourceLoader} via {@link + * ResourceDrawableDecoder} and {@link + * com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder}. + * * @param The type of data this {@code ModelLoader} will produce (e.g. {@link InputStream}, * {@link AssetFileDescriptor} etc). */ diff --git a/library/src/main/java/com/bumptech/glide/load/model/ResourceLoader.java b/library/src/main/java/com/bumptech/glide/load/model/ResourceLoader.java index 2c9087c27d..1175315a84 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/ResourceLoader.java +++ b/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. * + *

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 The type of data that will be loaded for the given android resource. */ public class ResourceLoader implements ModelLoader { @@ -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 { diff --git a/library/src/main/java/com/bumptech/glide/load/model/ResourceUriLoader.java b/library/src/main/java/com/bumptech/glide/load/model/ResourceUriLoader.java index 735c83b42c..7d2ba90801 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/ResourceUriLoader.java +++ b/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. * + *

This class works by parsing Uris into resource ids, then delegating the resource ID load to + * other {@link ModelLoader}s, typically {@link DirectResourceLoader}. + * *

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

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 The type of data produced, e.g. {@link InputStream} or {@link * AssetFileDescriptor}. */ diff --git a/library/src/main/java/com/bumptech/glide/load/model/UriLoader.java b/library/src/main/java/com/bumptech/glide/load/model/UriLoader.java index d72a35127b..06e5157b8d 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/UriLoader.java +++ b/library/src/main/java/com/bumptech/glide/load/model/UriLoader.java @@ -5,9 +5,6 @@ import android.net.Uri; import android.os.ParcelFileDescriptor; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import com.bumptech.glide.GlideBuilder.UseDirectResourceLoader; -import com.bumptech.glide.GlideExperiments; import com.bumptech.glide.load.Options; import com.bumptech.glide.load.data.AssetFileDescriptorLocalUriFetcher; import com.bumptech.glide.load.data.DataFetcher; @@ -30,31 +27,20 @@ */ public class UriLoader implements ModelLoader { - private static final Set SCHEMES_WITH_RESOURCE = + private static final Set SCHEMES = Collections.unmodifiableSet( new HashSet<>( Arrays.asList( ContentResolver.SCHEME_FILE, - ContentResolver.SCHEME_ANDROID_RESOURCE, - ContentResolver.SCHEME_CONTENT))); - - private static final Set SCHEMES = - Collections.unmodifiableSet( - new HashSet<>( - Arrays.asList(ContentResolver.SCHEME_FILE, ContentResolver.SCHEME_CONTENT))); + ContentResolver.SCHEME_CONTENT, + ContentResolver.SCHEME_ANDROID_RESOURCE))); private final LocalUriFetcherFactory factory; - @Nullable private final GlideExperiments glideExperiments; - - UriLoader(LocalUriFetcherFactory factory, @Nullable GlideExperiments glideExperiments) { - this.factory = factory; - this.glideExperiments = glideExperiments; - } // Public API. @SuppressWarnings("WeakerAccess") public UriLoader(LocalUriFetcherFactory factory) { - this(factory, /* glideExperiments= */ null); + this.factory = factory; } @Override @@ -65,13 +51,7 @@ public LoadData buildLoadData( @Override public boolean handles(@NonNull Uri model) { - Set schemesToCheck = - isUseDirectResourceLoaderEnabled() ? SCHEMES : SCHEMES_WITH_RESOURCE; - return schemesToCheck.contains(model.getScheme()); - } - - private boolean isUseDirectResourceLoaderEnabled() { - return glideExperiments != null && glideExperiments.isEnabled(UseDirectResourceLoader.class); + return SCHEMES.contains(model.getScheme()); } /** @@ -88,21 +68,9 @@ public static class StreamFactory implements ModelLoaderFactory, LocalUriFetcherFactory { private final ContentResolver contentResolver; - @Nullable private final GlideExperiments glideExperiments; - - /** - * @deprecated This method is experimental and will be removed in a future version without - * warning. - */ - @Deprecated - public StreamFactory( - ContentResolver contentResolver, @Nullable GlideExperiments glideExperiments) { - this.contentResolver = contentResolver; - this.glideExperiments = glideExperiments; - } public StreamFactory(ContentResolver contentResolver) { - this(contentResolver, /* glideExperiments= */ null); + this.contentResolver = contentResolver; } @Override @@ -113,7 +81,7 @@ public DataFetcher build(Uri uri) { @NonNull @Override public ModelLoader build(MultiModelLoaderFactory multiFactory) { - return new UriLoader<>(this, glideExperiments); + return new UriLoader<>(this); } @Override @@ -128,21 +96,9 @@ public static class FileDescriptorFactory LocalUriFetcherFactory { private final ContentResolver contentResolver; - @Nullable private final GlideExperiments glideExperiments; - - /** - * @deprecated This method is experimental and will be removed in a future version without - * warning. - */ - @Deprecated - public FileDescriptorFactory( - ContentResolver contentResolver, @Nullable GlideExperiments glideExperiments) { - this.contentResolver = contentResolver; - this.glideExperiments = glideExperiments; - } public FileDescriptorFactory(ContentResolver contentResolver) { - this(contentResolver, /* glideExperiments= */ null); + this.contentResolver = contentResolver; } @Override @@ -153,7 +109,7 @@ public DataFetcher build(Uri uri) { @NonNull @Override public ModelLoader build(MultiModelLoaderFactory multiFactory) { - return new UriLoader<>(this, glideExperiments); + return new UriLoader<>(this); } @Override @@ -168,26 +124,14 @@ public static final class AssetFileDescriptorFactory LocalUriFetcherFactory { private final ContentResolver contentResolver; - @Nullable private final GlideExperiments glideExperiments; - - /** - * @deprecated This method is experimental and will be removed in a future version without - * warning. - */ - @Deprecated - public AssetFileDescriptorFactory( - ContentResolver contentResolver, @Nullable GlideExperiments glideExperiments) { - this.contentResolver = contentResolver; - this.glideExperiments = glideExperiments; - } public AssetFileDescriptorFactory(ContentResolver contentResolver) { - this(contentResolver, /* glideExperiments= */ null); + this.contentResolver = contentResolver; } @Override public ModelLoader build(MultiModelLoaderFactory multiFactory) { - return new UriLoader<>(this, glideExperiments); + return new UriLoader<>(this); } @Override diff --git a/library/test/src/test/java/com/bumptech/glide/load/model/UriLoaderTest.java b/library/test/src/test/java/com/bumptech/glide/load/model/UriLoaderTest.java index 0568ef091a..3766c6dba2 100644 --- a/library/test/src/test/java/com/bumptech/glide/load/model/UriLoaderTest.java +++ b/library/test/src/test/java/com/bumptech/glide/load/model/UriLoaderTest.java @@ -51,19 +51,6 @@ public void testHandlesFileUris() throws IOException { .fetcher); } - @Test - public void testHandlesResourceUris() throws IOException { - Uri resourceUri = Uri.parse("android.resource://com.bumptech.glide.tests/raw/ic_launcher"); - when(factory.build(eq(resourceUri))).thenReturn(localUriFetcher); - - assertTrue(loader.handles(resourceUri)); - assertEquals( - localUriFetcher, - Preconditions.checkNotNull( - loader.buildLoadData(resourceUri, IMAGE_SIDE, IMAGE_SIDE, options)) - .fetcher); - } - @Test public void testHandlesContentUris() { Uri contentUri = Uri.parse("content://com.bumptech.glide");