6
\$\begingroup\$

I've made an attempt at writing class wrappers around basic OpenGL objects to make managing them easier and more intuitive. Writing a generic one for program uniforms proved to require a little bit more effort than the other objects, due to there being different OpenGL functions for manipulating uniforms of different types. So, I came upon this solution, using templates and inheritance:

/*
    An attempt at type-safe OpenGL shader uniform manipulation

    _UniformBase forms the base class that the implementation for each data type derives from
*/
template <class T>
class _UniformBase
{
protected:
    // Each program-uniform pair is stored in a forward_list
    std::forward_list<std::pair<GLuint,GLint>> Uniforms;

    virtual void SetUniform(const T&, const GLint&) const = 0;      // Must be implemented in child classes
    virtual void SetUniform_DSA(const T&, const GLuint&, const GLint&) const = 0;

public:
    // I don't see the necessity of a copy constructor with the way that I use this class
    _UniformBase() {}
    virtual ~_UniformBase() {}

    // Gets a handle to the OpenGL uniform, given the program and the uniform name
    int Register(_Program Program_in, const char *Name_in)
    {
        GLint Location;
        GLuint Program = Program_in.GetHandle();
        if((Location = glGetUniformLocation(Program, Name_in)) == -1) return 1;
        Uniforms.push_front(std::pair<GLuint, GLint>(Program, Location));
        return 0;
    }

    // Clears out the uniform handle list...haven't used it at all
    void ClearUniforms()
    {
        Uniforms.clear();
    }

    // Sets all stored uniforms to Data_in
    void SetData(const T &Data_in)
    {
        // Can use GL_EXT_direct_state_access extension or plain OpenGL
        // Guess which one I like better
        #ifdef DIRECT_STATE_ACCESS
            for(auto &i : Uniforms)
            {
                SetUniform_DSA(Data_in, i.first, i.second);
            }
        #else
            Data = Data_in;
            GLuint Initial;
            glGetIntegerv(GL_CURRENT_PROGRAM, (GLint*)(&Initial));
            GLuint Previous = Initial;
            for(auto &i : Uniforms)
            {
                if(i.first != Previous)
                {
                    glUseProgram(i.first);
                    Previous = i.first;
                }
                SetUniform(&Data_in, i.second);
            }
            if(Initial != Previous) glUseProgram(Initial);
        #endif
    }

};

/*
    Just a "dummy" class that allows me to make the specializations for each datatype.

    Attempting to instantiate an unimplemented datatype will result in a compile-time error
    due to SetUniform and SetUniform_DSA being pure virtual
    (which is what I want)
*/
template <class T>
class _Uniform : public _UniformBase<T>
{

};

/*
    Here are the implementations of the SetUniform and SetUniform_DSA methods

    Each uses an OpenGL function to set the program uniform
*/
template <>
class _Uniform<glm::mat4> : public _UniformBase<glm::mat4>
{
protected:
    void SetUniform(const glm::mat4& Data, const GLint &Uniform_in) const
    {
        glUniformMatrix4fv(Uniform_in, 1, GL_FALSE, glm::value_ptr(Data));
    }
    void SetUniform_DSA(const glm::mat4& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniformMatrix4fvEXT(Program_in, Uniform_in, 1, GL_FALSE, glm::value_ptr(Data));
    }
};

template <>
class _Uniform<glm::vec4> : public _UniformBase<glm::vec4>
{
protected:
    void SetUniform(const glm::vec4& Data, const GLint &Uniform_in) const
    {
        glUniform4fv(Uniform_in, 1, glm::value_ptr(Data));
    }
    void SetUniform_DSA(const glm::vec4& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform4fvEXT(Program_in, Uniform_in, 1, glm::value_ptr(Data));
    }
};

template <>
class _Uniform<glm::vec3> : public _UniformBase<glm::vec3>
{
protected:
    void SetUniform(const glm::vec3& Data, const GLint &Uniform_in) const
    {
        glUniform3fv(Uniform_in, 1, glm::value_ptr(Data));
    }
    void SetUniform_DSA(const glm::vec3& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform3fvEXT(Program_in, Uniform_in, 1, glm::value_ptr(Data));
    }
};

