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 );
}
2 Answers 2
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.
-
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– user6736792020年08月05日 08:24:56 +00:00Commented Aug 5, 2020 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– G. Sliepen2020年08月05日 09:24:10 +00:00Commented Aug 5, 2020 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– Jacajack2020年08月05日 20:20:10 +00:00Commented Aug 5, 2020 at 20:20
-
1\$\begingroup\$ Thanks to @Mask for editting your question and bringing it back to the front page :) \$\endgroup\$G. Sliepen– G. Sliepen2020年08月05日 20:42:00 +00:00Commented Aug 5, 2020 at 20:42
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 markedexplicit
to prevent accidents. (Personally I'd just delete it entirely, since we can do the same thing neatly and obviously with.id()
.)
-
1\$\begingroup\$ I added implicit
GLuint
cast operator only to be able to call OpenGL functions just like I normally would withGLuint
'objects'. Looking back, it seems to be excessive laziness... Thank you for your remarks and showing another approach :) \$\endgroup\$Jacajack– Jacajack2020年08月05日 20:27:26 +00:00Commented Aug 5, 2020 at 20:27
m_id
and derived classes that implement the constructors and destructors work? \$\endgroup\$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\$