Bug in raymath.h ( raylib 1.7 )

for opengl column major matrices this function is wrong , ( your declaration of Matrix struct is ok )

RMDEF Matrix MatrixTranslate(float x, float y, float z)
{
Matrix result = { 1.0f, 0.0f, 0.0f, 0.0f,
0.0f, 1.0f, 0.0f, 0.0f,
0.0f, 0.0f, 1.0f, 0.0f,
x, y, z, 1.0f };

return result;
}

it should be

RMDEF Matrix MatrixTranslate(float x, float y, float z)
{
Matrix result = { 1.0f, 0.0f, 0.0f, x,
0.0f, 1.0f, 0.0f, y,
0.0f, 0.0f, 1.0f, z,
0.0f, 0.0f, 0.0f, 1.0f };

return result;
}

I was doing this lesson :
https://learnopengl.com/#!Getting-started/Transformations
( In practice part )

Vector3 vec = {1.0, 0, 0};
Matrix mat = MatrixTranslate(1.0, 1.0, 0);
VectorTransform(&vec, mat);
printf("x = %f y = %f z = %f\n", vec.x, vec.y, vec.z);

my result was
vec.x = 1.0
vec.y = 0.0
vec.z = 0.0

his vector
vec.x = 2.0
vec.y = 1.0
vec.z = 0.0

so I dig a little and find that little bug , after corection result is identical
vec.x = 2.0
vec.y = 1.0
vec.z = 0.0

thx...




