Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/6.0.3xx] [SceneKit] Fix SCNMatrix4 in .NET. Fixes #15094. #15297

Conversation

vs-mobiletools-engineering-service2
Copy link
Collaborator

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

Backport of #15160

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.
* 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 xamarin#15094.
@vs-mobiletools-engineering-service2 vs-mobiletools-engineering-service2 added backported breaking-change If an issue or a pull request represents a breaking change bug If an issue is a bug or a pull request a bug fix dotnet An issue or pull request related to .NET (6) note-highlight Worth calling out specifically in release notes only-dotnet Issue or pull request that only affects .NET labels Jun 17, 2022
@vs-mobiletools-engineering-service2
Copy link
Collaborator Author

📋 [PR Build] API Diff 📋

API Current PR diff

✅ API Diff (from PR only) (no change)

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

Generator diff

Generator Diff (no change)

Pipeline on Agent XAMBOT-1105.Monterey'
Hash: 65244b2f1faa5aa283acd509a8af765581d1ab9b

@vs-mobiletools-engineering-service2
Copy link
Collaborator Author

🔥 [PR Build] Build failed 🔥

Build failed for the job 'Build packages'

Pipeline on Agent
Hash:

@vs-mobiletools-engineering-service2
Copy link
Collaborator Author

📚 [PR Build] Artifacts 📚

Artifacts were not provided.

Pipeline on Agent XAMBOT-1106.Monterey'
Hash: 65244b2f1faa5aa283acd509a8af765581d1ab9b

@vs-mobiletools-engineering-service2
Copy link
Collaborator Author

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1042.Monterey'
Hash: 65244b2f1faa5aa283acd509a8af765581d1ab9b

@vs-mobiletools-engineering-service2
Copy link
Collaborator Author

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

All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: 65244b2f1faa5aa283acd509a8af765581d1ab9b

@vs-mobiletools-engineering-service2
Copy link
Collaborator Author

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

Failed tests are:

  • introspection
  • monotouch-test

Pipeline on Agent
Hash: 65244b2f1faa5aa283acd509a8af765581d1ab9b

@vs-mobiletools-engineering-service2
Copy link
Collaborator Author

✅ [CI Build] Tests passed on VSTS: simulator tests iOS. ✅

Tests passed on VSTS: simulator tests iOS.

🎉 All 148 tests passed 🎉

Pipeline on Agent XAMBOT-1017.Monterey'
Merge 65244b2 into c325398

@rolfbjarne rolfbjarne merged commit 6de3f0c into xamarin:release/6.0.3xx Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported breaking-change If an issue or a pull request represents a breaking change bug If an issue is a bug or a pull request a bug fix dotnet An issue or pull request related to .NET (6) note-highlight Worth calling out specifically in release notes only-dotnet Issue or pull request that only affects .NET
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants