std::vector and VBOs render only the last shape

Question

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

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

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 1

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.

Answer 2

EDIT

The main problem is, that the destructor is called when you add the Mesh objects to the vector, therefore the underlying data gets cleaned up. Further reading: Why does my class's destructor get called when I add instances to a vector? | What is The Rule of Three?

I'd personally create separate init_buffers and free_buffers methods to my Mesh class and use them appropriately. (Initialize buffers after the OpenGL context is obtained, free the buffers when the window is closed.) This way you can start building meshes (and add them to the scene) before actually having the OpenGL context.

---

I've implemented the missing code parts and tried your code using GLFW using CLion.

It works. See code / CLion project here: OpenGLSandbox/main.cpp

The only code I've added are basically these, so it's your turn to figure out the difference / error.

// Constants
const size_t NUM_BUFFERS = 1;
const size_t POSITION_VB = 0;

// Vertex class
class Vertex {
private:
    glm::vec3 mCoords;
public:
    Vertex(glm::vec3 coords) : mCoords(coords) {};
};

// Mesh class
class Mesh {
private:
    GLuint m_vertexArrayObject;
    char m_flag;
    unsigned int m_drawCount;
    GLuint m_vertexArrayBuffers[NUM_BUFFERS];
public:
    /* your ctor and draw method */
}