[release/2.x] Backport some new APIs from 3.0 to 2.x allowing smoother migration. #2756

maxkatz6 · 2024-02-15T08:37:37Z

Description of Change

With these changes Avalonia should be able to work with both 2.x and 3.0 referenced (need to double check once nightly builds are there).

API Changes

Added:

SKCanvas.SetMatrix(in SKMatrix);
SKPath.Transform(in SKMatrix, SKPath destination);
SKImageFilter methods without SKImageFilter and SKImageFilter.CropRect overloads

Behavioral Changes

None.

Required skia PR

None.

PR Checklist

Has tests (if omitted, state reason in description)
Rebased on top of main at time of PR
Merged related skia PRs
Changes adhere to coding standard
Updated documentation

This reverts commit 19c3b67.

maxkatz6 · 2024-02-15T08:45:25Z

@mattleibow I went through breaking changes page, and these changes were the low hanging fruits of what can be backported from 3.0 to 2.x. Which is also enough for Avalonia to run both versions.

Initially I also planned to backport some new SKRuntimeEffect methods (which we don't need), but it feels a bit intrusive, as I am not sure what to pass as isOpaque parameter.

I also didn't want to mark obsolete any old APIs that are going to be removed in 3.0.

And I am not sure what to do about SKFilterQuality. Looks like 3.0 still handles SKFilterQuality well everywhere, except SKPaint and DrawImage
I guess I could do:

Backport SKSamplingOptions to 2.88 (only managed part)
Add SKCanvas.DrawImage method that accepts SKSamplingOptions and maps it back to SKPaint.SKFilterQuality (ABI compatible with 3.0)
Add SKBitmap.Scale/Resize methods that also accept SKSamplingOptions (Optional, as 3.0 still has working SKFilterQuality methods).

And that would be enough? Without changing 3.0.

maxkatz6 · 2024-02-15T08:53:13Z

binding/Binding/SKCanvas.cs

@@ -952,6 +952,13 @@ public void SetMatrix (SKMatrix matrix)
 			SkiaApi.sk_canvas_set_matrix (Handle, &matrix);
 		}

+		public void SetMatrix (in SKMatrix matrix)


Note: C# will always prefer more specific overload. Which means for 2.x builds old code will still use old SetMatrix(SKMatrix), unless they specify in explicitly.

One annoying thing is that this will make this method ambiguous to visual basic... VB appears not to be able to distinguish between in/ref and nothing. I know we have a scattering of ref and nothing in 2.x and I tried to clean this all up. However, maybe this is making migrations a bit more annoying.

Unfortunately, you are right. Just tested locally, and found this issue confirming it dotnet/vblang#505

I guess the only choice would be to use reflection/UnsafeAccessor then. I don't really want to modify SkiaSharp 3.0 API surface.

workgroupengineering · 2024-02-15T09:33:33Z

What do you think if instead of adding members to existing classes you added them as extension methods in a class called CompatibilyExtensions?

maxkatz6 · 2024-02-15T09:34:57Z

@workgroupengineering this extension method would have to be added to 3.0 as well. So, it can be binary compatible. Imo, it would add more unwanted API surface to maintain.

mattleibow · 2024-03-01T01:46:46Z

binding/Binding/SKCanvas.cs

@@ -952,6 +952,13 @@ public void SetMatrix (SKMatrix matrix)
 			SkiaApi.sk_canvas_set_matrix (Handle, &matrix);
 		}

+		public void SetMatrix (in SKMatrix matrix)


One annoying thing is that this will make this method ambiguous to visual basic... VB appears not to be able to distinguish between in/ref and nothing. I know we have a scattering of ref and nothing in 2.x and I tried to clean this all up. However, maybe this is making migrations a bit more annoying.

mattleibow · 2024-03-01T01:51:05Z

binding/Binding/SKImageFilter.cs

@@ -74,13 +74,22 @@ internal SKImageFilter(IntPtr handle, bool owns)

 		// CreateMatrix

+		public static SKImageFilter CreateMatrix(ref SKMatrix matrix)


I think this is in in 3.x.

maxkatz6 · 2024-03-07T01:43:06Z

@mattleibow update the PR, removed "in" overloads that will break VB. Current PR changes will already help with some migration, though not ideal.

mattleibow · 2024-03-07T05:41:14Z

Ok. Afk right now but will look tonight or tomorrow. I also think I may have some idea of how to do something quite nice for compat. It is an idea still, but it is coming together nicely.

mattleibow · 2024-03-08T10:00:31Z

This was idea and my testing of prototypes seemed to work nicely.

#2789

maxkatz6 · 2024-03-08T19:14:26Z

@mattleibow I like your idea. Closing this PR then.

maxkatz6 · 2024-03-08T19:19:07Z

Although, wait, that kind of API changes of this PR cannot be added to 3.0, as it already has these methods :D

Unless we add back overloads with SKImageFilter and SKImageFilter.CropRect:
https://github.com/mono/SkiaSharp/blob/v2.88.7/binding/Binding/SKImageFilter.cs#L101

Imo, for these methods specifically, it would be simpler to get this PR to 2.88 with overloads without optional parameters, keeping common API.

mattleibow · 2024-03-08T19:33:44Z

Ah yeah. Both. We can backport anything too ugly to do - such as croprect. I'm working on making 2.x shippable again.

