Help with my first SDL project stealing memory with time

Started by
6 comments, last by NDR008 15 years, 8 months ago
Hi, first a brief intro - I am BEng Motorsport Engineering student, so officially programming is not my strong suite. But then again, programming and computers were my first love, I just lost time. When I joined this forum back in 2003, I had just started doing some C++ and OpenGL stuff. Now I am working on an engineering project for which I want to program a graphical interface (2D type stuff), and as I love open source and cross-platform stuff, I decided to use SDL. I assembled a USB interface boards with digital/analogue inputs and outputs and PWM output. I am going to connect this to a car ECU and some homemade sensors, and create a car display. Eventually make it also play movies, audio etc... Now, right now I am creating the dummy interface. I've already made an OpenGL C++ program that actually communicates to my interface board, but now I am working on the actual thing. Now regarding my code. I originally did everything in int main() which I know is wrong, but I was trying to learn what things do. So I then started grouping lines of codes in procedures of the format: void func (void), purely to organise it till i get my head round it. Then I up-ed the resolution to 1280x800 which is my default desktop res. I noticed my program's display was not being refreshed while I moved around the mouse. When I re-ran the original version (all in int main()). The issue was not immediately apparent but now I realised that both version, given enough time, consume all of my system memory. I know there are SDL_FreeSurface - but should they be used constantly? Also, images and text files - should they be looped each time? Or am I wasting my memory elsewhere?

/***************************************************************************
 *   Copyright (C) 2008 by Nadir Syed Sammut                               *
 *   nadir.syedsammut@googlemail.com                                       *
 *                                                                         *
 *   This program is free software; you can redistribute it and/or modify  *
 *   it under the terms of the GNU General Public License as published by  *
 *   the Free Software Foundation; either version 2 of the License, or     *
 *   (at your option) any later version.                                   *
 *                                                                         *
 *   This program is distributed in the hope that it will be useful,       *
 *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
 *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
 *   GNU General Public License for more details.                          *
 *                                                                         *
 *   You should have received a copy of the GNU General Public License     *
 *   along with this program; if not, write to the                         *
 *   Free Software Foundation, Inc.,                                       *
 *   59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.             *
 ***************************************************************************/
 /********************
 * Long Live         *
 *    Open Source    *
 ********************/


#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

#include <iostream>
#include <stdlib.h>
#include <stdio.h>
#include <SDL.h>
#include <SDL/SDL_image.h>
#include <SDL/SDL_ttf.h>

using namespace std;

const int SCREEN_WIDTH=640;
const int SCREEN_HEIGHT=480;

 
SDL_Surface* g_pDisplaySurface = NULL;
SDL_Surface* temp = NULL;
SDL_Surface* g_pText = NULL;
SDL_Surface* LEDimage  = NULL;

Uint8* g_pKeys;
int rpm = 700;
int speed = 0;

SDL_Event g_Event;
SDL_Rect LED[3][2];
SDL_Rect g_DstRect;
SDL_Rect g_DTxtRect,g_STxtRect;
SDL_Rect g_Display;
int rev[19][3] = {34, 131, 0, 63, 116, 0, 92, 102, 0, 122, 89, 0, 152, 77, 0, 182, 66, 0, 213, 56, 0, 244, 46, 0, 275, 38, 0, 307, 30, 0, 338, 24, 0, 370, 18, 0, 402, 14, 0, 435, 12 , 1, 467, 11, 1, 500, 11, 2, 532, 14, 2, 564, 19, 2, 594, 29, 2};

TTF_Font* font_rpm;
TTF_Font* font_speed;
SDL_Color font_rpm_color={255, 0, 0, 0};
SDL_Color font_unit_color={255, 255, 0, 0};
SDL_Colour font_rpm_back={0, 0, 0, 255};

void draw_rpm(void);
void draw_speed(void);
void draw_rpm_bar(void);
void draw_speed_bar(void);
void check_keys(void);
void clean_up(void);

