0

I need to overload assignement in one of my opengl based classes : problem is that this case is not so simple, because it uses Opengl "objects" : so copying class attributes is not enough. So what would be the correct way to overload the =, and copy constructor methods ? Here is my code :

Shader.h :

#pragma once

#include <GL/glew.h>
#include <map>
#include <string>
#include "alpha/LogManager.h"
#include "alpha/Component.h"
#include "alpha/Exceptions.h"
#include <glm/glm.hpp>
#include <glm/gtc/type_ptr.hpp>

#define NUM_SHADER_TYPES 3
#define NO_UNIFORM -1

class Shader : public Component
{
public:
    Shader();
    virtual ~Shader();

    void loadFromText(GLenum type, const std::string& src);
    void loadFromFile(GLenum type, const char* fileName);
    void loadFromPreCompiledText(GLenum type, const std::string& src){}
    void loadFromPreCompiledFile(GLenum type, const char* fileName){}
    void CreateAndLink();
    GLuint GetObject() const;
    ///accesses elements : shaders/uniforms;
    GLuint GetAttribLocation(const char* attrib);
    GLuint GetUniformLocation(const char* unif);
    void UpdateUniformMatrix4f(const char* unif, const glm::mat4& val, GLboolean inverse = false);
    void UpdateUniformMatrix3f(const char* unif, const glm::mat3& val, GLboolean inverse = false);
    void UpdateUniformMatrix2f(const char* unif, const glm::mat2& val, GLboolean inverse = false);
    void Bind() const override;
    void UnBind() const override;
    void Dispose() override;

    ///MEMBER FUNCTION GENERATORS
    #define __ALPHA_SHADER_PROGRAM_UNIFORM_DECLERATION(OGL_TYPE, TYPE_SUFFIX)\
        void UpdateUniform1##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0); \
        void UpdateUniform2##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0, OGL_TYPE v1); \
        void UpdateUniform3##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0, OGL_TYPE v1, OGL_TYPE v2); \
        void UpdateUniform4##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0, OGL_TYPE v1, OGL_TYPE v2, OGL_TYPE v3); \
        void UpdateUniform1##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count = 1);\
        void UpdateUniform2##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count = 1);\
        void UpdateUniform3##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count = 1);\
        void UpdateUniform4##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count = 1);

    ///Implement Methods
    __ALPHA_SHADER_PROGRAM_UNIFORM_DECLERATION(GLfloat, f);
    __ALPHA_SHADER_PROGRAM_UNIFORM_DECLERATION(GLdouble, d);
    __ALPHA_SHADER_PROGRAM_UNIFORM_DECLERATION(GLint, i);
    __ALPHA_SHADER_PROGRAM_UNIFORM_DECLERATION(GLuint, ui);

    ///undef macro
    #undef __ALPHA_SHADER_PROGRAM_UNIFORM_DECLERATION(OGL_TYPE, TYPE_SUFFIX)

    Shader& operator=(const Shader& other) = delete;
    Shader(const Shader& shader) = delete;

private:
    enum ShaderType{
        VERTEX_SHADER,
        FRAGMENT_SHADER,
        GEOMETRY_SHADER
    };
    int m_numShaders;
    GLuint m_object;
    bool _hasCreated;
    bool _hasDisposed;
    GLuint _shaders[NUM_SHADER_TYPES]; /// VERTEX, FRAGMENT, GEOMETRY
    std::map<std::string, GLuint> _attribList;
    std::map<std::string, GLuint> _unifLocationList;
    std::map<std::string, bool> _registeredUnifs;
};

Shader.cpp

#include "alpha/Shader.h"
#include "alpha/LogManager.h"
#include <fstream>
#include <assert.h>

#define GL_NULL 0

Shader::Shader()
    : m_numShaders(0), m_object(0), _hasCreated(false), _hasDisposed(false)
{
    _shaders[VERTEX_SHADER] = 0;
    _shaders[FRAGMENT_SHADER] = 0;
    _shaders[GEOMETRY_SHADER] = 0;
    _attribList.clear();
    _unifLocationList.clear();
}

Shader::~Shader(){
    Dispose();
}

