C++: Storing resources whose copy operation is unreasonable or impossible

Question

I want to write a ContentManager class which loads and maintains different types of assets for a game (compare with XNA's ContentManager). My header file looks like the following:

class ContentManager
{
public:
  ContentManager(Direct3D& d3d, const std::string& rootDirectory = "Resource");
 ~ContentManager();

  template<typename T>
  const T& Load(const std::string& assetName);

private:
  Direct3D& d3d_;
  std::string rootDirectory_;

  std::map<std::string, Texture> textures_;
};

As you can see, I have a map for each asset type (only for textures at the moment) and a generic Load<T>() method, which I instantiate explicitly for each asset type I want to store. Load<Texture>() reads an image file from disk (if it's not already in the map), creates a new Texture, inserts it into the map and returns it.

My Texture class basically creates and encapsulates a raw IDirect3DTexture9 to follow the RAII idiom (the destructor calls Release() on the texture_):

class Texture
{
public:
  Texture();
  Texture(Direct3D& d3d, unsigned int width, unsigned int height, 
    const std::vector<unsigned char>& stream);

  ~Texture();

  IDirect3DTexture9* GetD3DTexture() const { return texture_; }

private:
  IDirect3DTexture9* texture_;
};

When testing my code, I realized that each texture was released twice, because (shallow) copies of the Texture objects were created at some point, and the destructor was called for each of these, of course.

Furthermore, although Load<T>() returns a reference to an element of the map, the caller could make a copy himself and trigger the same problem. My thoughts:

• Making the copy constructor of Texture private is no solution, since copies are required anyway when creating an std::pair to insert a new element into the map.

• Defining a custom copy constructor, which creates a deep copy, doesn't seem to be reasonable at all, since I would need to create a copy of the underlying Direct3D texture, which should really exist only once.

So what are my options here? Could this be solved by using smart pointers? Which type should be stored in the map and which type should Load<T>() return?

Answer 1

With C++11, the language added a feature called moving for exactly the reason you are encountering. And luckily, modifying your code to use move mechanics is amazingly simple:

class Texture
{
public:
  Texture() noexcept;
  Texture(Direct3D& d3d, unsigned int width, unsigned int height, 
    const std::vector<unsigned char>& stream);

  Texture(Texture&& rhs) noexcept
  : texture_(rhs.texture_) //take the content from rhs
  {rhs.texture_ = nullptr;} //rhs no longer owns the content

  Texture& operator=(Texture&& rhs) noexcept
  {
    Clear(); //erase previous content if any
    texture_ = rhs.texture_; //take the content from rhs
    rhs.texture_ = nullptr; //rhs no longer owns the content
    return *this;
  }

  ~Texture() noexcept {Clear();}

  IDirect3DTexture9* GetD3DTexture() const noexcept { return texture_; }

private:
  void Clear() noexcept; //does nothing if texture_ is nullptr
  IDirect3DTexture9* texture_;
};

This adds a "move constructor" and a "move assignment", which will move the content from one Texture to another, so that only one points to a given IDirect3DTexture9 at a time. The compiler should detect these two, and stop generating the implicit copy constructor and copy assignment, so your Texture can no longer be copied, which is exactly what you wanted, since deep-copying a IDirect3DTexture9 is hard and doesn't even really make sense. The Texture class is now magically fixed.

Now, what about the other classes? No changes. std::map<std::string, Texture> is smart enough to detect that your class has noexcept move operators, and so it will use them automatically instead of copies. It also makes the map itself movable but not copiable. And since the map is movable but not copiable, that automagically makes ContentManager movable but not copiable. Which makes sense when you think about it, moving content around is fine, but you don't want to copy all of that. So those don't need any changes

---

Now, since rvalues is obviously a new concept to you, here's a crash course:

Texture getter(); //returns a temporary Texture
Texture a = getter(); //since the right hand side is a temporary,
                        //the compiler will try to move construct it, and only 
                        //copy construct if it can't be moved.
a = getter(); //since the right hand side is a temporary,
                        //the compiler will try to move assign it, and only 
                        //copy assign if it can't be moved.

void setter1(Texture&); //receives a reference to an outside texture object
setter1(a); //works exactly as it always has
setter1(getter()); //compiler error, reference to temporary
setter1(std::move(a)); //compiler error, passing rreference && instead of lreference &.

void setter2(Texture); //receives a local texture object
setter2(a); //compiler error, no copy constructor
setter1(getter()); //creates the Texture by moving the data from the temporary
setter2(std::move(a)); //The compiler moves the content from a to the function;
                      //the function receives the content previously held by a, and 
                      //a now has no content.  Careful not to read from a after this.

void setter3(Texture&&); //receives a reference to a temporary texture
setter3(a); //compiler error, a is not a temporary
setter3(getter()); //function receives reference to the temporary, all is good
setter3(std::move(a)); //function thinks a is temporary, and will probably remove 
                       //it's content. Careful not to read from a after this.   

Answer 2

The answer is right at your finger tips: do not use insert :)

1. You forgot the rule of Big Three: if you write any of copy constructor, copy assignment operator or destructor, you should most probably write the other two. In this case, make them private
2. There are many ways to insert elements in a map, insert is just one of them

Pure C++03 solution:

class Texture
{
public:
  Texture(): texture_(0) {}

  Load(Direct3D& d3d, unsigned int width, unsigned int height, 
       const std::vector<unsigned char>& stream);

  ~Texture();

  IDirect3DTexture9* GetD3DTexture() const { return texture_; }

private:
  IDirect3DTexture9* texture_;
};


template<typename T>
T const& ContentManager::Load(const std::string& assetName) {
    Texture& t = textures_[assetName];

    t.Load(....);

    return ...;
}

Now, you first place the object in the map (using its default constructor) and then actually fill it.

In C++11, you have two more alternatives:

• define a move constructor to be able to relocate the object
• use emplace and target the piecewise_construct constructor of pair as in textures_.emplace(std::piecewise_construct, std::make_tuple(assetName), std::make_tuple(...));

Answer 3

Make the constructor private.
Make a map that holds a pointer to the object instead of the object itself.

Answer 4

The best is to keep a reference counter for each object. So instead, I would use a shared pointer (std::map< std::string, std::tr1::shared_ptr< ... > >).

You could also try to use the content manager to release the object as well (ie: Release( T &object )) and keep a reference counter in the map itself. But the best is the shared pointer..

Answer 5

Yes, I would suggest using smart pointers for this. In particular you could use shared_ptr Depending on your compiler version or the frameworks available, you might want want to use the C++11 variety or e.g. Boost's implementation.