BRabbit27 BRabbit27 - 1 month ago 7
C++ Question

Why the destructor gets called before doing the copy?

I have the following code

class Mesh
{
public:
Mesh();
Mesh(std::vector<Vertex> vertices, std::vector<GLuint> indices);
~Mesh();
void draw(Shader& shader);

private:
std::vector<Vertex> mVertices;
std::vector<GLuint> mIndices;
GLuint mVBO;
GLuint mEBO;
};

Mesh::Mesh(std::vector<Vertex> vertices, std::vector<GLuint> indices)
{
mIndices = indices;
glGenBuffers(1, &mEBO);
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, mEBO);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, mIndices.size() * sizeof(GLuint), &mIndices[0], GL_STATIC_DRAW);

mVertices = vertices;
glGenBuffers(1, &mVBO);
glBindBuffer(GL_ARRAY_BUFFER, mVBO);
glBufferData(GL_ARRAY_BUFFER, mVertices.size() * sizeof(Vertex), &mVertices[0], GL_STATIC_DRAW);

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

computeBoundingBox();
}

Mesh::~Mesh()
{
glDeleteBuffers(1, &mVBO);
glDeleteBuffers(1, &mEBO);
}

void Mesh::draw(Shader& shader)
{
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, mEBO);
glBindBuffer(GL_ARRAY_BUFFER, mVBO);

GLuint vpos = glGetAttribLocation(shader.program(), "vPosition");
GLuint vnor = glGetAttribLocation(shader.program(), "vNormal");

glVertexAttribPointer(vpos, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), 0);
glVertexAttribPointer(vnor, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)sizeof(Vector));

shader.bind();
glEnableVertexAttribArray(vpos);
glEnableVertexAttribArray(vnor);
glDrawElements(GL_TRIANGLES, mIndices.size(), GL_UNSIGNED_INT, 0);
shader.unbind();

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

void loadSquare(Mesh& mesh)
{
std::vector<Vertex> vertices;
vertices.push_back(Vertex(Vector(0.5f, 0.5f, 0.f), Vector(1.f, 0.f, 0.f)));
vertices.push_back(Vertex(Vector(-0.5f, 0.5f, 0.f), Vector(0.f, 1.f, 0.f)));
vertices.push_back(Vertex(Vector(-0.5f, -0.5f, 0.f), Vector(0.f, 0.f, 1.f)));
vertices.push_back(Vertex(Vector(0.5f, -0.5f, 0.f), Vector(1.f, 0.f, 1.f)));

std::vector<GLuint> indices;
indices.push_back(0);
indices.push_back(1);
indices.push_back(2);
indices.push_back(0);
indices.push_back(2);
indices.push_back(3);

mesh = Mesh(vertices, indices);
}

int main(int argc, char** argv)
{
// Create opengl context and window
initOGL();

// Create shaders
Shader shader("render.vglsl", "render.fglsl");

Mesh mesh;
loadSquare(mesh);

while (!glfwWindowShouldClose(window))
{
glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

mesh.draw(shader);

glfwSwapBuffers(window);
glfwPollEvents();
}

glfwTerminate();
return 0;
}


If I try to run it it just displays a gray image on the window it creates.

After tracking the application with the debugger, when it hits the line
mesh = Mesh(vertices, indices)
it creates the buffers for OpenGL and copy the vertices and indices std::vectors to the variable
mesh
that was passed as parameter.
However, it also calls the destructor of the object created by
Mesh(vertices,indices)
which in turn invalidate the buffers in the OpenGL context, so when then application reaches
mesh.draw(shader)
the buffers mesh points to are not valid anymore.

Does the move-constructor can help me solve my problem, i.e. avoid the call to the destructor of
Mesh(vertices,indices)
? Are there any other solutions?

Answer

From your source, your are doing the bind to vectors that are immediately destroyed.

Essentially, in your loadSquare function you are creating a mesh and binding it when you write Mesh(vertices, indices); on the right side of the assignment in the last line of loadSquare.

mesh = Mesh(vertices, indices);

Think that line like this:

  ...
  Mesh m1(vertices, indices);  // a 
  mesh= m1;                    // b
  // m1 gets destroyed here.
}

Line (a) creates and binds the mesh.

When you assign it to mesh in line b, mesh.mVertices and mesh.mIndices will get copies of the vectors and mVBO and mEBO will get copies of the bound values.

Think of that line (b) as writing

mesh.mVertices= m1.mVertices;  // mesh gets a new vector with the same values
mesh.mIndices= m1.mIndices;    // mesh gets a new vector with the same values
mesh.mVBO= m1.mVBO;
mesh.mEBO= m1.mEBO;

At the end of loadSquare() m1 will be destroyed (destructor called).

In your calling function you will end up with mesh containing mVBO and mEBO members bound to the vectors that were destroyed. It contains its own vectors, these have the same values, but these are copies in different memory locations that were never bound.

There are various ways to solve this, e.g. returning the square mesh through pointer. Or writing an assignment operator (google for shallow copy).

But my suggestion would be to create a an empty constructor and an additional fillMesh function like your current cotstructor.

Mesh::Mesh(void);            // set mVBO and mEBO to zero.
void Mesh::fillMesh(std::vector<Vertex> vertices, std::vector<GLuint> indices);  // same code as your current constructor.

Then rewrite your loadSquare function like this:

void loadSquare(Mesh& mesh)
{
 std::vector<Vertex> vertices;
 vertices.push_back(Vertex(Vector(0.5f, 0.5f, 0.f), Vector(1.f, 0.f, 0.f)));
 vertices.push_back(Vertex(Vector(-0.5f, 0.5f, 0.f), Vector(0.f, 1.f, 0.f)));
 vertices.push_back(Vertex(Vector(-0.5f, -0.5f, 0.f), Vector(0.f, 0.f, 1.f)));
 vertices.push_back(Vertex(Vector(0.5f, -0.5f, 0.f), Vector(1.f, 0.f, 1.f)));

 std::vector<GLuint> indices;
 indices.push_back(0);
 indices.push_back(1);
 indices.push_back(2);
 indices.push_back(0);
 indices.push_back(2);
 indices.push_back(3);

 mesh.fillMesh(vertices, indices);
}

Thus the loadSquare will create the vertices and indices and set them into the mesh from the calling function and bind them.

Further notes (for a clean solution):

  • The destructor should probably also unbind the vector and indices from GL.
  • The fillMesh function should probably check if the mesh is already bound and unbinds the old vectors before setting and binding the new ones (in case you are calling fillMesh again on an active mesh).
  • You should probably still write an assignment operator that calls fillMesh: Mesh::operator=(const Mesh &other); // google shallow copy
Comments