Quote:Original post by Palidine
Quote:Original post by nuvem
In any case, all that I can see is your assignment operator doesn't actually assign.
QFE. this is a huge problem. It means that every time you are pushing onto that list, no valid data is actually being copied into it.
-me
Yes.
It is worth nothing that in your case, the correct way to write the copy constructor, assignment operator and destructor is as follows:
I.e., nothing at all. The default behaviour - member-wise copies, assignments and cleanups - is exactly what you want.
Also, don't make 'load' functions. Do that work in the constructor, instead.
Also, use initialization lists to initialize data members where possible.
Also, it looks like you're storing a vector of "named" graphic objects, so that you can search through it, one at a time, to find the one you want later. Don't do that. Instead, use an associative container - std::map, in this case. It lets you use the graphic name directly as an "index" into the container (and then there is also no reason to store it as part of the graphic object).
class Databank { // etc. etc. std::map<string, Graphic> graphics; // instead of the vector};struct LoadFailure {};// Since constructors can't return, our Graphic constructor will throw an// exception on failure. Here's how we can wrap the exception to preserve the// original interface.bool Databank::loadGraphic(const string& id, const string& fileName) { try { graphics[id] = Graphic(fileName); return true; } catch (LoadFailure&) { return false; }}class Graphic { GLuint id; public: Graphic() : id(-1) {} Graphic(const std::string& filename); // You should only need == and <, and they should return bool. bool operator== (const Graphic &rhs) const { return id == rhs.id; } bool operator< (const Graphic &rhs) const { return id < rhs.id; } GLuint getTexture() { return id; }};Graphic::Graphic(const std::string& filename) { // Fix your AUX_RGBImageRec class to have a proper destructor etc., and // fix loadBMP to allocate it via 'new', and return a std::auto_ptr // instead of a raw pointer. std::auto_ptr<AUX_RGBImageRec> image = loadBMP(fileName); if (!image) { throw LoadFailure(); } glGenTextures(1, &id); glBindTexture(GL_TEXTURE_2D, id); glTexImage2D(GL_TEXTURE_2D, 0, 3, image->sizeX, image->sizeY, 0, GL_RGB, GL_UNSIGNED_BYTE, image->data); // Do you really want to redo this with each texture? glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);}
