Skip to content

Pre-compute Grid star values when possible#12912

Merged
mattleibow merged 1 commit intomainfrom
fix-12103
Jan 28, 2023
Merged

Pre-compute Grid star values when possible#12912
mattleibow merged 1 commit intomainfrom
fix-12103

Conversation

@hartez
Copy link
Contributor

@hartez hartez commented Jan 25, 2023

Description of Change

The current Grid measurement algorithm measures cells in Auto rows/columns more than is strictly necessary; a side-effect of the extra measure at the final Auto row/column size on Windows is that the underlying platform is confused about control size changes and does not report them correctly to the containing controls (thus, the non-update issue reported in #12103).

One of the reasons for the extra measurements stems from updating the sizes of Star rows/columns after all of the Auto values have been determined. The algorithm assumes the need for this any time a cell intersects an Auto row or column in any direction. However, in many cases either the row or column is entirely pre-computable, because the Grid is constrained in that direction and there are no Auto values. In those situations, we can compute the value for Star in that direction up front, and avoid multiple measurements. Pre-computing also means that we have final values for width/height to use with controls where the measurements are interdependent (e.g., with Images where the aspect ratio affects the measurement results, like in #7388).

Pre-computing also means that we can avoid the duplicate measure scenarios originally addressed by #12660, so we no longer need to cache the first measurement values.

These changes pre-compute all cell sizes where possible and avoid secondary measures of Auto values. They also add a benchmark for the GridLayoutManager so we can verify that future fixes don't adversely impact performance.

The benchmark is limited to the performance of the GridLayoutManager specifically; it does not speak to the performance of the Grid on any actual platform.

Benchmarks before these changes:

Method Mean Error StdDev Gen0 Gen1 Allocated
AllAbsolute 96.99 us 1.930 us 5.381 us 3.1738 1.0986 22 KB
AllStars 96.17 us 1.159 us 1.027 us 3.5400 1.2207 22.25 KB
AllAuto 88.05 us 1.235 us 1.095 us 3.1738 1.0986 20 KB

After these changes:

Method Mean Error StdDev Gen0 Gen1 Allocated
AllAbsolute 90.58 us 1.667 us 1.478 us 3.1738 1.0986 19.93 KB
AllStars 100.66 us 1.491 us 1.321 us 3.5400 1.2207 22.18 KB
AllAuto 92.77 us 1.788 us 1.987 us 3.1738 1.0986 19.93 KB

Issues Fixed

Fixes #12103
Fixes #7388
Fixes #7363
Fixes #11453
Fixes #10799
Fixes #9078
Fixes #5363

@Eilon Eilon added the area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label Jan 26, 2023
@mattleibow mattleibow merged commit 83c3ac9 into main Jan 28, 2023
@mattleibow mattleibow deleted the fix-12103 branch January 28, 2023 06:26
@ederbond
Copy link
Contributor

ederbond commented Feb 1, 2023

Please guys, can we have these Layout measuring and positioning PR's landing on the next Service Release of .NET 7 rather than .NET 8? I'm still suffering a lot to get my layouts measured and positioned correctly on .NET MAUI.
@hartez @mattleibow @davidortinau @jsuarezruiz

@hartez hartez added the backport/suggested The PR author or issue review has suggested that the change should be backported. label Feb 15, 2023
@hartez hartez added the backport/approved After some discussion or review, this PR or change was approved to be backported. label Feb 16, 2023
@hartez
Copy link
Contributor Author

hartez commented Feb 16, 2023

/backport to net7.0

@github-actions
Copy link
Contributor

Started backporting to net7.0: https://github.com/dotnet/maui/actions/runs/4195741934

@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.1.7762 Look for this fix in 8.0.0-preview.1.7762! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter backport/approved After some discussion or review, this PR or change was approved to be backported. backport/suggested The PR author or issue review has suggested that the change should be backported. fixed-in-8.0.0-preview.1.7762 Look for this fix in 8.0.0-preview.1.7762!

Projects

None yet

5 participants