Images displayed incorrectly in views Recycled in Recycler View

[gary-doolin]

Hi

I am displaying images in a recyclerview via Glide. The images are different widths and heights and are being pulled from the server.

The images all display fine until the views begin to be recycled. The image view has android:layout_width="wrap_content" android:layout_height="wrap_content" I have tried to sort this by setting holder.getImageView().setImageDrawable(null); in ` public void onBindViewHolder'. This didn't work. I have found item public void #388 and as that poster had a custom image view you suggested using ViewTarget and delaying size determination. As I'm using a standard ImageView the ImageViewTarget doesn't have a get size method so Im not sure how to apply this to my case

If you could offer any advice..here's my post on SO http://stackoverflow.com/questions/33283493/recyclerview-recycled-viewholder-image-view-wrong-size

Regards,
Gary

[TWiStErRob]

Few things that I noticed while reading your question:

Clearing image

image.setImageResource(0);
image.setImageDrawable(null);
image.setImageURI(null);

is pointless, image.setImageDrawable(null) is just enough (if there was a drawable before, that means that object != null is true):

public void setImageDrawable(Drawable drawable) {
    if (mDrawable != drawable) {
        mResource = 0;
        mUri = null;

Clearing the ImageView yourself may confuse Glide, it's suggested that you do Glide.clear(image); if you really want to manually clear them. You should do that in onViewRecycled, because you have many itemTypes. If you don't clear the views with many types you can easily get an OOM. Think about this case: you have 3 images that are followed by 5 text items. The 3 images fit on one screen, but would be only cleared later when another 3 images are displayed down in the list.

Loading into image

if(!MediaUtils.getMedia(feedContent).getMimeType().equals("image/gif")){
    gifTag.setVisibility(View.GONE);
    Glide.with(itemView.getContext()).load(Uri.parse(MediaUtils.getMedia(feedContent).getMediaUrl())).placeholder(R.drawable.placeholder).diskCacheStrategy(DiskCacheStrategy.SOURCE).crossFade().into(image);
}else {
    Glide.with(itemView.getContext()).load(Uri.parse(MediaUtils.getMedia(feedContent).getMediaUrl())).asGif().placeholder(R.drawable.placeholder).diskCacheStrategy(DiskCacheStrategy.RESULT).crossFade().into(image);
}

displayProfilePic(feedContent, profilePic);

Glide.with(itemView.getContext()).load(Uri.parse(MediaUtils.getMedia(feedContent).getMediaUrl())).placeholder(R.drawable.placeholder).diskCacheStrategy(DiskCacheStrategy.ALL).crossFade().into(image);

has a few problems:

• gifTag is not re-shown after a non-gif image
• SOURCE and RESULT is flipped for GIFs and non-gifs (GIF's should use SOURCE for faster loading)
• asGif() is not necessary, the default behavior will display an animated GIF is the source is a .gif file and has multiple frames, otherwise a normal Bitmap is loaded inside a Drawable. (this way it'll even work in case the mime type is lying :)
• duplicate .into(image) during a single bind
• duplicated code in general

Here's a version that you should consider:

DrawableRequestBuilder<Uri> request = Glide
    .with(itemView.getContext())
    .load(Uri.parse(MediaUtils.getMedia(feedContent).getMediaUrl()))
    .placeholder(R.drawable.placeholder)
    //.fitCenter() // automagic from XML (because there's no `scaleType`)
    //.crossFade() // this is the default
    .diskCacheStrategy(DiskCacheStrategy.ALL)
;
// notice that I flipped the if: special case(s) first, generic last
if (MediaUtils.getMedia(feedContent).getMimeType().equals("image/gif")) {
    gifTag.setVisibility(View.VISIBLE);
    request.diskCacheStrategy(DiskCacheStrategy.SOURCE); // load GIFs fast (see Glide issue 281)
} else {
    gifTag.setVisibility(View.GONE);
}
request.into(image);

displayProfilePic(feedContent, profilePic);

Android Layout

The biggest problem that's probably causing your trouble is the wrap_content and/or manipulating the layoutParams potentially just before binding. Glide needs either a non-laid out item or a laid out item that won't just change the next time it's laid out. I suggest you use match_parent in the XML and don't touch the layout in the Adapter/ViewHolder. Android does a pretty good job of separating view from logic. Your view is XML, and your logic is the Adapter/ViewHolder. The way I usually do it is to add tools:background="#ff0000" in XML and make it look correct in the Preview/editor, which will usually mean that I don't have to mess with layoutParams in code. Avoiding re-layouts is a really good practice, it saves cpu/battery and headaches like this one :)

I think a simple clear would be a better approach:

@Override public void onViewRecycled(AbstractHolder viewHolder){
    super.onViewRecycled(viewHolder);
    Glide.clear(viewHolder.getImageView());
}

Debugging & Resolving

Take a look at https://github.com/bumptech/glide/wiki/Debugging-and-Error-Handling, you can attach a .listener() and investigate ((ViewTarget)target).getView() for getWidth/getHeight/getLayoutParams and also the resource for how big an image was actually loaded. I wonder if your R.drawable.placeholder has a different size/ratio than your loaded image.

Other potential issues

if (feedContent.getName() != null) {
    body.setText((feedContent.getMessage()));

looks suspiciously a copy-paste error from the if above.

TextView [] tagsArray = {tag1, tag2, tag3};

Can be more static: a field initialized in the constructor. You don't need to allocate it every time you bind, because it's content don't depend on feedContent.

mimeType.contains(mContext.getString(R.string.video)) VS mimeType.contains("image")

Alert, alert, hardcoded :)

params.setMargins(0, (int) Utils.dpTopx(mContext,10),0,0);

Ditto, you can do this: context.getResources().getDimensionPixelSize(R.dimen.feed_item_margin) to match the exact value in XML.

[TWiStErRob]

Hopefully my comment helped resolve your issue, please reopen if you have questions about the above.

[gary-doolin]

Hi, apologies for not replying sooner. Thank you for your suggestions, I have streamlined my Glide calls now thanks to your help. I implemented the Glide.clear(imageview) which on it's own didn't work and then I saw you had mentioned the aspect ratio of the placeholder image.

The place holder image was just a temporary image I pulled from the internet until our designers created the correct placeholder. I removed the set place holder image method and combined with the Glide.clear(imageview) it works well. The problem is that this is a type of a news feed, so the images that are going to be displayed will be different orientation and sizes. For our designers creating the place holder image is there an optimum ration etc?

[TWiStErRob]

I asked about ratio because of #363. I guess you can double check if you're affected by this by setting a weird placeholder (50x1000 for example). The workaround for the issue is .dontAnimate(). If you add that (and remove .crossFade(), then any .placeholder() is fine.

Some people here mentioned they used 9-patches as placeholders, maybe that's the right answer for this kind of use case, I didn't test it myself yet.

[TWiStErRob]

There is one thing I can't explain yet: the padding around the image. The changing ratio (1.333->1.385) is explained by #363, but I'm not sure why it also gets smaller. I guess that's where the messing around with layoutParams and logging the runtime sizes comes in.