diff --git a/include/matrix4.h b/include/matrix4.h index b561fdcd..a15de295 100644 --- a/include/matrix4.h +++ b/include/matrix4.h @@ -142,7 +142,7 @@ namespace core //! Set this matrix to the product of two matrices /** Calculate b*a, no optimization used, - use it if you know you never have a identity matrix */ + use it if you know you never have an identity matrix */ CMatrix4& setbyproduct_nocheck(const CMatrix4& other_a,const CMatrix4& other_b ); //! Multiply by another matrix. @@ -150,7 +150,8 @@ namespace core CMatrix4 operator*(const CMatrix4& other) const; //! Multiply by another matrix. - /** Calculate and return other*this */ + /** Like calling: (*this) = (*this) * other + */ CMatrix4& operator*=(const CMatrix4& other); //! Multiply by scalar. @@ -186,14 +187,25 @@ namespace core //! Make a rotation matrix from Euler angles. The 4th row and column are unmodified. CMatrix4& setRotationDegrees( const vector3d& rotation ); - //! Get the rotation, as set by setRotation() when you already know the scale. - /** If you already know the scale then this function is faster than the other getRotationDegrees overload. - NOTE: You will have the same end-rotation as used in setRotation, but it might not use the same axis values. + //! 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. + NOTE: It will (usually) give wrong results when further transformations have been added in the matrix (like shear). + 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. */ core::vector3d getRotationDegrees(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: 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. */ core::vector3d getRotationDegrees() const; @@ -827,11 +839,9 @@ namespace core //! Returns the absolute values of the scales of the matrix. /** - Note that this returns the absolute (positive) values unless only scale is set. - Unfortunately it does not appear to be possible to extract any original negative - values. The best that we could do would be to arbitrarily make one scale - negative if one or three of them were negative. - FIXME - return the original values. + Note: You only get back original values if the matrix only set the scale. + Otherwise the result is a scale you can use to normalize the matrix axes, + but it's usually no longer what you did set with setScale. */ template inline vector3d CMatrix4::getScale() const @@ -894,33 +904,16 @@ namespace core } - //! Returns a rotation that is equivalent to that set by setRotationDegrees(). - /** This code was sent in by Chev. Note that 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. - This code was originally written by by Chev. + //! 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 core::vector3d CMatrix4::getRotationDegrees(const vector3d& scale_) const { const CMatrix4 &mat = *this; - core::vector3d scale(scale_); - // we need to check for negative scale on to axes, which would bring up wrong results - if (scale.Y<0 && scale.Z<0) - { - scale.Y =-scale.Y; - scale.Z =-scale.Z; - } - else if (scale.X<0 && scale.Z<0) - { - scale.X =-scale.X; - scale.Z =-scale.Z; - } - else if (scale.X<0 && scale.Y<0) - { - scale.X =-scale.X; - scale.Y =-scale.Y; - } + const core::vector3d scale(core::iszero(scale_.X) ? FLT_MAX : scale_.X , core::iszero(scale_.Y) ? FLT_MAX : scale_.Y, core::iszero(scale_.Z) ? FLT_MAX : scale_.Z); const core::vector3d invScale(core::reciprocal(scale.X),core::reciprocal(scale.Y),core::reciprocal(scale.Z)); f64 Y = -asin(core::clamp(mat[2]*invScale.X, -1.0, 1.0)); @@ -929,7 +922,7 @@ namespace core f64 rotx, roty, X, Z; - if (!core::iszero(C)) + if (!core::iszero((T)C)) { const f64 invC = core::reciprocal(C); rotx = mat[10] * invC * invScale.Z; @@ -956,14 +949,37 @@ namespace core } //! Returns a rotation that is equivalent to that set by setRotationDegrees(). - /** This code was sent in by Chev. Note that 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. - This code was originally written by by Chev. */ template inline core::vector3d CMatrix4::getRotationDegrees() const { - return getRotationDegrees(getScale()); + // 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 + core::vector3d scale(getScale()); + + // We assume the matrix uses rotations instead of negative scaling 2 axes. + // Otherwise it fails even for some simple cases, like rotating around + // 2 axes by 180° which getScale thinks is a negative scaling. + if (scale.Y<0 && scale.Z<0) + { + scale.Y =-scale.Y; + scale.Z =-scale.Z; + } + else if (scale.X<0 && scale.Z<0) + { + scale.X =-scale.X; + scale.Z =-scale.Z; + } + else if (scale.X<0 && scale.Y<0) + { + scale.X =-scale.X; + scale.Y =-scale.Y; + } + + return getRotationDegrees(scale); } diff --git a/tests/main.cpp b/tests/main.cpp index 9242ec53..c5205492 100644 --- a/tests/main.cpp +++ b/tests/main.cpp @@ -49,7 +49,7 @@ int main(int argumentCount, char * arguments[]) #if 0 // To interactively debug a test, move it (temporarily) in here and enable the define to only run this test // Otherwise debugging is slightly tricky as each test runs in it's own process. - TEST(ioScene); + TEST(matrixOps); #else TEST(disambiguateTextures); // Normally you should run this first, since it validates the working directory. diff --git a/tests/matrixOps.cpp b/tests/matrixOps.cpp index 63a0877c..a49f2979 100644 --- a/tests/matrixOps.cpp +++ b/tests/matrixOps.cpp @@ -354,6 +354,118 @@ bool setRotationAxis() return true; } +// Note: pretty high tolerance needed +bool check_getRotationDegreesWithScale2(const core::matrix4& m, const irr::core::vector3df& scale, irr::f32 tolerance = 0.01f) +{ + core::vector3df rot = m.getRotationDegrees(scale); + + core::matrix4 m2; + m2.setRotationDegrees(rot); + + core::matrix4 smat; + smat.setScale(scale); + m2 *= smat; + + core::vector3df v1(5,10,15); + core::vector3df v2 = v1; + m.transformVect(v1); + m2.transformVect(v2); + + if ( v1.equals(v2, tolerance) ) + return true; + + logTestString("v1: %.3f %.3f %.3f\nv2: %.3f %.3f %.3f\n", v1.X, v1.Y, v1.Z, v2.X, v2.Y, v2.Z); + //logTestString("matrix (3x3): "); + //for ( int k=0; k<3; ++k) + // for ( int i=0; i<3; ++i ) + // logTestString("%.3f ", m[k*4+i]); + //logTestString("\n"); + return false; +} + +// This can only work if the matrix is pure scale or pure rotation +bool check_getRotationDegreesWithScale(const core::matrix4& m, irr::f32 tolerance = 0.001f) +{ + core::vector3df scale = m.getScale(); + return check_getRotationDegreesWithScale2(m, scale, tolerance); +} + +// Lazy macro only to be used inside the loop where it is used +// (can't use lambda yet, still testing on older compilers) +#define log_check_getRotationDegreesWithScaleIJK \ +do { \ + smat.setScale(scale); \ + m2 = m1*smat; \ + if ( !check_getRotationDegreesWithScale2(m2, scale) ) { \ + logTestString("%s:%d i:%f j:%f k:%f\n", __FILE__, __LINE__, i, j, k); \ + result = false; } \ +} while (false) + +bool decompose() +{ + bool result = true; + core::matrix4 m1; + result &= check_getRotationDegreesWithScale(m1); + + // check pure scaling/90° rotations and 0 values + for ( irr::f32 i = -2.f; i <= 2.f; i += 1.f ) + for ( irr::f32 j = -2.f; j <= 2.f; j += 1.f ) + for ( irr::f32 k = -2.f; k <= 2.f; k += 1.f ) + { + m1 = core::matrix4(); + m1[0] = i; + m1[5] = j; + m1[10] = k; + if ( !check_getRotationDegreesWithScale(m1) ) + { + logTestString("%s:%d i:%f j:%f k:%f\n", __FILE__, __LINE__, i, j, k); + result = false; + } + } + + // check some rotations (note that we avoid the 0 case - which won't work) + for ( irr::f32 i = -180.f; i <= 360.f; i += 30.1f ) + for ( irr::f32 j = -120.f; j <= 200.f; j += 44.4f ) + for ( irr::f32 k = -10.f; k <= 180.f; k += 33.3f ) + { + m1 = core::matrix4(); + m1.setRotationDegrees(core::vector3df(i,j,k)); + result &= check_getRotationDegreesWithScale(m1); // pure rotation + + // rotation + scaling tests + // We can't use check_getRotationDegreesWithScale as we have no way so far to decompose a combined matrix + core::matrix4 smat, m2; + core::vector3df scale; + + scale = core::vector3df(2.f, 2.f, 2.f); // simple uniform scaling + log_check_getRotationDegreesWithScaleIJK; + + scale = core::vector3df(-2.f, 2.f, 2.f); // simple uniform scaling which swaps handedness + log_check_getRotationDegreesWithScaleIJK; // (TODO: can't decompose this yet) + + scale = core::vector3df(i, i, i); // flexible uniform scaling + log_check_getRotationDegreesWithScaleIJK; // (TODO: can't decompose this yet) + + scale = core::vector3df(1, 2, 3); // simple non-uniform scaling + log_check_getRotationDegreesWithScaleIJK; + + scale = core::vector3df(-1, -2, -3); // negative non-uniform scaling with swap of handedness + log_check_getRotationDegreesWithScaleIJK; // (TODO: can't decompose this yet) + + scale = core::vector3df(-1, 2, -3); // +- non-uniform scaling + log_check_getRotationDegreesWithScaleIJK; + + scale = core::vector3df(i,k,j); // non-uniform scaling + log_check_getRotationDegreesWithScaleIJK; // (TODO: can't decompose this yet) + } + + if ( !result ) + logTestString("decomposing matrix failed\n"); + + return result; +} + + // just calling each function once to find compile problems void calltest() { @@ -460,6 +572,7 @@ bool matrixOps(void) result &= isOrthogonal(); result &= transformations(); result &= setRotationAxis(); + result &= decompose(); return result; } diff --git a/tests/tests-last-passed-at.txt b/tests/tests-last-passed-at.txt index c1bc1c58..1b8b4fe4 100644 --- a/tests/tests-last-passed-at.txt +++ b/tests/tests-last-passed-at.txt @@ -1,4 +1,4 @@ Tests finished. 72 tests of 72 passed. Compiled as DEBUG -Test suite pass at GMT Thu Sep 29 16:32:30 2022 +Test suite pass at GMT Sat Oct 15 15:38:53 2022