OpenGL 4.5 Core Buffer wrapper

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
#include
#include

#include

#ifdef MAKINA_OPENGL_CUDA_INTEROP
#include
#include
#endif

#include

namespace mak
{
namespace gl
{
template
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::type>
void bind_range(GLuint index, GLintptr offset, GLsizeiptr size) const
{
glBindBufferRange(target, index, id_, offset, size);
}
template::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, stora

template

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.

StackExchange Code Review Q#161050, answer score: 3