Glide: Images fail to load in recyclerview

[renaudcerrato]

I'm switching from Picasso to Glide with pretty much success: everything went smooth, excepted a weird behavior on my "conversation" recyclerview:

Some image simply didn't load:

void bind(MessageDBO message) {
            text.setText(message.getContent());

            if(image != null) {
                image.setImageURI(message.getAvatar());
                image = null; // never refreshed
            }
   ...

   ...
@Override
public void setImageURI(String uri, Drawable placeHolder) {
        ScaleType scaleType = getScaleType();
        DrawableTypeRequest<String> req = Glide.with(getContext()).load(uri);

        if(placeHolder != null)
            req.placeholder(placeHolder).error(placeHolder).fallback(placeHolder);

        switch(scaleType) {
            case FIT_CENTER:
                req.fitCenter();
                break;
            case CENTER_CROP:
                req.centerCrop();
                break;
        }

        req.crossFade().into(this);
    }
   ...

The strange thing is scrolling the views outside the screen (and thus recycling them) make them appears once scrolled back. I can't understand why since calling bind() on a recycled holder won't call Glide again.

I tried adb shell setprop log.tag.GenericRequest VERBOSE, but there's no failure at all.

Sounds like a sychronization issue?

[sjudd]

Try adding a call to override() and see if that fixes it. One common way this happens is if you're using views that don't have fixed sizes and/or are dependent on the size of their content. Glide tries to wait until views that don't have fixed sizes go through layout. In some cases the view ends up waiting on Glide and Glide ends up waiting on the view, resulting in the load silently never starting.

If override works, consider a fixed size in your view, or maybe paste your xml here so we can try to suggest other alternatives.

[TWiStErRob]

Is it possible that getAvatar()/uri is null? Because you're using placeholder for all scenarios, you won't see a visual cue when something goes wrong. Here it's possible that fallback is displayed for load(null), but you don't see it because it's the same as the "loading" drawable. I suggest you try using different drawables for placeholder/error/fallback, that helps to diagnose problems faster. (Remember, you can still use the same drawable in production with BuildConfig.DEBUG.)

You can also try Engine and EngineRunnable tags. Also if you don't see "Got onSizeReady in " for GenericRequest, that kind of confirms Sam's idea.

As an aside, since you're transitioning to Glide: you can remove the whole scaleType logic, because it's handled in into(). Also crossFade() is the default. This cuts your Glide code by 75%.

[renaudcerrato]

Yes, avatar can return null, but it isn't for sure since scrolling the list make then appear.

Probably something related in sizes, I'll check it tomorrow at office, I'm out for now.

[sjudd]

Also two other points:

1. You don't need to set fallback, error, and placeholder all to the same thing. You can just set placeholder and it will be used in all three cases. Error and fallback function as overrides for placeholder when set.
2. You don't need to check the scale type, Glide does that for you as long as you're loading into a view (see https://github.com/bumptech/glide/blob/3.0/library/src/main/java/com/bumptech/glide/GenericRequestBuilder.java#L675)

[renaudcerrato]

Thanks @sjudd and @TWiStErRob for the valuable tips.

Adding override() solved it, but I can't understand why since my ProfileImageView has fixed size (through styles):

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_height="wrap_content">

    <myapp.view.ProfileImageView
        android:id="@+id/image"
        style="@style/Message.ListItem.Image.Left"/>
   ...

And here's my styles.xml:

 ...
    <style name="Message.ListItem.Image">
        <item name="android:layout_width">@dimen/grid_size_medium</item>
        <item name="android:layout_height">@dimen/grid_size_medium</item>
    </style>

    <style name="Message.ListItem.Image.Left">
        <item name="android:layout_alignParentLeft">true</item>
        <item name="android:layout_marginLeft">@dimen/spacing_medium</item>
        <item name="android:layout_alignParentTop">true</item>
    </style>
 ...

Something is going wrong?

[TWiStErRob]

Yes, you need to inherit styles, the dotting is just a naming convention as far as I know:

<style name="Message.ListItem.Image.Left" parent="Message.ListItem.Image">

Edit: I was wrong :) I found that dotting is enough here:

If you want to inherit from styles that you've defined yourself, you do not have to use the parent attribute. Instead, just prefix the name of the style you want to inherit to the name of your new style, separated by a period.

It still worth a try to be explicit or maybe even try to repeat the android:layout_width and android:layout_height in Image.Left to see if that works (remove the .override() call!); I suspect something may be fishy with LayoutParams: maybe it behaves like when you pull in a <layout and override some layout_*.

If you're inclined you can try to debug ViewTarget.SizeDeterminer#getSize and see what's Glide seeing from those styles.

[renaudcerrato]

Nope, I already (desperately) tried to explicit the layout_width and layout_height and, as expected, absolutely no change. Picasso was just working fine, but I'd really like to stick with Glide.

That sounds related to a ViewTreeObserver since, as I pointed out, scrolling the items back and forth make them appears despite Glide code isn't executed anymore on recycled views (the profile image won't change).

The debug ouput of GenericRequest isn't really helpful, how can I debug that for you?

[renaudcerrato]

Here's the phenomenon: scrolling the items (and so recycling the viewholders) makes the image appears, despite Glide isn't executed anymore (image == null) :

[TWiStErRob]

It's even weirder that even when you scroll back it sometimes doesn't show again. Can you just try loading every time:

//image = null; // never refreshed

I know it's a waste, it's a nice optimization, but Glide will serve from memory cache so there won't be I/O only CPU.

how can I debug that for you?