Comments

  • ok I just realised that there is a MatrixTranspose function so
    maybe( probably ) no bug at all , no need for change x,y,z elements to last column,
    new code snipet is :

    Vector3 vec = {1.0, 0, 0};

    Matrix mat = MatrixTranslate(1.0, 1.0, 0);
    MatrixTranspose(&mat);

    VectorTransform(&vec, mat);

    printf("x = %f y = %f z = %f\n", vec.x, vec.y, vec.z);

    with correct output:

    x = 2.0 y = 1.0 z =0.0

    but why then Matrix struct is column major and what is the right math order
    when dealing with matrices ?

    btw I am using raymath.h as standalone header
  • Hello dancho!

    Thank you very much for the feedback and thanks for your tests. Actually, there is a related issue on raylib github: https://github.com/raysan5/raylib/issues/291

    https://learnopengl.com/#!Getting-started/Transformations is agreat reference to start with matrix math, and there are correct on the math. But the current implementation of all that math is done using GLM math library, a bigger and more complex math library than raymath.

    So, what's the difference? Well, it's a matter of how actual data is aligned in memory. My matrix struct is defined as "column-major", at least that's the convention but the truth is that Matrix is just a vector like: m0, m4, m8, m12, m1, m5, m9, m13, m2, m6, m10, m14, m3, m7, m11, m15;

    For that reason, all transformations should be transposed to get back the right Matrix in raymath form.

    Please, correct me if I'm wrong. Maybe after dealing a lot with Matrix I just messed things up...



  • hey raysan,
    it may be a good idea to put some notification in the header file about this or in the wiki , I was aware that matrix struct is column major and so I assumed that matrices math is based on that convetion but math in the raymath is row orientation ,
    ( nothing wrong with that , the result is same , just maybe some consistently between matrix struct and functions on memory layout )...
    and one should be aware of this when sending matrix to shader...
    so I will continue with learnopengl tutorials ( as my free time allows me ) and report here any issues...

    thx
  • hey raysan,
    there are couple more quirks in the coordinate systems lesson,
    ( btw he is using right - hand system as default )

    his model matrix ( c++ with glm )
    glm::mat4 model;
    model = glm::rotate(model, glm::radians(-55.0f), glm::vec3(1.0f, 0.0f, 0.0f));

    so model is rotated around x and tilted back ( notice minus 55.0f)

    model matrix in c with raymath
    Matrix model = MatrixRotate((Vector3){1.0f, 0, 0}, 55.0f * DEG2RAD);

    so I have to drop minus of the rotation angle for the model to
    be tilted back to -z axis otherwise ( with minus ) it would tilt forward...

    projection matrix in c with raymath
    Matrix projection = MatrixPerspective(45.0f, (float)WINDOW_WIDTH / (float)WINDOW_HEIGHT, 0.1, 100.0);
    MatrixTranspose(&projection);
    shaderSetMatrix(basicShader, "projection", MatrixToFloat(projection));

    first, it would be useful to know if the fov has to be in rad or in deg without to look in function declaration ( so all angles have to be one or other type ),
    second, projection matrix is transposed twice to make this lesson to work,
    with MatrixTranspose and MatrixToFloat functions,
    ( and I have to guessing that ) ,
    so maybe you should review the way raymath is organised ( with Victor maybe , he is pro ) , because the result of the math is good , but path to there is sometimes confusing...

    thx

  • Thank you very much for pointing all those quirks... Ill try to review raymath for next raylib 1.8.
  • All raymath has been reviewed, no more transposing required and math seem way more consistent now. Implemented in commit: https://github.com/raysan5/raylib/commit/e52032f64660008fdcf0c8d707ef6aed1e6fc32f

    Still some issues with GetMouseRay() result of the changes... hope to address it soon.

    Please, let me know if you try raymath as standalone library! :)
  • @raysan
    so I tried new raymath.h in the coordinate system lesson ( from learnopengl , last part , more cubes ) where 10 cubes are displayed on the screen ,
    this is a link for C++ version

    https://learnopengl.com/code_viewer_gh.php?code=src/1.getting_started/6.3.coordinate_systems_multiple/coordinate_systems_multiple.cpp

    so, as you can see nothing complicated, and this is my main loop in C with raymath as standalone header ( if you need full snipet I will post it )

    while(!glfwWindowShouldClose(window)){

    processInput(window);

    glClearColor(0.2f, 0.3f, 0.3f, 1.0f);
    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

    glActiveTexture(GL_TEXTURE0);
    textureBind(texture1);
    glActiveTexture(GL_TEXTURE1);
    textureBind(texture2);

    Matrix view = MatrixTranslate(0, 0, -3.0f);

    Matrix projection = MatrixPerspective(45.0f, (float)WINDOW_WIDTH / (float)WINDOW_HEIGHT, 0.1, 100.0);

    shaderUse(basicShader);
    shaderSetMatrix(basicShader, "view", MatrixToFloat(view));
    shaderSetMatrix(basicShader, "projection", MatrixToFloat(projection));

    glBindVertexArray(VAO);

    for(unsigned int i = 0; i < 10; i++){
    Matrix trans = MatrixTranslate(cubePositions[i].x, cubePositions[i].y, cubePositions[i].z);

    float angle = 20.0f * i;

    Matrix rot = MatrixRotate((Vector3){1.0f, 0.3f, 0.5f}, angle * DEG2RAD);

    Matrix model = MatrixMultiply(rot, trans);

    shaderSetMatrix(basicShader, "model", MatrixToFloat(model));

    glDrawArrays(GL_TRIANGLES, 0, 36);
    }

    glfwSwapBuffers(window);
    glfwPollEvents();
    }

    but I just cant get the same result, cubes looking too much far on -z axis and fov of the screen is a bit weird too,
    with 1.7 version picture is not accurate too , there are diffs on the rotations of some cubes , but it was better , almost the same...

    plz try this simple lesson yourself and you'll see , something is off...



  • Ill review MatrixPerspective() internals... Just note that, as you proposed, now it works with radians, so its consistent on all the library.
  • ah ok, will change it to this
    Matrix projection = MatrixPerspective(45.0f * DEG2RAD, (float)WINDOW_WIDTH / (float)WINDOW_HEIGHT, 0.1, 100.0);

    with radians fov picture look better , but view matrix is just wrong , that -3.0 on z axis put cubes wayback then it should ,
    MatrixTranslate do some calculation wrong Ray,
    with 1.7 version view matrix was fine , the picture was almost same...
  • edited July 22
    Hey dancho, just reviewed MatrixPerspective(), there was a bug on angle calculation, sorry. Corrected in commit https://github.com/raysan5/raylib/commit/00d2768bc910ca0b6749878f0d142b62d30d55c1

    Just changed MatrixTranslate() as per your original post, actually, it should be that way now, works ok on my examples... View matrix set to -3.0 in z should just move "camera", how are you calculating it on shader?
  • ray, that 0.5f did a trick :smile:
    fov is ok, cubes are same as on his example ,
    view matrix is ok... nice job... gona try some more stuff from learnopengl and report here any issues...

    thy
  • Great to read that! Thanks for the feedback! Let me know any other issue! :)
  • ray,
    couple more things I had noticed from doing last lesson ( camera ),
    first , name convention for vector3 functions
    - it would be nice if they can follow vector2 functions naming ,
    so we have
    RMDEF Vector2 Vector2Zero(void);
    but for vector3
    RMDEF Vector3 VectorZero(void);
    wouldnt be better and logical
    RMDEF Vector3 Vector3Zero(void);

    second , propose for Vector3 Vector3Multiply function
    - in the lesson we have this ( C++ and glm)

    if (glfwGetKey(window, GLFW_KEY_W) == GLFW_PRESS)
    cameraPos += cameraSpeed * cameraFront;

    now in C and raymath it would be something like this ( cameraFront have to remain constant )

    if(glfwGetKey(window, GLFW_KEY_W) == GLFW_PRESS){
    Vector3 temp;
    temp.x = cameraFront.x * cameraSpeed;
    temp.y = cameraFront.y * cameraSpeed;
    temp.z = cameraFront.z * cameraSpeed;
    cameraPos = VectorAdd(cameraPos, temp);
    }

    but with Vector3Multiply function it could be shorten to this

    if(glfwGetKey(window, GLFW_KEY_W) == GLFW_PRESS){
    cameraPos = VectorAdd(cameraPos, Vector3Multiply(cameraFront, cameraSpeed));
    }

    definition for Vector3Multiply

    // Multiply vector by scalar
    RMDEF Vector3 Vector3Multiply(Vector3 v, float scalar)
    {
    v.x *= scalar;
    v.y *= scalar;
    v.z *= scalar;

    return v;
    }

    something similar could be done for vector3 divided by a nonzero scalar...

    thy

  • edited July 22
    Hey! Thanks for the review!

    About naming convention, I agree, it would be better for consistency despite being a big change. Originally raymath only contained functionality for Vector3, so the original naming... but that's a logic update.

    About Vector3Multiply(), it's a great addition, count also on that!

    EDIT: By the way, notice that VectorScale() exist but it's not the same...
  • @raysan,

    so I am doing Lighting tutorials and there is a necessity for the Vector3 by Vector3 multiplication, maybe you could also add it to the raymath.h...

    // Multiply vector by vector
    RMDEF Vector3 Vector3MultiplyV(Vector3 v1, Vector3 v2)
    {
    Vector3 result;

    result.x = v1.x * v2.x;
    result.y = v1.y * v2.y;
    result.z = v1.z * v2.z;

    return result;
    }

    and then to avoid name conflict

    // Multiply vector by scalar
    RMDEF Vector3 Vector3MultiplyF(Vector3 v, float scalar)
    {
    v.x *= scalar;
    v.y *= scalar;
    v.z *= scalar;

    return v;
    }


  • Ok, Just added Vector3MultiplyV(), Vector3Multiply() can stay for scalar.
Sign In or Register to comment.