Binding to OpenGL 3.x VBO

Question

I'm trying to update my engine that used to use OpenGL 2.x style vertex arrays to work with OpenGL 3.x, which means updating to VAOs/VBOs. I think I'm not binding to VBO's properly. Read below for more info or skip to the code and find what I'm doing wrong.

A quick overview of my mesh classes looks something like this:

Mesh

• root MeshNode

MeshNode

• transform
• VAO index
• index VBO index
• array of child MeshNodes
• array of MeshObjects

MeshObject

• all the vertex and index data loaded from file for a single piece of the total mesh
• vertex VBO index

If I draw a MeshNode with only one MeshObject, it seems to draw fine. When I draw a MeshNode with multiple MeshObjects, I get something that's the general shape of the model I'm trying to draw, but kind of garbled.

I've checked the vertex data in the Visual Studio debugger and the VBO data through gDEbugger and it all looks fine, so I'm pretty sure loading from file and loading into VBOs is working.

I used gDEbugger to force it to draw points for all vertices instead of triangles and it has the shape of a single MeshObject, which leads me to believe I'm just not binding to different VBOs properly. As if it's trying to draw with different indices, but the same vertices every time.

VertexData looks like this:

struct VertexData
{
    enum
    {
        NUM_TEXCOORDS = 1,
    };
    vector3 vertex;
    vector3 normal;
    vector2 texCoord[NUM_TEXCOORDS];
};

Relevant MeshNode code:

void MeshNode::initVAO(void)
{
    closeVAO();

    unsigned int scan;

    //init index data
    if (m_meshObjects.size() > 0)
    {
        glGenVertexArrays(1, &m_meshVAO);
        glBindVertexArray(m_meshVAO);
        {
            //add up the total index count for all the mesh objects in this node
            unsigned int indexCount = 0;
            for (scan = 0; scan < m_meshObjects.size(); ++scan)
            {
                indexCount = indexCount + m_meshObjects[scan].getIndices()->size();
            }
            //make the actual index buffer
            glGenBuffers(1, &m_indexVBO);
            glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_indexVBO);
            {
                glBufferData(GL_ELEMENT_ARRAY_BUFFER, indexCount * sizeof(unsigned short), NULL, GL_STATIC_DRAW);

                //set up VBOs and fill the index buffer with the index data from each mesh object
                unsigned int offset = 0;
                for (scan = 0; scan < m_meshObjects.size(); ++scan)
                {
                    m_meshObjects[scan].initVBOs(offset);
                }
            }
            glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
        }
        glBindVertexArray(0);
    }
    for (scan = 0; scan < m_childMeshNodes.size(); ++scan)
    {
        m_childMeshNodes[scan]->initVAO();
    }
}

void MeshNode::closeVAO(void)
{
    if (m_meshVAO != 0)
    {
        glBindVertexArray(m_meshVAO);
        {
            glDeleteBuffers(1, &m_indexVBO);
            glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
        }
        glBindVertexArray(0);
        glDeleteVertexArrays(1, &m_meshVAO);
        m_meshVAO = 0;
        m_indexVBO = 0;
    }
}

void MeshNode::render(const matrix4 &_parentTransform)
{
    matrix4 transform = _parentTransform * m_transform;

    if (m_meshObjects.size() > 0)
    {
        glBindVertexArray(m_meshVAO);
        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_indexVBO);
        {
            for (unsigned int objectScan = 0; objectScan < m_meshObjects.size(); ++objectScan)
            {
                m_meshObjects[objectScan].render(transform);
            }
        }
        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
        glBindVertexArray(0);
    }

    for (unsigned int childScan = 0; childScan < m_childMeshNodes.size(); ++childScan)
    {
        m_childMeshNodes[childScan]->render(transform);
    }
}

Relevant MeshObject code:

