Skip to content

Commit

Permalink
Merge pull request #4972 from sjudd:experimental_model_loader_resources
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 495694663
  • Loading branch information
glide-copybara-robot committed Dec 15, 2022
2 parents 472717c + 04f198e commit 5816903
Show file tree
Hide file tree
Showing 4 changed files with 273 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
24 changes: 24 additions & 0 deletions library/src/main/java/com/bumptech/glide/GlideBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*
* <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;
}
Expand Down Expand Up @@ -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 {}
}
50 changes: 35 additions & 15 deletions library/src/main/java/com/bumptech/glide/RegistryFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -274,15 +271,38 @@ 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);
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<String>())
.append(Uri.class, InputStream.class, new DataUrlLoader.StreamFactory<Uri>())
.append(String.class, InputStream.class, new StringLoader.StreamFactory())
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <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> {

private final Context context;
private final ResourceOpener<DataT> resourceOpener;

public static ModelLoaderFactory<Integer, InputStream> inputStreamFactory(Context context) {
return new InputStreamFactory(context);
}

public static ModelLoaderFactory<Integer, AssetFileDescriptor> assetFileDescriptorFactory(
Context context) {
return new AssetFileDescriptorFactory(context);
}

DirectResourceLoader(Context context, ResourceOpener<DataT> resourceOpener) {
this.context = context.getApplicationContext();
this.resourceOpener = resourceOpener;
}

@Override
public LoadData<DataT> 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> {
DataT open(Resources resources, int resourceId);

Class<DataT> getDataClass();
}

private static final class AssetFileDescriptorFactory
implements ModelLoaderFactory<Integer, AssetFileDescriptor>,
ResourceOpener<AssetFileDescriptor> {

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<AssetFileDescriptor> getDataClass() {
return AssetFileDescriptor.class;
}

@NonNull
@Override
public ModelLoader<Integer, AssetFileDescriptor> build(
@NonNull MultiModelLoaderFactory multiFactory) {
return new DirectResourceLoader<>(context, this);
}

@Override
public void teardown() {}
}

private static final class InputStreamFactory
implements ModelLoaderFactory<Integer, InputStream>, ResourceOpener<InputStream> {

private final Context context;

InputStreamFactory(Context context) {
this.context = context;
}

@NonNull
@Override
public ModelLoader<Integer, InputStream> build(@NonNull MultiModelLoaderFactory multiFactory) {
return new DirectResourceLoader<>(context, this);
}

@Override
public InputStream open(Resources resources, int resourceId) {
return resources.openRawResource(resourceId);
}

@Override
public Class<InputStream> getDataClass() {
return InputStream.class;
}

@Override
public void teardown() {}
}

private static final class ResourceDataFetcher<DataT extends Closeable>
implements DataFetcher<DataT> {

private final Resources resources;
private final ResourceOpener<DataT> resourceOpener;
private final int resourceId;
private @Nullable DataT data;

ResourceDataFetcher(Resources resources, ResourceOpener<DataT> resourceOpener, int resourceId) {
this.resources = resources;
this.resourceOpener = resourceOpener;
this.resourceId = resourceId;
}

@Override
public void loadData(
@NonNull Priority priority, @NonNull DataCallback<? super DataT> 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<DataT> getDataClass() {
return resourceOpener.getDataClass();
}

@NonNull
@Override
public DataSource getDataSource() {
return DataSource.LOCAL;
}
}
}

0 comments on commit 5816903

Please sign in to comment.