Improve iOS CollectionView performance by leveraging the new platform level invalidation mechanism #28225
Conversation
jsuarezruiz
added
area-controls-collectionview
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
albyrock87
force-pushed
the
avoid-using-measure-invalidate-in-collectionview
branch
from
25d9ddc to
ad15928
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| { | ||
| if (size.IsCloseTo(_currentSize)) | ||
| if (size.IsCloseTo(_currentSize) && !forceUpdate) |
There was a problem hiding this comment.
When used as ContentPage.Content = new CollectionView() the _currentSize never changes so this code path was not hit on the first LayoutSubviews, leading to wrong preferredAttributes computation and bad cell positioning.
You can verify this on main by testing out Issue13203 where the text is being positioned in the middle behind the safe area instead of being left aligned.
| @@ -193,18 +194,37 @@ public override void LoadView() | |||
| public override void ViewWillAppear(bool animated) | |||
| { | |||
| base.ViewWillAppear(animated); | |||
| ConstrainItemsToBounds(); | |||
There was a problem hiding this comment.
This makes no sense here because bounds are not set yet: they'll be in ViewWillLayoutSubviews.
So any computation made here cannot be trustworthy.
There was a problem hiding this comment.
If we navigate from a CV page to a new page and then back to the CV sometimes ViewWillLayoutSubviews will not always trigger.
There was a problem hiding this comment.
Interesting, but why is it necessary to trigger this method again considering the view was already laid out before pushing the new page? @rmarinho
albyrock87
force-pushed
the
avoid-using-measure-invalidate-in-collectionview
branch
2 times, most recently
from
3a4813d to
4a71e68
Compare
albyrock87
force-pushed
the
avoid-using-measure-invalidate-in-collectionview
branch
from
4a71e68 to
10e4731
Compare
albyrock87
force-pushed
the
avoid-using-measure-invalidate-in-collectionview
branch
from
da31518 to
025e9ad
Compare
|
@jsuarezruiz done, you can run AZP again |
|
/azp run |
| } | ||
| } | ||
|
|
||
| if (invalidatedPaths != null) |
There was a problem hiding this comment.
| if (invalidatedPaths != null) | |
| if (invalidatedPaths is not null) |
| @@ -37,6 +37,7 @@ protected void InitializeContentConstraints(UIView platformView) | |||
| ContentView.TrailingAnchor.ConstraintEqualTo(TrailingAnchor).Active = true; | |||
|
|
|||
| // And we want the ContentView to be the same size as the root renderer for the Forms element | |||
| // TODO: we should probably remove this to support `Margin` applied to the cell's root `VirtualView` | |||
There was a problem hiding this comment.
We should create a new issue for this, maybe for net10.0
github-project-automation
bot
moved this from Ready To Review
to Approved
in MAUI SDK Ongoing
|
/backport to inflight/candidate |
|
Started backporting to inflight/candidate: https://github.com/dotnet/maui/actions/runs/14072653418 |
|
@PureWeen backporting to "inflight/candidate" failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Use native invalidation mechanism to detect TemplatedCell size change.
Using index info to reconstruct a base tree...
M src/Controls/src/Core/Handlers/Items/iOS/TemplatedCell.cs
M src/Controls/src/Core/Handlers/Items2/iOS/TemplatedCell2.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Controls/src/Core/Handlers/Items2/iOS/TemplatedCell2.cs
CONFLICT (content): Merge conflict in src/Controls/src/Core/Handlers/Items2/iOS/TemplatedCell2.cs
Auto-merging src/Controls/src/Core/Handlers/Items/iOS/TemplatedCell.cs
CONFLICT (content): Merge conflict in src/Controls/src/Core/Handlers/Items/iOS/TemplatedCell.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Use native invalidation mechanism to detect TemplatedCell size change.
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
Description of Change
This is the second part of #25664 which leverages the work done in #26629.
The PR gets rid of
MeasureInvalidatedsubscription and fixes CV2 handler so thatTemplatedCell2can react to content size changes.You can see this by using "Item Size Gallery" for
CollectionView.Using #25671 UI test I've been able to measure the benefits of this PR over
mainbranch.Please note
no-resizeon CV2main(as item size detection was not implemented).mainPRmainPRTo highly reduce measurements I've marked the
TemplatedCell*.ContentViewwith aTagthat treats it as kind-of-ICrossPlatformLayoutBackingso that the containedMauiView/LayoutViewtreats it asIsFinalMeasureHandledBySuperViewand avoids remeasuring its content with the appliedBoundsonLayoutSubviews.Issues Fixed
Fixes #25650
Fixes #25391
Videos of AllTheLists
Captured using a modified version of https://github.com/davidortinau/AllTheLists.git compiled in
Releasemode with a constant number of lessons generated (high number).challenging.patch
Using CV1
PR-25664-CV1-AllTheLists.mp4
Using CV2
PR-25664-CV2-AllTheLists.mp4