Simple rendering system in OpenGL

Question

Recently, I added this 'batch' based rendering system in my game engine, which takes care of rendering Spatials (those are actually wrappers for a mesh and a transform) by giving it an environment parameter (taking care of uniform values, like light, fog, environmentMaps, ...). I would like to ask your advice on how I could improve it.

#pragma once

#include <cstdarg>
#include <algorithm>
#include "ResourceManager.h"
#include "../render/Light.h"
#include "../render/Material.h"
#include "../render/Camera.h"
#include "Spatial.h"

class Environment
{
public:

    Environment()
    {
    }

    Environment& Add(const Spiky::PointLight& light)
    {
        m_pointLights.push_back(light);
        return *this;
    }

    Environment& Add(const Spiky::DirectionalLight& light)
    {
        m_directLights.push_back(light);
        return *this;
    }

    std::vector<Spiky::PointLight> GetPointLights()
    {
        return m_pointLights;
    }

    std::vector<Spiky::DirectionalLight> GetDirectionalLights()
    {
        return m_directLights;
    }

private:
    //ShadowMap shadowMap; // those will be implemented later...
    //Clear color;
    //Fog fog;
    std::vector<Spiky::PointLight> m_pointLights;
    std::vector<Spiky::DirectionalLight> m_directLights;
};

struct Renderable
{
    Spiky::Mesh mesh;
    Environment environment;
    const glm::mat4 worldTransform;
    Spiky::Material material;
    GLenum primitiveType;
    Spiky::Shader shader;

    //glm::mat4[] bones;


    Renderable(Spiky::Mesh mesh, const Environment& environment, const glm::mat4& world_transform,
        const Spiky::Material& material, GLenum primitive_type = GL_TRIANGLES, Spiky::Shader shader = Spiky::ResourceManager::GetShader("__STANDART_SHADER"))
        : mesh(mesh),
          environment(environment),
          worldTransform(world_transform),
          material(material),
          primitiveType(primitive_type),
          shader(shader)
    {
        auto uniformBlockIndex = glGetUniformBlockIndex(shader->GetProgram(), "Viewport");
        glUniformBlockBinding(shader->GetProgram(), uniformBlockIndex, 0);
    }
};

class ModelBatch
{
public:
    ModelBatch()
        :   
            m_UBO_Viewport(GL_NONE)
    {
        glGenBuffers(1, &m_UBO_Viewport);
        glBindBuffer(GL_UNIFORM_BUFFER, m_UBO_Viewport);
        glBufferData(GL_UNIFORM_BUFFER, 3 * sizeof(glm::mat4) + sizeof(glm::vec2), nullptr, GL_STATIC_DRAW);
        glBindBuffer(GL_UNIFORM_BUFFER, 0);

        glBindBufferRange(GL_UNIFORM_BUFFER, 0, m_UBO_Viewport, 0, sizeof(glm::mat4));

    }

    void Begin(const Spiky::Camera& camera)
    {
        //Clear all render targets
        m_renderTargets.clear();

    m_camera = camera;

    //Update UBOs --> Common Uniforms
    glBindBuffer(GL_UNIFORM_BUFFER, m_UBO_Viewport);
    glBufferSubData(GL_UNIFORM_BUFFER, 0, sizeof(glm::mat4), glm::value_ptr(camera.Projection()));
    glBufferSubData(GL_UNIFORM_BUFFER, sizeof(glm::mat4), sizeof(glm::mat4), glm::value_ptr(camera.View()));
    glBufferSubData(GL_UNIFORM_BUFFER, 2 * sizeof(glm::mat4), sizeof(glm::mat4), glm::value_ptr(camera.Combined()));
    glBufferSubData(GL_UNIFORM_BUFFER, 3 * sizeof(glm::mat4), sizeof(glm::vec2), glm::value_ptr(glm::vec2(1000.0, 900.0)));
    glBindBuffer(GL_UNIFORM_BUFFER, 0);
}

void Render(const Spiky::Spatial& spatial, const Environment& environment)
{
    m_renderTargets.push_back(Renderable(spatial.GetMesh(), environment,
        spatial.GetTransform().GetModel(), spatial.GetMaterial()));
}

void End() const
{
    for (auto target : m_renderTargets)
    {
        target.shader->Bind();          
        target.shader->UpdateUniform3fv("cameraPosition", glm::value_ptr(m_camera.GetPosition()));
        target.shader->UpdateUniformMatrix4f("model", target.worldTransform);
        target.shader->UpdateUniformMatrix3f("normalMatrix",
            glm::inverseTranspose(glm::mat3(target.worldTransform))
            );

        target.environment.GetPointLights()[0].SetUniforms(target.shader);
        target.environment.GetPointLights()[0].SetUniforms(target.shader);

        target.material.SetUniforms(target.shader);
        target.material.Bind(target.shader);
            target.mesh->Render();
    }
}

private:
    Spiky::Camera m_camera;
    std::vector<Renderable> m_renderTargets;
    GLuint m_UBO_Viewport;
};

main()

#include "../render/render.h"
#include "../core/core.h"
#include "../physics/physics.h"

using namespace Spiky;

int main(int argc, char** args)
{
    Window window(200, 50, 1024, 900, "Spiky engine - release alpha 0.0", true, false);
    window.SetMouseVisible(false);


    Environment environment;
    ModelBatch batch;

    //more Init code...

    //here is the render loop : 
    util::InMainLoop(200, isRunning,
                     [&](float delta)
                     {
                     //Update code...
                     },
                     [&]()
                     {
                         window.SwapBuffers();
                         window.Clear(0.0f, 0.0f, 0.0f, 0.0f);

                         //here, it performs render : 
                     batch.Begin(camera);
                            batch.Render(woodFloor, environment);
                     batch.End();

                     },
                     FPS
    );
    return EXIT_SUCCESS;
}

Answer 1

The expression target.environment.GetPointLights()[0].SetUniforms(target.shader); appears twice in a row.

Is this on purpose? Can you be sure the vector of point lights always has at least one element?

If it doesn't, your code is invalid. If there are multiple lights, won't they set the same uniforms? There's not enough information about Spiky::PointLight to know this.

Also, a small note about the C++11 auto keyword: In the for loop for (auto target : m_renderTargets), you're creating a copy of each of your targets and working on that copy. Try using auto& or even better const Renderable&.

In my experience, the few keystrokes you save by typing auto are not worth the potential pain of fixing the possible compile errors when the type is not what you expect. Writing the actual type will produce a compilation error at that line if the type stored in the vector is not what you expected and also benefits the reader, who no longer has to refer back to the class definition to remember what type you're trying to use.