Merge pull request #158 from rubensousa/grid_reverse_layout

Fix grid laying out views in wrong spans when scrolling in opposite direction

docs/changelog.md

@@ -2,6 +2,14 @@
 
 ## Version 1.1.0
 
+### 1.1.0-alpha03
+
+2023-08-04
+
+#### Bug fixes
+
+- Fixed grid layout not placing views in the correct spans when scrolling in opposite direction: ([#156](https://github.com/rubensousa/DpadRecyclerView/issues/156))
+
 ### 1.1.0-alpha02
 
 2023-06-23

dpadrecyclerview/api/dpadrecyclerview.api

@@ -189,8 +189,7 @@ public abstract class com/rubensousa/dpadrecyclerview/DpadSpanSizeLookup {
 	public fun getSpanGroupIndex (II)I
 	public fun getSpanIndex (II)I
 	public abstract fun getSpanSize (I)I
-	public final fun invalidateSpanGroupIndexCache ()V
-	public final fun invalidateSpanIndexCache ()V
+	public final fun invalidateCache ()V
 	public final fun setSpanGroupIndexCacheEnabled (Z)V
 	public final fun setSpanIndexCacheEnabled (Z)V
 }

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/scrolling/VerticalGridScrollTest.kt

@@ -18,6 +18,7 @@ package com.rubensousa.dpadrecyclerview.test.tests.scrolling
 
 import androidx.recyclerview.widget.RecyclerView
 import androidx.test.platform.app.InstrumentationRegistry
+import com.google.common.truth.Truth.assertThat
 import com.rubensousa.dpadrecyclerview.ChildAlignment
 import com.rubensousa.dpadrecyclerview.DpadSpanSizeLookup
 import com.rubensousa.dpadrecyclerview.FocusableDirection
@@ -271,6 +272,40 @@ class VerticalGridScrollTest : DpadRecyclerViewTest() {
         assertFocusAndSelection(position = 78)
     }
 
+    @Test
+    fun testLayoutStaysTheSameWhenScrollingInOppositeDirection() {
+        launchFragment()
+        onRecyclerView("Change span size lookup") { recyclerView ->
+            recyclerView.setParentAlignment(ParentAlignment(fraction = 0.0f))
+            recyclerView.setChildAlignment(ChildAlignment(fraction = 0.0f))
+            recyclerView.setSpanSizeLookup(object : DpadSpanSizeLookup() {
+                override fun getSpanSize(position: Int): Int {
+                    return if (position == 0 || position.rem(9) == 0) {
+                        spanCount
+                    } else {
+                        1
+                    }
+                }
+            })
+        }
+        waitForIdleScrollState()
+
+        val childPositions = getChildrenBounds()
+
+        // Scroll down until last uneven child is visible
+        repeat(5) {
+            KeyEvents.pressDown(delay = 500L)
+        }
+
+        waitForIdleScrollState()
+
+        repeat(5) {
+            KeyEvents.pressUp(delay = 500L)
+        }
+
+        assertThat(getChildrenBounds()).isEqualTo(childPositions)
+    }
+
     private fun scrollUp(grid: VerticalGridLayout) {
         KeyEvents.pressUp()
         grid.scrollUp()

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/DpadSpanSizeLookup.kt

@@ -106,20 +106,8 @@ abstract class DpadSpanSizeLookup {
         cacheSpanGroupIndices = enabled
     }
 
-
-    /**
-     * Clears the span index cache. GridLayoutManager automatically calls this method when
-     * adapter changes occur.
-     */
-    fun invalidateSpanIndexCache() {
+    fun invalidateCache() {
         spanIndexCache.clear()
-    }
-
-    /**
-     * Clears the span group index cache. GridLayoutManager automatically calls this method
-     * when adapter changes occur.
-     */
-    fun invalidateSpanGroupIndexCache() {
         spanGroupIndexCache.clear()
     }
 

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/LayoutConfiguration.kt

@@ -169,10 +169,15 @@ internal class LayoutConfiguration(properties: Properties) {
 
     fun setSpanCount(count: Int) {
         spanCount = max(1, count)
+        spanSizeLookup.invalidateCache()
     }
 
     fun setSpanSizeLookup(spanSizeLookup: DpadSpanSizeLookup) {
         this.spanSizeLookup = spanSizeLookup
+        if (spanSizeLookup !== DpadSpanSizeLookup.DEFAULT) {
+            spanSizeLookup.setSpanIndexCacheEnabled(true)
+            spanSizeLookup.setSpanGroupIndexCacheEnabled(true)
+        }
     }
 
     fun setChildDrawingOrderEnabled(enabled: Boolean) {

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotLayoutManager.kt

@@ -187,20 +187,24 @@ class PivotLayoutManager(properties: Properties) : RecyclerView.LayoutManager()
     }
 
     override fun onItemsAdded(recyclerView: RecyclerView, positionStart: Int, itemCount: Int) {
+        configuration.spanSizeLookup.invalidateCache()
         pivotLayout.onItemsAdded(positionStart, itemCount)
         pivotSelector.onItemsAdded(positionStart, itemCount)
     }
 
     override fun onItemsChanged(recyclerView: RecyclerView) {
+        configuration.spanSizeLookup.invalidateCache()
         pivotSelector.onItemsChanged()
     }
 
     override fun onItemsRemoved(recyclerView: RecyclerView, positionStart: Int, itemCount: Int) {
+        configuration.spanSizeLookup.invalidateCache()
         pivotLayout.onItemsRemoved(positionStart, itemCount)
         pivotSelector.onItemsRemoved(positionStart, itemCount)
     }
 
     override fun onItemsMoved(recyclerView: RecyclerView, from: Int, to: Int, itemCount: Int) {
+        configuration.spanSizeLookup.invalidateCache()
         pivotLayout.onItemsMoved(from, to, itemCount)
         pivotSelector.onItemsMoved(from, to, itemCount)
     }

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/focus/SpanFocusFinder.kt

@@ -65,27 +65,31 @@ internal class SpanFocusFinder {
         if (spanCount == 1) {
             return RecyclerView.NO_POSITION
         }
-        val positionDirection = if (forward) 1 else -1
-        val spanDirection = getSpanDirection(forward, reverseLayout)
+        var positionDirection = if (forward) 1 else -1
         if (spanSizeLookup === DpadSpanSizeLookup.DEFAULT) {
             return findNextEvenSpanPosition(
                 spanSizeLookup, focusedPosition, edgePosition, positionDirection
             )
         }
         val focusedSpanIndex = spanSizeLookup.getCachedSpanIndex(focusedPosition, spanCount)
-        val focusedSpanSize = spanSizeLookup.getSpanSize(focusedPosition)
-        val currentSpanIndex = focusedSpanIndex + focusedSpanSize * spanDirection - spanDirection
 
-        // Move position to the start of the next span group
         val firstPositionInNextSpanGroup = moveToStartOfNextSpanGroup(
-            focusedPosition, currentSpanIndex, spanSizeLookup, spanDirection, positionDirection
+            position = focusedPosition,
+            spanIndex = focusedSpanIndex,
+            lookup = spanSizeLookup,
+            spanDir = getSpanDirection(forward, reverseLayout),
+            posDir = positionDirection,
+            edgePosition = edgePosition,
+            forward = forward,
+            reverseLayout = reverseLayout
         )
-        var currentPosition = firstPositionInNextSpanGroup
 
-        if (isPositionOutOfBounds(currentPosition, edgePosition, forward)) {
+        if (firstPositionInNextSpanGroup == RecyclerView.NO_POSITION) {
             return RecyclerView.NO_POSITION
         }
 
+        var currentPosition = firstPositionInNextSpanGroup
+
         // 1. If there's no cache, just return the current position that's sitting on an edge
         // 2. If the item takes the entire size, just return it since there's no other valid option
         if (cachedSpanIndex == RecyclerView.NO_POSITION
@@ -94,9 +98,15 @@ internal class SpanFocusFinder {
             return currentPosition
         }
 
+        positionDirection = if (forward || reverseLayout) {
+            positionDirection
+        } else {
+            positionDirection * -1
+        }
+
         // Now search until we find the cached span index or we go outside the edge
         while (!isPositionOutOfBounds(currentPosition, edgePosition, forward)) {
-            if (isPositionAtCachedSpan(currentPosition, spanSizeLookup, spanDirection)) {
+            if (isPositionAtCachedSpan(currentPosition, spanSizeLookup, reverseLayout)) {
                 return currentPosition
             }
             currentPosition += positionDirection
@@ -134,45 +144,87 @@ internal class SpanFocusFinder {
     private fun isPositionAtCachedSpan(
         position: Int,
         spanSizeLookup: DpadSpanSizeLookup,
-        spanDirection: Int
+        reverseLayout: Boolean,
     ): Boolean {
         val spanIndex = spanSizeLookup.getCachedSpanIndex(position, spanCount)
-        return if (spanDirection > 0) {
+        return if (!reverseLayout) {
             spanIndex >= cachedSpanIndex
         } else {
             spanIndex <= cachedSpanIndex
         }
     }
 
     private fun moveToStartOfNextSpanGroup(
-        currentPosition: Int,
-        currentSpanIndex: Int,
-        spanSizeLookup: DpadSpanSizeLookup,
-        spanDirection: Int,
-        positionDirection: Int,
+        position: Int,
+        spanIndex: Int,
+        lookup: DpadSpanSizeLookup,
+        spanDir: Int,
+        posDir: Int,
+        edgePosition: Int,
+        forward: Boolean,
+        reverseLayout: Boolean
     ): Int {
-        val targetSpanIndex = getEndSpanIndex(spanDirection)
-        val position = moveSpanIndexToTarget(
-            currentPosition,
-            currentSpanIndex,
-            targetSpanIndex,
-            spanSizeLookup,
-            spanDirection,
-            positionDirection
-        )
-        return position + positionDirection
+        var currentPos = position
+        var currentSpan = spanIndex
+        val startSpanIndex = if (!reverseLayout) 0 else spanCount - 1
+
+        // First step: move to edge of current span group
+        while (!isPositionOutOfBounds(currentPos + posDir, edgePosition, forward)
+            && fitsNextInCurrentSpanGroup(lookup, currentSpan, currentPos, spanDir, posDir)
+        ) {
+            currentPos += posDir
+            currentSpan = getNextSpanEnd(lookup, currentSpan, currentPos, spanDir, posDir)
+        }
+
+        // Move to next span group
+        currentPos += posDir
+
+        if (isPositionOutOfBounds(currentPos, edgePosition, forward)) {
+            return RecyclerView.NO_POSITION
+        }
+
+        // Second step: move to start of next span group
+        currentSpan = lookup.getCachedSpanIndex(currentPos, spanCount)
+        while (currentSpan != startSpanIndex
+            && currentSpan > 0
+            && currentSpan < spanCount
+            && !isPositionOutOfBounds(currentPos + posDir, edgePosition, forward)
+        ) {
+            currentSpan += lookup.getSpanSize(currentPos) * spanDir
+            currentPos += posDir
+        }
+
+        if (isPositionOutOfBounds(currentPos, edgePosition, forward)) {
+            return RecyclerView.NO_POSITION
+        }
+
+        return currentPos
     }
 
-    private fun isPositionOutOfBounds(position: Int, edgePosition: Int, forward: Boolean): Boolean {
-        return (position > edgePosition && forward) || (position < edgePosition && !forward)
+    private fun fitsNextInCurrentSpanGroup(
+        lookup: DpadSpanSizeLookup,
+        spanIndex: Int,
+        currentPos: Int,
+        spanDir: Int,
+        posDir: Int
+    ): Boolean {
+        val nextSpanEnd = getNextSpanEnd(lookup, spanIndex, currentPos, spanDir, posDir)
+        return nextSpanEnd >= 0 && nextSpanEnd <= spanCount - 1
     }
 
-    private fun getStartSpanIndex(spanDirection: Int): Int {
-        return if (spanDirection > 0) 0 else spanCount - 1
+    private fun getNextSpanEnd(
+        spanSizeLookup: DpadSpanSizeLookup,
+        spanIndex: Int,
+        currentPos: Int,
+        spanDir: Int,
+        posDir: Int
+    ): Int {
+        val currentSpanEnd = spanIndex + (spanSizeLookup.getSpanSize(currentPos) - 1) * spanDir
+        return currentSpanEnd + spanSizeLookup.getSpanSize(currentPos + posDir) * spanDir
     }
 
-    private fun getEndSpanIndex(spanDirection: Int): Int {
-        return getStartSpanIndex(-spanDirection)
+    private fun isPositionOutOfBounds(position: Int, edgePosition: Int, forward: Boolean): Boolean {
+        return (position > edgePosition && forward) || (position < 0 && !forward)
     }
 
     private fun getSpanDirection(forward: Boolean, reverseLayout: Boolean): Int {
@@ -184,27 +236,4 @@ internal class SpanFocusFinder {
         }
     }
 
-    private fun moveSpanIndexToTarget(
-        position: Int,
-        spanIndex: Int,
-        targetSpanIndex: Int,
-        spanSizeLookup: DpadSpanSizeLookup,
-        spanDirection: Int,
-        positionDirection: Int
-    ): Int {
-        if (spanIndex == targetSpanIndex) {
-            return position
-        }
-        var currentSpanIndex = spanIndex
-        var currentPosition = position
-        while (currentSpanIndex != targetSpanIndex
-            && currentSpanIndex >= 0
-            && currentSpanIndex < spanCount
-        ) {
-            currentSpanIndex += spanSizeLookup.getSpanSize(position) * spanDirection
-            currentPosition += positionDirection
-        }
-        return currentPosition
-    }
-
 }

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/StructureEngineer.kt

@@ -397,6 +397,7 @@ internal abstract class StructureEngineer(
         recycler: RecyclerView.Recycler,
         state: RecyclerView.State,
     ): Int {
+        var newSpace = 0
         var remainingSpace = layoutRequest.fillSpace
         layoutResult.reset()
 
@@ -411,6 +412,8 @@ internal abstract class StructureEngineer(
                 layoutResult.consumedSpace * layoutRequest.direction.value
             )
 
+            newSpace += layoutResult.consumedSpace
+
             if (!layoutResult.skipConsumption) {
                 remainingSpace -= layoutResult.consumedSpace
             }
@@ -429,7 +432,7 @@ internal abstract class StructureEngineer(
         // Recycle once again after layout is done
         viewRecycler.recycleByLayoutRequest(recycler, layoutRequest)
 
-        return layoutRequest.fillSpace - remainingSpace
+        return newSpace
     }
 
     private fun updateLoopingState() {