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.

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

Make Avalonia.Skia compatible with both SkiaSharp 2.88 and 3.0 #14510

Merged
merged 6 commits into from
Mar 7, 2024

Conversation

maxkatz6
Copy link
Member

@maxkatz6 maxkatz6 commented Feb 6, 2024

What does the pull request do?

We have a very little choice but try to sit on both chairs with SkiaSharp. Otherwise, Avalonia is going to be locked on 2.88 version until 12.0 release which won't happen this year.

For the most part SkiaSharp 3.0 is binary compatible with SkiaSharp 2.88. But not 100%, so it still breaks Avalonia rendering backend.

Ideally, my approach with this PR and that compat is general is something like this:

  1. If we are running on 2.88 (runtime check) - call original API directly
  2. If we are running on 3.0 and .NET 8 - use UnsafeAccessor
  3. If we are running on 3.0 and .NET 7 or lower - try to call reflection EDIT: just throw.

Note: this PR doesn't attempt to use SKSamplingOptions instead of old deprecated API as it was done in #12729. In other words, we will use deprecated API until Avalonia fully migrates to 3.0 as a minimum version.

What's tested so far:

  1. macOS + metal render
  2. All pages on ControlCatalog and RenderDemo

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0044485-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6
Copy link
Member Author

maxkatz6 commented Feb 6, 2024

@mattleibow hi! IIRC there was a document that tracks all breaking changes between 2.88 and 3.0 but can't find it now. Do you still have it?

@maxkatz6 maxkatz6 changed the title [Insane Experiment] Make Avalonia.Skia compatible with both SkiaSharp 2.88 and 3.0 [Experiment] Make Avalonia.Skia compatible with both SkiaSharp 2.88 and 3.0 Feb 6, 2024
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0044493-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6
Copy link
Member Author

maxkatz6 commented Feb 6, 2024

If anybody want/can test these packages in your apps with updated SkiaSharp to 3.0 previews, I would appreciate help with testing. Before agreeing on this approach with reusing the same Avalonia.Skia assembly, I want to know if it really works on majority of apps at least.

@BAndysc
Copy link
Contributor

BAndysc commented Feb 6, 2024

I've tried this in my App with skia 3.0.0-preview.1.8, I didn't spot any bugs (tho the app is not graphically intensive), but I have notice the default for the image scaling/antialiasing option has changed. Is intentional? (top screenshot is the old skia, the bottom one is the preview) (macOS @ net8)
image

@mattleibow
Copy link
Contributor

@mattleibow hi! IIRC there was a document that tracks all breaking changes between 2.88 and 3.0 but can't find it now. Do you still have it?

I think this file and sibling files: https://github.com/mono/SkiaSharp/blob/main/changelogs/SkiaSharp/3.0.0/SkiaSharp.humanreadable.md

@mattleibow
Copy link
Contributor

mattleibow commented Feb 6, 2024

If there are some APIs that are used and it is simple to add back, we can maybe add them back into SkiaSharp. I am hoping to ease the treansition if possible, but not at the cost of complex code/hacks. So, depending on what you need, I can potentially add some apis into 2.x or back into 3.x. Can't promise, but some easy wins to reduce breaks is nice to have.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0044970-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6
Copy link
Member Author

We don't need that reflection after we use builds with this PR included mono/SkiaSharp#2756

I still don't know what's wrong with antialiasing in 3.0. FilterQuality is obsolete, but as I can see it still should be supported more or less. Nevermind, native side of Skia doesn't use SkCompatPaint.getFilterQuality anymore. It would be a bit harder to support.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0045750-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 force-pushed the skiasharp3-compat branch from 345b615 to da16fd4 Compare March 7, 2024 03:54
@maxkatz6 maxkatz6 marked this pull request as ready for review March 7, 2024 03:54
@maxkatz6 maxkatz6 requested a review from kekekeks March 7, 2024 03:55
@maxkatz6 maxkatz6 changed the title [Experiment] Make Avalonia.Skia compatible with both SkiaSharp 2.88 and 3.0 Make Avalonia.Skia compatible with both SkiaSharp 2.88 and 3.0 Mar 7, 2024
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0045818-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0045826-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@kekekeks kekekeks enabled auto-merge March 7, 2024 05:57
@kekekeks kekekeks added this pull request to the merge queue Mar 7, 2024
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0045834-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Merged via the queue into master with commit 11f5203 Mar 7, 2024
7 checks passed
@kekekeks kekekeks deleted the skiasharp3-compat branch March 7, 2024 07:58
@maxkatz6 maxkatz6 mentioned this pull request Apr 25, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants