Back to General and Gameplay Programming

Matrix Class

General and Gameplay Programming Programming

Started by Hiewk April 25, 2005 01:11 PM

9 comments, last by ldeej 18 years, 12 months ago

Hiewk

122

Author

April 25, 2005 01:11 PM

Hi I'm wrapping all the matrix related math in a class, so that i can use matrices as objects, but i got a bug that i wasnt able to track it down. The problem appears when trying to multiply two matrices. Here is the relevant part of the class:


// In declaration
	union {
		struct {
			float _11, _12, _13, _14;
			float _21, _22, _23, _24;
			float _31, _32, _33, _34;
			float _41, _42, _43, _44;
		};

		float m[4][4];
	};

//In implementation
cMatrix cMatrix::operator*(const cMatrix &mtx) const
{
	cMatrix temp;
	
	temp._11 = (_11 * mtx._11) + (_12 * mtx._21) + (_13 * mtx._31) + (_14 * mtx._41);
	temp._12 = (_11 * mtx._12) + (_12 * mtx._22) + (_13 * mtx._32) + (_14 * mtx._42);
	temp._13 = (_11 * mtx._13) + (_12 * mtx._23) + (_13 * mtx._33) + (_14 * mtx._43);
	temp._14 = (_11 * mtx._14) + (_12 * mtx._24) + (_13 * mtx._34) + (_14 * mtx._44);

	temp._21 = (_21 * mtx._11) + (_22 * mtx._21) + (_23 * mtx._31) + (_24 * mtx._41);
	temp._22 = (_21 * mtx._12) + (_22 * mtx._22) + (_23 * mtx._32) + (_24 * mtx._42);
	temp._23 = (_21 * mtx._13) + (_22 * mtx._23) + (_23 * mtx._33) + (_24 * mtx._43);
	temp._24 = (_21 * mtx._14) + (_22 * mtx._24) + (_23 * mtx._34) + (_24 * mtx._44);

	temp._31 = (_31 * mtx._11) + (_32 * mtx._21) + (_33 * mtx._31) + (_34 * mtx._41);
	temp._32 = (_31 * mtx._12) + (_32 * mtx._22) + (_33 * mtx._32) + (_34 * mtx._42);
	temp._33 = (_31 * mtx._13) + (_32 * mtx._23) + (_33 * mtx._33) + (_34 * mtx._43);
	temp._34 = (_31 * mtx._14) + (_32 * mtx._24) + (_33 * mtx._34) + (_34 * mtx._44);

	temp._41 = (_41 * mtx._11) + (_42 * mtx._21) + (_43 * mtx._31) + (_44 * mtx._41);
	temp._42 = (_41 * mtx._12) + (_42 * mtx._22) + (_43 * mtx._32) + (_44 * mtx._42);
	temp._43 = (_41 * mtx._13) + (_42 * mtx._23) + (_43 * mtx._33) + (_44 * mtx._43);
	temp._44 = (_41 * mtx._14) + (_42 * mtx._24) + (_43 * mtx._34) + (_44 * mtx._44);

	return temp;
}
cMatrix& cMatrix::operator*=(const cMatrix &mtx)
{

	cMatrix temp;
	temp._11 = (_11 * mtx._11) + (_12 * mtx._21) + (_13 * mtx._31) + (_14 * mtx._41);
	temp._12 = (_11 * mtx._12) + (_12 * mtx._22) + (_13 * mtx._32) + (_14 * mtx._42);
	temp._13 = (_11 * mtx._13) + (_12 * mtx._23) + (_13 * mtx._33) + (_14 * mtx._43);
	temp._14 = (_11 * mtx._14) + (_12 * mtx._24) + (_13 * mtx._34) + (_14 * mtx._44);

	temp._21 = (_21 * mtx._11) + (_22 * mtx._21) + (_23 * mtx._31) + (_24 * mtx._41);
	temp._22 = (_21 * mtx._12) + (_22 * mtx._22) + (_23 * mtx._32) + (_24 * mtx._42);
	temp._23 = (_21 * mtx._13) + (_22 * mtx._23) + (_23 * mtx._33) + (_24 * mtx._43);
	temp._24 = (_21 * mtx._14) + (_22 * mtx._24) + (_23 * mtx._34) + (_24 * mtx._44);

	temp._31 = (_31 * mtx._11) + (_32 * mtx._21) + (_33 * mtx._31) + (_34 * mtx._41);
	temp._32 = (_31 * mtx._12) + (_32 * mtx._22) + (_33 * mtx._32) + (_34 * mtx._42);
	temp._33 = (_31 * mtx._13) + (_32 * mtx._23) + (_33 * mtx._33) + (_34 * mtx._43);
	temp._34 = (_31 * mtx._14) + (_32 * mtx._24) + (_33 * mtx._34) + (_34 * mtx._44);

	temp._41 = (_41 * mtx._11) + (_42 * mtx._21) + (_43 * mtx._31) + (_44 * mtx._41);
	temp._42 = (_41 * mtx._12) + (_42 * mtx._22) + (_43 * mtx._32) + (_44 * mtx._42);
	temp._43 = (_41 * mtx._13) + (_42 * mtx._23) + (_43 * mtx._33) + (_44 * mtx._43);
	temp._44 = (_41 * mtx._14) + (_42 * mtx._24) + (_43 * mtx._34) + (_44 * mtx._44);


	(*this) = temp;

	return (*this);
} 
cMatrix::cMatrix()
{
	LoadIdentity();
}
void cMatrix::LoadIdentity()
{
	memset(m, 0, sizeof(float) * 16);
	_11 = 1;
	_22 = 1;
	_33 = 1;
	_44 = 1;
}

Sample:


cMatrix mtx1;
cMatrix mtx2;
cMatrix mtx3;

mtx3 = mtx2 * mtx1;
mtx2 *= mtx1;

When using the operator *=, it works just fine. The problem is the operator *. After the operation, the fisrt line of the matrix is ok, but the remaining 3 will have junk data, like if it wasnt initialized. The weird thing is that in the operator*() function, just before returning, temp is correct. So the problem should be in returning the data. [Edited by - Hiewk on April 25, 2005 1:51:31 PM]

void0

202

April 25, 2005 05:20 PM

Seems to be working fine for me. Anyway, don't you have to define operator=() as well? I'm not sure if that would make any difference.

Zakwayda

2,389

April 25, 2005 05:25 PM

I'm not an expert on the c++ standard, but a couple of things I've read are that a) underscore prefixes are reserved by some compilers and should be avoided, and b) using unions in that way is not guaranteed to be portable and should also be avoided.

The default '=' operator should work fine here, unless the aforementioned 'union' issue is causing problems. Again, I'm not sure about the above - they're just things I've read (from fairly reliable sources though).

snk_kid

1,312

April 25, 2005 05:42 PM

Quote:Original post by jyk
b) using unions in that way is not guaranteed to be portable and should also be avoided.

To be a litte more precise it's anonymous structs/classes inside any form of unions that is not standard compliant, it's compiler extension thus not portable, a much better & C++ alternative can be seen here

JohnBolton

1,373

April 25, 2005 06:10 PM

Not the answer to your question, but it is a good idea to implement operator*() (likewise the +, -, /, <<, >>, &, ^, and | operators) like this in general.

    cMatrix cMatrix::operator*(const cMatrix &mtx) const    {        cMatrix temp( *this );        temp *= mtx;        return temp;    }

This way the operation is implemented in only one place, which is safer than duplicating code. Some compilers can optimize this to remove the temp variable and one or more copy operations, too.

John BoltonLocomotive Games (THQ)Current Project: Destroy All Humans (Wii). IN STORES NOW!

Anonymous

April 25, 2005 06:38 PM

I don't think it's necessary to overload operator= because class structures have fixed size. Anyway, i had already tried to overload it, and there were no positive results.
As you said, probably the union is causing the problem, i'm going to find another solution to replace it.
Also thanks for the optimization, i already implemented it, although the problem is still there (the problem is for sure located when operator*() returns temp)

Thanks for the replies.

Hiewk

122

Author

April 25, 2005 06:41 PM

Post as AP, Forget to log in.

Agony

3,452

April 25, 2005 06:46 PM

Do you have a proper copy constructor defined? I wouldn't actually expect it to be needed, since your data is pretty simple, but again, maybe the union is causing some problems. Besides, the copy constructor is the only thing I can see that might be getting called when using operator *, but not when using operator *=.

cMatrix::cMatrix(const cMatrix& matrix) :  _11(matrix._11), _13(matrix._12), _13(matrix._13), _14(matrix._14),  _21(matrix._21), _23(matrix._22), _23(matrix._23), _24(matrix._24),  _31(matrix._31), _33(matrix._32), _33(matrix._33), _34(matrix._34),  _41(matrix._41), _43(matrix._42), _43(matrix._43), _44(matrix._44){}

"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke

Hiewk

122

Author

April 25, 2005 06:54 PM

Im not using any copy constructor in the sample, but anyway it does a memcpy of the m[4][4] array.

Hiewk

122

Author

April 26, 2005 10:49 AM

Actually, the copy constructor was wrong. It was doing

memcpy(m, mtx_cpy.GetData(), 16);

Instead of

memcpy(m, mtx_cpy.GetData(), 16*sizeof(float));

Thanks Agony!

Matrix Class

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Matrix Class

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines