OpenGL 4.5 Core Buffer wrapper

Question

I recently wrote this OpenGL buffer wrapper which covers the 4.5 Core specification. I feel like the typed interface could be done better. Any feedback is greatly appreciated.

#ifndef MAKINA_CORE_RENDERING_BACKENDS_OPENGL_BUFFER_HPP_
#define MAKINA_CORE_RENDERING_BACKENDS_OPENGL_BUFFER_HPP_

#include <cstddef>
#include <type_traits>
#include <vector>

#include <makina/core/rendering/backends/opengl/opengl.hpp>

#ifdef MAKINA_OPENGL_CUDA_INTEROP
  #include <cuda_gl_interop.h>
  #include <cuda_runtime_api.h>
#endif

#include <export.hpp>

namespace mak
{
namespace gl
{
template<GLenum target>
class MAKINA_EXPORT buffer
{
public:
  // 6.0 Buffer objects.
  buffer()
  {
    glCreateBuffers(1, &id_);
  }
  buffer(GLuint id) : id_(id), managed_(false)
  {

  }
  buffer(const buffer&  that) : buffer()
  {
    that.is_immutable() 
      ? set_data_immutable(that.size(), nullptr, that.storage_flags())
      : set_data          (that.size(), nullptr, that.usage        ());
    copy_sub_data(that, 0, 0, size());
  }
  buffer(      buffer&& temp) = default;
 ~buffer()
  {
    if(managed_)
      glDeleteBuffers(1, &id_);
  }
  buffer& operator=(const buffer&  that)
  {
    that.is_immutable() 
      ? set_data_immutable(that.size(), nullptr, that.storage_flags())
      : set_data          (that.size(), nullptr, that.usage        ());
    copy_sub_data(that, 0, 0, size());
    return *this;
  }
  buffer& operator=(      buffer&& temp) = default;

  // 6.1 Create and bind buffer objects.
  void        bind      () const
  {
    glBindBuffer(target, id_);
  }
  static void unbind    ()
  {
    glBindBuffer(target, 0);
  }
  template<typename = typename std::enable_if<target == GL_ATOMIC_COUNTER_BUFFER || target == GL_SHADER_STORAGE_BUFFER || target == GL_UNIFORM_BUFFER || target == GL_TRANSFORM_FEEDBACK_BUFFER>::type>
  void        bind_range(GLuint index, GLintptr offset, GLsizeiptr size) const
  {
    glBindBufferRange(target, index, id_, offset, size);
  }
  template<typename = typename std::enable_if<target == GL_ATOMIC_COUNTER_BUFFER || target == GL_SHADER_STORAGE_BUFFER || target == GL_UNIFORM_BUFFER || target == GL_TRANSFORM_FEEDBACK_BUFFER>::type>
  void        bind_base (GLuint index) const
  {
    glBindBufferBase(target, index, id_);
  }

  // 6.2 Create / modify buffer object data (bindless).
  void  set_data_immutable (GLsizeiptr size, const void* data = nullptr, GLbitfield storage_flags = GL_DYNAMIC_STORAGE_BIT)
  {
    glNamedBufferStorage(id_, size, data, storage_flags);
  }
  void  set_data           (GLsizeiptr size, const void* data = nullptr, GLenum     usage         = GL_DYNAMIC_DRAW       )
  {
    glNamedBufferData   (id_, size, data, usage);
  }
  void  set_sub_data       (GLintptr offset, GLsizeiptr size, const void* data)
  {
    glNamedBufferSubData(id_, offset, size, data);
  } 
  void  clear_sub_data     (GLenum internal_format, GLintptr offset, GLsizeiptr size, GLenum format, GLenum data_type, const void* data)
  {
    glClearNamedBufferSubData(id_, internal_format, offset, size, format, data_type, data);
  }
  void  clear_data         (GLenum internal_format,                                   GLenum format, GLenum data_type, const void* data)
  {
    glClearNamedBufferData(id_, internal_format, format, data_type, data);
  }

