mikepenz / FastAdapter

The bullet proof, fast and easy to use adapter library, which minimizes developing time to a fraction...
https://mikepenz.dev
Apache License 2.0
3.85k stars 494 forks source link

slow/jittery scrolling when using spinner #421

Closed or-dvir closed 7 years ago

or-dvir commented 7 years ago

hi in my app i have various types of lists. for example: a list with edit text(s), spinner, checkbox etc (as can be seen in the screenshots of the google play store). everything works fine except that in the list with the spinners, if you have a lot of items (say about 30 or more) scrolling is slow/jittery/jumpy. my initial thoughts were that my bindView method is doing too much work and making scrolling slow because it is called lots of times. for example i have a settings/preferences which lets the user set the size of the text, and as far as i can tell (please correct me if im wrong) i have to set the text size programmatically inside bindView and not when the viewHolder is being created. this is because if the user changes the text size and then scrolls down the list, a previous viewHolder will be recycled with the old textSize value. so basically everytime bindView is called, i have to check sharedPreferences for the text size setting, check which setting is enabled (i have 4 options), and then apply it to the textView and spinner at run time. so like i mentioned, i suspected that this is the problem because that sounds like a lot of work to do every time bindView is called.

HOWEVER,

  1. i am doing exactly the same thing for all of my other lists and i dont have problems with them. the "worst" case being a list where i am performing this operation (setting text size) to 3 different views (2 EditTexts and a TextView) and the scrolling is fast and smooth.
  2. i tried removing the parts inside bindView which i think are "heavy" in the spinner list, but scrolling is still slow.

-any idea why scrolling is slow only when i am using a spinner? (maybe it has to do with creating and setting the adapter at runtime? i also have a setting for that...) -was i correct in my explanation above about setting the text size inside bindView instead of when creating the viewHolder?

i assume you'd like to see some code but i don't have access to it right now, so im posting this in the hopes that i was clear enough in my explanation to perhaps get an answer without it (i will edit this post with my code when i have access).

EDIT

here is my object which i attach to the adapter:

public class SpinnerItemBACKUP extends BaseItem<SpinnerItemBACKUP, SpinnerItemBACKUP.ViewHolder> implements Serializable
{
    @JsonIgnore
    private static final ViewHolderFactory<ViewHolder> FACTORY = new SpinnerItemBACKUP.ItemFactory();

    @JsonProperty("_number")
    private int number = 1;

    /**
     * constructor for JACKSON library
     */
    private SpinnerItemBACKUP()
    {
        super("");
    }

    /**
     *
     */
    public SpinnerItemBACKUP(String name)
    {
        super(name);
    }

    /**
     *
     */
    public int getNumber()
    {
        return number;
    }

    /**
     *
     */
    public void setNumber(int number)
    {
        this.number = number;
    }

    ////////////////////////////////////////////////////////////////////////////////////////
    ////////////////////////////////////////////////////////////////////////////////////////
    ////////////////////////////////////////////////////////////////////////////////////////
    ////////////////////////////////////////////////////////////////////////////////////////
    ////////////////////////////////////////////////////////////////////////////////////////
    ////////////////////////////////////////////////////////////////////////////////////////

    @Override
    public ViewHolderFactory<ViewHolder> getFactory()
    {
        return FACTORY;
    }

    @Override
    public int getType()
    {
        return R.id.fastadapter_id_spinnerItem;
    }

    @Override
    public int getLayoutRes()
    {
        return R.layout.item_spinner;
    }

