7
\$\begingroup\$

I created a universal OpenGL object RAII wrapper class, that only takes care of object creation and destruction. Here's my code and reasoning behind it:

I first wrote a class that would take glCreate/Delete*() function pointers as constructor arguments, but I quickly realized that's not the right way to go. I'd have to store the glDelete*() function pointer in each object (waste of memory + dereference overhead). Additionally, OpenGL functions have three different forms (see specializations section, further in my code), so that would require 3 different pointer types - ugly. I decided to change my approach.

So, here I am with this template code. As far as I'm concerned, there should be no unnecessary OpenGL call overhead. The class is standard layout, only contains the object ID and there's no virtual stuff involved, so that seems pretty nice. Also, classes wrapping different OpenGL object types cannot be assigned to each other, because template arguments differ.

I'd like to hear your remarks according my code, the class design and the reasoning behind it. Improvement ideas are also welcome.

This is the main code:

/**
    \brief Serves as a RAII wrapper for OpenGL objects
    \note This class provides no OpenGL error checking - this is up to derived classes.
    \template T is object type as in glObjectLabel
*/
template <GLenum T>
class gl_object
{
    public:
        //! Object type used for labeling
        const static GLenum object_type = T;

        inline gl_object( );
        inline explicit gl_object( GLenum target );

        inline ~gl_object( );

        // Deleted copy constructor and copy assignment operator
        gl_object( const gl_object &src ) = delete;
        gl_object &operator=( const gl_object &rhs ) = delete;

        // Move semantics with source invalidation
        gl_object( gl_object &&src );
        gl_object &operator=( gl_object &&rhs );

        //! Allows casting to object's ID (GLuint)
        inline operator GLuint( ) const noexcept;

        //! Returns object's ID
        inline GLuint id( ) const noexcept;

    protected:
        //! The object ID
        GLuint m_id;
};

//! Move constructor with source invalidation
template <GLenum T>
gl_object<T>::gl_object( gl_object<T> &&src ) :
    m_id( src.m_id )
{
    src.m_id = 0;
}

//! Move assignment operator with source invalidation
template <GLenum T>
gl_object<T> &gl_object<T>::operator=( gl_object<T> &&rhs )
{
    // Prevent self-move
    if ( this != &rhs )
    {
        m_id = rhs.m_id;
        rhs.m_id = 0;
    }

    return *this;
}

// Allows cast to GLuint
template <GLenum T>
gl_object<T>::operator GLuint( ) const noexcept
{
    return m_id;
}

// Used for acquiring object's ID
template <GLenum T>
GLuint gl_object<T>::id( ) const noexcept
{
    return m_id;
}

Then, there are many very similar template specializations. Here are the most interesting ones:

// Specializations for GL_BUFFER
template <>
gl_object<GL_BUFFER>::gl_object( )
{
    glCreateBuffers( 1, &m_id );
}

template <>
gl_object<GL_BUFFER>::~gl_object( )
{
    glDeleteBuffers( 1, &m_id );
}

// Specializations for GL_TEXTURE
template <>
gl_object<GL_TEXTURE>::gl_object( GLenum target )
{
    glCreateTextures( target, 1, &m_id );
}

template <>
gl_object<GL_TEXTURE>::~gl_object( )
{
    glDeleteTextures( 1, &m_id );
}

// Specializations for GL_SHADER
template <>
gl_object<GL_SHADER>::gl_object( GLenum type )
{
    m_id = glCreateShader( type );
}

template <>
gl_object<GL_SHADER>::~gl_object( )
{
    glDeleteShader( m_id );
}

// Specializations for GL_PROGRAM
template <>
gl_object<GL_PROGRAM>::gl_object( )
{
    m_id = glCreateProgram( );
}

