Basic OpenGL Renderer

Question

I recently added this ShaderBatch to my engine, which takes care of rendering objects which share the same render technique (shader), like billboards and lights. I would like you to point out if there are any relevant changes I could make to my class.

PS: Is the term "ShaderBatch" appropriate ?

ShaderBatch.h:

/*
 * ShaderTechnique.h
 *
 *  Created on: Aug 9, 2015
 *      Author: mattmatt
 */

#pragma once

#include "alpha/Shader.h"
#include "alpha/Light.h"
#include "alpha/Camera.h"
#include "alpha/Transform.h"
#include <glm/glm.hpp>

class ShaderBatch {
public:
    ShaderBatch();
    virtual ~ShaderBatch(){}

    void SetMatrices(const Camera& camera, const Transform& transform);

    void Begin();
    void End();
protected:
    Shader m_shader;
    Camera m_camera;
    Transform m_transform;
};

class LightBatch : public ShaderBatch{
public:
    LightBatch();
    virtual ~LightBatch(){}

    void SetMaterial(Material material){
        m_material = material;
    }
    void SetPointLight(PhongLight light){
        m_light = light;
    }
    void SetCameraPosition(glm::vec3 pos){
        m_camPosition = pos;
    }

    void Begin();
    void End();

private:
    Material m_material;
    PhongLight m_light;
    glm::vec3 m_camPosition;
};

ShaderBatch.cpp:

#include "ShaderBatch.h"

ShaderBatch::ShaderBatch(){}

void ShaderBatch::SetMatrices(const Camera& camera, const Transform& transform){
    m_camera = camera;
    m_transform = transform;
}

void ShaderBatch::Begin(){
     m_shader->Bind();
         m_camera.SetUniforms(m_shader, m_transform);
}

void ShaderBatch::End(){}

LightBatch::LightBatch()
    : ShaderBatch(),
      m_material(CreateTexture("res/images/Debug.png")),
      m_light(PhongLight()),
      m_camPosition(glm::vec3(0.0, 0.0, 0.0))
{
    ShaderBatch::m_shader = CreateShader("res/shaders/basicShader.glslv", "res/shaders/phongShader.glslf");
}

void LightBatch::Begin(){
    ShaderBatch::Begin();
        m_material.Bind(ShaderBatch::m_shader);
            m_material.SetUniforms(ShaderBatch::m_shader);
            m_light.SetUniforms(m_shader);
}

void LightBatch::End(){}

main.cpp:

#include "alpha.h"

#include <btBulletCollisionCommon.h>
#include <btBulletDynamicsCommon.h>

#include "ShaderBatch.h"

static const unsigned WIDTH = 1000.0;
static const unsigned HEIGHT = 900.0;

