Picasso loads into wrong imageview when using view holder pattern

Question

I'm trying to use the Picasso library to load external images into rows in a ListView. I have a custom ArrayAdapter as follows:

public class RevisedBusinessesAdapter extends ArrayAdapter<HashMap<String, String>> {

    Context context;
    int layoutResourceId;
    ArrayList<HashMap<String, String>> data = null;

    public RevisedBusinessesAdapter(Context context, int layoutResourceId,  ArrayList<HashMap<String, String>> data) {
        super(context, layoutResourceId, data);
        this.layoutResourceId = layoutResourceId;
        this.context = context;
        this.data = data;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        View row = convertView;
        RevisedBusinessHolder holder = null;

        if (row == null) {
            LayoutInflater inflater = ((Activity) context).getLayoutInflater();
            row = inflater.inflate(layoutResourceId, parent, false);
            holder = new RevisedBusinessHolder();
            holder.ivLogo = (ImageView) row.findViewById(R.id.ivBusinessLogo);
            row.setTag(holder);
        } else {
            holder = (RevisedBusinessHolder) row.getTag();
        }

        HashMap<String, String> business = data.get(position);

        String strLogoURL = business.get("logoURL");
        if (null != strLogoURL && !"".equals(strLogoURL)) {
            Picasso.with(this.context).load(strLogoURL).into(holder.ivLogo);        
        }

        return row;
    }

    static class RevisedBusinessHolder {
        ImageView ivLogo;
    }
}

where logoURL is an URL for a remotely located image; if not supplied, ivBusinessLogo has a local src set, and that is shown instead. When I scroll quickly, Picasso loads the image into the wrong ImageView and I end up with multiple copies of it in the list.

The answer to this question suggests adding

Picasso.with(context).cancelRequest(holder.ivLogo);

before the existing Picasso call, but that doesn't make any difference. If I remove the row == null check and always create a new view, it appears to work fine. In the full version of this, though, there are also four textviews and five other images (small icons loaded from local resources, not via Picasso) that need to be updated in each getView.

Is there a way to make this work with the View Holder pattern the Android documentation recommends?

You should add an else statement after the Picasso.with().load().into() statement. Add `else holder.ivLogo.setImageBitmap(null);`.` Or use a placeholder bitmap. — greenapps, Oct 14 '14 at 11:45
@greenapps ivLogo has a src set in the XML, so it already has a placeholder. — Ben Williams, Oct 14 '14 at 11:49
That is not enough as a recycled view will contain the wrong bitmap of an old position. — greenapps, Oct 14 '14 at 11:50
@greenapps That does appear to work, thanks! You should add it as an answer. — Ben Williams, Oct 14 '14 at 11:59

score 23 · Answer 1 · answered Oct 14 '14 at 14:56

23

You should always call Picasso, even if your URL is null. This way it knows that the image view was recycled.

Delete this if statement:

if (null != strLogoURL && !"".equals(strLogoURL)) {

You should also consider using a placeholder image or an error image so that something will be displayed when there is no URL.

If you insist on keeping the if statement (but you shouldn't!), you need to tell Picasso that the image view was recycled by calling cancelRequest:

Picasso.with(this.context).cancelRequest(holder.ivLogo);

answered Oct 14 '14 at 14:56

Jake Wharton

75,598
23
223
230

8

Deleting the `if` statement will make you feed Picasso an empty `String`, resulting in `IllegalArgumentException: Path must not be empty.`, am I right? – Ivan Morgillo Sep 26 '15 at 13:36
1

Jake you couldn't even imagine how this saved the day. I tried many combinations of canceling and pausing tags on fling events, but nothing helped as this cancelRequest at the right place. – miroslavign May 09 '16 at 20:35
1

@IvanMorgillo right, Jake made a mistake, but his point was that you should always make a call to Picasso and this if clause was preventing it. Anyway, thanks Jake! – Dmytro Karataiev Feb 14 '17 at 19:52
1

@IvanMorgillo In that case just pass `null`. – 0101100101 Sep 19 '17 at 00:18
After many hours of searching, this solved my problem, and I think this shouldn't so hard to find like this... – hiddeneyes02 May 01 '18 at 21:25
This line helped me "You should always call Picasso, even if your URL is null. This way it knows that the image view was recycled." Thank you. – kalandar Mar 09 '20 at 10:36

score 2 · Answer 2 · answered Feb 07 '15 at 16:03

The default src drawable set in the layout.xml (on ImageView) is beeing override by the last cached dowload image if the current item doesnt have a image to be download from the url.

You must manually set the default drawable for itens that dont have a image atribute:

try {
     Picasso.with(activity.getApplicationContext()).load(customer.getImage().getPath()).placeholder(R.drawable.image_placeholder)
                .error(R.drawable.image_placeholder).into(imageView);
    }
catch (Exception e) {
       imageView.setImageResource(R.drawable.default_customer_icon);
       // this set the default img source if the path provided in .load is null or some error happened on download.
    }

score 0 · Answer 3 · answered Feb 05 '15 at 18:37

I have same problem and fixed it. Please consider about param convertView in getView method

convertView is old view to reuse, if possible. Note: You should check that this view is non-null and of an appropriate type before using. If it is not possible to convert this view to display the correct data, this method can create a new view.

When you use holder, Picasso will load image to the imageview of an row and all reuse of it. That why you see duplicate image. And in my opinion you should load your image while first time row view created. Try to change your code to

if (row == null) {
    LayoutInflater inflater = ((Activity) context).getLayoutInflater();
    row = inflater.inflate(layoutResourceId, parent, false);
    holder = new RevisedBusinessHolder();
    ImageVIew ivLogo = (ImageView) row.findViewById(R.id.ivBusinessLogo);
    Picasso.with(this.context).load("Your Image URL").into(holder.ivLogo);
    row.setTag(holder);
}

score -1 · Answer 4 · answered Oct 14 '14 at 12:01

-1

Add an else statement after the Picasso.with().load().into() statement. Add else holder.ivLogo.setImageBitmap(null);. Or use a placeholder bitmap.

Having seen the solution of Octa George it is better to always execute holder.ivLogo.setImageBitmap(placeholderbitmap); before the call to Picasso. Otherwise when Picasso 'takes his time' you would first see a wrong recycled image.

answered Oct 14 '14 at 12:01

greenapps

11,154
2
16
19

1

This will not fix the problem. There's a still a potential for the incorrect image to be displayed because Picasso does not know the image view was recycled. – Jake Wharton Oct 14 '14 at 14:57
@Jake There is no reason to downvote an answer if it is not complete. It has helped to greatly reduce the problem of the OP as you can read in his comments. And it was on 'invitation'. – greenapps Oct 15 '14 at 12:41
6

The answer is incorrect and contains the same bug as the question. In today's rampant copy/paste programming I want to make sure no one uses this solution. – Jake Wharton Oct 15 '14 at 14:28

Picasso loads into wrong imageview when using view holder pattern

4 Answers4

Linked