template <>
class _Uniform<glm::vec2> : public _UniformBase<glm::vec2>
{
protected:
    void SetUniform(const glm::vec2& Data, const GLint &Uniform_in) const
    {
        glUniform2fv(Uniform_in, 1, glm::value_ptr(Data));
    }
    void SetUniform_DSA(const glm::vec2& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform2fvEXT(Program_in, Uniform_in, 1, glm::value_ptr(Data));
    }
};

template <>
class _Uniform<float> : public _UniformBase<float>
{
protected:
    void SetUniform(const float& Data, const GLint &Uniform_in) const
    {
        glUniform1fv(Uniform_in, 1, &Data);
    }
    void SetUniform_DSA(const float& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform1fvEXT(Program_in, Uniform_in, 1, &Data);
    }
};

template <>
class _Uniform<int> : public _UniformBase<int>
{
protected:
    void SetUniform(const int& Data, const GLint &Uniform_in) const
    {
        glUniform1iv(Uniform_in, 1, &Data);
    }
    void SetUniform_DSA(const int& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform1ivEXT(Program_in, Uniform_in, 1, &Data);
    }
};

This allows me to use a pretty simple interface for manipulating these:

_Uniform<glm::vec4> Test;
Test.Register(TheProgram, "SomeVec4");
Test.SetData(glm::vec4(1.0f, 2.0f, 3.0f, 4.0f);

maintaining a degree of type safety.

I like the interface as it stands, as it does what I want it to do. However, I am looking for suggestions for improvement in the implementation, as it is kinda messy.

\$\endgroup\$
2
\$\begingroup\$

Just came across your question, and since you never got any replies from other users, I'll give you a few comments:

The first thing that bothers me with your code is the naming you've chosen. Why appending every class name with _? Did you know that a name beginning with an underscore and an uppercase letter is reserved in any scope for use by the C++ implementation? Read this thread for a full discussion.


Empty constructors/virtual destructor:

_UniformBase() {}
virtual ~_UniformBase() {}

Can be defaulted in C++11, which is now preferred:

_UniformBase() = default;
virtual ~_UniformBase() = default;

But I don't think you need to define a default constructor at all. Don't declared it if it is not needed.


The logic of the return value of _Uniform::Register() is the opposite of what most people would expect. It return 0 on success and 1 on error. This is not a good idea, as most programmers would call it like this: if (unif.Register( ... )), thinking it returns a bool, and then wonder why it always fails. It should return a bool and true on success, false on failure.


You might want to run some tests an profiling and see if there isn't a gain in switching to an std::vector instead of an std::forward_list for storage. The forward list will incur a lot of memory allocations, plus data will be scattered through the memory. Lists make very good containers when you need to insert and remove frequently from arbitrary positions. For a storage that changes infrequently, a vector is usually better. In a shader program/uniform-var list, you only set up the set of variables once, when the the program is created, so I'm almost certain a vector would be better in this case.

\$\endgroup\$
  • 1
    \$\begingroup\$ This was unexpected! Thanks for your input, makes me want to get back into OpenGL programming. You're right about all that, I made really odd decisions for some reason. But, I definitely wasn't aware of the underscore-followed-by-capital rule, only the double-underscore rule; good to know. \$\endgroup\$ – chbaker0 Oct 22 '14 at 1:28
  • 1
    \$\begingroup\$ @mebob - Glad I could help ;) \$\endgroup\$ – glampert Oct 22 '14 at 1:33
3
\$\begingroup\$

I decided to invert the class scheme: instead of having a _UniformBase class and having each specialization derive from it, I now have a single _Uniform class which owns a copy of a _UniformHandler class that provides the specialized uniform-setting functions. This way, I have static polymorphism without too much complication, and with the same external interface.

template <class T>
class _UniformHandler;

