From dbe8ef58eccfe15491817bd0c8e65affec15b6a9 Mon Sep 17 00:00:00 2001 From: Ruben Sousa Date: Thu, 30 May 2024 13:29:43 +0200 Subject: [PATCH 1/5] Invalidate span cache to prevent infinite layout loops --- .../test/tests/AbstractTestAdapter.kt | 2 +- .../tests/mutation/GridAdapterMutationTest.kt | 96 +++++++++++++++++++ .../layoutmanager/PivotLayoutManager.kt | 8 +- .../layoutmanager/layout/LayoutInfo.kt | 4 + .../layout/grid/GridLayoutEngineer.kt | 2 + 5 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/mutation/GridAdapterMutationTest.kt diff --git a/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/AbstractTestAdapter.kt b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/AbstractTestAdapter.kt index ef517d59..3350ca8a 100644 --- a/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/AbstractTestAdapter.kt +++ b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/AbstractTestAdapter.kt @@ -140,6 +140,6 @@ abstract class AbstractTestAdapter( override fun getItemCount(): Int = items.size - protected fun getItem(position: Int) = items[position] + fun getItem(position: Int) = items[position] } diff --git a/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/mutation/GridAdapterMutationTest.kt b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/mutation/GridAdapterMutationTest.kt new file mode 100644 index 00000000..474f06d2 --- /dev/null +++ b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/mutation/GridAdapterMutationTest.kt @@ -0,0 +1,96 @@ +/* + * Copyright 2024 Rúben Sousa + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.rubensousa.dpadrecyclerview.test.tests.mutation + +import androidx.recyclerview.widget.RecyclerView +import com.google.common.truth.Truth.assertThat +import com.rubensousa.dpadrecyclerview.ChildAlignment +import com.rubensousa.dpadrecyclerview.DpadSpanSizeLookup +import com.rubensousa.dpadrecyclerview.ParentAlignment +import com.rubensousa.dpadrecyclerview.test.TestAdapterConfiguration +import com.rubensousa.dpadrecyclerview.test.TestLayoutConfiguration +import com.rubensousa.dpadrecyclerview.test.helpers.assertFocusAndSelection +import com.rubensousa.dpadrecyclerview.test.helpers.assertItemAtPosition +import com.rubensousa.dpadrecyclerview.test.helpers.getRelativeItemViewBounds +import com.rubensousa.dpadrecyclerview.test.helpers.onRecyclerView +import com.rubensousa.dpadrecyclerview.test.tests.AbstractTestAdapter +import com.rubensousa.dpadrecyclerview.test.tests.DpadRecyclerViewTest +import com.rubensousa.dpadrecyclerview.testing.KeyEvents +import com.rubensousa.dpadrecyclerview.testing.rules.DisableIdleTimeoutRule +import org.junit.Before +import org.junit.Rule +import org.junit.Test + +class GridAdapterMutationTest : DpadRecyclerViewTest() { + + @get:Rule + val idleTimeoutRule = DisableIdleTimeoutRule() + + override fun getDefaultLayoutConfiguration(): TestLayoutConfiguration { + return TestLayoutConfiguration( + spans = 4, + orientation = RecyclerView.VERTICAL, + parentAlignment = ParentAlignment( + edge = ParentAlignment.Edge.MIN_MAX, + fraction = 0.5f + ), + childAlignment = ChildAlignment( + fraction = 0.5f + ) + ) + } + + override fun getDefaultAdapterConfiguration(): TestAdapterConfiguration { + return super.getDefaultAdapterConfiguration().copy( + numberOfItems = 40 + ) + } + + @Before + fun setup() { + launchFragment() + } + + @Test + fun testGridRemovalWithSpanLookupDoesNotCrash() { + onRecyclerView("Change span size") { recyclerView -> + recyclerView.setSpanSizeLookup(object : DpadSpanSizeLookup() { + override fun getSpanSize(position: Int): Int { + val adapter = recyclerView.adapter as AbstractTestAdapter<*> + val item = adapter.getItem(position) + return if (item % 9 == 0) { + recyclerView.getSpanCount() + } else { + 1 + } + } + }) + } + KeyEvents.pressDown() + val oldViewBounds = getRelativeItemViewBounds(position = 1) + mutateAdapter { adapter -> + adapter.removeAt(3) + adapter.removeAt(13) + } + assertFocusAndSelection(1) + assertItemAtPosition(position = 1, item = 1) + + val newViewBounds = getRelativeItemViewBounds(position = 1) + assertThat(newViewBounds).isEqualTo(oldViewBounds) + } + +} diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotLayoutManager.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotLayoutManager.kt index 73286406..597ffeae 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotLayoutManager.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotLayoutManager.kt @@ -258,24 +258,24 @@ class PivotLayoutManager(properties: Properties) : RecyclerView.LayoutManager() } override fun onItemsAdded(recyclerView: RecyclerView, positionStart: Int, itemCount: Int) { - configuration.spanSizeLookup.invalidateCache() + layoutInfo.invalidateSpanCache() pivotLayout.onItemsAdded(positionStart, itemCount) pivotSelector.onItemsAdded(positionStart, itemCount) } override fun onItemsChanged(recyclerView: RecyclerView) { - configuration.spanSizeLookup.invalidateCache() + layoutInfo.invalidateSpanCache() pivotSelector.onItemsChanged() } override fun onItemsRemoved(recyclerView: RecyclerView, positionStart: Int, itemCount: Int) { - configuration.spanSizeLookup.invalidateCache() + layoutInfo.invalidateSpanCache() pivotLayout.onItemsRemoved(positionStart, itemCount) pivotSelector.onItemsRemoved(positionStart, itemCount) } override fun onItemsMoved(recyclerView: RecyclerView, from: Int, to: Int, itemCount: Int) { - configuration.spanSizeLookup.invalidateCache() + layoutInfo.invalidateSpanCache() pivotLayout.onItemsMoved(from, to, itemCount) pivotSelector.onItemsMoved(from, to, itemCount) } diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt index e4aa5c08..621d335b 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt @@ -134,6 +134,10 @@ internal class LayoutInfo( return getStartSpanIndex(position) + configuration.spanSizeLookup.getSpanSize(position) - 1 } + fun invalidateSpanCache() { + configuration.spanSizeLookup.invalidateCache() + } + fun getSpanGroupIndex(position: Int): Int { return configuration.spanSizeLookup.getCachedSpanGroupIndex( position, diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt index c15f7f48..f8f5b410 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt @@ -589,6 +589,7 @@ internal class GridLayoutEngineer( ) return layoutInfo.getStartSpanIndex(position) } + layoutInfo.invalidateSpanCache() return layoutInfo.getStartSpanIndex(adapterPosition) } @@ -608,6 +609,7 @@ internal class GridLayoutEngineer( ) return layoutInfo.getSpanSize(position) } + layoutInfo.invalidateSpanCache() return layoutInfo.getSpanSize(adapterPosition) } From d02fb8c324c711bd01bc7bf3848e55f3533e5718 Mon Sep 17 00:00:00 2001 From: Ruben Sousa Date: Thu, 30 May 2024 13:30:49 +0200 Subject: [PATCH 2/5] Remove layout request log --- .../java/com/rubensousa/dpadrecyclerview/DpadRecyclerView.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/DpadRecyclerView.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/DpadRecyclerView.kt index 9ef68d8c..85632400 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/DpadRecyclerView.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/DpadRecyclerView.kt @@ -237,9 +237,6 @@ open class DpadRecyclerView @JvmOverloads constructor( final override fun requestLayout() { if (isRequestLayoutAllowed()) { - if (DEBUG) { - Log.i(TAG, "Layout Requested") - } super.requestLayout() } else { hasPendingLayout = true From aa483253d91cdb14a421c8dc60798bb175b8d87a Mon Sep 17 00:00:00 2001 From: Ruben Sousa Date: Thu, 30 May 2024 16:08:25 +0200 Subject: [PATCH 3/5] Improve layout logic by skipping blocks that cannot be laid out before they are pending removal --- .../dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt | 8 ++++++-- .../layoutmanager/layout/StructureEngineer.kt | 8 ++++++++ .../layoutmanager/layout/grid/GridLayoutEngineer.kt | 7 +++---- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt index 621d335b..a2a37ae6 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/LayoutInfo.kt @@ -126,8 +126,12 @@ internal class LayoutInfo( return configuration.spanSizeLookup.getSpanSize(position) } - fun getStartSpanIndex(position: Int): Int { - return configuration.spanSizeLookup.getCachedSpanIndex(position, configuration.spanCount) + fun getStartSpanIndex(position: Int, fromCache: Boolean = true): Int { + return if (fromCache) { + configuration.spanSizeLookup.getCachedSpanIndex(position, configuration.spanCount) + } else { + configuration.spanSizeLookup.getSpanIndex(position, configuration.spanCount) + } } fun getEndSpanIndex(position: Int): Int { diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/StructureEngineer.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/StructureEngineer.kt index 4e2b4669..9275c781 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/StructureEngineer.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/StructureEngineer.kt @@ -424,6 +424,14 @@ internal abstract class StructureEngineer( */ if (layoutResult.consumedSpace > 0) { viewRecycler.recycleByLayoutRequest(recycler, layoutRequest) + } else if (viewProvider.hasNext(layoutRequest, state)) { + Log.w( + DpadRecyclerView.TAG, + "View at position ${layoutRequest.currentPosition} could not be laid out" + ) + layoutRequest.moveToNextPosition() + } else { + remainingSpace = 0 } layoutResult.reset() diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt index f8f5b410..937a6538 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/grid/GridLayoutEngineer.kt @@ -587,10 +587,10 @@ internal class GridLayoutEngineer( DpadRecyclerView.TAG, "Cannot find post layout position for pre layout position: $position" ) - return layoutInfo.getStartSpanIndex(position) + return layoutInfo.getStartSpanIndex(position, fromCache = false) } - layoutInfo.invalidateSpanCache() - return layoutInfo.getStartSpanIndex(adapterPosition) + // Do not re-use span index cache since we are in pre-layout + return layoutInfo.getStartSpanIndex(adapterPosition, fromCache = false) } private fun getSpanSize(recycler: Recycler, state: State, position: Int): Int { @@ -609,7 +609,6 @@ internal class GridLayoutEngineer( ) return layoutInfo.getSpanSize(position) } - layoutInfo.invalidateSpanCache() return layoutInfo.getSpanSize(adapterPosition) } From bc21cddcea1bea36283d58c743b19abad62a407d Mon Sep 17 00:00:00 2001 From: Ruben Sousa Date: Thu, 30 May 2024 16:14:27 +0200 Subject: [PATCH 4/5] Bump to next version --- docs/changelog.md | 10 ++++++++++ gradle.properties | 2 +- mkdocs.yml | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 38cd6bbf..1ff7b480 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,16 @@ ## Version 1.3.0 +### 1.3.0-alpha03 + +2024-05-30 + +#### Bug fixes + +- Fixed issue with grid layouts with different spans after item removals. ([#210](https://github.com/rubensousa/DpadRecyclerView/issues/210)) +- Disable layout passes during scroll events by default. This is an attempt to fix ([#206](https://github.com/rubensousa/DpadRecyclerView/issues/206)) and ([#207](https://github.com/rubensousa/DpadRecyclerView/issues/207)) +To fallback to the previous behavior, please use `setLayoutWhileScrollingEnabled(true)` + ### 1.3.0-alpha02 2024-03-23 diff --git a/gradle.properties b/gradle.properties index 1cfa48b4..c9556f88 100644 --- a/gradle.properties +++ b/gradle.properties @@ -22,4 +22,4 @@ kotlin.code.style=official # thereby reducing the size of the R class for that library android.nonTransitiveRClass=true android.enableR8.fullMode=true -LIBRARY_VERSION=1.3.0-alpha02 \ No newline at end of file +LIBRARY_VERSION=1.3.0-alpha03 \ No newline at end of file diff --git a/mkdocs.yml b/mkdocs.yml index ed49387f..22c818a4 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -24,7 +24,7 @@ theme: extra: dpadrecyclerview: - version: '1.3.0-alpha02' + version: '1.3.0-alpha03' social: - icon: 'fontawesome/brands/github' link: 'https://github.com/rubensousa/DpadRecyclerView' From 664f79920e4e47eeb7988dfe0f12c14355b41fb0 Mon Sep 17 00:00:00 2001 From: Ruben Sousa Date: Fri, 31 May 2024 00:43:22 +0200 Subject: [PATCH 5/5] Update changelog for latest changes --- docs/changelog.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 1ff7b480..1cb37180 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -4,13 +4,21 @@ ### 1.3.0-alpha03 -2024-05-30 +2024-05-31 + +#### New Features + +- Added `DpadStateRegistry` that assists in saving and restoring the scroll state or view state of ViewHolders ([#45](https://github.com/rubensousa/DpadRecyclerView/issues/45)) + +#### Improvements + +- Disable layout passes during scroll events by default. This is an attempt to fix ([#207](https://github.com/rubensousa/DpadRecyclerView/issues/207)) + To fallback to the previous behavior, please use `setLayoutWhileScrollingEnabled(true)` #### Bug fixes - Fixed issue with grid layouts with different spans after item removals. ([#210](https://github.com/rubensousa/DpadRecyclerView/issues/210)) -- Disable layout passes during scroll events by default. This is an attempt to fix ([#206](https://github.com/rubensousa/DpadRecyclerView/issues/206)) and ([#207](https://github.com/rubensousa/DpadRecyclerView/issues/207)) -To fallback to the previous behavior, please use `setLayoutWhileScrollingEnabled(true)` +- Fixed `DpadRecyclerView` losing focus in some cases when adapter contents are updated during scroll events. ([#206](https://github.com/rubensousa/DpadRecyclerView/issues/206)) ### 1.3.0-alpha02