    @Override
    public void bindView(final SpinnerItemBACKUP.ViewHolder holder, List<Object> payloads)
    {
        //in "BaseItem" (which this object extends)
        //we call the method "ActivityMain.setTextSizeAndColor()"
        //which is also described in this post
        super.bindView(holder, payloads);

        RelativeLayout.LayoutParams params = (RelativeLayout.LayoutParams)holder.spinner.getLayoutParams();

        if (isItemCheckable())
        {
            params.addRule(RelativeLayout.ALIGN_PARENT_RIGHT, 0);
        }

        else
        {
            params.addRule(RelativeLayout.ALIGN_PARENT_RIGHT, RelativeLayout.TRUE);
        }

        Context context = holder.itemView.getContext();

        String maxNumberStr =
                PreferenceManager.getDefaultSharedPreferences(context)
                                 .getString(context.getString(R.string.settings_key_numberedListMaxNumber), "50");

        int maxNumber = Integer.parseInt(maxNumberStr);

        ArrayList<Integer> spinnerList = new ArrayList<>();

        for (int i = 1; i <= maxNumber; i++)
        {
            spinnerList.add(i);
        }

        //i tried using both the standard adapter,
        //and my custom adapter (also described in this post) in order
        //to apply the "text size" settings value to the spinner.
        //also tried none of them (i.e. not setting the spinner text size)
        //but scrolling is still slow/jittery
//      ArrayAdapter<Integer> adapter = new ArrayAdapter<>(holder.itemView.getContext(),
//                                                  android.R.layout.simple_spinner_item,
//                                                  spinnerList);
        AdapterSpinner adapter = new AdapterSpinner(holder.itemView.getContext(),
                                                    android.R.layout.simple_spinner_item,
                                                    spinnerList);
        holder.spinner.setAdapter(adapter);

        //the position given starts at 0!!!
        holder.spinner.setSelection(number - 1);
    }

    @Override
    public void unbindView(SpinnerItemBACKUP.ViewHolder holder)
    {
        super.unbindView(holder);

        holder.spinner.setSelection(0);
    }

    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////

    @JsonIgnoreType
    private static class ItemFactory implements ViewHolderFactory<SpinnerItemBACKUP.ViewHolder>
    {
        public SpinnerItemBACKUP.ViewHolder create(View v)
        {
            return new SpinnerItemBACKUP.ViewHolder(v);
        }
    }

    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////
    //////////////////////////////////////////////////////////////////////////////////////////

    @JsonIgnoreType
    static class ViewHolder extends BaseItem.ViewHolder
    {
        private Spinner spinner;

        public ViewHolder(View view)
        {
            super(view);
            spinner = (Spinner) view.findViewById(R.id.spinner_spinnerList);
        }
    }
}

here is my custom spinner adapter to change the spinner text size:

public class AdapterSpinner extends ArrayAdapter<Integer>
{
    public AdapterSpinner(@NonNull Context context, @LayoutRes int resource, @NonNull List<Integer> objects)
    {
        super(context, resource, objects);
    }

    @NonNull
    @Override
    public View getView(int position, @Nullable View convertView, @NonNull ViewGroup parent)
    {
        View v = super.getView(position, convertView, parent);
        TextView tv = (TextView) v.findViewById(android.R.id.text1);

        ActivityMain.setTextSizeAndColor(getContext(), tv);

        return v;
    }

    @Override
    public View getDropDownView(int position, @Nullable View convertView, @NonNull ViewGroup parent)
    {
        View v = super.getDropDownView(position, convertView, parent);
        TextView tv = (TextView) v.findViewById(android.R.id.text1);

        ActivityMain.setTextSizeAndColor(getContext(), tv);

        return v;
    }
}

and here is the method ActivityMain.setTextSizeAndColor():

public static void setTextSizeAndColor(Context context, TextView tv)
    {
        String textSizeSettingsValue =
                PreferenceManager.getDefaultSharedPreferences(context)
                                 .getString(context.getString(R.string.settings_key_textSize),
                                            context.getString(R.string.settingsValue_textSize_small));

        if(textSizeSettingsValue.equals(context.getString(R.string.settingsValue_textSize_small)))
        {
            tv.setTextAppearance(context, android.R.style.TextAppearance_Small);
        }

        else if(textSizeSettingsValue.equals(context.getString(R.string.settingsValue_textSize_medium)))
        {
            tv.setTextAppearance(context, android.R.style.TextAppearance_Small);
            tv.setTextSize(TypedValue.COMPLEX_UNIT_SP, 16);
        }

        else if(textSizeSettingsValue.equals(context.getString(R.string.settingsValue_textSize_large)))
        {
            tv.setTextAppearance(context, android.R.style.TextAppearance_Medium);
        }

        else if(textSizeSettingsValue.equals(context.getString(R.string.settingsValue_textSize_extraLarge)))
        {
            tv.setTextAppearance(context, android.R.style.TextAppearance_Large);
        }

        else
        {
            throw new IllegalArgumentException("you forgot to add \"" +
                                               textSizeSettingsValue  +
                                               "\" to this method");
        }

        //IMPORTANT NOTE!!!!!!!!
        //this line MUST be LAST in this method because
        //setting text appearance also sets the color
        tv.setTextColor(context.getResources().getColor(android.R.color.black));
    }
mikepenz commented 7 years ago

@or-dvir so I see a few things which might lower performance. Setting the LayoutParams each time might result in an additional measure pass which makes things slower. So if you can avoid that it would be good.

In general setting the textSize or textColor shouldn't do too much harm if it stays the same as I guess the TV already optimizes it. I am just a bit surprised that you manually set small, medium or large. Just set a specific textSize using "sp" and you are fine. The user can anyways define in the phone settings how large he want the text to be displayed. It shouldn't be a app setting.

regarding the adapter. As you are not able to prevent setting the adapter, i'd still suggest that you look into the prefetching of items in the RV. (That's not adapter related):

LinearLayoutManager.setInitialPrefetchItemCount(N)

(Search for it in the changelog and read more in their docu: https://developer.android.com/topic/libraries/support-library/revisions.html)

or-dvir commented 7 years ago

@mikepenz about the LayoutParams thing: the user have an option to add a checkbox to a list (see pictures from the google play store) and she/she can show or hide the check boxes whenever they wants. therefore similarly to the text size thing, i need to check it when the view is being displayed in "bindView()" or else if the user has selected "show check box" from the menu, a view without a checkbox might be recycled. the checkbox is aligned to the right of the screen and so are the other views (e.g. spinner). so i must check it every time and change the layoutParams at runtime otherwise they will overlap. i could probably wrap all those views in a horizontal linearlayout, however im not sure if it will actually help because i dont know if a linearLayout simply does the same thing behind the scenes...

about the settings: i like giving the user a choice. what if the size i choose is too small for the user? he would have to leave the app, go to the phones settings, look for the right one, change it, and then come back to the app. also this will affect all the texts on his phone (only because of the size i chose in my app) - thats why i have it as a local setting.

while i agree that those things do affect performance, all of the those things are also done to every other type of list i have and there are no scrolling problems there - for some reason only the list with the spinner is suffering performance problems (even when i disable these options).

*i will look into the link you sent me later today

edit

ok i have look in the link you posted and it refers to nested recycler views, and also raises a bunch of questions:

  1. is it even relevant? i mean does does Spinner use a recycler view? i would assume so, but dont know for sure (and also is it Spinner or SpinnerCompat that uses it?)
  2. how and where would i apply the method LinearLayoutManager.setInitialPrefetchItemCount(N)? i assume i would have to make a custom Spinner, find in the source code where they apply the LayoutManager and override that method.
  3. what number should i put for N? the link says that i need to know how many items will be visible to the user. but that number depends on the screen size... so if i truly want to use this method, i need to write further code which determines N according to the screen size.

all of this sounds like a lot of work... and it might not even help @mikepenz dont get me wrong, i appreciate your help and finding this link for me. im just not sure it is the best solution... any other ideas?

mikepenz commented 7 years ago

About the text size. The Material Design Guidelines define exact values to use. Those should be used through all apps, so in fact the user most likely adjusted the text size in the past on his phone. But that was just a suggestion, if you think it gives some advantage its fine.

Regarding the link I posted. Yes it refers to RecyclerView, but pre-loading next items ahead of time should also improve scrolling for other types which need to display a list. and are a bit more expensive. As of the usage of it, please refer to the docs as it is not related to the adapter.

Well That are all things I can directly mention. I don't think you have many other options. Possibly using something else than a spinner. Or checking directly what people suggest how to use a spinner in a recyclerView. That are basically all non FastAdapter related issues, as it is a general problem then.

or-dvir commented 7 years ago

@mikepenz after further experimenting the problem is indeed the adapter. i mean not the adapter itself, but what you said about nested lists/recyclerViews. i tried changing the number of items in the spinner to see where the "break point" is, but even with just 1 item in the adapter scrolling is slow. only when i completely disabled the adapter (as in empty spinners) scrolling was smooth and fast.

thank you so much for you help. even though it turns out not to be a FastAdapter issue you helped me find the answer