JoeyDeVries / LearnOpenGL

Code repository of all OpenGL chapters from the book and its accompanying website https://learnopengl.com
https://learnopengl.com
Other
11.07k stars 2.8k forks source link

Tried to rewrite Mesh class with glBufferSubData but it doesn't work correctly #324

Open LooksForFuture opened 2 years ago

LooksForFuture commented 2 years ago

i wanted to rewrite the mesh class to can change attributes more easily with glBufferSubData and this is the output I get: image
But it should be: image
I tracked all the variables and found out the vertices data are being passed to shader, but normals and texture coordinations don't.
This is the costructor of my Mesh class:

// constructor
Mesh::Mesh(string name, vector<glm::vec3> vertices, vector<glm::vec3> normals, vector<glm::vec2> texCoords, vector<unsigned int> indices, vector<Texture> textures) {
    this->name = name;
    this->vertices = vertices;
    this->normals = normals;
    this->texCoords = texCoords;
    this->indices = indices;
    this->textures = textures;

    auto vec3Size = sizeof(glm::vec3);
    auto vec2Size = sizeof(glm::vec2);

    auto vertSize = vertices.size() * vec3Size;
    auto normSize = normals.size() * vec3Size;
    auto texSize = texCoords.size() * vec2Size;

    vector<float> data;

    for (glm::vec3 vec : vertices) {
        data.push_back(vec.x);
        data.push_back(vec.y);
        data.push_back(vec.z);
    }

    for (glm::vec3 vec : normals) {
        data.push_back(vec.x);
        data.push_back(vec.y);
        data.push_back(vec.z);
    }

    for (glm::vec2 vec : texCoords) {
        data.push_back(vec.x);
        data.push_back(vec.y);
    }

    // now that we have all the required data, set the vertex buffers and its attribute pointers.
    // create buffers/arrays
    glGenVertexArrays(1, &VAO);
    glGenBuffers(1, &VBO);
    glGenBuffers(1, &EBO);

    glBindVertexArray(VAO);
    glBindBuffer(GL_ARRAY_BUFFER, VBO);

    glBufferData(GL_ARRAY_BUFFER, vertSize + normSize + texSize, NULL, GL_STATIC_DRAW);
    glBufferSubData(GL_ARRAY_BUFFER, 0, vertSize, vertices.data());
    glBufferSubData(GL_ARRAY_BUFFER, vertSize, vertSize + normSize, normals.data());
    glBufferSubData(GL_ARRAY_BUFFER, vertSize + normSize, vertSize + normSize + texSize, texCoords.data());

    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, vec3Size, 0);
    glEnableVertexAttribArray(0);
    glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, vec3Size, (void*)(vertSize));
    glEnableVertexAttribArray(1);
    glVertexAttribPointer(2, 2, GL_FLOAT, GL_FALSE, vec2Size, (void*)(vertSize + normSize));
    glEnableVertexAttribArray(2);

    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, EBO);
    glBufferData(GL_ELEMENT_ARRAY_BUFFER, indices.size() * sizeof(unsigned int), &indices[0], GL_STATIC_DRAW);
}


But when I store all the attributes in a vector of float named data and change "glBufferData(GL_ARRAY_BUFFER, vertSize + normSize + texSize, NULL, GL_STATIC_DRAW);" to "glBufferData(GL_ARRAY_BUFFER, vertSize + normSize + texSize, data.data(), GL_STATIC_DRAW);" and comment glBufferSubData it works.

JG-Adams commented 2 years ago

I find the use of vector data; a little strange. Might need to check that. I think your normal and texture are supplied with vertex position information. normals.data() & texCoords.data()

The way the orignal code worked you can do it with this: glBufferData(GL_ARRAY_BUFFER, vertices.size() sizeof(Vertex), NULL, GL_STATIC_DRAW); glBufferSubData(GL_ARRAY_BUFFER, 0, vertices.size() sizeof(Vertex), &vertices[0]);

Maybe you could try a single line: glBufferSubData(GL_ARRAY_BUFFER, 0, data.size(), &data[0]);

Three line version: glBufferSubData(GL_ARRAY_BUFFER, 0, vertSize, &(this->vertices)[0]); glBufferSubData(GL_ARRAY_BUFFER, vertSize, vertSize + normSize, &(this->normals)[0]); glBufferSubData(GL_ARRAY_BUFFER, vertSize + normSize, vertSize + normSize + texSize, &(this->texCoords)[0]);

LooksForFuture commented 2 years ago

Thank you for your solution, but it still doesn't work.

JG-Adams commented 2 years ago

Aww. - _ Hmmmm.... O_o

Wait! I see a problem! The stride is wrong in the glVertexAttribPointer. The parameter is: (Index, size, type, normalized, stride, data) Originally the stride is sizeof(Vertex) Vertex is a struct that have all of the data member types. You put in a size of vec3 and 2. but a matter of fact, it should be the size of vertice + tex + normal I don't know the full purpose of having a temporary object data here. You can actually do better without. Remember that it goes out of scope and the information could no longer exist. It was stored in Vertex struct. You can use that instead.

Here is my code that worked for me. (Make sure your shader is right too.) glBufferData(GL_ARRAY_BUFFER, vertices.size() sizeof(Vertex), NULL, GL_STATIC_DRAW); glBufferSubData(GL_ARRAY_BUFFER, 0, vertices.size() sizeof(Vertex), &vertices[0]);

    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, EBO);
    glBufferData(GL_ELEMENT_ARRAY_BUFFER, indices.size() * sizeof(unsigned int), &indices[0], GL_STATIC_DRAW);

    // set the vertex attribute pointers
    glEnableVertexAttribArray(0);
    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)0);
    glEnableVertexAttribArray(1);
    glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, Normal));
    glEnableVertexAttribArray(2);
    glVertexAttribPointer(2, 2, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, TexCoords));

    glEnableVertexAttribArray(3);
    glVertexAttribPointer(3, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, Tangent));
    glEnableVertexAttribArray(4);
    glVertexAttribPointer(4, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, Bitangent));

    glEnableVertexAttribArray(5);
    glVertexAttribIPointer(5, 4, GL_INT, sizeof(Vertex), (void*)offsetof(Vertex, m_BoneIDs));
    glEnableVertexAttribArray(6);
    glVertexAttribPointer(6, 4, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, m_Weights));
    glBindVertexArray(0);

If that doesn't fix it. It could be something outside of the code too. Unless you observed it to happen here. Hope that work for you bud!

LooksForFuture commented 2 years ago

Thanks. I suddenly deleted the folder of my custom mesh class and I don't have a backup. So I use the premade class for the rest of tutorial. BTW, do you know why texture of the first loaded object is assigned to all?

JG-Adams commented 2 years ago

make sure you set the parameter before render. shader.setInt("Texture", i); glActiveTexture(GL_TEXTURE0); glBindTexture(GL_TEXTURE_2D, textureid); Draw.... glBindTexture(GL_TEXTURE_2D, 0); // good practice!

It render your sample based on what is currently set in the texture slots. Your shader sample2D will read from a slot it's set to use. Hence, shader.setInt("Texture", i);