dotnet / dotnet-api-docs

.NET API reference documentation (.NET 5+, .NET Core, .NET Framework)
https://docs.microsoft.com/dotnet/api/
Other
736 stars 1.57k forks source link

Clarify the Matrix3x2 docs to indicate that it is supposed to be used for 2d operations. #8884

Open synercoder opened 1 year ago

synercoder commented 1 year ago

The current docs for Matrix3x2 have little to no "leading" info or remarks about the type. I would love to see here that the Matrix3x2 is meant to be used for 2d operations, and that this matrix is actually a 3x3 matrix of which the last column is always [0 0 1].

In the remarks of the GetDeterminant method, it is stated that the method treats the matrix as a 3x3, but the way the remark is made, it seems that this only applies to the GetDeterminant method.

When working in 2d space, you can chain multiple matrix operations together into a single matrix, by multiplying the matrixes. Multiplying matrixes is therefore a very important operation, so when I took a look at the source of the multiply method, it showed me that this code also treats the matrix as a 3x3 matrix with the last column as [0 0 1].

If the Matrix3x2 was truly a 3x2 matrix, the operator * should (NO NO NO, I AM NOT SUGGESTING WE DO THIS XD) instead be:

            [MethodImpl(MethodImplOptions.AggressiveInlining)]
            public static Impl operator *(in Impl left, in Impl right)
            {
                Impl result;

                result.X = new Vector2(
                    left.X.X * right.X.X + left.X.Y * right.Y.X,
                    left.X.X * right.X.Y + left.X.Y * right.Y.Y
                );
                result.Y = new Vector2(
                    left.Y.X * right.X.X + left.Y.Y * right.Y.X,
                    left.Y.X * right.X.Y + left.Y.Y * right.Y.Y
                );
                result.Z = new Vector2(
-                    left.Z.X * right.X.X + left.Z.Y * right.Y.X + right.Z.X,
-                    left.Z.X * right.X.Y + left.Z.Y * right.Y.Y + right.Z.Y
+                    left.Z.X * right.X.X + left.Z.Y * right.Y.X,
+                    left.Z.X * right.X.Y + left.Z.Y * right.Y.Y
                );

                return result;
            }

But this would invalidate all the use cases for 2d space, since the calculations for multiplying matrixes for 2d use are then incorrect.

So that is why I would love to see some documentation indicating that this is what the class should be used for, and a general remark that the Matrix works as a 3x3 matrix with a default last column of [0 0 1].

I would like to hear if I understood the code & it's use correctly, or what the thinking here is. If need be, I can create a PR addressing as such in the near future.

dotnet-issue-labeler[bot] commented 1 year ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

synercoder commented 1 year ago

No write access, but https://github.com/dotnet/dotnet-api-docs/labels/area-System.Numerics for those that do have write permissions.

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-numerics See info in area-owners.md if you want to be subscribed.

Issue Details
The current docs for Matrix3x2 have little to no "leading" info or remarks about the type. I would love to see here that the Matrix3x2 is meant to be used for 2d operations, and that this matrix is actually a 3x3 matrix of which the last column is always [0 0 1]. In [the remarks](https://github.com/dotnet/runtime/blob/f8218f9c4aaf4d6725524dc5d9e14c985e0f1b51/src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.cs#L327) of the `GetDeterminant` method, it is stated that the method treats the matrix as a 3x3, but the way the remark is made, it seems that this only applies to the `GetDeterminant` method. When working in 2d space, you can chain multiple matrix operations together into a single matrix, by multiplying the matrixes. Multiplying matrixes is therefore a very important operation, so when I took a look at the source of the multiply method, it showed me that this code also treats the matrix as a 3x3 matrix with the last column as [0 0 1]. If the Matrix3x2 was truly a 3x2 matrix, the [operator *](https://github.com/dotnet/runtime/blob/f8218f9c4aaf4d6725524dc5d9e14c985e0f1b51/src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix3x2.Impl.cs#L145) should (NO NO NO, I AM NOT SUGGESTING WE DO THIS XD) instead be: ```diff [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Impl operator *(in Impl left, in Impl right) { Impl result; result.X = new Vector2( left.X.X * right.X.X + left.X.Y * right.Y.X, left.X.X * right.X.Y + left.X.Y * right.Y.Y ); result.Y = new Vector2( left.Y.X * right.X.X + left.Y.Y * right.Y.X, left.Y.X * right.X.Y + left.Y.Y * right.Y.Y ); result.Z = new Vector2( - left.Z.X * right.X.X + left.Z.Y * right.Y.X + right.Z.X, - left.Z.X * right.X.Y + left.Z.Y * right.Y.Y + right.Z.Y + left.Z.X * right.X.X + left.Z.Y * right.Y.X, + left.Z.X * right.X.Y + left.Z.Y * right.Y.Y ); return result; } ``` But this would invalidate all the use cases for 2d space, since the calculations for multiplying matrixes for 2d use are then incorrect. So that is why I would love to see some documentation indicating that this is what the class should be used for, and a general remark that the Matrix works as a 3x3 matrix with a default last column of [0 0 1]. I would like to hear if I understood the code & it's use correctly, or what the thinking here is. If need be, I can create a PR addressing as such in the near future.
Author: synercoder
Assignees: -
Labels: `untriaged`, `Pri3`, `area-System.Numerics`
Milestone: -