1

I'm using UIL to load images from URL to Gridview, but the problem is that images and items get duplicated or loaded on many wrong positions.

the item that is half way off-screen gets duplicated.

enter image description here

this is the GridView Adapter:

public class GridViewAdapter extends BaseAdapter {

private Activity _activity;
private ArrayList<Object_Wallpaper> wallpapersList;
private ImageLoader mImageLoader;
GridViewAdapter adapter;
ViewHolder holder;
Object_Wallpaper wall;

public GridViewAdapter(Activity activity, ArrayList<Object_Wallpaper> wallpapersList,
        int imageWidth) {
    this._activity = activity;
    this.wallpapersList = new ArrayList<Object_Wallpaper>();
    this.wallpapersList = wallpapersList;
    this.adapter = this;
    this.mImageLoader = ImageLoader.getInstance();
}

@Override
public int getCount() {
    return this.wallpapersList.size();
}

@Override
public Object getItem(int position) {
    return wallpapersList.get(position);
}

@Override
public long getItemId(int position) {
    return position;
}

/*private view holder class*/
private class ViewHolder {
    ImageView smallThumb;
    ProgressBar pb;
}

@Override
public View getView(final int position, View convertView, ViewGroup parent) {
    
    holder = new ViewHolder();
    if (convertView == null) {
        LayoutInflater inflater = _activity.getLayoutInflater();
        convertView = inflater.inflate(R.layout.grid_item_photo, null);
        holder.smallThumb = (ImageView) convertView.findViewById(R.id.imgThumbnail);
        holder.pb = (ProgressBar) convertView.findViewById(R.id.progressBar1);
        convertView.setTag(holder);
    } else {
        holder = (ViewHolder) convertView.getTag();
    }
    wall = wallpapersList.get(position);
    
    showImage(position);
    
    return convertView;
}

private void showImage(final int position){
    if(!wall.isDownloaded){
        mImageLoader.displayImage(wall.smallURL, holder.smallThumb, null,
                new ImageLoadingListener() {
            
            @Override
            public void onLoadingStarted(String imageUri, View view) {
                holder.pb.setVisibility(View.VISIBLE); 
            }
            
            @Override
            public void onLoadingFailed(String imageUri, View view,
                    FailReason failReason) {
                holder.pb.setVisibility(View.GONE);
            }

            @Override
            public void onLoadingComplete(String imageUri,
                                          View view, Bitmap loadedImage) {
                holder.pb.setVisibility(View.GONE);
                wall.isDownloaded = true;
                wallpapersList.set(position, wall);
            }

            @Override
            public void onLoadingCancelled(String imageUri, View view) {
            }
            
            
        }, new ImageLoadingProgressListener(){

            @Override
            public void onProgressUpdate(String imageUri, View view,
                    int current, int total) {
                holder.pb.setProgress(Math.round(100.0f * current / total));
            }
            
        });
    } else {
        mImageLoader.displayImage(wall.smallURL, holder.smallThumb);
        holder.pb.setVisibility(View.GONE);
    }
}

}

And this is the object:

public class Object_Wallpaper implements Serializable {
private static final long serialVersionUID = 1L;
public String id,title,tags,fullResURL, thumbURL, smallURL;
public boolean isDownloaded;

public Object_Wallpaper(String id, String title, String tags,
                        String fullResURL, String thumbURL, String smallURL) {      
    this.id = id;
    this.title = title;
    this.tags = tags;
    this.fullResURL = fullResURL;
    this.thumbURL = thumbURL;
    this.smallURL = smallURL;
    this.isDownloaded = false;
}
}

can you tell me what things have I done wrong so far please? thanks. I saw same questions elsewhere, but they didn't help either.

EDIT:

I realized that this happens only in fast scroll. what should I do?

Community
  • 1
  • 1
M D P
  • 4,525
  • 2
  • 23
  • 35

2 Answers2

2

The problem is that your adapter class has single instances of fields (holder and wall), where you need a copy of these variables for each call to getView. As it is, when the image loader finishes each image, that image is displayed in whatever item was last called. Does that make sense?

The first step is to change these fields to local variables in getView, and parameters to showImage. You would still have problems after this step when you scroll down your list, because Android will try to recycle your views during a scroll, and so when the image loader finishes loading the image for a position that has been scrolled out of view, it will display the loaded image in whatever position its view got recycled in. To solve this you can use a HashMap (or the more efficient SparseArray) to keep track of which recycled views are currently representing which positions.

