It looks like you are not creating the surfaces stored in your m_tiles array properly. You are just allocating the surface structures via new , which with C-style structures (as SDL_Surfaces are) doesn't initialize any data. All you are doing is allocating uninitialized SDL_Surface structures; the format, depth, pitch, flags or dimension fields are not filled in, and the pixels array is not allocated.

You should instead create your surfaces with the SDL_CreateRGBSurface() call, passing it the dimensions, bit depth, etc... of the surface you are trying to create so that the surface is properly allocated before you try to blit to it. Otherwise, you are blitting to an uninitialized surface, which will fail.

Also, on cleanup you should use the SDL_FreeSurface() function to properly dispose of a surface when you are done with it.

Check the documentation for the SDL_CreateRGBSurface() for more details.

EDIT: Also, when you allocate AllTiles via new, then proceed to assign to AllTiles the pointer returned by SDL_LoadBMP(), you are creating a memory leak, as the memory you allocated via new is not deallocated. SDL_LoadBMP() automatically allocates a new surface, prepares it with the image data, and returns a pointer to it, overwriting the new-allocated pointer.

Josh
vertexnormal AT linuxmail DOT org

Check out Golem: Lands of Shadow, an isometrically rendered hack-and-slash inspired equally by Nethack and Diablo.

[edited by - VertexNormal on January 3, 2004 7:16:08 PM]

quote:Original post by sanguineraven

*snip*  	SDL_Surface *CurrentTile;	SDL_Rect srcRect;*snip*		CurrentTile = &m_tiles[tilenum];	CurrentTile = SDL_CreateRGBSurface(SDL_SWSURFACE, tilesize, tilesize, 32, rmask, gmask, bmask, amask);*snip* 

Here, you are making a mistake. You are setting the temporary variable CurrentTile equal to the address of one of your m_tiles structures, then you immediately turn around and assign the value of the newly allocated surface returned from SDL_CreateRGBSurface(). This assignment doesn''t affect your m_tiles array at all, just the current value of CurrentTile, which is overwritten with a new surface pointer each time a surface is created.

The best way is to let SDL do all allocation and freeing of actual surface structures.

I think what you want instead is to change the declaration of your m_tiles array and your Initialise() function to something like this:

class CTileSet{public:	int				Initialise(char *filename, int tilesize, int setsize);	int				Free();	void			Draw(int num,int x,int y);	private:	SDL_Surface **	m_tiles;	// CHANGE: Make this a pointer to a pointer, or an array of surface pointers};// Initializeint CTileSet::Initialise(char *filename, int tilesize, int setsize){	m_tiles = new (SDL_Surface*)[setsize]; // CHANGE: Allocate an array of pointers, rather than array of surfaces	SDL_Surface *AllTiles = SDL_LoadBMP("data/Tilesets/test.bmp"); // change to dir/filename    Uint32 rmask, gmask, bmask, amask;	#if SDL_BYTEORDER == SDL_BIG_ENDIAN		rmask = 0xff000000;		gmask = 0x00ff0000;		bmask = 0x0000ff00;		amask = 0x000000ff;	#else		rmask = 0x000000ff;		gmask = 0x0000ff00;		bmask = 0x00ff0000;		amask = 0xff000000;	#endif	// SDL_Surface *CurrentTile;  // CHANGE: Don''t really need this.	SDL_Rect srcRect;	srcRect.w = tilesize;	srcRect.h = tilesize;	for(int tilenum=0; tilenum<setsize; tilenum++)	{		// CurrentTile = &m_tiles[tilenum];  // CHANGE: Don''t need this		// CurrentTile = SDL_CreateRGBSurface(SDL_SWSURFACE, tilesize, tilesize, 32, rmask, gmask, bmask, amask);  // CHANGE: Change this to...		m_tiles[tilenum] = SDL_CreateRGBSurface(SDL_SWSURFACE, tilesize, tilesize, 32, rmask, gmask, bmask, amask);  // CHANGE: Assign to pointer in array.		srcRect.x = (int) ((tilenum%10)*(tilesize + 1));		srcRect.y = (int) ((tilenum/10)*(tilesize + 1));		SDL_BlitSurface(AllTiles, &srcRect, m_tiles[tilenum], NULL);  // CHANGE: Remove the & operator before m_tiles since we don''t need it anymore.	}        SDL_FreeSurface(AllTiles);  // CHANGE: The proper way to free up the surface          // This ensures that the pixels member of SDL_Surface is deleted properly	return 0;}// Free// Note for this, you should probably remember your setsize parameter from// Initialise, so you know how many surfaces you need to free.int CTileSet::Free(){        for(int tilenum=0; tilenum<setsize; tilenum++)        {                SDL_FreeSurface(m_tiles[tilenum]);        }        delete[] m_tiles;        m_tiles=0;}// Drawvoid CTileSet::Draw(int num, int x, int y){	SDL_Rect destRect;	destRect.x = x;	destRect.y = y;	SDL_BlitSurface(m_tiles[num], NULL, screen, &destRect); // CHANGE: Change this to match the new way.}

Or something like that. Note that I haven''t tested it or anything, but the gist of it is that your m_tiles array is now an array of pointers to SDL_Surface, rather than an array of SDL_Surface. Each pass through the loop on tilenum assigns the newly created surface pointer returned by SDL_CreateRGBSurface() to one of the pointers in the m_tiles array.

The Free() function loops through all the pointers and calls SDL_FreeSurface() for each one, then deletes the array of pointers after all surfaces are freed. Note that for this you should store the setsize parameter passed to Initialise(), so that you know how many you need to loop through.


Josh
vertexnormal AT linuxmail DOT org

Check out Golem: Lands of Shadow, an isometrically rendered hack-and-slash inspired equally by Nethack and Diablo.