Jump to content
  • Advertisement
Sign in to follow this  
Hiewk

Matrix Class

This topic is 4865 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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]

Share this post


Link to post
Share on other sites
Advertisement
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.

Share this post


Link to post
Share on other sites
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).

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
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.

Share this post


Link to post
Share on other sites
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)
{
}

Share this post


Link to post
Share on other sites
Im not using any copy constructor in the sample, but anyway it does a memcpy of the m[4][4] array.

Share this post


Link to post
Share on other sites
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!

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!