Why are wrong images getting loaded in ListView occasionally?

Question

I have a listview which loads images in every cell in async. When I try to scroll down slowly(after all the images in the current view are loaded), it works flawlessly. But when I try to scroll down before they are even loaded and scroll up, I face this issue. The cells start to show up images which don't correspond to them.

My getView method looks like this:

public View getView(final int position, View convertView, ViewGroup parent) {

    // TODO Auto-generated method stub

    View rowView = null;

    if(convertView == null) {
        rowView = inflater.inflate(R.layout.list_posts_item, null);
        final Holder holder=new Holder();
        holder.tvTitle=(TextView) rowView.findViewById(R.id.tvTitleNamePost);
        holder.ivPrimaryImage=(ImageView) rowView.findViewById(R.id.ivPrimaryImage);
        holder.tvLocality=(TextView) rowView.findViewById(R.id.tvLocalityPosts);
        holder.tvDateCreated=(TextView) rowView.findViewById(R.id.tvDateCreated);
        rowView.setTag(holder);
    }else {
        rowView=convertView;
    }

    Holder holder = (Holder)rowView.getTag();
    holder.ivPrimaryImage.setId(position);
    holder.ivPrimaryImage.setTag(listOfPosts.get(position).getPostId());
    holder.ivPrimaryImage.setImageBitmap(null); // Added for flickering issue
    holder.tvTitle.setText(listOfPosts.get(position).getTitle());
    holder.tvLocality.setText(listOfPosts.get(position).getLocality());
    holder.tvDateCreated.setText(listOfPosts.get(position).getCreatedDate());
    postId = listOfPosts.get(position).getPostId();
    Image image = new Image();
    image.setImg(holder.ivPrimaryImage);

    if (!"N".equalsIgnoreCase(listOfPosts.get(position).getHasImage()) ) {
        if(!tagsCaching.containsKey(postId))
            new GetPrimaryImages().execute(image);
        else
             holder.ivPrimaryImage.setImageBitmap(tagsCaching.get(postId));
    }

    return rowView;
}

And my Async call class looks like this:

public class GetPrimaryImages extends AsyncTask<Image, Void, Bitmap> {

ImageView imageView = null;
    protected Bitmap doInBackground(Image... images) {
    this.imageView=images[0].getImg();



        // Building Parameters
        List<NameValuePair> params = new ArrayList<NameValuePair>();
        params.add(new BasicNameValuePair("postid",(String)(this.imageView.getTag()) ));


             json = jsonParser.makeHttpRequest(CommonResources.getURL("get_primary_image"),
                    "POST", params);


        if(json == null){
            return null;
        }
        Log.d("Fetching Image",imageView.getTag()+ json.toString());
        tagsDownloaded.add((String)imageView.getTag());
        // check for success tag
        String TAG_SUCCESS = "success";
        try {
            int success = json.getInt(TAG_SUCCESS);

            if (success == 0) {
               image =  json.getString("primaryimage");

            }
        } catch (JSONException e) {
            e.printStackTrace();
        }

        return getImage(image);

    }

    /**
     * After completing background task Dismiss the progress dialog
     * **/
    protected void onPostExecute(Bitmap result) {
        tagsCaching.put((String)imageView.getTag(), result);
        imageView.setImageBitmap(result);

    }

    public Bitmap getImage(String imageString) {
        if("null".equalsIgnoreCase(imageString)){
            return null;
        }else{
            byte[] decodedString = Base64.decode(imageString, Base64.DEFAULT);
            Bitmap decodedByte = BitmapFactory.decodeByteArray(decodedString, 0, decodedString.length);
            //image.setImageBitmap(decodedByte);
            return decodedByte;
        }

    }


}

Edit:

I added a new instance variable to Holder:

public class Holder
{
    TextView tvTitle;
    ImageView ivPrimaryImage;
    TextView tvLocality;
    TextView tvDateCreated;
    int position;
}

Set the same in the getView: holder.position = position; And passed the holder object to the Async task: new GetPrimaryImages(position, holder).execute(image);

