current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I adapted this piece of code from a math book for game developers and the result is correct albeit with slight floating point errors.

void rotate(matrix44_t &m, const float32_t x, const float32_t y, const float32_t z) {
    float32_t sin_x = math::sin(x), cos_x = math::cos(x);
    float32_t sin_y = math::sin(y), cos_y = math::cos(y);
    float32_t sin_z = math::sin(z), cos_z = math::cos(z);

    m.data[0] =  cos_y * cos_z;
    m.data[4] = -(cos_y * sin_z);
    m.data[8] = sin_y;
    m.data[12] = 0.0f;

    m.data[1] = (sin_x * sin_y * cos_z) + (cos_x * sin_z);
    m.data[5] = -(sin_x * sin_y * sin_z) + (cos_x * cos_z);
    m.data[9] = -(sin_x * cos_y);
    m.data[13] = 0.0f;

    m.data[2] = -(cos_x * sin_y * cos_z) + (sin_x * sin_z);
    m.data[6] = (cos_x * sin_y * sin_z) + (sin_x * cos_z);
    m.data[10] = (cos_x * cos_y);
    m.data[14] = 0.0f;

    m.data[3] = 0.0f;
    m.data[7] = 0.0f;
    m.data[11] = 0.0f;
    m.data[15] = 1.0f;
}

When I call the function like this:

rotate(m1, 0.0f, DEG_2_RAD(90.0f), 0.0f);

I get the following:

[  0.000001,  0.000000, -1.000000, 0.000000 ]
[ -0.000000,  1.000000,  0.000000, 0.000000 ]
[  1.000000, -0.000000,  0.000001, 0.000000 ]
[  0.000000,  0.000000,  0.000000, 1.000000 ]

That is pretty close except for the 1st row, 1st column and the 3rd row, 3rd column which should both be 0.

Is this slight margin of error usually tolerated in game development?

share|improve this question

put on hold as off-topic by RubberDuck, TheCoffeeCup, QPaysTaxes, SirPython, Alex L 13 hours ago

If this question can be reworded to fit the rules in the help center, please edit the question.
3  
Welcome to Code Review! While this code is reviewable, your question about suitability for games might be better asked on Game Development. – 200_success♦ 22 hours ago
2  
You tagged this question as C, but it looks like C++ to me. – 200_success♦ 22 hours ago
3  
I'm voting to close this question as off-topic because it is not seeking a Code Review. – RubberDuck 21 hours ago

1 Answer 1

up vote 1 down vote

This is a fairly straightforward implementation and very clear. Nice use of const on the parameters. Overall I like it. I'd only change a few things.

Readability

I find the declaration of 2 variables at once to be less readable than doing each on a separate line. Furthermore, I find it really unreadable when doing multiple variables on one line and initializing them with function calls. So I'd break it up like this:

float32_t sin_x = math::sin(x);
float32_t cos_x = math::cos(x);
float32_t sin_y = math::sin(y);
float32_t cos_y = math::cos(y);
float32_t sin_z = math::sin(z);
float32_t cos_z = math::cos(z);

It's only 3 more lines and is easier to see where each variable is declared.

Precision

Your precision in the function is probably fine. If you're worried about it, you can try replacing the float32_t types with doubles and see if it gets any better. If you do everything to double precision and only convert to single precision at the end, that might tell you whether it's sufficient.

But the other thing you should do is use the named constants for things that already have them. Instead of DEG_2_RAD(90.0f), just use M_PI_2 which is defined in <math.h> already. That may also be the source of your precision issue. (Or it could be both.)

share|improve this answer

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