Skip to content

Commit

Permalink
Fix vector/shape drawables not loading with DirectResourceLoader.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 500212780
  • Loading branch information
sjudd authored and glide-copybara-robot committed Jan 6, 2023
1 parent 4affb8d commit 250ac36
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import android.content.Context;
import android.content.res.Resources;
import android.graphics.Bitmap;
import android.graphics.Canvas;
import android.graphics.drawable.BitmapDrawable;
import android.graphics.drawable.Drawable;
import android.net.Uri;
Expand Down Expand Up @@ -51,6 +52,45 @@ public void before() {
assumeTrue(VERSION.SDK_INT >= VERSION_CODES.Q);
}

@Test
public void load_withDarkModeActivity_vectorDrawable_usesDarkModeColor() {
try (ActivityScenario<FragmentActivity> scenario = darkModeActivity()) {
scenario.onActivity(
activity -> {
ViewGroup container = findContainer(activity);
ImageView imageView = newFixedSizeImageView(activity);
container.addView(imageView);

Glide.with(activity)
.load(R.drawable.vector_drawable)
.override(Target.SIZE_ORIGINAL)
.into(imageView);
});

onIdle();
scenario.onActivity(
activity -> {
ViewGroup container = findContainer(activity);
ImageView imageView = (ImageView) container.getChildAt(0);
Bitmap result = drawToBitmap(imageView.getDrawable());
Bitmap expected = drawToBitmap(activity.getDrawable(R.drawable.vector_drawable_dark));
assertThat(result).sameAs(expected);
});
}
}

private static Bitmap drawToBitmap(Drawable drawable) {
int width = drawable.getIntrinsicWidth();
int height = drawable.getIntrinsicHeight();

Bitmap result = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888);
Canvas canvas = new Canvas(result);
drawable.setBounds(0, 0, width, height);
drawable.draw(canvas);
canvas.setBitmap(null);
return result;
}

@Test
public void load_withDarkModeActivity_useDarkModeDrawable() {
runActivityTest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
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;
Expand All @@ -31,19 +31,29 @@
import org.junit.function.ThrowingRunnable;
import org.junit.rules.TestName;
import org.junit.runner.RunWith;
import org.mockito.MockitoAnnotations;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameter;
import org.junit.runners.Parameterized.Parameters;

@RunWith(AndroidJUnit4.class)
@RunWith(Parameterized.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() {
MockitoAnnotations.initMocks(this);
context = ApplicationProvider.getApplicationContext();

Glide.init(context, new GlideBuilder().useDirectResourceLoader(useDirectResourceLoader));
}

@Test
Expand Down
4 changes: 3 additions & 1 deletion instrumentation/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
package="com.bumptech.glide.instrumentation">
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW" />
<application tools:ignore="MissingApplicationIcon" >
<application
tools:ignore="MissingApplicationIcon"
android:theme="@style/AppTheme">
<activity
android:name="com.bumptech.glide.test.GlideWithBeforeSuperOnCreateActivity"
android:exported="false" />
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/src/main/res/drawable/vector_drawable.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
android:viewportHeight="24.0"
android:viewportWidth="24.0">
<path
android:fillColor="#f9b840"
android:fillColor="?attr/colorControlNormal"
android:pathData="M15.5,5.5c1.1,0 2,-0.9 2,-2s-0.9,-2 -2,-2 -2,0.9
-2,2 0.9,2 2,2zM5,12c-2.8,0 -5,2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,
-5 -5,-5zM5,20.5c-1.9,0 -3.5,-1.6 -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,
Expand Down
18 changes: 18 additions & 0 deletions instrumentation/src/main/res/drawable/vector_drawable_dark.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="64dp"
android:height="64dp"
android:viewportHeight="24.0"
android:viewportWidth="24.0">
<path
android:fillColor="@color/colorPrimaryDark"
android:pathData="M15.5,5.5c1.1,0 2,-0.9 2,-2s-0.9,-2 -2,-2 -2,0.9
-2,2 0.9,2 2,2zM5,12c-2.8,0 -5,2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,
-5 -5,-5zM5,20.5c-1.9,0 -3.5,-1.6 -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,
1.6 3.5,3.5 -1.6,3.5 -3.5,3.5zM10.8,10.5l2.4,-2.4 0.8,0.8c1.3,1.3
3,2.1 5.1,2.1L19.1,9c-1.5,0 -2.7,-0.6 -3.6,-1.5l-1.9,-1.9c-0.5,-0.4
-1,-0.6 -1.6,-0.6s-1.1,0.2 -1.4,0.6L7.8,8.4c-0.4,0.4 -0.6,0.9 -0.6,
1.4 0,0.6 0.2,1.1 0.6,1.4L11,14v5h2v-6.2l-2.2,-2.3zM19,12c-2.8,0 -5,
2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,-5 -5,-5zM19,20.5c-1.9,0 -3.5,-1.6
-3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,1.6 3.5,3.5 -1.6,3.5 -3.5,3.5z" />
</vector>
18 changes: 18 additions & 0 deletions instrumentation/src/main/res/drawable/vector_drawable_light.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="64dp"
android:height="64dp"
android:viewportHeight="24.0"
android:viewportWidth="24.0">
<path
android:fillColor="@color/colorPrimary"
android:pathData="M15.5,5.5c1.1,0 2,-0.9 2,-2s-0.9,-2 -2,-2 -2,0.9
-2,2 0.9,2 2,2zM5,12c-2.8,0 -5,2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,
-5 -5,-5zM5,20.5c-1.9,0 -3.5,-1.6 -3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,
1.6 3.5,3.5 -1.6,3.5 -3.5,3.5zM10.8,10.5l2.4,-2.4 0.8,0.8c1.3,1.3
3,2.1 5.1,2.1L19.1,9c-1.5,0 -2.7,-0.6 -3.6,-1.5l-1.9,-1.9c-0.5,-0.4
-1,-0.6 -1.6,-0.6s-1.1,0.2 -1.4,0.6L7.8,8.4c-0.4,0.4 -0.6,0.9 -0.6,
1.4 0,0.6 0.2,1.1 0.6,1.4L11,14v5h2v-6.2l-2.2,-2.3zM19,12c-2.8,0 -5,
2.2 -5,5s2.2,5 5,5 5,-2.2 5,-5 -2.2,-5 -5,-5zM19,20.5c-1.9,0 -3.5,-1.6
-3.5,-3.5s1.6,-3.5 3.5,-3.5 3.5,1.6 3.5,3.5 -1.6,3.5 -3.5,3.5z" />
</vector>
6 changes: 6 additions & 0 deletions instrumentation/src/main/res/values/colors.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>

<resources>
<color name="colorPrimary">#f9b840</color>
<color name="colorPrimaryDark">#ffffff</color>
</resources>
8 changes: 8 additions & 0 deletions instrumentation/src/main/res/values/styles.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<style name="AppTheme" parent="Theme.AppCompat.DayNight.DarkActionBar">
<!-- Customize your theme here. -->
<item name="colorPrimary">@color/colorPrimary</item>
<item name="colorPrimaryDark">@color/colorPrimaryDark</item>
</style>
</resources>
15 changes: 10 additions & 5 deletions library/src/main/java/com/bumptech/glide/RegistryFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,15 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
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,
Expand All @@ -292,25 +296,26 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
} 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(int.class, ParcelFileDescriptor.class, resourceLoaderFileDescriptorFactory)
.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);
Integer.class, AssetFileDescriptor.class, resourceLoaderAssetFileDescriptorFactory);
}

