Shader class implementation

Question

I'm trying to design a Shader class. For now, I'm just trying to get basic outline for handling uniforms. Some design goals I aimed for is to avoid passing a string to a function as this is error prone and has no type checking. There's really only one place this can go wrong: if you mistype the name of the uniform when defining it.

There is a lot of potential for code bloat. I could potentially move the Name out of the template parameters for Parameter and store it as a member variable, which would get assigned in the default constructor.

template<typename ParamType>
class Shader
{
public:
 template<typename Type, const char* (*Name)()>
 class Parameter
 {
 static const char* name;
 static GLuint location;
 static GLuint program;
 template<typename> friend class Shader;
 };
 template<typename Type, const char* (*Name)()>
 void SetParameter(const Parameter<Type,Name>&, const Type& value )
 {
 // static_assert(!std::is_same<Type,Type>::value, "Unsupported type, needs specialization.");
 typedef Parameter<Type,Name> Param;
 if(program != -1 && Param::program != program)
 {
 // set Param::location
 Param::program = program;
 }
 std::cout << Param::name << std::endl;
 }
private:
 GLuint program;
};
template<typename ParamType>
template<typename Type, const char* (*Name)()>
const char* Shader<ParamType>::Parameter<Type, Name>::name = Name();
template<typename ParamType>
template<typename Type, const char* (*Name)()>
GLuint Shader<ParamType>::Parameter<Type, Name>::program = -1;
#include "shaderparameters.inl"

shaderparameters.inl:

#define MY_PARAMETER(shader, type, name) \
 private: static const char* ShaderParameterStr##shader##type##name##() { return #name ; } \
 public: typedef Shader< shader >::Parameter< type, &ShaderParameterStr##shader##type##name > name;
struct ShaderParam
{
 struct Generic
 {
 MY_PARAMETER( Generic, Vec4, position )
 MY_PARAMETER( Generic, Mat4, projection )
 };
};
#undef MY_PARAMETER

Usage:

Shader<ShaderParam::Generic> s;
s.SetParameter(ShaderParam::Generic::projection(), Mat4());
s.SetParameter(ShaderParam::Generic::position(), Vec4());

What are you thoughts on this implementation? Do you have suggestions for improvements or perhaps an entirely different system?

I know I've seen people use hash maps and such, but there's still the problem of continuously using a string literal to set the uniforms and such (I don't really like the thought of using a Macro to define the string, either).

Answer 1

What are you thoughts on this implementation?

It is simply awful. Besides code bloat, just look how you declare such objects, and how you call methods on such objects.

Do you have suggestions for improvements or perhaps an entirely different system?

So, to fix your implementation :

• add an interface for your Shader class. Like that, you can have different shader classes for vertex, geometric and pixel shaders
• remove all templates
• pass the shader's program id to constructor, and remove it from the Parameter class
• move 'class Parameter' out of the 'class Shader', and make it a structure. It's member variables do not need to be static. By the way, name should be std::string - not const char*

• skln: I think it is rather elegant how it all works, there is code bloat, but as I've said it can be reduced quite significantly. You miss the point of why this was done in the first place. The only difference between shaders are the values passed to them, creating a new class for every shader and defining functions to set each parameter is redundant. Shader's program exists in Parameter incase the context is destroyed along with the shader and the shader needs to be reloaded and thus the Parameter location. It shouldn't be std::string, you'd just be wasting twice as much memory.