diff --git a/irr/include/matrix4.h b/irr/include/matrix4.h index efb727098f359..40fb41e570fe5 100644 --- a/irr/include/matrix4.h +++ b/irr/include/matrix4.h @@ -179,9 +179,7 @@ class CMatrix4 CMatrix4 &setRotationDegrees(const vector3d &rotation); //! Get the rotation, as set by setRotation() when you already know the scale used to create the matrix - /** NOTE: The scale needs to be the correct one used to create this matrix. - You can _not_ use the result of getScale(), but have to save your scale - variable in another place (like ISceneNode does). + /** NOTE: No scale value can be 0 or the result is undefined. NOTE: It does not necessarily return the *same* Euler angles as those set by setRotationDegrees(), but the rotation will be equivalent, i.e. will have the same result when used to rotate a vector or node. @@ -189,15 +187,17 @@ class CMatrix4 WARNING: There have been troubles with this function over the years and we may still have missed some corner cases. It's generally safer to keep the rotation and scale you used to create the matrix around and work with those. */ - vector3d getRotationDegrees(const vector3d &scale) const; + vector3d getRotationRadians(const vector3d &scale) const; //! Returns the rotation, as set by setRotation(). /** NOTE: You will have the same end-rotation as used in setRotation, but it might not use the same axis values. - NOTE: This only works correct if no other matrix operations have been done on the inner 3x3 matrix besides - setting rotation (so no scale/shear). Thought it (probably) works as long as scale doesn't flip handedness. + NOTE: This only works correctly for TRS matrix products where S is a positive, component-wise scaling (see setScale). NOTE: It does not necessarily return the *same* Euler angles as those set by setRotationDegrees(), - but the rotation will be equivalent, i.e. will have the same result when used to rotate a vector or node. + but the rotation will be equivalent, i.e. will have the same result when used to rotate a vector or node. */ + vector3d getRotationRadians() const; + + //! Same as getRotationRadians, but returns degrees. vector3d getRotationDegrees() const; //! Make a rotation matrix from angle and axis, assuming left handed rotation. @@ -425,6 +425,9 @@ class CMatrix4 bool equals(const CMatrix4 &other, const T tolerance = (T)ROUNDING_ERROR_f64) const; private: + template + vector3d getRotation(const vector3d &scale) const; + //! Matrix data, stored in row-major order T M[16]; }; @@ -779,63 +782,60 @@ inline CMatrix4 &CMatrix4::setRotationRadians(const vector3d &rotation) return *this; } -//! Returns a rotation which (mostly) works in combination with the given scale -/** -This code was originally written by by Chev (assuming no scaling back then, -we can be blamed for all problems added by regarding scale) -*/ template -inline vector3d CMatrix4::getRotationDegrees(const vector3d &scale_) const +template +inline vector3d CMatrix4::getRotation(const vector3d &scale_) const { + // Based on code by Chev const CMatrix4 &mat = *this; const vector3d scale(iszero(scale_.X) ? FLT_MAX : scale_.X, iszero(scale_.Y) ? FLT_MAX : scale_.Y, iszero(scale_.Z) ? FLT_MAX : scale_.Z); const vector3d invScale(reciprocal(scale.X), reciprocal(scale.Y), reciprocal(scale.Z)); - f64 Y = -asin(clamp(mat[2] * invScale.X, -1.0, 1.0)); - const f64 C = cos(Y); - Y *= RADTODEG64; + f64 a = clamp(mat[2] * invScale.X, -1.0, 1.0); + f64 Y = -asin(a); f64 rotx, roty, X, Z; - if (!iszero((T)C)) { - const f64 invC = reciprocal(C); - rotx = mat[10] * invC * invScale.Z; - roty = mat[6] * invC * invScale.Y; - X = atan2(roty, rotx) * RADTODEG64; - rotx = mat[0] * invC * invScale.X; - roty = mat[1] * invC * invScale.X; - Z = atan2(roty, rotx) * RADTODEG64; + if (!core::equals(std::abs(a), 1.0)) { + // abs(a) = abs(sin(Y)) = 1 <=> cos(Y) = 0 + rotx = mat[10] * invScale.Z; + roty = mat[6] * invScale.Y; + X = atan2(roty, rotx); + rotx = mat[0] * invScale.X; + roty = mat[1] * invScale.X; + Z = atan2(roty, rotx); } else { X = 0.0; - rotx = mat[5] * invScale.Y; - roty = -mat[4] * invScale.Y; - Z = atan2(roty, rotx) * RADTODEG64; + rotx = mat[5]; + roty = -mat[4]; + Z = atan2(roty, rotx); } - // fix values that get below zero - if (X < 0.0) - X += 360.0; - if (Y < 0.0) - Y += 360.0; - if (Z < 0.0) - Z += 360.0; + if (degrees) { + X *= core::RADTODEG64; + Y *= core::RADTODEG64; + Z *= core::RADTODEG64; + } return vector3d((T)X, (T)Y, (T)Z); } -//! Returns a rotation that is equivalent to that set by setRotationDegrees(). template -inline vector3d CMatrix4::getRotationDegrees() const +inline vector3d CMatrix4::getRotationRadians(const vector3d &scale) const { - // Note: Using getScale() here make it look like it could do matrix decomposition. - // It can't! It works (or should work) as long as rotation doesn't flip the handedness - // aka scale swapping 1 or 3 axes. (I think we could catch that as well by comparing - // crossproduct of first 2 axes to direction of third axis, but TODO) - // And maybe it should also offer the solution for the simple calculation - // without regarding scaling as Irrlicht did before 1.7 - vector3d scale(getScale()); + return getRotation(scale); +} - return getRotationDegrees(scale); +template +inline vector3d CMatrix4::getRotationRadians() const +{ + return getRotationRadians(getScale()); +} + +template +inline vector3d CMatrix4::getRotationDegrees() const +{ + return getRotation(getScale()); } //! Sets matrix to rotation matrix defined by axis and angle, assuming LH rotation diff --git a/irr/src/CAnimatedMeshSceneNode.cpp b/irr/src/CAnimatedMeshSceneNode.cpp index 6facfcd069a7a..09d83038b77ed 100644 --- a/irr/src/CAnimatedMeshSceneNode.cpp +++ b/irr/src/CAnimatedMeshSceneNode.cpp @@ -619,7 +619,7 @@ void CAnimatedMeshSceneNode::animateJoints(bool CalculateAbsolutePositions) // Code is slow, needs to be fixed up - const core::quaternion RotationStart(PretransitingSave[n].getRotationDegrees() * core::DEGTORAD); + const core::quaternion RotationStart(PretransitingSave[n].getRotationRadians()); const core::quaternion RotationEnd(JointChildSceneNodes[n]->getRotation() * core::DEGTORAD); core::quaternion QRotation; diff --git a/irr/src/CGLTFMeshFileLoader.cpp b/irr/src/CGLTFMeshFileLoader.cpp index 4e653fbf864cc..b32ba5692bbd1 100644 --- a/irr/src/CGLTFMeshFileLoader.cpp +++ b/irr/src/CGLTFMeshFileLoader.cpp @@ -546,16 +546,6 @@ void SelfType::MeshExtractor::deferAddMesh( }); } -// Base transformation between left & right handed coordinate systems. -// This just inverts the Z axis. -static const core::matrix4 leftToRight = core::matrix4( - 1, 0, 0, 0, - 0, 1, 0, 0, - 0, 0, -1, 0, - 0, 0, 0, 1 -); -static const core::matrix4 rightToLeft = leftToRight; - static core::matrix4 loadTransform(const tiniergltf::Node::Matrix &m, SkinnedMesh::SJoint *joint) { // Note: Under the hood, this casts these doubles to floats. @@ -570,14 +560,7 @@ static core::matrix4 loadTransform(const tiniergltf::Node::Matrix &m, SkinnedMes auto scale = mat.getScale(); joint->Animatedscale = scale; - core::matrix4 inverseScale; - inverseScale.setScale(core::vector3df( - scale.X == 0 ? 0 : 1 / scale.X, - scale.Y == 0 ? 0 : 1 / scale.Y, - scale.Z == 0 ? 0 : 1 / scale.Z)); - - core::matrix4 axisNormalizedMat = inverseScale * mat; - joint->Animatedrotation = axisNormalizedMat.getRotationDegrees(); + joint->Animatedrotation = mat.getRotationRadians(scale); // Invert the rotation because it is applied using `getMatrix_transposed`, // which again inverts. joint->Animatedrotation.makeInverse(); diff --git a/irr/src/CXMeshFileLoader.cpp b/irr/src/CXMeshFileLoader.cpp index c09f2e4815c5e..ed8c18350c018 100644 --- a/irr/src/CXMeshFileLoader.cpp +++ b/irr/src/CXMeshFileLoader.cpp @@ -1526,8 +1526,6 @@ bool CXMeshFileLoader::parseDataObjectAnimationKey(SkinnedMesh::SJoint *joint) os::Printer::log("Line", core::stringc(Line).c_str(), ELL_WARNING); } - // core::vector3df rotation = mat.getRotationDegrees(); - AnimatedMesh->addRotationKey(joint, time, core::quaternion(mat.getTransposed())); AnimatedMesh->addPositionKey(joint, time, mat.getTranslation()); diff --git a/src/client/clientenvironment.cpp b/src/client/clientenvironment.cpp index 42a31ae8cde45..ed9a23ce35fbf 100644 --- a/src/client/clientenvironment.cpp +++ b/src/client/clientenvironment.cpp @@ -441,8 +441,8 @@ void ClientEnvironment::getSelectedActiveObjects( GenericCAO* gcao = dynamic_cast(obj); if (gcao != nullptr && gcao->getProperties().rotate_selectionbox) { gcao->getSceneNode()->updateAbsolutePosition(); - const v3f deg = obj->getSceneNode()->getAbsoluteTransformation().getRotationDegrees(); - collision = boxLineCollision(selection_box, deg, + const v3f rad = obj->getSceneNode()->getAbsoluteTransformation().getRotationRadians(); + collision = boxLineCollision(selection_box, rad, rel_pos, line_vector, ¤t_intersection, ¤t_normal, ¤t_raw_normal); } else { collision = boxLineCollision(selection_box, rel_pos, line_vector, diff --git a/src/client/game.cpp b/src/client/game.cpp index 12a39e6ee254f..246e6d22de1eb 100644 --- a/src/client/game.cpp +++ b/src/client/game.cpp @@ -3245,9 +3245,10 @@ PointedThing Game::updatePointedThing( hud->setSelectionPos(pos, camera_offset); GenericCAO* gcao = dynamic_cast(runData.selected_object); if (gcao != nullptr && gcao->getProperties().rotate_selectionbox) - hud->setSelectionRotation(gcao->getSceneNode()->getAbsoluteTransformation().getRotationDegrees()); + hud->setSelectionRotationRadians(gcao->getSceneNode() + ->getAbsoluteTransformation().getRotationRadians()); else - hud->setSelectionRotation(v3f()); + hud->setSelectionRotationRadians(v3f()); } hud->setSelectedFaceNormal(result.raw_intersection_normal); } else if (result.type == POINTEDTHING_NODE) { @@ -3267,7 +3268,7 @@ PointedThing Game::updatePointedThing( } hud->setSelectionPos(intToFloat(result.node_undersurface, BS), camera_offset); - hud->setSelectionRotation(v3f()); + hud->setSelectionRotationRadians(v3f()); hud->setSelectedFaceNormal(result.intersection_normal); } diff --git a/src/client/hud.cpp b/src/client/hud.cpp index e0b4e83fc0c3c..aa95444869c06 100644 --- a/src/client/hud.cpp +++ b/src/client/hud.cpp @@ -880,7 +880,7 @@ void Hud::drawSelectionMesh() core::matrix4 translate; translate.setTranslation(m_selection_pos_with_offset); core::matrix4 rotation; - rotation.setRotationDegrees(m_selection_rotation); + rotation.setRotationRadians(m_selection_rotation_radians); driver->setTransform(video::ETS_WORLD, translate * rotation); if (m_mode == HIGHLIGHT_BOX) { diff --git a/src/client/hud.h b/src/client/hud.h index aa4d53cda55c7..b42435f2560bd 100644 --- a/src/client/hud.h +++ b/src/client/hud.h @@ -74,9 +74,15 @@ class Hud v3f getSelectionPos() const { return m_selection_pos; } - void setSelectionRotation(v3f rotation) { m_selection_rotation = rotation; } + void setSelectionRotationRadians(v3f rotation) + { + m_selection_rotation_radians = rotation; + } - v3f getSelectionRotation() const { return m_selection_rotation; } + v3f getSelectionRotationRadians() const + { + return m_selection_rotation_radians; + } void setSelectionMeshColor(const video::SColor &color) { @@ -129,7 +135,7 @@ class Hud std::vector m_halo_boxes; v3f m_selection_pos; v3f m_selection_pos_with_offset; - v3f m_selection_rotation; + v3f m_selection_rotation_radians; scene::IMesh *m_selection_mesh = nullptr; video::SColor m_selection_mesh_color; diff --git a/src/raycast.cpp b/src/raycast.cpp index c84cb01e70aa1..fa6ad9cbd1e88 100644 --- a/src/raycast.cpp +++ b/src/raycast.cpp @@ -124,13 +124,13 @@ bool boxLineCollision(const aabb3f &box, const v3f start, return false; } -bool boxLineCollision(const aabb3f &box, const v3f rotation, - const v3f start, const v3f dir, +bool boxLineCollision(const aabb3f &box, v3f rotation_radians, + v3f start, v3f dir, v3f *collision_point, v3f *collision_normal, v3f *raw_collision_normal) { // Inversely transform the ray rather than rotating the box faces; // this allows us to continue using a simple ray - AABB intersection - core::quaternion rot(rotation * core::DEGTORAD); + core::quaternion rot(rotation_radians); rot.makeInverse(); bool collision = boxLineCollision(box, rot * start, rot * dir, collision_point, collision_normal); diff --git a/src/server/unit_sao.h b/src/server/unit_sao.h index 8cc27c967ac06..6930cd58322b1 100644 --- a/src/server/unit_sao.h +++ b/src/server/unit_sao.h @@ -30,7 +30,7 @@ class UnitSAO : public ServerActiveObject v3f res; // First rotate by m_rotation, then rotate by the automatic rotate yaw (core::quaternion(v3f(0, -m_rotation_add_yaw * core::DEGTORAD, 0)) - * core::quaternion(rot.getRotationDegrees() * core::DEGTORAD)) + * core::quaternion(rot.getRotationRadians())) .toEuler(res); return res * core::RADTODEG; } diff --git a/src/unittest/CMakeLists.txt b/src/unittest/CMakeLists.txt index 8a878cdd79868..8a635c8e80e73 100644 --- a/src/unittest/CMakeLists.txt +++ b/src/unittest/CMakeLists.txt @@ -13,6 +13,8 @@ set (UNITTEST_SRCS ${CMAKE_CURRENT_SOURCE_DIR}/test_filesys.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_inventory.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_irrptr.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test_irr_matrix4.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test_irr_rotation.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_logging.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_lua.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_map.cpp @@ -54,7 +56,6 @@ 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 index 764b0a5b27d4e..be6c6aa0897a9 100644 --- a/src/unittest/test_irr_matrix4.cpp +++ b/src/unittest/test_irr_matrix4.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: LGPL-2.1-or-later #include "catch.h" +#include "catch_amalgamated.hpp" #include "irrMath.h" #include "matrix4.h" #include "irr_v3d.h" @@ -83,4 +84,40 @@ SECTION("getScale") { } } +SECTION("getRotationRadians") { + auto test_rotation_degrees = [](v3f rad, v3f scale) { + matrix4 S; + S.setScale(scale); + matrix4 R; + R.setRotationRadians(rad); + v3f rot = (R * S).getRotationRadians(); + matrix4 B; + B.setRotationRadians(rot); + CHECK(matrix_equals(R, B)); + }; + SECTION("returns a rotation equivalent to the original rotation") { + test_rotation_degrees({1.0f, 2.0f, 3.0f}, v3f(1)); + Catch::Generators::RandomFloatingGenerator gen_angle(0.0f, 2 * core::PI, Catch::getSeed()); + Catch::Generators::RandomFloatingGenerator gen_scale(0.1f, 10, Catch::getSeed()); + auto draw = [](auto gen) { + f32 f = gen.get(); + gen.next(); + return f; + }; + auto draw_v3f = [&](auto gen) { + return v3f{draw(gen), draw(gen), draw(gen)}; + }; + for (int i = 0; i < 1000; ++i) + test_rotation_degrees(draw_v3f(gen_angle), draw_v3f(gen_scale)); + for (f32 i = 0; i < 4; ++i) + for (f32 j = 0; j < 4; ++j) + for (f32 k = 0; k < 4; ++k) { + v3f rad = core::PI / 4.0f * v3f(i, j, k); + for (int l = 0; l < 100; ++l) { + test_rotation_degrees(rad, draw_v3f(gen_scale)); + } + } + } +} + } diff --git a/src/unittest/test_irr_rotation.cpp b/src/unittest/test_irr_rotation.cpp new file mode 100644 index 0000000000000..0aa9c95737cb0 --- /dev/null +++ b/src/unittest/test_irr_rotation.cpp @@ -0,0 +1,109 @@ +// Luanti +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "catch.h" +#include "catch_amalgamated.hpp" +#include "irrMath.h" +#include "matrix4.h" +#include "irrMath.h" +#include "matrix4.h" +#include "irr_v3d.h" +#include "quaternion.h" +#include + +// Irrlicht provides three different representations of rotations: +// - Euler angles in radians (or degrees, but that doesn't matter much); +// - Quaternions; +// - Rotation matrices. +// These tests ensure that converting between these representations is rotation-preserving. + +using matrix4 = core::matrix4; +using quaternion = core::quaternion; + +// Despite the internal usage of doubles, matrix4::setRotationRadians +// simply incurs component-wise errors of the order 1e-3. +const f32 tolerance = 1e-2f; + +static bool matrix_equals(const matrix4 &mat, const matrix4 &mat2) +{ + return mat.equals(mat2, tolerance); +} + +static bool euler_angles_equiv(v3f rad, v3f rad2) +{ + matrix4 mat, mat2; + mat.setRotationRadians(rad); + mat2.setRotationRadians(rad2); + return matrix_equals(mat, mat2); +} + +static void test_euler_angles_rad(const std::function &test_euler_radians) +{ + Catch::Generators::RandomFloatingGenerator gen(0.0f, 2 * core::PI, Catch::getSeed()); + auto random_angle = [&gen]() { + f32 f = gen.get(); + gen.next(); + return f; + }; + for (int i = 0; i < 1000; ++i) + test_euler_radians(v3f{random_angle(), random_angle(), random_angle()}); + for (int i = 0; i < 4; i++) + for (int j = 0; j < 4; j++) + for (int k = 0; k < 4; k++) { + v3f rad = core::PI / 4.0f * v3f(i, j, k); + test_euler_radians(rad); + // Test very slightly nudged, "almost-perfect" rotations to make sure + // that the conversions are relatively stable at extremal points + for (int l = 0; l < 10; ++l) { + v3f jitter = v3f{random_angle(), random_angle(), random_angle()} * 0.001f; + test_euler_radians(rad + jitter); + } + } +} + +TEST_CASE("rotations") { + +SECTION("euler-to-quaternion conversion") { + test_euler_angles_rad([](v3f rad) { + core::matrix4 rot, rot_quat; + rot.setRotationRadians(rad); + quaternion q(rad); + q.getMatrix(rot_quat); + // Check equivalence of the rotations via matrices + CHECK(matrix_equals(rot, rot_quat)); + }); +} + +// Now that we've already tested the conversion to quaternions, +// this essentially primarily tests the quaternion to euler conversion +SECTION("quaternion-euler roundtrip") { + test_euler_angles_rad([](v3f rad) { + quaternion q(rad); + v3f rad2; + q.toEuler(rad2); + CHECK(euler_angles_equiv(rad, rad2)); + }); +} + +SECTION("matrix-quaternion roundtrip") { + test_euler_angles_rad([](v3f rad) { + matrix4 mat; + mat.setRotationRadians(rad); + quaternion q(mat); + matrix4 mat2; + q.getMatrix(mat2); + CHECK(matrix_equals(mat, mat2)); + }); +} + +SECTION("matrix-euler roundtrip") { + test_euler_angles_rad([](v3f rad) { + matrix4 mat, mat2; + mat.setRotationRadians(rad); + v3f rad2 = mat.getRotationRadians(); + mat2.setRotationRadians(rad2); + CHECK(matrix_equals(mat, mat2)); + }); +} + +}