[Bug] Regression: XF nightly completely broke CollectionView scrolling

[Tommigun1980]

Description

Steps to Reproduce

1. Update a project to XF 5.0.1.2000-nightly
2. Try to flick a CollectionView
3. Observe that scrolling is broken

Here is a video of the issue: https://drive.google.com/file/d/1CcDqvFsl8XihcqU-YA57eWQIAjF4PxPs/view?usp=sharing

There are two new issues here with scrolling:

1. When flicking the CollectionView it will start bouncing around randomly, sometimes getting stuck in a loop. This makes it impossible to scroll it.
2. The CollectionView can no longer be scrolled to bottom (visible at 0.08 in the video). No matter how much down you try to scroll, the last couple of centimetres can never be made visible.

Only tested this on iOS.

Expected Behavior

CollectionView scroll should work.

Actual Behavior

CollectionView starts doing random things when scrolled, and can not be scrolled to bottom.

Basic Information

• Version with issue: XF 5.0.1.2000-nightly
• Last known good version: XF 5.0.1.1876
• Platform Target Frameworks:
  • iOS: 14.4

Environment

Show/Hide Visual Studio info

=== Visual Studio Community 2019 for Mac (Preview) ===

Version 8.9 Preview (8.9 build 1582)
Installation UUID: f9c0a2f9-a5b5-4b5a-8b2d-b84062db3910
	GTK+ 2.24.23 (Raleigh theme)
	Xamarin.Mac 6.18.0.23 (d16-6 / 088c73638)

	Package version: 612000113

=== Mono Framework MDK ===

Runtime:
	Mono 6.12.0.113 (2020-02/4fdfb5b1fd5) (64-bit)
	Package version: 612000113

=== Roslyn (Language Service) ===

3.9.0-4.21104.8+2fc92d84e746c3aab75f0930278ea6675cd5bb5c

=== NuGet ===

Version: 5.8.0.6860

=== .NET Core SDK ===

SDK: /usr/local/share/dotnet/sdk/5.0.103/Sdks
SDK Versions:
	5.0.103
	5.0.102
	5.0.101
	5.0.100
	3.1.406
	3.1.405
	3.1.404
	3.1.403
MSBuild SDKs: /Applications/Visual Studio.app/Contents/Resources/lib/monodevelop/bin/MSBuild/Current/bin/Sdks

=== .NET Core Runtime ===

Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
	5.0.3
	5.0.2
	5.0.1
	5.0.0
	3.1.12
	3.1.11
	3.1.10
	3.1.9

=== .NET Core 3.1 SDK ===

SDK: 3.1.406

=== Xamarin.Profiler ===

Version: 1.6.15.68
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Updater ===

Version: 11

=== Xamarin.Android ===

Version: 11.2.0.0 (Visual Studio Community)
Commit: xamarin-android/d16-9/f908d16
Android SDK: /Users/tommikiviniemi/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		None installed

SDK Tools Version: 26.1.1
SDK Platform Tools Version: 30.0.4
SDK Build Tools Version: 30.0.2

Build Information: 
Mono: 5e9cb6d
Java.Interop: xamarin/java.interop/d16-9@1d382be
ProGuard: Guardsquare/proguard/v7.0.1@912d149
SQLite: xamarin/sqlite/3.32.2@cfe06e0
Xamarin.Android Tools: xamarin/xamarin-android-tools/main@ad80a42

=== Microsoft OpenJDK for Mobile ===

Java SDK: /Users/tommikiviniemi/Library/Developer/Xamarin/jdk/microsoft_dist_openjdk_1.8.0.25
1.8.0-25
Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Android SDK Manager ===

Version: 16.9.0.21
Hash: 57e40ba
Branch: remotes/origin/main
Build date: 2021-01-08 01:57:14 UTC

=== Android Device Manager ===

Version: 16.9.0.14
Hash: 0fdccda
Branch: remotes/origin/main
Build date: 2021-01-08 01:57:36 UTC

=== Apple Developer Tools ===

Xcode 12.4 (17801)
Build 12D4e

=== Xamarin.Mac ===

Xamarin.Mac not installed. Can't find /Library/Frameworks/Xamarin.Mac.framework/Versions/Current/Version.

=== Xamarin.iOS ===

Version: 14.14.1.0 (Visual Studio Community)
Hash: 17845a189
Branch: d16-9
Build date: 2021-01-26 03:42:38-0500

=== Xamarin Designer ===

Version: 16.9.0.309
Hash: 9a5259fb5
Branch: remotes/origin/9a5259fb55e54a445c145ab5ea29c04a418deed2
Build date: 2021-02-04 09:34:28 UTC

=== Build Information ===

Release ID: 809001582
Git revision: b0620b767dbf9f79bbacc2053ae155a66f147e04
Build date: 2021-02-04 12:13:43-05
Build branch: release-8.9
Xamarin extensions: b0620b767dbf9f79bbacc2053ae155a66f147e04

=== Operating System ===

Mac OS X 10.16.0
Darwin 20.3.0 Darwin Kernel Version 20.3.0
    Thu Jan 21 00:07:06 PST 2021
    root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64

Build Logs

Screenshots

Reproduction Link

1. Download the repro project from [Bug] Mega repro project for CollectionView issues here #13231
2. Update it to XF 5.0.1.2000-nightly
3. Flick the CollectionView

