-
Notifications
You must be signed in to change notification settings - Fork 540
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
Add compatibility overloads for SKFilterQuality #2963
Conversation
Ping @mattleibow for review |
I am not sure how this would work. These are all new APIs that never existed in 2.x: https://github.com/mono/SkiaSharp/blob/release/2.88.8/binding/Binding/SKBitmap.cs#L877-L884 What are you currently doing today? What version of SkiaSharp are you working with? |
@mattleibow We are compiling against 2.x, but still want to support users who upgrade to 3.x via a compatibility layer we did using UnsafeAccessor. Currently, we can't use UnsafeAccessor for With this PR, assuming ported to 2.x, we will be able to use the new methods both on 2.x and 3.x |
How will this work in 2.x? Are you able to create a PR against release/2.x to show. Not sure 2.x has this arg so is it just a no-op? Also, adding a test to make sure nothing blows up will also be great. Thanks! |
@mattleibow Okay I missed that this won't be possible to backport to 2.x (I had it right in the original PR description), still will allow us to easily use UnsafeAccessor for 3.x specifically, and fallback to set quality on the paint when on 2.x |
OK, I see how this works. I think this is fine then, they are obsolete and removed from the public API. So, this should make it safe. I see the 2.x version of skia had a more rudimentary form of the sampling options, and I contemplated adding that. But, rather add this than change the past and cause other issues. |
@mattleibow Great! I assume the CI failures are not related to my PR? Not sure if they need to be fixed separately before this PR can be merged or not necessary for merging this? |
The CI failures are a failure of CI. Still trying to figure it out. Every month it seems that CI needs some tweaks. |
Libraries may want to remain supporting both SkiaSharp 2.x and 3.x, as we currently do in Uno Platform.
We achieve this by detecting the assembly version and using UnsafeAccessor when needed for APIs that have changed in SkiaSharp 3.
However, there is one remaining issue for Uno Platform, which is SKFilterQuality. As the overloads now take SKSamplingOptions which is a completely new type in SkiaSharp 3, we can't use UnsafeAccessor to get this to work.
This PR adds new obsolete APIs that accepts SKFilterQuality and forwards it to the SKSamplingOptions overload. For Uno Platform, we will use
paint.FilterQuality
when on 2.x, and use the new shader overloads via UnsafeAccessor when on 3.xNOTE: This is applicable for any library that needs to remain on 2.x while allowing consumers to upgrade to 3.x.
@mattleibow, if you can review please