SkiaSharp icon indicating copy to clipboard operation
SkiaSharp copied to clipboard

[FEATURE] Fix the matrices

Open mattleibow opened this issue 4 years ago • 4 comments

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...

mattleibow avatar Jun 10 '20 22:06 mattleibow

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?

pauldendulk avatar Jun 11 '20 06:06 pauldendulk

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.

ziriax avatar Jun 11 '20 07:06 ziriax

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.

mattleibow avatar Jun 15 '20 14:06 mattleibow