Workaround

None, must downgrade Xamarin.Forms.

Please -- when fixing a bug in CollectionView, run the repro project at #13231 (or another project of equal complexity)! It will save all of us a lot of time. Fixing a bug without running a project that uses said component is a 100% certain way to break something else.
The test project must be complex enough to use more advanced features. It is untenable to fix one thing and break something else, if it can at all be avoided.
Thank you!!

[jsuarezruiz]

@Tommigun1980 To validate the issue, can use the repro project at #13231 (without changes) just using XF 5.0.1.2000-nightly?

[Tommigun1980]

@Tommigun1980 To validate the issue, can use the repro project at #13231 (without changes) just using XF 5.0.1.2000-nightly?

Hi. Yes.

[jsuarezruiz]

Can reproduce the issue.
cc @hartez

[hartez]

Well, this is interesting (and vexing).

I can reproduce this problem (or at least the "can't scroll all the way to the bottom" part of it) on notch devices. On an iPhone 8 and SE, everything is peachy.

[hartez]

... and I take that back. If I deliberately slow down the scrolling with some debugging statements, I can make this fail on an iPhone 8 as well. This appears to be a cache invalidation issue.

[programmation]

@hartez I have to admit after studying the XF source I'm puzzled as to why it uses its own cell cache when there's already a perfectly good one in iOS. The XF cell cache has been a fertile source of bugs, and I'm not entirely convinced it's necessary. Do things really slow down if the cell cache is removed?

[hartez]

@hartez I have to admit after studying the XF source I'm puzzled as to why it uses its own cell cache when there's already a perfectly good one in iOS.

The XF cell cache has been a fertile source of bugs, and I'm not entirely convinced it's necessary.

The cell size cache has only been in Forms since [checks notes] ... 5.0.0. It's closed more bugs than it's opened. Unless you're talking about something else.

Do things really slow down if the cell cache is removed?

Yes, but probably not in the way you're thinking.

UICollectionViewFlowLayout's cache works fine if the items you need are already in the cache at the proper size. But for certain situations (e.g., animating add/remove actions), the layout doesn't have cached data. So it has to get size information for cells from somewhere. (To be clear, the add animations aren't so much of a problem; it's the remove animations where things get really ugly.)

If you have a constant size set, no problem; UICVFL knows what size the item will be. If there's an estimatedItemSize set, then it can use that size until autolayout kicks in and the cell determines its final size. If the estimated size is pretty accurate, the size fluctuation during animation/layout is pretty minimal, and everything looks fine.

The problem comes when the actual final sizes are very different than the estimate. You start to get all kinds of weird ugly things happening during animation; you also end up having weird layout issues during scrolling, because the layout doesn't have good information on cell sizes to lay them out smoothly.

The way to deal with this is to implement sizeForItemAtIndexPath from the UICollectionViewDelegateFlowLayout protocol. The method returns the exact size for the item when requested, so the flow layout can get all the info it needs to layout things out properly. It can also cache this information.

Which sounds peachy - we can just implement that method, and for every indexPath the layout requests we just figure out what size it should be and return that size. Huzzah!

But there's a snag: the flow layout will call this method for every single cell. In Forms, because of databinding, we can't know the final size of ... well, pretty much anything until we've bound it. We can't even know for sure what DataTemplate we'll be inflating until we've run the BindingContext for the item through the DataTemplateSelector. So for every single item in the CollectionView, we would have to run the DataTemplateSelector, get the template, create the template's native content, bind all that, and then finally measure it.

Which works fine if you've got a CollectionView with a few dozen items. But it's disastrous (for both speed and memory) if you have a CollectionView with 10,000 items in it. The layout will gleefully ask for every one of them, and Forms will inflate and bind 10,000 items. Then it'll do it again later when you actually scroll to them.

So, instead, the Forms implementation of sizeForItemAtIndexPath returns the estimatedItemSize for most of the requests, which is very fast. And works just fine for the flow layout to do things like determining the scroll bar position.

And when autolayout kicks in and the actual Forms measure steps happen, we cache those final values in the Forms layer. That way, in the cases where the UICVFL's cache doesn't have them (remove actions, scrolling around, etc.) we can just return the cached sizes from the Forms implementation of sizeForItemAtIndexPath.

The upside is that it fixes a bunch of our problems with weird layout issues because of wildly varying cell sizes. The obvious downside is that now we have the potential for cache invalidation issues. (Caches are like regexes in this way.)

Anyway, that was a big infodump on why we cache cell sizes. I would very much prefer not to have to do it, but so far I don't have a good alternate solution. If someone has a better solution that doesn't require a cache and also scales, I will review that PR with gusto :)

[programmation]

@hartez Thank you so much for the detailed explanation. In my thinking the role currently played by the Forms cell cache could be played instead by the UICollectionView's internal cell cache, provided that there's some kind of mapping from the DataTemplateSelector's type for an item to the cell id used by the UICollectionView's GetCell callback. Then the UICollectionView itself would return (or create) a UICollectionViewCell of the correct type, which could be data bound, measured, and displayed.

[Tommigun1980]

@hartez Hi and thank you for looking into this!

Unfortunately the latest nightly build did not solve this even though the fix was committed to the main branch three days ago. Is this fix in the latest nightly? The error still happens both in my main project, and in the repro case when updating to latest nightly.
Thank you.