SDL slowdown woes

[SadisticToaster]

Hey

I dusted off an old Blitz Basic project about a week ago , and decided to try and translate it into a C / SDL program. All was going well , with it working a lot better then in Basic : but then a few days back noticed that if left running for longer then about a minute , it started to get very , very slow : and eventually crash , leaving me with a message that windows had run out of memory , and was increasing virtual memory.

This should not be happening on a 1000 line C program which only has to draw a few .BMP's to the screen and check for user input.

A little fiddling around revealed this function - the one which puts the stuff on the screen - to be the problem , though , for the life of me , can not figure out how.

The only thing I could think of is that each time the function is run - every cycle - it's reloading the images to a new chunk of memory ,eventually filling it all up , but surely the SDL_FreeSurface should delete the old image , + it's runing from a function , which should delete all locally declared variables when it's done anyway?

void drawuniverse(SDL_Surface *screen,int year)
{
SDL_Surface *stars;
SDL_Surface *star;
SDL_Surface *sidebar;
SDL_Surface *sidebartop;
SDL_Surface *sidebarside;

sidebar = SDL_DisplayFormat(SDL_LoadBMP("unisidebar.bmp"));
sidebartop = SDL_DisplayFormat(SDL_LoadBMP("Sidebartop.bmp"));
star = SDL_DisplayFormat(SDL_LoadBMP("star.bmp"));
stars = SDL_DisplayFormat(SDL_LoadBMP("Stars.bmp"));
sidebarside = SDL_DisplayFormat(SDL_LoadBMP("unisidebarside.bmp"));

SDL_Surface *message = NULL;

font = TTF_OpenFont( "FreeSans.ttf", 16 );
SDL_Color textColor = { 0, 0, 0 };
char result[5];
sprintf( result, "%d", year );
message = TTF_RenderText_Solid( font, result, textColor );

drawSprite (stars,screen,0,0,2,screenw-150-stars->h,1020,600);
drawSprite (sidebar,screen,0,0,0,screenw-150,1024,150);
drawSprite (sidebartop,screen,0,0,0,0,1024,26);
drawSprite (sidebarside,screen,0,0,0,26,2,600);
drawSprite (sidebarside,screen,0,0,screenh-2,26,2,600);
drawSprite (message,screen,0,0,950,-3,60,50);
for (int x=0;x<40;x++)
{
drawSprite (star,screen,0,0,unidata[x].x,unidata[x].y,19,22);
}

SDL_FreeSurface( stars );
SDL_FreeSurface( star );
SDL_FreeSurface( sidebar );
SDL_FreeSurface( sidebartop );
SDL_FreeSurface( sidebarside );
}

Any ideas? I'm totally stumped. Many Thanks in advance

[Snakey]

Not done much SDL but you appear to be creating a font and not freeing it?

Also, freeing your surfaces in the exact reverse order that you create them is more efficient.

[piku]

What's "unidata"? I don't see that declared anywhere...
Nor is "font" declared anywhere...
What happens if "year" is larger than four characters?
What does "drawSprite" actually do?

Your bug is either a buffer overrun, or dereferencing of a NULL pointer.

GDB is your friend, as is valgrind.

[andym00]

At a guess (no idea about SDL, but a quick google told me what I expected) I imagine you should be doing a TTF_CloseFont(font) otherwise you'll be createing a new instance of the truetype font each frame you draw, which isn't very useful

If font is some global somewhere as you want to use it in lots of other places, you might just want to change it to:
if (!font) font = TTF_OpenFont( "FreeSans.ttf", 16 );
But in reality I'd arrange it all in some more manageable fashion really

[KlaxonCow]

Not done much SDL but you appear to be creating a font and not freeing it?

I don't know SDL either but, using a quick Google search, I've found this page about the "TTF_CloseFont" function:

http://jonatkins.org/SDL_ttf/SDL_ttf_18.html#SEC18

3.2.5 TTF_CloseFont

void TTF_CloseFont(TTF_Font *font)

font
Pointer to the TTF_Font to free.

Free the memory used by font, and free font itself as well. Do not use font after this without loading a new font to it.

See Also:
3.2.1 TTF_OpenFont, 3.2.2 TTF_OpenFontRW, 3.2.3 TTF_OpenFontIndex, 3.2.4 TTF_OpenFontIndexRW

Which strongly suggests that the "TTF_OpenFont" should be matched up with a "TTF_CloseFont" after it's been used to free up that memory or, yes, it very much sounds like it's just constantly allocating new memory for the font over and over and over again until Windows blows up.

Try "closing" the font using "TTF_CloseFont" just after you've finished using it in the function and see if that cures the issue.


To be honest, though, if this function is executed every frame then it seems vastly inefficient to be constantly allocating and deallocating, loading up the same bitmaps and font over and over again.

A seemingly more sensible (OO-like) structure would be:

Code: Select all

SDL_Surface *stars = NULL;
SDL_Surface *star = NULL;
SDL_Surface *sidebar = NULL;
SDL_Surface *sidebartop = NULL;
SDL_Surface *sidebarside = NULL;

