Skip to content

Update SkiaSharp #26373

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
Merged
merged 10 commits into from
Mar 3, 2025
Merged

Update SkiaSharp #26373

merged 10 commits into from
Mar 3, 2025

Conversation

@mattleibow
Copy link
Member

@mattleibow mattleibow commented Dec 4, 2024 •
edited
Loading

Description of Change

This PR updates SkiaSharp to the latest stable version.

The code is mostly the same, however there are some obsolete things that we may have to use for now as doing it correctly will have breaking changes in the Microsoft.Maui.Graphics.Skia NuGet API.

The change is the fact that in SkiaSharp v2.x, there was just a single SKPaint object that was used for paint/color/appearance AND font/text rendering. In the new SkiaSharp v3.x, this has been split into SKPaint and SKFont objects. Unfortunately, these types leaked out of a few MAUI types and thus changing it would force us to now expose a new SKFont argument in methods and return types of properties/methods.

This may be acceptable though since these types are semi-implementation details and typically are not used directly, but are used via the abstract types.

Fixes #26099

@mattleibow mattleibow requested a review from a team as a code owner December 4, 2024 19:50
@mattleibow
Copy link
Member Author

mattleibow commented Dec 4, 2024

If the ABI breaks are acceptable, I can do it in this PR or in a separate one.

@Redth Redth requested a review from Copilot December 4, 2024 21:47
Copilot AI reviewed Dec 4, 2024
View reviewed changes
Copy link
Contributor

Copilot AI left a 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 12 changed files in this pull request and generated no suggestions.

Files not reviewed (7)
  • eng/Versions.props: Language not supported
  • src/Graphics/samples/GraphicsTester.Skia.Gtk/GraphicsTester.Skia.Gtk.csproj: Language not supported
  • src/Graphics/src/Graphics.Skia/SKPaintExtensions.cs: Evaluated as low risk
  • src/Graphics/src/Graphics.Skia/SkiaCanvas.cs: Evaluated as low risk
  • src/Graphics/src/Graphics.Skia/SkiaTextLayout.cs: Evaluated as low risk
  • src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs: Evaluated as low risk
  • src/Graphics/src/Graphics.Skia/SkiaCanvasState.cs: Evaluated as low risk
Comments skipped due to low confidence (1)

src/SingleProject/Resizetizer/src/SkiaSharpRasterTools.cs:9

  • [nitpick] The variable name 'img' could be more descriptive. Consider renaming it to 'skiaImage'.
SKImage img;

@jfversluis jfversluis added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Dec 9, 2024
@mattleibow mattleibow force-pushed the dev/update-skiasharp branch 4 times, most recently from 19bae90 to f780dcc Compare December 12, 2024 17:18
@mattleibow mattleibow mentioned this pull request Dec 12, 2024
Merged
@mattleibow
Copy link
Member Author

mattleibow commented Dec 16, 2024

I first want to merge tests in #26591 as we do not really have any skia graphics tests

@mattleibow mattleibow marked this pull request as draft December 17, 2024 19:54
@dartasen
Copy link
Contributor

dartasen commented Jan 7, 2025

Any news on this one 👀 ?

@mattleibow mattleibow force-pushed the dev/update-skiasharp branch from fa6255d to 1de39ce Compare January 7, 2025 21:12
@mattleibow mattleibow force-pushed the dev/update-skiasharp branch 2 times, most recently from 94bf9d9 to 0909368 Compare January 8, 2025 13:41
@mattleibow mattleibow force-pushed the dev/update-skiasharp branch from 0909368 to 8364ce2 Compare January 8, 2025 13:43
@mattleibow mattleibow added area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing and removed area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions labels Jan 13, 2025
@mattleibow mattleibow self-assigned this Jan 13, 2025
@mattleibow mattleibow added this to the .NET 9 SR4 milestone Jan 13, 2025
@mattleibow mattleibow marked this pull request as ready for review January 13, 2025 15:08
@mattleibow mattleibow removed their assignment Jan 15, 2025
@PureWeen PureWeen modified the milestones: .NET 9 SR4, .NET 9 SR5 Feb 3, 2025
@PureWeen
Copy link
Member

PureWeen commented Feb 15, 2025

