Skip to content

Wrong images being loaded in Recycler View's grid on Some devices #601

Closed
@gauravlnx

Description

@gauravlnx

In my app, I have a Recycler view with GridLayoutManager. The code I am using to load images bindImage method of recycler view adapter:

@Override
public void onBindViewHolder(ImageViewHolder holder, int position) {
    GSImage dbImage = mLandscapeImagesList.get(position);
    File imageFile = new File(mImageDirectoryFile, dbImage.getImageName());

    Glide.with(MyImageFragement.this).load(imageFile).centerCrop().into(holder.imageView);

    int indexInSelectedImagesSet = mSelectedLandscapeImages.indexOf(dbImage);
    if (indexInSelectedImagesSet>-1){
        holder.itemView.setSelected(true);
    } else {
        holder.itemView.setSelected(false);
    }

}

The Cell layout I am using is:

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:app="http://schemas.android.com/apk/res-auto"
      android:layout_width="wrap_content"
      android:layout_height="wrap_content"
      android:background="@drawable/images_background"
      android:layout_marginTop="9dp"
      android:layout_marginLeft="10dp"
      android:layout_marginRight="0dp"
      android:layout_marginBottom="0dp"
      android:padding="2dp">

      <com.gaurav.myProject.commons.AspectRatioImageView
                 android:id="@+id/image_thumb"
                 android:layout_width="match_parent"
                 android:layout_height="wrap_content"
                 android:scaleType="fitXY"
                 app:viewAspectRatio="1.33"/>

</RelativeLayout>

Here AspectRatioImageView(subclass of ImageView) automatically resizes the height of image view according to the provided aspect ratio.

The above code is working fine on my devices (Xiaomi MiPad, ViewSonic Tablet, Android emaulators and Samsung Galaxy S3) but is not working correctly on my client's device (Samsung 10.1 inch tablet).
In his end, images are being duplicated in the recycler grid. Also the the duplicates mostly occurs in adjacent recycler grid positions. I can't share any code as I even unable to reproduce the issue on my devices.

Do you guys have any idea what could the cause of this? Can you please suggest me any thing that I can try?

Thanks

Activity

TWiStErRob

TWiStErRob commented on Aug 28, 2015

@TWiStErRob
Collaborator

To be honest, while reading your description I was really surprised by the issue, based on what you have in code. I don't see anything weird in there.

Is it consistently happening to them? Is it always the same image? Is it possible that they have the wrong files in mImageDirectoryFile? Do you use any integration library to replace networking (maybe wrong cache)?