And modified the Async call class as follows: 1. Added cancel to the http call 2. Changed the onPostExecute method

  public class GetPrimaryImages extends AsyncTask<Image, Void, Bitmap> {

    int mPosition;
    Holder mHolder;
    public GetPrimaryImages(int position, Holder holder){
        mPosition = position;
        mHolder = holder;
    }

    ImageView imageView = null;
    protected Bitmap doInBackground(Image... images) {
    this.imageView=images[0].getImg();



        List<NameValuePair> params = new ArrayList<NameValuePair>();
        params.add(new BasicNameValuePair("postid",(String)(this.imageView.getTag()) ));


        JSONObject json;
        if(mHolder.position == mPosition)
             json = jsonParser.makeHttpRequest(CommonResources.getURL("get_primary_image"),
                    "POST", params);

        else {
            json = null;
            cancel(true);
        }

        // check log cat fro response
        if(json == null){
            return null;
        }
        Log.d("Fetching Image",imageView.getTag()+ json.toString());
        tagsDownloaded.add((String)imageView.getTag());
        // check for success tag
        String TAG_SUCCESS = "success";
        try {
            int success = json.getInt(TAG_SUCCESS);

            if (success == 0) {
               image =  json.getString("primaryimage");

            }
        } catch (JSONException e) {
            e.printStackTrace();
        }

        return getImage(image);

    }


    protected void onPostExecute(Bitmap result) {
        if (mHolder.position == mPosition) {
            tagsCaching.put((String) imageView.getTag(), result);
            imageView.setImageBitmap(result);
        }

    }

    public Bitmap getImage(String imageString) {


        //needs to wait
        if("null".equalsIgnoreCase(imageString)){
            return null;
        }else{
            byte[] decodedString = Base64.decode(imageString, Base64.DEFAULT);
            Bitmap decodedByte = BitmapFactory.decodeByteArray(decodedString, 0, decodedString.length);
            //image.setImageBitmap(decodedByte);
            return decodedByte;
        }

    }


}

It seems to be working. :)

Now my doubt is what would be the best way to cache the images? Should be writing it to a file? and reading it from it every time I scroll up?

Answer 1

The problem is, while your async task ends its background operation, the element it was linked to has been recycled to hold another element of your collection.

Let's focus on elements position, and let's say your listview can display up to 4 elements.

The first time the listview calls getview for the first 4 elements, and four asynctasks are created and run. Then you scroll to shouw positions 11 - 15, and the first element (the one related to position 1) gets recycled for position 11 before the asynctask ends.

Then the asynctask ends, and what you see is the image related to post 11 with the bitmap related to post 1.

A way to avoid this is knowing in the asynctask that the view was recycled, as suggested in this old post from Lucas Rocha.

Performance tips with listview

Check the post for insights on how listview works too:

Answer 2

Main "problem" is with ListViews implementation of reusing views and serial providing of AsyncTasks.

1) In ListView's adapter you correctly implement reusing of items. In ListView there are rendered only few items (items visible on screen + few top and down). If you start scrolling items which went out of screen are destroyed and theirs views are passed asi parameter to public View getView(final int position, View convertView, ViewGroup parent) as convertView.

This is first problem. List is reusing items.

2) Second thing is that AsyncTask is performed on another thread but more instances of asyncTask are performed on the same thread. it means they are performed serially. If you scroll quickly enough then you can make more requests on loading images with AsyncTask. At one time, there can be only few (i think 5) AsyncTask's jobs waiting in queue. Another ariving is ignored. So if you swipe quick enough you get this queue of 5 full and another image requests are ignored.

And this is It. You scroll, you start loading few images and the new displayed images are ignored. When you stop after while all AsyncTasks end and you got some "random" (previosly displaying) image loaded in your list item.

Sollution

This thing was discussed manny times and is solved. It is enough to use for example Picaso library:

https://futurestud.io/blog/picasso-adapter-use-for-listview-gridview-etc/

Answer 3

To add the answers, I would implement an image cache (e.g. as an URL-to-WeakReference-to-image hashmap). The getView() would access that cache and, if the image is not there, leave a request. When the image is loaded, the cache would examine the request list and notify the views that posted the requests (passing them both URL and the image). The views would compare the URL passed in notification to their current URL and either use the image or ignore it (if the view went out of screen or was reused, the image must be ignored).

Why request list. It is possible that multiple views manage to request some image and get reused before the image is loaded (especially if you scroll the list up and down several times).