Objects placed inside classes to be rendered with OpenGL

Question

I began to learn OpenGL about a week ago and I now want to create a mesh class. The code I'm about to show gives me a black screen (that's the color I fill it with).

I basically stripped the code from my main function and placed it inside a class, it worked inside main.

mesh.cpp

    #include "mesh.h"

mesh::mesh(std::vector<GLfloat> vertices, std::vector<GLuint> indices)
{
    this->vertices = vertices;
    this->indices = indices;
    glGenVertexArrays(1, &vertexArrayObject);
    glGenBuffers(1, &vertexBuffer);
    glGenBuffers(1, &triangleBuffer);

    glBindVertexArray(vertexArrayObject);


    glBindBuffer(GL_ARRAY_BUFFER, vertexBuffer);
    glBufferData(GL_ARRAY_BUFFER, this->vertices.size() * sizeof(GLfloat), this->vertices.data(), GL_STATIC_DRAW);

    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, triangleBuffer);
    glBufferData(GL_ELEMENT_ARRAY_BUFFER, this->indices.size() * sizeof(GLuint), this->indices.data(), GL_STATIC_DRAW);

    glBindVertexArray(0);

}

mesh::~mesh()
{
    glDeleteBuffers(1, &triangleBuffer);
    glDeleteBuffers(1, &vertexBuffer);

    glDeleteVertexArrays(1, &vertexArrayObject);
}

void mesh::update(){
    glBindVertexArray(vertexArrayObject);
    glDrawElements(GL_TRIANGLES, indices.size(), GL_UNSIGNED_INT, 0);
    glBindVertexArray(0);
}

mesh.h

    #ifndef MESH_H
#define MESH_H

#include <iostream>
#include <vector>
#include <GL/glew.h>
#include <glm/glm.hpp>

class mesh
{
    public:
        mesh(std::vector<GLfloat> vertices, std::vector<GLuint> triangles);
        ~mesh();
        void update();
    protected:
    private:
        GLuint vertexArrayObject, vertexBuffer, triangleBuffer;
        std::vector<GLfloat> vertices;
        std::vector<GLuint> indices;
};

#endif // MESH_H

According to this, this should be the correct way to do it (?).

BTW, all this code is a mashup from this and open.gl sites, here are the variables I pass into the constructor.

For the vertices:

    std::vector<GLfloat> vertices = {
//  Position        Color             Texcoords
-0.5f,  0.5f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, // Top-left
 0.5f,  0.5f, 0.0f, 0.0f, 1.0f, 0.0f, 1.0f, 0.0f, // Top-right
 0.5f, -0.5f, 0.0f, 0.0f, 0.0f, 1.0f, 1.0f, 1.0f, // Bottom-right
-0.5f, -0.5f, 0.0f, 1.0f, 1.0f, 1.0f, 0.0f, 1.0f  // Bottom-left
};

and the indices:

    std::vector<GLuint> elements = {
    0, 1, 2,
    2, 3, 0
};

Also please note that I changed the code snippets according to Anton's suggestions, that didn't seem to work

Answer 1

You have two problems that immediately come to mind:

1. this->triangles.size() * sizeof(GLfloat)

   This problem will not affect anything because GLfloat has the same size as GLuint, but if that was not a typo, it indicates to me you may be thinking about the index buffer wrong.

   • Make this triangles.size () * sizeof (GLuint) for consistency.
     
     

2. glDrawElements(GL_TRIANGLES, this->vertices.size(), GL_UNSIGNED_INT, 0);

   You have two separate lists in your class, vertices and triangles, and this call only cares about the size of one of those.

   • triangles is an array of indices that makes a list of triangles (3 indices per-triangle).

   When you draw your elements array, you pass the number of elements in that list, not the number of vertices in your mesh.

---

While not technically a problem, I think you use this-> a little bit too much when you write your code.

It is useful when you have variables at one scope that have the same name as members in your class (which one could argue builds a case for inadequately descriptive names), but completely unnecessary in functions like mesh::~mesh(). It makes your code harder to read (for me anyway) because the lines are much longer.

Following the theme of inadequately descriptive names, by calling your index array "triangles" you have inadvertently limited this mesh class to drawing triangles. That might be a good variable name for a specialized constructor that builds a triangle mesh, but not so much for a class member. If you use indices instead, that gets rid of the name collision and the need to qualify everything using this->.