facebook / fresco

An Android library for managing images and the memory they use.
https://frescolib.org/
MIT License
17.07k stars 3.75k forks source link

Vector drawable does not work as placeholder image on API 17 (and probably below API 21) #1779

Open gregkorossy opened 7 years ago

gregkorossy commented 7 years ago

Description

I have a SimpleDraweeView and a vector drawable XML. I tried to use the said XML as a placeholder in the Drawee but on API 17 (so probably below API 21) could not inflate the view and crashed the app.

AppCompatDelegate.setCompatVectorFromResourcesEnabled(true) does not seem to be a viable option as

This feature defaults to disabled, since enabling it can cause issues with memory usage, and problems updating Configuration instances.

Reproduction

Define the layout XML for the Drawee:

 <com.facebook.drawee.view.SimpleDraweeView
    android:id="@+id/cover"
    android:layout_width="60dp"
    android:layout_height="87dp"
    android:layout_marginEnd="12dp"
    android:layout_marginRight="12dp"
    app:imageUri="@{item.cover}"
    fresco:actualImageScaleType="centerCrop"
    fresco:placeholderImage="@drawable/ic_image_white_24dp"
    fresco:placeholderImageScaleType="centerInside"/>

where @drawable/ic_image_white_24dp is ic_image_white_24dp.xml:

<vector xmlns:android="http://schemas.android.com/apk/res/android"
        android:width="24dp"
        android:height="24dp"
        android:viewportWidth="24.0"
        android:viewportHeight="24.0">
    <path
        android:fillColor="#FFFFFFFF"
        android:pathData="M21,19V5c0,-1.1 -0.9,-2 -2,-2H5c-1.1,0 -2,0.9 -2,2v14c0,1.1 0.9,2 2,2h14c1.1,0 2,-0.9 2,-2zM8.5,13.5l2.5,3.01L14.5,12l4.5,6H5l3.5,-4.5z"/>
</vector>

Note that the SimpleDraweeView has an app:imageUri attribute this is here because the data binding would not work otherwise, the implementation is pretty simple (it is not relevant to the bug though):

@BindingAdapter({"imageUri"})
public static void loadImage(SimpleDraweeView view, Uri imageUrl) {
    view.setImageURI(imageUrl);
}

The layout gets inflated by DataBindingUtil from a RecyclerView.Adapter located in a (support) Fragment that is attached to the parent AppCompatActivity.

The gradle file contains the necessary line for supporting vector drawables on lower API levels:

android {
    defaultConfig {
        vectorDrawables.useSupportLibrary = true
    }
}

Additional Information

lambdapioneer commented 7 years ago

Thanks @Gericop for the detailed issue. Can you also print the relevant error message output from logcat?

gregkorossy commented 7 years ago

Sure, here you go: error_log.txt

The most important part is this:

Caused by: org.xmlpull.v1.XmlPullParserException: Binary XML file line #1: invalid drawable tag vector
    at android.graphics.drawable.Drawable.createFromXmlInner(Drawable.java:881)
    at android.graphics.drawable.Drawable.createFromXml(Drawable.java:822)
    at android.content.res.Resources.loadDrawable(Resources.java:1995)
    ... 83 more

It's pretty easy to find the problem: the GenericDraweeHierarchyInflater class' getDrawable(...) method uses context.getResources() to inflate the drawable. However, this cannot be used on pre-lollipop devices to inflate vector drawables, that task requires AppCompatResources.getDrawable(...).

oprisnik commented 7 years ago

1176 describes why this is currently not working. However, for now you can set the drawable in Java and it should work. I don't think we can easily change the inflater to use AppCompatResources since the whole library would have to depend on AppCompatResources otherwise. Another solution would be to extend SimpleDraweeView and inflate the placeholder manually using AppCompatResources.

gregkorossy commented 7 years ago

I think there are two options to solve this:

Extending SimpleDraweeView would only solve the problem if custom attributes were introduced for all of the image types because the default placeholderImage (and other image setter) attribute gets resolved before anything else due to the super call, which would result in the same exception. Custom attributes might also mean that the *ScaleType attributes would need to be parsed too for the custom attributes.

vbuberen commented 7 years ago

@Gericop I don't have such issues and can use vector drawables in XML in placeholderImage and there are no crashes on APIs down to 16 (min version in the actual production app). You might have a problem because of vectorDrawables.useSupportLibrary = true property in your Gradle file according to this StackOverflow thread.

gregkorossy commented 7 years ago

@vbuberen Do you use AppCompatDelegate.setCompatVectorFromResourcesEnabled(true); instead? Because that is still a no-no for the memory issues it might introduce (see the documentation).

vbuberen commented 7 years ago

Nope, I don'. As you might know, for devices with API lower than 21 PNGs are generated instead of vectors automatically - on some really old devices with small resolution screen you can see that some drawables aren`t clear-cut like vector drawables should be. But still everything works fine with Fresco and placeholders.

gregkorossy commented 7 years ago

@vbuberen Well, I've always had vectorDrawables.useSupportLibrary = true in my build.gradle, yet it crashes on < API 21 devices.

vbuberen commented 7 years ago

@Gericop Kind of strange. Have you tried to check everything again without DataBinding? And what happens if you remove vectorDrawables.useSupportLibrary = true?

ibtisamasif commented 6 years ago

@oprisnik I was able to fix this issue by adding AppCompatDelegate.setCompatVectorFromResourcesEnabled(true); in the onCreate of Application class. and this automatically fixes fresco:placeholderImage="@drawable/vector_graphic"

I have tested this in API level 19.

A similar question is posted on stackoverflow

This should also be added on official documentation FAQ's section.

gregkorossy commented 6 years ago

@ibtisamasif The problem with that call is clearly documented:

This feature defaults to disabled, since enabling it can cause issues with memory usage, and problems updating Configuration instances. If you update the configuration manually, then you probably do not want to enable this. You have been warned.