int main(int argc, char **args){

    Window window(200, 10, WIDTH, HEIGHT, "Alpha engine - release alpha 0.0", true, false);
    window.SetMouseVisible(false);

    Mesh box1 = CreateMesh("res/models/Terrain.obj");

    Shader shader = CreateShader("res/shaders/basicShader.glslv", "res/shaders/phongShader.glslf");

    LightBatch batch;

    Camera camera(glm::vec3(0, 5.0, 2.5), 70.0f, (float)window.GetWidth() / (float)window.GetHeight(), 0.01, 1000.0f);
    Transform transform;
    InputHandler input = InputHandler();
    PhongLight phongLight;
    phongLight.SetPosition(glm::vec3(-1.0f, 5.0f, 1.0f));
    phongLight.SetIntensity(glm::vec3(1.0f, 1.0f, 1.0f));
    phongLight.SetAttenuation(0.01);

    Material material(CreateTexture("res/images/Terrain.jpg"), CreateTexture("res/images/TerrainNormalMap.jpg"));

    bool isRunning = true;
    unsigned FPS = 0;

    transform.GetPos() -= glm::vec3(.0, 8.5, .0);
    transform.GetScale() *= 7;

    util::InMainLoop(2000, isRunning,
    [&](float delta){
        window.Update();
        input.Update();
        isRunning = !(input.CloseRequested() || input.KeyDown(SDL_SCANCODE_ESCAPE));


        if(!input.KeyDown(SDL_SCANCODE_TAB)){
            util::ApplyFreeFPSLook(camera, input, window, delta, 0.01, 5.0, glm::ivec2(WIDTH / 2, HEIGHT / 2));
            window.SetMouseVisible(false);
        }
        else
            window.SetMouseVisible(true);


        if(input.KeyDown(SDL_SCANCODE_RIGHT))
            phongLight.SetPosition(phongLight.GetPosition() += glm::vec3(0.01, 0.0, 0.0));
        if(input.KeyDown(SDL_SCANCODE_LEFT))
            phongLight.SetPosition(phongLight.GetPosition() -= glm::vec3(0.01, 0.0, 0.0));
        if(input.KeyDown(SDL_SCANCODE_UP))
                    phongLight.SetPosition(phongLight.GetPosition() += glm::vec3(0.0, 0.01, 0.0));
        if(input.KeyDown(SDL_SCANCODE_DOWN))
            phongLight.SetPosition(phongLight.GetPosition() -= glm::vec3(0.0, 0.01, 0.0));


        if(input.KeyDown(SDL_SCANCODE_N))
            material = Material(CreateTexture("res/images/Terrain.jpg"),CreateTexture("res/images/TerrainNormalMap.jpg"));
        if(input.KeyDown(SDL_SCANCODE_P))
            material = Material(CreateTexture("res/images/Terrain.jpg") ,CreateTexture("res/images/DefaultNormalMap.jpg"));
        if(input.KeyDown(SDL_SCANCODE_F))
            material = Material(CreateTexture("res/images/Checkered.png") ,CreateTexture("res/images/TerrainNormalMap.jpg"));
        if(input.KeyDown(SDL_SCANCODE_TAB) && input.KeyDown(SDL_SCANCODE_L))
            glDrawMode(GL_LINES);
        if(input.KeyDown(SDL_SCANCODE_TAB) && input.KeyDown(SDL_SCANCODE_S))
                glDrawMode(GL_TRIANGLES);
        if(input.KeyDown(SDL_SCANCODE_TAB) && input.KeyDown(SDL_SCANCODE_P))
                glDrawMode(GL_POINTS);

    },
    [&](){
            window.Clear(.0f, .0f, .0f, .0f);
            /*
            shader->Bind();
            camera.SetUniforms(shader, transform);
            material.SetUniforms(shader);
            phongLight.SetUniforms(shader);

            material.Bind(shader);
*/
            batch.SetMatrices(camera, transform);
            batch.SetMaterial(material);
            batch.SetPointLight(phongLight);
            batch.Begin();

            box1->Draw();

            window.SwapBuffers();
    },
    FPS
    );
    return EXIT_SUCCESS;///YEHE ! EVERYTHING WENT RIGHT !

}

Answer 1

Polling versus events

In the update function you're polling the state. This means that every single frame, you're requesting state. In this case, it's about a few keys but if you have support for multiple players and controllers, this method is going to get computationally expensive. Responding on events is a little more complex, but worth considering for larger projects. Have a look at events in sdl. The same happens in the render function, where you set the material every single frame. Why not set the material when the material changes, so in the update? It is weird that a render function changes materials!

Structure

It may be that copy/pasting your code may have changed it a little, but at certain places you have more spaces than others, sometimes the { is on a newline, sometimes it is on the same, the destructor is in the headers while the constructor is not, ...

C versus C++

The style of CreateShader that returns a shader struct is a C-style construct. The C++ way of doing the same would be to give the Shader class a constructor that does the same as CreateShader.

Const references & move operators

If you pass a parameter by value it is copied over. So that means that every time someone calls SetMaterial, a copy of a Material object is created. The same for parameters in setters and constructors. To prevent useless copying, use const references or move operators where possible.

Remove dead/useless code

There is an end function, but it is never called nor implemented. So just remove it, and perhaps rename begin. (There is no use in creating functions because you think it may be useful in the future).

Virtual/Override

Mark functions you are going to overload as such. Mark the function in the base class virtual and add the override keyword in the derived class. This will help the compiler catch errors.

virtual void ShaderBatch::Begin();
virtual void LightBatch::Begin() override;

ShaderBatch

The naming seems to suggest a collection of meshes to be rendered with the same shader. Currently it is used to store the settings. For me personally, it would sound more logical if there is a list of items to render in that batch, and a call to a render function does all logic required to set-up and render all registered items. (e.g. store box1 (that's a bit of an odd name for a terrain, isn't it?) inside of a list inside of the ShaderBatch)