Managed snapping (ListView, ScrollViewer) uses hard-coded delay out of sync with inertia

[takla21]

Current behavior

While using the uno implementation for item snapping, it takes time before snap.

Uno.UI.FeatureConfiguration.NativeListViewBase.UseNativeSnapHelper = false;

Expected behavior

It should be a bit faster.

How to reproduce it (as minimally and precisely as possible)

• Download this sample SnappingListView_slow.zip
• Deploy on android
• Swipe through items

Workaround

N/A

Works on UWP/WinUI

N/A

Environment

Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia, Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia

NuGet package version(s)

4.6.39

Affected platforms

Android

IDE

Visual Studio 2022

IDE version

17.4.3

Relevant plugins

No response

Anything else we need to know?

No response

[jeromelaban]

@Xiaoy312 @dr1rrb does this look familiar?

[Xiaoy312]

yeah, so in order to get rid of the unoplatform/nventive-private#421 infinite oscillating bug
i added the opt-in option to disable the native snapping (FeatureConfiguration.NativeListViewBase.UseNativeSnapHelper), which were fighting with our implementation of snapping
by doing so, it had also turned off the scrolling velocity/momentum that comes with it

[jeromelaban]

@takla21 try this out and let us know, we'll otherwise close the issue. Thanks!

[Xiaoy312]

@jeromelaban they already applied this UseNativeSnapHelper=false to fix another bug
however, by doing so, it also disabled scrolling momentum which they desired to keep (hence this issue)
edit: we still have momentum, it is the snapping is triggered on a delay*

[takla21]

@Xiaoy312 @jeromelaban So to summarize what was discussed here, if the uno snap implementaion, is there a way we can make it smoother or it's something we lost by disabling scrolling momentum?

[Xiaoy312]

For context, in #10390, we disabled the native snapping because its snap target is different than the uno snap target.
and because uno snap can trigger from native snap and vis-versa, we get a no-ending oscillation on any scroll.
As a ~~solution~~workaround, we added a FeatureConfiguration.NativeListViewBase.UseNativeSnapHelper to disable native snap, keeping only the uno snap. (native is incorrect here.)

ScrollViewer::OnPresenterScrolled(horizontalOffset, verticalOffset, isIntermediate)
    ^ continuously fired on each "delta" of scrolling with final release marked by: isIntermediate=false
    ^ the 250ms delay on _snapPointsTimer is the main complaint of this issue
    
We can replace the 250ms with a configurable property. The problem with this, is that it only trigger after the velocity has depleted. So even if we use a delay of 0ms, the LV would 1st perform the user scroll, come to a full stop, and then a 2nd scroll is initiated toward the desired snap-point. This is a pretty jarring experience, especially the snap-point is in the opposite direction of the initial user scroll.
---
with uno snap disabled, native snap is kind of broken by itself..? always snap back to original item, prevent us from scrolling to another item
    ^ even if we repeatedly scroll quickly to reach items#4-5, it will still manage to snap back to item#0...
delving into NativeListViewBase::SnapPointsSnapHelper impl and its references doesnt seem to provide anything interesting
randomly changing SnapPointsSnapHelper also doesnt lead anywhere...

we should also be able to just simply use an concrete SnapHelper, like PagerSnapHelper or LinearSnapHelper,
but that has also revealed another problem in uno:
- PagerSnapHelper makes it behave like a FlipView that works nicely in all case
- LinearSnapHelper gets the expected result in some case:
	^ with small scroll/fling that moves no more than 1 items, it scrolls and snaps nicely
	^ any gesture bigger than that, it launches across 10-ish items and then bounce back to about the starting position and repeating bounce between that 10 items infinitely... until another manual scroll takes over
		^ while that happens the overridable methods are only ever called once at the beginning
		^ so it is unclear what is even causing it..? and, it should be in no way fault of LinearSnapHelper, as that is Android's impl
	^ and there is no middle ground between 1-item scroll vs 10-items scroll

[jeromelaban]

@dr1rrb can help

[jeromelaban]

@Xiaoy312 mentions that we need to reevaluate after #11962 is merged.

[Xiaoy312]

#11962 may address the some irks with the LinearSnapHelper that I've noted in my last comment
which is why we should reevaluate once that is merged

note: we would need to update the code, replacing SnapPointsSnapHelper with LinearSnapHelper in
https://github.com/unoplatform/uno/blob/4.8.15/src/Uno.UI/UI/Xaml/Controls/ListViewBase/NativeListViewBase.SnapPoints.Android.cs#L20

GitHub

uno/NativeListViewBase.SnapPoints.Android.cs at 4.8.15 · unoplatform/uno

Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/NativeListViewBase.SnapPoints.Android.cs at 4.8.15 · unoplatform/uno

[dr1rrb]

With #11962 and with Uno.UI.FeatureConfiguration.NativeListViewBase.UseNativeSnapHelper = true; (i.e. the default value) in the startup of the app, the sample is working pretty well (even if I increase the number of items to force the LV to virtualize items).

So I think this is no longer "urgent" and as it will require a significant amount of work, I think we can postpone this. Opinion @jeromelaban @takla21 @Xiaoy312 ?

TODO: We need to remove the hard-coded delay and configure the inertia end on a snappoint - currently the idea was to wait the given delay before requesting the snapping in order to let the inertia slow down a bit before aborting it and starting the "scroll to" animation at a fairly equivalent speed)

[dr1rrb]

note: we would need to update the code, replacing SnapPointsSnapHelper with LinearSnapHelper in https://github.com/unoplatform/uno/blob/4.8.15/src/Uno.UI/UI/Xaml/Controls/ListViewBase/NativeListViewBase.SnapPoints.Android.cs#L20

@Xiaoy312 Tried with both the SnapPointsSnapHelper and LinearSnapHelper, I didn't see any differences 🤷‍♂️

GitHub

uno/NativeListViewBase.SnapPoints.Android.cs at 4.8.15 · unoplatform/uno

Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/NativeListViewBase.SnapPoints.Android.cs at 4.8.15 · unoplatform/uno

[takla21]

@dr1rrb I forgot to mention it, but I recently tested the previous workaround with the delay variable and it's should be enough for us.

[dr1rrb]

@takla21 did you tried with some inertia? Indeed with delay set to 0 it works well if you release the move pointer in the center of the screen, wait a few and then release. However if you "swipe" the screen you might have some weird animations, so I would recommend you, once #11962 has been merged, to use the native snapping (i.e. set Uno.UI.FeatureConfiguration.NativeListViewBase.UseNativeSnapHelper = true;).

[takla21]

@dr1rrb Here's a screenrecording from the sample

So it seems that it will wait until it's done moving before doing the snap thing. Which makes it a bit robotic in the end, but yeah we're still going with it for now. I'll revisit using the native one once it's ready

Here's the previous sample updated to latest uno version (just in case)
SnappingListView_new.zip