mattleibow · 2024-03-27T19:01:49Z

I did some looking around and I had to also remove the default args from the existing ones. #2810

For example, if there is this:

public static SKImageFilter CreatePaint(SKPaint paint, SKImageFilter.CropRect cropRect = null)

And we just add:

public static SKImageFilter CreatePaint (SKPaint paint)

People that are using it like this:

SKImageFilter.CreatePaint(mypaint);

Will now get an ambiguous overload as both match. So I removed the = null value so if you are not specifying a crop rect, then you are going to use the new overload. If you are, well you have specified a value.

mattleibow · 2024-03-27T20:14:01Z

Thanks for this PR. I am going to close this one as I think I got all the APIs from 3.x in my PR and a few more things: #2810

maxkatz6 added 5 commits February 15, 2024 00:07

Backport new compatible methods for SKImageFilter

e3f335e

Make SKRuntimeEffect slightly more compatible with 3.0

19c3b67

Backport SKCanvas.SetMatrix compatible with 3.0

79c8274

Backport SKPath.Transform compatbile with 3.0

2951193

Revert "Make SKRuntimeEffect slightly more compatible with 3.0"

a0ff234

This reverts commit 19c3b67.

maxkatz6 commented Feb 15, 2024

View reviewed changes

maxkatz6 changed the title ~~[release/2.x] Backport new APIs from 3.0 to 2.x allowing smoother migration.~~ [release/2.x] Backport some new APIs from 3.0 to 2.x allowing smoother migration. Feb 15, 2024

maxkatz6 mentioned this pull request Feb 15, 2024

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

Merged

mattleibow reviewed Mar 1, 2024

View reviewed changes

maxkatz6 added 3 commits March 6, 2024 17:40

Remove SKCanvas.SetMatrix(in SKMatrix)

38d70a5

Remove SKPath.Transform(in SKMatrix)

ee5c52e

Use

0772de4

maxkatz6 closed this Mar 8, 2024

maxkatz6 reopened this Mar 8, 2024

Merge branch 'release/2.x' into backport-some-new-APIs

07b711e

mattleibow mentioned this pull request Mar 27, 2024

[release/2.x] Add some new overloads for better compat #2810

Merged

5 tasks

mattleibow closed this Mar 27, 2024

mattleibow mentioned this pull request Mar 28, 2024

Improve 3.x compatibility with 2.x (or vice versa) #2790

Open

maxkatz6 mentioned this pull request Apr 6, 2024

Add and remove the compatibility APIs #2789

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[release/2.x] Backport some new APIs from 3.0 to 2.x allowing smoother migration. #2756

[release/2.x] Backport some new APIs from 3.0 to 2.x allowing smoother migration. #2756

maxkatz6 commented Feb 15, 2024

maxkatz6 commented Feb 15, 2024 •

edited

Loading

maxkatz6 Feb 15, 2024

mattleibow Mar 1, 2024

maxkatz6 Mar 7, 2024

maxkatz6 Mar 7, 2024

workgroupengineering commented Feb 15, 2024

maxkatz6 commented Feb 15, 2024

mattleibow Mar 1, 2024

mattleibow Mar 1, 2024

maxkatz6 Mar 7, 2024

maxkatz6 commented Mar 7, 2024

mattleibow commented Mar 7, 2024

mattleibow commented Mar 8, 2024

maxkatz6 commented Mar 8, 2024

maxkatz6 commented Mar 8, 2024

mattleibow commented Mar 8, 2024

mattleibow commented Mar 27, 2024 •

edited

Loading

mattleibow commented Mar 27, 2024

		@@ -74,13 +74,22 @@ internal SKImageFilter(IntPtr handle, bool owns)

		// CreateMatrix

		public static SKImageFilter CreateMatrix(ref SKMatrix matrix)

[release/2.x] Backport some new APIs from 3.0 to 2.x allowing smoother migration. #2756

[release/2.x] Backport some new APIs from 3.0 to 2.x allowing smoother migration. #2756

Conversation

maxkatz6 commented Feb 15, 2024

maxkatz6 commented Feb 15, 2024 • edited Loading

maxkatz6 Feb 15, 2024

Choose a reason for hiding this comment

mattleibow Mar 1, 2024

Choose a reason for hiding this comment

maxkatz6 Mar 7, 2024

Choose a reason for hiding this comment

maxkatz6 Mar 7, 2024

Choose a reason for hiding this comment

workgroupengineering commented Feb 15, 2024

maxkatz6 commented Feb 15, 2024

mattleibow Mar 1, 2024

Choose a reason for hiding this comment

mattleibow Mar 1, 2024

Choose a reason for hiding this comment

maxkatz6 Mar 7, 2024

Choose a reason for hiding this comment

maxkatz6 commented Mar 7, 2024

mattleibow commented Mar 7, 2024

mattleibow commented Mar 8, 2024

maxkatz6 commented Mar 8, 2024

maxkatz6 commented Mar 8, 2024

mattleibow commented Mar 8, 2024

mattleibow commented Mar 27, 2024 • edited Loading

mattleibow commented Mar 27, 2024

maxkatz6 commented Feb 15, 2024 •

edited

Loading

mattleibow commented Mar 27, 2024 •

edited

Loading