One thing you might try is to generalize your aspect view, since we have no idea about the cause, it may or may not help:

  • create a new ViewGroup (e.g. AspectLayout extend FrameLayout)
  • move onMeasure override to the layout (read an article about how to create a layout, it's single view inside so it should be simple)
  • change grid item root layout to AspectLayout
  • move aspect attribute to layout
  • revert to using simple ImageView (w=match, h=match, scaleType=centerCrop, remove centerCrop() from code)
gauravlnx

gauravlnx commented on Aug 28, 2015

@gauravlnx
Author

I am not using any networking library as my app doesn't require to be online for its functionalities. All the images my app uses are local images from the device itself. In my app, I have created an Image Picker using which user pick images to use in the app. The weird thing is that the Image Picker is also using RecylerView GridLayoutManager, but they haven't reported anything about the picker.

They say they are able to reproduce the issue almost every time. The issue isn't related to any particular image as I have seen the videos they have sent me.

There is nothing wrong with images in mImageDirectoryFile as have asked them to check and they have reported that the images are not duplicated in the storage directory.

In all the screens where there are image grids( except for image picker), there are two different recycler views used but I don't think it has anything to do with that.

TWiStErRob

TWiStErRob commented on Aug 28, 2015

@TWiStErRob
Collaborator

I don't think it has anything to do with that.

Don't rule out anything yet. Just to be sure double check that you're not sharing any Adapter/Layout/... between the two lists.

You can try to ask them for a backup of your app: http://www.howtogeek.com/125375/how-to-create-a-full-android-phone-or-tablet-backup-without-rooting-or-unlocking-your-device/ (you can replace all that SDK install with minimal ADB) and then you can restore it to your own device to see if you can reproduce it. Sadly, again, this may not work due to "There is nothing wrong with images", but there may be other switches in settings or database (if you have one), you know these parts best.

Another idea: Do you have any w600-like resource folders? Maybe it only happens if a particular constraint is met, like big screen, or as simple as landscape, and you have different parameters to the RecyclerView layouts or the item layout in those different folders. You can try to set up a Genymotion emu for the Galaxy Tab to try to reproduce.

TWiStErRob

TWiStErRob commented on Aug 28, 2015

@TWiStErRob
Collaborator

One more: do you have any .transform() calls or custom targets (.into(new ...)) ANYWHERE in your app? We've found that if the Bitmap pool is tainted, weird things can happen, see for example Sam's explanation: #483 (comment)

gauravlnx

gauravlnx commented on Aug 28, 2015

@gauravlnx
Author

Don't rule out anything yet. Just to be sure double check that you're not sharing any Adapter/Layout/... between the two lists.

All the recycler views use their individual adapters and layout and nothing is shared between them.

One more: do you have any .transform() calls or custom targets (.into(new ...)) ANYWHERE in your app? We've found that if the Bitmap pool is tainted, weird things can happen, see for example Sam's explanation: #483 (comment)

I am not using .transform() but using custom targets in my app that too just before the content is displayed in the recycler view grid.

The flow used in my app is that:

  1. The user picks one or more images using the image picker.
  2. My app imports the images sequentially.
  3. To import image first, my app loads the image in a custom target in a predefined size.
Glide.with(MyImageFragement.this).load(imageFile).asBitmap().into(new SimpleTarget<Bitmap>(reqWidth, reqHeight) {
            @Override
            public void onResourceReady(Bitmap resource, GlideAnimation<? super Bitmap> glideAnimation) {
                new SaveImageTask().execute(resource);
            }
        });

4.The SaveImageTask async task saves the file to the storage directory and then add the path of this file to mSelectedLandscapeImages ArrayList at index 0 and notifies the adapter that a new cell is added at position 0.
5. If there are more images available to import, steps 3 to 5 are repeated till all the images are processed.

TWiStErRob

TWiStErRob commented on Aug 28, 2015

@TWiStErRob
Collaborator

I really hope that my below theory is correct and this solves your issue. I think you can likely reproduce your issue by loading a lot of very small images on a fast phone with your current code.

From your flow description it sounds like a sync issue: imagine that an addition notifies the adapter and RecyclerView starts updating. It has to rebind all visible items so it goes sequentially. It reads item i, and then before reading item i+1 another image is ready, so that new item is added to the list and being notified of change. I'm not sure what happens if notification comes mid-update but from the looks it's "thanks for telling me, but I'm already doing it". Continuing the rebinds it reads i+1, which is actually the item i (again) because now the whole thing is shifted by one (new item came mid update). This is how I imagine your clients ended up with 2 of the same images next to each other. I think you'll find this somewhere in the documentation: notifications and updates to adapter must be on the main thread.

Assuming your SaveImageTask looks like this:

doInBackground()
    // saves the file to the storage directory
    // add the path of this file to mSelectedLandscapeImages ArrayList at index 0
    // notifies the adapter that a new cell is added at position 0

change it to:

doInBackground()
    // saves the file to the storage directory
onPostExecute()
    // add the path of this file to mSelectedLandscapeImages ArrayList at index 0
    // notifies the adapter that a new cell is added at position 0

This way access to the adapter and its model is ordered by the main thread's queue.

I think another, cleaner and probably faster way would be, based on the above idea is to write an AsyncTask<String, File, Void> (media url String, saved File, nothing) and use onProgressUpdate to batch UI refresh for all selected files. You can use Glide synchronously with: .asBytes().into(w, h).get() inside a for loop in doInBackground, you just have to write the File contents from byte[]. And call reportProgress after each file saved. AsyncTask will batch these and call onProgressUpdate with multiple files so you can call "k new items inserted" on the adapter. This leads to less rebinding while saving if saving is fast. Also likely much better memory utilisation because Glide doesn't hand you the Bitmap iteself, so it can pool them, most likely only needing 2 Bitmap allocations (max) to save N images, instead of N Bitmaps never returned to the pool with the current SimpleTarget implementation.

Feel free to ask for clarification if you're having trouble implementing this. Happy to sketch up what I described above in compilable code.

gauravlnx

gauravlnx commented on Aug 29, 2015

@gauravlnx
Author

Thanks Róbert for your reply.

So SimpleTarget could be the problem here?

I think you can likely reproduce your issue by loading a lot of very small images on a fast phone with your current code.
Is there anything that I have written in my code that I shouldn't have?

All the videos that I have seen about this issue looked like are produced using small images.

doInBackground()
// saves the file to the storage directory
onPostExecute()
// add the path of this file to mSelectedLandscapeImages ArrayList at index 0
// notifies the adapter that a new cell is added at position 0

My SaveImageTask works just like this. I always refresh adapters on the main thread.

I will surely try .asBytes().into(w, h).get() on Monday in my office.
Can I directly write the bytes received by the above Glide method as a JPG file or Do I need to perform any other operations on the bytes so that I can save them in JPGs.

TWiStErRob

TWiStErRob commented on Aug 29, 2015

@TWiStErRob
Collaborator

Which method do you use to notify the adapter?

My SaveImageTask works just like this.

That debunks my theory :( it sounded convincing though. I'm out of ideas. @sjudd, anything to add?

All the videos that I have seen about this issue looked like are produced using small images.

Another idea to consistently reproduce this is to put a Thread.sleep(500); at the end of onBindViewHolder so a lot of Glide requests can be finished in the background. Also whenever you try to reproduce timing issues, make sure you don't use Debug mode, the debugging overhead may hide the issue; I usually use just logging to see the order of execution.

So SimpleTarget could be the problem here?

onResourceReady is called on the UI thread too so it is ordered. Maybe RecyclerView doesn't like multiple quick inserts, however I checked the sources and didn't see anything that would warrant this behavior. Are you using the latest support libs (22.2.1 or 23.0.0)?

I remembered the asBytes() syntax incorrectly, here's what I meant:

IOTools.writeAllBytes(fileInImageDirectory, Glide
        .with(this)
        .load(sourceUri) // can be a random content Uri too held in Intent.getData()
        .asBitmap()
        .toBytes(CompressFormat.JPEG, 80) // forwarded to Bitmap.compress
        .diskCacheStrategy(DiskCacheStrategy.NONE) // no point in caching because it's saved to a File anyway
        .skipMemoryCache(true) // make sure to re-read if original image changed
        .atMost() // together with into() no image dimension will be bigger than the requested max
        .into(2048, 2048) // both the same, but aspect will be kept
        .get() // makes the Glide call sync, but has to be called on a background thread
);
publishProgress(fileInImageDirectory);

I use something like this myself to load user picked content from Gallery (or any other external app).

sjudd

sjudd commented on Sep 2, 2015

@sjudd
Collaborator

No great ideas unfortunately. Maybe try with a standard ImageView instead of AspectRatioImageView?

It's also possible there's a Bitmap re-use issue on that particular device. You could try disabling BitmapReuse (call setBitmapPool(new BitmapPoolAdapter() in your GlideModule) for that device and see if that fixes the problem.

AdamFarhat

AdamFarhat commented on Sep 5, 2015

@AdamFarhat

I had a similar issue and I don't think that this is an issue with the Glide library. This seems to be related to RecyclerView#onBindViewholder reusing the views. The common solution to this problem is to set the ImageView's drawable to null on each call to onBindViewholder.
http://stackoverflow.com/questions/26525999/recyclerview-async-image-loading

TWiStErRob

TWiStErRob commented on Sep 6, 2015

@TWiStErRob
Collaborator

That shouldn't be necessary. When using Glide inside an adapter, the into(holder.im) call stores the request inside ImageView.tag and when rebinding it always clears that previously bound stored request resulting in a setImageDrawable(null) call automatically (see ImageViewTarget.onLoadCleared method). So Glide does that "common solution" of clearing for you.

The problem here is that the wrong image shows up after the newly bound image has been loaded, so it can't be residue. That being said I noticed that @gauravlnx you inlined the Glide line in your opening comment for brevity. Is there something you missed, like a conditional Glide load? If you have if(XXX) Glide.load then in the else branch you should call Glide.clear(holder.imageView) to avoid showing a residue previous image, this is equivalent to what Adam said, just the "Glide way".

gauravlnx

gauravlnx commented on Sep 6, 2015

@gauravlnx
Author

I am not using any conditional Glide load. The code in the onBindViewHolder mentioned in the first comment is all that I am using.

My client had tried a different device and reported that the issue is not there on the new device. The situation on the Samsung tablet is still the same.

sjudd

sjudd commented on Sep 6, 2015

@sjudd
Collaborator

Try disabling Bitmap re-use as I suggested above and see if it works. It's plausible that there are framework or manufacturer bugs on older devices, which would explain why you only see the issue on a Samsung tablet.

sjudd

sjudd commented on Sep 14, 2015

@sjudd
Collaborator

Let us know if you figure this out or if we can provide any more suggestions.

21 remaining items

ferPrieto

ferPrieto commented on Jan 26, 2016

@ferPrieto

I had exactly the same problem, with the same kind of code. I wasn't using the APK above, but my code.
So I think it's the same issue, and I have solved with the line I told you.

TWiStErRob

TWiStErRob commented on Jan 26, 2016

@TWiStErRob
Collaborator

We're looking for a consistent repro case that manifests the issue on most devices. Without a repro we can't fix the bug because there's no easy way to validate any fix. The APK (or any publicly available source that demonstrates the issue) should be an easy way to have a consistent repro. I tried multiple devices and failed to reproduce. I asked you to try your device so we can extend the list of device/version pair to maybe find a pattern.

Using any kind of workaround (like adding a placeholder) will help you in the short term, but it won't help fixing the real problem. In general over time these little bugs may add up and any system experiencing this becomes unmaintainable. Glide is quite stable, but if we don't put effort in to finding and fixing these problems it will cost a lot of aggregate man-hours around the world...

Your recognition of similarities in code doesn't mean you have the same problem, this is the biggest issue: the code above is extremely common and should work in 99% of the cases, that 1% is what we need to deal with without sweeping it under the rug. I hope this helps in understanding why I'm asking what I'm asking.

Anyone who can reproduce this issue should tell that fact along with device/version/code if any. I'm hoping that Sam may be able to repro this when he gets around to it.

levischmidt

levischmidt commented on Mar 2, 2016

@levischmidt

Hey @TWiStErRob - any movement on this? I'm confident that it is reproducible on any 5.0 device using the apk or source zip I provided above (must be 5.0.x, not 5.1.x or 5.2.x). I've reproduced on various devices on 5.0, as well as by flashing a nexus 5 with a 5.0 build. let me know if I can be of any assistance it getting this solved! thanks!

gauravlnx

gauravlnx commented on Mar 2, 2016

@gauravlnx
Author

I think this issue was with a version of Android lollypop most probably 5.0. I don't remember the exact Android version but from the video my client sent it looked like lollypop. All the devices that I had were on kitkat or lower.

jclova

jclova commented on Mar 15, 2016

@jclova

I can reproduce the same issue on Note 4 (5.1.1) and Samsung Galaxy S6 (5.1.1).

BobOtike

BobOtike commented on Jul 29, 2016

@BobOtike

@TWiStErRob Using the sample APK provided above, I can reproduce the issue on my Galaxy S4 running 5.0.1.

I happened to observe the issue in my app and came here via search.

TWiStErRob

TWiStErRob commented on Jul 29, 2016

@TWiStErRob
Collaborator

Huh, I gave in another try on my new Galaxy S5 (5.0.0) and I was able to repro, though it took a while to find a consistent one: Cinemax at the bottom usually has the wrong icon. I'll take a look tomorrow, fingers crossed I can repro it while a debugger is attached...

TWiStErRob

TWiStErRob commented on Jul 30, 2016

@TWiStErRob
Collaborator

I noticed in the logs these lines correlate with the wrong placements:

07-30 10:45:01.465 12077-12143/com.bumptech.glide.supportapp.v3 E/Adreno-ES20: <get_texture_formats:3059>: Invalid internal format(0)!
07-30 10:45:01.465 12077-12143/com.bumptech.glide.supportapp.v3 E/Adreno-ES20: <TexSubImageLoad:421>: Init_texel_data failed
07-30 10:45:01.465 12077-12143/com.bumptech.glide.supportapp.v3 W/Adreno-ES20: <core_glTexSubImage2D:600>: GL_INVALID_OPERATION
07-30 10:45:01.475 12077-12143/com.bumptech.glide.supportapp.v3 E/OpenGLRenderer: GL error:  GL_INVALID_OPERATION

then I went ahead and put this into the GlideModule:

builder.setDecodeFormat(DecodeFormat.PREFER_ARGB_8888);

which fixed the issue, but still don't know about the root cause.

I also noticed that there's a border appearing sometimes around the wrongly placed images:
image

Enabling more logging revealed that some images are loaded as 565 and some as 8888, even when decode format is PREFER_RGB_565. This is probably because of transformations, see TransformationUtils.getSafeConfig; and later both of these types of Bitmaps are reused. Couldn't prove it yet, because my debugger doesn't really work. I saw examples of both 565 and 8888 Bitmaps showing up in the wrong position.

Setting

builder.setBitmapPool(new BitmapPoolAdapter());

also fixes the issue, but it disables bitmap re-use, which is at the core of Glide's performance.

I went down a level (.asBitmap()) to see which Bitmaps are actually used where. The issue still reproed and one example was:

  • originally displayed Bitmap: Disney at Disney's position
  • reused same Bitmap: Epix at Cinemax's position

and then I put a breakpoint to see what's the actual Bitmap in memory. The bad news is that Glide is loading the correct image into the Bitmap according to the debugger's "View Bitmap". To double check I dumped the bitmaps for Cinemax position to the sdcard via .compress(PNG,...) and they're all the Cinemax logo. And indeed the following also fixes the problem:

imageView.setLayerType(View.LAYER_TYPE_SOFTWARE, null);

Notice that it's not even about recycling, as the above example suggests. It's not Disney's image showing up (where the viewholder came from), but the image just above the current item, most likely the most recent GPU-uploaded/rendered texture.

I also tried a lot of combinations of these:

imageView.setWillNotCacheDrawing(true);
imageView.setDrawingCacheEnabled(true);
imageView.postInvalidate();

but it didn't help.

Summary

According to the above investigation I think the problem is in the framework, it mixes up OpenGL textures, and I found no way to detect if it'll happen or not from code yet. From the above workarounds the best solution is setting PREFER_ARGB_8888, because it hurts performance the least, and it will be the default in Glide v4 anyway. Also I'm strongly against using MemoryCacheAdapter and BitmapPoolAdapter in production code, as they disable the best parts of Glide, but they were great help diagnosing this problem.

@sjudd or anyone else: what can we do about this?

added a commit that references this issue on Jul 30, 2016

bumptech/glide#601 image shows up in wrong position

stale

stale commented on Nov 12, 2017

@stale

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

BenjaminNjoroge

BenjaminNjoroge commented on Apr 9, 2018

@BenjaminNjoroge

This problem comes when the bindViewHolder logic is coded wrongly. I had a similar problem and this is how i solved it
import android.content.Context;
import android.support.v7.widget.RecyclerView;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageView;
import android.widget.TextView;

import com.bumptech.glide.Glide;

import net.webnetworksolutions.mama.R;
import net.webnetworksolutions.mama.pojo.Counties;

import java.util.List;

/**

  • Created by Benja on 4/4/2018.
    */

public class CountiesAdapter extends RecyclerView.Adapter<CountiesAdapter.MyViewHolder> {

private Context mContext;
private List<Counties> countiesList;

public CountiesAdapter(Context mContext, List<Counties> countiesList) {
    this.mContext = mContext;
    this.countiesList = countiesList;
}

@Override
public MyViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
   View itemView= LayoutInflater.from(parent.getContext())
           .inflate(R.layout.county_model,parent, false);

    return new MyViewHolder(itemView);
}

@Override
public void onBindViewHolder(final MyViewHolder holder, int position) {
   Counties counties= countiesList.get(position);
   holder.title.setText(counties.getTitle());
   holder.hospitals.setText(counties.getHospitals());


    // loading album cover using glide library
    Glide.with(mContext.getApplicationContext())
            .load(counties.getImg()).placeholder(R.drawable.pg)
            .override(220,180).centerCrop()
            .into(holder.countyImages);
}

@Override
public int getItemCount() {
    return countiesList.size();
}

public class MyViewHolder extends RecyclerView.ViewHolder{
    public TextView title, hospitals;
    public ImageView countyImages;

    public MyViewHolder(View itemView) {
        super(itemView);
        title= itemView. findViewById(R.id.title);
        hospitals= itemView.findViewById(R.id.count);
        countyImages= itemView.findViewById(R.id.thumbnail);
    }
}

}

isabsent

isabsent commented on Dec 11, 2019

@isabsent

I have faced this problem on Xiaomi and Honor (Android Pie) with Glide 4.9.0 while loading thumbnail files from file system. The solution of xfort works:

in RecyclerView.Adapter,please set setHasStableIds(true), and getItemId(position)
return a stable and unique ID.
please try

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @sjudd@TWiStErRob@gauravlnx@xfort@isabsent

      Issue actions

        Wrong images being loaded in Recycler View's grid on Some devices · Issue #601 · bumptech/glide