Basic Sdl Help Needed

[Awakening]

I am new to C++ and SDL, have read through most of Lazy Foo's tutorials. I'm coding a little puzzle game for Pandora as a learning project (I have quite some programming experience). I've run into a problem pretty early but perhaps someone here can help me out.

I have 3 surfaces: image, mask and composed

image and mask are loaded from existing images and I have no problem blitting them to the screen with full transparency. I can also blit the mask on top of the image and blit the result to the screen, no problems there.

You might have guessed what my problem is by now. I want to blit the image and mask surfaces to the composed surface but I apparently need to do something to the composed surface to make this work. My guess is that it got something to do with the clipping rectangle of the surface but so far I got no luck setting it. Or am I missing something else?

This might not be important knowledge at this point but I figure it would be a good thing to know.

Please help me

 

[mindlord]

SDL_SetClipRect

Also, make sure that the Compose surface does not have an alpha channel, or if it does make sure that the entire surface's alpha channel is SDL_ALPHA_OPAQUE ( 255 ). SDL does not do alpha blending in the way you would expect. SDL uses the destination surface's alpha, and discards the source surface alpha. There are software ways to get alpha blending, but they are very slow.

 

[Awakening]

I've tried using that, but maybe I'm doing something wrong.

I got this at the top of my code:
SDL_Surface *frontPanel = NULL;


Then inside one of my functions I got this:
SDL_Rect panel;
panel.x = SCREEN_WIDTH;
panel.y = SCREEN_HEIGHT;
SDL_SetClipRect(frontPanel, &panel);


And finally in my main function I got this:
apply_surface( 0, 0, frontImage, frontPanel);
apply_surface( 0, 0, frontMask, frontPanel);
apply_surface( 0, 0, frontPanel, screen);

 

[mindlord]

Try:
SDL_Surface *frontpanel;

without the NULL. You could also try using SDL_CreateRGBSurface to create a surface with features that match the screen.

 

[PokeParadox]

I think it's just that you don't have a valid surface, so you need to create a surface as mentioned above.

 

[Awakening]

Deleting NULL didn't do it but the create function did it.

frontPanel = SDL_CreateRGBSurface( SDL_SWSURFACE, SCREEN_WIDTH, SCREEN_HEIGHT, 32, 0, 0, 0, 0 );

It's a bit long, gonna make my own shorter version. I hope doing all zeros at the end will work with the Pandora as well.

Thanks for the help, now I think I got all the image blitting basics covered

 

[PokeParadox]

Make sure you SDL_FreeSurface(SDL_Surface* surf); when finished with surfaces too otherwise you'll get memleaks...

 

[Awakening]

Yeah, that's covered by the tutorial

But I just got thinking. I use this function to load images:

SDL_Surface *load_image( std::string filename )
{
    SDL_Surface *loadedImage = NULL;
    SDL_Surface *optimizedImage = NULL;

    loadedImage = IMG_Load( filename.c_str() );

    if( loadedImage != NULL ) {
        optimizedImage = SDL_DisplayFormatAlpha( loadedImage );

        SDL_FreeSurface( loadedImage );
    }

    return optimizedImage;
}

But what happens when I load another image to the same surface? Does it overwrite the existing one or do I end up with multiple surfaces in memory?

For example when I change the frontImage surface (by using that function above twice) should I free the surface before loading another image?

 

[mindlord]

But what happens when I load another image to the same surface? Does it overwrite the existing one or do I end up with multiple surfaces in memory?

For example when I change the frontImage surface (by using that function above twice) should I free the surface before loading another image?

You should pass the surface in by reference, that way you can safely destroy it from the calling function. The way you have it now, you've created a possibility for a leak, just like you think. You should also be destroying loadedImage regardless of whether or not the load succeeded, put the SDL_FreeSurface outside the if.

SDL_Surface *load_image(SDL_Surface &optimizedImage, std::string filename )
{
    SDL_Surface *loadedImage = NULL;

    loadedImage = IMG_Load( filename.c_str() );

    if( loadedImage != NULL ) {
        optimizedImage = SDL_DisplayFormatAlpha( loadedImage );
    }
    SDL_FreeSurface( loadedImage );
    return optimizedImage;
}

Now your function doesn't know or care where the original surface came from or how it gets is supposed to be destroyed, just like nature intended.

 

[Awakening]

I just split up my code into separate source and header files and my head is tired, I'm not used to header files and declaring stuff. I thought I might as well get used to that now when the code is tiny rather then later. I find it easier to work on the main loop when the file is less cluttered.

Let me get back to you on that tomorrow if I have the time, when my head is fresh again.

 

[Awakening]

I don't get why I should pass optimizedImage to the function. Correct me if I'm wrong but since it is a pointer that gets returned to a different pointer, from what I understand it, I should not have to care about optimizedImage.

Shouldn't this work great with the function I got?

if( frontImage != NULL) SDL_FreeSurface( frontImage );
frontImage = load_image( "Fronts/Brushed Metal.jpg" );

 

[mindlord]

I don't get why I should pass optimizedImage to the function. Correct me if I'm wrong but since it is a pointer that gets returned to a different pointer, from what I understand it, I should not have to care about optimizedImage.

Shouldn't this work great with the function I got?

if( frontImage != NULL) SDL_FreeSurface( frontImage );
frontImage = load_image( "Fronts/Brushed Metal.jpg" );

It is one possible solution to the question you asked. It saves cycles if your loadimage function doesn't have to check if the surface is NULL every time you use it. By just passing a reference you alleviate loadImage's need to know, and save a few cycles. Now that I look at it, you actually don't need to return anything, in fact you can return a success/failure instead. As an added bonus you aren't passing huge amounts of surface data back from the function, just a bool.

bool load_image(SDL_Surface *optimizedImage, std::string filename )
{
    SDL_Surface *loadedImage = NULL;

    loadedImage = IMG_Load( filename.c_str() );

    if( loadedImage != NULL ) {
        optimizedImage = SDL_DisplayFormatAlpha( loadedImage );
        SDL_FreeSurface( loadedImage );
        return 1; // Success
    }
    SDL_FreeSurface( loadedImage );
    return 0; // Exception
}

Can be used like so:

SDL_Surface *frontImage;
if( !load_image( &frontImage, "Fronts/Brushed Metal.jpg" )) { handle_load_failure; }

With this scenario - if frontImage is being recycled it's contents are just replaced with the newly loaded image. If the load fails the contents aren't changed, so at least it's not NULL, your program won't crash if you try to use the surface, and you still have the ability of handling the load failure gracefully.

 

[senquack]

It is one possible solution to the question you asked. It saves cycles if your loadimage function doesn't have to check if the surface is NULL every time you use it. By just passing a reference you alleviate loadImage's need to know, and save a few cycles. Now that I look at it, you actually don't need to return anything, in fact you can return a success/failure instead. As an added bonus you aren't passing huge amounts of surface data back from the function, just a bool.

Why are you worried about saving cycles in a function that should really only be used once each time your game initializes or a specific level is loaded? Here, you are worried about saving a few cycles (literally less than 10 cycles) in a function that probably spends tens of thousands loading a jpg image and making OS system calls.

Maybe *maybe* you can worry about this in your blitting function but even there you'd probably not notice the speedup from removing one comparison to NULL. You should really only worry about this stuff if you have it in a deeply nested loop.

 

[mindlord]

Well, saving cycles is saving cycles. But more importantly it's a strategy that can be adopted for many other functions that can squeeze some speed out overall. Besides, it's a good idea to foster cycle saving programming practice early on. It's easier to accept that large structures like SDL_Surface should ONLY be passed by reference whenever possible. It's just a good idea.

 

[Dimacus]

It is one possible solution to the question you asked. It saves cycles if your loadimage function doesn't have to check if the surface is NULL every time you use it. By just passing a reference you alleviate loadImage's need to know, and save a few cycles. Now that I look at it, you actually don't need to return anything, in fact you can return a success/failure instead. As an added bonus you aren't passing huge amounts of surface data back from the function, just a bool.

He didn't pass a huge amount of surface data before either, he just returned a pointer.

There is also the danger of a memory leak in your example, since the load function doesn't check if the surface optimizedImage earlready contains surface data.
Perhaps it's taste dependent, but from what I have heard; the functions are responsible for what is sent into them, and the caller is responsible for what is returned.

 

[Pickle]

Im also not seeing the benefits to passing a reference/pointer in this case. I agree passing by reference has it merits, but not in this case.
I typically use it when a function needs to access an class/struct.

 

[Awakening]

Most of the time I'm only loading images once, this is only to alter the look of the front panel (which is user configurable) and that might not even happen or if it does not during gameplay. So most of the time I just need a basic image loading function.

Thanks for all the replies, this turned out to be an interesting little discussion

One more question came up. What happens in mindlord's case when optimizedImage get a new image without being freed first?

 

[Dimacus]

It won't do anything to the original surface, but it will create a new surface (optimizedImage) which is never freed and therefor creates a memory leak.

If however the function parameters are altered from
(First case)

bool load_image(SDL_Surface *optimizedImage, std::string filename )

to
(Second case)

bool load_image(SDL_Surface *&optimizedImage, std::string filename )

then the original surface will still exist in memory but the only reference to it will be lost, and replaced with optimizedImage.


This happens because (in the first case) you send in a copy of the pointer, with the same value but located in a different place.
Thus if you change the value of pointer in that function, you only changed the value of the copy, not of the original.

In the second case, you change the value of original pointer.


The outcome of both is undesirable, but if you free the surface in the second case (first check if it is loaded), it will do what you want it to do (without any mem leaks).

 

[Awakening]

Thanks again