Android GridView loading the 0 indexed item in a later index's slot when data set changes

Question

Using a GridView with an extended BaseAdapter I am loading a collection of RelativeLayouts which contain an image and caption. When the user clicks on one of these items in the GridView, it should be removed from the grid and replaced by the items behind it.

This works fine for any item other than the first one (index 0 in the ArrayList of items which the adapter manages).

On the device I'm testing on, the screen is able to display 12 items when the screen is fully scrolled to the top of the grid (3 columns and 4 rows) with the last row slightly cut off. The 5th row isn't showing at all. In this situation, if I click on the first item (to remove it), all of the items fill in correctly, however, the first item of the 5th row (index 12) returns the view of the first item (index 0) instead of the item at index 12.

Some screens to help explain the issue:

Clicking on Brian McKnight will remove him, and BECK and the rest will fill in.

BECK has filled in along with the rest, now to scroll down to the next row.

Uh oh. BECK is there too. I left it partially scrolled so you can see that both items have the BECK picture.

Scrolling back up so item 12 (row 5) is out of view, then back down corrects the problem.

This process for any other item in the grid is working correctly, just item 0. I did some logging and found a couple of things:

• The getView(...) method for object 0 is called a lot. When the grid loads for the first time, it is called 3 times, whereas all of the other items are only called once.
• When an item other than item 0 is deleted, getView(...) for position 0 is called 19 times. 3 at first, then 16 more times after all of the other object's getView(...) method has been called.
• When item 0 is deleted, the getView(...) method for 0 is called twice AFTER you scroll down to the 5th row (item 12). I'm thinking it is here where the issue is occurring, because sometimes if you can see the correct item begin to load in before being overwritten by the item @ position 0.

So I'm pretty sure those extra 2 calls of getView(...) for position 0, called when the item @ index 12 appears, is what is causing this problem. What I don't know is why getView(...) for position 0 would be called at that time, and why it would decide to overwrite the item @ position 12.

I'll include code for my adapter below, I think that will be sufficient to figure out the problem.

public class PopularArtistsGridAdapter extends BaseAdapter {

    private ArrayList<ArtistsListItem> mArtists = new ArrayList<ArtistsListItem>();
    private LayoutInflater mInflater;
    private Context mContext;

    private int mRemovedIndex = -1;
    private int mRefresher = -1;
    private int mLastVisible = -1;

    public PopularArtistsGridAdapter(Context context) {
        mContext = context;
        mInflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
    }

    public void setItems(ArrayList<ArtistsListItem> artists) {
        if(artists == null)
            throw new NullPointerException();

        mArtists = artists;
        notifyDataSetChanged();
    }

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

    public void removeItem(int position, int lastVisible) {
        mLastVisible = lastVisible;

        mRemovedIndex = position;
        mRefresher = mRemovedIndex;

        mArtists.remove(position);
        notifyDataSetChanged();
    }

    public void addItem(int position) {
        ArtistsListItem item = getItem(position);

        mArtists.add(0, item);
        notifyDataSetChanged();
    }

    @Override
    public ArtistsListItem getItem(int position) {
        return mArtists.get(position);
    }

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

    private void iterateRefresher() {
        mRefresher++;

        if(mRefresher > mLastVisible) {
            mRemovedIndex = -1;
            mRefresher = -1;
            mLastVisible = -1;
        }
    }

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

        final ArtistsListItem item = getItem(position);

        if(convertView == null)
            convertView = mInflater.inflate(R.layout.popular_artist_item, null);

        final TextView tv = (TextView) convertView.findViewById(R.id.pai_stupid_bubble_button);
        final ImageView iv = (ImageView) convertView.findViewById(R.id.pai_image);

        final BitmapDrawable drawable = DrawableManager.fetchBitmapDrawable(mContext, item.getImageURL(), true, false);

        iv.invalidate();
        tv.invalidate();

        if(mRefresher >= 0 && position >= mRefresher) {
            Animation anim = AnimationUtils.loadAnimation(mContext, R.anim.grid_item_fadein);
            anim.setAnimationListener(new AnimationListener() {

                @Override
                public void onAnimationStart(Animation animation) {
                    iv.setImageDrawable(drawable);
                    tv.setText(item.getName());
                }

                @Override
                public void onAnimationRepeat(Animation animation) {
                    // TODO Auto-generated method stub

                }

                @Override
                public void onAnimationEnd(Animation animation) {
                    // TODO Auto-generated method stub

                }
            });

            anim.setStartOffset(300 + (50 * (position - mRemovedIndex)));

            convertView.startAnimation(anim);
            iterateRefresher();
        }
        else {
            iv.setImageDrawable(drawable);
            tv.setText(item.getName());
        }

        return convertView;
    }

}

The refresher is something I made to allow all visible items on screen to fade in when replacing the removed item. If anyone sees anything here that is suspect, or has any other ideas as to why this is happening, I'd appreciate the help. Thanks!

Wolf

Out of curiosity, if you remove your animation, does that fix anything? – Ultramarine

Hm, removing the Animation does fix the issue. But I want it to work with that animation as well. I'll keep tinkering with it, thanks for the tip – Wolf

Ok, I think I know what the problem is then. I'll post a solution later. – Ultramarine

Answer 1

The issue is due to the animation you are using to fade the view. You start the animation with an offset of at least 350msec. This means the animation won't run until that amount of time has elapsed. The problem arises in the onAnimationStart method of your AnimationListener. It is referencing a local variables and manipulating them. This is where things go wrong.

This is a bit tricky to explain, so try to follow along. By the time onAnimationStart has been called the first time getView will have been called many times. Since item views are recycled (that's what convertView is), by the time onAnimationStart is invoked the view it is referencing could be at a different position.

Crude illustration:

// Views being held by the adapter after first pass.
view0 --> onScreenItemtemAtPosition0
view1 --> onScreenItemtemAtPosition1
view2 --> onScreenItemtemAtPosition2

// schedule animation, listener uses view0 reference

// Scrolling occurs, item0 goes offscreen
view0 --> onScreenItemtemAtPosition3 // view0 is now being used in a new position onscreen
view1 --> onScreenItemtemAtPosition1
view2 --> onScreenItemtemAtPosition2

// onAnimationStart triggers and mutates view0
// !!!! Boom !!! view0 is being used for the onscreen item at position 3
// which isn't what you want because the drawable in your case is for item 0

As you can see this is a tricky situation. One solution would be to let your AnimationListener know about the position of the view you are intending to mutate so it can reconcile that with the view it is about to mutate.

Roughly:

class MyAnimationListener extends AnimationListener {
    private int positionToMutate;

    public MyAnimationListener(int positionToMutate) {
        this.positionToMutate = positionToMutate;
    }

    @Override
    public void onAnimationStart(Animation animation) {
        Integer viewPosition = (Integer)localConvertView.getTag(); // add this to getView as final View localConvertView
        if (positionToMutate == viewPosition) {
            iv.setImageDrawable(drawable);
            tv.setText(item.getName());
        }
    }
    // rest of the methods
}

And in your getView method update the convertView's tag with the position info:

final View localConvertView = convertView;
localConvertView.setTag(new Integer(position));

This will ensure that the animation listener knows whether it should mutate the view or not.

Ultramarine

Makes sense, but because the AnimationListener is an interface, it can't have a constructor and stuff. I'll have to find some way to implement this, but you're right, using the position as a tag for the view to determine whether or not it should animate should work. Thanks – Wolf

Created an anonymous inner class which extended AnimationListener, works now. Thanks for the tip! – Wolf