Using a CRTP approach for loading OpenGL programs

I wanted to try using CRTP (also new for me) to try making the loading of shared resources implicit. (Specifically I'm using it for loading OpenGl shader programs)

This will also help separate some of the boiler plate setup code (combining shaders into a program) from the more specific setup code (assigning locations for shader uniforms), and making sure that no one program is loaded more than once.

If it applies (pretty certain it does), what concepts/ fundamentals am I missing/ not seeing?

By using statics I understand that this isn't thread safe, however I don't think I'll be using multiple threads- but even with that said, any ideas/ tips on that matter would still be much appreciated!

The code lightly uses the Mesh class from the online tutorial series Learning Modern 3D Graphics Programming by Jason L. McKesson, and the source code for that can be found here under framework. The shaders being used are found under Tut 10/ data (but I will post them as they are very short). It also uses glm, freeglut and other libraries found in the glsdk folder of the source code.

CRTP base header, ProgramBase.h

const std::string LOCALFILEDIR = "data\\";
const int PROJECTION_BLOCK_INDEX = 2;

template class ProgramBase
{
public:
    static GLuint program_location();
protected:
    ProgramBase();
    ~ProgramBase(){}
private:
    static GLuint LoadShader(GLenum shader_type_e, const std::string &base_shader_filename);
    static GLuint CreateProgram(const std::vector &shader_locations);

    static GLuint program_location_;
};

The idea is that every CustomProgram will have to define at least three members in order for the below code to work:

• vertex_shader_names_ A container of vertex shaders names to be found and linked

• fragment_shader_names_A list of fragment shaders names to be found and linked

• Init() A function specific to shaders named above that will find uniforms

```
template ProgramBase::ProgramBase()
{
if(program_location_ == -1)

Nothing major, but here are a few things you could change:

-
Since you don't define any special constructor/operator= in ProgramBase, you don't need to explicitly define its destructor. You can let the compiler do the job for you.

-
In C++, std::ifstream has a constructor that can take an std::string instead of a const char*. Therefore, you need not call c_str anymore in the following line:

std::ifstream shader_file((LOCALFILEDIR + base_shader_filename).c_str());

-
When opening a file, the idiomatic way t check for errors is to use operator! which may catch more errors [citation needed] than is_open:

std::ifstream shader_file(LOCALFILEDIR + base_shader_filename);
if(!shader_file)
{
    throw std::runtime_error("Could not find the file " + base_shader_filename);
}

-
To clarify your intent, you could use an std::ostringstream instead of an std::stringstream in the method ProgramBase::LoadShader.

-
This line seems a little bit C-ish:

fprintf(stderr, "%s\n", e.what());

Couldn't you simply replace it by this one?

std::cerr << e.what();

-
Please, only use FULL_CAPS names for macros, not for constants in general. A usual goal of the FULL_CAPS case in C/C++ is to know with a glance that there may be something fishy going on. Since macros were often used to define constants in C89, many people still write their constants in FULL_CAPS case, but pretty please, try to only use this case for macros :)

-
You could use constructor delegation to slightly simplify your constructors:

BasicProgram()
    // Delegate to the other BasicProgram constructor
    : BasicProgram(glm::mat4(1.0f), glm::vec4(1.0f))
{ }

BasicProgram(glm::mat4& model_matrix, glm::vec4& color_vector)
    : model_matrix_(model_matrix), color_vector_(color_vector)
{ }

StackExchange Code Review Q#69135, answer score: 4