3
\$\begingroup\$

I am very new to matrix math. I have the following that sets the rotation in a 4x4 matrix. It is pretty ugly, so can anyone suggest how I could clean this up?

I would like to not have to call MultiplyTwoMatrixes twice. A better way of copying the 3x3 matrix into the end 4x4 matrix would be nice as well.

void MultiplyTwoMatrixes(float aMatrix[9], float bMatrix[9], float product[9])
{
    for (int i = 0; i < 3; i++)
    {
        for (int j = 0; j < 3; j++)
        {
            product[j + i * 3] = 0;
            for (int k = 0; k < 3; k++)
            {
                product[j + i * 3] += aMatrix[k + i * 3] * bMatrix[j + k * 3];
            }
        }
    }
}

void setrotation(float x, float y, float z)
{
    const float* oldMatrix = node->GetTransform();

    float xRot[9] = { 1, 0, 0, 0, cos(x), sin(x), 0, -sin(x), cos(x) };
    float yRot[9] = { cos(y), 0 ,-sin(y), 0, 1, 0, sin(y), 0, cos(y) };
    float zRot[9] = { cos(z), sin(z), 0, -sin(z), cos(z), 0, 0, 0, 1 };

    float xyResult[9];
    float xyzResult[9];
    float newMatrix[16];
    MultiplyTwoMatrixes(xRot, yRot, xyResult);
    MultiplyTwoMatrixes(xyResult, zRot, xyzResult);

    newMatrix[0] = xyzResult[0]; newMatrix[1] = xyzResult[1]; newMatrix[2] = xyzResult[2];
    newMatrix[4] = xyzResult[3]; newMatrix[5] = xyzResult[4]; newMatrix[6] = xyzResult[5];
    newMatrix[8] = xyzResult[6]; newMatrix[9] = xyzResult[7]; newMatrix[10] = xyzResult[8];
    newMatrix[12] = oldMatrix[12]; newMatrix[13] = oldMatrix[13]; newMatrix[14] = oldMatrix[14];

    node->SetTransform(newMatrix);
}
\$\endgroup\$
  • \$\begingroup\$ You marked this as C++. Rather than rolling your own, use a preexisting package. If your target domain is graphics, use something like openGL. If you're target domain is mathematics or physics, use something like Eigen. \$\endgroup\$ – David Hammen Oct 9 '15 at 15:28
  • \$\begingroup\$ That is not a option for me, I cannot use any external libraries. \$\endgroup\$ – marsh Oct 9 '15 at 15:31
  • \$\begingroup\$ Is this a school project, some corporate project where the powers that be have ruled against using open source, or something else? \$\endgroup\$ – David Hammen Oct 9 '15 at 15:34
  • 1
    \$\begingroup\$ Does it matter? There is a variety of reasons but that is off topic. \$\endgroup\$ – marsh Oct 9 '15 at 15:37
  • 1
    \$\begingroup\$ @DavidHammen OpenGL expects you to pass it matrices, it doesn't calculate them for you (unless you're talking about deprecated stuff from a decade ago?) \$\endgroup\$ – Alec Teal Oct 9 '15 at 21:01
4
\$\begingroup\$

Arrays as Arguments

When you write this:

void MultiplyTwoMatrixes(float aMatrix[9], float bMatrix[9], float product[9])

That is exactly equivalent to:

void MultiplyTwoMatrixes(float*, float*, float*);

That is, there's no size checking on any of the inputs whatsoever - I could call your function with three arrays of size 3, 1, and 72. Would compile fine. Which is a problem!

Encapsulation and Operators

We want type safety. That suggests a Matrix class. For simplicity, let's just make it 2d:

template <typename T, int Rows, int Cols>
struct Matrix;

And then we can write a multiply operator:

template <typename T, int R1, int C1, int C2>
Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1>& lhs, const Matrix<T, C1, C2>& rhs);

Notice that this already handles dimensional analysis for you. You just have to write the internals, which isn't complicated. And then you would be able to write your rotation function naturally:

Matrix<float, 3, 3> xyzResult = xRot * yRot * zRot;

Type-safe and more readable!

newMatrix

I don't understand the construction of newMatrix. So I can't really offer a better solution? You're not setting lots of indices of newMatrix, for one thing...

\$\endgroup\$
  • \$\begingroup\$ At the end of the result I need a [4][4] matrix. While my calculation gave me a [3][3] matrix. So I am putting the [3][3] into the [4][4]. \$\endgroup\$ – marsh Oct 9 '15 at 15:39
3
\$\begingroup\$

This is really minor, but one thing that can improve the performance would be to only calculate your sin and cos values once. You calculate each of sin(x), cos(x), sin(y), cos(y), and sin(z), cos(z) twice. Something like this:

float sinx = sin(x);
float cosx = cos(x);
...
float xRot[9] = { 1, 0, 0, 0, cosx, sinx, 0, -sinx, cosx };
\$\endgroup\$
  • \$\begingroup\$ The compiler might be smart enough to do this anyway. It's worth a peek at the code generated for something like this (in a release build). \$\endgroup\$ – terriblememory Oct 9 '15 at 22:36
2
\$\begingroup\$

@Barry has some good advice, but if you want to use your existing code and make it more manageable, I would write some helper functions:

inline int offset3x3(int i, int j) { return 3*i+j; }
inline int offset4x4(int i, int j) { return 4*i+j; }

and then to transfer results to the 4x4 matrix:

for (int i = 0 ; i < 3; ++i) {
  for (int j = 0; j < 3; ++j) {
    newMatrix[ offset4x4(i,j) ] = xyzResult[ offset3x3(i,j) ];
  }
}

You can even use the offset3x3 function in your matrix multiplication routine.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.