Images are repeating in ListView

Question

I have implemented android app which should download images from server and display them in ListView, but very interesting thing occures while images are downloading

As you can see in video pictures which haven't been downloaded yet are represented by those which have been already downloaded. How that can happen? I've thinking about it almost two days.

http://www.youtube.com/watch?v=lxY-HAuJO0o&feature=youtu.be

here is my code of ListView adapter.

public class MoviesAdapter extends ArrayAdapter<ParkCinema> {
        private ArrayList<ParkCinema> movieDataItems;   
        private Activity context;

        public MoviesAdapter(Activity context, int textViewResourceId, ArrayList<ParkCinema> movieDataItems) {
            super(context, textViewResourceId, movieDataItems);
            this.context = context;
            this.movieDataItems = movieDataItems;
        }

        @Override
        public View getView(int position, View convertView, ViewGroup parent) { 
            if (convertView == null) {
                LayoutInflater vi = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
                convertView = vi.inflate(R.layout.movie_data_row, null);
                }

            ParkCinema movie = movieDataItems.get(position);

            if (movie!=null){
                        ImageView imageView = (ImageView) convertView.findViewById(R.id.movie_thumb_icon);
                        String url = movie.poster();

                         if (url!=null) {
                            Bitmap bitmap = fetchBitmapFromCache(url);
                            if (bitmap==null) { 
                                new BitmapDownloaderTask(imageView).execute(url);
                            }
                            else {
                                imageView.setImageBitmap(bitmap);
                            } 
                        } 
            }
            return convertView;
        }

        private LinkedHashMap<String, Bitmap> bitmapCache = new LinkedHashMap<String, Bitmap>();

        private void addBitmapToCache(String url, Bitmap bitmap) {
            if (bitmap != null) {
                synchronized (bitmapCache) {
                    bitmapCache.put(url, bitmap);
                }
            }
        }

        private Bitmap fetchBitmapFromCache(String url) {

            synchronized (bitmapCache) {
                final Bitmap bitmap = bitmapCache.get(url);
                 if (bitmap != null) {
                    return bitmap;
                } 
            }

            return null;

        }


    private class BitmapDownloaderTask extends AsyncTask<String, Void, Bitmap> {

            private String url;
            private final WeakReference<ImageView> imageViewReference;

            public BitmapDownloaderTask(ImageView imageView) {
                imageViewReference = new WeakReference<ImageView>(imageView);
            }

            @Override
            protected Bitmap doInBackground (String... source) {
                url = source[0];
                Bitmap image;
                try{
                    image = BitmapFactory.decodeStream(new URL(url).openConnection().getInputStream());
                    return image;
                    }
                catch(Exception e){Log.e("Error", e.getMessage()); e.printStackTrace();}
                return null;
                } 


            @Override
            protected void onPostExecute(Bitmap bitmap) {       
                addBitmapToCache(url, bitmap);
                imageViewReference.get().setImageBitmap(bitmap);               
            }
        }
    }

Edit 3:

public View getView(int position, View convertView, ViewGroup parent) { 
    if (convertView == null) {
        LayoutInflater vi = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE); 
        convertView = vi.inflate(R.layout.movie_data_row, null);
        }
    ParkCinema movie = movieDataItems.get(position);
    ImageView imageView = (ImageView) convertView.findViewById(R.id.movie_thumb_icon);
    if (movie!=null){
                String url = movie.poster();

                    if (url != null) {
                        Bitmap bitmap = fetchBitmapFromCache(url);
                        if (bitmap == null) {
                            imageView.setImageResource(R.drawable.no_image);
                            new BitmapDownloaderTask(imageView).execute(url);
                        }
                        else {
                            imageView.setImageBitmap(bitmap);
                        }
                    }
                    else {
                        imageView.setImageResource(R.drawable.no_image);
                    }
                }
                else {
                    imageView.setImageResource(R.drawable.no_image);
                } 

    return convertView;

}

After `ImageView imageView = (ImageView) convertView.findViewById(R.id.movie_thumb_icon);`, add `imageView.setImageBitmap(null);`. What you are seeing is a normal behaviour — Sherif elKhatib, Mar 14 '13 at 15:03
Thanks for answering. If I'll set null to imageView same thing happens — Andrew Rahimov, Mar 14 '13 at 15:07
sorry! use `imageView.setImageDrawable(null)` or `imageView.setImageResource(0)` — Sherif elKhatib, Mar 14 '13 at 15:12
are you adding it in the right place? just after `ImageView imageView = (ImageView) convertView.findViewById(R.id.movie_thumb_icon);` — Sherif elKhatib, Mar 14 '13 at 16:23
I had the same problem and `imageView.setImageBitmap(null);` worked for me. But why is this "normal behaviour"? — Stefan Arn, Jun 11 '14 at 12:54

Steven Byle · Accepted Answer · 2013-03-14T18:32:02.077

Aha! I think I may know the issue. Right now, your getView method sets your ImageView like this:

Gets movie object at position
Pulls out the movie's thumbnail url
Using that url, it tries to find the image in the cache
If it finds the image, it sets it
If it can't find the image, it starts an async network request to go get it, and sets it after it gets downloaded.

Your issus arises since ListView reuses its rows' Views. When the first View scrolls off the screen, rather than inflate a new one, ListView passes the now offscreen row's View in as convertView for you to reuse (this is for efficiency).

When your getView gets a convertView that is getting reused, its ImageView has already been set from the row that had it before, so you see the old image from the offscreen row's View. With your current getView process, you check for the new row's image, and it doesn't find it in the cache, it starts a request to download it. While it is downloading, you see the old image until you get the new image.

To fix this, you need to make sure you set every field in the row's View immediately, to make sure you don't have any Views showing stale data. I would suggest you set the ImageView to the default drawable resource (you have set in your R.layout.movie_data_row) while you wait for the network download to get the image.

@Override
public View getView(int position, View convertView, ViewGroup parent) {
    if (convertView == null) {
        LayoutInflater vi = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        convertView = vi.inflate(R.layout.movie_data_row, null);
    }

    ParkCinema movie = movieDataItems.get(position);
    ImageView imageView = (ImageView) convertView.findViewById(R.id.movie_thumb_icon);
    if (movie != null) {
        String url = movie.poster();

        if (url != null) {
            Bitmap bitmap = fetchBitmapFromCache(url);
            if (bitmap == null) {
                // Set the movie thumbnail to the default icon while we load
                // the real image
                imageView.setImageResource(R.drawable.movie_thumb_icon);
                new BitmapDownloaderTask(imageView).execute(url);
            }
            else {
                // Set the image to the bitmap we get from the cache
                imageView.setImageBitmap(bitmap);
            }
        }
        else {
            // Set the movie thumbnail to the default icon, since it doesn't
            // have a thumbnail URL
            imageView.setImageResource(R.drawable.movie_thumb_icon);
        }
    }
    else {
        // Set the movie thumbnail to the default icon, since there's no
        // movie data for this row
        imageView.setImageResource(R.drawable.movie_thumb_icon);
    }

-Edit-

Updated to be even more robust, using your drawable. You also have an issue with your BitmapDownloaderTask, it does not handle errors/null. Try adding this as well.

@Override
protected void onPostExecute(Bitmap bitmap) { 
    addBitmapToCache(url, bitmap);
    if (bitmap == null) {
        // Set the movie thumbnail to the default icon, since an error occurred while downloading
        imageViewReference.get().setImageResource(R.drawable.movie_thumb_icon);
    }
    else {
        imageViewReference.get().setImageBitmap(bitmap);
    }            
}

Thank you for so detailed response. I've modified code as you told, but the same thing happens :( I think ListView sets convertView to new row object and shows it before reaching the part of code which we have modified now, so it displays old image. — Andrew Rahimov, Mar 14 '13 at 17:35
I have implemeted Pramod J George's suggested method, it solved my problem, but created another one. Now app runs slower in emulator, and I don't like it. Just want to be everything perfect :/ — Andrew Rahimov, Mar 14 '13 at 17:42
Pramod J George's suggestion is a hack, it's like smashing through a wall vs using the door. You will get lag when you scroll using that method, because it is not reusing `View`s. Can you edit the question and show the code that you have attempted with my method? I'm feeling like there might be a small piece missing. — Steven Byle, Mar 14 '13 at 17:54
Done! Also I've looked up two examples from here - http://stackoverflow.com/questions/541966/how-do-i-do-a-lazy-load-of-images-in-listview In both of them images are downloading without lag. I've read the source code, and I think they are using almost the same implementation of adapter as mine, but their code works well :/ — Andrew Rahimov, Mar 14 '13 at 18:04
You need to copy my methods **exactly**, you did not completely implement what I suggested the first time. Please copy my methods and paste over yours, and if they do not work, please explain how it is behaving. I have image loading like this working in my applications just fine, so something you are doing must be off. — Steven Byle, Mar 14 '13 at 18:35
I've changed everything as you told and checked it twice. (Edit 3 in my post) But the problem still exists, and some images are flipping 2-3 times while downloading :( — Andrew Rahimov, Mar 14 '13 at 19:02
Ahhh I think what is happening is that you are scrolling faster the `AsyncTask`s can finish, and they aren't being cancelled. So if you scroll and the same row gets reused 3 times and starts 3 network calls, you will see the image get set multiple times since each `AsyncTask` share the same reference to the same `ImageView`, and are setting it when they finish. I am using the google lib for loading/caching images, which appears to handle this issue - https://code.google.com/p/libs-for-android/wiki/ImageLoader. Also check out http://developer.android.com/shareables/training/BitmapFun.zip. — Steven Byle, Mar 14 '13 at 20:08
I don't understand the source code of those libs you have provided :( There is so much stuff compared to my implementation, and so I can't catch that difference which makes those libs working properly — Andrew Rahimov, Mar 15 '13 at 15:27
This is a complicated issue, which is why Google took the time to make the lib. Basically they handle the case where your image may be done loading, but now the `ImageView` has already been recycled, so they don't set it and you don't see the image changed several times. I checked my source and noticed that check is in there. You would have to implement it yourself, which would require a bit of work. I'd suggest you dig into their example/wiki and use their impl, as it works excellently for me. — Steven Byle, Mar 15 '13 at 15:33
Thanks for nice explanation. Every time I confused with google answers. May be I will not for next time from yours. Thanks — shyam.y, Jul 25 '15 at 03:41

score 1 · Answer 2 · answered Jul 21 '13 at 17:48

i had this issue and implemented lruCache ...i believe you need api 12 and above or use the compatiblity v4 library. lurCache is fast memory but it also has a budget, so if your worried about that you can use a diskcache ...its all described here http://developer.android.com/training/displaying-bitmaps/cache-bitmap.html

I'll now provide my implementation which is a singleton i call from anywhere like this:

//where first is a string and other is a imageview to load

DownloadImageTask.getInstance().loadBitmap(avatarURL, iv_avatar);

here's the ideal code to cache and then call the above in getView of an adapter when retrieving the web image:

 public class DownloadImageTask {

private LruCache<String, Bitmap> mMemoryCache;

/* create a singleton class to call this from multiple classes */

private static DownloadImageTask instance = null;

public static DownloadImageTask getInstance() {
    if (instance == null) {
        instance = new DownloadImageTask();
    }
    return instance;
}

//lock the constructor from public instances
private DownloadImageTask() {

    // Get max available VM memory, exceeding this amount will throw an
    // OutOfMemory exception. Stored in kilobytes as LruCache takes an
    // int in its constructor.
    final int maxMemory = (int) (Runtime.getRuntime().maxMemory() / 1024);

    // Use 1/8th of the available memory for this memory cache.
    final int cacheSize = maxMemory / 8;

    mMemoryCache = new LruCache<String, Bitmap>(cacheSize) {
        @Override
        protected int sizeOf(String key, Bitmap bitmap) {
            // The cache size will be measured in kilobytes rather than
            // number of items.
            return bitmap.getByteCount() / 1024;
        }
    };

}

public void loadBitmap(String avatarURL, ImageView imageView) {
    final String imageKey = String.valueOf(avatarURL);

    final Bitmap bitmap = getBitmapFromMemCache(imageKey);
    if (bitmap != null) {
        imageView.setImageBitmap(bitmap);
    } else {
        imageView.setImageResource(R.drawable.ic_launcher);

        new DownloadImageTaskViaWeb(imageView).execute(avatarURL);
    }
}

private void addBitmapToMemoryCache(String key, Bitmap bitmap) {
    if (getBitmapFromMemCache(key) == null) {
        mMemoryCache.put(key, bitmap);
    }
}

private Bitmap getBitmapFromMemCache(String key) {
    return mMemoryCache.get(key);
}

/* a background process that opens a http stream and decodes a web image. */

class DownloadImageTaskViaWeb extends AsyncTask<String, Void, Bitmap> {
    ImageView bmImage;

    public DownloadImageTaskViaWeb(ImageView bmImage) {
        this.bmImage = bmImage;
    }

    protected Bitmap doInBackground(String... urls) {

        String urldisplay = urls[0];
        Bitmap mIcon = null;
        try {
            InputStream in = new java.net.URL(urldisplay).openStream();
            mIcon = BitmapFactory.decodeStream(in);

        } catch (Exception e) {
            Log.e("Error", e.getMessage());
            e.printStackTrace();
        }

        addBitmapToMemoryCache(String.valueOf(urldisplay), mIcon);

        return mIcon;
    }

    /* after decoding we update the view on the mainUI */
    protected void onPostExecute(Bitmap result) {
        bmImage.setImageBitmap(result);

    }

}

}

score 0 · Answer 3 · answered Mar 14 '13 at 15:13

Views are reused for performance with Adapters. You should use another approch . You have to have a class holder which reuse your views. In your case you class should be something like this:

   public class MoviesAdapter extends ArrayAdapter<ParkCinema> {
    private ArrayList<ParkCinema> movieDataItems;   
    private Activity context;

    public MoviesAdapter(Activity context, int textViewResourceId,       ArrayList<ParkCinema> movieDataItems) {
        super(context, textViewResourceId, movieDataItems);
        this.context = context;
        this.movieDataItems = movieDataItems;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) { 
        if (convertView == null) {
            LayoutInflater vi = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
            convertView = vi.inflate(R.layout.movie_data_row, null);

                holder = new ViewHolder();

                holder.imageView = (BarImageView) convertView.findViewById(R.id.movie_thumb_icon);
              } else {
                        holder = (ViewHolder) convertView.getTag();

                  }

        ParkCinema movie = movieDataItems.get(position);

        if (movie!=null){

                    String url = movie.poster();

                     if (url!=null) {
                        Bitmap bitmap = fetchBitmapFromCache(url);
                        if (bitmap==null) { 
                            new BitmapDownloaderTask(imageView).execute(url);
                        }
                        else {
                            imageView.setImageBitmap(bitmap);
                        } 
                    } 
        }
        return convertView;
    }

    private LinkedHashMap<String, Bitmap> bitmapCache = new LinkedHashMap<String, Bitmap>();

    private void addBitmapToCache(String url, Bitmap bitmap) {
        if (bitmap != null) {
            synchronized (bitmapCache) {
                bitmapCache.put(url, bitmap);
            }
        }
    }

    private Bitmap fetchBitmapFromCache(String url) {

        synchronized (bitmapCache) {
            final Bitmap bitmap = bitmapCache.get(url);
             if (bitmap != null) {
                return bitmap;
            } 
        }

        return null;


public static class ViewHolder {


        ImageView imageView; 


    }

    }

When getView gets called the first time your views are initialized(inside if (convertView == null) { ). Then, for the next rows the Views are not instantiated anymore but reused. This is the reason why you got your image replicated. — Gyonder, Mar 14 '13 at 15:37
and what does viewholder do in this situation? It creates new ImageView objests for every row? — Andrew Rahimov, Mar 14 '13 at 15:40
viewholder holds the objects that were created for the first row — Gyonder, Mar 14 '13 at 15:43
sorry, but I didn't get that.. In my code ImageView object is reused for all rows excluding first, but if I'll use ViewHolder will it change something? — Andrew Rahimov, Mar 14 '13 at 15:52

score 0 · Answer 4 · answered Jan 13 '14 at 04:47

I have spent hours trying to figure this one out as well...Thanks to Steven Byle's solution... Here is my solution to something similar when a user selects an item from a list:

adapter.setSelectedIndex(position);

then in the custom adapter:

public void setSelectedIndex(int ind)
{
    selectedIndex = ind;
    notifyDataSetChanged();
}

and then finally in the getView method of the adapter:

if(selectedIndex!= -1 && position == selectedIndex)
         {
             holder.tab.setBackgroundColor(Color.BLACK);
         }
         else{
             holder.tab.setBackgroundColor(Color.DKGRAY);
         }

So in conclusion make sure you assign default values

score 0 · Answer 5 · answered Apr 01 '16 at 07:32

0

In my case i used Picasso library instead of AsyncTask for downloading image. enter link description here

Also write if else condition, that is set null to image if url is not available

answered Apr 01 '16 at 07:32

badal soyantar

1
2

Welcome to SO. You should add the images here instead of linking to them. If a link stops working, the answer is lost. – Karl Gjertsen Apr 01 '16 at 07:55

score -2 · Answer 6 · answered Mar 14 '13 at 15:56

-2

instead of using the convertview object create a new view each time.

View localView = ((LayoutInflater)parentscreen.getSystemService("layout_inflater")).inflate(R.layout.activity_list_row, null);

By inflating as above.

answered Mar 14 '13 at 15:56

Pramod J George

1,723
12
24

Won't this consume much memory? – Andrew Rahimov Mar 14 '13 at 16:00
Tried your solution, but nothing changed. Maybe I'm doing something wrong,Can you post more detailed example? – Andrew Rahimov Mar 14 '13 at 16:37
Inflating the row layout every time is bad practice. `ListView` reuses row layouts and passes them in as `convertView`. That part of his code is correct, the issue does not have to do with his inflating code. – Steven Byle Mar 14 '13 at 16:44
Can you explain why it is bad practice? is there any other solution? – Andrew Rahimov Mar 14 '13 at 16:53

Images are repeating in ListView

6 Answers6

-Edit-

Linked