RecyclerView with GridLayoutManager and Picasso showing wrong image

Question

Update #1

Added hasStableIds(true) and updated Picasso to version 2.5.2. It does not solve the issue.

Reproduction:

RecyclerView with GridLayoutManager (spanCount = 3). List items are CardViews with ImageView inside.

When all the items does not fit the screen calling notifyItemChanged on one item causes more than one calls to onBindViewHolder(). One call is for position from notifyItemChanged others for items not visible on the screen.

Issue:

Sometimes the item at position passed to the notifyItemChanged is loaded with an image belonging to an item that is not on the screen (most likely due to recycling of the view holder - although I would assume that if the item remains in place then the passed viewholder would be the same).

I have found Jake's comment on other issue here about calling load() even if the file/uri is null. Image is loaded on every onBindViewHolder here.

Simple sample app:

git clone https://github.com/gswierczynski/recycler-view-grid-layout-with-picasso.git

Tap on an item calls notifyItemChanged with parameter equal to the position of that item.

Code:

public class MainActivity extends ActionBarActivity {

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        if (savedInstanceState == null) {
            getSupportFragmentManager().beginTransaction()
                    .add(R.id.container, new PlaceholderFragment())
                    .commit();
        }
    }

    public static class PlaceholderFragment extends Fragment {

        public PlaceholderFragment() {
        }

        @Override
        public View onCreateView(LayoutInflater inflater, ViewGroup container,
                                 Bundle savedInstanceState) {
            View rootView = inflater.inflate(R.layout.fragment_main, container, false);

            RecyclerView rv = (RecyclerView) rootView.findViewById(R.id.rv);

            rv.setLayoutManager(new GridLayoutManager(getActivity(), 3));
            rv.setItemAnimator(new DefaultItemAnimator());
            rv.setAdapter(new ImageAdapter());

            return rootView;
        }
    }

    private static class ImageAdapter extends RecyclerView.Adapter<ImageViewHolder> implements ClickableViewHolder.OnClickListener {

        public static final String TAG = "ImageAdapter";
        List<Integer> resourceIds = Arrays.asList(
                R.drawable.a0,
                R.drawable.a1,
                R.drawable.a2,
                R.drawable.a3,
                R.drawable.a4,
                R.drawable.a5,
                R.drawable.a6,
                R.drawable.a7,
                R.drawable.a8,
                R.drawable.a9,
                R.drawable.a10,
                R.drawable.a11,
                R.drawable.a12,
                R.drawable.a13,
                R.drawable.a14,
                R.drawable.a15,
                R.drawable.a16,
                R.drawable.a17,
                R.drawable.a18,
                R.drawable.a19,
                R.drawable.a20);

        @Override
        public ImageViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
            View v = LayoutInflater.from(parent.getContext()).inflate(R.layout.list_item, parent, false);
            return new ImageViewHolder(v, this);
        }

        @Override
        public void onBindViewHolder(ImageViewHolder holder, int position) {
            Log.d(TAG, "onBindViewHolder position: " + position + " | holder obj:" + holder.toString());
            Picasso.with(holder.iv.getContext())
                    .load(resourceIds.get(position))
                    .fit()
                    .centerInside()
                    .into(holder.iv);
        }

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

        @Override
        public void onClick(View view, int position) {
            Log.d(TAG, "onClick position: " + position);
            notifyItemChanged(position);
        }

        @Override
        public boolean onLongClick(View view, int position) {
            return false;
        }
    }

    private static class ImageViewHolder extends ClickableViewHolder {

        public ImageView iv;

        public ImageViewHolder(View itemView, OnClickListener onClickListener) {
            super(itemView, onClickListener);
            iv = (ImageView) itemView.findViewById(R.id.iv);
        }
    }
}

public class ClickableViewHolder extends RecyclerView.ViewHolder implements View.OnClickListener, View.OnLongClickListener {
    OnClickListener onClickListener;


