Sign in to follow this  
Mozly

Why wont SDL_DisplayFormat work?!?!?!

Recommended Posts

Im following lazy foo's SDL tutorial to set the color key on an image.
I did many of his tutorials and now I have a problem that didn't exist before.

I load img1 but the line SDL_DisplayFormat(img1) DOES NOTHING ( found this out during debug) and so the program exits.What am I doing wrong?

Here is the snippet:

SDL_Surface* loadimg(std::string filename)
{
SDL_Surface* img1 = NULL;
SDL_Surface* img2 =NULL;
img1 = IMG_Load(filename.c_str());

if(img1 !=NULL)
{
img2 = SDL_DisplayFormat(img1);
SDL_FreeSurface(img1);
if(img2 != NULL)
SDL_SetColorKey(img2,SDL_SRCCOLORKEY,SDL_MapRGB(img2->format,0xFF,0xFF,0xFF));
}
return img1;
}


Thanks in advance.

Share this post


Link to post
Share on other sites
Quote:
Original post by Mozly
I load img1 but the line SDL_DisplayFormat(img1) DOES NOTHING ( found this out during debug) and so the program exits.What am I doing wrong?
How do you know that line does nothing? What does 'nothing' mean to you?

Anyway, let's assume did actually work and the converted img1 is put into img2. That means you are leaking img2 and returning img1 after freeing it. I'd have written your code more like this:
SDL_Surface* loadimg(std::string filename)
{
SDL_Surface* img1 = IMG_Load(filename.c_str());

if(img1 ==NULL)
{
return NULL;
}

SDL_Surface* img2 = SDL_DisplayFormat(img1);
SDL_FreeSurface(img1);
if(img2 == NULL)
{
return NULL;
}

SDL_SetColorKey(img2,SDL_SRCCOLORKEY,SDL_MapRGB(img2->format,0xFF,0xFF,0xFF));

return img2;
}
I dislike nesting statements unless necessary and I think "checking for failure and bailing immediately" makes more sense than "checking for success and continuing".

Hope this helps.

Share this post


Link to post
Share on other sites
Quote:
Original post by Mozly
Actually i did return img2; return img1 is a typo. it is display format that doesent work.
Still not sure what that means, but have you tried calling SDL_GetError after that line?

Share this post


Link to post
Share on other sites
Quote:
Original post by Mozly
the code now reads as you wrote it but SDL_DisplayFormat() is still not doing its job
You keep saying what it isn't doing('its job'), but I think it might help if you say what it is doing. That is, does it return NULL or non-NULL? Does it cause your program to crash? Something entirely different?

Also, did you try SDL_GetError or not? If SDL_DisplayFormat returns non-NULL then SDL_GetError probably won't be helpful.

Share this post


Link to post
Share on other sites
ok, sdl_displayFormat() is supposed to set all the values of the image such as img->rectclips etc. none of the values are set. That is what im saying. Sorry if it sounded scetchy

Share this post


Link to post
Share on other sites
Quote:
Original post by Mozly
ok, sdl_displayFormat() is supposed to set all the values of the image such as img->rectclips etc. none of the values are set. That is what im saying. Sorry if it sounded scetchy


What makes you think this?

SDL_DisplayFormat optimises the image data. It does not preserve other data.

Looking through the source* it doesn't touch the "rectclips" member. You'll need to write a wrapper function that handles this.

* SDL_DisplayFormat (line 919)which delegates to SDL_ConvertSurface (line 719).

Share this post


Link to post
Share on other sites
Oops, I missed the part where you are trying to set the color key. If I understand the tutorial correctly, it works because he is operating on BMP files. I'm not sure if loading another format via IMG_Load will result in colors stable enough to have them filtered through a color key.

--- --- ---

Here is my image loader. It has worked fine.


// Surfaces are always dealt with as pointers.
// This typedef is just for my convenience.
typedef SDL_Surface* Surface;

Surface loadImage(const char* inFile)
{
Surface t = IMG_Load(inFile);

if (!t) return NULL; // failed to load image

Surface outSurface = SDL_DisplayFormatAlpha(t);
SDL_FreeSurface(t); // dispose of temporary data

return outSurface;
}



Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this