Fix iOS infinite layout invalidation loop from bit-exact Frame equality (#35142)#35144
Fix iOS infinite layout invalidation loop from bit-exact Frame equality (#35142)#35144Qythyx wants to merge 1 commit intodotnet:mainfrom
Conversation
VisualElement.Frame's setter uses Rect's bit-exact equality (Rect.Equals
on doubles) to decide whether to enter UpdateBoundsComponents. On iOS,
ULP-level non-determinism from UILabel.SizeThatFits / CoreText
propagates through VerticalStackLayout accumulation and Grid("*","Auto")
star-row arithmetic into a child's Frame, producing a Rect that differs
from the cached one by ~10-22 ULP. Bit-exact equality treats the rects
as different, fires Width/Height PropertyChanged + SizeChanged, layout
invalidates, UIKit reschedules layoutSubviews, and the cycle repeats
without converging. The UI thread becomes permanently stuck.
Add Rect.EqualsApproximately(other, epsilon) and use it in the Frame
setter with epsilon = 1e-9 - well above ~10^-13 ULP at typical layout
magnitudes and well below sub-pixel resolution on any current iOS
device. Mirrors the precedent set by SafeAreaPadding.EqualsAtPixelLevel
(introduced for issues dotnet#32586 and dotnet#33934).
Adds two test files:
- VisualElementTests.FrameAssignmentIgnoresSubPixelDifferences:
Regression test using the actual border heights captured in the repro
trace (556.00000063578295 vs 556.00000063578273); fails on
pre-fix main, passes after the change.
- RectTests: focused coverage of Rect.EqualsApproximately.
Fixes dotnet#35142
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35144Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35144" |
|
Hey there @@Qythyx! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
🤖 AI Summary
📊 Review Session —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🧪 RectTests RectTests |
✅ FAIL — 9s | ✅ PASS — 5s |
🧪 VisualElementTests VisualElementTests |
✅ FAIL — 28s | ✅ PASS — 22s |
🔴 Without fix — 🧪 RectTests: FAIL ✅ · 9s
Determining projects to restore...
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 743 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/samples/GraphicsTester.Portable/GraphicsTester.Portable.csproj (in 892 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Text.Markdig/Graphics.Text.Markdig.csproj (in 892 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj (in 1.63 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics.Skia/Graphics.Skia.csproj (in 1.81 sec).
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics.Skia -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics.Skia/Debug/net10.0/Microsoft.Maui.Graphics.Skia.dll
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/netstandard2.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics.Text.Markdig -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics.Text.Markdig/Debug/netstandard2.0/Microsoft.Maui.Graphics.Text.Markdig.dll
GraphicsTester.Portable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/GraphicsTester.Portable/Debug/net10.0/GraphicsTester.Portable.dll
/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/RectTests.cs(11,21): error CS1061: 'Rect' does not contain a definition for 'EqualsApproximately' and no accessible extension method 'EqualsApproximately' accepting a first argument of type 'Rect' could be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/RectTests.cs(23,18): error CS1061: 'Rect' does not contain a definition for 'EqualsApproximately' and no accessible extension method 'EqualsApproximately' accepting a first argument of type 'Rect' could be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/RectTests.cs(32,22): error CS1061: 'Rect' does not contain a definition for 'EqualsApproximately' and no accessible extension method 'EqualsApproximately' accepting a first argument of type 'Rect' could be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/RectTests.cs(33,22): error CS1061: 'Rect' does not contain a definition for 'EqualsApproximately' and no accessible extension method 'EqualsApproximately' accepting a first argument of type 'Rect' could be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/RectTests.cs(34,22): error CS1061: 'Rect' does not contain a definition for 'EqualsApproximately' and no accessible extension method 'EqualsApproximately' accepting a first argument of type 'Rect' could be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/RectTests.cs(35,22): error CS1061: 'Rect' does not contain a definition for 'EqualsApproximately' and no accessible extension method 'EqualsApproximately' accepting a first argument of type 'Rect' could be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/RectTests.cs(44,21): error CS1061: 'Rect' does not contain a definition for 'EqualsApproximately' and no accessible extension method 'EqualsApproximately' accepting a first argument of type 'Rect' could be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Graphics/tests/Graphics.Tests/Graphics.Tests.csproj]
🟢 With fix — 🧪 RectTests: PASS ✅ · 5s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics.Skia -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics.Skia/Debug/net10.0/Microsoft.Maui.Graphics.Skia.dll
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/netstandard2.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics.Text.Markdig -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics.Text.Markdig/Debug/netstandard2.0/Microsoft.Maui.Graphics.Text.Markdig.dll
GraphicsTester.Portable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/GraphicsTester.Portable/Debug/net10.0/GraphicsTester.Portable.dll
Graphics.Tests -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics.Tests/Debug/net10.0/Graphics.Tests.dll
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics.Tests/Debug/net10.0/Graphics.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (arm64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.04] Discovering: Graphics.Tests
[xUnit.net 00:00:00.10] Discovered: Graphics.Tests
[xUnit.net 00:00:00.10] Starting: Graphics.Tests
[xUnit.net 00:00:00.13] Finished: Graphics.Tests
Passed Microsoft.Maui.Graphics.Tests.RectTests.EqualsApproximatelyTreatsHalfEpsilonDifferenceAsEqual [4 ms]
Passed Microsoft.Maui.Graphics.Tests.RectTests.EqualsApproximatelyReturnsTrueForIdenticalRects [< 1 ms]
Passed Microsoft.Maui.Graphics.Tests.RectTests.EqualsApproximatelyAbsorbsUlpDifferences [< 1 ms]
Passed Microsoft.Maui.Graphics.Tests.RectTests.EqualsApproximatelyReturnsFalseWhenAnyComponentExceedsEpsilon [< 1 ms]
Test Run Successful.
Total tests: 4
Passed: 4
Total time: 0.4644 Seconds
🔴 Without fix — 🧪 VisualElementTests: FAIL ✅ · 28s
Determining projects to restore...
Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/TestUtils/TestUtils.csproj (in 399 ms).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj (in 4.65 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 4.72 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 5.05 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 5.05 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/maps/src/Maps.csproj (in 5.05 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 5.05 sec).
Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 5.07 sec).
2 of 10 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
TestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
Controls.Core.UnitTests -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (arm64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.10] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.67] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.68] Starting: Microsoft.Maui.Controls.Core.UnitTests
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.RadialGradientBrush), defaultCtor: True) [24 ms]
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.ImmutableBrush), defaultCtor: False) [10 ms]
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.SolidColorBrush), defaultCtor: False) [9 ms]
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.LinearGradientBrush), defaultCtor: True) [9 ms]
Passed HandlerDoesPropagateWidthChangesWhenUpdatedDuringSizedChanged [34 ms]
Passed If HeightRequest has been set and is reset to -1, the Core Height should return to being Unset [2 ms]
Passed ShadowDoesNotLeak [34 ms]
Passed RectangleGeometrySubscribed [9 ms]
Passed BindingContextPropagatesToBackground [2 ms]
[xUnit.net 00:00:00.91] FrameAssignmentIgnoresSubPixelDifferences [FAIL]
[xUnit.net 00:00:00.91] Assert.Equal() Failure: Values differ
[xUnit.net 00:00:00.91] Expected: 0
[xUnit.net 00:00:00.91] Actual: 1
[xUnit.net 00:00:00.91] Stack Trace:
[xUnit.net 00:00:00.91] /_/src/Controls/tests/Core.UnitTests/VisualElementTests.cs(345,0): at Microsoft.Maui.Controls.Core.UnitTests.VisualElementTests.FrameAssignmentIgnoresSubPixelDifferences()
[xUnit.net 00:00:00.91] at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
[xUnit.net 00:00:00.91] at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Passed WidthAndHeightRequestPropagateToHandler [1 ms]
Passed GradientBrushSubscribed [11 ms]
Passed ShadowSubscribed [9 ms]
Passed HandlerDoesntPropagateWidthChangesDuringBatchUpdates [< 1 ms]
Passed If WidthRequest has been set and is reset to -1, the Core Width should return to being Unset [< 1 ms]
Passed FocusedElementGetsFocusedVisualState [2 ms]
Failed FrameAssignmentIgnoresSubPixelDifferences [1 ms]
Error Message:
Assert.Equal() Failure: Values differ
Expected: 0
Actual: 1
Stack Trace:
at Microsoft.Maui.Controls.Core.UnitTests.VisualElementTests.FrameAssignmentIgnoresSubPixelDifferences() in /_/src/Controls/tests/Core.UnitTests/VisualElementTests.cs:line 345
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Passed ClipDoesNotLeak(type: typeof(Microsoft.Maui.Controls.Shapes.RectangleGeometry)) [10 ms]
Passed ClipDoesNotLeak(type: typeof(Microsoft.Maui.Controls.Shapes.EllipseGeometry)) [13 ms]
[xUnit.net 00:00:00.91] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed ContainerChangedFiresWhenMapContainerIsCalled [1 ms]
Test Run Failed.
Total tests: 19
Passed: 18
Failed: 1
Total time: 1.1726 Seconds
🟢 With fix — 🧪 VisualElementTests: PASS ✅ · 22s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
##vso[build.updatebuildnumber]10.0.70-ci+azdo.13944879
Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0/Microsoft.Maui.Controls.Maps.dll
Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0/Microsoft.Maui.Controls.Xaml.dll
TestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/TestUtils/Debug/netstandard2.0/Microsoft.Maui.TestUtils.dll
Controls.Core.UnitTests -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (arm64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.07] Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.58] Discovered: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.59] Starting: Microsoft.Maui.Controls.Core.UnitTests
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.RadialGradientBrush), defaultCtor: True) [20 ms]
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.ImmutableBrush), defaultCtor: False) [7 ms]
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.SolidColorBrush), defaultCtor: False) [7 ms]
Passed BackgroundDoesNotLeak(type: typeof(Microsoft.Maui.Controls.LinearGradientBrush), defaultCtor: True) [6 ms]
Passed HandlerDoesPropagateWidthChangesWhenUpdatedDuringSizedChanged [30 ms]
Passed If HeightRequest has been set and is reset to -1, the Core Height should return to being Unset [3 ms]
Passed ShadowDoesNotLeak [44 ms]
Passed RectangleGeometrySubscribed [8 ms]
Passed BindingContextPropagatesToBackground [2 ms]
Passed WidthAndHeightRequestPropagateToHandler [1 ms]
Passed GradientBrushSubscribed [10 ms]
Passed ShadowSubscribed [9 ms]
Passed HandlerDoesntPropagateWidthChangesDuringBatchUpdates [< 1 ms]
Passed If WidthRequest has been set and is reset to -1, the Core Width should return to being Unset [< 1 ms]
Passed FocusedElementGetsFocusedVisualState [3 ms]
Passed FrameAssignmentIgnoresSubPixelDifferences [< 1 ms]
Passed ClipDoesNotLeak(type: typeof(Microsoft.Maui.Controls.Shapes.RectangleGeometry)) [17 ms]
Passed ClipDoesNotLeak(type: typeof(Microsoft.Maui.Controls.Shapes.EllipseGeometry)) [9 ms]
[xUnit.net 00:00:00.80] Finished: Microsoft.Maui.Controls.Core.UnitTests
Passed ContainerChangedFiresWhenMapContainerIsCalled [< 1 ms]
Test Run Successful.
Total tests: 19
Passed: 19
Total time: 1.0981 Seconds
📁 Fix files reverted (4 files)
src/Controls/src/Core/VisualElement/VisualElement.cssrc/Graphics/src/Graphics/PublicAPI/net/PublicAPI.Unshipped.txtsrc/Graphics/src/Graphics/PublicAPI/netstandard/PublicAPI.Unshipped.txtsrc/Graphics/src/Graphics/Rect.cs
🧪 UI Tests — Category Detection
No UI test categories detected for this PR.
🔍 Pre-Flight — Context & Validation
Issue: #35142 - [iOS] Grid("*,Auto") over VerticalStackLayout enters non-converging measurement 2-cycle on iPhone 16e
PR: #35144 - Fix iOS infinite layout invalidation loop from bit-exact Frame equality (#35142)
Platforms Affected: iOS (confirmed; does not reproduce on iPhone 17 Pro with wider screen)
Files Changed: 3 implementation, 3 test
Key Findings
- The root cause is bit-exact
Rectequality inVisualElement.Framesetter — ULP-level floating-point differences from iOSUILabel.SizeThatFitspropagate throughVerticalStackLayout→Grid("*,Auto")star-row arithmetic, producing a 2-cycle that never converges - The fix adds
Rect.EqualsApproximately(other, epsilon)with epsilon=1e-9 (well above ULP drift ~10⁻¹³, well below sub-pixel resolution ~0.333pt on 3× displays) - The PR is community contribution from the issue reporter (same person), showing deep analysis
- Precedent established by
SafeAreaPadding.EqualsAtPixelLevel(issues Layout issue using TranslateToAsync causes infinite property changed cycle on iOS #32586, [iOS] TranslateToAsync causes spurious SizeChanged events after animation completion, triggering infinite layout loops #33934) for the same class of problem - Gate: ✅ PASSED — both RectTests and VisualElementTests fail on main, pass with fix
UpdateBoundsComponentsis called only fromFrame.set— the redundant bit-exact inner guard at line 1840 is now effectively unreachable but harmless
Code Review Summary
Verdict: LGTM
Confidence: high
Errors: 0 | Warnings: 0 | Suggestions: 3
Key code review findings:
- 💡
UpdateBoundsComponents(line 1840) has a now-unreachable_frame == boundsguard — dead code, harmless but misleading - 💡
EqualsApproximatelypublic API has no documented/enforced precondition forepsilon >= 0(negative epsilon silently makes all comparisons false) - 💡
RectTests.csmissing exact-boundary test case (diff == epsilonshould return true per<=semantics)
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35144 | Add Rect.EqualsApproximately(epsilon=1e-9) and use it in VisualElement.Frame setter instead of bit-exact equality |
✅ PASSED (Gate) | Rect.cs, VisualElement.cs, PublicAPI files + tests |
Original PR |
Impacted UI Test Categories
Layout — Grid layout measurement/convergence is directly affected
🔬 Code Review — Deep Analysis
Code Review — PR #35144
Independent Assessment
What this changes:
Rect gains a new public method EqualsApproximately(Rect other, double epsilon) that compares each component with Math.Abs(a - b) <= epsilon. The VisualElement.Frame setter's existing bit-exact equality guard (_frame == value) is replaced with _frame.EqualsApproximately(value, 1e-9). Two test files are added: RectTests.cs (focused on the new method) and a new test case in VisualElementTests.cs (using real values from a repro trace). Both PublicAPI.Unshipped.txt files are updated correctly.
Inferred motivation:
iOS CoreText / UILabel.SizeThatFits produces values that differ from one layout pass to the next by ~10–22 ULP (order of magnitude ~1e-12). Because Rect == is bit-exact, these "same" rectangles are treated as different, firing Width/Height PropertyChanged and SizeChanged → layout invalidation → UIKit reschedules layoutSubviews → cycle never converges. The fix absorbs these ULP-level differences so no spurious events fire.
Reconciliation with PR Narrative
Author claims: ULP-level non-determinism from UILabel.SizeThatFits feeds through VerticalStackLayout/Grid("*","Auto") arithmetic into a child's Frame, creating a non-converging 2-cycle. Fix mirrors SafeAreaPadding.EqualsAtPixelLevel precedent from issues #32586 and #33934.
Agreement: Root-cause analysis matches. The SafeAreaPadding.EqualsAtPixelLevel comparison is apt. Note that EqualsAtPixelLevel is dynamic (reads UIScreen.MainScreen.Scale at call time), while EqualsApproximately uses a static absolute epsilon — a deliberate simplification since Rect lives in Microsoft.Maui.Graphics (cross-platform, no UIKit). ✅ Correct tradeoff.
Findings
💡 Suggestion — Dead exact-equality guard inside UpdateBoundsComponents
Location: src/Controls/src/Core/VisualElement/VisualElement.cs:1840
void UpdateBoundsComponents(Rect bounds)
{
if (_frame == bounds) // ← now unreachable dead code
return;UpdateBoundsComponents has exactly one caller: the Frame setter. The setter's EqualsApproximately check is a strict superset of exact equality — if _frame == value were true, EqualsApproximately would have already returned early. This guard can never be true in practice. Not a bug (dead code is harmless here), but misleading. Consider removing it or replacing with Debug.Assert(_frame != bounds).
💡 Suggestion — EqualsApproximately lacks epsilon precondition (public API)
Location: src/Graphics/src/Graphics/Rect.cs:87
A negative epsilon silently causes all comparisons to return false (since Math.Abs(x) >= 0 > epsilon always). The method is public API. Adding either a <param> doc note (Must be non-negative) or an ArgumentOutOfRangeException guard for epsilon < 0 would make the contract explicit.
💡 Suggestion — Test coverage gap at exact epsilon boundary
Location: src/Graphics/tests/Graphics.Tests/RectTests.cs
The tests verify 0.5 * epsilon returns true and 2 * epsilon returns false, but the exact-boundary case (diff == epsilon) is untested. Since the implementation uses <=, that case should return true. Adding one assertion like:
Assert.True(rect.EqualsApproximately(new Rect(0, 0, 100, 100 + epsilon), epsilon));would nail down the contract at the boundary.
Blast Radius
Scope: VisualElement.Frame setter in Microsoft.Maui.Controls.Core. Every view in MAUI goes through this setter during layout. The change only affects the early-return condition — it widens what is considered "equal" from exact to approximate. On non-iOS platforms (Android/Windows), frame values are typically pixel-aligned and will never produce sub-1e-9 ULP noise, so the behavior is identical. On iOS the change suppresses spurious layout cycles. No other call sites for UpdateBoundsComponents exist.
Failure mode probes:
- Could epsilon suppress real frame updates?
1e-9pt is sub-nanometer. No layout engine produces intentional deltas that small. Not a real concern. - Cross-platform safety? Windows/Android produce pixel-aligned values. Safe.
- Negative epsilon edge case? API misuse would silently fail; already noted in suggestions above.
Verdict
LGTM (confidence: high)
The fix is correct, precisely scoped, and directly mirrors established MAUI precedent (SafeAreaPadding.EqualsAtPixelLevel). The epsilon value of 1e-9 is safe on all platforms. The new Rect.EqualsApproximately method is a clean general-purpose addition with minimal, well-documented public API surface. Three minor suggestions above, none blocking merge.
Errors: 0 | Warnings: 0 | Suggestions: 3
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Round-to-Canonical in UpdateBoundsComponents — snap frame components with Math.Round(value, 10) before storing, eliminating ULP noise at source; keeps bit-exact Frame comparison |
✅ PASS | VisualElement.cs only |
No new public API; normalization vs. tolerance strategy |
| 2 | try-fix (claude-sonnet-4.6) | Sticky Width/Height Coercion in UpdateBoundsComponents — preserve current Width/Height value if ` | new-current | < 1e-9`; X/Y updated at full precision | ✅ PASS |
| 3 | try-fix (gpt-5.3-codex) | Coerce Width/Height near-equality in bindable property coerce layer — ULP-scale deltas keep current value instead of propagating change notifications | ✅ PASS | VisualElement.cs, Rect.cs, PublicAPI files |
Targets event source (W/H change) not frame comparison |
| 4 | try-fix (gpt-5.4, gemini unavailable) | Quantize entire Frame to 0.001pt grid before compare/store — collapses ULP noise to deterministic discrete state | ✅ PASS | VisualElement.cs, Rect.cs, PublicAPI files |
Grid quantization is 1,000,000× coarser than PR's epsilon |
| PR | PR #35144 | Add Rect.EqualsApproximately(epsilon=1e-9) public method; use in Frame setter instead of bit-exact equality |
✅ PASSED (Gate) | Rect.cs, VisualElement.cs, PublicAPI files |
Original PR — new general-purpose API, mirrors SafeAreaPadding precedent |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | NO NEW IDEAS |
Exhausted: Yes
Selected Fix: PR's fix — Reason:
- Only fix that adds a reusable general-purpose public API (
Rect.EqualsApproximately) that any MAUI code can use - Fix is at the right architectural layer (Frame setter gate) — clean, minimal blast radius
- Best epsilon choice: 1e-9 is well-justified (above ULP noise ~1e-12, below sub-pixel ~0.333pt); alternative approaches use coarser tolerances or rounded-off values
- Mirrors established precedent:
SafeAreaPadding.EqualsAtPixelLevel(issues Layout issue using TranslateToAsync causes infinite property changed cycle on iOS #32586, [iOS] TranslateToAsync causes spurious SizeChanged events after animation completion, triggering infinite layout loops #33934) - Best documented: XML docs on new method, thorough PR description, comments at usage site
- Alternatives (Attempt 1: Math.Round; Attempt 2: sticky coercion; Attempt 3: bindable layer; Attempt 4: 0.001pt quantize) all work but are more opaque about intent, affect X/Y precision, or modify noisier layers
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #35142 root-cause confirmed; 3 impl files, 3 test files classified |
| Code Review | LGTM (high) | 0 errors, 0 warnings, 3 suggestions (all non-blocking) |
| Gate | ✅ PASSED | iOS — RectTests & VisualElementTests fail without fix, pass with fix |
| Try-Fix | ✅ COMPLETE | 4 attempts, 4 passing; PR's fix selected as best |
| Report | ✅ COMPLETE |
Code Review Impact on Try-Fix
Code review found no errors or warnings — only 3 minor suggestions (dead code guard, epsilon precondition doc, boundary test gap). None influenced the fix approach itself. The LGTM verdict with high confidence confirmed the PR's design is sound, so Try-Fix models were directed to explore genuinely different architectural layers (normalization, sticky coercion, bindable layer, quantization) rather than variations on the same approach. All 4 alternatives passed the regression tests, validating that multiple fix strategies are viable — but the PR's approach remains the clearest and most reusable.
Summary
PR #35144 fixes an iOS-specific infinite layout loop caused by ULP-level non-determinism in UILabel.SizeThatFits propagating through MAUI's layout pipeline into VisualElement.Frame. The fix adds Rect.EqualsApproximately(epsilon) and uses it in the Frame setter to absorb sub-pixel floating-point noise before firing PropertyChanged/SizeChanged events. The fix is correct, well-tested, well-documented, and follows established MAUI precedent (SafeAreaPadding.EqualsAtPixelLevel).
Root Cause
On iOS, UILabel.SizeThatFits returns ULP-non-deterministic values (~1e-12 variation) across consecutive layout passes. This variation propagates through VerticalStackLayout accumulation and Grid("*","Auto") star-row arithmetic, reaching a child element's Frame setter with values differing by ~10–22 ULP (~1e-12). The Frame setter's bit-exact equality (_frame == value) treats these as different, fires Width/Height PropertyChanged + SizeChanged → layout invalidates → UIKit reschedules layoutSubviews → the 2-cycle repeats indefinitely.
Fix Quality
The fix is high quality:
- Epsilon choice (
1e-9) is correctly calibrated: orders of magnitude above the observed ULP noise (~1e-12) and orders of magnitude below sub-pixel resolution (~0.333pt on 3× displays). No real layout updates will be suppressed. - Scope:
Rect.EqualsApproximatelyis added as a new non-breaking public method;Rect.Equals/==are unchanged. TheFramesetter change is a one-line replacement of an equality check. - Architecture: Fix is at the correct layer (Frame setter gate), preventing spurious
UpdateBoundsComponentsentry entirely. - Tests: Two test files —
RectTests.cs(4 focused tests of the new method) andVisualElementTests.FrameAssignmentIgnoresSubPixelDifferences(using actual repro trace values) — provide strong coverage. - Three minor suggestions from code review: (1) remove now-dead
if (_frame == bounds)guard inUpdateBoundsComponents; (2) add epsilon precondition doc/guard toEqualsApproximately; (3) add exact-boundary test case (diff == epsilon). None are blocking.
MauiBot
added
s/agent-approved
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Agents didn't know what to do when no CI builds existed (common for community PRs) or when devicetests/uitests weren't triggered. Added note about /azp run commands and explicit pipeline triggers. Discovered via multi-agent testing against PRs #35144 and #35150. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Expert Code Review completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Expert Code Review — PR #35144
Methodology: 3 independent reviewers with adversarial consensus (findings only included when ≥2/3 agree; disputed 1/3 findings verified via targeted follow-up).
Summary
The core fix — replacing bit-exact Rect equality with epsilon-based approximate comparison in the VisualElement.Frame setter — is sound, well-motivated, and consistent with existing MAUI patterns (mirrors SafeAreaPadding.EqualsAtPixelLevel). The epsilon value (1e-9) is well-justified: ~330 million times smaller than a Retina pixel, yet large enough to absorb the ~22 ULP non-determinism observed in the iOS layout trace.
Tests are thorough, covering identical rects, ULP-level differences from the actual repro trace, per-component boundary violations, and the Frame setter's event suppression behavior.
Findings
| # | Severity | File | Finding | Consensus |
|---|---|---|---|---|
| 1 | 🔴 CRITICAL | PublicAPI.Unshipped.txt (platform TFMs) |
Missing EqualsApproximately entry in 6 platform-specific PublicAPI files — CI will fail with RS0016 |
3/3 reviewers (after follow-up) |
| 2 | 🟡 MODERATE | VisualElement.cs |
UpdateBoundsComponents internal bit-exact guard is now dead code — maintenance trap for future callers |
2/3 reviewers |
| 3 | 🟢 MINOR | Rect.cs |
EqualsApproximately NaN semantics diverge from Equals (NaN components → always false) — should be documented |
2/3 reviewers |
| 4 | 🟢 MINOR | Rect.cs |
No validation on epsilon parameter — negative/NaN epsilon silently returns false for all inputs |
2/3 reviewers |
Discarded findings (1/3 only, not confirmed by follow-up): cross-platform epsilon comment suggestion, missing boundary test suggestion.
Blocking Issue
Finding #1 is the only blocker. The 6 platform-specific PublicAPI.Unshipped.txt files under src/Graphics/src/Graphics/PublicAPI/ (net-ios, net-android, net-maccatalyst, net-windows, net-tizen, net-macos) need the same entry added:
Microsoft.Maui.Graphics.Rect.EqualsApproximately(Microsoft.Maui.Graphics.Rect other, double epsilon) -> bool
Non-Blocking Suggestions
Findings #2–#4 are documentation/comment improvements that can be addressed in this PR or follow-up:
- Add a comment in
UpdateBoundsComponentsnoting the Frame setter is the sole caller - Add
<remarks>toEqualsApproximatelydocumenting NaN behavior and epsilon requirements
Test Coverage Assessment
✅ Good coverage. The PR includes:
RectTests.cs— 4 tests covering the newEqualsApproximatelyAPI (identity, ULP absorption, per-component rejection, half-epsilon boundary)VisualElementTests.cs— Regression test using actual values from the iOS repro trace, verifying no spuriousSizeChanged/PropertyChangedevents
No device/UI tests are included, but the fix is in shared cross-platform code testable via unit tests, which is appropriate.
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #35144
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - #35144
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review for issue #35144 · ● 14M
| @@ -1 +1,2 @@ | |||
| #nullable enable | |||
| Microsoft.Maui.Graphics.Rect.EqualsApproximately(Microsoft.Maui.Graphics.Rect other, double epsilon) -> bool | |||
There was a problem hiding this comment.
🔴 CRITICAL — Missing PublicAPI entries for platform-specific TFMs (3/3 reviewers)
Rect.EqualsApproximately is defined in shared Rect.cs and compiles for all TFMs. This PR adds the entry to net/ and netstandard/ only, but PublicAPI.targets resolves platform TFMs (e.g., net10.0-ios) to platform-specific directories (net-ios/PublicAPI.Unshipped.txt, etc.). CI runs in Validate mode and will emit RS0016 for every platform build.
Fix: Add Microsoft.Maui.Graphics.Rect.EqualsApproximately(Microsoft.Maui.Graphics.Rect other, double epsilon) -> bool to all 6 platform-specific PublicAPI.Unshipped.txt files:
net-iosnet-androidnet-maccatalystnet-windowsnet-tizennet-macos
| /// </summary> | ||
| /// <param name="other">The rectangle to compare against.</param> | ||
| /// <param name="epsilon">The maximum absolute difference, per component, that is treated as equal.</param> | ||
| public bool EqualsApproximately(Rect other, double epsilon) |
There was a problem hiding this comment.
🟢 MINOR — NaN semantics diverge from Rect.Equals (2/3 reviewers)
Equals(Rect) uses double.Equals(double) which returns true for NaN.Equals(NaN). In contrast, EqualsApproximately uses Math.Abs(X - other.X) <= epsilon, where NaN - NaN = NaN and NaN <= epsilon = false. So a NaN-component rect compares equal to itself with Equals but not with EqualsApproximately.
In the Frame setter context: if a platform produced a NaN frame, the old code would no-op (dedup), but the new code would call UpdateBoundsComponents and fire events on every pass.
Not a practical risk (NaN frames indicate an upstream bug), but worth documenting.
Suggestion: Add a <remarks> to the XML doc: "Unlike Equals, this method returns false when any component is NaN."
| set | ||
| { | ||
| if (_frame == value) | ||
| if (_frame.EqualsApproximately(value, FrameEqualityEpsilon)) |
There was a problem hiding this comment.
🟡 MODERATE — UpdateBoundsComponents internal guard is now dead code (2/3 reviewers)
UpdateBoundsComponents (called on line 1895) has an internal if (_frame == bounds) return; bit-exact guard. With this PR, the Frame setter now exits early for any difference ≤ 1e-9, so UpdateBoundsComponents is only called when the difference exceeds the epsilon — making the internal bit-exact check guaranteed false (dead code).
This isn't a correctness bug today (confirmed UpdateBoundsComponents is only called from the Frame setter), but it's a maintenance trap: a future caller bypassing the Frame setter would re-expose the original oscillation.
Suggestion: Add a brief comment inside UpdateBoundsComponents noting that the Frame setter is the sole caller and the primary deduplication path is the approximate check above.
| /// <param name="epsilon">The maximum absolute difference, per component, that is treated as equal.</param> | ||
| public bool EqualsApproximately(Rect other, double epsilon) | ||
| { | ||
| return Math.Abs(X - other.X) <= epsilon |
There was a problem hiding this comment.
🟢 MINOR — No epsilon validation on public API (2/3 reviewers)
epsilon is an unrestricted double parameter. A negative epsilon makes Math.Abs(...) <= epsilon always false (since Math.Abs ≥ 0), so the method silently returns false for all inputs — including identical rects. A NaN epsilon has the same effect.
The internal caller uses a hardcoded const 1e-9 (safe), but this is a new public API surface.
Suggestion: Either add an ArgumentOutOfRangeException guard for epsilon < 0, or (more likely preferred on a struct hot-path) document the requirement: "epsilon must be non-negative; a negative value causes the method to always return false."
|
Note, 10.0.51 fixes this, so my fix is probably not necessary. I had actually updated to .51 a while ago, but it introduced new bugs around SafeAreaInsets so I had reverted back to .41. Then I found this bug and spent a few days debugging and reproing it without trying to see if .51 fixed it. |
|
Alright! @Qythyx Thanks for letting me know <3 |
- Community PR: replaced PR #35144 (got closed during eval) with PR #34710 (older, stable state). Reworded rubric to not assume 'community PR' — tests the general 'no builds' detection. - Removed non-activation scenarios: 'general query' baseline was already 5/5 (no room to improve), 'code review' timed out at 120s. These dragged the improvement score negative. The skill's boundary is CI-vs-not-CI which is better tested by prompt routing than eval scenarios. - Kept 3 positive scenarios that showed real improvement (+1.0 and +2.3 in the first eval run). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 participants
Description of Change
VisualElement.Frame's setter usesRect's bit-exact equality (Rect.Equalson doubles) to decide whether to enterUpdateBoundsComponents. On iOS, ULP-level non-determinism fromUILabel.SizeThatFits/ CoreText propagates throughVerticalStackLayoutaccumulation andGrid(\"*\",\"Auto\")star-row arithmetic into a child'sFrame, producing aRectthat differs from the cached one by ~10–22 ULP.Bit-exact equality treats those rects as different, fires
Width/HeightPropertyChanged+SizeChanged, layout invalidates, UIKit rescheduleslayoutSubviews, and the cycle repeats without converging. The UI thread becomes permanently stuck.This change adds
Rect.EqualsApproximately(other, epsilon)(non-breaking, no change toRect.Equals/==) and uses it in theFramesetter withepsilon = 1e-9— well above ~10⁻¹³ ULP at typical layout magnitudes and well below sub-pixel resolution on any current iOS device (a 3× display's pixel is ~0.333pt). It mirrors the precedent set bySafeAreaPadding.EqualsAtPixelLevel, introduced for issues #32586 and #33934 to fix the analogous infinite-layout-cycle problem on the safe-area path.Captured per-cycle pattern from the repro (G17 precision)
```text
*** PROPCHG border Height=556.00000063578295
*** SIZECHG border 390x556.00000063578295
*** PROPCHG vstack Y=566.00000063578295
*** PROPCHG vstack Height=149.99999936421713
*** SIZECHG vstack 370x149.99999936421713
*** PROPCHG border Height=556.00000063578273
*** SIZECHG border 390x556.00000063578273
*** PROPCHG vstack Y=566.00000063578273
*** PROPCHG vstack Height=149.99999936421725
*** SIZECHG vstack 370x149.99999936421725
```
The two heights toggle indefinitely (~22 ULP for the border, ~12 ULP for the vstack) and never converge.
Issues Fixed
Fixes #35142
Tests
Two new test files exercise both the loop trigger and the new helper:
VisualElementTests.FrameAssignmentIgnoresSubPixelDifferences— uses the actual border heights captured in the repro trace (556.00000063578295vs556.00000063578273). On unmodified `main` the test fails (`SizeChanged` fires after the second `Frame` assignment); after the fix it passes.Local results on macOS / .NET 10:
End-to-end verification
A minimal MauiReactor repro (`Grid(",Auto", "", Border, VerticalStackLayout-of-labels)`) reliably froze the UI thread on iPhone 16e simulator within ~1 second. With the fix, the same repro renders and stays responsive — no `PROPCHG` flood, no freeze.