Jump to content

  • Log In with Google      Sign In   
  • Create Account

Resource Pointer Design


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
6 replies to this topic

#1 VReality   Members   -  Reputation: 436

Like
0Likes
Like

Posted 12 July 2012 - 06:26 PM

Working with the D3D API involves handling API resources via pointers. The API user is responsible for disposing of those resources.

In the spirit of modern C++ style, this is a job for some sort of smart pointer. But unique_ptr isn't a great fit, for a couple of reasons.

The way resources are acquired
The D3D API produces resources via "out parameters", which would require explict temporary pointers to transport resource addesses to owning unique_ptrs:
resource_type * pResource;	// Temporary pointer.
D3D_Make_Resource(&pResource);  // D3D style of producing resources.
upResource.reset(pResource);	// unique_ptr's style of assigning.

The way resources are freed
D3D resources are released, not deleted. Not only would unique_ptr require a custom deleter type to free the resources, it would require a unique deleter type per resource type, because each resource class has its own member Release() method, rather than sharing a common non-member function, like delete():
unique_ptr<resource_type, resource_releaser<resource_type>> upResource;

These aren't exactly show stoppers, but they're enough to take a couple of minutes to see if I can't do a little better. So I threw together a resource pointer class along the lines of this:
template<typename T>
  class d3d_resource_ptr
  {
	public:
	  d3d_resource_ptr() : pResource(nullptr) {}
	  ~d3d_resource_ptr() {Release();}

	  T & operator *  () const {return *pResource;}  // Dereferencing works
	  T * operator -> () const {return  pResource;}  // like a normal pointer.

	private:
	  d3d_resource_ptr		 (d3d_resource_ptr const &);  // Unique ownership.  No need
	  d3d_resource_ptr & operator =  (d3d_resource_ptr const &);  // for copying (or moving).

	  void Release() {if(pResource != nullptr){pResource->Release();} pResource = nullptr;}

	  T * pResource;
  };

The question is how best to get and set the raw pointer. Which is the lesser evil?

We could use a reference operator and implicit pointer conversion:
public:
  T * * operator & () {Release(); return &pResource;}  // Pointer to pointer, for reassignment.

  operator T * () const {return pResource;}  // Conversion to T pointer, for storage or passing.

Which provides about as much convenience as possible:
D3D_Make_Resource(&rpResource); // operator & passes a T * * so the API can assign through it.
D3D_Use_Resource(rpPesource);   // operator T * generates a T * so the API can use it.

The evil here is that the resource pointer interface is a little too clever. It's hiding the fact that taking a reference automatically releases any resource (in anticipation of assigning a new one). That's an accident waiting to happen. And I suspect that implicit pointer conversions are trouble makers as well.

Alternatively, we could use explicit functions:
public:
  T * Get() const {return  pResource;}
  T * * Assign() {Release(); return &pResource;}

private:
  d3d_resource_ptr * operator & ();  // Protect against errant references.

Which produces code like this:
D3D_Make_Resource(rpResource.Assign());
D3D_Use_Resource(rpResource.Get());

I guess the thing that bugs me the most about this is that I can't think of a better name for Assign(). I don't think its intention is entirely clear. What is clear is that this approach is not as clean as the previous option, and adds noise to already noisy function calls which handle several resources at a time.

So which is the lesser of the two evils? Or is there another way which is even less evil?

Would this be better or worse? :
public:
  void Release() {if(pResource != nullptr){pResource->Release();} pResource = nullptr;}  // Now public.
  T * * operator & () {assert(pResource == nullptr); return &pResource;}  // Require Release() rather than calling it.

Edited by VReality, 12 July 2012 - 11:09 PM.


Sponsor:

#2 SiCrane   Moderators   -  Reputation: 9597

Like
4Likes
Like

Posted 12 July 2012 - 06:34 PM

One option is to use an existing, well-known COM smart pointer like CComPtr. The fact that the interface is well-known helps compensate for some of the oddities that you've noticed trying to design your own smart pointer.

#3 Telastyn   Crossbones+   -  Reputation: 3726

Like
1Likes
Like

Posted 12 July 2012 - 06:58 PM

It's been many, many years since I've worked with C++ and DirectX, but I remember that shared_ptr had the ability for you to tie in specialized destructor behavior.

#4 wqking   Members   -  Reputation: 756

Like
0Likes
Like

Posted 12 July 2012 - 11:41 PM

Which produces code like this:

D3D_Make_Resource(rpResource.Assign());
	D3D_Use_Resource(rpResource.Get());

I guess the thing that bugs me the most about this is that I can't think of a better name for Assign(). I don't think its intention is entirely clear. What is clear is that this approach is not as clean as the previous option, and adds noise to already noisy function calls which handle several resources at a time.

So which is the lesser of the two evils? Or is there another way which is even less evil?

Would this be better or worse? :
public:
	  void Release() {if(pResource != nullptr){pResource->Release();} pResource = nullptr;}  // Now public.
	  T * * operator & () {assert(pResource == nullptr); return &pResource;}  // Require Release() rather than calling it.

IMHO, both are worst.
You should never get the raw pointer's address from a smart pointer. That is not the way smart pointer is supposed to work.
That will cause the raw pointer being assigned without notifying the smart pointer. That may either cause memory leak or bad pointer.

What you'd better do is,
SomeType * pointer = NULL;
D3D_make_resource(&pointer);
SomeSmartPointer.reset(pointer);

Also, you'd better prefer an existing smart pointer rather than rolling your own one. It may be not as easy as you thought.

http://www.cpgf.org/
cpgf library -- free C++ open source library for reflection, serialization, script binding, callbacks, and meta data for OpenGL Box2D, SFML and Irrlicht.
v1.5.5 was released. Now supports tween and timeline for ease animation.


#5 VReality   Members   -  Reputation: 436

Like
0Likes
Like

Posted 13 July 2012 - 02:00 AM

You should never get the raw pointer's address from a smart pointer. That is not the way smart pointer is supposed to work.
That will cause the raw pointer being assigned without notifying the smart pointer. That may either cause memory leak or bad pointer.


Smart pointers manage ownership, and resource lifetime. While it's certainly possible to misuse raw pointers, there are safe ways to use them which are preferable to reference counted shared ownership (when it's unnecessary or inappropriate). Hence std::unique_ptr and std::unique_ptr::get().

In this case, the D3D API only accepts raw pointers. So if we can't get them, we're dead in the water. But don't worry, D3D won't store them and try to use them later.

Also, you'd better prefer an existing smart pointer rather than rolling your own one. It may be not as easy as you thought.


If a general purpose smart pointer would do, then you're right - std::unique_ptr and std::shared_ptr are top notch. std::unique pointer is my top choice for this ownership scheme.

It turns out that these resources don't quite fit the mold. You can't delete them, and they each have unique Release members. But the fact that this is such a narrowly focused problem greatly reduces the complexity of arriving at an acceptable design.

#6 wack   Members   -  Reputation: 1306

Like
0Likes
Like

Posted 15 July 2012 - 10:37 AM

As SiCrane pointed out, there are already COM smart pointers, which are created for this very purpose. Is there something that will prevent you from using them?

#7 VReality   Members   -  Reputation: 436

Like
0Likes
Like

Posted 18 July 2012 - 01:59 PM

One option is to use an existing, well-known COM smart pointer like CComPtr. The fact that the interface is well-known helps compensate for some of the oddities that you've noticed trying to design your own smart pointer.


Ah...
template <class T>
class CComPtrBase
{
  ...
  public:
	  //The assert on operator& usually indicates a bug.  If this is really
	  //what is needed, however, take the address of the p member explicitly.
	  T** operator&() throw()
	  {
		  ATLASSERT(p==NULL);
		  return &p;
	  }
	  ...
	  T* p;
...

They're making the raw pointer public and going with the assert in operator &.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS