Fix CMatrix<T>::getScale returning negative scale (#15687)

This commit is contained in:
Lars Müller 2025-01-18 00:27:27 +01:00 committed by GitHub
parent c8b5e3b741
commit 8719a816e7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 29 additions and 58 deletions

View file

@ -782,7 +782,7 @@ inline CMatrix4<T> &CMatrix4<T>::setScale(const vector3d<T> &scale)
return *this;
}
//! Returns the absolute values of the scales of the matrix.
//! Returns the absolute values of the scales of the 3x3 submatrix.
/**
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,
@ -791,19 +791,15 @@ but it's usually no longer what you did set with setScale.
template <class T>
inline vector3d<T> CMatrix4<T>::getScale() const
{
// See http://www.robertblum.com/articles/2005/02/14/decomposing-matrices
// Deal with the 0 rotation case first
// Prior to Irrlicht 1.6, we always returned this value.
if (core::iszero(M[1]) && core::iszero(M[2]) &&
core::iszero(M[4]) && core::iszero(M[6]) &&
core::iszero(M[8]) && core::iszero(M[9]))
return vector3d<T>(M[0], M[5], M[10]);
// We have to do the full calculation.
return vector3d<T>(sqrtf(M[0] * M[0] + M[1] * M[1] + M[2] * M[2]),
sqrtf(M[4] * M[4] + M[5] * M[5] + M[6] * M[6]),
sqrtf(M[8] * M[8] + M[9] * M[9] + M[10] * M[10]));
auto row_vector_length = [this](int col) {
int i = 4 * col;
return sqrtf(M[i] * M[i] + M[i+1] * M[i+1] + M[i+2] * M[i+2]);
};
return {
row_vector_length(0),
row_vector_length(1),
row_vector_length(2),
};
}
template <class T>
@ -891,7 +887,7 @@ inline core::vector3d<T> CMatrix4<T>::getRotationDegrees(const vector3d<T> &scal
//! Returns a rotation that is equivalent to that set by setRotationDegrees().
template <class T>
inline core::vector3d<T> CMatrix4<T>::getRotationDegrees() const
inline vector3d<T> CMatrix4<T>::getRotationDegrees() 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
@ -899,21 +895,7 @@ inline core::vector3d<T> CMatrix4<T>::getRotationDegrees() const
// 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;
}
vector3d<T> scale(getScale());
return getRotationDegrees(scale);
}

View file

@ -1459,34 +1459,6 @@ void GenericCAO::updateBones(f32 dtime)
bone->setScale(props.getScale(bone->getScale()));
}
// search through bones to find mistakenly rotated bones due to bug in Irrlicht
for (u32 i = 0; i < m_animated_meshnode->getJointCount(); ++i) {
scene::IBoneSceneNode *bone = m_animated_meshnode->getJointNode(i);
if (!bone)
continue;
//If bone is manually positioned there is no need to perform the bug check
bool skip = false;
for (auto &it : m_bone_override) {
if (it.first == bone->getName()) {
skip = true;
break;
}
}
if (skip)
continue;
// Workaround for Irrlicht bug
// We check each bone to see if it has been rotated ~180deg from its expected position due to a bug in Irricht
// when using EJUOR_CONTROL joint control. If the bug is detected we update the bone to the proper position
// and update the bones transformation.
v3f bone_rot = bone->getRelativeTransformation().getRotationDegrees();
float offset = fabsf(bone_rot.X - bone->getRotation().X);
if (offset > 179.9f && offset < 180.1f) {
bone->setRotation(bone_rot);
bone->updateAbsolutePosition();
}
}
// The following is needed for set_bone_pos to propagate to
// attached objects correctly.
// Irrlicht ought to do this, but doesn't when using EJUOR_CONTROL.

View file

@ -66,4 +66,21 @@ SECTION("setRotationRadians") {
}
}
SECTION("getScale") {
SECTION("correctly gets the length of each row of the 3x3 submatrix") {
matrix4 A(
1, 2, 3, 0,
4, 5, 6, 0,
7, 8, 9, 0,
0, 0, 0, 1
);
v3f scale = A.getScale();
CHECK(scale.equals(v3f(
v3f(1, 2, 3).getLength(),
v3f(4, 5, 6).getLength(),
v3f(7, 8, 9).getLength()
)));
}
}
}