8

this must be somethng very simple I'm overlooking, but I have the following problem (the post is rather lengthy, but I want to provide as much info as possible :) ).

I have a gridview in my Android application where each cell holds custom view:

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout
  xmlns:android="http://schemas.android.com/apk/res/android"
  android:orientation="vertical"
  android:layout_width="fill_parent"
  android:layout_height="fill_parent"
  >
    <GridView
    android:id = "@+id/photosGridView"
    android:layout_width="fill_parent"
    android:layout_height="fill_parent" 
    android:clickable="true" 
    android:drawSelectorOnTop="true" 
    android:focusable="true" 
    android:focusableInTouchMode="true" 
    android:numColumns="6" 
    android:columnWidth="90dp"
    android:verticalSpacing="5dp"
    android:horizontalSpacing="5dp"
    android:stretchMode="columnWidth"     
    >
</GridView>

</RelativeLayout>

and each cell is

<?xml version="1.0" encoding="utf-8"?>
<com.myapp.widgets.ImageThumbView
  xmlns:android="http://schemas.android.com/apk/res/android"
  android:layout_width="wrap_content"
  android:layout_height="wrap_content"
  android:background = "@android:color/transparent"
  android:paddingLeft = "1dip"
  android:paddingRight = "1dip"
  android:paddingTop = "2dip"
  android:paddingBottom = "2dip"
  >
<ImageView
    android:id="@+id/thumbImage"
    android:layout_width="fill_parent"
    android:layout_height = "fill_parent"
    android:src="@drawable/icon_small"
    /> 

  <RelativeLayout
    android:layout_width="fill_parent"
    android:layout_height = "fill_parent"
    android:background = "@android:color/transparent"
   >

  <ImageView
   android:id="@+id/iconRight"
   android:layout_width="40px"
   android:layout_height = "40px"  
   android:src="@drawable/album_check"
   android:visibility="gone"
   android:layout_alignParentTop = "true"
   android:layout_alignParentRight = "true"     
   />

  <ImageView
   android:id="@+id/iconLeft"
   android:src="@drawable/album_check"
   android:visibility="gone"
   android:layout_width = "40px"
   android:layout_height="40px"
   android:layout_alignParentTop = "true"
   android:layout_alignParentLeft = "true"     
   /> 
</RelativeLayout>

</com.myapp.widgets.ImageThumbView>

My adapter looks like this:

public class ImageAdapter extends BaseAdapter {
    private List<String> mPictures = null;

    public ImageAdapter(List<String> pictures) {
         mPictures = pictures;
    }

    public int getCount() {
          return mPictures != null ? mPictures.size() : 0;
    }
    public Object getItem(int position) {
          return mPictures != null ?  mPictures.get(position) : null;
    }
    public long getItemId(int position) {
          return mPictures != null ? position : -1;
    }
    @Override
    public View getView(int position,View convertView,ViewGroup parent) 
    {
        ImageThumbView i = null;
        try
            {               
            Thread.sleep(100);

            if (convertView == null) 
            {
                String path = mPictures.get(position);
                Log.d(((Integer)position).toString(), path);
                i = addSingleView(_li, path);
                TextView idx = (TextView) i.findViewById(R.id.caption);
                if (idx != null)
                    idx.setText(((Integer)position).toString());
            }
            else 
            {
                Log.d(((Integer)position).toString(), "ALREADY NOT NULL");
                i = (ImageThumbView) convertView;
                                    // These 2 lines were added only in desperate attempt to get it working, but it makes no difference
                String path = mPictures.get(position);
                i.updateView(path);
            }
        }
        catch(InterruptedException ie)
        {
            ie.printStackTrace();
        }
    return i;
        }
}

So initially it works properly, ie it shows first 18 images and few pixels from the next row. But when I start scrolling the gridvew, the images start to appear at random, ie after the last image I see few from the beginning and so on. Out of curiosity, I've tried few samples like this one: http://androidsamples.blogspot.com/2009/06/how-to-display-thumbnails-of-images.html ...and see the same result.

So, am I doing something wrong? why on earth would GridView display more items than it is supposed to do? and why do items appear at the wrong positions?

BR, Alex

