From 23e502fa0efc4db4052ed26ed12e9666fbf51279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20M=C3=BCller?= <34514239+appgurueu@users.noreply.github.com> Date: Sat, 14 Dec 2024 17:02:16 +0100 Subject: [PATCH] Test & document conventions used by `matrix4::setRotation*` (#15542) Also includes a minor `matrix4::transformVect` refactor to make testing easier. --- irr/include/matrix4.h | 47 ++++++++++----------- src/unittest/CMakeLists.txt | 1 + src/unittest/test_irr_matrix4.cpp | 69 +++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 23 deletions(-) create mode 100644 src/unittest/test_irr_matrix4.cpp diff --git a/irr/include/matrix4.h b/irr/include/matrix4.h index 7897b9acb..ed74dc9d2 100644 --- a/irr/include/matrix4.h +++ b/irr/include/matrix4.h @@ -178,9 +178,13 @@ public: CMatrix4 &setInverseTranslation(const vector3d &translation); //! Make a rotation matrix from Euler angles. The 4th row and column are unmodified. + //! NOTE: Rotation order is ZYX. This means that vectors are + //! first rotated around the X, then the Y, and finally the Z axis. + //! NOTE: The rotation is done as per the right-hand rule. + //! See test_irr_matrix4.cpp if you're still unsure about the conventions used here. inline CMatrix4 &setRotationRadians(const vector3d &rotation); - //! Make a rotation matrix from Euler angles. The 4th row and column are unmodified. + //! Same as `setRotationRadians`, but uses degrees. CMatrix4 &setRotationDegrees(const vector3d &rotation); //! Get the rotation, as set by setRotation() when you already know the scale used to create the matrix @@ -236,12 +240,21 @@ public: [[nodiscard]] vector3d rotateAndScaleVect(const vector3d &vect) const; //! Transforms the vector by this matrix - /** This operation is performed as if the vector was 4d with the 4th component =1 */ - void transformVect(vector3df &vect) const; + /** This operation is performed as if the vector was 4d with the 4th component = 1 */ + [[nodiscard]] vector3d transformVect(const vector3d &v) const; + + //! Transforms the vector by this matrix + /** This operation is performed as if the vector was 4d with the 4th component = 1 */ + void transformVect(vector3d &vect) const { + const vector3d &v = vect; + vect = transformVect(v); + } //! Transforms input vector by this matrix and stores result in output vector - /** This operation is performed as if the vector was 4d with the 4th component =1 */ - void transformVect(vector3df &out, const vector3df &in) const; + /** This operation is performed as if the vector was 4d with the 4th component = 1 */ + void transformVect(vector3d &out, const vector3d &in) const { + out = transformVect(in); + } //! An alternate transform vector method, writing into an array of 4 floats /** This operation is performed as if the vector was 4d with the 4th component =1. @@ -1099,25 +1112,13 @@ inline vector3d CMatrix4::scaleThenInvRotVect(const vector3d &v) const } template -inline void CMatrix4::transformVect(vector3df &vect) const +inline vector3d CMatrix4::transformVect(const vector3d &v) const { - T vector[3]; - - vector[0] = vect.X * M[0] + vect.Y * M[4] + vect.Z * M[8] + M[12]; - vector[1] = vect.X * M[1] + vect.Y * M[5] + vect.Z * M[9] + M[13]; - vector[2] = vect.X * M[2] + vect.Y * M[6] + vect.Z * M[10] + M[14]; - - vect.X = static_cast(vector[0]); - vect.Y = static_cast(vector[1]); - vect.Z = static_cast(vector[2]); -} - -template -inline void CMatrix4::transformVect(vector3df &out, const vector3df &in) const -{ - out.X = in.X * M[0] + in.Y * M[4] + in.Z * M[8] + M[12]; - out.Y = in.X * M[1] + in.Y * M[5] + in.Z * M[9] + M[13]; - out.Z = in.X * M[2] + in.Y * M[6] + in.Z * M[10] + M[14]; + return { + v.X * M[0] + v.Y * M[4] + v.Z * M[8] + M[12], + v.X * M[1] + v.Y * M[5] + v.Z * M[9] + M[13], + v.X * M[2] + v.Y * M[6] + v.Z * M[10] + M[14], + }; } template diff --git a/src/unittest/CMakeLists.txt b/src/unittest/CMakeLists.txt index 2b70a6918..b7ca17199 100644 --- a/src/unittest/CMakeLists.txt +++ b/src/unittest/CMakeLists.txt @@ -53,6 +53,7 @@ set (UNITTEST_CLIENT_SRCS ${CMAKE_CURRENT_SOURCE_DIR}/test_eventmanager.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_gameui.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_irr_gltf_mesh_loader.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test_irr_matrix4.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_mesh_compare.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_keycode.cpp PARENT_SCOPE) diff --git a/src/unittest/test_irr_matrix4.cpp b/src/unittest/test_irr_matrix4.cpp new file mode 100644 index 000000000..6272c8c26 --- /dev/null +++ b/src/unittest/test_irr_matrix4.cpp @@ -0,0 +1,69 @@ +// Luanti +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "catch.h" +#include "irrMath.h" +#include "matrix4.h" +#include "irr_v3d.h" + +using matrix4 = core::matrix4; + +static bool matrix_equals(const matrix4 &a, const matrix4 &b) { + return a.equals(b, 0.00001f); +} + +constexpr v3f x{1, 0, 0}; +constexpr v3f y{0, 1, 0}; +constexpr v3f z{0, 0, 1}; + +TEST_CASE("matrix4") { + +SECTION("setRotationRadians") { + SECTION("rotation order is ZYX (matrix notation)") { + v3f rot{1, 2, 3}; + matrix4 X, Y, Z, ZYX; + X.setRotationRadians({rot.X, 0, 0}); + Y.setRotationRadians({0, rot.Y, 0}); + Z.setRotationRadians({0, 0, rot.Z}); + ZYX.setRotationRadians(rot); + CHECK(!matrix_equals(X * Y * Z, ZYX)); + CHECK(!matrix_equals(X * Z * Y, ZYX)); + CHECK(!matrix_equals(Y * X * Z, ZYX)); + CHECK(!matrix_equals(Y * Z * X, ZYX)); + CHECK(!matrix_equals(Z * X * Y, ZYX)); + CHECK(matrix_equals(Z * Y * X, ZYX)); + } + + const f32 quarter_turn = core::PI / 2; + + // See https://en.wikipedia.org/wiki/Right-hand_rule#/media/File:Cartesian_coordinate_system_handedness.svg + // for a visualization of what handedness means for rotations + + SECTION("rotation is right-handed") { + SECTION("rotation around the X-axis is Z-up, counter-clockwise") { + matrix4 X; + X.setRotationRadians({quarter_turn, 0, 0}); + CHECK(X.transformVect(x).equals(x)); + CHECK(X.transformVect(y).equals(z)); + CHECK(X.transformVect(z).equals(-y)); + } + + SECTION("rotation around the Y-axis is Z-up, clockwise") { + matrix4 Y; + Y.setRotationRadians({0, quarter_turn, 0}); + CHECK(Y.transformVect(y).equals(y)); + CHECK(Y.transformVect(x).equals(-z)); + CHECK(Y.transformVect(z).equals(x)); + } + + SECTION("rotation around the Z-axis is Y-up, counter-clockwise") { + matrix4 Z; + Z.setRotationRadians({0, 0, quarter_turn}); + CHECK(Z.transformVect(z).equals(z)); + CHECK(Z.transformVect(x).equals(y)); + CHECK(Z.transformVect(y).equals(-x)); + } + } +} + +}