diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java index 80c5b9af7a..86781acf0d 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java @@ -4,12 +4,15 @@ import static com.bumptech.glide.testutil.BitmapSubject.assertThat; import static org.junit.Assume.assumeTrue; +import android.content.ContentResolver; import android.content.Context; +import android.content.res.Resources; import android.graphics.Bitmap; import android.graphics.drawable.BitmapDrawable; import android.graphics.drawable.Drawable; import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; +import android.net.Uri; import android.os.Bundle; import android.view.LayoutInflater; import android.view.View; @@ -47,7 +50,7 @@ public void before() { assumeTrue(VERSION.SDK_INT >= VERSION_CODES.Q); } - // TODO(judds): The way we handle data loads in the background for resoures is not Theme + // TODO(judds): The way we handle data loads in the background for resources is not Theme // compatible. In particular, the theme gets lost when we convert the resource id to a Uri and // we don't use the user provided theme. While ResourceBitmapDecoder and ResourceDrawableDecoder // will use the theme, they're not called for most resource ids because those instead go through @@ -105,6 +108,50 @@ public void load_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() { .theme(activity.getTheme())); } + @Test + public void loadResourceNameUri_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() { + runActivityTest( + darkModeActivity(), + R.raw.dog_dark, + activity -> + Glide.with(activity) + .load(newResourceNameUri(activity, R.drawable.dog)) + .override(Target.SIZE_ORIGINAL) + .theme(activity.getTheme())); + } + + @Test + public void loadResourceIdUri_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() { + runActivityTest( + darkModeActivity(), + R.raw.dog_dark, + activity -> + Glide.with(activity) + .load(newResourceIdUri(activity, R.drawable.dog)) + .override(Target.SIZE_ORIGINAL) + .theme(activity.getTheme())); + } + + private static Uri newResourceNameUri(Context context, int resourceId) { + Resources resources = context.getResources(); + return newResourceUriBuilder(context) + .appendPath(resources.getResourceTypeName(resourceId)) + .appendPath(resources.getResourceEntryName(resourceId)) + .build(); + } + + private static Uri newResourceIdUri(Context context, int resourceId) { + return newResourceUriBuilder(context) + .appendPath(String.valueOf(resourceId)) + .build(); + } + + private static Uri.Builder newResourceUriBuilder(Context context) { + return new Uri.Builder() + .scheme(ContentResolver.SCHEME_ANDROID_RESOURCE) + .authority(context.getPackageName()); + } + @Test public void load_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() { runFragmentTest( @@ -117,6 +164,30 @@ public void load_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() { .theme(fragment.requireActivity().getTheme())); } + @Test + public void loadResourceNameUri_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() { + runFragmentTest( + darkModeActivity(), + R.raw.dog_dark, + fragment -> + Glide.with(fragment) + .load(newResourceNameUri(fragment.requireContext(), R.drawable.dog)) + .override(Target.SIZE_ORIGINAL) + .theme(fragment.requireActivity().getTheme())); + } + + @Test + public void loadResourceIdUri_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() { + runFragmentTest( + darkModeActivity(), + R.raw.dog_dark, + fragment -> + Glide.with(fragment) + .load(newResourceIdUri(fragment.requireContext(), R.drawable.dog)) + .override(Target.SIZE_ORIGINAL) + .theme(fragment.requireActivity().getTheme())); + } + @Test public void load_withApplicationContext_darkTheme_usesDarkModeDrawable() { runActivityTest( @@ -129,6 +200,30 @@ public void load_withApplicationContext_darkTheme_usesDarkModeDrawable() { .theme(input.getTheme())); } + @Test + public void loadResourceNameUri_withApplicationContext_darkTheme_usesDarkModeDrawable() { + runActivityTest( + darkModeActivity(), + R.raw.dog_dark, + input -> + Glide.with(input.getApplicationContext()) + .load(newResourceNameUri(input.getApplicationContext(), R.drawable.dog)) + .override(Target.SIZE_ORIGINAL) + .theme(input.getTheme())); + } + + @Test + public void loadResourceIdUri_withApplicationContext_darkTheme_usesDarkModeDrawable() { + runActivityTest( + darkModeActivity(), + R.raw.dog_dark, + input -> + Glide.with(input.getApplicationContext()) + .load(newResourceIdUri(input.getApplicationContext(), R.drawable.dog)) + .override(Target.SIZE_ORIGINAL) + .theme(input.getTheme())); + } + @Test public void load_withApplicationContext_lightTheme_usesLightModeDrawable() { runActivityTest( diff --git a/library/src/main/java/com/bumptech/glide/RegistryFactory.java b/library/src/main/java/com/bumptech/glide/RegistryFactory.java index da6e657e30..a8b7462db5 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/library/src/main/java/com/bumptech/glide/RegistryFactory.java @@ -32,6 +32,7 @@ import com.bumptech.glide.load.model.MediaStoreFileLoader; import com.bumptech.glide.load.model.ModelLoaderFactory; import com.bumptech.glide.load.model.ResourceLoader; +import com.bumptech.glide.load.model.ResourceUriLoader; import com.bumptech.glide.load.model.StreamEncoder; import com.bumptech.glide.load.model.StringLoader; import com.bumptech.glide.load.model.UnitModelLoader; @@ -280,7 +281,12 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm .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(Integer.class, AssetFileDescriptor.class, assetFileDescriptorFactory) + .append(Uri.class, InputStream.class, ResourceUriLoader.newStreamFactory(context)) + .append( + Uri.class, + AssetFileDescriptor.class, + ResourceUriLoader.newAssetFileDescriptorFactory(context)); } else { ResourceLoader.StreamFactory resourceLoaderStreamFactory = new ResourceLoader.StreamFactory(resources); @@ -325,15 +331,16 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm new QMediaStoreUriLoader.FileDescriptorFactory(context)); } registry - .append(Uri.class, InputStream.class, new UriLoader.StreamFactory(contentResolver)) + .append( + Uri.class, InputStream.class, new UriLoader.StreamFactory(contentResolver, experiments)) .append( Uri.class, ParcelFileDescriptor.class, - new UriLoader.FileDescriptorFactory(contentResolver)) + new UriLoader.FileDescriptorFactory(contentResolver, experiments)) .append( Uri.class, AssetFileDescriptor.class, - new UriLoader.AssetFileDescriptorFactory(contentResolver)) + new UriLoader.AssetFileDescriptorFactory(contentResolver, experiments)) .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/ResourceUriLoader.java b/library/src/main/java/com/bumptech/glide/load/model/ResourceUriLoader.java new file mode 100644 index 0000000000..4e387a9098 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/load/model/ResourceUriLoader.java @@ -0,0 +1,150 @@ +package com.bumptech.glide.load.model; + +import android.annotation.SuppressLint; +import android.content.ContentResolver; +import android.content.Context; +import android.content.res.AssetFileDescriptor; +import android.net.Uri; +import android.util.Log; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import com.bumptech.glide.load.Options; +import java.io.InputStream; +import java.util.List; + +/** + * Converts Resource Uris to resource ids if the resource Uri points to a resource in this package. + * + *

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. + * + * @param The type of data produced, e.g. {@link InputStream} or + * {@link AssetFileDescriptor}. + */ +public final class ResourceUriLoader implements ModelLoader { + /** + * See the javadoc on {@link + * android.content.res.Resources#getIdentifier(java.lang.String, java.lang.String, java.lang.String)}. + */ + private static final int INVALID_RESOURCE_ID = 0; + private static final String TAG = "ResourceUriLoader"; + + private final Context context; + private final ModelLoader delegate; + + public static ModelLoaderFactory newStreamFactory(Context context) { + return new InputStreamFactory(context); + } + + public static ModelLoaderFactory newAssetFileDescriptorFactory( + Context context) { + return new AssetFileDescriptorFactory(context); + } + + ResourceUriLoader(Context context, ModelLoader delegate) { + this.context = context.getApplicationContext(); + this.delegate = delegate; + } + + @Nullable + @Override + public LoadData buildLoadData(@NonNull Uri uri, int width, int height, @NonNull Options options) { + List pathSegments = uri.getPathSegments(); + // android.resource/// + if (pathSegments.size() == 1) { + return parseResourceIdUri(uri, width, height, options); + } + // android.resource//// + if (pathSegments.size() == 2) { + return parseResourceNameUri(uri, width, height, options); + } + if (Log.isLoggable(TAG, Log.WARN)) { + Log.w(TAG, "Failed to parse resource uri: " + uri); + } + return null; + } + + @Nullable + private LoadData parseResourceNameUri( + @NonNull Uri uri, int width, int height, @NonNull Options options) { + List pathSegments = uri.getPathSegments(); + String resourceType = pathSegments.get(0); + String resourceName = pathSegments.get(1); + + // Yes it's bad, but the caller has chosen to give us a resource uri... + @SuppressLint("DiscouragedApi") int identifier = + context.getResources().getIdentifier(resourceName, resourceType, context.getPackageName()); + if (identifier == INVALID_RESOURCE_ID) { + if (Log.isLoggable(TAG, Log.WARN)) { + Log.w(TAG, "Failed to find resource id for: " + uri); + } + return null; + } + + return delegate.buildLoadData(identifier, width, height, options); + } + + @Nullable + private LoadData parseResourceIdUri( + @NonNull Uri uri, int width, int height, @NonNull Options options) { + try { + int resourceId = Integer.parseInt(uri.getPathSegments().get(0)); + if (resourceId == INVALID_RESOURCE_ID) { + if (Log.isLoggable(TAG, Log.WARN)) { + Log.w(TAG, "Failed to parse a valid non-0 resource id from: " + uri); + } + return null; + } + return delegate.buildLoadData(resourceId, width, height, options); + } catch (NumberFormatException e) { + if (Log.isLoggable(TAG, Log.WARN)) { + Log.w(TAG, "Failed to parse resource id from: " + uri ,e); + } + } + return null; + } + + @Override + public boolean handles(@NonNull Uri uri) { + return ContentResolver.SCHEME_ANDROID_RESOURCE.equals(uri.getScheme()) + && context.getPackageName().equals(uri.getAuthority()); + } + + private static final class InputStreamFactory implements ModelLoaderFactory { + + private final Context context; + + InputStreamFactory(Context context) { + this.context = context; + } + + @NonNull + @Override + public ModelLoader build(@NonNull MultiModelLoaderFactory multiFactory) { + return new ResourceUriLoader<>(context, multiFactory.build(Integer.class, InputStream.class)); + } + + @Override public void teardown() {} + } + + private static final class AssetFileDescriptorFactory implements ModelLoaderFactory { + + private final Context context; + + AssetFileDescriptorFactory(Context context) { + this.context = context; + } + + @NonNull + @Override + public ModelLoader build( + @NonNull MultiModelLoaderFactory multiFactory) { + return new ResourceUriLoader<>( + context, multiFactory.build(Integer.class, AssetFileDescriptor.class)); + } + + @Override public void teardown() {} + } +} 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 543ee9dbc2..04833ac6f1 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,6 +5,9 @@ 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; @@ -26,7 +29,8 @@ * @param The type of data that will be retrieved for {@link android.net.Uri}s. */ public class UriLoader implements ModelLoader { - private static final Set SCHEMES = + + private static final Set SCHEMES_WITH_RESOURCE = Collections.unmodifiableSet( new HashSet<>( Arrays.asList( @@ -34,12 +38,27 @@ public class UriLoader implements ModelLoader { ContentResolver.SCHEME_ANDROID_RESOURCE, ContentResolver.SCHEME_CONTENT))); + + private static final Set SCHEMES = + Collections.unmodifiableSet( + new HashSet<>( + Arrays.asList( + ContentResolver.SCHEME_FILE, + ContentResolver.SCHEME_CONTENT))); + 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 = factory; + this(factory, /* glideExperiments= */ null); } @Override @@ -50,7 +69,13 @@ public LoadData buildLoadData( @Override public boolean handles(@NonNull Uri model) { - return SCHEMES.contains(model.getScheme()); + Set schemesToCheck = + isUseDirectResourceLoaderEnabled() ? SCHEMES : SCHEMES_WITH_RESOURCE; + return schemesToCheck.contains(model.getScheme()); + } + + private boolean isUseDirectResourceLoaderEnabled() { + return glideExperiments != null && glideExperiments.isEnabled(UseDirectResourceLoader.class); } /** @@ -67,9 +92,22 @@ 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 = contentResolver; + this(contentResolver, /* glideExperiments= */ null); } @Override @@ -80,7 +118,7 @@ public DataFetcher build(Uri uri) { @NonNull @Override public ModelLoader build(MultiModelLoaderFactory multiFactory) { - return new UriLoader<>(this); + return new UriLoader<>(this, glideExperiments); } @Override @@ -95,9 +133,21 @@ 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 = contentResolver; + this(contentResolver, /* glideExperiments= */ null); } @Override @@ -108,7 +158,7 @@ public DataFetcher build(Uri uri) { @NonNull @Override public ModelLoader build(MultiModelLoaderFactory multiFactory) { - return new UriLoader<>(this); + return new UriLoader<>(this, glideExperiments); } @Override @@ -123,14 +173,27 @@ 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 = contentResolver; + this(contentResolver, /* glideExperiments= */ null); } @Override public ModelLoader build(MultiModelLoaderFactory multiFactory) { - return new UriLoader<>(this); + return new UriLoader<>(this, glideExperiments); } @Override