• Advertisement
Sign in to follow this  

C++ deleting a sprite with delete operator

This topic is 3574 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi everyone! I've been reading an old post from this forum where one person sayed that when he deleted an object with the delete operator, it appeared not to be any memory amount decrease in the Windows Task manager. Also, this person was told that the Windows Task manager is not very reliable in this way. So my question is: when I delete (with delete operator) a sprite in my game I don't see in Task manager that memory amount is decreasing, but the game continues creating sprites, the memory amount increases. Do I have to consider this as a memory leak or should I just ignore Windows Task manager? Also I have a related question. I delete my sprite with this line: delete mySprite; // being mySprite a pointer to a MySprite class object MySprite is a class that I developed myself, but I left the destructor in it empty (as it came by default). Is this okay or should I manually deallocate class members in the destructor's body? Hope I made it clear. Thanks in advance for your help! Bye, bye! ;)

Share this post


Link to post
Share on other sites
Advertisement
You should manually deallocate class members.
Don't pay too much attention to the task manager.
If you deleted the sprite and everything went ok you can assume
the memory was freed.

HTH.

Share this post


Link to post
Share on other sites
Quote:
Original post by PezMutanT
Hi everyone!

I've been reading an old post from this forum where one person sayed that when he deleted an object with the delete operator, it appeared not to be any memory amount decrease in the Windows Task manager.

Also, this person was told that the Windows Task manager is not very reliable in this way.

So my question is: when I delete (with delete operator) a sprite in my game I don't see in Task manager that memory amount is decreasing, but the game continues creating sprites, the memory amount increases. Do I have to consider this as a memory leak or should I just ignore Windows Task manager?
Ignore task manager. Task manager shows your application's working set size, which is the amount of memory the OS has set aside for your application. The way the CRT memory allocation functions work is they request blocks of memory from the OS (Which are something like 64KB I think) when they need it, and free them when it's done. However, for performance reasons, the CRT is likely to keep a certain number of these blocks allocated and not free them as soon as it can. Further, the OS doesn't have to actually free the blocks when the CRT asks it to; it might keep them allocated for performance reasons as well.

Quote:
Original post by PezMutanT
Also I have a related question. I delete my sprite with this line:

delete mySprite; // being mySprite a pointer to a MySprite class object

MySprite is a class that I developed myself, but I left the destructor in it empty (as it came by default). Is this okay or should I manually deallocate class members in the destructor's body?
That's perfectly fine. After your destructor is called (Even if it's empty), the compiler will call the destructor of all class member variables manually. You should only ever explicitly call the destructor of a class (Member variable or otherwise) if you're writing a memory manager and are using placement new.
However, if any of the member variables are created with new, you need to delete them from the destructor.

Share this post


Link to post
Share on other sites
Thanks a lot to both of you for replying!

Quote:
Original post by Evil Steve
That's perfectly fine. After your destructor is called (Even if it's empty), the compiler will call the destructor of all class member variables manually. You should only ever explicitly call the destructor of a class (Member variable or otherwise) if you're writing a memory manager and are using placement new.
However, if any of the member variables are created with new, you need to delete them from the destructor.


That's what I understood, you don't call explicitly the destructor except when you are using placement new. But if I want to delete a sprite from memory, am I doing well "deleting" it, right?

In conclusion, my destructor should only have "delete's" for those members that I created earlier with a new operator? The rest of members should deallocate automatically even if the destructor is empty, right?

The sprite I am trying to delete is of a class named Fire. This class is derived from a class named Sprite. Fire class has no own member variables, but Sprite class is like this:


class Sprite
{
protected:
LPDIRECT3DDEVICE9 m_Device;
LPD3DXSPRITE m_pSprite;
LPDIRECT3DTEXTURE9 m_pTextura;
D3DCOLOR m_AlphaColor;
RECT m_Rect;
D3DXVECTOR3 m_v3Center;
D3DXVECTOR3 m_v3Position;
int m_Anchura;
int m_Altura;
int m_VelocidadX;
int m_VelocidadY;
BOOL m_bDestruir;

...



The Fire class constructor is this:


Fire::Fire(LPDIRECT3DDEVICE9 device, LPCTSTR archivo, int rLeft, int rTop, int rRight, int rBottom, int x, int y)
{
m_bDestruir = FALSE;
m_Device = device;
m_Rect.left = rLeft;
m_Rect.top = rTop;
m_Rect.right = rRight;
m_Rect.bottom = rBottom;
m_Anchura = m_Rect.right - m_Rect.left;
m_Altura = m_Rect.bottom - m_Rect.top;
m_v3Center = D3DXVECTOR3(0, 0, 0);
m_v3Position = D3DXVECTOR3(FLOAT(x), FLOAT(y), 0);
m_AlphaColor = D3DCOLOR_ARGB(255, 255, 255, 255);
m_VelocidadX = m_VelocidadY = 0;
D3DXCreateSprite(device, &m_pSprite);
D3DXCreateTextureFromFileEx(m_Device, archivo,
D3DX_DEFAULT, D3DX_DEFAULT, D3DX_DEFAULT, 0, D3DFMT_UNKNOWN, D3DPOOL_MANAGED,
D3DX_DEFAULT, D3DX_DEFAULT, m_AlphaColor, NULL, NULL, &m_pTextura);
}


As I am not initializing any member variable with new operator, I would say I can keep my destructor empty. Is this correct?

Thanks a lot, people!

Share this post


Link to post
Share on other sites
You are not but D3DXCreateSprite and D3DXCreateTextureFromFileEx probably
do, you need to free them using the appropriate d3dx functions.

Share this post


Link to post
Share on other sites
Quote:
Original post by PezMutanT
That's what I understood, you don't call explicitly the destructor except when you are using placement new. But if I want to delete a sprite from memory, am I doing well "deleting" it, right?

In conclusion, my destructor should only have "delete's" for those members that I created earlier with a new operator? The rest of members should deallocate automatically even if the destructor is empty, right?
Correct. The destructor is automatically called for all your member variables. If you have any pointers as member variables, the destructor for a pointer does nothing (Since you might have a pointer to an object that's still in use; e.g. a parent sprite or something). If that pointer is allocated with new, you need to delete it. If it's a C FILE handle, you need to fclose() it. Etc, etc.

Quote:
Original post by PezMutanT
The sprite I am trying to delete is of a class named Fire. This class is derived from a class named Sprite. Fire class has no own member variables, but Sprite class is like this:

*** Source Snippet Removed ***

The Fire class constructor is this:

*** Source Snippet Removed ***
As I am not initializing any member variable with new operator, I would say I can keep my destructor empty. Is this correct?
You still need to free your sprite and texture, because they're pointers (Unless you're still using them after the object has been destroyed - e.g. if you pass the texture to some sort of resource manager for caching).
So your Sprite destructor (Since that's what has the members, not the Fire class, so it makes most sense to do it there) should probably be:

Sprite::~Sprite()
{
if(m_pSprite)
m_pSprite->Release();
if(m_pTextura)
m_pTextura->Release();
}


The base class destructor (Sprite in this case) is automatically called when the derived class's (Fire's) destructor is called.


Also, a little off topic; it might be an idea to call AddRef() on the device pointer when you construct the sprite, and Release() it in the destructor. That keeps everything nice, since if you end up releasing your device and then use your sprite, it won't crash because your sprite has incremented the reference count.

Also off topic, it's generally better to use one ID3DXSprite class in your entire app. The ID3DXSprite class has a vertex and index buffer internally, so creating multiple ID3DXSprite's means multiple VBs and IBs, which is bad for memory usage and also bad for speed (Since you need to swap between them when rendering).

Share this post


Link to post
Share on other sites
Ok, thank both of you very much! You made things clearer to me.

But those off-topic comments from Evil Steve about ID3DXSprite made me think... So it's supposed to use only one ID3DXSprite pointer in the whole application? If so, can I manage several sprites with it? Is it done with the AddRef() and Release() functions that you were saying?

This is my first contact with DirectX, I was using it just to know how it works, but I see I didn't get it very well... :)

Thanks a lot!!

Bye, bye! ;)

Share this post


Link to post
Share on other sites
Quote:
Original post by PezMutanT
But those off-topic comments from Evil Steve about ID3DXSprite made me think... So it's supposed to use only one ID3DXSprite pointer in the whole application? If so, can I manage several sprites with it? Is it done with the AddRef() and Release() functions that you were saying?

This is my first contact with DirectX, I was using it just to know how it works, but I see I didn't get it very well... :))
There's nothing too bad about using one ID3DXSprite per sprite, it's just better if you didn't.

I'd handle this by creating the ID3DXSprite after you create your device, and passing it to the constructor of Fire in the same way that you pass your device pointer in. And yeah, I'd AddRef() it in the constructor and Release() it in the destructor just to keep things clean.

Also, if you're not already, you should be using the Debug Runtimes, since they'll help track down bugs and show up memory leaks from not Release()ing D3D objects.

That would mean your code would look like:

class Sprite
{
Sprite(LPDIRECT3DDEVICE9 pDevice, LPD3DXSPRITE pSprite) :
m_Device(pDevice),
m_pSprite(pSprite),
m_pTexture(NULL),
m_AlphaColor(D3DCOLOR_ARGB(255, 255, 255, 255)),
m_v3Center(0, 0, 0),
m_v3Position(0, 0, 0),
m_Anchura(0),
m_Altura(0),
m_VelocidadX(0),
m_VelocidadY(0),
m_bDestruir(FALSE)
{
m_Rect.top = 0;
m_Rect.bottom = 0;
m_Rect.left = 0;
m_Rect.right = 0;

m_Device->AddRef();
m_pSprite->AddRef();
}

virtual ~Sprite()
{
if(m_pSprite)
m_pSprite->Release();
if(m_pTextura)
m_pTextura->Release();
}

protected:
LPDIRECT3DDEVICE9 m_Device;
LPD3DXSPRITE m_pSprite;
LPDIRECT3DTEXTURE9 m_pTextura;
D3DCOLOR m_AlphaColor;
RECT m_Rect;
D3DXVECTOR3 m_v3Center;
D3DXVECTOR3 m_v3Position;
int m_Anchura;
int m_Altura;
int m_VelocidadX;
int m_VelocidadY;
BOOL m_bDestruir;
};

Fire::Fire(LPDIRECT3DDEVICE9 device, LPD3DXSPRITE sprite, LPCTSTR archivo, int rLeft, int rTop, int rRight, int rBottom, int x, int y) : Sprite(device, sprite)
{
m_Rect.left = rLeft;
m_Rect.top = rTop;
m_Rect.right = rRight;
m_Rect.bottom = rBottom;
m_Anchura = m_Rect.right - m_Rect.left;
m_Altura = m_Rect.bottom - m_Rect.top;
m_v3Position = D3DXVECTOR3(FLOAT(x), FLOAT(y), 0);
D3DXCreateTextureFromFileEx(m_Device, archivo,
D3DX_DEFAULT, D3DX_DEFAULT, D3DX_DEFAULT, 0, D3DFMT_UNKNOWN, D3DPOOL_MANAGED,
D3DX_DEFAULT, D3DX_DEFAULT, m_AlphaColor, NULL, NULL, &m_pTextura);
}


I changed the code a little so you use an initialiser list (Which is more of a C++ way than initialising the members in the constructor body), and also made the Sprite destructor virtual (So you can delete a pointer to a Sprite without having to cast it to a Fire or other type).


EDIT: It would probably be cleaner to pass the RECT and width, height and position parameters to the Sprite() constructor, since they'll need set for every sprite anyway. They'd just be passed in like how the device and sprite pointer are passed in. But that's really just a style thing [smile]