void draw_rpm(void){
  char buffer[10];
  
  g_DTxtRect.y = 80;
  g_pText = TTF_RenderText_Shaded( font_rpm, "    ",  font_rpm_color, font_rpm_back);
  g_STxtRect.h = g_DTxtRect.h = g_pText->h;
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (560-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  sprintf(buffer, "%d", rpm);
  g_pText = TTF_RenderText_Shaded( font_rpm, buffer,  font_rpm_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (560-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  SDL_FreeSurface( g_pText );
  
}

void draw_speed(void){
  const int speed_loc=104;
  const int mph_loc = 324;
  const int mps_loc = 540;
  char buffer[10];
  
  g_DTxtRect.y = 280;
  g_pText = TTF_RenderText_Shaded( font_speed, "   ",  font_rpm_color, font_rpm_back);
  g_STxtRect.h = g_DTxtRect.h = g_pText->h;
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (speed_loc-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  sprintf(buffer, "%d", speed);
  g_pText = TTF_RenderText_Shaded( font_speed, buffer,  font_rpm_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (speed_loc-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  g_pText = TTF_RenderText_Shaded( font_speed, "km/h",  font_unit_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (speed_loc);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  g_pText = TTF_RenderText_Shaded( font_speed, "   ",  font_rpm_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (mph_loc-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  int mph=speed/1.6;
  sprintf(buffer, "%d", mph);
  g_pText = TTF_RenderText_Shaded( font_speed, buffer,  font_rpm_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (mph_loc-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  g_pText = TTF_RenderText_Shaded( font_speed, "mph",  font_unit_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (mph_loc);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  g_pText = TTF_RenderText_Shaded( font_speed, "    ",  font_rpm_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (mps_loc-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  int mps=speed*1000/60;
  sprintf(buffer, "%d", mps);
  g_pText = TTF_RenderText_Shaded( font_speed, buffer,  font_rpm_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (mps_loc-g_STxtRect.w);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  g_pText = TTF_RenderText_Shaded( font_speed, "m/s",  font_unit_color, font_rpm_back);
  g_STxtRect.w = g_DTxtRect.w = g_pText->w;
  g_DTxtRect.x = (mps_loc);
  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);
  
  SDL_FreeSurface( g_pText ); 
}


void draw_rpm_bar(void){
      for (int i=0; i < 19; i++)
      {
        g_DstRect.x = rev[0];
        g_DstRect.y = rev[1];
        // cout << "x: " << g_DstRect.x << ", y: " << g_DstRect.y << endl;
        if (rpm/500 > i)
        {
        if (rev[2] == 0) { SDL_BlitSurface(LEDimage,&LED[0][0],g_pDisplaySurface,&g_DstRect);}
        if (rev[2] == 1) { SDL_BlitSurface(LEDimage,&LED[1][0],g_pDisplaySurface,&g_DstRect);}
        if (rev[2] == 2) { SDL_BlitSurface(LEDimage,&LED[2][0],g_pDisplaySurface,&g_DstRect);}
        }
        else
        {
        if (rev[2] == 0) { SDL_BlitSurface(LEDimage,&LED[0][1],g_pDisplaySurface,&g_DstRect);}
        if (rev[2] == 1) { SDL_BlitSurface(LEDimage,&LED[1][1],g_pDisplaySurface,&g_DstRect);}
        if (rev[2] == 2) { SDL_BlitSurface(LEDimage,&LED[2][1],g_pDisplaySurface,&g_DstRect);}
        }
      }
}

void draw_speed_bar(void){
      g_DstRect.y = 240;
      for (int i=0; i< 26; i++)
      {
        g_DstRect.x = 34+i*22;
        if (speed/10 > i) { SDL_BlitSurface(LEDimage,&LED[0][0],g_pDisplaySurface,&g_DstRect);}
        else { SDL_BlitSurface(LEDimage,&LED[0][1],g_pDisplaySurface,&g_DstRect);}
      }
}

void check_keys(void) {
    g_pKeys = SDL_GetKeyState(NULL);
    
    if((g_pKeys[SDLK_KP_PLUS]==1) && rpm < 9500 ) {rpm+=31; if (rpm > 9500) { rpm = 9500;} }
    if((g_pKeys[SDLK_KP_MINUS]==1) && rpm > 0 ) {rpm-=31; if (rpm < 0) { rpm = 0;} }
    if((g_pKeys[SDLK_a]==1) && speed < 260 ) {speed+=1; if (speed > 260) { speed = 260;} }
    if((g_pKeys[SDLK_z]==1) && speed > 0 ) {speed-=1; if (speed < 0) { speed = 0;} }
    if(g_pKeys[SDLK_q]==1  ) {clean_up();}
}

void clean_up(void) { 
/*
//Free the surfaces 
SDL_FreeSurface( g_pDisplaySurface ); 
SDL_FreeSurface( g_pText ); 
SDL_FreeSurface(LEDimage);
SDL_FreeSurface(temp);
//Close the font that was used 
TTF_CloseFont( font_rpm ); 
TTF_CloseFont( font_speed ); 
//Quit SDL_ttf 
TTF_Quit();
*/ 
//Quit SDL 
SDL_Quit();
exit(0);
} 

int main(int argc, char* argv[])
{
  SDL_Init(SDL_INIT_VIDEO);
  
  if(TTF_Init()==0)
  {
    atexit(clean_up);
  }
  //SDL_FULLSCREEN
  //SDL_ANYFORMAT
  g_pDisplaySurface = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, 0, SDL_ANYFORMAT);
  if(!g_pDisplaySurface) {cout << "Could not Set Video Mode" << endl; exit(1);}
  g_Display.x = g_Display.x = 0;
  g_Display.w = SCREEN_WIDTH;
  g_Display.h = SCREEN_HEIGHT;
  
  font_rpm = TTF_OpenFont("LEDBDREV.TTF", 72);
  if (font_rpm == NULL) {cout << "Font not found1" << endl;}
  
  font_speed = TTF_OpenFont("LEDBDREV.TTF", 28);
  if (font_speed == NULL) {cout << "Font not found2" << endl;}
  
  temp = IMG_Load("LED.png");
  if(!temp) {cout << "File not found" << endl; exit(1);}
  LEDimage = SDL_DisplayFormat(temp);
  SDL_FreeSurface(temp);

  for (int i = 0; i < 3; i++){
  for (int j = 0; j < 2; j++){
  LED[j].x = j*24; LED[j].y = i*24; LED[j].w=24; LED[j].h=24;
  }
  }
  
  SDL_SetColorKey(LEDimage,SDL_SRCCOLORKEY, SDL_MapRGB(g_pDisplaySurface->format, 0, 0, 0));
  
  g_DstRect.w = 24;
  g_DstRect.h = 24;
  g_STxtRect.x = 0;
  g_STxtRect.y = 0;
    
  for(;;)
  {  
    
    //SDL_FillRect( g_pDisplaySurface, &g_Display, SDL_MapRGB( g_pDisplaySurface->format, 0, 0, 0));
    
    g_pKeys = SDL_GetKeyState(NULL);
    
    check_keys();
    
    if(SDL_PollEvent(&g_Event)==0)
    {
      draw_rpm();
      draw_rpm_bar();
      draw_speed();
      draw_speed_bar();
    }
      //SDL_UpdateRect(g_pDisplaySurface,0,0,0,0); //For Non Fullscreen
      SDL_Flip(g_pDisplaySurface);

  }
  return(0);
}

This just simply displays an 'LED' bar rev counter and 'LED' bar speedometer. As well as a text version (trying to mimic the look of KITT's dash). I know I did some things - aspecially the way I displayed the text, in a very unclean style, but besides bad style, could someone help me figure out why am I losing memory so much? Only been back on C++ for 2 weeks in the last 5 years and on SDL for the last 2 days. So go easy on me guys (& gals if any). [Edited by - NDR008 on August 12, 2008 9:23:29 AM]
Advertisement
In each of draw_rpm() and draw_speed(), you create multiple SDL_Surfaces (using TTF_RenderText_Shaded), but you only delete the last one. This means that each frame, multiple SDL_Surfaces are being created and then 'lost', which is using up your memory.

The simplest fix would be to free each surface as soon as you are done with it. Since you are reusing the same pointer for each surface, you need to call SDL_FreeSurface on the pointer immediately before assigning the next surface to the pointer.

Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]

TTF_RenderText_Shaded() creates a new surface and returns it, so you need to call SDL_FreeSurface() before calling it again, you have this problem in rather a few places which will be eating memory rather quickly.
Game development blog and portfolio: http://gamexcore.co.uk
I see, forgive my ignorance, as I am still learning but, each time I do a render, since I am returning it to the same pointer, would it not be kind of overwritting in memory the previous allocation?

Or it is actually creating a new memory space and pointer to that new space? I think I got it. Will try tomorrow (1:33am here).

Is this issue only with TTF_RenderText_Shaded or is it also with SDL_Surfaces that I am using to store the loaded images since those return pointers too?
Quote:Original post by NDR008
Or it is actually creating a new memory space and pointer to that new space? I think I got it. Will try tomorrow (1:33am here).
Pretty much correct. It allocates a new chunk of memory, and overwrites the previous pointer. The previous block of memory is still around, but you now have no way to refer to it.
Quote:Is this issue only with TTF_RenderText_Shaded or is it also with SDL_Surfaces that I am using to store the loaded images since those return pointers too?
they do need to be freed eventually, but if you are using them throughout the program, it is fine to free them when you exit (or even let the OS do it for you, as it automatically frees all memory at exit).

Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]

I know I said I was off to bed, but I am forcing myself to try get this:

I made changes like so:

  SDL_FreeSurface(g_pText);    g_DTxtRect.y = 80;  g_pText = TTF_RenderText_Shaded( font_rpm, "    ",  font_rpm_color, font_rpm_back);  g_STxtRect.h = g_DTxtRect.h = g_pText->h;  g_STxtRect.w = g_DTxtRect.w = g_pText->w;  g_DTxtRect.x = (560-g_STxtRect.w);  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);


I am getting an error. Do I have to reallocate space by means of SDL_Surface* g_pText; before each TTF_RenderText_Shaded?

As is I get segment fault on run.

[Edited by - NDR008 on August 12, 2008 9:15:02 AM]
Youve got the SDL_FreeSurface() in the wrong place^^ when it called for the first time g_pText is still jsut a pointer to NULL since you havnt created a surface yet, the correct order is:

[source language="cpp"]  g_DTxtRect.y = 80;  g_pText = TTF_RenderText_Shaded( font_rpm, "    ",  font_rpm_color, font_rpm_back);  g_STxtRect.h = g_DTxtRect.h = g_pText->h;  g_STxtRect.w = g_DTxtRect.w = g_pText->w;  g_DTxtRect.x = (560-g_STxtRect.w);  SDL_BlitSurface(g_pText,&g_STxtRect,g_pDisplaySurface,&g_DTxtRect);SDL_FreeSurface(g_pText);


TTF_RenderText_Shaded() creates a surface which g_pText is pointing to, you blit that surface then you call SDL_FreeSurface() to delete it and clean up. after that you can create a new surface and have g_pText point to it, without leaks.
Game development blog and portfolio: http://gamexcore.co.uk
Thanks, solved, thought that having an extra SDL_FreeSurface at the start would just reset it to point to a NULL again and that it would serve its purpose when the programme loops, but I was wrongs.

Thanks everyone, Program running fast and low on memory again. :)

This topic is closed to new replies.

Advertisement