CMap::CMap(CMap &map)
{
// This function is the copy constructor for the CMap class
int index = 0; // looping variable
// copy the shallow members
this->map_height = map.map_height;
this->map_width = map.map_width;
this->scale = map.scale;
this->scrn_height = map.scrn_height;
this->scrn_width = map.scrn_width;
this->start_tile = map.start_tile;
strcpy(this->mapname, map.mapname);
strcpy(this->tileset, map.tileset);
// Make new bmps and hdcs
this->hdc = *new HDC;
this->bmp = *new HBITMAP;
this->old_bmp = *new HBITMAP;
// Copy the deep members
this->hdc = CreateCompatibleDC(map.hdc); // is this right? or does it need to be "this->hdc = map.hdc"?
this->bmp = map.bmp; // Same question here?
this->old_bmp = map.old_bmp;
} // end CMap(CMap &map);
Memory leak?
Hey guys,
This is my first attempt at a copy constructor, but I'm not sure how exactly to go about copying the pointers. For example, I know I have to create a new HDC via the 'new' pointer, but do I set the new HDC equal to the hdc of the object being copied, or do I create a HDC that is compatible with it? Here's the code:
Thanks,
The Joshinator
If you have the desire and need for the most accurate answer, download MMGR and add it to your project. Follow the directions in the .h and .cpp file which are #include the mmgr.h after system header files but before yours so it will compile correctly. Simply run once and look at the log generated. It will tell you if you have any memory leaks. I can personally vouch for this free code. It is the best thing I have ever seen. Not only that, Fluid Studios has some more great code, such as a logger for use.
Now if you do verify that you have memory leaks, someone else will have to exaplain the whole logics behind it. I am not that well versed with the whole Win32 drawing contexts. Hope this helps.
- Drew
Now if you do verify that you have memory leaks, someone else will have to exaplain the whole logics behind it. I am not that well versed with the whole Win32 drawing contexts. Hope this helps.
- Drew
I think you're just creating a new pointer to point to the original data. Try memcpy and use some profiler to check for leakage like the above poster suggest.
Also try creat an instance of the map and then copy it then delete the original and see what happen to the copy data.
Also try creat an instance of the map and then copy it then delete the original and see what happen to the copy data.
You dont' need to create a new pointer to an hdc, just using createcompatableDC will work.
You should also modify your decalaration to take a const CMap& not a CMap&. Same for the Bitmaps, generally a deep copy cstr would create a clone of each object not another reference to each object. If all you need is a reference then a shallow copy cstr should work as none of your types is reference counted.
Cheers
Chris
You should also modify your decalaration to take a const CMap& not a CMap&. Same for the Bitmaps, generally a deep copy cstr would create a clone of each object not another reference to each object. If all you need is a reference then a shallow copy cstr should work as none of your types is reference counted.
Cheers
Chris
Quote:...generally a deep copy cstr would create a clone of each object not another reference to each object.
That is what I want to do - create a deep copy with clones instead of references. How do i clone a HBITMAP, data and all?
You should never have a '*' in front of new like that. Not only are you not ever storing the pointer ANYWHERE, but since the fields themselves are not pointers to begin with then there would never be any need to allocate at all, so yes you definately have 3 leaks there.
You most certainly do mean to have pointers however, and want to allocate new new bitmaps etc and copy memory from one to the other.
Can you show us the declaration of CMap please?
You most certainly do mean to have pointers however, and want to allocate new new bitmaps etc and copy memory from one to the other.
Can you show us the declaration of CMap please?
Here it is:
// CMap classclass CMap{ public: // public functions CMap(); // Constructor: Sets everything to 0 or NULL CMap(const CMap &map); // Copy Constructor: makes deep copies of a CMap object void Set_Mapname(char *name); // Set the mapname void Set_Tileset(char *filename); // The tileset to use void Set_Start_Tile(int index) {start_tile = index;} // Set the starting tile void Set_Map_Width(int width) {map_width = width+2;} // Set the map width void Set_Map_Height(int height) {map_height = height+2;} // Set the map height void Set_Screen_Width(int width) {scrn_width = width+2;} // Set the screen width void Set_Screen_Height(int height) {scrn_height = height+2;} // Set the screen height void Set_Tile_Width(int width) {tile_width = width;} // Set the tile width void Set_Tile_Height(int height) {tile_height = height;} // Set the tile height void Set_Scale(float percent_change) {scale = percent_change/100;} // Set the scale of the map char *Get_Mapname() {return mapname;} // Returns the mapname int Get_Tile_Width() {return tile_width;} // Returns the width of the tiles int Get_Tile_Height() {return tile_height;} // Returns the height of the tiles int Get_Map_Width() {return map_width;} // Returns the width of the map in tiles int Get_Map_Height() {return map_height;} // Returns the height of the map in tiles int Add_Layer(CWindraw *windraw); // Adds one layer to the map void Add_Tileset(int tile_index, // Adds one tileset to the map char *filename); int Draw_Tiles(HDC dest_hdc); // Draw all the tiles int Load(char *filename); // Loads a map from disk void Reset(); // Resets all the variables to clear the way for the new data int Save(char *filename); // Save the current map ~CMap(); // Frees up the memory protected: // protected members char mapname[80], // Name of the map tileset[80]; // Name of the tileset to use int start_tile, // Index of the starting tile x,y, // Absolute coords of the map for scrolling purposes map_width, // Width of the map in tiles map_height, // Height of the map in tiles scrn_width, // Width of the screen in tiles scrn_height, // Height of the screen in tiles tile_width, // Width of each tile tile_height; // Height of each tile float scale; // scale to use for zooming vector<CLayer*> layers; // Vector of layers HDC hdc; // HDC to use for tilesets HBITMAP bmp; // bmp to use for tilesets HBITMAP old_bmp; // used to save old bmp};
Sidenote
Storing HDC like you do is a no-no. HDC are global resources, and you should release them when you don't use them anymore. Moreover, it seems you are trying to duplicate them - because you are storing them. Using this you may create a system wide HDC shortage which could result in undefined behaviors (unpaint buttons or control are the first symptomes).
Are you sure you really need it? And, if you need it, are you sure you really want to clone it?
Regards,
Storing HDC like you do is a no-no. HDC are global resources, and you should release them when you don't use them anymore. Moreover, it seems you are trying to duplicate them - because you are storing them. Using this you may create a system wide HDC shortage which could result in undefined behaviors (unpaint buttons or control are the first symptomes).
Are you sure you really need it? And, if you need it, are you sure you really want to clone it?
Regards,
Well I use this class in a tilemap editor, and I do release the hdc in the deconstructor. This isn't the only class that does this though. I also have CLayer and CTile, each of which have their own HBITMAPs and HDCs. A CMap is an array of CLayers, which in turn are arrays of CTiles. But like I said, all three classes release their resources when they go out of scope. I couldn't find any other way to have an array of bitmaps that I could blit to a backbuffer. Before I started the project I suspected that this wasn't exactly the best way, but the response from a few people at GT.com gave me the impression that it was a good way to go. This was a looooong time ago, so don't ask me for the thread or anything cause I won't remember lol. But is there a better way to do this?
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement