Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[iOS] GraphicsView DrawString - fix #26304

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[iOS] GraphicsView DrawString - fix #26304

wants to merge 2 commits into from
+92 −19

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Dec 2, 2024 •
edited
Loading

Issues Fixed

Fixes #24450
Fixes #8486
Fixes #4993

Before After

@kubaflo kubaflo requested a review from a team as a code owner December 2, 2024 15:00
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Dec 2, 2024
@kubaflo kubaflo added area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing platform/iOS 🍎 labels Dec 2, 2024
@kubaflo kubaflo mentioned this pull request Dec 2, 2024
Merged
jfversluis
jfversluis requested changes Dec 3, 2024
View reviewed changes
Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase this one for the new GraphicsView category?

@kubaflo
Copy link
Contributor Author

kubaflo commented Dec 3, 2024

Can you rebase this one for the new GraphicsView category?

Sure thing!

@jfversluis
Copy link
Member

jfversluis commented Dec 3, 2024 •
edited
Loading

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/rebase

jsuarezruiz
jsuarezruiz requested changes Jan 10, 2025
View reviewed changes
src/Controls/tests/TestCases.HostApp/Issues/Issue8486.cs Outdated
canvas.FontColor = Colors.Black;
canvas.FontSize = 10;
canvas.Font = Microsoft.Maui.Graphics.Font.Default;
canvas.DrawString("GraphicsText", dirtyRect.Left + dirtyRect.Width / 2, dirtyRect.Top + dirtyRect.Height / 2, HorizontalAlignment.Left);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the test to include multiple strings using all the different HorizontalAlignment options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! Done :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simulator Screenshot - iPhone 16 Pro Max - 2025-01-10 at 19 41 23

@riccardominato
Copy link

riccardominato commented Jan 17, 2025 •
edited
Loading

@kubaflo Thank you for this PR!

Since looking at the commits I've noticed GetStringSize() is used, I would like to point out #18679.
Don't know it the issue applies to this PR, but maybe it's worth a check.

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rokmeglic71
Copy link

@Eilon and @StephaneDelcroix please let me know what needs to be done here to make a step forward with this? Thank you for your time and effort.

@riccardominato
Copy link

riccardominato commented Mar 17, 2025 •
edited
Loading

I want to add a little bit to my previous comment #26304 (comment).
It seems that the same problem addressed by this PR also happens on other overloads of DrawString and on DrawText if the bounding rectangle provided as parameter is obtained using canvas.GetStringSize().

Could it be possible to extend this fix to all the Draw*() methods?.
Thank you.

@kubaflo kubaflo self-assigned this Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfversluis jfversluis jfversluis requested changes

@jsuarezruiz jsuarezruiz jsuarezruiz requested changes

@Eilon Eilon Awaiting requested review from Eilon Eilon is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

@StephaneDelcroix StephaneDelcroix Awaiting requested review from StephaneDelcroix StephaneDelcroix is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

Requested changes must be addressed to merge this pull request.

Assignees

@kubaflo kubaflo

Labels
area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing community ✨ Community Contribution platform/iOS 🍎
Projects
None yet
Milestone
No milestone
5 participants
@kubaflo @jfversluis @jsuarezruiz @riccardominato @rokmeglic71