Oh crap, a seg fault in SDL.

Started by
5 comments, last by Dood77 16 years, 6 months ago
I went through the first part of the cone3D SDL tutorials where it shows you how to fill the screen with colorful pixels, and I've been trying to expand it into a pong game with a nice object-oriented approach. (I'm using Dev-C++) Here's the deal: 3 struct definitions: color, box, and wholeScreen. (box and wholeScreen contain an instance or two of the color struct for various color properties) (box and wholeScreen also contain the function draw() that will use for() loops and the DrawPixel function in the tutorial to draw themselves on the buffer, but the buffer is not flipped until the end of the 'game loop' in main()) I've created a successful instance of the wholeScreen struct called bg, for coloring the whole background. I've created two instances of box, one for player 1 called p1, and one for a center line called vline. When I created the third, (p2), the program compiled, but ran and shut very quickly, with stderr.txt reading seg fault (SDL Parachute, yadda, yadda) If I comment out any one of the instances of box, (p1, p2 or vline) the seg fault doesn't happen, it also works to comment out the wholeScreen instance (bg). When I use the debugger, I get a bunch of confusing stuff, but I do see that the segmentation fault happens at the DrawPixel() function. Here's my source:

#include <stdlib.h>
#include <iostream>

#include <SDL/SDL.h>

using namespace std;
//screen resolution and bits per pixel
const int SCRRESX = 640;
const int SCRRESY = 480;
const int SCRBPP = 32;

// Check if screen needs to be locked and lock it on func call:
void Slock(SDL_Surface *screen)
{
  if ( SDL_MUSTLOCK(screen) )
  {
    if ( SDL_LockSurface(screen) < 0 )
    {
      return;
    }
  }
};
//unlock screen:
void Sulock(SDL_Surface *screen)
{
  if ( SDL_MUSTLOCK(screen) )
  {
    SDL_UnlockSurface(screen);
  }
};


//function to draw a pixel as dictated by SDL tutorial
void DrawPixel(SDL_Surface *screen, int x, int y,
                                    Uint8 R, Uint8 G, Uint8 B)
{
  Uint32 color = SDL_MapRGB(screen->format, R, G, B);
  switch (screen->format->BytesPerPixel)
  {
    case 1: // Assuming 8-bpp
      {
        Uint8 *bufp;
        bufp = (Uint8 *)screen->pixels + y*screen->pitch + x;
        *bufp = color;
      }
      break;
    case 2: // Probably 15-bpp or 16-bpp
      {
        Uint16 *bufp;
        bufp = (Uint16 *)screen->pixels + y*screen->pitch/2 + x;
        *bufp = color;
      }
      break;
    case 3: // Slow 24-bpp mode, usually not used
      {
        Uint8 *bufp;
        bufp = (Uint8 *)screen->pixels + y*screen->pitch + x * 3;
        if(SDL_BYTEORDER == SDL_LIL_ENDIAN)
        {
          bufp[0] = color;
          bufp[1] = color >> 8;
          bufp[2] = color >> 16;
        } else {
          bufp[2] = color;
          bufp[1] = color >> 8;
          bufp[0] = color >> 16;
        }
      }
      break;
    case 4: // Probably 32-bpp
      {
        Uint32 *bufp;
        bufp = (Uint32 *)screen->pixels + y*screen->pitch/4 + x;
        *bufp = color;
      }
      break;
  }
};
//structs
struct color 
{
       int r;
       int g;
       int b;
};
struct box
{
       int width;
       int height;
       int x;
       int y;
       struct color borderColor;
       struct color innerColor;
       void draw(SDL_Surface *screen)
       {
            Slock(screen);
       //border
         for (int xloop = x;xloop<x+width;xloop++)
         {
              for (int yloop = y;yloop<y+height;yloop++)
              {
                  
                  DrawPixel(screen,xloop,yloop,borderColor.r,borderColor.g,borderColor.b);
                  
              }
         }
       //inside
         for (int xloop = (x+1);xloop<x+(width-1);xloop++)
         {
              for (int yloop = (y+1);yloop<y+(height-1);yloop++)
              {
                  
                  DrawPixel(screen,xloop,yloop,innerColor.r,innerColor.g,innerColor.b);
                  
              }
         }
             Sulock(screen);
       }
};
struct wholeScreen
{
       int width;
       int height;
       struct color color;
       void draw(SDL_Surface *screen)
       {

        Slock(screen);
         for(int x=0;x<width;x++)
          {
             for(int y=0;y<height;y++)
             {
                      DrawPixel(screen, x, y, color.r, color.g, color.b);
             }
          }

         Sulock(screen);
        }
};

//draw background


