-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Improve iOS CollectionView performance by leveraging the new platform level invalidation mechanism #28225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve iOS CollectionView performance by leveraging the new platform level invalidation mechanism #28225
Changes from all commits
fbb04bd
10e4731
025e9ad
0e6fecc
d569066
edec326
f8e5c54
27ac934
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,7 @@ public abstract class ItemsViewController<TItemsView> : UICollectionViewControll | |||||
| protected ItemsViewLayout ItemsViewLayout { get; set; } | ||||||
|
|
||||||
| bool _initialized; | ||||||
| bool _laidOut; | ||||||
| bool _isEmpty = true; | ||||||
| bool _emptyViewDisplayed; | ||||||
| bool _disposed; | ||||||
|
|
@@ -193,18 +194,48 @@ public override void LoadView() | |||||
| public override void ViewWillAppear(bool animated) | ||||||
| { | ||||||
| base.ViewWillAppear(animated); | ||||||
| ConstrainItemsToBounds(); | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes no sense here because bounds are not set yet: they'll be in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we navigate from a CV page to a new page and then back to the CV sometimes ViewWillLayoutSubviews will not always trigger.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting, but why is it necessary to trigger this method again considering the view was already laid out before pushing the new page? @rmarinho |
||||||
| } | ||||||
|
|
||||||
| public override void ViewWillLayoutSubviews() | ||||||
| { | ||||||
| ConstrainItemsToBounds(); | ||||||
|
|
||||||
| if (CollectionView is Items.MauiCollectionView { NeedsCellLayout: true } collectionView) | ||||||
| { | ||||||
| InvalidateLayoutIfItemsMeasureChanged(); | ||||||
| collectionView.NeedsCellLayout = false; | ||||||
| } | ||||||
|
|
||||||
| base.ViewWillLayoutSubviews(); | ||||||
| InvalidateMeasureIfContentSizeChanged(); | ||||||
| LayoutEmptyView(); | ||||||
|
|
||||||
| _laidOut = true; | ||||||
| } | ||||||
|
|
||||||
| void InvalidateLayoutIfItemsMeasureChanged() | ||||||
| { | ||||||
| var visibleCells = CollectionView.VisibleCells; | ||||||
| List<NSIndexPath> invalidatedPaths = null; | ||||||
|
|
||||||
| var visibleCellsLength = visibleCells.Length; | ||||||
| for (int n = 0; n < visibleCellsLength; n++) | ||||||
| { | ||||||
| if (visibleCells[n] is TemplatedCell { MeasureInvalidated: true } cell) | ||||||
rmarinho marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| { | ||||||
| invalidatedPaths ??= new List<NSIndexPath>(visibleCellsLength); | ||||||
| var path = CollectionView.IndexPathForCell(cell); | ||||||
| invalidatedPaths.Add(path); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (invalidatedPaths != null) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or is null return here |
||||||
| { | ||||||
| var layoutInvalidationContext = new UICollectionViewFlowLayoutInvalidationContext(); | ||||||
| layoutInvalidationContext.InvalidateItems(invalidatedPaths.ToArray()); | ||||||
| CollectionView.CollectionViewLayout.InvalidateLayout(layoutInvalidationContext); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| void MauiCollectionView.ICustomMauiCollectionViewDelegate.MovedToWindow(UIView view) | ||||||
| { | ||||||
|
|
@@ -284,7 +315,7 @@ void ConstrainItemsToBounds() | |||||
| { | ||||||
| var contentBounds = CollectionView.AdjustedContentInset.InsetRect(CollectionView.Bounds); | ||||||
| var constrainedSize = contentBounds.Size; | ||||||
| ItemsViewLayout.UpdateConstraints(constrainedSize); | ||||||
| ItemsViewLayout.UpdateConstraints(constrainedSize, !_laidOut); | ||||||
| } | ||||||
|
|
||||||
| void EnsureLayoutInitialized() | ||||||
|
|
@@ -373,7 +404,6 @@ protected virtual void UpdateDefaultCell(DefaultCell cell, NSIndexPath indexPath | |||||
|
|
||||||
| protected virtual void UpdateTemplatedCell(TemplatedCell cell, NSIndexPath indexPath) | ||||||
| { | ||||||
| cell.ContentSizeChanged -= CellContentSizeChanged; | ||||||
| cell.LayoutAttributesChanged -= CellLayoutAttributesChanged; | ||||||
|
|
||||||
| var bindingContext = ItemsSource[indexPath]; | ||||||
|
|
@@ -382,7 +412,6 @@ protected virtual void UpdateTemplatedCell(TemplatedCell cell, NSIndexPath index | |||||
| if (_measurementCells != null && _measurementCells.TryGetValue(bindingContext, out TemplatedCell measurementCell)) | ||||||
| { | ||||||
| _measurementCells.Remove(bindingContext); | ||||||
| measurementCell.ContentSizeChanged -= CellContentSizeChanged; | ||||||
| measurementCell.LayoutAttributesChanged -= CellLayoutAttributesChanged; | ||||||
| cell.UseContent(measurementCell); | ||||||
| } | ||||||
|
|
@@ -391,7 +420,6 @@ protected virtual void UpdateTemplatedCell(TemplatedCell cell, NSIndexPath index | |||||
| cell.Bind(ItemsView.ItemTemplate, ItemsSource[indexPath], ItemsView); | ||||||
| } | ||||||
|
|
||||||
| cell.ContentSizeChanged += CellContentSizeChanged; | ||||||
| cell.LayoutAttributesChanged += CellLayoutAttributesChanged; | ||||||
|
|
||||||
| ItemsViewLayout.PrepareCellForLayout(cell); | ||||||
|
|
@@ -407,29 +435,6 @@ protected object GetItemAtIndex(NSIndexPath index) | |||||
| return ItemsSource[index]; | ||||||
| } | ||||||
|
|
||||||
| [UnconditionalSuppressMessage("Memory", "MEM0003", Justification = "Proven safe in test: CollectionViewTests.ItemsSourceDoesNotLeak")] | ||||||
| void CellContentSizeChanged(object sender, EventArgs e) | ||||||
| { | ||||||
| if (_disposed) | ||||||
| return; | ||||||
|
|
||||||
| if (!(sender is TemplatedCell cell)) | ||||||
| { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| var visibleCells = CollectionView.VisibleCells; | ||||||
|
|
||||||
| for (int n = 0; n < visibleCells.Length; n++) | ||||||
| { | ||||||
| if (cell == visibleCells[n]) | ||||||
| { | ||||||
| ItemsViewLayout?.InvalidateLayout(); | ||||||
| return; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| [UnconditionalSuppressMessage("Memory", "MEM0003", Justification = "Proven safe in test: CollectionViewTests.ItemsSourceDoesNotLeak")] | ||||||
| void CellLayoutAttributesChanged(object sender, LayoutAttributesChangedEventArgs args) | ||||||
| { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,9 +104,9 @@ protected virtual void HandlePropertyChanged(PropertyChangedEventArgs propertyCh | |
| } | ||
| } | ||
|
|
||
| internal virtual bool UpdateConstraints(CGSize size) | ||
| internal virtual bool UpdateConstraints(CGSize size, bool forceUpdate = false) | ||
| { | ||
| if (size.IsCloseTo(_currentSize)) | ||
| if (size.IsCloseTo(_currentSize) && !forceUpdate) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When used as |
||
| { | ||
| return false; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create a new issue for this, maybe for net10.0