template <>
gl_object<GL_PROGRAM>::~gl_object( )
{
    glDeleteProgram( m_id );
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Is the intention that application writers directly use this code, or is it used internally by other classes that wrap OpenGL functionality? An example of how your code is to be used would be helpful. Also, why are templates necessary? Wouldn't a base class for m_id and derived classes that implement the constructors and destructors work? \$\endgroup\$ – G. Sliepen Aug 4 '20 at 13:48
  • \$\begingroup\$ This class was supposed to be a general purpose OpenGL wrapper meant for internal use in a game engine I was making at the time. Base class for m_id would of course work (I think I've even used this approach for a while), but I thought that use of templates instead of a common base class would better emphasize that these objects shouldn't be mixed together (now they are completely distinct types). \$\endgroup\$ – Jacajack Aug 4 '20 at 15:47
3
\$\begingroup\$

Ensuring you have distinct types

You said this in the comments:

I thought that use of templates instead of a common base class would better emphasize that these objects shouldn't be mixed together (now they are completely distinct types).

Two classes that derive from the same base class are also two distinct types. Furthermore, you can make the constructor of the base class protected, so you cannot instantiate a bare base class, and you can use protected or private inheritance to ensure you cannot get a pointer to the base class from a derived class. For example:

class base {
protected:
    base() = default;
};

class derived: base {
    ...
};

derived foo;      // OK
base bar;         // Error: constructor is protected
base *baz = &foo; // Error: base is inaccessible

The problem with template specialization

Your construction using template specialization has a flaw. The problem is that all member functions you might ever want to use in specializations need to be present in the default template. This is why you have:

template <GLenum T>
class gl_object
{
    ...
    inline gl_object( );
    inline explicit gl_object( GLenum target );
    ...
};

The second constructor that takes the target parameter is only used for gl_object<GL_TEXTURE>, and in fact you must have a target for the texture in order to call glCreateTextures(). The problem is now that I can do the following:

gl_object<GL_BUFFER> buffer(GL_TEXTURE_2D);
gl_object<GL_TEXTURE> texture;

The above statements will compile without errors (but perhaps with a warning), because you promised both constructors would exist. Only the linker will give an error, because there is no implementation of those constructors. But in a large project it might be hard to track down this issue.

You won't have this problem with derived classes, since you have full control over which public member functions (including constructors) each derived class has.

Alternatives without templates

There are two ways to implement this without templates. Either you say that, for example, a gl_texture "is a" gl_object, and use class inheritance, or you say that a gl_texture "has a" gl_name (the OpenGL term for the GLuint identifier given to objects), and use composition. In the first case, you'd structure your code as:

class gl_object {
protected:
    GLuint m_id;

    gl_object() = default;

    // delete copy constructors
    // add move semantics
    // access the ID
};

class gl_buffer: gl_object {
public:
    gl_buffer() {
        glCreateBuffers(1, &m_id);
    }
    ...

    // Enable the default constructor and assignment operator:
    gl_buffer(gl_buffer &&other) = default;
    gl_buffer &operator=(gl_buffer &&other) = default;
};

In the second case, you would write:

class gl_name {
private:
    GLuint m_id;

public:
    // delete copy constructors
    // add move semantics
    // access the ID

    GLuint &get() {
        return m_id;
    }
};

class gl_buffer {
private:
    gl_name m_id;

public:
    gl_buffer() {
        glCreateBuffers(1, &m_id.get());
    }
    ...

    // Enable the default constructor and assignment operator:
    gl_buffer(gl_buffer &&other) = default;
    gl_buffer &operator=(gl_buffer &&other) = default;
};

In the second case, since you deleted the copy constructors for gl_name, classes that use that as a member variable will also no longer be default copy constructable.

The main drawback of either two cases is that you need to implement the move constructors and move assignment operators in each derived class. In this case the default move constructor and move assignment operator would have worked just fine, but they have been implicitly deleted. To re-enable them, just explicitly add the default move constructor and move assignemnt operator.

\$\endgroup\$
  • 1
    \$\begingroup\$ The move constructor and move assignment operator for gl_buffer could use = default to simplify things (assuming that worked with any other class members). \$\endgroup\$ – user673679 Aug 5 '20 at 8:24
  • \$\begingroup\$ @user673679 Good point, in this case no other state is in the derived classes that needs different treatment when moving, so enabling the defaults works. \$\endgroup\$ – G. Sliepen Aug 5 '20 at 9:24
  • \$\begingroup\$ For some reason, I haven't thought of using private inheritance, which really helps here. Thank you for your remarks - it's been over a year since I wrote that, but it's better to learn late than never :) \$\endgroup\$ – Jacajack Aug 5 '20 at 20:20
  • 1
    \$\begingroup\$ Thanks to @Mask for editting your question and bringing it back to the front page :) \$\endgroup\$ – G. Sliepen Aug 5 '20 at 20:42
2
\$\begingroup\$

G.Sliepen points out this problem:

gl_object<GL_BUFFER> buffer(GL_TEXTURE_2D);
gl_object<GL_TEXTURE> texture;

Another possible way to avoid this issue is to define the gl_object constructor using variadic template arguments. These arguments are forwarded to a class template specialization for the specific object type:

    template<class... ArgsT>
    gl_object(ArgsT&&... args):
        m_id(gl_object_impl<T>::create(std::forward<ArgsT>(args)...)) { }

    ~object_id()
    {
        if (m_id != GLuint{ 0 })
            gl_object_impl<T>::destroy(m_id);
    }

So we forward declare gl_object_impl in the header with gl_object:

        template<class T>
        struct gl_object_impl;

And each object type provides a specialization along with the rest of the operations for that object:

        template<>
        struct gl_object_impl<GL_FRAMEBUFFER>
        {
            static GLuint create()
            {
                auto id = GLuint{ 0 };
                glGenFramebuffers(1, &id);
                return id;
            }

            static void destroy(GLuint id)
            {
                glDeleteFramebuffers(1, &id);
            }
        };


        class gl_framebuffer : public gl_object<GL_FRAMEBUFFER> { ... };

A few more things:

  • Note that the use of inline on those functions isn't necessary. (Standard compiler settings allow the compiler to decide what to inline, because they're better at that than programmers).

  • The GLuint cast operator should be marked explicit to prevent accidents. (Personally I'd just delete it entirely, since we can do the same thing neatly and obviously with .id().)

\$\endgroup\$
  • 1
    \$\begingroup\$ I added implicit GLuint cast operator only to be able to call OpenGL functions just like I normally would with GLuint 'objects'. Looking back, it seems to be excessive laziness... Thank you for your remarks and showing another approach :) \$\endgroup\$ – Jacajack Aug 5 '20 at 20:27

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.