-
Notifications
You must be signed in to change notification settings - Fork 2k
[Windows/Android] FlexLayout: Fix wrap misalignment due to floating-point precision #31341
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
Changes from 8 commits
a9699f0
fb5cc85
eb5dead
ea29df0
daf4479
0aafc4b
a7885aa
8e47640
d183cca
2440738
52ffd1d
243f142
784a5f7
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 |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| using Microsoft.Maui.Layouts; | ||
|
|
||
| namespace Maui.Controls.Sample.Issues | ||
| { | ||
| [Issue(IssueTracker.Github, 30957, "FlexLayout Wrap Misalignment with Dynamically-Sized Buttons in .NET MAUI", PlatformAffected.All)] | ||
| public class Issue30957 : ContentPage | ||
| { | ||
| FlexLayout _testFlexLayout; | ||
| Label _statusLabel; | ||
| Button _toggleButton1, _toggleButton2, _toggleButton3; | ||
| bool _isToggled = false; | ||
|
|
||
| public Issue30957() | ||
| { | ||
| Title = "Issue30957"; | ||
|
|
||
| var stackLayout = new StackLayout { Padding = new Thickness(20) }; | ||
|
|
||
| var instructionsLabel = new Label | ||
| { | ||
| AutomationId = "Issue30957FirstLabel", | ||
| Text = "This reproduces FlexLayout wrapping issue with font family switching. Click 'Toggle Font' to trigger precision issues.", | ||
| Margin = new Thickness(0, 0, 0, 20) | ||
| }; | ||
| stackLayout.Children.Add(instructionsLabel); | ||
|
|
||
| var toggleAllButton = new Button | ||
| { | ||
| Text = "Toggle Font Family", | ||
| AutomationId = "Issue30957ToggleButton", | ||
| BackgroundColor = Colors.LightBlue, | ||
| Margin = new Thickness(0, 0, 0, 10) | ||
| }; | ||
| toggleAllButton.Clicked += OnToggleFontClicked; | ||
| stackLayout.Children.Add(toggleAllButton); | ||
|
|
||
| var border = new Border | ||
| { | ||
| Stroke = Colors.Black, | ||
| StrokeThickness = 1, | ||
| BackgroundColor = Colors.White, | ||
| HorizontalOptions = LayoutOptions.Start, | ||
| Padding = new Thickness(4) | ||
| }; | ||
|
|
||
| _testFlexLayout = new FlexLayout | ||
| { | ||
| Wrap = FlexWrap.Wrap | ||
| }; | ||
|
|
||
| _toggleButton1 = new Button | ||
| { | ||
| Text = "Button1", | ||
| AutomationId = "Issue30957Button1", | ||
| BackgroundColor = Colors.White, | ||
| FontFamily = "OpenSansRegular", | ||
| CornerRadius = 0, | ||
| FontSize = 16, | ||
| TextColor = Colors.Black | ||
| }; | ||
|
|
||
| _toggleButton2 = new Button | ||
| { | ||
| Text = "Button2", | ||
| AutomationId = "Issue30957Button2", | ||
| BackgroundColor = Colors.White, | ||
| FontFamily = "OpenSansRegular", | ||
| CornerRadius = 0, | ||
| FontSize = 16, | ||
| TextColor = Colors.Black | ||
| }; | ||
|
|
||
| _toggleButton3 = new Button | ||
| { | ||
| Text = "Button3", | ||
| AutomationId = "Issue30957Button3", | ||
| BackgroundColor = Colors.White, | ||
| FontFamily = "OpenSansRegular", | ||
| CornerRadius = 0, | ||
| FontSize = 16, | ||
| TextColor = Colors.Black | ||
| }; | ||
|
|
||
| _testFlexLayout.Children.Add(_toggleButton1); | ||
| _testFlexLayout.Children.Add(_toggleButton2); | ||
| _testFlexLayout.Children.Add(_toggleButton3); | ||
|
|
||
| border.Content = _testFlexLayout; | ||
| stackLayout.Children.Add(border); | ||
|
|
||
| _statusLabel = new Label | ||
| { | ||
| AutomationId = "Issue30957StatusLabel", | ||
| Text = "Status: Ready to test - Click 'Toggle Font Family' to trigger precision issue", | ||
| Margin = new Thickness(0, 10, 0, 0), | ||
| TextColor = Colors.DarkGreen | ||
| }; | ||
| stackLayout.Children.Add(_statusLabel); | ||
|
|
||
| Content = stackLayout; | ||
| } | ||
|
|
||
| void OnToggleFontClicked(object sender, EventArgs e) | ||
| { | ||
| _isToggled = !_isToggled; | ||
|
|
||
| string fontFamily = _isToggled ? "OpenSansSemibold" : "OpenSansRegular"; | ||
| Color backgroundColor = _isToggled ? Colors.Black : Colors.White; | ||
| Color textColor = _isToggled ? Colors.White : Colors.Black; | ||
|
|
||
| _toggleButton1.FontFamily = fontFamily; | ||
| _toggleButton1.BackgroundColor = backgroundColor; | ||
| _toggleButton1.TextColor = textColor; | ||
|
|
||
| _toggleButton2.FontFamily = fontFamily; | ||
| _toggleButton2.BackgroundColor = backgroundColor; | ||
| _toggleButton2.TextColor = textColor; | ||
|
|
||
| _toggleButton3.FontFamily = fontFamily; | ||
| _toggleButton3.BackgroundColor = backgroundColor; | ||
| _toggleButton3.TextColor = textColor; | ||
|
|
||
| _statusLabel.Text = $"Status: Font toggled to {fontFamily} - This triggers precision issue that tolerance fix addresses"; | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,52 @@ | ||||||
| using NUnit.Framework; | ||||||
| using UITest.Appium; | ||||||
| using UITest.Core; | ||||||
|
|
||||||
| namespace Microsoft.Maui.TestCases.Tests.Issues | ||||||
| { | ||||||
| public class Issue30957 : _IssuesUITest | ||||||
| { | ||||||
| public Issue30957(TestDevice testDevice) : base(testDevice) | ||||||
| { | ||||||
| } | ||||||
|
|
||||||
| public override string Issue => "FlexLayout Wrap Misalignment with Dynamically-Sized Buttons in .NET MAUI"; | ||||||
|
|
||||||
| [Test] | ||||||
| [Category(UITestCategories.Layout)] | ||||||
| public void FlexLayoutWrappingWithToleranceWorksCorrectly() | ||||||
| { | ||||||
|
|
||||||
| App.WaitForElement("Issue30957ToggleButton"); | ||||||
|
|
||||||
|
||||||
| App.WaitForElement("Issue30957ToggleButton"); |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -533,10 +533,15 @@ static void layout_item(Item item, float width, float height, bool inMeasureMode | |||||||||||
| child.Frame[layout.frame_size_i] = basis - child.MarginThickness(layout.vertical); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| float flex_tolerance = layout.flex_dim; | ||||||||||||
| #if WINDOWS | ||||||||||||
| // Windows requires tolerance for floating-point precision issues in flex wrapping | ||||||||||||
| flex_tolerance += 0.1f; | ||||||||||||
|
||||||||||||
| float flex_tolerance = layout.flex_dim; | |
| #if WINDOWS | |
| // Windows requires tolerance for floating-point precision issues in flex wrapping | |
| flex_tolerance += 0.1f; | |
| flex_tolerance += WindowsFlexTolerance; |
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.
Seems can be still be reproduced on Android. #31341 (comment)
Could the test verify the rendering with snapshots?
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.
Hi @jsuarezruiz , The issue could not be reproduced on my end. Based on the reported issue on windows, it appears to be related to device density or another rendering factor.
Also, the test case has been updated to capture a screenshot for verification. Please review and confirm if any additional concerns remain.
Uh oh!
There was an error while loading. Please reload this page.
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.
Yes, it does seem to be related to device density or other device display factor.
I tested with a pixel 5 emulator that has a density of 2.75, and the issue does not reproduce with the provided sample in the linked issue ( #30957 ). I then tested with a pixel 7, density 2.625, and it does reproduce the issue with the provided sample. 2.625 is a pretty common density among Android devices, and all of them with that density reproduce this issue with the sample.
I will note that almost every Android device I have tested will reproduce this issue, but in slightly different cases (different text in the buttons) depending on the device's density. I can confirm that the "Button1/Button2/Button3" text in the buttons used in the UI test doesn't reproduce on any of the Android device's I've tested. I don't know the details of what the UI tests are run on, but if there are details somewhere I can try to find a better repro for the automated test cases. Does the text in the #30957 sample also pass automated tests?
I also will note that because my team was experiencing this issue mostly on Android, we applied the workaround in the code here to our own branch of the FlexLayout and just removed the
#if WINDOWSconditionals, and the issue seems to be mitigated on both platforms. I understand you probably can't merge code in without a valid A/B test case, but I do think this fix is best applied to all platforms, not just Windows. Again I am willing to help find better repro steps -- and I recommend testing with the emulator config below (and the text in the original issue) to validate the linked issue repros on Android.config.ini