-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Windows] Fix for Argument Exception raised when the GetStringSize method of ICanvas called with default font #29048
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
[Windows] Fix for Argument Exception raised when the GetStringSize method of ICanvas called with default font #29048
Conversation
…th default font )
…th default font )
…th default font )
…th default font )
…th default font )
…th default font )
…th default font )
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.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
src/Graphics/src/Graphics/Platforms/Windows/PlatformStringSizeService.cs:33
- Consider adding a unit test that explicitly verifies the clamped FontWeight value to prevent potential regressions.
FontWeight = new FontWeight { Weight = (ushort)Math.Clamp(font.Weight, 1, 999) },
src/Graphics/src/Graphics/Platforms/Windows/FontExtensions.cs:33
- Consider adding a unit test to verify that FontWeight is correctly clamped for both non-null and null font scenarios.
FontWeight = new FontWeight { Weight = (ushort)Math.Clamp(font?.Weight ?? FontWeights.Regular, 1, 999) },
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20419.cs:15
- [nitpick] Consider enhancing the test to include assertions that verify the clamped font weight instead of solely waiting for a UI element, ensuring that the fix is directly validated.
[Test]
|
/azp run MAUI-UITests-public |
| FontSize = textSize, | ||
| FontWeight = new FontWeight { Weight = (ushort)font.Weight }, | ||
| // Ensure font weight stays within the valid range (1–999) to avoid runtime errors | ||
| FontWeight = new FontWeight { Weight = (ushort)Math.Clamp(font.Weight, 1, 999) }, |
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.
We need the same approach in different classes. Can create an extension method and reuse it.
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.
should we add a extension method to parse this?
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 , I've updated the fix by creating the CanvasTextFormat instance using the ToCanvasTextFormat extension method defined in FontExtensions.
|
Azure Pipelines successfully started running 1 pipeline(s). |
…the ToCanvasTextFormat extension method defined in FontExtensions
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…thod of ICanvas called with default font (#29048) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * Modified the test case * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * Have updated the fix by creating the CanvasTextFormat instance using the ToCanvasTextFormat extension method defined in FontExtensions * Removed unused namespace references
…thod of ICanvas called with default font (#29048) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * Modified the test case * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * Have updated the fix by creating the CanvasTextFormat instance using the ToCanvasTextFormat extension method defined in FontExtensions * Removed unused namespace references
For more information about inflight process check https://github.com/dotnet/maui/wiki/Inflight-Branch-Process ## .NET MAUI Release Notes - inflight/candidate This document contains release notes for changes from main branch to inflight/candidate branch. ### MAUI Product Fixes * [iOS] Fix: FlyoutPage memory leak by @bhavanesh001 in #28769 * [Windows] Fix for CarouselView IsSwipeEnabled=False Prevents Visual Navigation by @SubhikshaSF4852 in #29286 * [Windows] Fix for Argument Exception raised when the GetStringSize method of ICanvas called with default font by @SyedAbdulAzeemSF4852 in #29048 * Removed frame styles by @Vignesh-SF3580 in #29222 * [Android] Fixed the CollectionView Header and Footer Do Not Align with Horizontal ItemsLayout When EmptyView is Displayed by @Ahamed-Ali in #28779 * Add global xmlns in template by @StephaneDelcroix in #29203 * Fixed - On iOS GestureRecognizers don't work on Span in a Label, which doesn't get IsVisible (=true) update from its parent by @KarthikRajaKalaimani in #29024 * Fixed Footer not displayed at the Bottom When EmptyView in CV2 by @Dhivya-SF4094 in #28681 * Fixed typo in Connectivity.shared.cs by @corvinsz in #29213 ### Testing * Re-enabled flaky UI test TextInEditorShouldScroll by @NirmalKumarYuvaraj in #29167 * [Testing] Re-Enabled UI Test - Issue10222Test by @TamilarasanSF4853 in #29226 * [Testing] Feature Matrix UITest Cases for CollectionView Selection Feature by @LogishaSelvarajSF4525 in #29165 ### Dependency Updates *No dependency updates in this release* ### Docs *No documentation changes in this release* ### Housekeeping *No housekeeping changes in this release* ## New Contributors * @corvinsz made their first contribution in #29213 **Full Changelog**: main...inflight/candidate
…thod of ICanvas called with default font (dotnet#29048) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * Modified the test case * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * [Windows] Fix for 20419 ( Bug in ICanvas.GetStringSize when called with default font ) * Have updated the fix by creating the CanvasTextFormat instance using the ToCanvasTextFormat extension method defined in FontExtensions * Removed unused namespace references
Issue Details
Root Cause
Description of Change
Before assigning the font's weight to the Weight property of the FontWeight class while creating a CanvasTextFormat object, clamped the font's Weight value using Math.Clamp to ensure it stays within the valid range of 1 to 999.
Reference : FontWeight Struct
Validated the behaviour in the following platforms
Issues Fixed
Fixes #20419
Output