diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java index 94dd8e856e..80c5b9af7a 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java @@ -29,7 +29,6 @@ import com.bumptech.glide.test.ForceDarkOrLightModeActivity; import com.google.common.base.Function; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -40,7 +39,7 @@ public class DarkModeTest { @Rule public final IdlingGlideRule idlingGlideRule = - IdlingGlideRule.newGlideRule(glideBuilder -> glideBuilder); + IdlingGlideRule.newGlideRule(glideBuilder -> glideBuilder.useDirectResourceLoader(true)); @Before public void before() { @@ -62,7 +61,6 @@ public void before() { // when it's specified by the user. Otherwise whether or not we'd obey dark mode would depend on // the user also providing the theme from the activity. We'd want to try to make sure that doesn't // leak the Activity. - // TODO(judds): Add tests for Fragments for load(). @Test public void load_withDarkModeActivity_usesLightModeDrawable() { runActivityTest( @@ -95,7 +93,6 @@ public void load_withLightModeFragment_usesLightModeDrawable() { fragment -> Glide.with(fragment).load(R.drawable.dog).override(Target.SIZE_ORIGINAL)); } - @Ignore("We do not asynchronously load resources correctly") @Test public void load_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() { runActivityTest( @@ -108,7 +105,6 @@ public void load_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() { .theme(activity.getTheme())); } - @Ignore("We do not asynchronously load resources correctly") @Test public void load_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() { runFragmentTest( @@ -121,6 +117,30 @@ public void load_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() { .theme(fragment.requireActivity().getTheme())); } + @Test + public void load_withApplicationContext_darkTheme_usesDarkModeDrawable() { + runActivityTest( + darkModeActivity(), + R.raw.dog_dark, + input -> + Glide.with(input.getApplicationContext()) + .load(R.drawable.dog) + .override(Target.SIZE_ORIGINAL) + .theme(input.getTheme())); + } + + @Test + public void load_withApplicationContext_lightTheme_usesLightModeDrawable() { + runActivityTest( + lightModeActivity(), + R.raw.dog_light, + input -> + Glide.with(input.getApplicationContext()) + .load(R.drawable.dog) + .override(Target.SIZE_ORIGINAL) + .theme(input.getTheme())); + } + @Test public void load_withLightModeActivity_lightModeTheme_usesLightModeDrawable() { runActivityTest( diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index da94a046f8..c0edb38fd8 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/library/src/main/java/com/bumptech/glide/GlideBuilder.java @@ -499,6 +499,23 @@ public GlideBuilder useLifecycleInsteadOfInjectingFragments(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; } @@ -621,4 +638,11 @@ public static final class LogRequestOrigins implements Experiment {} * and activities. */ public static final class UseLifecycleInsteadOfInjectingFragments implements Experiment {} + + /** + * Use {@link com.bumptech.glide.load.model.DirectResourceLoader} instead of {@link + * com.bumptech.glide.load.model.ResourceLoader} so that we load resources asynchronously with the + * correct theme. + */ + public static final class UseDirectResourceLoader implements Experiment {} } diff --git a/library/src/main/java/com/bumptech/glide/RegistryFactory.java b/library/src/main/java/com/bumptech/glide/RegistryFactory.java index 736ab6ec08..da6e657e30 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/library/src/main/java/com/bumptech/glide/RegistryFactory.java @@ -13,6 +13,7 @@ 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; @@ -25,9 +26,11 @@ import com.bumptech.glide.load.model.ByteBufferEncoder; import com.bumptech.glide.load.model.ByteBufferFileLoader; import com.bumptech.glide.load.model.DataUrlLoader; +import com.bumptech.glide.load.model.DirectResourceLoader; import com.bumptech.glide.load.model.FileLoader; import com.bumptech.glide.load.model.GlideUrl; 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.StreamEncoder; import com.bumptech.glide.load.model.StringLoader; @@ -177,13 +180,7 @@ private static void initializeDefaults( } ResourceDrawableDecoder resourceDrawableDecoder = new ResourceDrawableDecoder(context); - 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); + BitmapEncoder bitmapEncoder = new BitmapEncoder(arrayPool); BitmapBytesTranscoder bitmapBytesTranscoder = new BitmapBytesTranscoder(); @@ -274,15 +271,38 @@ 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); + 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); + } 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(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); + } + registry - .append(int.class, InputStream.class, resourceLoaderStreamFactory) - .append(int.class, ParcelFileDescriptor.class, resourceLoaderFileDescriptorFactory) - .append(Integer.class, InputStream.class, resourceLoaderStreamFactory) - .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) .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 new file mode 100644 index 0000000000..b01e6e532a --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/load/model/DirectResourceLoader.java @@ -0,0 +1,189 @@ +package com.bumptech.glide.load.model; + +import android.content.Context; +import android.content.res.AssetFileDescriptor; +import android.content.res.Resources; +import android.content.res.Resources.Theme; +import android.os.Build; +import android.os.Build.VERSION_CODES; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import com.bumptech.glide.Priority; +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.ResourceDrawableDecoder; +import com.bumptech.glide.signature.ObjectKey; +import java.io.Closeable; +import java.io.IOException; +import java.io.InputStream; + +/** + * Loads themed resource ids using {@link Resources#openRawResource(int)} or {@link + * Resources#openRawResourceFd(int)} using the theme from {@link ResourceDrawableDecoder#THEME} when + * it's available. + * + * @param The type of data this {@code ModelLoader} will produce (e.g. {@link InputStream}, + * {@link AssetFileDescriptor} etc). + */ +public final class DirectResourceLoader + implements ModelLoader { + + private final Context context; + private final ResourceOpener resourceOpener; + + public static ModelLoaderFactory inputStreamFactory(Context context) { + return new InputStreamFactory(context); + } + + public static ModelLoaderFactory assetFileDescriptorFactory( + Context context) { + return new AssetFileDescriptorFactory(context); + } + + DirectResourceLoader(Context context, ResourceOpener resourceOpener) { + this.context = context.getApplicationContext(); + this.resourceOpener = resourceOpener; + } + + @Override + public LoadData buildLoadData( + @NonNull Integer resourceId, int width, int height, @NonNull Options options) { + Theme theme = options.get(ResourceDrawableDecoder.THEME); + Resources resources = + Build.VERSION.SDK_INT >= VERSION_CODES.LOLLIPOP && theme != null + ? theme.getResources() + : context.getResources(); + return new LoadData<>( + // TODO(judds): We try to apply AndroidResourceSignature for caching in RequestBuilder. + // Arguably we should mix in that information here instead. + new ObjectKey(resourceId), + new ResourceDataFetcher<>(resources, resourceOpener, resourceId)); + } + + @Override + public boolean handles(@NonNull Integer integer) { + // We could check that this is in fact a resource ID, but doing so isn't free and in practice + // it doesn't seem to have been an issue historically. + return true; + } + + private interface ResourceOpener { + DataT open(Resources resources, int resourceId); + + Class getDataClass(); + } + + private static final class AssetFileDescriptorFactory + implements ModelLoaderFactory, + ResourceOpener { + + private final Context context; + + AssetFileDescriptorFactory(Context context) { + this.context = context; + } + + @Override + public AssetFileDescriptor open(Resources resources, int resourceId) { + return resources.openRawResourceFd(resourceId); + } + + @Override + public Class getDataClass() { + return AssetFileDescriptor.class; + } + + @NonNull + @Override + public ModelLoader build( + @NonNull MultiModelLoaderFactory multiFactory) { + return new DirectResourceLoader<>(context, this); + } + + @Override + public void teardown() {} + } + + private static final class InputStreamFactory + implements ModelLoaderFactory, ResourceOpener { + + private final Context context; + + InputStreamFactory(Context context) { + this.context = context; + } + + @NonNull + @Override + public ModelLoader build(@NonNull MultiModelLoaderFactory multiFactory) { + return new DirectResourceLoader<>(context, this); + } + + @Override + public InputStream open(Resources resources, int resourceId) { + return resources.openRawResource(resourceId); + } + + @Override + public Class getDataClass() { + return InputStream.class; + } + + @Override + public void teardown() {} + } + + private static final class ResourceDataFetcher + implements DataFetcher { + + private final Resources resources; + private final ResourceOpener resourceOpener; + private final int resourceId; + private @Nullable DataT data; + + ResourceDataFetcher(Resources resources, ResourceOpener resourceOpener, int resourceId) { + this.resources = resources; + this.resourceOpener = resourceOpener; + this.resourceId = resourceId; + } + + @Override + public void loadData( + @NonNull Priority priority, @NonNull DataCallback callback) { + try { + data = resourceOpener.open(resources, resourceId); + callback.onDataReady(data); + } catch (Resources.NotFoundException e) { + callback.onLoadFailed(e); + } + } + + @Override + public void cleanup() { + DataT local = data; + if (local != null) { + try { + local.close(); + } catch (IOException e) { + // Ignored. + } + } + } + + @Override + public void cancel() {} + + @NonNull + @Override + public Class getDataClass() { + return resourceOpener.getDataClass(); + } + + @NonNull + @Override + public DataSource getDataSource() { + return DataSource.LOCAL; + } + } +}