void Shader::loadFromText(GLenum type, const std::string& text){
    try
    {
        switch (type)
        {
            case GL_VERTEX_SHADER:
                if(glIsShader(_shaders[VERTEX_SHADER]) == GL_TRUE)
                    throw TypeRegisteredTwiceException();
                break;
            case GL_FRAGMENT_SHADER:
                if(glIsShader(_shaders[FRAGMENT_SHADER]) == GL_TRUE)
                    throw TypeRegisteredTwiceException();
                break;
            case GL_GEOMETRY_SHADER:
                if(glIsShader(_shaders[GEOMETRY_SHADER]) == GL_TRUE  )
                    throw TypeRegisteredTwiceException();
                break;
            default:
                throw UnknownTypeException();
                break;
        }

        GLuint shader = glCreateShader(type);
        const char* cstr = text.c_str();
        glShaderSource(shader, 1, &cstr, nullptr);

        ///compile + check shader load status
        GLint status;
        glCompileShader(shader);
        glGetShaderiv(shader, GL_COMPILE_STATUS, &status);

        if(status == GL_FALSE){
            GLint infoLogSize;
            glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &infoLogSize);
            GLchar *infoLog = new GLchar[infoLogSize];
            glGetShaderInfoLog(shader, infoLogSize, nullptr, infoLog);
            LOG_ERROR("Shader", infoLog);
            delete [] infoLog; /// no throw between new and delete !
        }

        if(m_numShaders >= 4)
        {
            throw OutOfBoundsExcpetion();
        }
        _shaders[m_numShaders++]=shader;
    }
    catch(std::exception & e){
        LOG_ERROR_INFO("AN ERROR OCCURED", "\n", e.what() );
        LOG_ERROR("Shader", "IGNORING ADDED SHADER !");
    }
}

void Shader::CreateAndLink(){
    try{
        if(glIsProgram(m_object) == GL_TRUE)
            throw CreatedTwiceException();

        m_object = glCreateProgram();

        ///OK CREATED PROGRAM !
        _hasCreated = true;

        if(_shaders[VERTEX_SHADER] != 0)
            glAttachShader(m_object, _shaders[VERTEX_SHADER]);
        else
            throw NotLoadedException();
        if(_shaders[FRAGMENT_SHADER] != 0)
            glAttachShader(m_object, _shaders[FRAGMENT_SHADER]);
        else
            throw NotLoadedException();
        if(_shaders[GEOMETRY_SHADER] != 0)
            glAttachShader(m_object, _shaders[GEOMETRY_SHADER]);/// OK, not compulsary...

        ///link + check
        GLint status;
        glLinkProgram(m_object);
        glGetProgramiv(m_object, GL_LINK_STATUS, &status);
        if(status == GL_FALSE){
            GLint infoLogSize;
            glGetProgramiv(m_object, GL_INFO_LOG_LENGTH, &infoLogSize);
            GLchar *infoLog = new GLchar[infoLogSize];
            glGetProgramInfoLog(m_object, infoLogSize, nullptr, infoLog);
            delete [] infoLog;
        }

        std::cout << "Shader created : ID = " << m_object << std::endl;

        glDetachShader(m_object, _shaders[VERTEX_SHADER]);
        glDetachShader(m_object, _shaders[FRAGMENT_SHADER]);
        glDetachShader(m_object, _shaders[GEOMETRY_SHADER]);

        glDeleteShader(_shaders[VERTEX_SHADER]);
        glDeleteShader(_shaders[FRAGMENT_SHADER]);
        glDeleteShader(_shaders[GEOMETRY_SHADER]);
    }
    catch(std::exception & e){
         LOG_ERROR_INFO("Shader", "AN ERROR OCCURED\n", e.what() );
         EXIT(1);
    }
}

void Shader::Bind() const{
    glUseProgram(m_object);
}
void Shader::UnBind() const{
    glUseProgram(GL_NULL);
}

GLuint Shader::GetAttribLocation(const char* attrib){
    auto it = _attribList.find(attrib);
    if(it == _attribList.end())
    {
        GLuint loc = glGetAttribLocation(m_object, attrib);
        if(loc == (unsigned)-1)
        {
            LOG_INFO("Shader Warning : Attrib", attrib, "is inactive !");
        }
        it = _attribList.insert({attrib, loc}).first;
    }
    return it->second;
}

GLuint Shader::GetUniformLocation(const char* unif){
    auto it = _unifLocationList.find(unif);
    if(it == _unifLocationList.end())
    {
        GLuint loc = glGetUniformLocation(m_object, unif);
        if(loc == (unsigned)-1)
        {
            LOG_INFO("Shader Warning : Uniform", unif, "is inactive !");
        }
        it = _unifLocationList.insert({unif, loc}).first;
    }
    return it->second;
}


GLuint Shader::GetObject() const{ return m_object; }


void Shader::UpdateUniformMatrix4f(const char* unif, const glm::mat4& val, GLboolean inverse){
    glUniformMatrix4fv(this->GetUniformLocation(unif), 1, inverse, glm::value_ptr(val));
}

void Shader::UpdateUniformMatrix3f(const char* unif, const glm::mat3& val, GLboolean inverse){
    glUniformMatrix3fv(this->GetUniformLocation(unif), 1, inverse, glm::value_ptr(val));
}

void Shader::UpdateUniformMatrix2f(const char* unif, const glm::mat2& val, GLboolean inverse){
    glUniformMatrix2fv(this->GetUniformLocation(unif), 1, inverse, glm::value_ptr(val));
}


#define __ALPHA_SHADER_PROGRAM_UNIFORM_IMPLEMENTATION(OGL_TYPE, TYPE_SUFFIX)\
    void Shader::UpdateUniform1##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0)\
    { glUniform1 ## TYPE_SUFFIX (this->GetUniformLocation(name), v0); }\
\
    void Shader::UpdateUniform2##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0, OGL_TYPE v1)\
    { glUniform2 ## TYPE_SUFFIX (this->GetUniformLocation(name), v0, v1); }\
\
    void Shader::UpdateUniform3##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0, OGL_TYPE v1, OGL_TYPE v2)\
    { glUniform3 ## TYPE_SUFFIX (this->GetUniformLocation(name), v0, v1, v2); }\
\
    void Shader::UpdateUniform4##TYPE_SUFFIX(const GLchar* name, OGL_TYPE v0, OGL_TYPE v1, OGL_TYPE v2, OGL_TYPE v3)\
    { glUniform4 ## TYPE_SUFFIX (this->GetUniformLocation(name), v0, v1, v2, v3); }\
\
    void Shader::UpdateUniform1##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count) /** sizei=1 by default */\
    { glUniform1 ## TYPE_SUFFIX ## v (this->GetUniformLocation(name), count, v); }\
\
    void Shader::UpdateUniform2##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count) /** sizei=1 by default */\
    { glUniform2 ## TYPE_SUFFIX ## v (this->GetUniformLocation(name), count, v); }\
\
    void Shader::UpdateUniform3##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count) /** sizei=1 by default */\
    { glUniform3 ## TYPE_SUFFIX ## v (this->GetUniformLocation(name), count, v); }\
\
    void Shader::UpdateUniform4##TYPE_SUFFIX##v(const GLchar* name, const OGL_TYPE* v, GLsizei count) /** sizei=1 by default */\
    { glUniform4 ## TYPE_SUFFIX ## v (this->GetUniformLocation(name), count, v); }\

__ALPHA_SHADER_PROGRAM_UNIFORM_IMPLEMENTATION(GLfloat, f);
__ALPHA_SHADER_PROGRAM_UNIFORM_IMPLEMENTATION(GLdouble, d);
__ALPHA_SHADER_PROGRAM_UNIFORM_IMPLEMENTATION(GLint, i);
__ALPHA_SHADER_PROGRAM_UNIFORM_IMPLEMENTATION(GLuint, ui);

///undef macro
#undef __ALPHA_SHADER_PROGRAM_UNIFORM_IMPLEMENTATION()

void Shader::loadFromFile(GLenum which, const char* fileName){
    std::ifstream fparser;
    fparser.open(fileName, std::ios_base::in);
    if(fparser){
        ///read + load
        std::string buffer(std::istreambuf_iterator<char>(fparser), (std::istreambuf_iterator<char>()));
        loadFromText(which, buffer);
    }
    else{
        LOG_ERROR_INFO("Shader", "Invalid fileName path", fileName);
    }
}

void Shader::Dispose(){
    if(!_hasDisposed)
    {
        glDeleteProgram(m_object);

        std::cout << "Disposed Shader object : ID was " <<  m_object << std::endl;

        m_object = -1;
        _hasDisposed = true;///no more calls of Dispose() !
    }
}

Edit : I ended up adding a move constructor as you suggested :

Shader::Shader(Shader&& other){
    this->m_object = other.m_object;
    other.m_object = 0;
    this->m_numShaders = other.m_numShaders;
    this->_hasCreated = other._hasCreated;
    this->_hasDisposed = other._hasDisposed;
    for(unsigned i = 0; i < NUM_SHADER_TYPES; ++i)
    {
        this->_shaders[i] = other._shaders[i];
        other._shaders[i] = 0;
    }
    this->_attribList = std::move(other._attribList);
    this->_unifLocationList = std::move(other._unifLocationList);
    this->_unifLocationList = std::move(other._unifLocationList);
}

Thanks for helping :)

  • 1
    Related: http://stackoverflow.com/questions/28929452/mesh-class-called-with-default-constructor-not-working-opengl-c. My answer there mentions a few options for handling this. – Reto Koradi Jul 15 '15 at 15:20
  • Ok... but is my way of wrapping objects good ? Can you help me please :) ? –  Jul 15 '15 at 15:23
  • well don't know how to solve your problem but some notes : 1-you're not using has_created, using it is better than calling glIsProgram(m_object) 2-what's with (loc==(unsigned)-1) ? I assume(unsigned) is a cast, why don't you write (loc==1) ? – niceman Jul 15 '15 at 15:25
  • thank you, i'll change those :) –  Jul 15 '15 at 15:28

2 Answers2

0

OpenGL objects are managed by the driver and if you want to make a deep copy, unless the API exposes the option to do so, you can only create references to it and not a deep copy. AFAIK, OpenGL doesn't expose such a functionality.

E.g. a shader program represented by the integer 1 will be the only instance that the user of the API can handle. Making a copy is not possible. In your C++ object if you encapsulate the same shader object (1), you're essentially making another reference to the same OpenGL object and this without proper reference counting you'll end up with problems like double deletion.

legends2k
  • 31,634
  • 25
  • 118
  • 222
  • So i cannot implement the =operator, ans I should delete them –  Jul 15 '15 at 15:27
  • I would say it's better not to allow copy but only a move. – legends2k Jul 15 '15 at 15:27
  • can you show me how to implement "move" in the edit section pls ? Are you talking about move semantics from C++11 ? –  Jul 15 '15 at 15:29
  • Yes. Reto's linked answer has the details you need. When you move, the resource owned by the moved-from object will be now owned by the moved-to object and the moved-from object will have no resource to own and thus the destructor will be a no-op. – legends2k Jul 15 '15 at 15:30
  • I do not really understand... can you give me an example please ? –  Jul 15 '15 at 15:33
  • Read about move semantics in C++11 [here](http://stackoverflow.com/q/3106110/183120). It's not something to be explained in a comment box with a limitation of 300 chars. – legends2k Jul 15 '15 at 15:34
0

I suppose this is an interesting exercise, since copying a shader program is not as simple as copying the GLuint handle. Off the top of my head, I can think of two options:

  1. Shallow copy: implement the handle as a reference counted shared resource (similar to how shared_ptr works). This is the cheaper option, however, I am guessing that this is not what you want, because doing this would mean that a change in one Shader object will affect all the others.

  2. Deep Copy: Delay the glDetachShader() and glDeleteShader() operations until the destructor (or the Dispose() method), and then recompile the shader program in the copy assignment operator. You can do something like this (without error-checking):

    Shader(const Shader &other)
    {
        DetachAllShaders();
        GLuint shaders[NUM_SHADER_TYPES];
        GLsizei shaderCount;
        glGetAttachedShaders(other.GetObject(), NUM_SHADER_TYPES, &shaderCount, shaders);
    
        for(auto shader : shaders)
        {
            glAttachShader(m_program, shader);
        }
    
        if(other.isLinked())
        {
            glLinkProgram(m_program);
        }
    }
    

If I were you, I wouldn't go for it though. I cannot think of a reason why you would need multiple copies of the same shader program. If you are returning it from a function, for example, I would suggest simply implementing a move constructor. Something simple like:

Shader(Shader &&other)
{
    m_object = other.m_object;
    other.m_object = 0;

    _attribList = std::move(other._attribList);
    // ...etc
}

In which case deleting m_object is silently ignored by OpenGL when it is 0.

Nasser Al-Shawwa
  • 3,573
  • 17
  • 27
  • Thank you :) I think I'm going to do the same thing for all my gl wrappers ! Is it good, though for a library ? Wouldn't it bother the user to no beeing able to use the = operator ? Though I think I'll wrap those classes too (like in gameObject)... –  Jul 15 '15 at 15:54
  • Maybe it would bother the user, but it would bother her even more if there are performance issues with the program due to multiple recompilations for example. This will encourage them to think of better designs for their program. Take a look at [QOpenGLShaderProgram](http://doc.qt.io/qt-5/qopenglshaderprogram.html) for example. They just implement a default constructor, no copy anything. Their source code is available online if you want to compare implementations and learn. – Nasser Al-Shawwa Jul 15 '15 at 16:03
  • I posted the new move contructor in my question . –  Jul 15 '15 at 21:10