Items not drawn when scrolling up (extension of issues #19 and #15)

[GauthierChan]

Hi :)

Another case of items not being drawn when scrolling up, I followed #19 and #15 and it solved it for most configurations but I found new cases where it doesn't work.

In the GIFs below I have a SpannedGridLayoutManager with a 4-width span. I mix 2x2, 1x1 and 4x1 blocks.

Working case

Just one 2x2 block and it works fine:

Non working case

Same as above but more 1x1 blocks after the 2x2 blocks

Two "intertwined" 2x2 blocks

Two adjacent 2x2 blocks

Possible solution

In SpannedGridLayoutManager#fillBefore(), if I remove the canAddMoreViews(Direction.START, limit) condition as such:

/**
 * Fill gaps before the given position
 * @param position Item position in adapter
 * @param recycler Recycler
 * @param extraSpace Extra space to add to current [scroll] and use as limit for filling the gaps
 */
protected fun fillBefore(position: Int, recycler: RecyclerView.Recycler, extraSpace: Int) {
    var position = position
    val limit = scroll - extraSpace

    while (/* canAddMoreViews(Direction.END, limit) && */ position >= 0 ) {
        makeAndAddView(position, Direction.START, recycler)
        position--
    }
}

Then scrolling up doesn't seems to have drawing problems. But it has huge performance issues as I guess it draws all views that are above the current start 😄

So I found a hotfix to make the LayoutManager draw only necessary views:

/**
 * Checks if more views can be added between the current scroll and a limit
 */
protected fun canAddMoreViews(direction: Direction, limit: Int): Boolean {
    if (direction == Direction.START) {
        println(layoutStart.toString()+" | "+limit)
        return firstVisiblePosition > 0 && layoutStart > limit 
/***** add this -> *****/ || getChildAt(0) != null && getChildAt(0).top > -getChildAt(0).height
    } else {
        return limit > layoutEnd
    }
}

It makes the canAddMoreViews() more acceptable as of to when the LayoutManager should add a view at the top. Basically second-checking when it looks like there's not going to be any more views at the top 😄

The -getChildAt(0).height is totally arbitrary though, and might not work if views on the same column don't have the same height. And it is an ugly hotfix as it doesn't solve the original problem of why views weren't added. Do you have any ideas on how to improve this ?

This also seems to make scrolling up working without having to add an ItemDecoration to the RecyclerView.

---

Here's an Adapter where you can reproduce the layouts I showed above. Change the onBindViewHolder() example call methods (They are in the same order as above) for the layout you want 😃

import android.graphics.Color
import android.support.v7.widget.RecyclerView
import android.view.View
import android.view.ViewGroup
import android.widget.FrameLayout

class GridAdapter: RecyclerView.Adapter<RecyclerView.ViewHolder>(){

    init {
        setHasStableIds(true)
    }

    override fun getItemId(position: Int): Long {
        return position.toLong()
    }

    override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {

        showExample1(holder)
    }

    override fun getItemViewType(position: Int): Int {
        return position
    }

    override fun getItemCount(): Int {
        return 500
    }

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): GridItemViewHolder {
        val gridItemView = FrameLayout(parent.context)

        return GridItemViewHolder(gridItemView)
    }

    fun showExample1(holder: RecyclerView.ViewHolder){
        when(holder.adapterPosition) {

            0 -> set4x1Block(holder.itemView)
            in 1..4 -> set1x1Block(holder.itemView)
            5 -> set2x2Block(holder.itemView)
            else -> {

                if(holder.adapterPosition%5 == 0){
                    set4x1Block(holder.itemView)
                }else{
                    set1x1Block(holder.itemView)
                }
            }
        }
    }

    fun showExample2(holder: RecyclerView.ViewHolder){

        when(holder.adapterPosition) {

            0 -> set4x1Block(holder.itemView)
            in 1..4 -> set1x1Block(holder.itemView)
            5 -> set2x2Block(holder.itemView)
            in 6..10 -> set1x1Block(holder.itemView)
            else -> {

                if(holder.adapterPosition%5 == 0){
                    set4x1Block(holder.itemView)
                }else{
                    set1x1Block(holder.itemView)
                }
            }
        }
    }

    fun showExample3(holder: RecyclerView.ViewHolder){

        when(holder.adapterPosition) {

            0 -> set4x1Block(holder.itemView)
            in 1..4 -> set1x1Block(holder.itemView)
            5 -> set2x2Block(holder.itemView)
            in 6..7 -> set1x1Block(holder.itemView)
            8 -> set2x2Block(holder.itemView)
            else -> {

                if(holder.adapterPosition%5 == 0){
                    set4x1Block(holder.itemView)
                }else{
                    set1x1Block(holder.itemView)
                }
            }
        }
    }

    fun showExample4(holder: RecyclerView.ViewHolder){

        when(holder.adapterPosition) {

            0 -> set4x1Block(holder.itemView)
            in 1..4 -> set1x1Block(holder.itemView)
            in 5..6 -> set2x2Block(holder.itemView)
            else -> {

                if(holder.adapterPosition%5 == 0){
                    set4x1Block(holder.itemView)
                }else{
                    set1x1Block(holder.itemView)
                }
            }
        }
    }

    fun set1x1Block(itemView: View){
        val spanSize = SpanSize(1, 1)
        itemView.layoutParams = SpanLayoutParams(spanSize)
        itemView.setBackgroundColor(Color.RED)
    }
    fun set4x1Block(itemView: View){
        val spanSize = SpanSize(4, 1)
        itemView.layoutParams = SpanLayoutParams(spanSize)
        itemView.setBackgroundColor(Color.BLUE)
    }

    fun set2x2Block(itemView: View){
        val spanSize = SpanSize(2, 2)
        itemView.layoutParams = SpanLayoutParams(spanSize)
        itemView.setBackgroundColor(Color.GREEN)
    }

class GridItemViewHolder(itemView: View?) : RecyclerView.ViewHolder(itemView)

}

[manfcas]

Yeah, getChildAt(0).height should take into account at least the orientation value.

[jmartinesp]

@GauthierChan this issue was automatically marked as fixed by the commit, but I wanted to thank you for writing this issue and all the cases, it made my job a lot easier and faster. Thanks again!