mono / SkiaSharp

SkiaSharp is a cross-platform 2D graphics API for .NET platforms based on Google's Skia Graphics Library. It provides a comprehensive 2D API that can be used across mobile, server and desktop models to render images.
MIT License
4.17k stars 522 forks source link

Add and remove the compatibility APIs #2789

Closed mattleibow closed 1 month ago

mattleibow commented 2 months ago

Description of Change

In SkiaSharp 3.0 we are focusing on performance and making use of new APIs and structures.

However, we can add a few compatibility things back to make upgrades much simple/easier.

maxkatz6 commented 2 months ago

SKPath.Transform(SKMatrix) can be added as well.

mattleibow commented 2 months ago

I am trying to add back magic here, but I am wondering if this is actually useful. For example, we added some overloads - but what about all the other things that are changed - like switching from array to a span? Are we just making things better for 1% of apps?

maxkatz6 commented 2 months ago

@mattleibow testing on Avalonia, I didn't have any incompatibilities with span/array methods. Possibly because we didn't use these. The only issues I had were CreateBlur/DrowShadow methods, as well as SKPath.Transform and SKCanvas.SetMatrix. Not sure if it covers 1% or more.

I would expect apps that heavily use SkiaSharp API would have to upgrade either way. Also, it seems the biggest breaking change PRs weren't merged (yet?), like other Span related PRs.

mattleibow commented 1 month ago

Doing some more testing and I think this is working nicely. We have some new tolling that you can use to help detect things. Check out this issue description for more information: https://github.com/mono/SkiaSharp/issues/2790

Add comments on that issue if there are any general upgrade issues or issues with the tools.

maxkatz6 commented 1 month ago

Wow, didn't know about this tool. Quite useful. I will test 2.88.8-preview1 on Avalonia, thank you!

maxkatz6 commented 1 month ago

@mattleibow for some reason, I see some false positives with this tool.

That all the compat issues it found (comparing with the latest nuget preview):

<member fullname="System.Void SkiaSharp.SKCanvas::SetMatrix(SkiaSharp.SKMatrix)" />
<member fullname="System.Void SkiaSharp.SKPath::Transform(SkiaSharp.SKMatrix)" />
<member fullname="SkiaSharp.SKPoint[] SkiaSharp.SKRoundRect::get_Radii()" />
<member fullname="System.Void SkiaSharp.SKMatrix::Concat(SkiaSharp.SKMatrix&amp;,SkiaSharp.SKMatrix,SkiaSharp.SKMatrix)" />
<member fullname="System.Boolean SkiaSharp.SKPathMeasure::GetPosition(System.Single,SkiaSharp.SKPoint&amp;)" />
<member fullname="System.Boolean SkiaSharp.SKPathMeasure::GetPositionAndTangent(System.Single,SkiaSharp.SKPoint&amp;,SkiaSharp.SKPoint&amp;)" />
<member fullname="System.Boolean SkiaSharp.SKRegion/RectIterator::Next(SkiaSharp.SKRectI&amp;)" />

SetMatrix and Transform make sense, as these changes weren't yet pushed.

But all other APIs: SKRoundRect::get_Radii, SKMatrix::Concat, SKPathMeasure::GetPosition, SKPathMeasure::GetPositionAndTangent, SKRegion/RectIterator::Next were not really changed, as I can see, right?

Like, before and after for RectIterator.

maxkatz6 commented 1 month ago

Enabled render tests for SkiaSharp 3.0, some outputs are different, most likely we need to somehow use SKSamplingOptions apis with SKBitmap now (see for details). But at least compilation works well, and all the tests are passing. I think it will be enough for us, until we move to SkiaSharp 3.0 as a minimum version somewhere with 12.0 (no eta, but probably early 2025).

Will remove SKCanvas/SKPath hacks later as well, when this PR is shipped. Thank you!

mattleibow commented 1 month ago

for some reason, I see some false positives with this tool.

@maxkatz6 thanks for letting me know. I had a bug in the tool that was not reporting ref/out params, but I fixed that and pushed an update to nuget.

some outputs are different

Do you have some examples? There has been over a year of changes to skia, and before the sampling options the filter quality was whatever the implementation felt like. Now it is a bit more predictable. Not sure how one would handle cases where they did a pixel rounding adjustment or a slightly different algorithm for a path or something. Technically neither will be "wrong" just different based on your old baselines. If there are major differences, then maybe there is a bug in v2 or v3.

maxkatz6 commented 1 month ago

@mattleibow created https://github.com/AvaloniaUI/Avalonia/issues/15503 tracking issue, what I should have done earlier. Thanks to these PRs, Avalonia now works with SkiaSharp 3.0 without any reflection hacks which is great. Only difference, what I mentioned before, is that paint.FilterQuality is a no-op now in native Skia code, effectively disabling rendering interpolation for bitmaps in Avalonia. But that's what I would call a "best effort" support, sufficient until 12.0.

mattleibow commented 1 month ago

I see we do store the filter quality on the paint, but I think it was like that for some time.

Do you have an example of where it is not working properly? Maybe I am using some wrong api and not passing the quality from paint.