-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fixed Rectangle renders as thin line instead of filled shape for small height #31651
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
base: main
Are you sure you want to change the base?
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| public void UpdateSizeOnlyWhenStrokeExists() | ||
| { | ||
| App.WaitForElement("Issue31330BoxView"); | ||
| VerifyScreenshot(); |
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.
Running a build, pending snapshot on some platforms.
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.
Test failing on Windows:
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2530
at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2547
at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 743
at Microsoft.Maui.TestCases.Tests.Issues.Issue31330.UpdateSizeOnlyWhenStrokeExists() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31330.cs:line 18
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
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.
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.
@jsuarezruiz, Added pending snapshots for Android, Mac, and Windows platforms. Re-saved images for failed cases due to recent changes related to shapes.
| AutomationId = "Issue31330BoxView" | ||
| }; | ||
|
|
||
| var ellipse = new Ellipse |
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.
Can maintain the same test, but, including a Button to change at runtime the StrokeThickness value?
Start at 0, and tapping the Button, change the value to 20.
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.
@jsuarezruiz Updated the test case to include a Button that changes the StrokeThickness at runtime, starting from 0 and updating to 20 on tap.
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.
Pull Request Overview
This PR fixes a rendering issue where Rectangle shapes with small height values (e.g., 1.2px) were rendering as thin lines instead of filled rectangles. The issue occurred because the Shape.cs class was always reducing the shape bounds by the stroke thickness, even when no stroke was present.
- Modified stroke bounds adjustment logic to only apply when a stroke is actually present
- Added comprehensive UI tests to validate the fix across platforms
- Ensures small filled shapes render correctly without collapsing to imperceptible lines
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Shapes/Shape.cs | Added conditional check to only apply stroke inset when stroke exists |
| src/Controls/tests/TestCases.HostApp/Issues/Issue31330.cs | Created UI test page demonstrating rectangle rendering with small height |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31330.cs | Added automated test to verify screenshot-based validation |
| new RowDefinition { Height = GridLength.Star } | ||
| }; | ||
|
|
||
| Label labelText = new Label { Text = "The test passes if the edges of the circle touch the BoxView.", FontAttributes = FontAttributes.Bold }; |
Copilot
AI
Sep 18, 2025
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.
The label text describes a circle/BoxView test, but this UI test is specifically for Rectangle rendering with small height values. The label should describe the actual test being performed for the Rectangle shape.
| Ellipse ellipse = new Ellipse | ||
| { | ||
| Fill = Colors.Yellow, | ||
| StrokeThickness = 0, | ||
| WidthRequest = 100, | ||
| HeightRequest = 100 | ||
| }; | ||
|
|
||
| Button button = new Button { Text = "Update StrokeThickness" }; | ||
| button.Clicked += (s, e) => | ||
| { | ||
| ellipse.StrokeThickness = 20; | ||
| }; |
Copilot
AI
Sep 18, 2025
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.
The Ellipse and Button components appear unrelated to the Rectangle rendering issue being tested. These elements may confuse the test's purpose and should be removed unless they serve a specific validation purpose for this issue.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| public void UpdateSizeOnlyWhenStrokeExists() | ||
| { | ||
| App.WaitForElement("Issue31330BoxView"); | ||
| VerifyScreenshot(); |
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.
Test failing on Windows:
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2530
at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2547
at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 743
at Microsoft.Maui.TestCases.Tests.Issues.Issue31330.UpdateSizeOnlyWhenStrokeExists() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31330.cs:line 18
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
| public void UpdateSizeOnlyWhenStrokeExists() | ||
| { | ||
| App.WaitForElement("Issue31330BoxView"); | ||
| VerifyScreenshot(); |
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.
06493b2 to
e574466
Compare
jsuarezruiz
left a comment
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.
Could you rebase and fix the conflicts?
56a1a23 to
1eb8c9e
Compare
jsuarezruiz
left a comment
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.
There are some tests failing that I think could be related with the changes. Example: UpdateSizeOnlyWhenStrokeExists
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2566
at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 745
at Microsoft.Maui.TestCases.Tests.Issues.Issue31330.UpdateSizeOnlyWhenStrokeExists() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31330.cs:line 18
at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
|
This behavior is related to the known design issue described in #3558. |
22468c2 to
e42a1cb
Compare

Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details:
When height is a small non-integer value (e.g., 1.2), the shape renders incorrectly as a thin stroked line instead of a filled rectangle.
Root Cause
In Shape.cs, the TransformPathForBounds method always adjusts the path bounds for stroke thickness, reducing the dimensions by the StrokeThickness value.
For small heights with the default stroke thickness (1.0px), this resulted in a tiny internal rectangle that appeared as a line rather than a filled shape, unlike BoxView which rendered at the full specified height.
Description of Change
Added a condition to only apply stroke inset when a stroke is actually present (Stroke != null && StrokeThickness > 0). This prevents the shape's interior path from collapsing to an imperceptible height when rendering small filled shapes, particularly when no stroke is applied.
Validated the behaviour in the following platforms
Issues Fixed:
Fixes #31330
Screenshots