Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mismatch between the comment order and actual data order in main.cpp and potential confusion for learners. #34

Open
MatinAfzal opened this issue Jan 21, 2025 · 0 comments

Comments

@MatinAfzal
Copy link

In the file YoutubeOpenGL 12 - Mesh Class, specifically in main.cpp, we have a Vertex vertices[] array where we store most of our mesh data:

Vertex vertices[] =
{ //               COORDINATES           /            COLORS          /           NORMALS         /       TEXTURE COORDINATES    //
    Vertex{glm::vec3(-1.0f, 0.0f,  1.0f), glm::vec3(0.0f, 1.0f, 0.0f), glm::vec3(1.0f, 1.0f, 1.0f), glm::vec2(0.0f, 0.0f)},
    Vertex{glm::vec3(-1.0f, 0.0f, -1.0f), glm::vec3(0.0f, 1.0f, 0.0f), glm::vec3(1.0f, 1.0f, 1.0f), glm::vec2(0.0f, 1.0f)},
    Vertex{glm::vec3( 1.0f, 0.0f, -1.0f), glm::vec3(0.0f, 1.0f, 0.0f), glm::vec3(1.0f, 1.0f, 1.0f), glm::vec2(1.0f, 1.0f)},
    Vertex{glm::vec3( 1.0f, 0.0f,  1.0f), glm::vec3(0.0f, 1.0f, 0.0f), glm::vec3(1.0f, 1.0f, 1.0f), glm::vec2(1.0f, 0.0f)}
};

The comment above mentions the order as:
COORDINATES - COLORS - NORMALS - TEXTURE COORDINATES

However, the actual data in the Vertex array does not follow this order. The order of the data is as follows:
COORDINATES - NORMALS (first) - COLORS (second) - TEXTURE COORDINATES

The struct Vertex is defined in VBO.h like this:

struct Vertex
{
    glm::vec3 position;
    glm::vec3 normal;
    glm::vec3 color;
    glm::vec2 texUV;
};

In mesh.cpp, the correct rendering of the object relies on this order, and the LinkAttrib calls follow this pattern:

VAO.LinkAttrib(VBO, 0, 3, GL_FLOAT, sizeof(Vertex), (void*)0);
VAO.LinkAttrib(VBO, 1, 3, GL_FLOAT, sizeof(Vertex), (void*)(3 * sizeof(float)));
VAO.LinkAttrib(VBO, 2, 3, GL_FLOAT, sizeof(Vertex), (void*)(6 * sizeof(float)));
VAO.LinkAttrib(VBO, 3, 2, GL_FLOAT, sizeof(Vertex), (void*)(9 * sizeof(float)));

As you can see, there is a swap between LinkAttrib calls for NORMALS and TEXTURE COORDINATES, which is different from the order in previous chapters. Although this is currently working correctly, this could be misleading and confusing for learners.

Proposed Solutions:
1 - Fix the comment to reflect the correct order (i.e., change the comment above Vertex vertices[] to match the actual order of data).

2 - Fix the data order in the struct Vertex and the corresponding LinkAttrib calls to match the order presented in the comment (COORDINATES, COLORS, NORMALS, TEXTURE COORDINATES). This would require changing the order of attributes in the struct and adjusting the LinkAttrib calls accordingly to preserve the correct rendering behavior.

Thank you for maintaining this amazing repository!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant