Fix some problems with CMatrix4::getRotationDegrees

- 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 committed by sfan5
parent edb381bd50
commit 9f9d22ca33
1 changed files with 54 additions and 38 deletions

View File

@ -143,7 +143,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.
@ -151,7 +151,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.
@ -187,14 +188,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;
@ -828,11 +840,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
@ -895,33 +905,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));
@ -930,7 +923,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;
@ -957,14 +950,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);
}