OpenGL Mesh Class

Question

I've written a simple mesh class. The purpose of it is to build a mesh, draw it to the screen, and provide some means by which the mesh can be transformed/scaled, etc. This was done with GLAD, GLFW, GLM, and OpenGL.

/*
The mesh class takes vertex data, binds VAOs, VBOs, drawing orders, etc, and draws it.
Other classes can inherit from this class.
*/
class Mesh {
private:

    //-------------------------------------------------------------------------------------------
    // GLenum: drawing mode (ie GL_STATIC_DRAW) and primitive type (ie GL_TRIANGLES)
    GLenum DRAW_MODE, PRIMITIVE_TYPE;

    //-------------------------------------------------------------------------------------------
    // Vertex buffer object, vertex array object, element buffer object
    unsigned int VBO, VAO, EBO;

protected:
    //-------------------------------------------------------------------------------------------
    // Vectors holding vertex and index data
    std::vector<Vertex> vertices;
    std::vector<unsigned int> indices;

    //-------------------------------------------------------------------------------------------
    void init() {
        // Generate vertex arrays
        glGenVertexArrays(1, &VAO);
        // Generate VBO
        glGenBuffers(1, &VBO);
        // Generate EBO
        glGenBuffers(1, &EBO);

        // Bind the VAO
        glBindVertexArray(VAO);

        // Bind the buffer
        glBindBuffer(GL_ARRAY_BUFFER, VBO);
        // Detail the VBO buffer data - attach the vertices
        glBufferData(GL_ARRAY_BUFFER, vertices.size() * sizeof(Vertex), &vertices[0], DRAW_MODE);

        // Bind the indices
        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, EBO);
        // Detail the EBO data
        glBufferData(GL_ELEMENT_ARRAY_BUFFER, indices.size() * sizeof(unsigned int),
            &indices[0], DRAW_MODE);

        glEnableVertexAttribArray(0);
        // Tell OpenGL how the vertex data is structured
        glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)0);
        glBindVertexArray(0);
    }

    //-------------------------------------------------------------------------------------------
    void set_vertices(std::vector<Vertex> _vertices) {
        vertices = _vertices;
    }

    //-------------------------------------------------------------------------------------------
    void set_indices(std::vector<unsigned int> _indices) {
        indices = _indices;
    }

    //-------------------------------------------------------------------------------------------
    void set_primitive_type(GLenum _PRIMITIVE_TYPE) {
        PRIMITIVE_TYPE = _PRIMITIVE_TYPE;
    }

    //-------------------------------------------------------------------------------------------
    void set_draw_mode(GLenum _DRAW_MODE) {
        DRAW_MODE = _DRAW_MODE;
    }

public:
    //-------------------------------------------------------------------------------------------
    Mesh(std::vector<Vertex> _vertices, std::vector<unsigned int> _indices,
        GLenum _DRAW_MODE = GL_STATIC_DRAW, GLenum _PRIMITIVE_TYPE = GL_TRIANGLES) {
        this->vertices = _vertices;
        this->indices = _indices;
        this->DRAW_MODE = _DRAW_MODE;
        this->PRIMITIVE_TYPE = _PRIMITIVE_TYPE;
        //std::cout << vertices[0].position.x << std::endl;
        init();
    }

    //-------------------------------------------------------------------------------------------
    // Constructor for an empty mesh. Note: it MUST RECIEVE VERTEX DATA
    Mesh(GLenum _DRAW_MODE = GL_STATIC_DRAW, GLenum _PRIMITIVE_TYPE = GL_TRIANGLES) {
        this->DRAW_MODE = _DRAW_MODE;
        this->PRIMITIVE_TYPE = _PRIMITIVE_TYPE;
    }

    //-------------------------------------------------------------------------------------------
    virtual ~Mesh() {
        glDeleteVertexArrays(1, &VAO);
        glDeleteBuffers(1, &VBO);
        glDeleteBuffers(1, &EBO);
    }

    //-------------------------------------------------------------------------------------------
    virtual void update() {}

    //-------------------------------------------------------------------------------------------
    void draw() {
        // Bind the EBO
        glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, EBO);
        // Bind the vertex array object
        glBindVertexArray(VAO);

        // Ready to draw
        glDrawElements(PRIMITIVE_TYPE, indices.size(), GL_UNSIGNED_INT, 0);
        // Unbind the vertex array (although this isn't entirely necessary)
        glBindVertexArray(0);
    }

    //-------------------------------------------------------------------------------------------
    /* Now I will introduce some simple mesh transformation functions */
    void move(glm::vec3 _position) {
        // Transform each vertex in the given vector to achieve the desired effect
        for (std::size_t i = 0; i < vertices.size(); i++) {
            vertices[i].position += _position;
        }
    }

    //-------------------------------------------------------------------------------------------
    void scale(float factor) {
        // Initalise as identity matrix
        glm::mat3 scaling_matrix = glm::mat3();
        // Multiply by the scaling factor
        scaling_matrix = factor * scaling_matrix;
        // Apply the transformation
        for (std::size_t i = 0; i < vertices.size(); i++) {
            vertices[i].position = scaling_matrix * vertices[i].position;
        }
    }
};

I have also made a simple application of the mesh class: drawing a plane.

// A simple plane, the test shape that we'll use for drawing
class Plane : public Mesh {
private:
    //-------------------------------------------------------------------------------------------
    // Amount of vertices in the x direction
    std::size_t SIZE_X = 100;
    // Amount of vertices in the y direction
    std::size_t SIZE_Y = 100;
    // Width between vertices (x direction)
    std::size_t VERTEX_WIDTH = 1;
    // 'Height' between vertices (y direction)
    std::size_t VERTEX_HEIGHT = 1;

    //-------------------------------------------------------------------------------------------
    std::vector<Vertex> vertices;
    std::vector<unsigned int> indices;

    //-------------------------------------------------------------------------------------------
    // Set up the plane
    void create_mesh_plane() {
        const int w = SIZE_X + 1;
        for (std::size_t i = 0; i < SIZE_X + 1; i++) {
            for (std::size_t j = 0; j < SIZE_Y + 1; j++) {
                Vertex v;
                v.position.x = i * VERTEX_WIDTH;
                v.position.y = j * VERTEX_HEIGHT;
                v.position.z = 0;
                vertices.push_back(v);

                unsigned int n = j * (SIZE_X + 1) + i;

                if (j < SIZE_Y && i < SIZE_X) {
                    // First face
                    indices.push_back(n);
                    indices.push_back(n + 1);
                    indices.push_back(n + w);
                    // Second face
                    indices.push_back(n + 1);
                    indices.push_back(n + 1 + w);
                    indices.push_back(n + 1 + w - 1);
                }
            }
        }

        //-------------------------------------------------------------------------------------------
        set_vertices(vertices);
        set_indices(indices);
        set_primitive_type(GL_TRIANGLES);
        set_draw_mode(GL_STATIC_DRAW);
        init();
    }

public:

    //-------------------------------------------------------------------------------------------
    Plane() {
        create_mesh_plane();
    }

    //-------------------------------------------------------------------------------------------
    ~Plane() {

    }
};

This code works, and you can instantiate a plane object and draw it in your render function and it will work quickly and nicely.

I'm looking at a review on this code because it's going to become a centrepiece of future things I work on with OpenGL and am concerned about the efficiency, particularly:

• Multiple instantiations of mesh-like objects lead to multiple VBOs, causing unnecessary buffer switches.

• The vectors seem like an incredibly inefficient way of storing the data. Particularly because as can be seen in my move() and scale() functions, I'm iterating over them, which is extremely slow in realtime and very detrimental to performance. Also, if I want to dynamically update mesh vertex data (I added a virtual update function for this purpose) it would be extremely slow.

• I could probably split up the init() function such that it doesn't have to be recalled every time the vertex data changes (ie, the drawing order is still the same, I could just feed in the new vertex data if I wanted to update the vertex data of the mesh during its existence).

I'd be grateful for any feedback.

Answer 1

• Use lower case for data members. It's fine for VBO, VAO, EBO since they are abbreviations (and you might consider renaming them as well), but change DRAW_MODE and PRIMITIVE_TYPE to lower case.

• Presumably, Vertex is a struct containing just 3 floats for the position. When setting the attributes, you have a single call to set attribute 0. What happens when you expand your Vertex struct to contain normals, color, uv coordinates, tangents, et cetera? Okay, then you can go back into your init function and add those attributes. But what if a certain mesh doesn't contain color data? Since you have hardcoded the attributes, you have no way of changing it without breaking some other meshes.

• You can use vertices.data() instead of &vertices[0].

• Don't use identifiers beginning with an underscore. See here.

• Pass the vectors using const reference.

• Consider using std::move if you're not going to be using the vector again.

• this is usually omitted by C++ programmers unless one needs to explicitly refer to the current object.

• In draw() you don't need to bind the index buffer again.

• As you mentioned, updating each vertex is horrible tactic. If you have a mesh with 10000 vertices, you will have spent a good part of the time available updating each mesh. Also it might be possible that certain entities share the mesh data (they might be instanced). If you update the data in the mesh, they will all get updated.

  Here's a way to change it. Each entity has a handle to a Mesh (or a pointer) and a model matrix. Any update to position or scale or rotation is reflected in a change to the model matrix. Then, when rendering the entity, you bind your mesh's vertex array and pass the model matrix to the shader, which then combines it with the view-projection matrix to render the object.

  You can of course do other optimizations, such as instanced rendering.

• You have two duplicate vertices and indices vectors, one inherited from Mesh and other you declare as private.

• Consider using move semantics on objects that you're throwing away after use, such as the Vertex in create_mesh_plane().