Share this post


Link to post
Share on other sites
Wow, I really appreciate your work with my code, Evil Steve :)

I'm sorry but I still don't understand two things: (by the way, I am sorry because I know this discussion shouldn't be in this forum)

- Reading the ID3DXSprite reference, it mention something about a list of batched sprites, like here:

"ID3DXSprite::Draw Adds a sprite to the list of batched sprites."

I don't understand how I should manage that list and if I could access every element of it (because every element would be a different sprite). Is it related with the AddRef() and Release() functions?


- Why a virtual destructor? If I only have one destructor in the parent class, shouldn't it be enough?


I'm sorry, but I got a little confuse after your last reply.

Thank you very much!

Share this post


Link to post
Share on other sites
Quote:
Original post by PezMutanT
- Why a virtual destructor? If I only have one destructor in the parent class, shouldn't it be enough?


Whenever you call a non-virtual member function through a pointer or reference to the base class, the base class version of the function is called, not the derived one. The destructor is a member function; therefore, whenever you call 'delete' on a pointer to the base class, and the destructor of the base class is not virtual, the base class version is called. That's bad, because if the pointer actually points to a derived instance (and the ability to do that is what motivated you in the first place), then the program will try to clean up a derived instance as if it were a base instance. Note: this does not simply clean up the base part of the class and leave the derived part alone. It is explicitly undefined behaviour.

Share this post


Link to post
Share on other sites
Think about storing things using RAII -- resource allocation is initialization -- so you don't forget to deallocate them.

Either use a boost auto pointer, or roll your own quickly:

template<typename T>
class EasyAutoPtr {
T* t;
public:
static EasyAutoPtr FromNewCreation(T* t_) {return EasyAutoPtr(t_, true);}
explicit EasyAutoPtr(T* t_, bool take_reference=false):t(t_){if (!take_reference) t->AddRef();}
T* StealReference() {
T* tmp = t;
t = 0;
return tmp;
}
T* BorrowReference() {return t;}
T* BorrowReference() const {return t;}
T* DuplicateReference() {if (t) t->AddRef(); return t;}
T* DuplicateReference() const {if (t) t->AddRef(); return t;}
void swap(EasyAutoPtr<T>& other) {swap(t, other.t);}
virtual ~EasyAutoPtr() {if(t)t->RemoveRef();}

template<typename Other>
EasyAutoPtr(Other const& o):t(o.DuplicateReference()) {}

template<typename Other>
EasyAutoPtr<T> const& operator=(Other const& o) {
EasyAutoPtr<T> tmp = o;
this->swap(tmp);
return *this;
}

operator T*() { return BorrowReference(); }
operator T const*() const { return BorrowReference(); };
T* operator->() { return BorrowReference(); }
T const* operator->() const { return BorrowReference(); }
T& operator*() { return BorrowReference(); }
T const& operator*() const { return BorrowReference(); }
};



(I would advise using a boost auto pointer -- I've written multiple auto pointer classes, and odds are the above one I whipped together has a bug!)

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Whenever you call a non-virtual member function through a pointer or reference to the base class, the base class version of the function is called, not the derived one. The destructor is a member function; therefore, whenever you call 'delete' on a pointer to the base class, and the destructor of the base class is not virtual, the base class version is called. That's bad, because if the pointer actually points to a derived instance (and the ability to do that is what motivated you in the first place), then the program will try to clean up a derived instance as if it were a base instance. Note: this does not simply clean up the base part of the class and leave the derived part alone. It is explicitly undefined behaviour.


Perfecly explained, I understand now. Thanks a lot to both of you for your answers!

Share this post


Link to post
Share on other sites
Quote:
Original post by PezMutanT
- Reading the ID3DXSprite reference, it mention something about a list of batched sprites, like here:

"ID3DXSprite::Draw Adds a sprite to the list of batched sprites."

I don't understand how I should manage that list and if I could access every element of it (because every element would be a different sprite). Is it related with the AddRef() and Release() functions?
You don't. I mentioned that ID3DXSprite contains a vertex and index buffer internally; the Draw() function looks like this (Psuedo-code):

ID3DXSprite::Draw(x, y, w, h) // Whatever the parameters are
{
add_4_vertices_to_vertex_buffer(x, y, w, h);
add_4_indices_to_index_buffer(x, y, w, h);
m_numSprites++;
}

ID3DXSprite::Flush()
{
set_vb_and_ib();
m_pDevice->DrawIndexedPrimitive(blah, foo, m_numSprites*2);
m_numSprites = 0;
}

ID3DXSprite::End()
{
Flush();
}




What I mean is that Draw() just adds a sprite to the batch, and Flush() or End() actually draws the sprite. It's actually a bit more complicated than that, because ID3DXSprite deals with multiple textures, multiple transforms, and so on. But that's the general idea.
You don't need to access the list at all, it's all handled for you.

If you're interested in the internal workings, I made a Journal Entry a while ago about what ID3DXSprite does internally. That information was from a fairly old SDK, so the specifics have probably changed, but it gives you a rough idea of what it's really doing.

Share this post


Link to post
Share on other sites
Thanks again, Evil Steve.

So I suppose I only have to keep a list (for example a vector) of references (pointers to my own Sprite class and/or its derived ones) to the different sprites in the game just to interact with them (for example killing one of them or moving one of them along the window), right?

I took a look at your journal, that's great stuff man! How can I access to it directly from the home page of gamedev.net?

Well, I don't know what I would do without you, guys...

Thanks a lot! ;)

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Quote:
Original post by PezMutanT
- Why a virtual destructor? If I only have one destructor in the parent class, shouldn't it be enough?


Whenever you call a non-virtual member function through a pointer or reference to the base class, the base class version of the function is called, not the derived one. The destructor is a member function; therefore, whenever you call 'delete' on a pointer to the base class, and the destructor of the base class is not virtual, the base class version is called.


To be nit-picky, and as an interesting bit of trivia, a const base-class reference to a derived class temporary will correctly call the derived class destructor when it falls out of scope without a virtual dispatch. See the Guru question here.

Share this post


Link to post
Share on other sites
Quote:
Original post by PezMutanT
So I suppose I only have to keep a list (for example a vector) of references (pointers to my own Sprite class and/or its derived ones) to the different sprites in the game just to interact with them (for example killing one of them or moving one of them along the window), right?
Yup, exactly. What a lot of games do is have a scene graph. The most basic kind would just be a vector of pointers to a "game object". A Game Object would be a virtual base class, which would be inherited from by every game object type, and contains functions like Render() and Update() (Since update ticks might be different from render ticks; e.g. rendering at 60Hz and doing logic as fast as possible). You could also have OnLostDevice() and OnResetDevice() functions in there and use them to reset your ID3DXSprite.
Every game tick, the game just needs to call the Update() function for each object in this vector, and at render time call Render().

Quote:
Original post by PezMutanT
I took a look at your journal, that's great stuff man! How can I access to it directly from the home page of gamedev.net?)
Thanks [smile] All the journals can be accessed from the "Members" menu at the top of each page -> Developer Journals.

Share this post


Link to post
Share on other sites
Ok, I think I get the idea. Every tick I have to call the Update() function in every element of the vector (every sprite), and every render tick I have to call the Render() function in every element of the vector. And this Render() function would have one call to ID3DXSprite::Draw() in order to add that sprite to the batched list of sprites, of course between Begin() and End() functions. Is this right?

That "update ticks different from render ticks" thing... I hope being able to manage it...

Thanks again Evil Steve! ;)