alexey_gusev
  • 113
  • 1
  • 1
  • 6
  • checkout this link [http://stackoverflow.com/questions/11245620/items-inside-gridview-getting-repeated-when-screen-scrolls][1] [1]: http://stackoverflow.com/questions/11245620/items-inside-gridview-getting-repeated-when-screen-scrolls – vineeth Jun 06 '14 at 04:50

5 Answers5

21

Actually the problem here is that you set the content of the "convertView" within the if or else. Or you should always do that after instantiating the view if it is null and set the content only before returning the view.

Consequently, you're sure the content of the view is always the right one, updated using the position and not a false recycled view.

So your should generally speaking do the following:
(based on the tutorial from Android Guide here)

public View getView(int position, View convertView, ViewGroup parent) {
    ImageView imageView;
    if (convertView == null) { 
        imageView = new ImageView(mContext);
        //just creating the view if not already present
    } else {
        imageView = (ImageView) convertView;
        //re-using if already here
    }

    //here is the tricky part : set the content of the view out of the if, else
    //just before returning the view
    imageView.setImageResource(mThumbIds[position]);
    return imageView;
}
inazaruk
  • 74,247
  • 24
  • 188
  • 156
cedric
  • 241
  • 1
  • 5
  • 1
    i was getting the same problem but was not able to understand that but you make me understand that problem thanksssss, now i got ma code working by making convertview==null every time on refreshing the gridview thanks – varun bhardwaj Oct 18 '11 at 05:05
10

The answer is view recycling.

In general your getView should always be something of the form:

public class ImageAdapter extends BaseAdapter {

    private List<String> mUrls; // put your urls here
    private Map<String, Drawable> mImages; // cache your images here

    public ImageAdapter() {
        ...
        mUrls = new ArrayList<String>();
        mImages = new HashMap<String, Drawable>();
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        ViewHolder holder; // Use the ViewHolder pattern for efficiency

        if (convertView == null) {
            // first time this view has been created so inflate our layout
            convertView = View.inflate(this, R.layout.my_grid_item, null);
            holder = new ViewHolder();
            holder.image = convertView.findViewById(R.id.image);
            holder.text = convertView.findViewById(R.id.text);
            convertView.setTag(holder); // set the View holder
        } else {
           holder = (ViewHolder) convertView.getTag();
        }

        // update the current view - this must be done EVERY
        // time getView is called due to view recycling
        holder.text.setText(Integer.toString(position));

        // check our cache for the downloaded image
        final String url = mUrls.get(position);
        if (mImages.get(url) != null)
            holder.image.setImageDrawable(mImages.get(url));
        else
            loadImage(url, holder.image);

        // return our view
        return convertView;
    }

    public loadImage(final String url, final ImageView image) {
        // load an image (maybe do this using an AsyncTask
        // if you're loading from network
    }

    ...
}

Where your ViewHolder class would look something like

public class ViewHolder {
    ImageView thumbImage;
    TextView text;
}

Then you shouldn't run into any problems. Also I'm not sure why you needed to sleep in your getView? That will slow down scrolling of your GridView.

Joseph Earl
  • 23,351
  • 11
  • 76
  • 89
  • ok, I believe it was view recycling that killed my original code. Thanks a lot! – alexey_gusev Mar 20 '11 at 17:03
  • actually, I've spoken too soon. The proposed solution doesn't work because the main difference between it and the code I used is that it sets predefined drawables. But at least I now see the problem. – alexey_gusev Mar 20 '11 at 19:16
  • I've changed my code to hopefully suit your use case better. Let me know if it's any better. – Joseph Earl Mar 21 '11 at 20:58
  • 2
    Hello! I have the same code as you have but I still encounter the random image upon scrolling. Is this a known bug on gridview? – iamtheexp01 May 11 '12 at 07:15
  • if (convertView == null) { this part is very important. Avoided my java.lang.outofmemoryerror problem in my custom adapter. – Zafer Jul 04 '15 at 18:41
3

Use the standard Lazy Loader to get the images but never update onpostexecute, which causes the images to update too fast and get the randomness you don't desire.

I even used the code:

if (result != null && !mFlinging && myPosition < 12)
{   
   imageView.setImageBitmap(result);
}

int the onpostexecute which would then update the images correctly for the first screen, but if you fling the images screen then come back too quickly to the first screen get the randomness again so I now use the :

mPhotoView.setOnScrollListener(new OnScrollListener() { 



            public void onScroll(AbsListView view, int firstVisibleItem,
                    int visibleItemCount, int totalItemCount) {
                // TODO Auto-generated method stub

            }

            public void onScrollStateChanged(AbsListView view, int scrollState) {
                // TODO Auto-generated method stub
                if(scrollState != SCROLL_STATE_IDLE) {
                  mAdapter.setFlinging(true);
                } else {
                  mAdapter.setFlinging(false);   
                }

                int first = view.getFirstVisiblePosition(); 
                int count = view.getChildCount(); 

                if (scrollState == SCROLL_STATE_IDLE || (first + count > mAdapter.getCount()) ) { 
                    mPhotoView.invalidateViews(); 
                }

            } 
        }); 

to update the view.

0

I am experiencing the same problem. I'm moving in the direction of rendering views in the background;

I'm studying these codes, if should help:

http://blog.tomgibara.com/post/7665158012/android-adapter-view-rendering

https://github.com/desertjim/LazyLoadingGridView

mmmx
  • 521
  • 4
  • 7
-8

guys to avoid this problem always return newly created object in getView()

public View getView(int position, View convertView, ViewGroup parent) {
                ImageView       imageView;
                TextView        textView;
                LinearLayout    linearlayout  = new LinearLayout(mContext);
                //if (convertView == null) {
                    imageView       = new ImageView(mContext);
                    textView        = new TextView(mContext);

                    //imageView.setLayoutParams(new GridView.LayoutParams(85, 85));
                    imageView.setAdjustViewBounds(false);
                    imageView.setScaleType(ImageView.ScaleType.CENTER_CROP);
                    imageView.setPadding(4, 8, 8, 8);                   
                    imageView.setBackgroundResource(mThumbIds[position]);

                    textView.setText(mThumbText[position]);                
                    textView.setGravity(0x01);
                    textView.setMaxLines(2);

                    //textView.setPadding(20, 0, 0, 0);
                    linearlayout.addView(imageView,0);
                    linearlayout.addView(textView,1);
                    linearlayout.setPadding(4, 4, 4, 4);
                    linearlayout.setOrientation(LinearLayout.VERTICAL);
                    linearlayout.setGravity(0x01);

               // } else {
                    //linearlayout = (LinearLayout) convertView;
                //}

                return linearlayout;
            }
Ashay
  • 11
  • 1
  • 11
    It's a very bad idea to return a new object in each getView(). That will cause your list to stutter when you're scrolling as well as cause excessive garbage collection. – Matthew Runo Apr 11 '12 at 20:57