template<>
class _UniformHandler<glm::mat4>
{
public:
    inline void SetUniform(const glm::mat4& Data, const GLint &Uniform_in) const
    {
        glUniformMatrix4fv(Uniform_in, 1, GL_FALSE, glm::value_ptr(Data));
    }
    inline void SetUniform_DSA(const glm::mat4& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniformMatrix4fvEXT(Program_in, Uniform_in, 1, GL_FALSE, glm::value_ptr(Data));
    }
};
template <>
class _UniformHandler<glm::vec4>
{
public:
    inline void SetUniform(const glm::vec4& Data, const GLint &Uniform_in) const
    {
        glUniform4fv(Uniform_in, 1, glm::value_ptr(Data));
    }
    inline void SetUniform_DSA(const glm::vec4& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform4fvEXT(Program_in, Uniform_in, 1, glm::value_ptr(Data));
    }
};

template <>
class _UniformHandler<glm::vec3>
{
public:
    inline void SetUniform(const glm::vec3& Data, const GLint &Uniform_in) const
    {
        glUniform3fv(Uniform_in, 1, glm::value_ptr(Data));
    }
    inline void SetUniform_DSA(const glm::vec3& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform3fvEXT(Program_in, Uniform_in, 1, glm::value_ptr(Data));
    }
};

template <>
class _UniformHandler<glm::vec2>
{
public:
    inline void SetUniform(const glm::vec2& Data, const GLint &Uniform_in) const
    {
        glUniform2fv(Uniform_in, 1, glm::value_ptr(Data));
    }
    inline void SetUniform_DSA(const glm::vec2& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform2fvEXT(Program_in, Uniform_in, 1, glm::value_ptr(Data));
    }
};

template <>
class _UniformHandler<float>
{
public:
    inline void SetUniform(const float& Data, const GLint &Uniform_in) const
    {
        glUniform1fv(Uniform_in, 1, &Data);
    }
    inline void SetUniform_DSA(const float& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform1fvEXT(Program_in, Uniform_in, 1, &Data);
    }
};

template <>
class _UniformHandler<int>
{
public:
    inline void SetUniform(const int& Data, const GLint &Uniform_in) const
    {
        glUniform1iv(Uniform_in, 1, &Data);
    }
    inline void SetUniform_DSA(const int& Data, const GLuint &Program_in, const GLint &Uniform_in) const
    {
        glProgramUniform1ivEXT(Program_in, Uniform_in, 1, &Data);
    }
};

template <class T>
class _Uniform
{
protected:
    // Each program-uniform pair is stored in a forward_list
    std::forward_list<std::pair<GLuint,GLint>> Uniforms;
    _UniformHandler<T> UniformHandler;

public:
    _Uniform() {}
    _Uniform(const _Uniform& cp): Uniforms(cp.Uniforms) {}
    _Uniform(_Uniform&& mv): Uniforms(std::move(mv.Uniforms)) {}
    _Uniform& operator=(const _Uniform& cp)
    {
        Uniforms = cp.Uniforms;
        return *this;
    }
    _Uniform& operator=(_Uniform&& mv)
    {
        std::swap(Uniforms, mv.Uniforms);
        return *this;
    }
    ~_Uniform() {}

    // Gets a handle to the OpenGL uniform, given the program and the uniform name
    int Register(_Program Program_in, const char *Name_in)
    {
        GLint Location;
        GLuint Program = Program_in.GetHandle();
        if((Location = glGetUniformLocation(Program, Name_in)) == -1) return 1;
        Uniforms.push_front(std::pair<GLuint, GLint>(Program, Location));
        return 0;
    }

    // Clears out the uniform handle list...haven't used it at all
    void ClearUniforms()
    {
        Uniforms.clear();
    }

    // Sets all stored uniforms to Data_in
    void SetData(const T &Data_in)
    {
        // Can use GL_EXT_direct_state_access extension or plain OpenGL
        // Guess which one I like better
        #ifdef DIRECT_STATE_ACCESS
            for(auto &i : Uniforms)
            {
                UniformHandler.SetUniform_DSA(Data_in, i.first, i.second);
            }
        #else
            Data = Data_in;
            GLuint Initial;
            glGetIntegerv(GL_CURRENT_PROGRAM, (GLint*)(&Initial));
            GLuint Previous = Initial;
            for(auto &i : Uniforms)
            {
                if(i.first != Previous)
                {
                    glUseProgram(i.first);
                    Previous = i.first;
                }
                UniformHandler.SetUniform(&Data_in, i.second);
            }
            if(Initial != Previous) glUseProgram(Initial);
        #endif
    }

};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.