  // 6.3 Map / unmap buffer data (bindless).
  void* map_range          (GLintptr offset, GLsizeiptr size, GLbitfield access_flags = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT) const
  {
    return glMapNamedBufferRange(id_, offset, size, access_flags);
  }     
  void* map                (                                  GLenum     access       = GL_READ_WRITE) const
  {
    return glMapNamedBuffer(id_, access);
  }
  void  flush_mapped_range (GLintptr offset, GLsizeiptr size) const
  {
    glFlushMappedNamedBufferRange(id_, offset, size);
  }
  void  unmap              () const
  {
    glUnmapNamedBuffer(id_);
  }

  // 6.5 Invalidate buffer data (bindless).
  void  invalidate_sub_data(GLintptr offset, GLsizeiptr size)
  {
    glInvalidateBufferSubData(id_, offset, size);
  }
  void  invalidate         ()
  {
    glInvalidateBufferData(id_);
  }

  // 6.6 Copy between buffers (bindless).
  void  copy_sub_data      (const buffer& source, GLintptr source_offset, GLintptr offset, GLsizeiptr size)
  {
    glCopyNamedBufferSubData(source.id_, id_, source_offset, offset, size);
  }

  // 6.7 Buffer object queries (bindless).
  bool                is_valid     () const
  {
    return glIsBuffer(id_);
  }
  std::vector<GLbyte> sub_data     (GLintptr offset, GLsizeiptr size) const
  {  
    std::vector<GLbyte> data(size);
    glGetNamedBufferSubData(id_, offset, size, static_cast<void*>(data.data()));
    return data;
  }
  GLsizeiptr          size         () const
  {
    return get_parameter(GL_BUFFER_SIZE);
  }
  GLenum              usage        () const
  {
    return get_parameter(GL_BUFFER_USAGE);
  }
  GLenum              access       () const
  {
    return get_parameter(GL_BUFFER_ACCESS);
  }
  GLbitfield          access_flags () const
  {
    return get_parameter(GL_BUFFER_ACCESS_FLAGS);
  }    
  bool                is_mapped    () const
  {
    return get_parameter(GL_BUFFER_MAPPED);
  }
  bool                is_immutable () const
  {
    return get_parameter(GL_BUFFER_IMMUTABLE_STORAGE);
  }
  GLbitfield          storage_flags() const
  {
    return get_parameter(GL_BUFFER_STORAGE_FLAGS);
  }
  GLintptr            map_offset   () const
  {
    return get_parameter_64(GL_BUFFER_MAP_OFFSET);
  }
  GLsizeiptr          map_size     () const
  {
    return get_parameter_64(GL_BUFFER_MAP_LENGTH);
  }
  void*               map_pointer  () const
  {
    void* pointer;
    glGetNamedBufferPointerv(id_, GL_BUFFER_MAP_POINTER, &pointer);
    return pointer;
  }

  GLuint id() const
  {
    return id_;
  }

#ifdef MAKINA_OPENGL_CUDA_INTEROP
  void  cuda_register  (cudaGraphicsMapFlags flags = cudaGraphicsMapFlagsNone)
  {
    if (resource_ != nullptr)
      cuda_unregister();
    cudaGraphicsGLRegisterBuffer(&resource_, id_, flags);
  }
  void  cuda_unregister()
  {
    if (resource_ == nullptr)
      return;
    cudaGraphicsUnregisterResource(resource_);
    resource_ = nullptr;
  }

  template<typename type> 
  type* cuda_map  ()
  {
    type*  buffer_ptr;
    size_t buffer_size;
    cudaGraphicsMapResources(1, &resource_, nullptr);
    cudaGraphicsResourceGetMappedPointer(static_cast<void**>(&buffer_ptr), &buffer_size, resource_);
    return buffer_ptr;
  }
  void  cuda_unmap()
  {
    cudaGraphicsUnmapResources(1, &resource_, nullptr);
  }
#endif

protected:
  GLint   get_parameter   (GLenum parameter) const
  {
    GLint result;
    glGetNamedBufferParameteriv(id_, parameter, &result);
    return result;
  }
  GLint64 get_parameter_64(GLenum parameter) const
  {
    GLint64 result;
    glGetNamedBufferParameteri64v(id_, parameter, &result);
    return result;
  }

