matrix4f rotation & scale functions mutually incompatibl

[SuperElectric]

The scale and rotation-related functions in core::matrix4 all seem to assume that a single matrix4 will never represent both a rotation and a scaling operation. This is a pretty risky assumption, especially if someone's trying to get an ISceneNode's global rotation using:

Code: Select all

 
vector3df absRotation = myISceneNode->getAbsoluteTransformation().getRotationDegrees();

If you take two of myISceneNode's ancestors and apply a scale to one and a rotation to the other, the above line will break and return the wrong rotation, possibly with NaNs. Then you get question threads like <this> one that last for 23 posts .

To fix matrix4::setScale, replace it with the following:

Code: Select all

inline void matrix4::setScale( const vector3df& scale )
{
  
  M[0] *= scale.X;
  M[1] *= scale.X;
  M[2] *= scale.X;
  
  M[4] *= scale.Y;
  M[5] *= scale.Y;
  M[6] *= scale.Y;
  
  M[8] *= scale.Z;
  M[9] *= scale.Z;
  M[10] *= scale.Z;
  
}

To fix matrix4::getRotationDegrees, replace the following line:

Code: Select all

const matrix4 &mat = *this;

with:

Code: Select all

matrix4 mat(*this);
vector3df scale = getScale();
mat.setScale( vector3df(1.0/scale.X, 1.0/scale.Y, 1.0/scale.Z) );

where getScale() is defined as:

Code: Select all

inline vector3df matrix4::getScale() const{
  return vector3df(  sqrt(M[0]*M[0] + M[1]*M[1] + M[2]*M[2]),
						sqrt(M[4]*M[4] + M[5]*M[5] + M[6]*M[6]),
						sqrt(M[8]*M[8] + M[9]*M[9] + M[10]*M[10]) );
}

setRotationRadians() and setInverseRotationRadians should also be fixed so that they preserve scale instead of overwriting it. At the very beginning of these functions, insert the line:

Code: Select all

vector3df scale = getScale();

and at the very end of the functions, insert the line:

Code: Select all

setScale(scale);

Fixing setRotationRadians() will also fix setRotationDegrees(), since the latter just calls the former.

A similar fix is needed for the rotateVect() and inverseRotateVect() functions, or else when you use them to rotate a vector, the vector also ends up getting scaled by the matrix's scale (like calling transformVect, except without the translation). At the beginning of these functions, insert the line:

Code: Select all

f32 len = vect.getLength();

At the end of the function, insert this line:

Code: Select all

vect.setLength(len);

I don't think you need to do a check for len==0; it's done as part of vect.setLength(). I've applied all the changes above, and it hasn't broken anything yet, knock on wood.

Finally, if all this norm-taking raises efficiency concerns, one could just maintain boolean flags in matrix4f like "rotationWasSet" and "scaleWasSet", such that the scale-sensitive code above is only run when both flags are true. But I didn't bother, as my frames per second didn't go down at all.

-- Matt

[niko]

Hm, or maybe add some new methods which do what you mentioned, which don't overwrite rotations. For backward compatibility and speed etc.