[fixed]CMatrix4 getRotationDegrees() returns #QNAN0

[pc0de]

This is only a bug when a transform contains both rotation & scale.

A detailed description is located in this thread: matrix4f rotation & scale functions mutually incompatibl

I ran into this when syncing between scaled Irrlicht nodes & Bullet. My quick fix is to add a new member function to CMatrix4:

Code: Select all

			//! Returns the rotation, as set by setRotation() and takes scale into account.
			core::vector3d<T> getRotationDegreesDivScale() const;

	//! Returns a rotation - taking scaling into account.
	template <class T>
	inline core::vector3d<T> CMatrix4<T>::getRotationDegreesDivScale() const
	{
		const CMatrix4<T> &mat = *this;
      T SX = sqrtf(M[0] * M[0] + M[1] * M[1] + M[2] * M[2]);
      T SY = sqrtf(M[4] * M[4] + M[5] * M[5] + M[6] * M[6]);
      T SZ = sqrtf(M[8] * M[8] + M[9] * M[9] + M[10] * M[10]);

		f64 Y = -asin(mat(0,2) / SX);
		const f64 C = cos(Y);
		Y *= RADTODEG64;

		f64 rotx, roty, X, Z;

		if (fabs(C)>ROUNDING_ERROR_f64)
		{
			const T invC = (T)(1.0/C);
			rotx = mat(2,2) / SZ * invC;
			roty = mat(1,2) / SY * invC;
			X = atan2( roty, rotx ) * RADTODEG64;
			rotx = mat(0,0) / SX * invC;
			roty = mat(0,1) / SX * invC;
			Z = atan2( roty, rotx ) * RADTODEG64;
		}
		else
		{
			X = 0.0;
			rotx = mat(1,1) / SY;
			roty = -mat(1,0) / SY;
			Z = atan2( roty, rotx ) * RADTODEG64;
		}

		// fix values that get below zero
		// before it would set (!) values to 360
		// that where above 360:
		if (X < 0.0) X += 360.0;
		if (Y < 0.0) Y += 360.0;
		if (Z < 0.0) Z += 360.0;

		return vector3d<T>((T)X,(T)Y,(T)Z);
	}

Test code:

Code: Select all

void test3(const core::vector3df& Rotation, 
           const core::vector3df& Scale) 
{
    core::vector3df temp;

    printf("----------------------------------------------------------------------------\n");
    printf("                   org Scale: (%f, %f, %f)\n", Scale.X, Scale.Y, Scale.Z);
    printf("                org Rotation: (%f, %f, %f)\n\n", Rotation.X, Rotation.Y, Rotation.Z);

    // set scale via multiplication (this is what ISceneNode->getAbsoluteTransformation does)
    core::matrix4 mat;
    mat.setRotationDegrees(Rotation);
    temp = mat.getRotationDegrees();
    core::matrix4 smat;
    smat.setScale(Scale);
    mat *= smat;

    temp = mat.getRotationDegrees();
    printf("        getRotationDegrees(): (%f, %f, %f)\n", temp.X, temp.Y, temp.Z);
    temp = mat.getRotationDegreesDivScale();
    printf("getRotationDegreesDivScale(): (%f, %f, %f)\n", temp.X, temp.Y, temp.Z);
    temp = mat.getScale();
    printf("                     Scale(): (%f, %f, %f)\n\n", temp.X, temp.Y, temp.Z);
}

//-----------------------------------------------------------------------------
//                                 m a i n
//-----------------------------------------------------------------------------
#ifdef _IRR_WINDOWS_
#pragma comment(lib, "Irrlicht.lib") 
#pragma comment(linker, "/subsystem:console /ENTRY:mainCRTStartup") 
#endif
int main(int argc, char* argv[])
{

    core::vector3df Scale(10.f, 10.f, 10.f);

    test3(core::vector3df(25.f, 25.f, 25.f), Scale);
    test3(core::vector3df(25.f, 0.f, 0.f), Scale);
    test3(core::vector3df(0.f, 25.f, 0.f), Scale);
    test3(core::vector3df(0.f, 0.f, 25.f), Scale);

    test3(core::vector3df(10.5f, 25.5f, 60.f), core::vector3df(1.f,165.f,1.f));

    //materialAttributes();
    return 0;
}

output from test:

Code: Select all

-------------------------------------------------------------------------
                   org Scale: (10.000000, 10.000000, 10.000000)
                org Rotation: (25.000000, 25.000000, 25.000000)

        getRotationDegrees(): (0.000000, 1.#QNAN0, 13.851571)
getRotationDegreesDivScale(): (25.000002, 25.000002, 25.000000)
                     Scale(): (10.000000, 9.999999, 10.000000)

-------------------------------------------------------------------------
                   org Scale: (10.000000, 10.000000, 10.000000)
                org Rotation: (25.000000, 0.000000, 0.000000)

        getRotationDegrees(): (25.000002, -0.000000, 0.000000)
getRotationDegreesDivScale(): (25.000002, -0.000000, 0.000000)
                     Scale(): (10.000000, 10.000000, 10.000000)

-------------------------------------------------------------------------
                   org Scale: (10.000000, 10.000000, 10.000000)
                org Rotation: (0.000000, 25.000000, 0.000000)

        getRotationDegrees(): (0.000000, 1.#QNAN0, -0.000000)
getRotationDegreesDivScale(): (0.000000, 25.000002, 0.000000)
                     Scale(): (10.000000, 10.000000, 10.000000)

-------------------------------------------------------------------------
                   org Scale: (10.000000, 10.000000, 10.000000)
                org Rotation: (0.000000, 0.000000, 25.000000)

        getRotationDegrees(): (0.000000, -0.000000, 25.000002)
getRotationDegreesDivScale(): (0.000000, -0.000000, 25.000002)
                     Scale(): (10.000000, 10.000000, 10.000000)

-------------------------------------------------------------------------
                   org Scale: (1.000000, 1.000000, 1.000000)
                org Rotation: (10.500000, 25.500000, 60.000000)

        getRotationDegrees(): (10.500000, 25.499998, 60.000004)
getRotationDegreesDivScale(): (10.500000, 25.499998, 60.000004)
                     Scale(): (1.000000, 1.000000, 1.000000)

I can submit a patch if you think getRotationDegreesDivScale is acceptable. Thanks.

[pc0de]

Note to self - search the bug tracker before reporting... matrix4::getRotationDegrees() and getScale() problems - ID: 2526240

So if someone was to fix this the correct way, would it be with new methods as suggested in the referenced thread above or by fixing the existing methods?

[hybrid]

That depends on the impact on existing apps. But it might be that we will fix the existing methods for the next release (1.7) because the results right now are not good enough to deal with.

[pc0de]

- apparently Mrs. pc0de has somehow hijacked hybrid's account - She wants something, but is being vague on what it is.

As usual, I'll just pretend you didn't say anything and wait for you to bring me a beer...

[hybrid]

Seems you already had far too many
Anyway, I meant that I start to tend to replace the old method's behavior, because the existing ones return too many wrong results. But too many API breaking is bad, so nothing's fixed for now.