// Handles resources from other applications or converting Drawable resource types to Bitmaps.
ResourceLoader.UriFactory resourceLoaderUriFactory = new ResourceLoader.UriFactory(resources);
registry
.append(Integer.class, Uri.class, resourceLoaderUriFactory)
.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
Expand Up @@ -4,6 +4,7 @@
import android.content.res.AssetFileDescriptor;
import android.content.res.Resources;
import android.content.res.Resources.Theme;
import android.graphics.drawable.Drawable;
import android.os.Build;
import android.os.Build.VERSION_CODES;
import androidx.annotation.NonNull;
Expand All @@ -12,9 +13,9 @@
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.DrawableDecoderCompat;
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;

Expand All @@ -26,8 +27,7 @@
* @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> {
public final class DirectResourceLoader<DataT> implements ModelLoader<Integer, DataT> {

private final Context context;
private final ResourceOpener<DataT> resourceOpener;
Expand All @@ -41,6 +41,10 @@ public static ModelLoaderFactory<Integer, AssetFileDescriptor> assetFileDescript
return new AssetFileDescriptorFactory(context);
}

public static ModelLoaderFactory<Integer, Drawable> drawableFactory(Context context) {
return new DrawableFactory(context);
}

DirectResourceLoader(Context context, ResourceOpener<DataT> resourceOpener) {
this.context = context.getApplicationContext();
this.resourceOpener = resourceOpener;
Expand Down Expand Up @@ -71,6 +75,8 @@ public boolean handles(@NonNull Integer integer) {
private interface ResourceOpener<DataT> {
DataT open(Resources resources, int resourceId);

void close(DataT data) throws IOException;

Class<DataT> getDataClass();
}

Expand All @@ -89,6 +95,11 @@ public AssetFileDescriptor open(Resources resources, int resourceId) {
return resources.openRawResourceFd(resourceId);
}

@Override
public void close(AssetFileDescriptor data) throws IOException {
data.close();
}

@Override
public Class<AssetFileDescriptor> getDataClass() {
return AssetFileDescriptor.class;
Expand Down Expand Up @@ -125,6 +136,11 @@ public InputStream open(Resources resources, int resourceId) {
return resources.openRawResource(resourceId);
}

@Override
public void close(InputStream data) throws IOException {
data.close();
}

@Override
public Class<InputStream> getDataClass() {
return InputStream.class;
Expand All @@ -134,8 +150,45 @@ public Class<InputStream> getDataClass() {
public void teardown() {}
}

private static final class ResourceDataFetcher<DataT extends Closeable>
implements DataFetcher<DataT> {
/**
* Handles vectors, shapes and other resources that cannot be opened with
* Resources.openRawResource. Overlaps in functionality with {@link ResourceDrawableDecoder} and
* {@link com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder} but it's more efficient
* for simple resource loads within a single application.
*/
private static final class DrawableFactory
implements ModelLoaderFactory<Integer, Drawable>, ResourceOpener<Drawable> {

private final Context context;

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

@Override
public Drawable open(Resources resources, int resourceId) {
return DrawableDecoderCompat.getDrawable(context, resourceId, resources.newTheme());
}

@Override
public void close(Drawable data) throws IOException {}

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

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

@Override
public void teardown() {}
}

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

private final Resources resources;
private final ResourceOpener<DataT> resourceOpener;
Expand Down Expand Up @@ -164,7 +217,7 @@ public void cleanup() {
DataT local = data;
if (local != null) {
try {
local.close();
resourceOpener.close(local);
} catch (IOException e) {
// Ignored.
}
Expand Down

0 comments on commit 250ac36

Please sign in to comment.