Status Update
Comments
yb...@google.com <yb...@google.com>
yb...@google.com <yb...@google.com> #2
Hey, I wanted to give an update since we've been silent here and you've provided an amazing bug report (seriously, we hardly ever get good repro samples, let alone a sample w/ states) We were silent because we were focusing on APIs to get to beta.
For the last 3 days, I've been trying to improve how we send updates to the RecyclerView about placeholders (your case A). There are two reasons why you are seeing those weird placeholers:
a) When placeholders change position, we don't send updates for them. We can start doing that though keep in mind, RecyclerView does not rebind an item just because its position changed so if you use bind position in your UI in any other case, you might hit that kind of issue (even w/o using Paging)
b) Once we fix issue A, you'll notice that the list starts jumping in your example. This happens when the previous list and updated list do not have any overlaps and/or we have placeholders on screen (specifically top of the screen which RecyclerView/LinearLayoutManager uses as a reference point to relayout). We simply do not have enough information to send those placeholder updates correctly (but neither the data source).
To give an example, Assume that you have a paging list that has 10 leading placeholders, 5 items (a,b,c,d,e) and 20 trailing placeholders. So it looks like:
placeholder_0, placeholder_1...placeholder_9, a, b, c, d, e, placeholder_15... placeholder 34
And then assume user is viewing index 9 until 12 (so that is placeholder_9, a, b
)
Then a reload happens and the new list comes as 20 leading placeholders, 5 items (a,b,c,d,e) and 10 trailing placeholders. So it looks like:
placeholder_0, placeholder_1...placeholder_19, a, b, c, d, e, placeholder_25... placeholder 34
Now what paging does not know is that whether that newly added 10 leading placeholders are:
- added before placeholder 0
- added somewhere in leading placeholders
- added right after the 10 placeholders
- And a lot more variations based on the content, e.g. maybe those items also moved
The updates we send to the RecyclerView is crucial because it will use its top item (placeholder_9 in this case) as a reference point to relayout if it is still in the list.
So if we paging assumed (1) or (2),
Recyclerview would realize that index 9 in the previous list (placeholder_9) is now in index 19 in the new list and will relayout from there, showing the expected layout (placeholder_19, a, b
)
But if we assumed (3),
RV would think that index 9 is still at index 9 so it would layout (placeholder_9, p_10, p_11
). Practically changing what user sees in an undesirable way.
Now even though (3) looks completely wrong in that case, it could also be the desired behavior in other cases. For instance, imagine in the first list, user was viewing indices 3 until 6 (p_3, p_4, p_5
)
Then when we get the update, if we did 1, RecyclerView would think p_3
is now at p_13
and layout p_13, p_14, p_15
which causes the jump
effect visually.
This is what makes it impossible to properly guess which updates the send. There are plenty of examples like this. :(
Now we could technically improve this heuristic based on what user is seeing but even then adapter does not have enough information. For instance, if the LayoutManager is configured to stackFromEnd
, the whole reference point would change, making such calculations even more wrong. And to make matters worse, Paging has no information about the layout manager (remember, it is just an adapter) to make these assumptions.
I'm still trying to see how we can make these heuristics correct in more common cases but it will never be 100% correct without some additional API where developer can aide to the process and optimize it for their use cases.
yb...@google.com <yb...@google.com> #3
yb...@google.com <yb...@google.com> #4
btw, a bug in your sample (in case it is present in your real app)
for prepend, you are returning the data in reverse. you are not expected to do that. the bug does not show up in your sample because you are constantly refreshing the list but w/o that, you would see the list loading in weird order if it ever started from bottom.
ap...@google.com <ap...@google.com> #5
Branch: androidx-main
commit 4eabdc68446c5268a9a6f1d80546703eba18d3b1
Author: Yigit Boyar <yboyar@google.com>
Date: Fri Mar 05 10:47:07 2021
Improve NullPaddedList diffing
This CL updates NullPaddedList diffing heuristic to better optimize for
common cases.
There two common cases we optimize for:
a) For completely distinct lists, we dispatch them as change animations
whenever possible based on positions (as long as there is no item in the
same global position in the old and new list)
b) For cases where lists partially overlap, we use available
placeholders to dispatch inserts or convert removals to placeholders
WHEN those updates happen at the boundaries of the list. This especially
handles cases where placeholders are loaded and prevents recyclerview
from scrolling to keep placeholders on screen.
See NullPaddedDiffing.md document for more details.
Bug: 170027529
Bug: 177338149
Test: NullPaddedListDiffWithRecyclerViewTest
Test: AsyncPagedListDifferTest
Test: NullPaddedListDiffHelperTest
Relnote: "We've rewamped how placeholders are handled when list is
reloaded to prevent unexpected jumps in RecyclerView. See
NullPaddedDiffing.md for details."
Change-Id: If1490f5bc833a61793d27eeaae9b37b26153df87
M paging/common/api/public_plus_experimental_3.0.0-beta03.txt
M paging/common/api/public_plus_experimental_current.txt
M paging/common/src/main/kotlin/androidx/paging/PagingDataDiffer.kt
M paging/runtime/build.gradle
M paging/runtime/src/androidTest/java/androidx/paging/AsyncPagedListDifferTest.kt
M paging/runtime/src/androidTest/java/androidx/paging/AsyncPagingDataDifferTest.kt
M paging/runtime/src/androidTest/java/androidx/paging/NullPaddedListDiffHelperTest.kt
A paging/runtime/src/androidTest/java/androidx/paging/NullPaddedListDiffWithRecyclerViewTest.kt
M paging/runtime/src/main/java/androidx/paging/AsyncPagedListDiffer.kt
A paging/runtime/src/main/java/androidx/paging/NullPaddedDiffing.md
M paging/runtime/src/main/java/androidx/paging/NullPaddedListDiffHelper.kt
Description
Component used: androidx.paging:paging-runtime
Version used: 3.0.0-alpha07
Discovered a bug where the scroll position is not preserved when page data is loaded outside visible area while only the placeholders are visible. This can happen when a PagingSource with placeholders enabled takes a while to load the data while user is scrolling through the recyclerview.
Sample project: Issue A fromhttps://github.com/yhpark/androidx-paging3-issues