[FEATURE] Fix the matrices

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.4k stars 537 forks source link

[FEATURE] Fix the matrices #1330

Open mattleibow opened 4 years ago

mattleibow commented 4 years ago

Current we have some soon-to-be-obsolete types (in m84, so not the next version, but the next next version):

SKMatrix44
SK3dView

This is all replaced with a new SKM44.

But we have System.Numerics...

However, Numerics does not have a matrix of 3x3 which we use all over. Having a mix is going to be weird and annoying.

Also, I may need to add members to the matrix for SkiaSharp uses, so using Numerics will get a bit messy.

But, intrinsics...

We could use SkiaSharp matrices on the members and fields, and then have then be implicitly convertible to VectorX and Matrix4x4 for things...

We also have SKPoint, SKPoint3, SKSize...

And no breaking changes...

Performance is important...

What to do, what to do...

pauldendulk commented 4 years ago

A broad comment from my user perspective:

Breaking changes are not such a problem. Just document them well. It helps to put tags like these on the deprecated fields to the compiler can help in the migration: [Obsolete("Use X instead", true)]
Performance is important. We iteratively draw again and again, so especially avoiding newing objects for simple operations is important.
If it is possible to avoid the slower solution by using the matrices directly while other users can use the implicit conversion this is fine as well.

It it hard to give more specific comments. Could you point to some code that would be affected and what it would look like after the changes?

ziriax commented 4 years ago

For a major version bump, I am perfectly fine with breaking changes. After all, it is a new major version.

For SkM44, it seems to binary compatible at first sight with Matrix4

We should look at the memory layout of these matrices. Passing a matrix by ref and just casting the pointer might be the fastest way.

That only would work for matrices passed by reference. If Skia expects a matrix to be passed by value, then a copy is made anyway, so it doesn't matter what type we use.

mattleibow commented 4 years ago

We can't really do breaking changes too much since folks like Syncfusion/Telerik and other vendors will all have to update and then all users will have to make sure thy are all in sync.

@Ziriax If the ABI for Matrix4 and SKM44 are the same, then we could just ref it over... For Matrix, I cheat a bit as they have a whole bunch of other private fields. So I ref it over to my own type, and then copy it to the real matrix type.

taublast commented 3 months ago

We can't really do breaking changes too much since folks like Syncfusion/Telerik and other vendors will all have to update and then all users will have to make sure thy are all in sync.

Can't they just either update, either use older nugets?

Like one of the skia maintainers said "Our ability to evolve Skia effectively depends on our ability to drop old features."