Skip to content

Commit

Permalink
[SceneKit] Fix SCNMatrix4 in .NET. Fixes #15094. (#15160)
Browse files Browse the repository at this point in the history 
When we changed SCNMatrix4 to be column-major instead of row-major in .NET, there
were several other related changes we should have done but didn't do. In particular
we should have made transformation operations based on column-vectors instead of
row-vectors.

In legacy Xamarin, a vector would be transformed by a transformation matrix by doing
matrix multiplication like this:

    [ x y z w] * [ 11 21 31 41 ]
                 | 12 22 32 42 |
                 | 13 23 33 43 |
                 [ 14 24 34 41 ]

In this case the vector is a row-vector, and it's the left operand in the multiplication.
When using column-major matrices, we want to use column-vectors, where the vector
is the right operand, like this:

    [ 11 21 31 41 ] * [ x ]
    | 12 22 32 42 |   | y |
    | 13 23 33 43 |   | z |
    [ 14 24 34 41 ]   [ w ]

This affects numerous APIs in SCNMatrix4, SCNVector3 and SCNVector4:

* The M## fields have been changed to make the first number the column and the
  second number the row, to reflect that it's a column-major matrix (this is
  also how it's defined in the native SCNMatrix4 type).
* Functions that return a transformation matrix have been modified to return column-vector
  transformers. Technically this means that these matrices are transposed compared
  to legacy Xamarin. The functions involved are:
    * CreateFromAxisAngle
    * CreateRotation[X|Y|Z]
    * CreateTranslation
    * CreatePerspectiveFieldOfView
    * CreatePerspectiveOffCenter
    * Rotate
    * LookAt
* Combining two column-vector transforming transformation matrices is done by multiplying
  them in the reverse order, so the Mult function (and the multiplication operator)
  have been modified to multiply the given matrices in the opposite order (this matches
  how the SCNMatrix4Mult function does it). To make things clearer I've changed the
  parameter names for XAMCORE_5_0.
* Functions that transform a vector using a transformation matrix have been modified
  to do a column-vector transformation instead of a row-vector transformation. This
  involves the following functions:
    * SCNVector3.TransformVector
    * SCNVector3.TransformNormal
    * SCNVector3.TransformNormalInverse
    * SCNVector3.TransformPosition
    * SCNVector4.Transform
* Numerous new tests.

Fixes #15094.
  • Loading branch information
@rolfbjarne
rolfbjarne authored Jun 17, 2022
1 parent 893cd77 commit 42c1c66
Show file tree
Hide file tree
Showing 14 changed files with 1,772 additions and 372 deletions.
401 changes: 259 additions & 142 deletions src/SceneKit/SCNMatrix4_dotnet.cs
View file

Large diffs are not rendered by default.

133 changes: 105 additions & 28 deletions src/SceneKit/SCNVector3.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,29 +740,53 @@ public static void BaryCentric(ref SCNVector3 a, ref SCNVector3 b, ref SCNVector

#region Transform

#if NET
/// <summary>Transform a direction vector by the given Matrix
/// Assumes the matrix has a right-most column of (0,0,0,1), that is the translation part is ignored.
/// </summary>
/// <param name="vec">The column vector to transform</param>
#else
/// <summary>Transform a direction vector by the given Matrix
/// Assumes the matrix has a bottom row of (0,0,0,1), that is the translation part is ignored.
/// </summary>
/// <param name="vec">The vector to transform</param>
/// <param name="vec">The row vector to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <returns>The transformed vector</returns>
public static SCNVector3 TransformVector(SCNVector3 vec, SCNMatrix4 mat)
{
SCNVector3 v;
v.X = SCNVector3.Dot(vec, new SCNVector3(mat.Column0));
v.Y = SCNVector3.Dot(vec, new SCNVector3(mat.Column1));
v.Z = SCNVector3.Dot(vec, new SCNVector3(mat.Column2));
TransformVector (ref vec, ref mat, out var v);
return v;
}

#if NET
/// <summary>Transform a direction vector by the given Matrix
/// Assumes the matrix has a right-most column of (0,0,0,1), that is the translation part is ignored.
/// </summary>
/// <param name="vec">The column vector to transform</param>
#else
/// <summary>Transform a direction vector by the given Matrix
/// Assumes the matrix has a bottom row of (0,0,0,1), that is the translation part is ignored.
/// </summary>
/// <param name="vec">The vector to transform</param>
/// <param name="vec">The row vector to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <param name="result">The transformed vector</param>
public static void TransformVector(ref SCNVector3 vec, ref SCNMatrix4 mat, out SCNVector3 result)
{
#if NET
result.X = vec.X * mat.Row0.X +
vec.Y * mat.Row0.Y +
vec.Z * mat.Row0.Z;

result.Y = vec.X * mat.Row1.X +
vec.Y * mat.Row1.Y +
vec.Z * mat.Row1.Z;

result.Z = vec.X * mat.Row2.X +
vec.Y * mat.Row2.Y +
vec.Z * mat.Row2.Z;
#else
result.X = vec.X * mat.Row0.X +
vec.Y * mat.Row1.X +
vec.Z * mat.Row2.X;
Expand All @@ -774,14 +798,19 @@ public static void TransformVector(ref SCNVector3 vec, ref SCNMatrix4 mat, out S
result.Z = vec.X * mat.Row0.Z +
vec.Y * mat.Row1.Z +
vec.Z * mat.Row2.Z;
#endif
}

/// <summary>Transform a Normal by the given Matrix</summary>
/// <remarks>
/// This calculates the inverse of the given matrix, use TransformNormalInverse if you
/// already have the inverse to avoid this extra calculation
/// </remarks>
/// <param name="norm">The normal to transform</param>
#if NET
/// <param name="norm">The column-based normal to transform</param>
#else
/// <param name="norm">The row-based normal to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <returns>The transformed normal</returns>
public static SCNVector3 TransformNormal(SCNVector3 norm, SCNMatrix4 mat)
Expand All @@ -795,7 +824,11 @@ public static SCNVector3 TransformNormal(SCNVector3 norm, SCNMatrix4 mat)
/// This calculates the inverse of the given matrix, use TransformNormalInverse if you
/// already have the inverse to avoid this extra calculation
/// </remarks>
/// <param name="norm">The normal to transform</param>
#if NET
/// <param name="norm">The column-based normal to transform</param>
#else
/// <param name="norm">The row-based normal to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <param name="result">The transformed normal</param>
public static void TransformNormal(ref SCNVector3 norm, ref SCNMatrix4 mat, out SCNVector3 result)
Expand All @@ -809,15 +842,16 @@ public static void TransformNormal(ref SCNVector3 norm, ref SCNMatrix4 mat, out
/// This version doesn't calculate the inverse matrix.
/// Use this version if you already have the inverse of the desired transform to hand
/// </remarks>
/// <param name="norm">The normal to transform</param>
#if NET
/// <param name="norm">The column-based normal to transform</param>
#else
/// <param name="norm">The row-based normal to transform</param>
#endif
/// <param name="invMat">The inverse of the desired transformation</param>
/// <returns>The transformed normal</returns>
public static SCNVector3 TransformNormalInverse(SCNVector3 norm, SCNMatrix4 invMat)
{
SCNVector3 n;
n.X = SCNVector3.Dot(norm, new SCNVector3(invMat.Row0));
n.Y = SCNVector3.Dot(norm, new SCNVector3(invMat.Row1));
n.Z = SCNVector3.Dot(norm, new SCNVector3(invMat.Row2));
TransformNormalInverse (ref norm, ref invMat, out var n);
return n;
}

Expand All @@ -826,11 +860,28 @@ public static SCNVector3 TransformNormalInverse(SCNVector3 norm, SCNMatrix4 invM
/// This version doesn't calculate the inverse matrix.
/// Use this version if you already have the inverse of the desired transform to hand
/// </remarks>
/// <param name="norm">The normal to transform</param>
#if NET
/// <param name="norm">The column-based normal to transform</param>
#else
/// <param name="norm">The row-based normal to transform</param>
#endif
/// <param name="invMat">The inverse of the desired transformation</param>
/// <param name="result">The transformed normal</param>
public static void TransformNormalInverse(ref SCNVector3 norm, ref SCNMatrix4 invMat, out SCNVector3 result)
{
#if NET
result.X = norm.X * invMat.Column0.X +
norm.Y * invMat.Column0.Y +
norm.Z * invMat.Column0.Z;

result.Y = norm.X * invMat.Column1.X +
norm.Y * invMat.Column1.Y +
norm.Z * invMat.Column1.Z;

result.Z = norm.X * invMat.Column2.X +
norm.Y * invMat.Column2.Y +
norm.Z * invMat.Column2.Z;
#else
result.X = norm.X * invMat.Row0.X +
norm.Y * invMat.Row0.Y +
norm.Z * invMat.Row0.Z;
Expand All @@ -842,27 +893,49 @@ public static void TransformNormalInverse(ref SCNVector3 norm, ref SCNMatrix4 in
result.Z = norm.X * invMat.Row2.X +
norm.Y * invMat.Row2.Y +
norm.Z * invMat.Row2.Z;
#endif
}

/// <summary>Transform a Position by the given Matrix</summary>
/// <param name="pos">The position to transform</param>
#if NET
/// <param name="pos">The column-based position to transform</param>
#else
/// <param name="pos">The row-based position to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <returns>The transformed position</returns>
public static SCNVector3 TransformPosition(SCNVector3 pos, SCNMatrix4 mat)
{
SCNVector3 p;
p.X = SCNVector3.Dot(pos, new SCNVector3(mat.Column0)) + mat.Row3.X;
p.Y = SCNVector3.Dot(pos, new SCNVector3(mat.Column1)) + mat.Row3.Y;
p.Z = SCNVector3.Dot(pos, new SCNVector3(mat.Column2)) + mat.Row3.Z;
TransformPosition (ref pos, ref mat, out var p);
return p;
}

/// <summary>Transform a Position by the given Matrix</summary>
/// <param name="pos">The position to transform</param>
#if NET
/// <param name="pos">The column-based position to transform</param>
#else
/// <param name="pos">The row-based position to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <param name="result">The transformed position</param>
public static void TransformPosition(ref SCNVector3 pos, ref SCNMatrix4 mat, out SCNVector3 result)
{
#if NET
result.X = mat.Row0.X * pos.X +
mat.Row0.Y * pos.Y +
mat.Row0.Z * pos.Z +
mat.Row0.W;

result.Y = mat.Row1.X * pos.X +
mat.Row1.Y * pos.Y +
mat.Row1.Z * pos.Z +
mat.Row1.W;

result.Z = mat.Row2.X * pos.X +
mat.Row2.Y * pos.Y +
mat.Row2.Z * pos.Z +
mat.Row2.W;
#else
result.X = pos.X * mat.Row0.X +
pos.Y * mat.Row1.X +
pos.Z * mat.Row2.X +
Expand All @@ -877,25 +950,29 @@ public static void TransformPosition(ref SCNVector3 pos, ref SCNMatrix4 mat, out
pos.Y * mat.Row1.Z +
pos.Z * mat.Row2.Z +
mat.Row3.Z;
#endif
}

/// <summary>Transform a Vector by the given Matrix</summary>
/// <param name="vec">The vector to transform</param>
#if NET
/// <param name="vec">The column vector to transform</param>
#else
/// <param name="vec">The row vector to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <returns>The transformed vector</returns>
public static SCNVector4 Transform(SCNVector3 vec, SCNMatrix4 mat)
{
SCNVector4 v4 = new SCNVector4(vec.X, vec.Y, vec.Z, 1.0f);
SCNVector4 result;
result.X = SCNVector4.Dot(v4, mat.Column0);
result.Y = SCNVector4.Dot(v4, mat.Column1);
result.Z = SCNVector4.Dot(v4, mat.Column2);
result.W = SCNVector4.Dot(v4, mat.Column3);
return result;
return SCNVector4.Transform (v4, mat);
}

/// <summary>Transform a Vector by the given Matrix</summary>
/// <param name="vec">The vector to transform</param>
#if NET
/// <param name="vec">The column vector to transform</param>
#else
/// <param name="vec">The row vector to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <param name="result">The transformed vector</param>
public static void Transform(ref SCNVector3 vec, ref SCNMatrix4 mat, out SCNVector4 result)
Expand Down
42 changes: 34 additions & 8 deletions src/SceneKit/SCNVector4.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -843,25 +843,50 @@ public static void BaryCentric(ref SCNVector4 a, ref SCNVector4 b, ref SCNVector
#region Transform

/// <summary>Transform a Vector by the given Matrix</summary>
/// <param name="vec">The vector to transform</param>
#if NET
/// <param name="vec">The column vector to transform</param>
#else
/// <param name="vec">The row vector to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <returns>The transformed vector</returns>
public static SCNVector4 Transform(SCNVector4 vec, SCNMatrix4 mat)
{
SCNVector4 result;
result.X = SCNVector4.Dot(vec, mat.Column0);
result.Y = SCNVector4.Dot(vec, mat.Column1);
result.Z = SCNVector4.Dot(vec, mat.Column2);
result.W = SCNVector4.Dot(vec, mat.Column3);
Transform(ref vec, ref mat, out var result);
return result;
}

/// <summary>Transform a Vector by the given Matrix</summary>
/// <param name="vec">The vector to transform</param>
/// <summary>Transform a Vector by the given Matrix.</summary>
#if NET
/// <param name="vec">The column vector to transform</param>
#else
/// <param name="vec">The row vector to transform</param>
#endif
/// <param name="mat">The desired transformation</param>
/// <param name="result">The transformed vector</param>
public static void Transform(ref SCNVector4 vec, ref SCNMatrix4 mat, out SCNVector4 result)
{
#if NET
result.X = vec.X * mat.Column0.X +
vec.Y * mat.Column1.X +
vec.Z * mat.Column2.X +
vec.W * mat.Column3.X;

result.Y = vec.X * mat.Column0.Y +
vec.Y * mat.Column1.Y +
vec.Z * mat.Column2.Y +
vec.W * mat.Column3.Y;

result.Z = vec.X * mat.Column0.Z +
vec.Y * mat.Column1.Z +
vec.Z * mat.Column2.Z +
vec.W * mat.Column3.Z;

result.W = vec.X * mat.Column0.W +
vec.Y * mat.Column1.W +
vec.Z * mat.Column2.W +
vec.W * mat.Column3.W;
#else
result.X = vec.X * mat.Row0.X +
vec.Y * mat.Row1.X +
vec.Z * mat.Row2.X +
Expand All @@ -881,6 +906,7 @@ public static void Transform(ref SCNVector4 vec, ref SCNMatrix4 mat, out SCNVect
vec.Y * mat.Row1.W +
vec.Z * mat.Row2.W +
vec.W * mat.Row3.W;
#endif
}

#endregion
Expand Down
20 changes: 20 additions & 0 deletions tests/bindings-test/StructsAndEnums.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using Foundation;
using ObjCRuntime;
using SceneKit;

#if NET
using MatrixFloat2x2 = global::CoreGraphics.NMatrix2;
Expand All @@ -16,6 +17,16 @@
using MatrixFloat4x4 = global::OpenTK.NMatrix4;
#endif

#if __MACOS__
#if NET
using pfloat = System.Runtime.InteropServices.NFloat;
#else
using pfloat = System.nfloat;
#endif
#else
using pfloat = System.Single;
#endif

public static class LibTest {
[DllImport ("__Internal")]
public static extern int theUltimateAnswer ();
Expand Down Expand Up @@ -126,5 +137,14 @@ public static MatrixFloat4x4 MDLTransform_GetRotationMatrix (INativeObject obj,
r3c0, r3c1, r3c2, r3c3);
}
#endif

[DllImport ("__Internal")]
public static extern SCNMatrix4 x_SCNMatrix4MakeTranslation (pfloat tx, pfloat ty, pfloat tz);

[DllImport ("__Internal")]
public static extern SCNMatrix4 x_SCNMatrix4MakeScale (pfloat tx, pfloat ty, pfloat tz);

[DllImport ("__Internal")]
public static extern SCNMatrix4 x_SCNMatrix4Translate (SCNMatrix4 m, pfloat tx, pfloat ty, pfloat tz);
}
}
Loading

5 comments on commit 42c1c66

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ [CI Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌

Failed tests are:

  • monotouch-test

Pipeline on Agent
Hash: 42c1c66a142e4f2deed095f3f91fa2da4dfa6d12 [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💻 [CI Build] Tests on macOS Mac Catalina (10.15) passed 💻

✅ All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: 42c1c66a142e4f2deed095f3f91fa2da4dfa6d12 [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📚 [CI Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMMINI-051.Monterey'
Hash: 42c1c66a142e4f2deed095f3f91fa2da4dfa6d12 [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ [CI Build] Tests failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

7 tests failed, 227 tests passed.

Failed tests

  • fsharp/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemXmlLinqTests/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemWebServicesTests/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono MicrosoftCSharpXunit/watchOS 32-bits - simulator/Debug: Crashed
  • mscorlib Part 2/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemCoreXunit Part 1/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemXunit/watchOS 32-bits - simulator/Debug: Crashed

Pipeline on Agent XAMBOT-1023.Monterey'
[SceneKit] Fix SCNMatrix4 in .NET. Fixes #15094. (#15160)

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Status for 'xamarin-macios - sample testing (build)': failure.

  • ❌ Debug_iPhone_AF: Failed
  • ❌ Debug_iPhone_GR: Failed
  • ❌ Debug_iPhone_SZ: Failed
  • ❌ Debug_iPhoneSimulator: Failed
  • ❌ Release_iPhone_AF: Failed
  • ❌ Release_iPhone_GR: Failed
  • ❌ Release_iPhone_SZ: Failed
  • ❌ Release_iPhoneSimulator: Failed
  • ❌ Debug_Mac: Failed
  • ❌ Release_Mac: Failed
  • ❌ PublishPerformanceData: Failed

Please sign in to comment.