questions about resize() and memcpy()

[peej]

I'm using some old code that won't compile any more. I have 2 questions about it, first the code snippet:

#define TM_VERTEX_STRIDE 4   
vector<float> tm_vertices;
 
appendVertexData(float * vertex/* = NULL*/, float * normal/* = NULL*/, 
         float * color/* = NULL*/, float * texture_coord/* = NULL*/)
{
  unsigned int vi = 0;
  unsigned int ret = 0;
  if(vertex != NULL){
    vi = tm_vertices.size();
    tm_vertices.resize(vi+TM_VERTEX_STRIDE);
     memcpy(tm_vertices.begin() + vi, vertex, 
    sizeof(float)*TM_VERTEX_STRIDE);
   ret = vi / TM_VERTEX_STRIDE;
  }
.....
 

The error : error: no matching function for call to `memcpy(__gnu_cxx::__normal_iterator<float*, std::vector<float, std::allocator<float> > >)'
/usr/include/string.h:40: note: candidates are: void* memcpy(void*, const void*, size_t)

My first question is how can i fix it?
i have tried changing the memcpy() to

   std::memcpy((float*)(*(tm_vertices.begin() + vi)), vertex, 
                       sizeof(float)*TM_VERTEX_STRIDE);
 

The error being: cannot convert '....' from type `float' to type `float*'

My second question is, what costs would be associated with changing the resize() and memcpy() lines to use a push_back() instead?

Any help is greatly appreciated 8).

cheers,

pj

[Banfa]

#define TM_VERTEX_STRIDE 4   
vector<float> tm_vertices;
 
appendVertexData(float * vertex/* = NULL*/, float * normal/* = NULL*/, 
         float * color/* = NULL*/, float * texture_coord/* = NULL*/)
{
  unsigned int vi = 0;
  unsigned int ret = 0;
  if(vertex != NULL){
    vi = tm_vertices.size();
    tm_vertices.resize(vi+TM_VERTEX_STRIDE);
     memcpy(tm_vertices.begin() + vi, vertex, 
    sizeof(float)*TM_VERTEX_STRIDE);
   ret = vi / TM_VERTEX_STRIDE;
  }
.....
 

This is extremely bad, it is making an assumption about the way a vector works internally, that is that the memory for consecutive entries in the vector is contiguous. While you might hope that this is the case there is nothing in the standard that requires this as far as I am aware. This is a short cut hack to filling the vector to try to increase speed (I guess) but is very very poor form as it is not guaranteed to work.

   std::memcpy((float*)(*(tm_vertices.begin() + vi)), vertex, 
                       sizeof(float)*TM_VERTEX_STRIDE);
 

Your problem is this

(float*)(*(tm_vertices.begin() + vi))

*(tm_vertices.begin() + vi) has type float (that is how vectors work) and you are trying to cast it to float * which just doesn't work, hence the error

cannot convert '....' from type `float' to type `float*'

My second question is, what costs would be associated with changing the resize() and memcpy() lines to use a push_back() instead?

Well it is almost bound to be a little slower. A slightly better plan is to leave in the resize() because that will speed things up by getting the extra memory required in a single chunk. The memcpy has to be replaced because of what I have said above, I would replace it with a simple loop using the array operator []

LOOP
tm_vertices[vi+LoopCounter] = vertex[LoopCounter];
LOOP END

[] assumes the memory is already available so wont be checking to see if more memory is required making it faster than push_back but necessitating the resize call first.