help using vectors with structs

Hello all,
I'm trying to get back into C++ programming, after a few years of Java (damned uni). I am particularly trying to use STL and classes, which i hardly used before (I guess java showed me how useful 'vectors' and more OO can be).

I'm trying to make a basic particle engine in OpenGL. It works by holding a vector of 'emitters', that then emit particles at a known rate (10/sec in my early tests). The particles are then copied into a vector (the emitter holds the values of the standard particle it emits). Both the emitter and particle is a struct.

My problem is that the particles don't seem to be created correctly. As soon as they are used (moved or drawn for example) the program crashes. I suspect it is a problem with using structs in a vector, or somehow a problem with my use of vectors.

Here are the structs, declared in a header file:

Code:

struct particle {
    GLfloat col[4]; //rgba
    GLfloat colfade[4]; //rgba
    GLfloat pos[3]; //x,y,z
    GLfloat vel[3]; //x,y,z
    GLfloat acc[3]; //x,y,z: mainly for gravity
    GLfloat partsize; //the size of the particles: height and width of each
};

struct emitter {
    //the default particle that will be created
    GLfloat col[4]; //rgba
    GLfloat colfade[4]; //rgba
    GLfloat pos[3]; //x,y,z
    GLfloat vel[3]; //x,y,z
    GLfloat acc[3]; //x,y,z: mainly for gravity
    GLfloat partsize; //the size of particles
    //add some 'variation', especially to velocity / colour
    double createrate, lastcreated; //the rate that particles are created per second
    double ttl; //time the emitter will 'die'
};

the vectors are then created:

Code:

std::vector <particle> parts;
std::vector <emitter> emitters;

And here is the function that actually makes the particles

Code:

void ParticleSystem::addparticles(double time){
    for (std::vector<emitter>::iterator iter = emitters.begin(); iter!=emitters.end(); ++iter) {
        emitter it = *iter;
        int numcreated = (int)((time-it.lastcreated)*it.createrate);
        if (numcreated>0){ //this ensures not creating excessive particles due to extra calls
            for (int i=0; i<numcreated; i++){ //create a particle at the given rate
                //create the particle
                std::cout << "creating a particle\n";
                particle temp;
                for (int c=0; c<4; c++) {
                    temp.col[c]=it.col[c];
                    temp.colfade[c]=it.colfade[c];
                }
                for (int c=0; c<3; c++){
                    temp.pos[c]=it.pos[c];
                    temp.vel[c]=it.vel[c];
                    temp.acc[c]=it.acc[c];
                }
                temp.partsize = it.partsize;
                //add it to the vector
                parts.push_back(temp);
            }
            it.lastcreated = time;
        }
    }
}

The for loops etc. are probably irrelevant, but shown to make sure I am iterating correctly. Basically, it works out how many particles should have been created since the method was last called, then creates that many by copying the 'default' values from the emitter into a 'temp' particle that is then copied into the vector. Once it is working I will add some randomness to the particles created, but for now I'd be happy with a single particle being drawn successfully :S

Thanks for reading, hopefully you can help me

Have you considered using a third party library. Or even an open source engine? so you could learn some from their code?

I use HGE for 2D projects, and Ogre Engine for 3D projects. They are both open source.

You have to implement a copy constructor for the vector to know how to allocate the internal array containing your custom data type.

I don't see anything particularly wrong with this code (not knowing what GLfloat is for sure). Are you sure that the problem is not where the crashes occur (stepping over array bounds, invalidated iterators, bad values in structs etc)?

One thing you might notice is that the emitter contains the same members as particle and some others in addition. Therefore it might be composed of a particle instance and the additional members. In this case you should be able to retrieve the particle item from emitter with a single assignment (automatically generated copy assignment/constructor should do the member-wise copy for you correctly).

>> You have to implement a copy constructor for the vector to know how to allocate the internal
>> array containing your custom data type.

No, you don't. The arrays in the structs will be copied correctly (unless GLFloat is a poorly designed class that doesn't copy correctly, which I doubt is true). The default copy constructor works fine.

I agree with anon, I see nothing wrong with that code. I don't see anything wrong with the way you are using the structs in the vectors in theory or practice.

