CroCo CroCo - 5 days ago 4
C++ Question

std::vector and VBOs render only the last shape

I'm running through a weird problem. Basically I have Mesh class depending on a flag, I can draw a point, a line, or a triangle. For example, if I want to draw two lines, I can do the following

Vertex vertices1[] = {
Vertex(glm::vec3(-.5, -.5, 0)),
Vertex(glm::vec3( 0, .5, 0))
};

Vertex vertices2[] = {
Vertex(glm::vec3( .5, -.5, 0)),
Vertex(glm::vec3( -.5, .5, 0))
};

Mesh mesh1(vertices1, sizeof(vertices1)/sizeof(vertices1[0]), 'L');
Mesh mesh2(vertices2, sizeof(vertices2)/sizeof(vertices2[0]), 'L');

// Rendering Loop:
while( Window.isOpen() ){
...
//================( Rendering )=========================
ourShader.Use();
mesh1.draw();
mesh2.draw();
//======================================================
...
}


The result is

enter image description here

Now I would like to use
std::vector<Mesh>
and loop through meshes. My attempt is as follows

std::vector<Mesh> meshes;
meshes.push_back(mesh1);
meshes.push_back(mesh2);

while( Window.isOpen() ){
...
//================( Rendering )=========================
ourShader.Use();
for ( int i(0); i < meshes.size(); ++i )
meshes[i].draw();
//======================================================
...
}


With the preceding approach, only the last line is drawn and this is the result

enter image description here

Moreover, once I use
.push_back()
even if I don't loop through the vector, the last line is drawn. I don't understand why using
std::vector
deteriorates the rendering. I even tried
meshes[0].draw()
but with no luck. Any suggestions?




Edit:
This is the constructor of Mesh class

#include <iostream>
#include <vector>
#include <glm/glm.hpp>
#include <GL/glew.h>
#include <GLFW/glfw3.h>
#include "display.h"
#include "keyboard.h"
#include "shader.h"

class Vertex
{
public:
Vertex(const glm::vec3& p) : m_position(p)
{}

private:
glm::vec3 m_position;

};

class Mesh
{
public:
Mesh(Vertex* vertices, unsigned int numVertices, const char& flag);
~Mesh();
void draw();
private:
enum{
POSITION_VB,
NUM_BUFFERS
};

GLuint m_vertexArrayObject;
GLuint m_vertexArrayBuffers[NUM_BUFFERS];
unsigned int m_drawCount;

char m_flag;
};


Mesh::Mesh(Vertex* vertices, unsigned int numVertices, const char& flag) : m_flag(flag), m_drawCount(numVertices)
{

glGenVertexArrays(1, &m_vertexArrayObject);
glBindVertexArray(m_vertexArrayObject);

glGenBuffers(NUM_BUFFERS, m_vertexArrayBuffers);
glBindBuffer(GL_ARRAY_BUFFER, m_vertexArrayBuffers[POSITION_VB]);
glBufferData(GL_ARRAY_BUFFER, numVertices*sizeof(vertices[0]), vertices, GL_STATIC_DRAW);

glEnableVertexAttribArray(0);

glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, 0);

glBindVertexArray(0);
}

Mesh::~Mesh()
{
glDeleteVertexArrays(1, &m_vertexArrayObject);
glDeleteBuffers(1, m_vertexArrayBuffers);
}

void Mesh::draw()
{
switch(m_flag)
{
case 'P':
glBindVertexArray(m_vertexArrayObject);
glDrawArrays(GL_POINTS, 0, m_drawCount);
glBindVertexArray(0);
break;
case 'L':
glBindVertexArray(m_vertexArrayObject);
glDrawArrays(GL_LINES, 0, m_drawCount);
glBindVertexArray(0);
break;
case 'T':
glBindVertexArray(m_vertexArrayObject);
glDrawArrays(GL_TRIANGLES, 0, m_drawCount);
glBindVertexArray(0);
break;
}

}



