OpenGL object in C++ RAII class no longer works

I have an OpenGL object in a C++ class. Since I'm employing RAII, I want to have the destructor delete it. So my class looks something like:

class BufferObject
{
private:
  GLuint buff_;

public:
  BufferObject()
  {
    glGenBuffers(1, &buff_);
  }

  ~BufferObject()
  {
    glDeleteBuffers(1, &buff_);
  }

//Other members.
};

This seems like it works. But any time I do any of the following, I start getting various OpenGL errors when I use it:

vector<BufferObject> bufVec;
{
  BufferObject some_buffer;
  //Initialize some_buffer;
  bufVec.push_back(some_buffer);
}
bufVec.back(); //buffer doesn't work.

BufferObject InitBuffer()
{
  BufferObject buff;
  //Do stuff with `buff`
  return buff;
}

auto buff = InitBuffer(); //Returned buffer doesn't work.

What's going on?

Note: this is an attempt to build a canonical answer to these questions.

Solution 1:

All of those operations copy the C++ object. Since your class did not define a copy constructor, you get the compiler-generated copy constructor. This simply copies all of the members of the object.

Consider the first example:

vector<BufferObject> bufVec;
{
  BufferObject some_buffer;
  //Initialize some_buffer;
  bufVec.push_back(some_buffer);
}
bufVec.back(); //buffer doesn't work.

When you call push_back, it copies some_buffer into a BufferObject in the vector. So, right before we exit that scope, there are two BufferObject objects.

But what OpenGL buffer object do they store? Well, they store the same one. After all, to C++, we just copied an integer. So both C++ objects store the same integer value.

When we exit that scope, some_buffer will be destroyed. Therefore, it will call glDeleteBuffers on this OpenGL object. But the object in the vector will still have its own copy of that OpenGL object name. Which has been destroyed.

So you can't use it anymore; hence the errors.

The same thing happens with your InitBuffer function. buff will be destroyed after it is copied into the return value, which makes the returned object worthless.

This is all due to a violation of the so-called "Rule of 3/5" in C++. You created a destructor without creating copy/move constructors/assignment operators. That's bad.

To solve this, your OpenGL object wrappers should be move-only types. You should delete the copy constructor and copy assignment operator, and provide move equivalents that set the moved-from object to object 0:

class BufferObject
{
private:
  GLuint buff_;

public:
  BufferObject()
  {
    glGenBuffers(1, &buff_);
  }

  BufferObject(const BufferObject &) = delete;
  BufferObject &operator=(const BufferObject &) = delete;

  BufferObject(BufferObject &&other) : buff_(other.buff_)
  {
    other.buff_ = 0;
  }

  BufferObject &operator=(BufferObject &&other)
  {
    //ALWAYS check for self-assignment
    if(this != &other)
    {
      Release();
      buff_ = other.buff_;
      other.buff_ = 0;
    }

    return *this;
  }

  ~BufferObject() {Release();}

  void Release();
  {
    if(buff_)
      glDeleteBuffers(1, &buff_);
  }

//Other members.
};

There are various other techniques for making move-only RAII wrappers for OpenGL objects.

Solution 2:

All of your operations copy the buffer object. But as your class don't have copy constructor, it's just a shallow copy. As your destructor deletes the buffer without further checking, the buffer is deleted with the original object. Nicol Bolas suggested to define a move constructor and delete copy constructor and copy assignment operator, I would describe a different way so both buffers will be usable after a copy.

You can keep a track of how many objects use a single easily with an std::map array. Consider the following example code which is an extension of your code:

#include <map>

std::map<unsigned int, unsigned int> reference_count;

class BufferObject
{
private:
    GLuint buff_;

public:
    BufferObject()
    {
        glGenBuffers(1, &buff_);
        reference_count[buff_] = 1; // Set reference count to it's initial value 1
    }

    ~BufferObject()
    {
        reference_count[buff_]--; // Decrease reference count
        if (reference_count[buff_] <= 0) // If reference count is zero, the buffer is no longer needed
            glDeleteBuffers(1, &buff_);
    }
    
    BufferObject(const BufferObject& other) : buff_(other.buff_)
    {
        reference_count[buff_]++; // Increase reference count
    }
    
    BufferObject operator = (const BufferObject& other)
    {
        if (buff_ != other.buff_) { // Check if both buffer is same
            buff_ = other.buff_;
            reference_count[buff_]++; // Increase reference count
        }
    }

// Other stuffs
};

The code is pretty self-explaining. When the buffer object is initialized, a new buffer is created. Then the constructor creates a new element in reference_count array with the buffer as key and sets its value as 1. Whenever the object is copied, the count increases. An when the object is destructed, the count decreases. Then destructor checks if the count is 0 or less, which mean buffer is no longer needed, so the buffer is deleted.

I recommend to not put the implementation (or least the reference_count array) in a header file so linker errors are not generated.