    public ClickableViewHolder(View itemView, OnClickListener onClickListener) {
        super(itemView);
        this.onClickListener = onClickListener;
        itemView.setOnClickListener(this);
        itemView.setOnLongClickListener(this);
    }

    @Override
    public void onClick(View view) {
        onClickListener.onClick(view, getPosition());
    }

    @Override
    public boolean onLongClick(View view) {
        return onClickListener.onLongClick(view, getPosition());
    }

    public static interface OnClickListener {
        void onClick(View view, int position);
        boolean onLongClick(View view, int position);
    }
}

Epistasis

did you find a solution? I'm having the same issue. Does this happen only with RecyclerView? Have your tried with ListView? – Fann

Not yet. Since I am not sure if this is an issue with Picasso or GridLayoutManager I have posted issues on both PIcasso github project page (github.com/square/picasso/issues/954) and AOSP Google Code (code.google.com/p/android/issues/detail?id=162699). I do not think this issue exists on ListView. – Epistasis

Did you resolve this issue, I still see that this issue exists in com.squareup.picasso:picasso:2.5.2 setSupportsChangeAnimations(false) and setHasStableIds(true) seem to prevent this from happening – Circus

Answer 1

I spent more time than I'd like to admit to work around oddities with RecyclerView and the new adapter that comes with it. The only thing that finally worked for me in terms of correct updates and making sure notifyDataSetChanges and all of its other siblings didn't cause odd behavior was this:

On my adapter, I set

setHasStableIds(true);

In the constructor. I then overrode this method:

@Override
public long getItemId(int position) {
    // return a unique id here
}

And made sure that all my items returned a unique id.

How you achieve this is up to you. For me, the data was supplied from my web service in the form of a UUID and I cheated by converting parts of the UUID to long using this:

SomeContent content = _data.get(position);
Long code = Math.abs(content.getContentId().getLeastSignificantBits());

Obviously this is not a very safe approach but chances are it works for my lists which will contain < 1000 items. So far I haven't run into any trouble with it.

What I recommend is to try this approach and see if it works for you. Since you have an array, getting a unique number for you should be simple. Maybe try returning the position of the actual item (and not the position that is passed in the getItemId()) or create a unique long for each of your records and pass that in.

Suellen

The presented code is just to show the issue. My actuall implementation is way more complicated (with different ways of sorting and filtering). I do have UUID to (maybe hashcode will do the job). I saw that optimization, but haven't tried it yet. – Epistasis

Same for me. My adapter code is much more complex but I had lots of issues with getting images to behave in recycler views (loading via UniversalImageLoader asynchronously). The only thing that worked for me was what I wrote. – Suellen

Thanks for your input @kha. Unfortunately this does not solve the issue. I have committed changes you proposed. – Epistasis

use url.hashCode() for itemid – Park

Answer 2

Have you tried calling the mutate() method on the Drawable? See here, for instance.

Polyphone

Answer 3

here a working solution but has graphics glitches when calling notifyDataSetChanged()

holder.iv.post(new Runnable() {
            @Override
            public void run() {
                     Picasso.with(holder.iv.getContext())
                              .load(resourceIds.get(position))
                              .resize(holder.iv.getWidth(), 0)
                              .into(holder.iv);
            });

it works because at this point image has a width, unfortunately when I need to update all the checkboxes the in the viewholder (like a select all action), and I call notifyDataSetChanged() and the effect is very ugly

still searching for a better solution

edit: this solution works for me:

    holder.iv.getViewTreeObserver().addOnGlobalLayoutListener(new ViewTreeObserver.OnGlobalLayoutListener() {
        @Override
        public void onGlobalLayout() {
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN)
                holder.iv.getViewTreeObserver().removeOnGlobalLayoutListener(this);
            else
                holder.iv.getViewTreeObserver().removeGlobalOnLayoutListener(this);

                Picasso.with(holder.iv.getContext())
                         .load(resourceIds.get(position))
                         .resize(holder.iv.getMeasuredWidth(), 0)
                         .into(holder.iv);
        }
    });

Park

Solution works but it brings new bugs, so I don't recommend this. – Shayna