No, I would not make the window or surface global variables. There doesn’t need to be a function to free the surface, either; that seems excessive.
As for structs: if you declare one as a local variable then no, it isn’t going to be a memory leak. You don’t need to make it a typedef either.
I see a number of examples / tutorials in which SDL_Surface and SDL_Window are global. I am not fond of globals, but it seems (please correct me if I am wrong) there is something to it.
First of all, there is nothing wrong with using globals, especially for things that are central to your application and which you only have one (or very few) of and that you’re unlikely to need more of. Like the Window and maybe the surface, if you only have one or two surfaces anyway.
If you want things to look a bit cleaner, but the global stuff that belongs together in a struct and make that struct global, so you could use something like windowState.sdlWindow instead of just sdlWindow in your code. Reduces the number of globals…
If you’re using an object oriented language like C++, you could use a class instead of a struct and call it WindowManager or something like that and tell people who complain about globals that it’s a Singleton so it’s ok…
Yes, but that has nothing to do with SDL, but it’s just how pointers (in C) work.
The proper signature of that function is void func_free_the_surface( SDL_Surface* Surf ).
So if you call func_free_the_surface( mySurface ), what’s happening is that the pointer mySurface gets copied into Surf, because C always copies the function arguments.
So if you modify the pointer Surf in func_free_the_surface() (like you do with Surf = NULL;) it only affects this copy of mySurface (i.e. Surf), not mySurface itself.
On the other hand, if you modify the memory Surf points to - like you do with SDL_FreeSurface(Surf); - that of course also modifies the memory mySurface points to, because it’s the same memory (unless you make Surf point to something else first).
If you want a function to modify a pointer itself (and not just the memory it points to), you need to use a pointer to a pointer as function argument.
Like:
int main()
{
// ...
SDL_Surface* mySurface = whatever();
// using get address operator (&) on mySurface
// to get the pointer to this pointer
func_free_the_surface( &mySurface );
// ...
return 0;
}
void func_free_the_surface( SDL_Surface** SurfPtr )
{
// SurfPtr is &mySurface (of type SDL_Surface**)
// i.e. pointer to SDL_Surface* , i.e. pointer to pointer to SDL_Surface
if(SurfPtr != NULL) { // make sure SurfPtr is not NULL (optional)
// *SurfPtr is mySurface (of type SDL_Surface*)
SDL_FreeSurface( *SurfPtr );
*SurfPtr = NULL; // this sets mySurface in main() to NULL
}
}
The question: Is it advisable to maintain globally declared Windows and Surfaces if you are going to be using them for multiple surface creations?
If you only ever have one surface at a time, you can put it in a global. If you can have multiple at a time (esp. if you don’t know how many) I wouldn’t put it in a global, at least not directly - but if you have a global struct WindowState or whatever that you also put your SDL_Window* in, then I’d argue that putting the surfaces that you’re using with the window in there as well is fine.
And if you don’t wanna use a global you need to pass all that data around somehow, so a struct like that is still useful to bundle data that belongs together (so for example you only need to pass that struct to your rendering function instead of the window and the surfaces and whatever other global rendering state you have in multiple function arguments)
Enlightenment. Thanks both @sjr and @Daniel_Gibson !
I was pleased that I understood both replies first read (which must mean some progress).
You made me think, here. I do not know why I was under the impression that you always have to start creating surfaces with a clean slate (that is, a NULL and free SDL_Surface*). Feeling a bit like a fugitive from the preconceptions of “rules”, but armed with the new found skill of throwing memory around like pies in a 30’s flick, I dared to put this together to check…
#include "SDL.h"
struct bmpfiledata
{
int w;
int h;
int bits;
int pitch;
Uint32 format;
};
int main(int argc, char* argv[])
{
SDL_Surface* localSurface = NULL;
SDL_Window* localWindow = NULL;
SDL_Surface* localWindowSurface = NULL;
struct bmpfiledata filedata;
Uint8* pixeldata = NULL;
SDL_Init(SDL_INIT_VIDEO);
localWindow = SDL_CreateWindow("Test", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED,
800, 700, SDL_WINDOW_SHOWN);
localWindowSurface = SDL_GetWindowSurface(localWindow);
localSurface = SDL_LoadBMP_RW(SDL_RWFromFile("MC_01.bmp", "rb"), 1);
// Store the first image in memory for later test.
filedata.w = localSurface->w;
filedata.h = localSurface->h;
filedata.bits = (int)localSurface->format->BitsPerPixel;
filedata.pitch = localSurface->pitch;
filedata.format = localSurface->format->format;
size_t imagedatasize = (size_t)(filedata.w * filedata.h);
pixeldata = SDL_calloc(imagedatasize, sizeof(Uint32));
SDL_memcpy(pixeldata, localSurface->pixels, (imagedatasize * 4));
SDL_SetWindowSize(localWindow, localSurface->w, localSurface->h);
localWindowSurface = SDL_GetWindowSurface(localWindow);
SDL_BlitSurface(localSurface, NULL, localWindowSurface, NULL);
SDL_UpdateWindowSurface(localWindow);
SDL_Delay(2000);
localSurface = SDL_LoadBMP_RW(SDL_RWFromFile("MC_02.bmp", "rb"), 1);
SDL_SetWindowSize(localWindow, localSurface->w, localSurface->h);
localWindowSurface = SDL_GetWindowSurface(localWindow);
SDL_BlitSurface(localSurface, NULL, localWindowSurface, NULL);
SDL_UpdateWindowSurface(localWindow);
SDL_Delay(2000);
localSurface = SDL_CreateRGBSurfaceWithFormatFrom(
pixeldata, filedata.w, filedata.h, filedata.bits, filedata.pitch, filedata.format);
SDL_SetWindowSize(localWindow, localSurface->w, localSurface->h);
localWindowSurface = SDL_GetWindowSurface(localWindow);
SDL_BlitSurface(localSurface, NULL, localWindowSurface, NULL);
SDL_UpdateWindowSurface(localWindow);
SDL_Delay(2000);
SDL_Quit();
SDL_FreeSurface(localSurface);
SDL_FreeSurface(localWindowSurface);
SDL_DestroyWindow(localWindow);
SDL_free(pixeldata);
SDL_Quit();
return 0;
}A couple of things were clarified. Those two images that are loaded up there were deliberately different sizes, in order that I was effectively challenging the creation of the surface with its dimensions. I was blown to bits that it works, so I conclude that at least creating a surface by loading an image from disk or from memory (with an SDL_CreateRGB variant), the surface is prepared for the new image by the respective function. The other point was that the same applies to the window surface itself, when the window is resized. The call to SDL_GetWindowSurface() is necessary, yes, but recreating the window surface from scratch is not. This is all good to know.
This made sense, seeing it here so. I had to kick myself.
Regarding global variables:
This would certainly seem the way to go. I am wary of them because they have caused me trouble in the distant past, in a different language (Blitz on the Amiga 500, I think). The selection of what to actually put as a global is the critical bit. For example, I remember giving myself a huge debug headache when I decided that PLAYERSCORE was a good, central variable to put as a global. It was not. Somewhere, as the program progressed, it was being reset, and finding where was a nightmare, in those days when debugging was largely manual. It is a classic case of what not to represent as a global.
Thus, I shy away from putting something as easily and accidentally fiddled as a Surface to a global variable. But now, I realize that they are somewhat more impervious to abuse than I initially thought. I am happy to go either way with this, now, as each case may dictate.
Many, many thanks!
And a Happy 2022, all.