HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

4x4 matrix multiplication

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
matrix4x4multiplication

Problem

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

Solution

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 
struct Matrix;


And then we can write a multiply operator:

template 
Matrix operator*(const Matrix& lhs, const Matrix& 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 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...

Code Snippets

void MultiplyTwoMatrixes(float aMatrix[9], float bMatrix[9], float product[9])
void MultiplyTwoMatrixes(float*, float*, float*);
template <typename T, int Rows, int Cols>
struct Matrix;
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);
Matrix<float, 3, 3> xyzResult = xRot * yRot * zRot;

Context

StackExchange Code Review Q#107068, answer score: 4

Revisions (0)

No revisions yet.