Share this post


Link to post
Share on other sites
Quote:
Original post by PezMutanT
Ok, I think I get the idea. Every tick I have to call the Update() function in every element of the vector (every sprite), and every render tick I have to call the Render() function in every element of the vector. And this Render() function would have one call to ID3DXSprite::Draw() in order to add that sprite to the batched list of sprites, of course between Begin() and End() functions. Is this right?

That "update ticks different from render ticks" thing... I hope being able to manage it...

Thanks again Evil Steve! ;)
Yup, pretty much. You don't need to have update seperate from render, but it's a good practice to get into, since it's slightly more efficient to fill vertex buffers, etc and then wait as long as possible before rendering from them (Although it might not make any difference, it depends on the drivers), and various other reasons (If logic takes 2ms and rendering takes 10ms, you can process 3 frames of logic + 1 render frame and still vsync at 60Hz for instance).

Your code might look something like:

class GameObject
{
public:
GameObject() {}
virtual ~GameObject() {}

virtual void Update() = 0;
virtual void Render() = 0;

// Any other info you may want such as bounding spheres for culling,
// object names for debugging, etc
};

class Sprite : public GameObject
{
public:
// Various Sprite() member functions here

virtual void Update() { /* Do update code */ }
virtual void Render() { /* Do render code */ }
};

