push_back, structs, and void*

Started by
8 comments, last by Agony 18 years, 11 months ago
I've created a structure containing a 4-member vertex array and an image. My programming method forces me to send void pointers back for logging (so that I can change my graphics class without having to edit the whole program). Here's how it looks:

struct SPRITE
{
	D3DVERTEX mVertex[4];
	LPDIRECT3DTEXTURE9 spriteTexture;
};
vector<SPRITE> sprite;

...
//my function ends like this, where genericSprite (of type SPRITE)'s
//members are set up successfully earlier in the function
sprite.push_back(genericSprite);

//spriteCounter is a counter containing the end of the array
return &sprite[spriteCounter++];

When I refer back to that sprite with (SPRITE*)imagePointer after only one SPRITE created, everything works fine. The problem occurs after a second sprite is created. With the first member of the array, when I use a void pointer to recall the position of the structure, the spriteTexture member gains an (probably) invalid address of 0xfeeefeee. Why am I losing the value of my spriteTexture after pushing a new SPRITE into the vector?

bool dx9::drawImage(void *imageSource, const RECT *rSource, const POINT *pDest)
{
	SPRITE *image = (SPRITE*)imageSource;

	//...

	pd3dDevice->SetTexture(0, (image->spriteTexture));

	//...
}

image->spriteTexture, with the previous texture, loses its valid address for some reason. Only the most recently created member of the vector retains its texture address.
Advertisement
Its in this line:
return &sprite[spriteCounter++];


Think: The ++ is a postfix increment, right? It increments the variable AFTER the statement...but return is called first, and the function returns early, so the ++ never gets called. basically, it says this:
return &sprite[spriteCounter];spriteCounter = spriteCounter + 1;


An option is that you could set your initial index as -1, and do a ++spriteCounter instead of spriteCounter++.
Quote:Original post by visage
Its in this line:
return &sprite[spriteCounter++];


Think: The ++ is a postfix increment, right? It increments the variable AFTER the statement...but return is called first, and the function returns early, so the ++ never gets called. basically, it says this:
return &sprite[spriteCounter];spriteCounter = spriteCounter + 1;


An option is that you could set your initial index as -1, and do a ++spriteCounter instead of spriteCounter++.


...are you smoking something? Or do you not really mean "++ never gets called". Good thinking, but you're wrong. Try this:

#include <iostream>using namespace std;int foo = 1;int bar( void ){	return foo++;}int main ( int argc , char ** argv ){	cout << bar() << endl;	cout << bar() << endl;}


According to your theory as stated, the program should spit out 1 for both results. It does not. Incrementation does not occur after the statement, it occurs directly at that point of code.

Take a look at the iterator classes for how this works. You'll notice they're implemented similar to so:

iterator temp = *this;++(this->position);return temp;


This is the same kind of behavior as happens with integers - as such:

int temp = spriteCounter;++spriteCounter;return &sprite[ temp ];


Quote:Original post by startreky498
I've created a structure containing a 4-member vertex array and an image. My programming method forces me to send void pointers back for logging (so that I can change my graphics class without having to edit the whole program).

Here's how it looks:

*** Source Snippet Removed ***


Thoughts so far: you shouldn't be mantaining a seperate count. If you need to know how many elements are in the vector, call sprite.size() (that should be named sprites, plural, btw). If you want the last element, either: sprites[ sprites.size() - 1 ], or sprites.back(); will do nicely. I.e. that return statement should read: "return &(sprites.back());".

Quote:When I refer back to that sprite with (SPRITE*)imagePointer after only one SPRITE created, everything works fine. The problem occurs after a second sprite is created. With the first member of the array, when I use a void pointer to recall the position of the structure, the spriteTexture member gains an (probably) invalid address of 0xfeeefeee. Why am I losing the value of my spriteTexture after pushing a new SPRITE into the vector?

*** Source Snippet Removed ***

image->spriteTexture, with the previous texture, loses its valid address for some reason. Only the most recently created member of the vector retains its texture address.


Are you sure it was valid in the first place? Have you tried stepping through your code? I'm seeing nothing obviously buggy with your code, only a couple cases of bad naming and one of bad practice. Care to post more of the source?

[Edited by - MaulingMonkey on May 14, 2005 9:20:26 PM]
@MaulingMonkey, yes visage is wrong, but your tone is agressive.
@startreky498, we need to see more code, maybe a rendering loop that calls the draw function?

also,
I'm not so sure about using C++ with void pointers, can't you use inheritence to avoid the c-style cast?

struct DrawableItem {   vec2 position;};struct Sprite : DrawableItem {   D3DVERTEX mVertex[4];   LPDIRECT3DTEXTURE9 spriteTexture;};bool dx9::drawImage(DrawableItem *imageSource, const RECT *rSource, const POINT *pDest){	SPRITE *image = static_cast<SPRITE*>( imageSource );	//...         pd3dDevice->Translate( image->position ); // im not sure about this, im an OpenGL programmer!	pd3dDevice->SetTexture(0, (image->spriteTexture));	//...}


also also, to build on what maulingmonkey said, if you're worried about memory alocations you can use std::vector::reserve and still get away with push_back/size()

eg.
std::vector< SPRITE > sprites;sprites.reserve( NUM_SPRITES );...sprites.push_back( SPRITE(...) );return &(sprites.back());
"I am a donut! Ask not how many tris/batch, but rather how many batches/frame!" -- Matthias Wloka & Richard Huddy, (GDC, DirectX 9 Performance)

http://www.silvermace.com/ -- My personal website
Herm...I suppose thats what I get for being entirely self taught.

Can someone explain to me why exactly I am wrong? You say the increment occurs right after that point in code, but if that point is a return, then how is the increment called?

Thanks.
Quote:Original post by visage
Herm...I suppose thats what I get for being entirely self taught.

Can someone explain to me why exactly I am wrong? You say the increment occurs right after that point in code, but if that point is a return, then how is the increment called?

Thanks.


Actually, "after" is the wrong word to best describe this. Take this:

int x = y++ + z;

The compiler translates this into assembly code that looks more or less like this C++ code:

int temp1 = y;++y;int x = temp1 + z;


The compiler is smart enough that it can screw around with the actual ordering of stuff, but that's an implementation detail - the code will allways preform the same as this.

When I used the term "after", i meant "after the 'int temp1 = y;' step".

There's allways that implicit temporary in there. Even iterators are forced to return a temporary value for the postfix operators. Whereas:

int x = ++y + z;

Can be internally done like so:

y += 1;
int x = y + z;

The postfix version:

int x = y++ + z;

Must use a the temporary:

int temp1 = y;
y += 1;
int x = temp1 + z;

This means that with iterators, the postfix version can sometimes be marginally faster. This is why I've gotten myself into the habit of typing:

for ( ... ; ... ; ++i )

instead of:

for ( ... ; ... ; i++ )
Woah. Learn something new everyday.

Why does the compiler write the assembly in this way? It seems like...a unecessary extra step to me.

Thanks for correcting me.
Quote:Original post by visage
Herm...I suppose thats what I get for being entirely self taught.

Can someone explain to me why exactly I am wrong? You say the increment occurs right after that point in code, but if that point is a return, then how is the increment called?

Thanks.


when translated to actual instructions, return sprite[a++] actually produces something similar to this:

1. array access, store pointer
2. incrment a
3. return stored pointer

where this code: return sprite[++a] would be:
1. incrment a
2. array access, store pointer
3. return stored pointer

i hope that kinda clears it up, if not take a look at your compiler output.
"I am a donut! Ask not how many tris/batch, but rather how many batches/frame!" -- Matthias Wloka & Richard Huddy, (GDC, DirectX 9 Performance)

http://www.silvermace.com/ -- My personal website
Quote:Original post by visage
Woah. Learn something new everyday.

Why does the compiler write the assembly in this way? It seems like...a unecessary extra step to me.


Well, for one, x86 assembly can't directly do "int x = y + z". Well, it kind of can, if x is going to be where y or z is. It's more like:

register = 0; //sometimes done as: register ^= register;, same effect.
register += y;
register += z;

Very basic steps, so for the most basic of postincrements ( e.g. postincrementing an integer ) it's not actually going to add complexity to the assembly code.

There's also the fact that they want "return bar++;" to work "right" :-).
I can't tell if you've solved your problem yet or not, but if not, I'm guessing your primary problem is with the way std::vector allocates memory. When you push_back() an element, there is a chance (a higher chance the smaller your vector is) that it will allocate a new (larger) chunk of contiguous memory, copy all of it's old contents to the beginning of the new chunk, and then deallocate the old (smaller) chunk. When this happens, all pointers and all iterators to elements in your vector become invalid, since the elements are now sitting in a new location in memory.

To fix this, you can A) reserve() enough memory in the very beginning to ensure that no reallocation ever has to happen (this is a poor solution, though), B) store pointers in your vector rather than elements themselves (this might require a bit more effort to prevent memory leaks), or C) use a different container that promises to never invalidate it's iterators (I don't know about pointers) when you add or remove elements, such as std::list or std::map/multimap or std::set/multiset.
"We should have a great fewer disputes in the world if words were taken for what they are, the signs of our ideas only, and not for things themselves." - John Locke

This topic is closed to new replies.

Advertisement