SDL_Surface *message = NULL;

void InitUniverse(void)
{
   stars = SDL_DisplayFormat(SDL_LoadBMP("Stars.bmp"));
   star = SDL_DisplayFormat(SDL_LoadBMP("star.bmp"));
   sidebar = SDL_DisplayFormat(SDL_LoadBMP("unisidebar.bmp"));
   sidebartop = SDL_DisplayFormat(SDL_LoadBMP("Sidebartop.bmp"));
   sidebarside = SDL_DisplayFormat(SDL_LoadBMP("unisidebarside.bmp"));

   font = TTF_OpenFont( "FreeSans.ttf", 16 );
}

void DrawUniverse(SDL_Surface *screen,int year)
{
SDL_Color textColor = { 0, 0, 0 };
char result[5];

   sprintf( result, "%d", year );
   message = TTF_RenderText_Solid( font, result, textColor );

   drawSprite (stars,screen,0,0,2,screenw-150-stars->h,1020,600);
   drawSprite (sidebar,screen,0,0,0,screenw-150,1024,150);
   drawSprite (sidebartop,screen,0,0,0,0,1024,26);
   drawSprite (sidebarside,screen,0,0,0,26,2,600);
   drawSprite (sidebarside,screen,0,0,screenh-2,26,2,600);
   drawSprite (message,screen,0,0,950,-3,60,50);
   for (int x=0;x<40;x++)
   {
      drawSprite (star,screen,0,0,unidata[x].x,unidata[x].y,19,22);
   }
}

void FreeUniverse(void)
{
   TTF_CloseFont(font);

   SDL_FreeSurface( stars );
   SDL_FreeSurface( star );
   SDL_FreeSurface( sidebar );
   SDL_FreeSurface( sidebartop );
   SDL_FreeSurface( sidebarside );
}

(Code is illustrative only. No guarantee that it actually works provided. In fact, if "TTF_RenderText_Solid" creates a new surface every time then you would need to throw in a "SDL_FreeSurface" beforehand to avoid another memory leak. Just trying to show the kind of structure I'm talking about.)

Then call "InitUniverse" once to set everything up, call "DrawUniverse" every frame to do the drawing and call "FreeUniverse" once at the end to clean everything back up.

If you only strictly need to do something once then you should strictly only do it once.

[Firefox]

At a guess (no idea about SDL, but a quick google told me what I expected) I imagine you should be doing a TTF_CloseFont(font) otherwise you'll be createing a new instance of the truetype font each frame you draw, which isn't very useful

If font is some global somewhere as you want to use it in lots of other places, you might just want to change it to:
if (!font) font = TTF_OpenFont( "FreeSans.ttf", 16 );
But in reality I'd arrange it all in some more manageable fashion really

You've hit the nail on the head there, Andy.

I've used SDL and the SDL Truetype font library a lot. You want to be calling TTF_OpenFont() once for each font + size combo when your program starts, and TTF_CloseFont() for each when your program closes.

Usually I just have some globals like:

Code: Select all

TTF_Font *lucida_typewriter_regular_12;
TTF_Font *lucida_typewriter_regular_16;

and then a function that sets up the fonts with calls like:

Code: Select all

lucida_typewriter_regular_12 = TTF_OpenFont("font/path/LucidaTypewriterRegular.ttf", 12);
if (NULL == lucida_typewriter_regular_12)
{
   fprintf(stderr, "Couldn't open TTF font: %s\n", SDL_GetError());
   exit(EXIT_FAILURE); // Or whatever...
}

lucida_typewriter_regular_16 = TTF_OpenFont("font/path/LucidaTypewriterRegular.ttf", 16);
if (NULL == lucida_typewriter_regular_16)
{
   fprintf(stderr, "Couldn't open TTF font: %s\n", SDL_GetError());
   exit(EXIT_FAILURE); // Or whatever...
}

or sometimes I have an array for each font type like this:

Code: Select all

#define MAX_FONT_SIZE (24)

TTF_Font *lucida_typewriter_regular[MAX_FONT_SIZE];
TTF_Font *lucida_sans_demi_bold[MAX_FONT_SIZE];

etc etc, and then have a function with some loops in it to open the fonts I'm interested in at different sizes and then populate the arrays.

And then there's a function to clean up all the fonts when the program exits.

[SadisticToaster]

Sorry for delay in responce : my net connection's been down

What's "unidata"? I don't see that declared anywhere...
Nor is "font" declared anywhere...

Globals : maybe a bad idea for a large data structre ?

What happens if "year" is larger than four characters?

Gives me 999 turns : which seems enought for a strategy game. Besides - this is verrrry early on , and I can easily stick on another character later if needed.

KlaxonCow : You've cracked it. Re-loading , and converting to a useable format each graphic each cycle was the killer. Re-done the program so it is now laid out in your style : and problem == fixed. However , for some reason , the free() functions ( one for each game mode ) cause it to occasionally crash. *sigh*.

( incidentally, the reason this never happened in the basic version was that I had all the graphics as globals , and thus only declared them once , at the beginning )

Many Thanks all