views:

232

answers:

1

I'm working with a ListView, trying to get the convertView / referenceHolder optimisation to work properly but it's giving me trouble. (This is the system where you store the R.id.xxx pointers in as a tag for each View to avoid having to call findViewById). I have a ListView populated with simple rows of an ImageView and some text, but the ImageView can be formatted either for portrait-sized images (tall and narrow) or landscape-sized images (short and wide). It's adjusting this formatting for each row which isn't working as I had hoped.

The basic system is that to begin with, it inflates the layout for each row and sets the ImageView's settings based on the data, and includes an int denoting the orientation in the tag containing the R.id.xxx values. Then when it starts reusing convertViews, it checks this saved orientation against the orientation of the new row. The theory then is that if the orientation is the same, then the ImageView should already be set up correctly. If it isn't, then it sets the parameters for the ImageView as appropriate and updates the tag.

However, I found that it was somehow getting confused; sometimes the tag would get out of sync with the orientation of the ImageView. For example, the tag would still say portrait, but the actual ImageView would still be in landscape layout. I couldn't find a pattern to how or when this happened; it wasn't consistent by orientation, position in the list or speed of scrolling. I can solve the problem by simply removing the check about convertView's orientation and simply always set the ImageView's parameters, but that seems to defeat the purpose of this optimisation.

Can anyone see what I've done wrong in the code below?

static LinearLayout.LayoutParams layoutParams;
(...)

public View getView(int position, View convertView, ViewGroup parent){
    ReferenceHolder holder;

    if (convertView == null){
        convertView = inflater.inflate(R.layout.pick_image_row, null);
        holder = new ReferenceHolder();
        holder.getIdsAndSetTag(convertView, position);
        if (data[position][ORIENTATION] == LANDSCAPE) {
            // Layout defaults to portrait settings, so ImageView size needs adjusting. 
            // layoutParams is modified here, with specific values for width, height, margins etc
            holder.image.setLayoutParams(layoutParams);
        }
        holder.orientation = data[position][ORIENTATION];

    } else {
        holder = (ReferenceHolder) convertView.getTag();
        if (holder.orientation != data[position][ORIENTATION]){  //This is the key if statement for my question
            switch (image[position][ORIENTATION]) {
            case PORTRAIT:
                // layoutParams is reset to the Portrait settings
                holder.orientation = data[position][ORIENTATION];
                break;

            case LANDSCAPE:
                // layoutParams is reset to the Landscape settings
                holder.orientation = data[position][ORIENTATION];
                break;
            }
            holder.image.setLayoutParams(layoutParams);
        }               
    }

    // and the row's image and text is set here, using holder.image.xxx
    // and holder.text.xxx

    return convertView;
}

static class ReferenceHolder {
    ImageView image;
    TextView text;
    int orientation;

    void getIdsAndSetTag(View v, int position){
        image = (ImageView) v.findViewById(R.id.pickImageImage);
        text = (TextView) v.findViewById(R.id.pickImageText);
        orientation = data[position][ORIENTATION];
        v.setTag(this);
    }
}

Thanks!

+2  A: 

Rather than putting orientation as a data member of ReferenceHolder, examine the actual LayoutParams of the ImageView to see what orientation it is in. This way, by definition, you can't get out of sync somehow.

To be honest, I'm confused by the code you have there, as you never seem to change layoutParams, which would seem to be kinda important. Or, shouldn't you have layoutParamsPortrait and layoutParamsLandscape or something? To me, it looks like the rules are:

  • If it's portrait and the row is initially created, leave it portrait
  • Everything else is landscape, regardless of what the orientation flag says, since you always set it to layoutParams, which is presumably landscape
CommonsWare
Oh, err, sorry, I wasn't clear about that. In the section where I wrote "//set the layout params...", I meant that I had several lines giving margins, widths and so forth but I commented out because the exact details weren't specific. I'll edit the question to make that more clear. As for examining the actual parameters of the ImageView, I didn't realise there was a parameter of the ImageView itself I could check since I was just messing with sizes - but you're right, I probably could just check the size. I'll experiment.
Steve H
It's clearly too late for me to be trying to write sensibly - in the comment above I meant that I commented out the specific details of the layout parameters because they weren't relevant.
Steve H
Are you trying to reuse the same `LayoutParams` object across all rows? That could be your problem.
CommonsWare
Aha, yes, that was it. I assumed that when layout parameters were set using a `LayoutParams` object, the View copied the values out of the `LayoutParams`, leaving it free to be reused. That was not the case, and the object remained somehow tied to the View. Creating a new `LayoutParams` for each row did the trick and everything now works properly. Thanks!
Steve H