-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Entry Height gets updated when loaded inside the RoundRectangle Shape #25166
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
Entry Height gets updated when loaded inside the RoundRectangle Shape #25166
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
b8f7161 to
fbc0cd2
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
fbc0cd2 to
d16742e
Compare
|
/azp run |
This comment was marked as outdated.
This comment was marked as outdated.
1 similar comment
|
Azure Pipelines successfully started running 3 pipeline(s). |
| // TODO: not using this.GetPath().Bounds.Size; | ||
| // since default GetBoundsByFlattening(0.001) returns incorrect results for curves | ||
| RectF pathBounds = this.GetPath().GetBoundsByFlattening(1); | ||
| // Get the inner path by subtracting the stroke thickness for a rounded rectangle, as the GetPath() method returns the path including the stroke thickness. |
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.
A better approach would be to align the behavior between the RoundRectangle and the rest of the shapes.
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 @PureWeen, The issue occurs specifically with RoundRectangle because its GetPath() method does not exclude StrokeThickness from width and height calculations. In contrast, other shapes like Rectangle and Ellipse already account for StrokeThickness in their GetPath() implementations.
For example, in Rectangle.cs, GetPath() subtracts StrokeThickness from both width and height before creating the path. However, in RoundRectangle.cs, GetPath() directly uses the full width and height without adjusting for StrokeThickness.
As a result, each MeasureOverride() call includes StrokeThickness in the size calculation, causing the height to increase on every input event for RoundRectangle. To align behavior across all shapes, I have updated the RoundRectangle implementation to exclude StrokeThickness from GetInnerPath(), ensuring consistency with Rectangle.cs.
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.
What would happen if we just aligned the GetPath method of RoundRectangle to work like the GetPath of Rectangle and Ellipse?
The worry here is that we're creating an extra method that's purely for a special case scenario
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.
@PureWeen The GetPath method of RoundRectangle was intentionally modified in a previous PR to remove any compensation for StrokeThickness, as the calling code now handles that separately. The PR description explicitly states that GetPath should use the width and height as-is, without adjustments for StrokeThickness.
To maintain consistency with the updated approach while still resolving the issue, I have applied the fix within GetInnerPath. This ensures that the behavior aligns with other shapes without altering GetPath, which has already been adjusted for a broader set of changes.
PureWeen
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.
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 we align the behavior between all the shapes? #25166 (comment)
reply : #25166 (comment) |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@SuthiYuvaraj Could you rebase? Thanks in advance. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
3ad59bc to
26649d2
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
2cc27d8
into
dotnet:inflight/current
…#25166) * Fix commit for 18092 * Update Shape.cs * Commit for Maccatalyst * Update Shape.cs * Update Shape.cs * Commit for RoundRectangle change * commit name changes * Update Issue18092.xaml * Update Issue18092.xaml * Update Issue18092.xaml * Commit for method name
…#25166) * Fix commit for 18092 * Update Shape.cs * Commit for Maccatalyst * Update Shape.cs * Update Shape.cs * Commit for RoundRectangle change * commit name changes * Update Issue18092.xaml * Update Issue18092.xaml * Update Issue18092.xaml * Commit for method name
…#25166) * Fix commit for 18092 * Update Shape.cs * Commit for Maccatalyst * Update Shape.cs * Update Shape.cs * Commit for RoundRectangle change * commit name changes * Update Issue18092.xaml * Update Issue18092.xaml * Update Issue18092.xaml * Commit for method name
…#25166) * Fix commit for 18092 * Update Shape.cs * Commit for Maccatalyst * Update Shape.cs * Update Shape.cs * Commit for RoundRectangle change * commit name changes * Update Issue18092.xaml * Update Issue18092.xaml * Update Issue18092.xaml * Commit for method name
RootCause
In the case of a rounded rectangle, the stroke thickness is added to the measured size during each
Measurecall. As a result, for every input in theEntrycontrol, the width and height of the rounded rectangle are updated.This issue was introduced due to the following PRs:
Rendering Shapes without explicit bounds. by VSadov · Pull Request #5817 · dotnet/maui (github.com)
Fixed Android RoundRectangle border logic by jstedfast · Pull Request #17087 · dotnet/maui (github.com)
While reverting change in above PR, current fix is fixed and issue mentioned above PR replicates.
Description of Change
The fix ensures that RoundRectangle handles stroke thickness consistently across measure calls. Previously, GetPath() directly used the full width and height without adjusting for StrokeThickness, causing the measured size to increase with each input event.
To resolve this, GetInnerPath() has been updated to exclude StrokeThickness from the size calculation, aligning RoundRectangle behavior with Rectangle. This prevents the height from increasing on each MeasureOverride() call.
Issues Fixed
Fixes #18092
Tested the behaviour in the following platforms
Output Videos
18092_BeforeCahnges.mp4
18092_AfterChanges.mp4