4
\$\begingroup\$

This code seems to be working. I am allocating matrices on the stack and passing them to functions to modify in place. Is this a standard practice, or am I doing it wrong?

Header:

#ifndef _MATRIX_H
#define _MATRIX_H

typedef enum {MT_OK, MT_INVAL} MT_RESULT;

typedef float mat4[4][4];
typedef float vec4[4];

MT_RESULT mat4multvec4(vec4 target, mat4 m, vec4 v);
MT_RESULT zero4(mat4 target);
MT_RESULT id4(mat4 target);
MT_RESULT translate(mat4 target, float x, float y, float z);
MT_RESULT scale(mat4 target, float x, float y, float z);
MT_RESULT perspective(mat4 target, float fov, float aspectratio, float nearclip, float farclip);

#endif

Implementation:

#include <stdlib.h>
#include <stdio.h>
#include <string.h> /* memcpy */
#include <math.h>

#include "matrix.h"

#undef PI
#define PI 3.14159265


/*
Multiply 4x4 matrix with 1x4 vector
returns MT_OK
*/
MT_RESULT mat4multvec4(vec4 target, mat4 m, vec4 v)
{
    int i, j, sum;

    for(i = 0; i < 4; i++) {
        sum = 0;
        for(j=0; j<4; j++) {
            sum = sum + m[i][j] * v[j];
        }
        target[i] = sum;
    }

    return MT_OK;
}

/*
make a 4x4 zeros matrix
returns MT_OK
*/
MT_RESULT zero4(mat4 target)
{
    memset(target, 0, sizeof(float) * 16);
    return MT_OK;
}

/*
make a 4x4 identity matrix
returns MT_OK
*/
MT_RESULT id4(mat4 target)
{
    int i;
    zero4(target);

    for(i = 0; i < 4; i++) 
                target[i][i] = 1;

    return MT_OK;
}

/*
Create 4x4 translation matrix for opengl
return MT_OK
*/
MT_RESULT translate(mat4 target, float x, float y, float z)
{
    id4(target);

    target[0][3] = x;
    target[1][3] = y;
    target[2][3] = z;

    return MT_OK;
}

/*
Create 4x4 scale matrix for opengl
return MT_OK
*/
MT_RESULT scale(mat4 target, float x, float y, float z)
{
    zero4(target);

    target[0][0] = x;
    target[1][1] = y;
    target[2][2] = z;
    target[3][3] = 1;

    return MT_OK;
}

/*
mat4 perspective(float fov, float aspectratio, float nearclip, float farclip)
{
    mat4 m = new_mat4();
    float frustumdepth = farclip - nearclip;
    float inversefrustumdepth = 1 / frustumdepth;

    m[]
}*/

void main(void)
{
    int i;
    mat4 myMatrix;
    vec4 myVector = {10.0f, 10.0f, 10.0f, 1.0f};
    vec4 myTVector;

    translate(myMatrix, 10.0f, 0.0f, 0.0f);

    mat4multvec4(myTVector, myMatrix, myVector);

    printf("Original Vector\n");
    for(i=0;i<4;i++) {
        printf(" %f", myVector[i]);
    }

    printf("\nTransformed Vector\n");
    for(i=0;i<4;i++) {
        printf(" %f", myTVector[i]);
    }
}
\$\endgroup\$
5
\$\begingroup\$

Answering some of your questions...

Is passing arrays to functions memory safe?

No, not really, but the C language offers no other approach, so you just have to be extra careful, since neither the compiler nor the runtime will alert you if you try to access invalid or out-of-bounds memory.

I am allocating matrices on stack and passing them to function to modify in place. Is this a standard practice?

It can be, yes. If you prefer to have them modified in place, it is fine. Some libraries prefer to have separate input and output parameters, and that tends to produce cleaner code, but sometimes it's simpler to just modify in place. E.g.: It would be annoying to have, for instance, a "make identity" function in a matrix library take two parameters, like: matrix_identity(const mat* in, mat* out). Much more convenient to just write:

matrix_identity(&matrix);

However, for arithmetical operations it would be more natural to have inputs and outputs:

mat a = { ... };
mat b = { ... };
mat result;
matrix_multiply(&result, &a, &b); // result = a * b

As to allocating on the stack, leave that up to the user. Your code should only be concerned with pointers to objects, how that's allocated is up to the caller. Leave the option for both, since stack variables are cheap, but sometimes one might have to use the global heap to extend lifetime.


Other details about your code:

  • The error codes you're returning in each function are irrelevant, since every function always returns the same MT_OK, which means they could very well be void functions. You don't have to make all functions return an error code for the sake of uniformity. Save the error codes for functions that can actually fail for some reason.

  • Since C doesn't have any construct for scoping names (like modules/namespaces), it is common practice to prefix your function names with a pseudo namespace, to reduce the chance of collisions. You could maybe prefix your function names with mat4_, e.g.: mat4_ translate(), mat4_identity(), mat4_mult_vec4() etcetera.

  • Don't redefine PI risking using less precision than the actual supported. Most math.h implementations will have M_PI defined, which you can use. You may still provide a fallback if necessary, but default to M_PI:

    #ifndef M_PI
      #define M_PI 3.14159265f
    #endif // M_PI
    
  • Which matrix layout are you using? Column major, I suspect? State that very clearly somewhere, like in a documentation comment in the header file, to leave no room for confusion.

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