Deprecate stride math for numerics alternatives

[Doprez]

PR Details

Doing some benchmarks and discussing with @Eideren and @ericwj there are some major speed benefits using System.Numerics instead of Stride math.

This PR is meant to deprecate the math functions that are available with the System.Numerics equivalents.

Related Issue

#2238
#2576

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

IMPORTANT

#2238 must be merged first for the implicit conversions

[Doprez]

first hurdle:
Invert seems to give NaN values where Stride did not. It seems like the

Assert.Equal() Failure: Values differ
Expected: [M11:0 M12:0 M13:0 M14:0] [M21:0 M22:0 M23:0 M24:0] [M31:0 M32:0 M33:0 M34:0] [M41:0 M42:0 M43:0 M44:0]
Actual:   [M11:NaN M12:NaN M13:NaN M14:NaN] [M21:NaN M22:NaN M23:NaN M24:NaN] [M31:NaN M32:NaN M33:NaN M34:NaN] [M41:NaN M42:NaN M43:NaN M44:NaN]

same with Orthonomolize

Assert.Equal() Failure: Values differ
Expected: [M11:0.18257418 M12:0.36514837 M13:0.5477226 M14:0.73029673] [M21:0.8164966 M22:0.40824836 M23:-1.4600097E-07 M24:-0.40824822] [M31:0 M32:0 M33:9.536743E-07 M34:0] [M41:-0.4714045 M42:-0.4714045 M43:0.70710677 M44:-0.23570225]
Actual:   [M11:0.18257418 M12:0.36514837 M13:0.5477225 M14:0.73029673] [M21:0.8164966 M22:0.40824836 M23:1.4600097E-07 M24:-0.40824822] [M31:NaN M32:NaN M33:NaN M34:NaN] [M41:NaN M42:NaN M43:NaN M44:NaN]

So it seems like 0 values in System.Numerics arent handled in these methods?

[Doprez]

Beginning benchmarks mostly for fun for now since Invert is broken:

[ericwj]

Missing a space here?

[ericwj]

And here.

[ericwj]

It is a bit unfortunate that Matrix4x4 can't be read as 4 vectors, but it should be and you can do it from M11, M21, etc. There are several ways of doing that, e.g.:

var rows = MemoryMarshal.Cast<Matrix4x4, Vector4>(MemoryMarshal.CreateSpan(ref m, 1));
var row4 = rows[3]; // get the bounds check out of the way asap
var row3 = rows[2];
...

Or, even faster:

var row1 = Unsafe.As<float, Vector4>(ref m.M11);
var row2 = Unsafe.As<float, Vector4>(ref m.M21);
var row3 = Unsafe.As<float, Vector4>(ref m.M31);
var row4 = Unsafe.As<float, Vector4>(ref m.M41);

Could also use Bitcast, but since it is by value and the sizes don't match, I'd have to try. And either way not entirely sure the above code will be optimal.

Below you should do the reverse, since certainly 16 float assignments is a bad idea that you'd want to prevent everywhere you see them.

[ericwj]

Generally speaking, not sure whether it is a good idea to add public API for System.Numerics to a type that may get scrapped when all deprecations and substitutions are done. Basically what you're doing by having these public is hinting at future work to get Stride.Core.Mathematics surface area on par between System.Numerics types and Stride types. Those two things are sort of in competition with each other.

Before that decision is made, I would keep these internal.

[ericwj]

Use an attribute constructor that takes DiagnosticId.

[Obsolete("Use MathUtil.Invert instead.", DiagnosticId = "STRIDE2100")]
public void Invert();

or

[Obsolete("Use MathUtil.Invert instead.", DiagnosticId = "STRIDE2100", UrlFormat = "https://doc.stride3d.net/latest/en/whatever/{0}")]
public void Invert();

The diagnostic STRIDE2000 was used previously, so just reserve the range 2100-2199. Use as few diagnostics codes as possible, but do use a different one if the situation is different - if people would need to supress them in different situations, for example. For example use STRIDE2101 for deprecations of overloads that take in, ref or ref ro but which are otherwise operations that shouldn't be deprecated such as Add. STRDIAG is a bad prefix imho. STR is string, not Stride, at least that is the automatic context free association. And yeah its a diagnostic, why even bother saying that. Talk to the docs team on Discord (@VaclavElias) to figure out how much effort it is and what the URL format should be. There should be basically only one format for all of Stride.

[ericwj]

My misplaced comment from the now merged PR #2238, slightly clarified.

I wouldn't add any surface area to Stride's math in terms of parameters it accepts. It accepts Stride types and returns Stride types now. Whatever how it is implemented is a private detail and that can use System.Numerics types. In addition these conversion methods what #2238 is about make it a lot easier to mix-and-match, so let people use them. The exception is Matrix, since the conversion involves a transpose and hence is not free.

Don't deprecate math that is perfectly fine and which can be optimized by changing private implementation. Deprecate performance smells such as by-ref and in-place modification (i.e. void Transpose() and void Add(ref ro, ref ro, out), Subtract(in, in) etc).

• This will eventually reduce the surface area and align what remains with System.Numerics for as far as the features exist. Whether you use the one or the other, hence when/if they are deprecated, is moot.
• The discussion about removing Stride's math in favor of some third party library should be about what has no equivalent in System.Numerics and the occasional feature that with differences in behavior, such as your first comment above.

One goal is to get Stride's math DRY. It is so freaking silly to have a handful of implementations for one operation! So if any Stride math is touched, for the purpose of deprecations or otherwise, have them all delegate to the most performant implementation. This will be the by-value one, which would work for readonly struct. And let the JIT handle the ref in ref ro dilemmas - it should often be able to inline and hide the smell when these methods are used in larger blocks in user code.