Test & document conventions used by matrix4::setRotation* (#15542)

Also includes a minor `matrix4::transformVect` refactor to make testing easier.

irr/include/matrix4.h

@@ -178,9 +178,13 @@ public:
 	CMatrix4<T> &setInverseTranslation(const vector3d<T> &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<T> &setRotationRadians(const vector3d<T> &rotation);
 
-	//! Make a rotation matrix from Euler angles. The 4th row and column are unmodified.
+	//! Same as `setRotationRadians`, but uses degrees.
 	CMatrix4<T> &setRotationDegrees(const vector3d<T> &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<T> rotateAndScaleVect(const vector3d<T> &vect) const;
 
 	//! Transforms the vector by this matrix
-	/** This operation is performed as if the vector was 4d with the 4th component =1 */
+	/** This operation is performed as if the vector was 4d with the 4th component = 1 */
-	void transformVect(vector3df &vect) const;
+	[[nodiscard]] vector3d<T> transformVect(const vector3d<T> &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<T> &vect) const {
+		const vector3d<T> &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 */
+	/** This operation is performed as if the vector was 4d with the 4th component = 1 */
-	void transformVect(vector3df &out, const vector3df &in) const;
+	void transformVect(vector3d<T> &out, const vector3d<T> &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<T> CMatrix4<T>::scaleThenInvRotVect(const vector3d<T> &v) const
 }
 
 template <class T>
-inline void CMatrix4<T>::transformVect(vector3df &vect) const
+inline vector3d<T> CMatrix4<T>::transformVect(const vector3d<T> &v) const
 {
-	T vector[3];
+	return {
-
+		v.X * M[0] + v.Y * M[4] + v.Z * M[8] + M[12],
-	vector[0] = vect.X * M[0] + vect.Y * M[4] + vect.Z * M[8] + M[12];
+		v.X * M[1] + v.Y * M[5] + v.Z * M[9] + M[13],
-	vector[1] = vect.X * M[1] + vect.Y * M[5] + vect.Z * M[9] + M[13];
+		v.X * M[2] + v.Y * M[6] + v.Z * M[10] + M[14],
-	vector[2] = vect.X * M[2] + vect.Y * M[6] + vect.Z * M[10] + M[14];
+	};
-
-	vect.X = static_cast<f32>(vector[0]);
-	vect.Y = static_cast<f32>(vector[1]);
-	vect.Z = static_cast<f32>(vector[2]);
-}
-
-template <class T>
-inline void CMatrix4<T>::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];
 }
 
 template <class T>

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)

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));
+        }
+    }
+}
+
+}