Status Update
Comments
li...@gmail.com <li...@gmail.com> #2
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
du...@google.com <du...@google.com> #3
du...@google.com <du...@google.com> #4
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
du...@google.com <du...@google.com> #5
Hmm so I thought about this for awhile and it seems quite difficult to solve - essentially what is happening is we are prepending more items than there are number of placeholders due to the addition of separators, so it's impossible to scroll to the top before prepending because we don't know how many items to scroll up by. Basically the placeholders don't map 1:1 to the original item, so we have to decide how and where to notify RV that there are extra items and the current behavior for prepend is to always send the onInserted event to notify new items above the current position to handle the regular scrolling via swipe scenario.
In the swipe scroll case, user wants to see the bottom items from prepended page first, and then have extra items added above the scroll position, but in the scroll to top case we want to see the top items first, and then add extra items to the bottom. Since scrollToPosition always puts the desired index to position at the top, we need some way to tell paging that if we prepend due to call to scrollToPosition, we basically need to switch the order of the differ events to work like append instead.
As a simple workaround, you can set jumpThreshold
in PagingConfig
to be the same as maxSize
, and this will cause paging to use the refresh / invalidate flow to load from the scrollToPosition index (which is what you want anyway in large jumps instead of prepending until you get there).
ta...@monzo.com <ta...@monzo.com> #6
Maybe there's another way to look at this. Is there a way to 'reset' the RecyclerView, the PagingDataAdapter and PagingSource that's coming from Room, so that they ignore anything that's happened up to now (any scrolling or loading different pages from the db), and behave as if they've just been bound for the first time again?
yb...@google.com <yb...@google.com> #7
I'm afraid this is also hard for recyclerview (both both smooth and non-smooth scroll).
Basically, when you try to scroll to a position, recyclerview just lays out in that position. but if paging adds more items above, that happens after RecyclerView scrolls. The same thing happens if you try to scroll to a non-existing position (e.g. you only have 50 in memory and you scroll to 100) It will just stop 50.
Due to the nature of this problem (not knowing when data will stop coming), a possible solution is to actually write a custom scroller over recyclerview that keeps pushing it to 0 as items are loaded.
For jump scenerios, it might be even better to just get a new paging flow that starts w/o an initial key and also set a new adapter on the recyclerview (so that it does not try to diff). If you use the same adapter, you can consider calling notifyDataSetChanged when list is updated on the adapter, that should fool RecyclerView to just render from scratch.
ta...@monzo.com <ta...@monzo.com> #8
yb...@google.com <yb...@google.com> #9
Without stable list, jumping won't really be feasible. I think the only way it can work is to create a new pager w/ an initial key to restart the data from that location (combined w/ scrollToPosition(0) )
du...@google.com <du...@google.com> #10
While playing with my workaround I noticed it only works in some cases and as there is a separate bug causing multiple jumps to happen on scroll to top. This is why it ends up scrolling downwards after jumping to the top due to jumping.
I believe the root cause is due to conflated hints causing new generation to reload a second time.
du...@google.com <du...@google.com> #11
So it looks like the second jump up is due to the differ events out of diffutil, it doesn't understand the overlap since the list sizes are mismatches so we need to add some special case handling in paging for this.
ap...@google.com <ap...@google.com> #12
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
du...@google.com <du...@google.com> #13
After this change - setting jump threshold now works as expected :)
Description
Version used: 3.0.0-alpha11
Devices/Android versions reproduced on: All devices
I've attached a modified version of the PagingSample app, which demonstrates the problem.
The only changes I made where to:
- Add a button at the bottom of the layout which is supposed to scroll the list of cheeses to the top
- Add separators in random intervals between cheeses (whenever the previous cheese name has more than 9 characters)
- Prefixed cheese rows with the adapter position. I know that this is not always correct due to paging, but it helps a bit to know where you are at each point.
Steps to reproduce the problem:
- Scroll all the way to bottom of the list (or at least more than maxSize)
- Press the button to scroll to the top
What happens:
- You are positioned higher in the list, but not at the top
What should happen:
- You should be positioned to the top of the list
My theory is that the placeholders only appear for the rows that come from the DB, but not for the date separators. That’s why all the calculations are wrong when a page is dropped from the top. If you remove the separators, the problem goes away.
Of course the problem is that in order to calculate whether a placeholder should appear, you need the information from the db. In a real world scenario, think of date separators in a timeline feed. So I don't know whether there is a solution to this problem, but I'd love to hear it.
PS: In my real app, all I really want to do is scroll to the top. Ideally we'd need a solution that works for every position, but if you have a workaround for just going all the way to the top, that's good enough for me for now.