Broken Vertex Data in Mesh Class

Question

I'm trying to implement model loading, but I'm stuck with one problem. When i try to draw a mesh (a single textured quad written by hand for test purposes) for some reason duplicated data associated with the first vertex is passed to the vertex shader (RenderDoc screen).

Here is an example using a stripped down version of my class that still exhibits this behaviour:

#define TEST_MESH

#include <glad/glad.h>
#include <GLFW/glfw3.h>

#include <iostream>
#include <vector>

void framebuffer_size_callback(GLFWwindow* window, int width, int height);
void processInput(GLFWwindow* window);

struct Vertex {
    float position[3];
    float color[3];
};

class Mesh {
public:
    Mesh(std::vector<Vertex> vertices, std::vector<unsigned int> indices);
    ~Mesh();
    Mesh(const Mesh&) = delete;
    Mesh& operator=(const Mesh&) = delete;
    Mesh(Mesh&& other);
    Mesh& operator=(Mesh&& other);

    void Draw(unsigned int program_id);

    std::vector<Vertex> m_Vertices;
    std::vector<unsigned int> m_Indices;

private:
    unsigned int m_VAO, m_VBO, m_EBO;

    void Setup();
    void Release();
};

int main() {
    //GLFW INIT
    glfwInit();
    glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 4);
    glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 6);
    glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE);

    //GLFW Window setup
    const unsigned int SCR_WIDTH = 800;
    const unsigned int SCR_HEIGHT = 600;

    GLFWwindow* window = glfwCreateWindow(SCR_WIDTH, SCR_HEIGHT, "Mesh Test", NULL, NULL);
    if (window == NULL) {
        std::cout << "Failed to create GLFW window" << std::endl;
        glfwTerminate();
        return -1;
    }

    glfwMakeContextCurrent(window);
    glfwSetFramebufferSizeCallback(window, framebuffer_size_callback);

    //GLAD INIT
    if (!gladLoadGLLoader((GLADloadproc)glfwGetProcAddress)) {
        std::cout << "Failed to initialize GLAD" << std::endl;
        return -1;
    }

    //Shader setup:
    const char* vertex = "#version 460 core\n"
                         "layout (location = 0) in vec3 aPos;\n"
                         "layout (location = 1) in vec3 aCol;\n"
                         "out vec3 vColor;\n"
                         "void main() {\n"
                         "   gl_Position = vec4(aPos.x, aPos.y, aPos.z, 1.0);\n"
                         "   vColor = aCol;\n"
                         "}\0";

    const char* fragment = "#version 460 core\n"
                           "in vec3 vColor;\n"
                           "out vec4 FragColor;\n"
                           "void main() {\n"
                           "    FragColor = vec4(vColor, 1.0);\n"
                           "}\0";

    unsigned int vertexShader;
    vertexShader = glCreateShader(GL_VERTEX_SHADER);
    glShaderSource(vertexShader, 1, &vertex, NULL);
    glCompileShader(vertexShader);

    unsigned int fragmentShader;
    fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
    glShaderSource(fragmentShader, 1, &fragment, NULL);
    glCompileShader(fragmentShader);

    unsigned int program;
    program = glCreateProgram();
    glAttachShader(program, vertexShader);
    glAttachShader(program, fragmentShader);
    glLinkProgram(program);

    glDeleteShader(vertexShader);
    glDeleteShader(fragmentShader);

    //Data:
    float floats[24] = {
        -0.5f, 0.5f, 0.0f, 1.0f, 0.0f, 0.0f,
         0.5f, 0.5f, 0.0f, 0.0f, 1.0f, 0.0f,
         0.5f,-0.5f, 0.0f, 0.0f, 0.0f, 1.0f,
        -0.5f,-0.5f, 0.0f, 1.0f, 1.0f ,0.0f };

    unsigned int uints[6] = { 0, 1, 2, 2, 3, 0 };

#ifdef TEST_MESH
    //Mesh assembly
    std::vector<Vertex> vertices;
    for (int i = 0; i < 4; i++)
        vertices.push_back(Vertex{ floats[6 * i], floats[6 * i + 1], floats[6 * i + 2],
                                   floats[6 * i + 3], floats[6 * i + 4], floats[6 * i + 5] });

    std::vector<unsigned int> indices{ 0, 1, 2, 2, 3, 0 };

    Mesh mesh(vertices, indices);
#else
    unsigned int VAO, VBO, EBO;
    glGenVertexArrays(1, &VAO);
    glGenBuffers(1, &VBO);
    glGenBuffers(1, &EBO);

    glBindVertexArray(VAO);

    glBindBuffer(GL_ARRAY_BUFFER, VBO);
    glBufferData(GL_ARRAY_BUFFER, sizeof(floats), &floats[0], GL_STATIC_DRAW);
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, EBO);
    glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(uints), &uints[0], GL_STATIC_DRAW);

    //Positions
    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 6 * sizeof(float), (void*)0);
    glEnableVertexAttribArray(0);
    //Colors
    glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, 6 * sizeof(float), (void*)(3 * sizeof(float)));
    glEnableVertexAttribArray(1);
