4x4 matrix multiplication

Question

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);
}

Answer 1

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...

Answer 2

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 };

Answer 3

@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.