// Elsewhere:
std::vector<GameObject*> m_sceneGraph;

// Add some objects:
m_sceneGraph.push_back(new Fire(...));
m_sceneGraph.push_back(new Fire(...));
m_sceneGraph.push_back(new Fire(...));

// And then your main loop would be something like:
for(size_t i=0; i<m_sceneGraph.size(); ++i)
{
m_sceneGraph[i]->Update();
m_sceneGraph[i]->Render();
}


Two changes you could make to that would be passing in a frame time to the Update() function, so each object can update itself with respect to time (Which makes the game framerate-independant), and you could do batching; make each GameObject have an ID (an enum type probably), and sort the m_sceneGraph vector by this ID. Then when you render the first of each object, call a BeginBatchRender() virtual function, and when you render the last, call a EndBatchRender() virtual function.
Then your Sprite class could call ID3DXSprite::Begin() from BeginBatchRender() and ID3DXSprite::End() from EndBatchRender(), which would give you better performance than doing it once per sprite.

However, I'd recommend getting the basics working first, then adding on stuff like this.

Share this post


Link to post
Share on other sites
Nice idea that last one about "ID'ing" every sprite in order to only make one call to ID3DXSprite::Begin() and ID3DXSprite::End() :)

But like you said, I will try doing the basics for now and adding new features one by one. I was trying to get into collision detection and that was the origin of my post, because I thought it could be handled having several derived classes for the different kinds of sprites (characters, fire, walls, ...) in order to being able to have different Update() functions for each one and also for being able to distinguish collisions between different sprites...

All of this is what I have in mind, but I still don't know how I'm going to do it... I feel kind of guilty asking again for advice, but... maybe someone could lead me to get me started...

Anyway, thanks Evil Steve!!

Share this post


Link to post
Share on other sites
Well I've read about "double dispatch" and the "visitor pattern" and it looks like the best way I've seen so far to implement collision detection. It looks a little complicated but at least I know where to start...

Share this post


Link to post
Share on other sites
Quote:
Original post by Evil Steve
Yup, pretty much. You don't need to have update seperate from render, but it's a good practice to get into, since it's slightly more efficient to fill vertex buffers, etc and then wait as long as possible before rendering from them (Although it might not make any difference, it depends on the drivers), and various other reasons (If logic takes 2ms and rendering takes 10ms, you can process 3 frames of logic + 1 render frame and still vsync at 60Hz for instance).


Sorry to bring this post up, but I was thinking what Evil Steve said about separating updating from rendering...

What I do now in the Render() function is go through the vector where I have all sprites in the game with a for statement. Inside the for, I update each sprite's position, check for collisions with other sprites and draw it.

I was thinking that if I had separate updating and rendering, then I would have to go over the vector once every cycle for updating the sprites, plus I would have to check the whole vector again in the rendering cycle. Is that what you meant with having separate updating and rendering? Isn't it more inefficient than before where I did everything in the same for statement?

I'm confused...

Thanks! ;)

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement