[3.0] Generic Matrix Math - #2528
Conversation
|
Remaining concerns:
|
|
Small (maybe important) nits: You need to Bitcast your vectors to System.Numerics when T is float in order to allow enregistering to float regs. Also the formatting to span should not use intermediate buffers if possible. |
add TryFormat with less stack allocation if possible & enregister in system.numerics if possible & do not throw exceptions directly
Perksey
left a comment
Perksey
left a comment
There was a problem hiding this comment.
Pretty agreeable PR! I have a few comments, but they should be pretty easy all things considered :) thank you so much for your hard work!
| @@ -0,0 +1,33 @@ | |||
| namespace GenerateMatrixMath.Model | |||
| { | |||
| public record class Dimension(int Rows, int Columns) : IComparable<Dimension> | |||
There was a problem hiding this comment.
In general, we are trying to get in the habit of documenting even the "private" parts of the codebase (see SilkTouch being documented extensively, even for some of the private APIs) in an attempt to make the code a bit more readable/maintainable for our future selves. 2.X lacked this quite a bit. I'm not necessarily saying go through and add documentation to everything as a requirement for this PR being merged, it's just something to bear in mind for the future - you can use your own judgement on whether you want to go back to what you've done on this PR and apply this retroactively as I appreciate this probably isn't documented anywhere yet, so this is probably the first you're learning of this.
| @@ -0,0 +1,26 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Can we move this to sources/Maths/Generator/Silk.NET.Maths.Generator.csproj? Or possibly somewhere in eng/? (could call it MatrixGenerator instead of Generator, whatever the most appropriate name is!)
| @@ -0,0 +1,26 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
| <PropertyGroup> | |||
There was a problem hiding this comment.
I would add IsPackable = false as well to ensure we don't accidentally end up shipping this.
| using System.Runtime.CompilerServices; | ||
| using System.Runtime.Serialization; | ||
|
|
||
| /// <summary>A structure encapsulating a {{= size}} matrix.</summary> |
There was a problem hiding this comment.
Should the documentation comment denote this structure as row-major?
| /// </summary> | ||
| /// <param name="row">The row to select. Zero based.</param> | ||
| [UnscopedRef] | ||
| public ref {{@Name rowType}}<T> this[int row] |
There was a problem hiding this comment.
Recommend aggressively inlining to ensure the row parameter is constant folded thereby eliding the switch statement.
| private const float BillboardEpsilon = 1e-4f; | ||
| private const float DecomposeEpsilon = 0.0001f; | ||
|
|
||
| /* |
There was a problem hiding this comment.
Can this commented code be removed now?
| /// <param name="left">The first source matrix.</param> | ||
| /// <param name="right">The second source matrix.</param> | ||
| /// <returns>The result of the multiplication.</returns> | ||
| public static Matrix4X4<T> operator *(Matrix4X4<T> left, Matrix4X4<T> right) => |
There was a problem hiding this comment.
Shouldn't this use System.Numerics? Worth reviewing across the PR.
| /// <summary> | ||
| /// Extensions for <see cref="Quaternion{T}"/>. | ||
| /// </summary> | ||
| public static class Quaternion |
There was a problem hiding this comment.
Trusting you with this file! There's nothing to review here other than maths, so I'm noping out.
| ans.W = Scalar.Add(value1.W, value2.W); | ||
| get | ||
| { | ||
| switch (index) |
| private const float SlerpEpsilon = 1e-6f; | ||
| /// <summary>The 1st row of the matrix represented as a vector.</summary> | ||
| [DataMember] | ||
| public Vector3D<T> Axis; |
There was a problem hiding this comment.
Interesting, was this design decision to store (Vector3D, T) instead of (T, T, T, T) documented anywhere? I'm not sure it makes a difference, it's just interesting...
4 participants
This PR is a continuation of #2459 and supersedes it.
Status snapshot:
Vector structs:
TdefinesIRootFunctions<T>.+,-,*,/, and%operators defined between two vectors of the same type which returns a vector which has had each operation applied component-wise.+,-,*,/, and%operators defined between a vector and a scalar value that matches the generic type which returns a vector which has had each operation applied component-wise with the scalar value. Both vector first and scalar first should be implemented.-unary operator which returns the negated vector.+unary operator which returns the vector.Vector2D<T>toVector2D<float>)Try variants of these methods should also be defined which out the resulting vector and return a bool representing success or failure of the operation.Matrix2Xn,Matrix3Xn, andMatrix4Xn) and return a vector containing the output (type should match the outer type e.g.Vector2D.Transform(Matrix4X4)returnsVector2D)VectorND<T> * MatrixNXMoperators where N is the same for both Vector and Matrix, but M is any numberMatrix2Xn,Matrix3Xn, andMatrix4Xn) and return a vector containing the output (type should match the outer type e.g.Vector2D.Transform(Matrix4X4)returnsVector2D)BitwiseAnd,BitwiseOr, andExclusiveOrstatic methods defined between two vectors which returns a vector which has had these operators applied on a component-wise basis.BitwiseAnd,BitwiseOr, andExclusiveOrstatic methods operators defined between a vectors and a scalar value that matches the generic type which returns a vector which has had these operators applied on a component-wise basis with the scalar.OnesComplementstatic method defined which negates the bits of the vector components.TimplementsIRootFunctions<T>VectorND<int>, where N matches the dimensionality of the vectorint digitsMidpointRounding modeint digits, MidpointRounding modeVectorND<int>, where N matches the dimensionality of the vectorVectorND<int>, where N matches the dimensionality of the vectorVectorND<int>, where N matches the dimensionality of the vectorVectorND<int>, where N matches the dimensionality of the vectorSystem.Runtime.Intrinsics.Vector64<TScalar>System.Runtime.Intrinsics.Vector128<TScalar>System.Runtime.Intrinsics.Vector256<TScalar>System.Runtime.Intrinsics.Vector512<TScalar>Matrix structs must fulfill the following requirements:
IEquatable<T>whereTis the same matrix classSeveral of these functions exist, but not for all types or not for both LH/RH versions.
A Quaternion struct:
Twhich is constrained byINumber<T>, IRootFunctions<T>, ITrigonometricFunctions<T>representing the scalar typeVector3D<T>and a Scalar, with the vector 3 mapping to X, Y, Z and the Scalar to the WVector4D<T>Vector3D<T>Axis property mapping to (X, Y, Z)+,-,*, and/between two Quaternions*withTmultiplying each component by the scalar value returning a new quaternion~Vector3D<T>and an angle and returns a Quaternion representing that rotationVector3D<T>and outputs a Quaternion representing that rotationGeometric Types: