Sign in to follow this  
pharoah0110

Trouble with Dynamic array implementation

Recommended Posts

I've been trying to set up a simple method of creating primitives using a simple algorithm, and I need to be able to set the array sizes for the Vertex and Indice lists during runtime using passed parameters.

Of course, the simple answer is to use a dynamic array, as opposed to a "compile-time-set" array,

My problem is that when I set up and use a dynamic array, I can't seem to get the vertex list into the v_buffer.  The compiler doesn't complain, but all I get is an empty world.

To test it out, I made a small, simple Vertex/Indice list for a simple triangle.  It works just fine if I use a "set-at-compile-time" array, but when I use a dynamic array, I get nothing loaded or displayed.

here's the simple code:

 

CUSTOMVERTEX *vertices = new CUSTOMVERTEX[3]; <<=====THIS IS WHAT I NEED TO USE, BUT DOESN'T WORK
//CUSTOMVERTEX vertices[3]; <<=====THIS WORKS JUST FINE
 
vertices[0].pos = D3DXVECTOR3(-1.0f, 0.0f, 0.0f);
vertices[0].COLOR = RED;
vertices[1].pos = D3DXVECTOR3(0.0f, 2.0f, 0.0f);
vertices[1].COLOR = GREEN;
vertices[2].pos = D3DXVECTOR3(1.0f, 0.0f, 0.0f);
vertices[2].COLOR = BLUE;
 
 
d3ddev->CreateVertexBuffer(3 * sizeof(CUSTOMVERTEX), 0, CUSTOMFVF, D3DPOOL_MANAGED, &v_buffer, NULL);
VOID* pVoid;
v_buffer->Lock(0, 0, (void**)&pVoid, 0);
//memcpy(pVoid, vertices, sizeof(vertices)); <<====THIS WORKS JUST FINE WITH A FIXED-AT-COMPILE ARRAY
v_buffer->Unlock();
 
short indices[] =
{
0,1,2,
};
 
d3ddev->CreateIndexBuffer(3 * sizeof(short), 0, D3DFMT_INDEX16, D3DPOOL_MANAGED, &i_buffer, NULL);
i_buffer->Lock(0, 0, (void**)&pVoid, 0);
memcpy(pVoid, indices, sizeof(indices));
i_buffer->Unlock();
 
vertices = nullptr;
delete[] vertices;
 

I Know that "new" variables pass a pointer, so I think that it's a simple error on my part in the code for loading the v_buffer, but I can't figure this out.

Any help aould be very much appreciated.

Cheers!

Share this post


Link to post
Share on other sites
memcpy(pVoid, vertices, sizeof(vertices));

sizeof(vertices) in this case would be the size of the pointer (probably 4 or 8 bytes) and not the size of the data you want to copy.

 

Whoops!!  You're right, I need the SIZE of the array, not the location (**facepalm**).  Correct code turns out to be:

memcpy(pVoid, vertices, sizeof(CUSTOMVERTEX) * 3);

Thanks for the help.  Much appreciated!

 

[edited for spelling]

Edited by pharoah0110

Share this post


Link to post
Share on other sites
I cannot recommend strongly enough that you just use std::vector instead of what you're doing, and use the .size() member for finding the size of the contents.

If you really do need a fixed-at-compile-time size and for some (weird) reason don't want to just use std::vector anyway, use std::unique_ptr<std::array<T, SIZE>>.

Share this post


Link to post
Share on other sites

I cannot recommend strongly enough that you just use std::vector instead of what you're doing, and use the .size() member for finding the size of the contents.

If you really do need a fixed-at-compile-time size and for some (weird) reason don't want to just use std::vector anyway, use std::unique_ptr<std::array<T, SIZE>>.

I tried using the Vector method, and it works, but I'm not getting the complete terrain (only a portion of it). I'm using:

 

std::vector<CUSTOMVERTEX>vertices(totalVerts);
std::vector<short>indices(totalIndices);
 
<<same filling algorithm, except using ".at()">>>
 
...and then for the buffer-fillers:
 
d3ddev->CreateVertexBuffer(totalVerts * sizeof(CUSTOMVERTEX), 0, CUSTOMFVF, D3DPOOL_MANAGED, &terrainV_buffer, NULL);
terrainV_buffer->Lock(0, 0, (void**)&pVoid, 0);
memcpy(pVoid, &vertices[0], sizeof(CUSTOMVERTEX) * vertices.size());
terrainV_buffer->Unlock();
 
d3ddev->CreateIndexBuffer(totalIndices * sizeof(short), 0, D3DFMT_INDEX16, D3DPOOL_MANAGED, &terrainI_buffer, NULL);
terrainI_buffer->Lock(0, 0, (void**)&pVoid, 0);
memcpy(pVoid, &indices[0], sizeof(short) * indices.size());
terrainI_buffer->Unlock();
 
Almost there, just working out the last few bugs.  Again, thanks for all the help thus far!

Share this post


Link to post
Share on other sites

std::vector<CUSTOMVERTEX>vertices(totalVerts);
std::vector<short>indices(totalIndices);


There's nothing immediately wrong-looking here to me. If you're saying that the version without vectors works and that doesn't, there's something else going on in code that you're not showing.

One thing possibly worth noting with vectors - there's a difference between _size_ and _capacity_. Those constructors are setting a _size_. You indicate that's what is desired, but you didn't share the code that would validate it's correct.
 

same filling algorithm, except using ".at()"


Don't use .at(). Like, ever. It's just a slower version of [] that throws exceptions on out-of-bounds access. All that using .at() accomplishes is making all of your access slower (due to the bounds checks) and shoves C++ exceptions into your code (and you might prefer compiling with exceptions disabled).

Also, don't do this:
 

memcpy(pVoid, &indices[0], sizeof(short) * indices.size());


That has two problems. First, what happens if you change the type of indices? You'll possibly forget to update that sizeof(short). Use sizeof(indices[0]) or the like instead. Second don't use the &indices[0] idiom, as that can trigger problems for zero-sized vectors (the sizeof is okay because it's an unevaluated context). Use the indices.data() member instead, which is a fast and simple equivalent to (!indices.empty() ? &indices[0] : nullptr). This applies to the vertices vector and all other vectors too, of course.

I do see you using both variables like totalVerts and the .size() access in your buffer setup, and it's not clear why. That could be part of the problem if you're mixing those elsewhere where it matters.

Share this post


Link to post
Share on other sites

 

std::vector<CUSTOMVERTEX>vertices(totalVerts);
std::vector<short>indices(totalIndices);

There's nothing immediately wrong-looking here to me. If you're saying that the version without vectors works and that doesn't, there's something else going on in code that you're not showing.

One thing possibly worth noting with vectors - there's a difference between _size_ and _capacity_. Those constructors are setting a _size_. You indicate that's what is desired, but you didn't share the code that would validate it's correct.
 

same filling algorithm, except using ".at()"


Don't use .at(). Like, ever. It's just a slower version of [] that throws exceptions on out-of-bounds access. All that using .at() accomplishes is making all of your access slower (due to the bounds checks) and shoves C++ exceptions into your code (and you might prefer compiling with exceptions disabled).

Also, don't do this:
 
memcpy(pVoid, &indices[0], sizeof(short) * indices.size());

That has two problems. First, what happens if you change the type of indices? You'll possibly forget to update that sizeof(short). Use sizeof(indices[0]) or the like instead. Second don't use the &indices[0] idiom, as that can trigger problems for zero-sized vectors (the sizeof is okay because it's an unevaluated context). Use the indices.data() member instead, which is a fast and simple equivalent to (!indices.empty() ? &indices[0] : nullptr). This applies to the vertices vector and all other vectors too, of course.

I do see you using both variables like totalVerts and the .size() access in your buffer setup, and it's not clear why. That could be part of the problem if you're mixing those elsewhere where it matters.

 

Alright, first of all, I really appreciate your taking your time to help.  Here's the overview.

My "Terrain" class has the sole responsibility of generating the triangle-based terrain in an x,y arrangement so that every "cell" (x2 triangles) can be indexed for terrain-collision, shadows, etc.  In order to generate the terrain size during runtime, I can't use a "set-at-compile-time" array; it has to be done dynamically using passed parameters.

First, the Vertex list is generated, followed by the Indice list, which sets the triangles.  This works just fine.

As is the custom, both the Vertex, then the Indice buffers are filled with those lists.

To make sure the generating algorithm works, at first I used fixed-arrays. Worked just fine. Then I went on to use something that could be dynamically set during runtime, so that the "size" of the terrain (in length, width, and cells per dimension) could be set.

After some help (see a few posts back) I managed to get the dynamic-array ("new") to work; to be honest, I wasn't comfortable with this either. Here's the full "Terrain member-function" as it stands, with an attempt at VECTOR:

 

<<CLASS PRIVATE VARIABLES>>

LPDIRECT3DVERTEXBUFFER9 terrainV_buffer = NULL;
LPDIRECT3DINDEXBUFFER9 terrainI_buffer = NULL;
D3DXVECTOR3 minBounds = D3DXVECTOR3(-16.0f, 0.0f, -16.0f);
D3DXVECTOR3 maxBounds = D3DXVECTOR3(16.0f, 0.0f, 16.0f);
int sumVerts;
int totalVerts;
int totalPolys;
int totalIndices;
VOID* pVoid;
 
<<In CLASS DEFINITION>>
void Terrain::InitTerrain()
{
int numCellsWide{ 128 };
int numCellsHigh{ 128 };
int numVertsX = numCellsWide + 1;
int numVertsZ = numCellsHigh + 1;
float stepX = (maxBounds.x - minBounds.x) / numCellsWide;
float stepZ = (maxBounds.z - minBounds.z) / numCellsHigh;
totalVerts = numVertsX * numVertsZ;
totalPolys = numCellsHigh * (numCellsWide * 2);
totalIndices = 6 * (numCellsWide * numCellsHigh);
 
std::vector<CUSTOMVERTEX>vertices(totalVerts);
std::vector<short>indices(totalIndices);
 
D3DXVECTOR3 pos = D3DXVECTOR3(minBounds.x, 0.0f, minBounds.z);
int count = 0;
for (int z = 0; z < numVertsZ; z++)
{
pos.x = minBounds.x;
for (int x = 0; x < numVertsX; x++)
{
 
vertices.at(x).pos = pos;
vertices.at(x).nor = D3DXVECTOR3(0.0f, 1.0f, 0.0f);
pos.x += stepX;
count++;
}
pos.z += stepZ;
}
 
count = 0;
int vIndex = 0;
for (int z = 0; z < numCellsHigh; z++)
{
for (int x = 0; x < numCellsWide; x++)
{
indices.at(count++) = vIndex;
indices.at(count++) = vIndex + numVertsX;
indices.at(count++) = vIndex + numVertsX + 1;
indices.at(count++) = vIndex;
indices.at(count++) = vIndex + numVertsX + 1;
indices.at(count++) = vIndex + 1;
vIndex++;
}
vIndex++;
}
 
d3ddev->CreateVertexBuffer(totalVerts * sizeof(CUSTOMVERTEX), 0, CUSTOMFVF, D3DPOOL_MANAGED, &terrainV_buffer, NULL);
terrainV_buffer->Lock(0, 0, (void**)&pVoid, 0);
memcpy(pVoid, &vertices[0], sizeof(CUSTOMVERTEX) * vertices.size());
terrainV_buffer->Unlock();
 
d3ddev->CreateIndexBuffer(totalIndices * sizeof(short), 0, D3DFMT_INDEX16, D3DPOOL_MANAGED, &terrainI_buffer, NULL);
terrainI_buffer->Lock(0, 0, (void**)&pVoid, 0);
memcpy(pVoid, &indices[0], sizeof(short) * indices.size());
terrainI_buffer->Unlock();
}
 
I'm at a loss as to how to implement the vector properly here, specifically as to the buffer-loading calls. As usual, the answer is probably right in front of me, but I can't quite figure this one out.
As for the different types of indices, I'm not exactly sure what you mean.  (short) works, and I can't see a reason to change it.
Any further help is more than appreciated.  Cheers!
 
[ETA] edited for spelling.
Edited by pharoah0110

Share this post


Link to post
Share on other sites

Ok, finally got the Vector thing figured out.

You were right about not using "at()", simply replacing every occurrence of it with "vertices[n]" did the trick, and it's working fine now.

Just one final question, though.

in the v_buffer fill, using

 

memcpy(pVoid, &vertices[0], sizeof(CUSTOMVERTEX) * vertices.size());

 

works quite well.  Despite that, is there a reason I should use another format?  Just asking because you said not to use the "&vertices[0]" idiom.  Not quite sure what you meant by that.  Removing the ampersand, or the "[0]" reference causes a crash.

 

Again, I really appreciate all the help and the time you've given me.  Thanks, and Cheers!!

Share this post


Link to post
Share on other sites

Also, don't do this:
 

memcpy(pVoid, &indices[0], sizeof(short) * indices.size());

That has two problems. First, what happens if you change the type of indices? You'll possibly forget to update that sizeof(short). Use sizeof(indices[0]) or the like instead. Second don't use the &indices[0] idiom, as that can trigger problems for zero-sized vectors (the sizeof is okay because it's an unevaluated context). Use the indices.data() member instead, which is a fast and simple equivalent to (!indices.empty() ? &indices[0] : nullptr). This applies to the vertices vector and all other vectors too, of course.

 

Regarding this, I think EASTL had a member function for getting the byte length of a vector. Are you aware of any talk about upgrades to std::vector along these lines? I was disappointed recently to find that there doesn't appear to be a standard way to refer to a vector's type that works with sizeof(). I was thinking that sizeof(vec.value_type) was possible until I tried it.

Share this post


Link to post
Share on other sites

Ok, finally got the Vector thing figured out. You were right about not using "at()", simply replacing every occurrence of it with "vertices[n]" did the trick, and it's working fine now.


That mildly surprises me, but I'm not intimately familiar with the semantics of .at() since I have never once used it in 20 years. :)
 

works quite well.  Despite that, is there a reason I should use another format?  Just asking because you said not to use the "&vertices[0]" idiom.  Not quite sure what you meant by that.  Removing the ampersand, or the "[0]" reference causes a crash.


Like I said, use .data() instead. Replace &indices[0] with indices.data()
 

Regarding this, I think EASTL had a member function for getting the byte length of a vector. Are you aware of any talk about upgrades to std::vector along these lines? I was disappointed recently to find that there doesn't appear to be a standard way to refer to a vector's type that works with sizeof(). I was thinking that sizeof(vec.value_type) was possible until I tried it.


I have an extremely vague recollection of it being discussed on the main isocpp reflector a while ago, but I don't recall an actual paper being submitted.

Quite possibly worth bringing up on the c++ sg14 reflector. *nudge* :)

Share this post


Link to post
Share on other sites

I'm about to be conscripted for some housework, but I'll see what I can do later. I was looking at it yesterday and apparently there's a workaround at present in the form of sizeof(decltype(vec)::value_type), so in theory it shouldn't be too difficult to work out something more convenient.

 

Edit: Posted.

Edited by Khatharr

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this