#endif

    //Render loop
    while (!glfwWindowShouldClose(window)) {
        processInput(window);

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

#ifdef TEST_MESH
        mesh.Draw(program);
#else
        glUseProgram(program);
        glBindVertexArray(VAO);
        glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_INT, 0);
        glBindVertexArray(0);
#endif
        glfwSwapBuffers(window);
        glfwPollEvents();
    }

    glfwTerminate();
    return 0;
}

Mesh::Mesh(std::vector<Vertex> vertices, std::vector<unsigned int> indices) {
    m_Vertices = vertices;
    m_Indices = indices;

    Setup();
}

Mesh::Mesh(Mesh&& other)
    : m_VAO(other.m_VAO), m_VBO(other.m_VBO), m_EBO(other.m_EBO)
    , m_Vertices(other.m_Vertices), m_Indices(other.m_Indices)
{
    other.m_VAO = 0;
    other.m_VBO = 0;
    other.m_EBO = 0;
}

Mesh& Mesh::operator=(Mesh&& other) {
    if (this != &other) {
        Release();

        std::swap(m_VAO, other.m_VAO);
        std::swap(m_VBO, other.m_VBO);
        std::swap(m_EBO, other.m_EBO);

        m_Vertices = other.m_Vertices;
        m_Indices = other.m_Indices;
    }
    return *this;
}

Mesh::~Mesh() {
    Release();
}

void Mesh::Release() {
    glDeleteVertexArrays(1, &m_VAO);
    glDeleteBuffers(1, &m_VBO);
    glDeleteBuffers(1, &m_EBO);
}

void Mesh::Setup() {
    glGenVertexArrays(1, &m_VAO);
    glGenBuffers(1, &m_VBO);
    glGenBuffers(1, &m_EBO);

    glBindVertexArray(m_VAO);

    glBindBuffer(GL_ARRAY_BUFFER, m_VBO);
    glBufferData(GL_ARRAY_BUFFER, m_Vertices.size() * sizeof(Vertex), &m_Vertices[0], GL_STATIC_DRAW);
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_EBO);
    glBufferData(GL_ELEMENT_ARRAY_BUFFER, m_Indices.size() * sizeof(unsigned int), &m_Indices[0], GL_STATIC_DRAW);

    //Positions
    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, position));
    glEnableVertexAttribArray(0);
    //Colors
    glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, color));
    glEnableVertexAttribArray(1);

    glBindBuffer(GL_ARRAY_BUFFER, 0);
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
    glBindVertexArray(0);
}

void Mesh::Draw(unsigned int program_id) {
    glUseProgram(program_id);

    glBindVertexArray(m_VAO);
    glDrawElements(GL_TRIANGLES, m_Indices.size(), GL_UNSIGNED_INT, 0);
}

void processInput(GLFWwindow* window) {
    if (glfwGetKey(window, GLFW_KEY_ESCAPE) == GLFW_PRESS)
        glfwSetWindowShouldClose(window, true);
}

void framebuffer_size_callback(GLFWwindow* window, int width, int 
height) {
    glViewport(0, 0, width, height);
}

Your `setup` function passes a pointer to the storage area of `m_Vertices`. Did you verify that that address is also the storage area of `m_Vertices` after the copy? — Botje, Nov 03 '21 at 12:59
There's not enough here to diagnose the problem. Please reduce your code to an SSCCE. Try to remove your object wrappers and write a new program that only does those operations in order. Oh, and BTW, `Mesh` constructor should take values, not const-refs, if the intent is to move them into the class. — Bartek Banachewicz, Nov 03 '21 at 12:59
@BartekBanachewicz Thank you for your comment. I've edited my post, trying to maximally simplify the problem. — Xenophilius Mildrententus, Nov 03 '21 at 15:02
@Botje Thank you for your comment. You're right, it's not the same. It's not the cause of my problem as it happens even with a single instance of "Mesh" but I'll have to do something with it as well. — Xenophilius Mildrententus, Nov 03 '21 at 15:10
Where do you actually invoke the move constructor from `other.m_Vertices` ? — Botje, Nov 03 '21 at 15:21
@Botje In the current simplified example I don't, but in general I want to use it to move meshes created when constructing a Model into a vector — Xenophilius Mildrententus, Nov 03 '21 at 15:44

Yakov Galka · Accepted Answer · 2021-11-03T15:52:44.757

In your Mesh::Setup you have this line at the end:

glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);

This unbinds your EBO from your VAO. Your implementation seems to treat undefined reads as zero, therefore all you see is the 0th vertex replicated six times.

Once you bound the EBO you don't need to unbind it.

(Note: unbinding GL_ARRAY_BUFFER, on the other hand, is OK. This is because the VBO is attached to the VAO at the time you call any of the *Pointer functions, not at the time you bind it to GL_ARRAY_BUFFER.)

(Note: Since you use the latest OpenGL version, I strongly recommend that you use the Direct State Access (DSA) functions. In this case you would bind the EBO with

glVertexArrayElementBuffer(m_VAO, m_EBO);

call, which I think would make the mistake much more obvious. See a quick reference of VAO state and the recommended functions to use to manipulate it.)

Thank you so much, that was exactly it. – Xenophilius Mildrententus Nov 03 '21 at 16:36 — Xenophilius Mildrententus, Nov 03 '21 at 16:36

Broken Vertex Data in Mesh Class

1 Answers1