@mattleibow is it concerning at all that the border on the regenerated images is different?

image

@PureWeen PureWeen added the p/0 Current heighest priority issues that we are targeting for a release. label Feb 15, 2025
@mattleibow
Copy link
Member Author

mattleibow commented Feb 15, 2025

The red to green border? That is the github added and removed preview border. The images have no border.

Unless you see another border being added/removed in the actual image?

@PureWeen
Copy link
Member

PureWeen commented Feb 15, 2025 •
edited
Loading

The red to green border? That is the github added and removed preview border. The images have no border.

Unless you see another border being added/removed in the actual image?

aw yea, derp
I was just curious what the diff is on these 500 images that my human eye can't quite see
I see a subtle shift if I swipe really fast between the two.

Just validating a bit what changed here and what (if anything) we should alert people with regards to breaking changes

image

jsuarezruiz
jsuarezruiz reviewed Feb 17, 2025
View reviewed changes
src/Graphics/tests/Graphics.Tests/TestImages/Mac/arcdirection-including-background-ellipses.png
Copy link
Contributor

@jsuarezruiz jsuarezruiz Feb 17, 2025

Choose a reason for hiding this comment

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

@mattleibow What are the images differences? Seems to be the same, same resolution, etc.

PureWeen
PureWeen reviewed Feb 18, 2025
View reviewed changes
src/Graphics/src/Graphics.Skia/SkiaCanvasState.cs
}


public SKFont FontFont
Copy link
Member

@PureWeen PureWeen Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Some additional possible suggestions?

TextAttributes
SKFont
FontConfiguration
FontAttributes
FontDetails
FontProperties
FontInfo

PureWeen
PureWeen requested changes Feb 19, 2025
View reviewed changes
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

#26373 (comment)

@jsuarezruiz jsuarezruiz requested a review from PureWeen February 26, 2025 12:30
@github-project-automation github-project-automation bot moved this from Changes Requested to Approved in MAUI SDK Ongoing Mar 3, 2025
@PureWeen PureWeen merged commit 3ba0bf1 into main Mar 3, 2025
108 checks passed
@PureWeen PureWeen deleted the dev/update-skiasharp branch March 3, 2025 15:20
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Mar 3, 2025
tj-devel709 pushed a commit that referenced this pull request Mar 3, 2025
@mattleibow @tj-devel709
Update SkiaSharp (#26373)
3abb44f 
* Update SkiaSharp package versions

* Update GTK to match the new versions

* Use the new SkiaSharp APIs in resizetizer

* Regenerate resizetizer test images

* Improve test assertions

* Color ToString is invariant

* Update Graphics.Skia with new SkiaSharp APIs

* Regenerate graphics images

* Force all images to be regenerated on CI

* Fix stack overflow by setting the typeface
@drasticactions drasticactions mentioned this pull request Mar 6, 2025
Closed
bhavanesh2001 pushed a commit to bhavanesh2001/maui that referenced this pull request Mar 7, 2025
@mattleibow @bhavanesh2001
Update SkiaSharp (dotnet#26373)
56cab2f 
* Update SkiaSharp package versions

* Update GTK to match the new versions

* Use the new SkiaSharp APIs in resizetizer

* Regenerate resizetizer test images

* Improve test assertions

* Color ToString is invariant

* Update Graphics.Skia with new SkiaSharp APIs

* Regenerate graphics images

* Force all images to be regenerated on CI

* Fix stack overflow by setting the typeface
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@StephaneDelcroix StephaneDelcroix StephaneDelcroix approved these changes

@PureWeen PureWeen PureWeen approved these changes

@Eilon Eilon Awaiting requested review from Eilon Eilon was automatically assigned from dotnet/dotnet-maui-reviewers

+1 more reviewer

@jsuarezruiz jsuarezruiz jsuarezruiz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jsuarezruiz jsuarezruiz

Labels

area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing p/0 Current heighest priority issues that we are targeting for a release.

Projects

MAUI SDK Ongoing
Status: Done

Milestone

.NET 9 SR5

Development

Successfully merging this pull request may close these issues.

Update SkiaSharp in maui to the latest 3.x stable to get the security fixes

7 participants

@mattleibow @dartasen @PureWeen @StephaneDelcroix @jsuarezruiz @jfversluis