BRNSystems/irrlicht
1
0
Fork 0
forked from Mirrorlandia_minetest/irrlicht
Code Pull Requests Activity

Fix some problems with CMatrix4::getRotationDegrees

Browse Source 
- Only the getRotationDegrees without parameter is allowed to try fixing scale.
  My fault when I added a new function which takes scale parameter, that one is 
  not allowed to be changed.
  On the up-side - we know have for the first time an option which works in cases only
  scale and rotation had been used and the user still has the correct scale. 
  Before any solution for that was broken
- getRotationDegrees fixes 2 places which caused wrong results due to floating point inaccuracies
  New test for that got added
- Document the current restrains and problems of getRotationDegrees and getScale some more.
- Improve docs for other matrix4 functions.
- Add some comments about further improvements (I'll try if I find time)

Note: Irrlicht still assumes in at least 2 places (getting bone animations and Collada loader) that matrix
decomposing works. Which it doesn't yet for matrices which switch handedness (or have further transformations like skewing axes)
The bone animation is mostly fine for now with recent workaround (but that might cause other problems as it may be used too often), haven't checked Collada yet in detail.

TL/DR: This improves things with getRotationDegrees, but does not yet fix all troubles.


git-svn-id: svn://svn.code.sf.net/p/irrlicht/code/trunk@6439 dfc29bdd-3216-0410-991c-e03cc46cb475
This commit is contained in:
cutealien 2022-10-15 15:46:03 +00:00
parent 3f00ea004e
commit 3f372af486
4 changed files with 169 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
include/matrix4.h
View File

@ -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<T>& setbyproduct_nocheck(const CMatrix4<T>& other_a,const CMatrix4<T>& other_b );
//! Multiply by another matrix.
@ -150,7 +150,8 @@ namespace core
CMatrix4<T> operator*(const CMatrix4<T>& other) const;
//! Multiply by another matrix.
/** Calculate and return other*this */
/** Like calling: (*this) = (*this) * other
*/
CMatrix4<T>& operator*=(const CMatrix4<T>& 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<T>& setRotationDegrees( const vector3d<T>& 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<T> getRotationDegrees(const vector3d<T>& 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<T> 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 <class T>
inline vector3d<T> CMatrix4<T>::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 <class T>
inline core::vector3d<T> CMatrix4<T>::getRotationDegrees(const vector3d<T>& scale_) const
{
const CMatrix4<T> &mat = *this;
core::vector3d<T> 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<f64> 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<f64> 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 <class T>
inline core::vector3d<T> CMatrix4<T>::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<T> 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);
}

2
tests/main.cpp
View File

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

113
tests/matrixOps.cpp
View File

@ -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;
}

2
tests/tests-last-passed-at.txt
View File

@ -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