  GLuint id_      = 0;
  bool   managed_ = true;

#ifdef MAKINA_OPENGL_CUDA_INTEROP
  cudaGraphicsResource* resource_ = nullptr;
#endif
};

template<typename type, GLenum target>
class MAKINA_EXPORT typed_buffer : public buffer<target>
{
public:
  // 6.0 Buffer objects.
  using buffer<target>::buffer;
  using buffer<target>::operator=;

  // 6.1 Create and bind buffer objects.
  template<typename = typename std::enable_if<target == GL_ATOMIC_COUNTER_BUFFER || target == GL_SHADER_STORAGE_BUFFER || target == GL_UNIFORM_BUFFER || target == GL_TRANSFORM_FEEDBACK_BUFFER>::type>
  void bind_range(GLuint index, GLintptr offset, GLsizeiptr size) const
  {
    buffer<target>::bind_range(index, sizeof(type) * offset, sizeof(type) * size);
  }

  // 6.2 Create / modify buffer object data (bindless).
  void  set_data_immutable (GLsizeiptr size, const type* data = nullptr, GLbitfield storage_flags = GL_DYNAMIC_STORAGE_BIT)
  {
    buffer<target>::set_data_immutable(sizeof(type) * size, static_cast<void*>(data), storage_flags);
  }
  void  set_data           (GLsizeiptr size, const type* data = nullptr, GLenum     usage         = GL_DYNAMIC_DRAW       )
  {
    buffer<target>::set_data(sizeof(type) * size, static_cast<void*>(data), usage);
  }
  void  set_sub_data       (GLintptr offset, GLsizeiptr size, const type* data)
  {
    buffer<target>::set_sub_data(sizeof(type) * offset, sizeof(type) * size, static_cast<void*>(data));
  }
  void  clear_sub_data     (GLenum internal_format, GLintptr offset, GLsizeiptr size, GLenum format, GLenum data_type, const void* data)
  {
    buffer<target>::clear_sub_data(internal_format, sizeof(type) * offset, sizeof(type) * size, format, data_type, data);
  }

  // 6.3 Map / unmap buffer data (bindless).
  type* map_range          (GLintptr offset, GLsizeiptr size, GLbitfield access_flags = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT) const
  {
    return static_cast<type*>(buffer<target>::map_range(sizeof(type) * offset, sizeof(type) * size, access_flags));
  }
  type* map                (                                  GLenum     access       = GL_READ_WRITE) const
  {
    return static_cast<type*>(buffer<target>::map(access));
  }
  void  flush_mapped_range (GLintptr offset, GLsizeiptr size) const
  {
    buffer<target>::flush_mapped_range(sizeof(type) * offset, sizeof(type) * size);
  }

  // 6.5 Invalidate buffer data (bindless).
  void  invalidate_sub_data(GLintptr offset, GLsizeiptr size)
  {
    buffer<target>::invalidate_sub_data(sizeof(type) * offset, sizeof(type) * size);
  }

  // 6.6 Copy between buffers (bindless).
  void  copy_sub_data      (const buffer<target>& source, GLintptr source_offset, GLintptr offset, GLsizeiptr size)
  {
    buffer<target>::copy_sub_data(source, sizeof(type) * source_offset, sizeof(type) * offset, sizeof(type) * size); 
  }

  // 6.7 Buffer object queries (bindless).
  std::vector<GLbyte> sub_data   (GLintptr offset, GLsizeiptr size) const
  {
    return buffer<target>::sub_data(sizeof(type) * offset, sizeof(type) * size);
  }
  GLsizeiptr          size       () const
  {
    return buffer<target>::size() / sizeof(type);
  }
  GLintptr            map_offset () const
  {
    return buffer<target>::map_offset() / sizeof(type);
  }
  GLsizeiptr          map_size   () const
  {
    return buffer<target>::map_size() / sizeof(type);
  }
  type*               map_pointer() const
  {
    return static_cast<type*>(buffer<target>::map_pointer());
  }
};

template<typename type> using array_buffer              = typed_buffer<type, GL_ARRAY_BUFFER>;
template<typename type> using atomic_counter_buffer     = typed_buffer<type, GL_ATOMIC_COUNTER_BUFFER>;
template<typename type> using copy_read_buffer          = typed_buffer<type, GL_COPY_READ_BUFFER>;
template<typename type> using copy_write_buffer         = typed_buffer<type, GL_COPY_WRITE_BUFFER>;
template<typename type> using dispatch_indirect_buffer  = typed_buffer<type, GL_DISPATCH_INDIRECT_BUFFER>;
template<typename type> using draw_indirect_buffer      = typed_buffer<type, GL_DRAW_INDIRECT_BUFFER>;
template<typename type> using element_array_buffer      = typed_buffer<type, GL_ELEMENT_ARRAY_BUFFER>;
template<typename type> using pixel_pack_buffer         = typed_buffer<type, GL_PIXEL_PACK_BUFFER>;
template<typename type> using pixel_unpack_buffer       = typed_buffer<type, GL_PIXEL_UNPACK_BUFFER>;
template<typename type> using query_buffer              = typed_buffer<type, GL_QUERY_BUFFER>;
template<typename type> using shader_storage_buffer     = typed_buffer<type, GL_SHADER_STORAGE_BUFFER>;
template<typename type> using texture_buffer            = typed_buffer<type, GL_TEXTURE_BUFFER>;
template<typename type> using transform_feedback_buffer = typed_buffer<type, GL_TRANSFORM_FEEDBACK_BUFFER>;
template<typename type> using uniform_buffer            = typed_buffer<type, GL_UNIFORM_BUFFER>;
template<typename type> using vertex_buffer             = array_buffer        <type>;
template<typename type> using index_buffer              = element_array_buffer<type>;
}
}

#endif

Answer 1

template<GLenum target>

As I've explained elsewhere, buffer objects are not typed. There's no such thing as a "vertex buffer object", "transform feedback buffer object", "uniform buffer object", or any other such thing. There are just buffer objects. You can transform feedback into a buffer, then use that same buffer as vertex input for rendering, or even as a UBO or SSBO.

So templating your buffer object on the bind target is absolutely incorrect.

buffer(GLuint id) : id_(id), managed_(false)

It's good that you allow your buffer object type to be able to be given an already created buffer object. However, it's not good that you can give it such a buffer without allowing it to adopt ownership of that buffer. That is, if a user creates a buffer object and wants to wrap it in your type, and then allow your type to destroy it, that should be allowed.

Think about how unique_ptr works. Yes, there's make_unique, but you can give it a pointer you allocated yourself and it will delete it.

I'm not saying that this behavior should necessarily be the default. But if you're going to allow wrapping user-created buffers, you should also give the user the option to allow the wrapped buffer to delete it.

Regardless of any of that, this constructor must be explicit. Otherwise, buffer is implicitly convertible from integers, and that is something that can really get out of control. Do you really want someone to be able to pass NULL as the argument to a function that takes a buffer?

buffer(const buffer&  that)

Given the sheer expense of the copy operation (glCopyBufferSubData is not exactly cheap), I would strongly suggest not making it a function of the copy constructor. Make the type non-copyable and give it a member function to do copies, if you even want to support buffer object copying.

buffer(      buffer&& temp) = default;
buffer& operator=(      buffer&& temp) = default;

This is wrong. If managed_ is true, this will result in multiple objects deleting the same OpenGL object. That's bad.

You need a real move constructor. Remember the Rule of 5.

glNamedBufferData   (id_, size, data, usage);
glMapNamedBuffer(id_, access);

I find it curious that you're allowing the creation of buffers using the older APIs, despite still requiring OpenGL 4.5. Sure, those are valid 4.5 calls, but they're effectively obsolete, having been superseeded by superior functions.

---

As for all of the get_* calls, I would personally file those under the YAGNI principle. Yes, OpenGL does allow you to get pretty much any state you set. But really, how often do you ever need to?

Lastly, typed_buffer is not a good type. By combining both the binding target and an object type with the buffer object, you encourage users to allocate lots and lots of buffer objects. This is well known to be a bad idea. You should try to have a few large buffers, and sub-section between them. There's no reason why you should have two separate buffer objects, just because you use different vertex formats.

All of your vertex data ought to be able to live in one buffer. Even if your engine can't really do it, it shouldn't be because it's impossible because of a quirk of your buffer object abstraction.