Weird behavior (bug?) with RecyclerView And LinearLayoutManager with reverseLayout == true
Asked Answered
I

0

10

When using RecyclerView with LinearLayoutManager and "reverseLayout" flag set to true, when notifying any item via notifyItemChanged it also calls onBindViewHolder for the first non-visible item. And it doesn't call onViewRecycled for that item afterwards whatsoever. So in case of a ViewHolder doing some kind of subscription in onBind it will never be released because onRecycle won't be called.

This actually looks like a bug in LinearLayoutManager. If you take a look at fill method in LinearLayoutManager, there is this code:

if (!layoutChunkResult.mIgnoreConsumed || layoutState.mScrapList != null || !state.isPreLayout()) {
    layoutState.mAvailable -= layoutChunkResult.mConsumed;
    // we keep a separate remaining space because mAvailable is important for recycling
    remainingSpace -= layoutChunkResult.mConsumed;
}

As far as I understand we iterate over child views until we fill all the needed space, in other words layoutState.mAvailable and remainingSpace which are both measured in pixels. And if you look further at what's going on in layoutChunk method you will see this piece of code:


// Consume the available space if the view is not removed OR changed
if (params.isItemRemoved() || params.isItemChanged()) {
    result.mIgnoreConsumed = true;
}

So the LLM will SKIP any item that has FLAG_UPDATE which, in my case, is the item I call notifyItemChanged on. By skipping I mean that the height of the item won't be subtracted from these two variables:

layoutState.mAvailable -= layoutChunkResult.mConsumed;
// we keep a separate remaining space because mAvailable is important for recycling
remainingSpace -= layoutChunkResult.mConsumed;

And this will make the loop iterate one more additional view. And since that view is not cached by the LLM (see tryGetViewHolderForPositionByDeadline -> getScrapOrHiddenOrCachedHolderForPosition) (because if I'm not wrong it's outside of the screen bounds) it will be created anew. But in case of reverseLayout set to false (default LLM state) it won't iterate it because it will reach the end of the list first:

while ((layoutState.mInfinite || remainingSpace > 0) && layoutState.hasMore(state)) { ... }

Namely here:

boolean hasMore(RecyclerView.State state) {
    return mCurrentPosition >= 0 && mCurrentPosition < state.getItemCount();
}

In case of reverseLayout == false, we start iterating from the current position going in ascending order, i.e.:

    [ 0 ]   
 +--[ 1 ]--+
 |  [ 2 ]  |
 |  [ 3 ]  |
 |  [ 4 ]  |
 |  [ 5 ]  |
 +--[ 6 ]--+

Let's say we call notifyItemChanged on item with position 3. So the LLM will iterate over 1, 2, (skip 3), 4, 5 and 6. Since it skipped 3 there will be pixels left to fill in layoutState.mAvailable variable BUT because we are at the end of the loop it will stop right away.

Now let's see what happens when reverseLayout == true.

    [ 6 ]   
 +--[ 5 ]--+
 |  [ 4 ]  |
 |  [ 3 ]  |
 |  [ 2 ]  |
 |  [ 1 ]  |
 +--[ 0 ]--+

So again, we call notifyItemChanged(3). LLM will start iterating in reversed order: 0, 1, 2, (skip 3), 4 and 5. Then since it skipped 3 there are still pixels to fill and we are NOT at the end of the list so it will iterate 6 as well.

What's the most strange thing is that in this code example it is only reproducible on the first long tap, afterwards first off-screen view's onBind won't get called. But in the project where this thing was discovered it is 100% reproducible every time you call notifyItemChanged on a view.

Here is minimal reproducible example:

class MainActivity : AppCompatActivity() {
    private val id = AtomicLong(0)

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)

        val images = (0..6).map {
            return@map ImageItem(
                "https://i.imgur.com/BBcy6Wc.jpg",
                id.getAndIncrement()
            )
        }

        recycler.layoutManager = LinearLayoutManager(this, RecyclerView.VERTICAL, true)
        recycler.adapter = Adapter(images).apply { setHasStableIds(true) }
        recycler.setRecyclerListener { viewHolder ->
            if (viewHolder is Adapter.MyViewHolder) {
                viewHolder.onRecycle()
            }
        }
        recycler.adapter!!.notifyDataSetChanged()
    }

    data class ImageItem(val url: String, val id: Long)

    class Adapter(
        private val items: List<ImageItem>
    ) : RecyclerView.Adapter<Adapter.MyViewHolder>(), OnRecyclerItemClick {

        override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): MyViewHolder {
            println("TTTAAA onCreateViewHolder")

            val view = LayoutInflater.from(parent.context)
                .inflate(R.layout.my_view_holder, parent, false)

            return MyViewHolder(view, this)
        }

        override fun onBindViewHolder(holder: MyViewHolder, position: Int) {
            holder as MyViewHolder
            holder.onBind(items[position])
        }

        override fun getItemCount(): Int {
            return items.size
        }

        override fun getItemId(position: Int): Long {
            return items[position].id
        }

        override fun onLongClick(position: Int) {
            println("TTTAAA onLongClick($position)")
            notifyItemChanged(position)
        }

        class MyViewHolder(
            private val view: View,
            private val callback: OnRecyclerItemClick
        ) : RecyclerView.ViewHolder(view) {
            private val imageView: AppCompatImageView = view.findViewById(R.id.my_image)

            fun onBind(imageItem: ImageItem) {
                println("TTTAAA onBind $layoutPosition")

                imageView.setOnLongClickListener {
                    callback.onLongClick(layoutPosition)
                    return@setOnLongClickListener true
                }

                Glide.with(imageView.context)
                    .load(imageItem.url)
                    .centerCrop()
                    .into(imageView)
            }

            fun onRecycle() {
                println("TTTAAA onRecycle $layoutPosition")
                imageView.setOnClickListener(null)

                Glide.with(imageView.context)
                    .clear(imageView)
            }

        }
    }
}

interface OnRecyclerItemClick {
    fun onLongClick(position: Int)
}

And here is the log:

onLongClick(0)
onCreateViewHolder
onBind 3
onCreateViewHolder
onBind 0
onRecycle 0
onLongClick(0)
onBind 0
onRecycle 0
onLongClick(0)
onBind 0
onRecycle 0

I'm longtapping the view with position 0 and an additional view with position 3 (which is off-screen) gets binded as well.

Incriminate answered 20/2, 2020 at 10:11 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.