private SparseArray<ViewHolder> holderMap = new SparseArray<ViewHolder>();
@Override
public View getView(final int position, View convertView, ViewGroup parent) {
    ViewHolder holder;
    if (convertView == null) {
        holder = new ViewHolder();
        LayoutInflater inflater = _activity.getLayoutInflater();
        convertView = inflater.inflate(R.layout.grid_item_photo, null);
        holder.smallThumb = (ImageView) convertView.findViewById(R.id.imgThumbnail);
        holder.pb = (ProgressBar) convertView.findViewById(R.id.progressBar1);
        convertView.setTag(holder);
    } else {
        holder = (ViewHolder) convertView.getTag();
        // If recycled, remove the holder's previous position from map
        int oldPosition = holderMap.indexOfValue(holder);
        if (oldPosition >= 0) {
          holderMap.remove(oldPosition);
        }
    }
    // Keep track of which view is representing this position
    holderMap.put(position, holder);

    Object_Wallpaper wall = wallpapersList.get(position);
    showImage(position, holderMap, wall);

    return convertView;
}

private void showImage(final int position, final SparseArray<ViewHolder> holderMap, final Object_Wallpaper wall) {
    if(!wall.isDownloaded){
        mImageLoader.displayImage(wall.smallURL, holder.smallThumb, null,
                new ImageLoadingListener() {

            @Override
            public void onLoadingStarted(String imageUri, View view) {
                ViewHolder holder = holderMap.get(position);
                if (holder != null) {
                    holder.pb.setVisibility(View.VISIBLE); 
                }
            }

            @Override
            public void onLoadingFailed(String imageUri, View view,
                    FailReason failReason) {
                ViewHolder holder = holderMap.get(position);
                if (holder != null) {
                    holder.pb.setVisibility(View.GONE); 
                }
            }

            @Override
            public void onLoadingComplete(String imageUri,
                                          View view, Bitmap loadedImage) {
                ViewHolder holder = holderMap.get(position);
                if (holder != null) {
                    holder.pb.setVisibility(View.GONE); 
                    wall.isDownloaded = true;
                    wallpapersList.set(position, wall);
                }
            }

            @Override
            public void onLoadingCancelled(String imageUri, View view) {
            }


        }, new ImageLoadingProgressListener(){

            @Override
            public void onProgressUpdate(String imageUri, View view,
                    int current, int total) {
                ViewHolder holder = holderMap.get(position);
                if (holder != null) {
                    holder.pb.setProgress(Math.round(100.0f * current / total));
                }
            }

        });
    } else {
        mImageLoader.displayImage(wall.smallURL, holder.smallThumb);
        holder.pb.setVisibility(View.GONE);
    }
}

EDIT: Please see my previous answer to a similar problem (where I used a SparseArray called ivMap -- a slightly better option than a HashMap when the key is an int). Android BaseAdapter With LruCache Some ui problems

2nd EDIT: modified code with adapted map solution.

Community
  • 1
  • 1
Bruce
  • 2,377
  • 1
  • 17
  • 11
  • nope it seems like it works only if I don't scroll, when I scroll down, it shows items were loading, on wrong positions, it even shows previously loaded images on slots that are not yet loaded – M D P Dec 31 '14 at 00:46
  • Right, that sounds like what I warned about, with the recycled views. Have you looked at the answer in the link I posted? You can use a map (called ivMap in my other answer), maintain it inside getView, and refer to it when the image loader finishes. – Bruce Dec 31 '14 at 00:50
  • yes, but see here: wallpapersList.set(position, wall); I already have something like that, it's doing the same think, it's not helping. – M D P Dec 31 '14 at 00:54
  • No, it's doing a different thing. wallpapersList is basically your data: which wallpaper belongs at a particular position. You need the ivMap to say which recycled view is representing a particular position (which may be none). – Bruce Dec 31 '14 at 00:59
  • but it should not be that much complicated, it should be a lot easier, I know there is a way to get it work easily but I don't know what's the problem. – M D P Dec 31 '14 at 01:26
  • Well, working with multiple threads is tricky. You're loading an image in the background, and then wanting to modify the views on the UI thread, but the views might have changed because the user scrolled. That's a tricky problem, and I don't know of another way to solve it. – Bruce Dec 31 '14 at 01:31
  • the problem is in returning the wrong position number while scrolling. – M D P Dec 31 '14 at 01:34
0

Java is pass-by-reference in assigning objects. In getView method you are assigning new value to the same pointer for previous objects:

 wall = wallpapersList.get(position);

You should create new instance each time. It should be changed to:

 wall = new Object_Wallpaper();
 wall = wallpapersList.get(position);
M D P
  • 4,525
  • 2
  • 23
  • 35