I just did some more testing and found that 'it.lastcreated' isn't being updated. I suspect I have at least got that bit wrong.

You are making a copy of the emitter, so when you change the lastcreated value you are only changing the copy. You could use the iterator directly (use iter-> instead of it. in the code). If you want to create an alias for the value stored by the iterator use a reference:

Code:

emitter& it = *iter;

Then any changes to it will be reflected in the emitter stored in the vector.

BTW, "I see nothing wrong with the code" doesn't mean the code is error free, I just mean that I don't see any problems and especially not any that would relate to the crash you refer to.

thanks for the help so far. I fixed up that bit, and found where the problem was. The problem is when I delete a particle or emitter, it crashes. It looked like the problem was in the creation of the particles, since one of the other functions also called 'removeparticle', so it crashed in a function where I didn't expect it to (if the remove methods were flawed).

So here is one of the remove functions

Code:

void ParticleSystem::removeemitters(double time){
    for (std::vector<emitter>::iterator iter = emitters.begin(); iter!=emitters.end(); ++iter) {
        emitter it = *iter;
        if (time>it.ttl){
            std::cout <<"erasing an emitter\n";
            emitters.erase(iter);
        }
    }
}

I think the flaw is using .end() as the ending condition, especially when I am deleting the last (or only) element in the vector: so the current will never meet the '.end()'. It seems to output "erasing an emitter" endlessly (for only 1 emitter).

So how can I fix this function, or get a better ending condition?

Calling vector::erase() invalidates any iterators you might have that "point" into the vector. This means that you can no longer use those iterators, or if you do, the behaviour will be undefined.

Because vectors keep an array format, erasing on positions other than the vector end also moves all the elements after the segment erased to their new positions,
This invalidates all iterator and references to elements after position or first.

So . . . how can you do this? This may not be the best way, but I'd probably use a size_type index into the vector.

Code:

void ParticleSystem::removeemitters(double time){
    for (std::vector<emitter>::size_type x = 0; x < emitters.size(); ++x) {
        if (time>emitters[x].ttl){
            std::cout <<"erasing an emitter\n";
            emitters.erase(emitters.begin() + x);
        }
    }
}

That's what you get when you ask a C programmer . . . .

Oh my god, that delete is slow

Thanks, the code you showed me works. It's just that in a vector of 12,000 or so particles (once it reaches a 'steady state'), it feels like there's far too much lag in deletion. In fact, it was faster to just keep redrawing the 'dead' ones and let it get to 200,000 particles.

I don't really care about the order particles are redrawn in, would it be worthwhile writing my own delete function, perhaps swapping the deleted particle to the end of the vector then manually decreasing the vector size, instead of letting the function move every particle forwards by 1 if it deletes an early one.

I'm sure others have other suggestions, but what I'd do, maybe is to make an array of particles to delete and have another thread delete them. Synchronization might be a small problem, however.

One common way of deleting in a loop looks like this:

Code:

    std::vector<emitter>::iterator iter = emitters.begin();
    while (iter!=emitters.end()) {
        if (time>it.ttl){
            std::cout <<"erasing an emitter\n";
            iter = emitters.erase(iter);
        }
        else {
            ++iter;
        }
    }

erase returns the next iterator you should use to continue in the list, and you should only increment if you didn't erase (because the iterator returned by erase is already incremented).

If you're feeling spunky you can use remove_if instead.

You might want to consider not using a vector. vectors are very slow for insertion and deletion, IIRC. lists are very good for fast insertion and deletion. They're bad for random access, but it looks like you usually just step through the list anyway.

** smacks head on desk **

That's genius. I changed the particles to a list, and can't believe the speed increase I got for it. I left the emitters as a vector, since there are far fewer of them (probably less than 50), and I might want some random access later (also, it meant changing less code).

Thanks a lot all. This project is done, and I got a fancy looking flame along with heaps of experience in vectors and lists.

If the vector doesn't have to remain sorted after erasing, you can always swap the item that you want to erase with the last item in the vector and do a pop_back. That way you wouldn't move all the items around in the vector.