void MeshObject::initVBOs(unsigned int& _indexOffset)
{
    //sub in this section of the index data
    m_indexOffset = _indexOffset;
    _indexOffset = _indexOffset + m_indices.size();
    glBufferSubData(GL_ELEMENT_ARRAY_BUFFER, m_indexOffset * sizeof(unsigned short), m_indices.size() * sizeof(unsigned short), &(m_indices[0]));

    //init vertex data
    glGenBuffers(1, &m_vertexVBO);
    glBindBuffer(GL_ARRAY_BUFFER, m_vertexVBO);
    {
        glBufferData(GL_ARRAY_BUFFER, m_data.size() * sizeof(VertexData), &(m_data[0]), GL_STATIC_DRAW);

        glVertexAttribPointer(Shader::POSITION, 3, GL_FLOAT, GL_FALSE, sizeof(VertexData), (char*)0);
        glEnableVertexAttribArray(Shader::POSITION);
        glVertexAttribPointer(Shader::NORMAL, 3, GL_FLOAT, GL_FALSE, sizeof(VertexData), (char*)12);
        glEnableVertexAttribArray(Shader::NORMAL);
        glVertexAttribPointer(Shader::TEXCOORD0, 2, GL_FLOAT, GL_FALSE, sizeof(VertexData), (char*)24);
        glEnableVertexAttribArray(Shader::TEXCOORD0);
    }
    glBindBuffer(GL_ARRAY_BUFFER, 0);
}

void MeshObject::closeVBOs(void)
{
    glDeleteBuffers(1, &m_vertexVBO);

    m_vertexVBO = 0;
}

void MeshObject::render(const matrix4& _transform)
{
    m_material->bind(_transform);
    glBindBuffer(GL_ARRAY_BUFFER, m_vertexVBO);
    {
        glEnableVertexAttribArray(Shader::POSITION);
        glEnableVertexAttribArray(Shader::NORMAL);
        glEnableVertexAttribArray(Shader::TEXCOORD0);
        glDrawRangeElements(GL_TRIANGLES, m_indexOffset, m_indexOffset + m_indices.size(), m_indices.size(), GL_UNSIGNED_SHORT, (char*)0);
        glDisableVertexAttribArray(Shader::POSITION);
        glDisableVertexAttribArray(Shader::NORMAL);
        glDisableVertexAttribArray(Shader::TEXCOORD0);
    }
    glBindBuffer(GL_ARRAY_BUFFER, 0);
}

Answer 1

A quick overview of my mesh classes looks something like this:

I think your scene graph hierarchy is a bit muddled. All a Node in the scene graph needs is a transform, the list of Node children, and a list of meshes to draw in that Node. It should not have an element buffer or a VAO; those are conceptually part of the mesh data.

Indeed, your problem stems from that last point. Vertex Array Objects contain the state set by glVertexAttrib(I)Pointer calls. That means, each time you call MeshObject::initVBOs within the same MeshNode, you are overwriting the data set by the previous call.

Each mesh needs its own VAO, not the node. The meshes can share index data, which is what you seem to be doing (though they should also share the same buffer object for their vertex data, if you're concerned about having too many buffers). But the VAOs need to be different. Multiple VAOs can refer to the same element buffer.

Answer 2

I think you're making incorrect use of glDrawRangeElements. The start and end parameters are the minimal and maximal vertex index that might occur in the index array, but you provide then m_indexOffest and m_indexOffset+m_indices.size(), which is the index-data range you render and not necessarily the range of the vertex indices inside those index arrays.

And as a side note, a VAO encapsulates every vertex array/buffer state, all buffer bindings, the pointers and the enabled flags, so you're making many unneccessary calls. Just bind the index buffer in the initVAO method and then it always gets bound when you bind the VAO in the MeshNode::render method (of course in this case omit all the glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0) calls). Likewise do you only need to enable the attribtue arrays in the initVAO method (assuming all MeshObjects have the same attributes enabled) and they automatically get enabled when you bind the VAO. In your current configuration the VAO is completely unneccessary and you don't proft from it in any way.