-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fixed HorizontalStackLayout Crashes Debugger on Negative Spacing #26927
base: main
Are you sure you want to change the base?
Fixed HorizontalStackLayout Crashes Debugger on Negative Spacing #26927
Conversation
Hey there @devanathan-vaithiyanathan! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
@@ -62,7 +63,8 @@ protected override WSize ArrangeOverride(WSize finalSize) | |||
var height = finalSize.Height; | |||
|
|||
var actual = CrossPlatformArrange(new Rect(0, 0, width, height)); |
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.
I wonder if CrossPlatformArrange
is allowed to return negative widths and heights. Does it make sense in some cases or should it return always strictly >=0 values?
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.
I initially added the Math.Max logic as a safeguard to ensure the layout wouldn't encounter issues due to negative dimensions. However, upon reviewing the source and testing the behavior without this change, I found that CrossPlatformArrange works as expected and does not produce negative widths or heights under normal circumstances.
Since the framework inherently handles this case correctly, clamping the values to zero is unnecessary.
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.
Thank you for the reply. I still struggle a bit to understand why the last two lines:
var measure = CrossPlatformMeasure(availableSize.Width, availableSize.Height);
measure.Width = Math.Max(measure.Width, 0);
measure.Height = Math.Max(measure.Height, 0);
are necessary. Why CrossPlatformMeasure
can return negative widths and/or heights? I mean you fix it in MauiPanel
but is it OK that CrossPlatformMeasure
returns negative values at all?
(I'm mostly asking because one can put there an assert or log a warning if it is not desirable. I fail to see why a measurement should be negative but my knowledge of this topic is not great.)
Issue Details:
When negative spacing value is given for StackLayout, System.ArgumentOutOfRangeException is thrown in WinUI.
The issue occurs when an Image control is placed inside a StackLayout without explicitly setting the HeightRequest and WidthRequest properties.
Root Cause:
The measured value (Width/Height)of the elements like image has calculated as 0 during first measure call in WinUI platform when negative spacing is passed for Layouts. When deducting the negative spacing from measured value(Width / height) in Measure() of LayoutManager.cs it will return negative value in Measure override of MauiPanel.cs. So when converting this negative value to ToPlatform(), System.ArgumentOutOfRangeException is thrown.
Description of Change:
As Measured value of Image is negative during first call from native, the negative value has been constrained and the measured value is set to 0 using Math.Max() and converting 0 to ToPlatform() will not throw exception.
On the subsequent call, the Image is measured correctly, and the negative spacing is appropriately deducted from the measured value. This adjustment ensures that the spacing is applied accurately and aligns with the intended layout behavior.
Issues Fixed
Fixes #19513
Tested the behavior in the following platforms.
#Output Screenshot