//----------------MAIN------------------
int main(int argc, char *argv[])
{
                   //error checking
  if ( SDL_Init(SDL_INIT_AUDIO|SDL_INIT_VIDEO) < 0 )
  {
    cout << "Unable to init SDL: " << SDL_GetError() <<endl;
    exit(1);
  }
  atexit(SDL_Quit);
                   //setup surface
  SDL_Surface *screen;
  screen=SDL_SetVideoMode(SCRRESX,SCRRESY,SCRBPP,SDL_HWSURFACE|SDL_DOUBLEBUF);
                   //error check surface
  if ( screen == NULL )
  {
    cout << "Unable to set 640x480 video: " << SDL_GetError() << endl;
    exit(1);
  }

//objects and properties

struct wholeScreen bg;
       bg.width = SCRRESX;
       bg.height = SCRRESY;
       bg.color.r = 255;
       bg.color.g = 255;
       bg.color.b = 255;
       
struct box p1;
       p1.width = 8;
       p1.height = 32;
       p1.borderColor.r = 0;
       p1.borderColor.g = 0;
       p1.borderColor.b = 0;
       p1.innerColor.r = 0;
       p1.innerColor.g = 200;
       p1.innerColor.b = 0;
       p1.x = 32;
       p1.y = (SCRRESY/2)-(p1.height/2);
       
struct box p2;
       p2.width = 8;
       p2.height = 32;
       p2.borderColor.r = 0;
       p2.borderColor.g = 0;
       p2.borderColor.b = 0;
       p2.innerColor.r = 200;
       p2.innerColor.g = 0;
       p2.innerColor.b = 0;
       p2.x = (SCRRESX - 32);
       p2.y = (SCRRESY/2)-(p2.height/2);
       
struct box vline;
       vline.width = 4;
       vline.height = SCRRESY;
       vline.x = (SCRRESX/2)-(vline.width/2);
       vline.borderColor.r = 0;
       vline.borderColor.g = 100;
       vline.borderColor.b = 100;
       vline.innerColor.r = 0;
       vline.innerColor.g = 100;
       vline.innerColor.b = 100;
    
    //----------Game loop-----------
int done = 0;
while (done == 0)
{
      SDL_Event event;

    while ( SDL_PollEvent(&event) )
    {
      if ( event.type == SDL_QUIT )  {  done = 1;  }

      if ( event.type == SDL_KEYDOWN )
      {
           //toying around with movement...
        p1.y+=20;
        if (p1.y>480)
        {
                     p1.y = 0;
        }
      } 
    }
    
    //draw objects
    bg.draw(screen);
    vline.draw(screen);
    p1.draw(screen);
    p2.draw(screen);
    //flip buffer
    SDL_Flip(screen);
}
//program end
delete screen;
return 0;
}


[Edited by - Dood77 on October 4, 2007 11:54:17 AM]
Advertisement
[ source ]...code here...[ /source ]
First off, only delete what you new. In this case, since you didn't use new to get the "screen" pointer, you should not call delete on it. In fact, the SDL_SetVideoMode documentation explicitly states:
Quote:The surface returned is freed by SDL_Quit and should not be freed by the caller. Note: This rule includes consecutive calls to SDL_SetVideoMode (i.e. resize or resolution change) - the pre-existing surface will be released automatically.


As for your problem, I think you should really check out SDL_FillRect.

Finally, one must note that your current approach isn't really "Object Oriented" at all. This is not a criticism however, as OO can be overkill for smaller projects such as pong.

As for the reason, I'm guessing you are walking past the edge of the screen buffer while copying pixels. Copying pixels in this manner is not only unsafe if you don't do proper range checking, but also likely to be horribly slow. Hence FillRect.

Finally, to get a pretty source code box, hit "edit" on the top right hand of your post and put [source]/* code goes here */[/source] around the code.

You'll get something like:
this!
Thanks for the help, I'll check out that link.

I guess I thought, since I'm redrawing over pixels that were previously drawn, then they would just be replaced in the buffer and the size wouldn't increase. Obviously I don't have that much of an understanding of what exactly is happening here...

This is my first attempt at OOP, why do you say it's not truly Object-Oriented? Is there a tutorial you recommend?
Wow topics get buried fast in this forum,
I'll just *bump* this thread once...
Quote:Original post by Dood77
This is my first attempt at OOP, why do you say it's not truly Object-Oriented?


Well, you don't even have any classes. If not for the few functions inside structs, this code could be C.

You have some global functions floating around, which should probably be methods in some kind of SDL_Surface wrapper class. And your main function does way too much.

The first half of this Wikipedia page is worth reading carefully. You're welcome to Google for OOP tutorial to find friendlier resources.
Classes are exactly the same as structs, except by default, all of its members are public. Since I didn't have any need for private or protected members, I used structs of which I was more familiar with.

Thanks for the tips, I've found a nicely detailed tutorial at http://www.cplusplus.com

EDIT: It would be a lot easier for me to know the markup for this asp board if it told me somewhere on the reply page...

This topic is closed to new replies.

Advertisement