I guess the best would be to extract this part of the application to a mini-project where it's still reproducible and then we could take a look.
I also noted a debug point in my previous comment, you can try filling those methods with logging breakpoints and see what gets called and when.

I'd really like to stick with Glide.

Remember that you have a workaround with override(), you just need to add a new method to your ImageView in the worst case which can receive R.dimen.grid_size_medium as use it as size.

[TWiStErRob]

I just had an revelation: what if your ImageView is the cause? I'm not sure what is in your ImageView doing, but if it's a circling one, you could use this solution with a regular imageView instead. That way the circling will be also be cached.

Also double check if all your constructors are correctly implemented (important if you use styles!):

public ProfileImageView(Context context) { super(context); doMyInit(); }
public ProfileImageView(Context context, AttributeSet attrs) { super(context, attrs); doMyInit(); }
public ProfileImageView(Context context, AttributeSet attrs, int defStyleAttr) { super(context, attrs, defStyleAttr); doMyInit(); }
public ProfileImageView(Context context, AttributeSet attrs, int defStyleAttr, int defStyleRes) { super(context, attrs, defStyleAttr, defStyleRes); doMyInit(); }

[renaudcerrato]

Removing image = null looks better, images are refreshed on scroll (as expected) - most probably from cache:

For a given user, the image URL is exactly the same: maybe some race conditions occurs when loading multiple time the same URL from the network?

I'll try to reproduce it in a smaller project.

[sjudd]

Oh wait, this looks like a view re-use issue:

void bind(MessageDBO message) {
            text.setText(message.getContent());

            if (image != null) {
                image.setImageURI(message.getAvatar());
                image = null; // never refreshed
            }

It's hard to tell from this context, but it looks like you don't re-bind once you've asked Glide to load the image once for a given view? Who owns image? Any time recycler view calls bind you should always load an image, Glide keeps images in a memory cache so calling notifyDataSetChanged and rebinding all the views should still be relatively efficient.

[renaudcerrato]

@sjudd : the imageview content never changes once set (same image profile)

@TWiStErRob : just tried to replace my ProfileImageView by an ImageView, and it worked flawlessly... I missed the revelation!

Something is going weird into my ProfileImageView, I'll have to figure out (or use a Transformation instead as suggested).

I once suspected that CircleImageView, but since there's wasn't any onMeasure() (or similar) overrides, I forgot about it.

Sorry for your time.

[TWiStErRob]

@sjudd I think he's trying to simulate setting the image on inflation, that bind looks like it's in a viewholder which forgets about the image after it's bound once. Since the views don't change in a viewholder it looks safe.

@renaudcerrato it's a weird one, and I think I figured out by reading the CircleImageView code: It hacks the Bitmap out of the Drawable, but since TransitionDrawable (default anim is crossFade() in Glide) is not a BitmapDrawable it goes ahead and draws the drawable to a Bitmap. At the time this happens TransitionDrawable is still at 0% meaning you'll get the placeholder drawn onto the bitmap and cached. It looks like you need a few scrolls to invalidate the drawable (call to onSizeChanged) and re-draw it onto the bitmap.
Just out of curiosity: can you please put your ProfileImageView back and add .dontAnimate() to your Glide load line to confirm the above theory?

If it's true, there's only one question left: How did override() fix it? My guess is memory cache: _somehow_ it got cached quickly and when images are loaded from memory cache they are not animated.

Not a waste, thanks for the lessons.

[renaudcerrato]

@TWiStErRob : you're true, dontAnimate() did the trick!

[TWiStErRob]

Thanks! I would still suggest to replace those custom ImageViews with something less fragile and more dynamic, like transformations. It'll be worth in the long run.

[renaudcerrato]

Sure. Transformations looks better, but I'll have to find a way to apply that transformation to the placeholder too (or something similar), since I've a dynamic border around the cropped image and I don't want to hardcode it into the resources.

[Guang1234567]

@TWiStErRob

@renaudcerrato it's a weird one, and I think I figured out by reading the CircleImageView code: It hacks the Bitmap out of the Drawable, but since TransitionDrawable (default anim is crossFade() in Glide) is not a BitmapDrawable it goes ahead and draws the drawable to a Bitmap. At the time this happens TransitionDrawable is still at 0% meaning you'll get the placeholder drawn onto the bitmap and cached. It looks like you need a few scrolls to invalidate the drawable (call to onSizeChanged) and re-draw it onto the bitmap.

• It looks like you need a few scrolls to invalidate the drawable (call to onSizeChanged) and re-draw it onto the bitmap. is not the cause.
• Why looks normal need a few scrolls, because if the bitmap is loadedFromMemoryCache, the animation is NoAnimation, the relevant source code is here:

GenericRequest.java
 private void onResourceReady(Resource<?> resource, R result) {
          .....other code

            GlideAnimation<R> animation = animationFactory.build(loadedFromMemoryCache, isFirstResource);

         .......other code 
    }

DrawableCrossFadeFactory.java
@Override
    public GlideAnimation<T> build(boolean isFromMemoryCache, boolean isFirstResource) {
        if (isFromMemoryCache) {
            return NoAnimation.get();
        } else if (isFirstResource) {
            return getFirstResourceAnimation();
        } else {
            return getSecondResourceAnimation();
        }
    }

[TWiStErRob]

Ah, yeah, it's not the layout, but the TransitionDrawable, again. I think we figured it out in other issues after this. This was among the first of many. Thanks for pointing out the real root cause.

The first time it's a TransitionDrawable because loadedFromMemoryCache is false and the other times it's just a normal drawable, because it's loaded from cache and there's no animation. dontAnimate() solves the problem by forcing that normal drawable (i.e. anything other than TransitionDrawable) all the time.