int main(void)
{
Display Window(800, 600, "OpenGL Window");
Keyboard myKeyboard( Window.getWindowPointer() );

Vertex vertices1[] = {
Vertex(glm::vec3(-.5, -.5, 0)),
Vertex(glm::vec3( 0, .5, 0))
};

Vertex vertices2[] = {
Vertex(glm::vec3( .5, -.5, 0)),
Vertex(glm::vec3( -.5, .5, 0))
};

Mesh mesh1(vertices1, sizeof(vertices1)/sizeof(vertices1[0]), 'L');
Mesh mesh2(vertices2, sizeof(vertices2)/sizeof(vertices2[0]), 'L');

std::vector<Mesh> meshes;
meshes.emplace_back(mesh1);
meshes.emplace_back(mesh2);
std::cout << meshes.size() << std::endl;
//*****************( SHADER )************************
Shader ourShader("shader.vs", "shader.frag");

glEnable(GL_PROGRAM_POINT_SIZE);


while( Window.isOpen() ){
Window.PollEvents();
Window.clear();

//================( Rendering )=========================
ourShader.Use();
//mesh1.draw();
//mesh2.draw();
for ( int i(0); i < meshes.size(); ++i )
meshes[i].draw();
//meshes[0].draw();
//meshes[1].draw();
//======================================================

Window.SwapBuffers();
}



glfwTerminate();
return 0;
}


Shaders

#version 330 core

out vec4 color;
void main()
{
color = vec4(1.0f,0.5f,0.2f,1.0f);
}

#version 330 core

layout (location = 0) in vec3 position;

void main()
{
gl_PointSize = 10.0;
gl_Position = vec4(position, 1.0);
}

Answer

As I suspected, the problem is with the (lack of) copy constructor. The default one just copies all the members. As a result your VAOs and buffers get deleted multiple times, even before you manage to draw anything (vectors move during reallocation, and if they can't move they copy). As a rule of thumb: if you have a non-default destructor, you must implement also a copy constructor and an assignment operator, or explicitly delete them if your class is not meant to be copyable.

For your concrete case the solutions are:

  1. Quick solution: store pointers to meshes in the vector:

    std::vector<Mesh*> meshes;
    meshes.emplace_back(&mesh1);
    meshes.emplace_back(&mesh2);
    
  2. Correct solution: use proper RAII for resource management. Using the unique_ptr technique from here your MCVE code becomes:

    class Mesh
    {
    public:
        Mesh(Vertex* vertices, unsigned int numVertices, const char& flag);
        void draw();
    private:
        //...
        GLvertexarray m_vertexArrayObject;
        GLbuffer m_vertexArrayBuffers[NUM_BUFFERS];
        unsigned int m_drawCount;
        char m_flag;
    };
    
    
    Mesh::Mesh(Vertex* vertices, unsigned int numVertices, const char& flag) : m_flag(flag), m_drawCount(numVertices)
    {
        GLuint id;
        glGenVertexArrays(1, &id);
        glBindVertexArray(id);
        m_vertexArrayObject.reset(id);
        for(int i = 0; i < NUM_BUFFERS; ++i)
        {
            glGenBuffers(1, &id);
            glBindBuffer(GL_ARRAY_BUFFER, id);
            m_vertexArrayBuffers[i].reset(id);
            glBufferData(GL_ARRAY_BUFFER, numVertices*sizeof(vertices[0]), vertices, GL_STATIC_DRAW);
        }
    
        glEnableVertexAttribArray(0);
        glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, 0);
        glBindVertexArray(0);
    }
    
    void Mesh::draw()
    {
        switch(m_flag)
        {
            case 'P':
                glBindVertexArray(m_vertexArrayObject.get());
                glDrawArrays(GL_POINTS, 0, m_drawCount);
                glBindVertexArray(0);
            break;
            case 'L':
                glBindVertexArray(m_vertexArrayObject.get());
                glDrawArrays(GL_LINES, 0, m_drawCount);
                glBindVertexArray(0);
            break;
            case 'T':
                glBindVertexArray(m_vertexArrayObject.get());
                glDrawArrays(GL_TRIANGLES, 0, m_drawCount);
                glBindVertexArray(0);
            break;
        }
    
    }
    
    int main()
    {
        //...
    
        Mesh mesh1(vertices1, sizeof(vertices1)/sizeof(vertices1[0]), 'L');
        Mesh mesh2(vertices2, sizeof(vertices2)/sizeof(vertices2[0]), 'L');
    
        std::vector<Mesh> meshes;
        meshes.emplace_back(std::move(mesh1));
        meshes.emplace_back(std::move(mesh2));
    
        // ...
    
        return 0;
    }
    

    Notice how there is no more need for defining a destructor, and your class automatically becomes movable but not copyable. Furthermore, if you have OpenGL 4.5 or ARB_direct_state_access then things get even simpler.

Comments