Simple question:
Am I creating a memory leak if I do this:
aTexture = SDL_CreateTextureFromSurface(renderer, SDL_LoadBMP(file path))
I can free the texture by aTexture but I can’t free the surface internally created, right?
EDIT: A better, clearer title was not accepted by discourse with message “Title seems unclear, is it a complete sentence?” - Don’t know what this means…
No, the code frees the data for you. You can see it on line 887ish? of src\render\SDL_render.c.
No, the code frees the data for you. You can see it on line 887ish? of
src\render\SDL_render.c.
Are you sure? Line 887 is some random line in SDL_UpdateTextureNative(). But I guess 887ish means somewhere there.
If you are refering to SDL_Texture *
SDL_CreateTextureFromSurface(SDL_Renderer * renderer, SDL_Surface * surface), it starts at line 590 (2.0.8), but I do not see how the surface is free’d anywhere. Maybe you could indicate this?
Thanks in advance.
I don’t think this function frees the SDL_Surface.
Follow the path of the SDL_CreateTextureFromSurface function in SDL_render.c.
The only FreeSurface I can see is to a surface that the function created itself and not the passed parameter surface.
The documentation of SDL_CreateTextureFromSurface() is quite explicit: “The surface is not modified or freed by this function”.
Oooo, ouch, I was wrong again… I saw this line: SDL_FreeSurface(temp); in the SDL source and made a poor assumption. That surface is used to convert the surface format to the texture’s format. I might have been thinking about IMG_Load where it opens a file, and then frees the file after creating the surface.
Sorry about that. 
Sorry about that.
Never mind. 
Well, to came back to my original question, so the combination of SDL_CreateTextureFromSurface(renderer, SDL_LoadBMP()) will lead to a memory leak, or are there other ways to not have to declare a surface?
Whats wrong with declaring a surface ?
Is there something your trying to achieve ?
I mean its only 2 extra lines of code, one to declare and one to release.
SDL_Texture* loadTexture(SDL_Renderer* renderer, const char* filePath) {
SDL_Surface* surface(SDL_LoadBMP(filePath));
if (surface) {
SDL_Texture* texture = SDL_CreateTextureFromSurface(renderer, surface);
SDL_FreeSurface(surface);
return texture;
}
return NULL;
}
If you use smart pointers and c++11 you could get around having to type the extra lines.
But the unique pointer would be still doing the delete.
Whats wrong with declaring a surface ?
Is there something your trying to achieve ?
I mean its only 2 extra lines of code, one to declare and one to release.
Well, its ONLY 2 extra lines, or its ADDITIONAL 2 extra lines. In complex code it may happen that you forget to free the surface, or what not. It would be desirable to not even care about it. But it seems to be necessary here.
If you use smart pointers and c++11 you could get around having to type the extra lines.
But the unique pointer would be still doing the delete.
Unfortunately this is not possible for me.
By the way, also here the surface has to be free’d manually (unfortunately).
So the reason is that you may forget to free the surface ?
Well the solution is to write a tried and tested function similar to the one I wrote above that loads a texture for you and releases the surface. All I have to do is call that function and your 100% guaranteed you won’t leak.
Well, thanks for your reply and effort.
I guess your missing my point, though. Your solution is fine under these circumstances, although it introduces new complexity. If it would be able to load the file with a one-liner (instead of a whole function), this would be preferable (at least, to my mind).
Why I’m asking this in the first place is, because I look for the most efficient and best practice solution to (especially simple) tasks like this in SDL2. If I didn’t miss anything here, it’s fine to me and my question can be considered solved 
.
The file is created and free’d automatically. I think what you want is a modified version of SDL that will free the surface after you finish. That’s feasible, and it’s what I’m doing, which is why I posted line 887 earlier. Really, that’s what Smiles was suggesting, and the function he posted would probably work as desired.
Is that not what all functions do? Turn a lot of lines into a single one liner?
Thanks for your effort, Blerg.
The solution presented is the best regarding the circumstances. It would still be preferable (and probably best practice) to have a one-liner without having to invent a new function or modifying the sdl source, if that would be possible. That’s my point. But it is not as I know now. That’s fine for me, but that is what my initial question was about.
Pro-Tip: Both the “proper” SDL_Image and my more minimal (but with very
similar API) SDL_stbimage.h have functions that load image files in
various formats and directly return a SDL_Texture, like
aTexture = IMG_LoadTexture(renderer, "/path/to/file.bmp");
(or STBIMG_LoadTexture(…) in the case of SDL_stbimage.h)
SDL_stbimage.h can be found at
https://github.com/DanielGibson/Snippets/blob/master/SDL_stbimage.h and
is very easy to integrate (see the big comment at beginning of the file).
It supports reading all image formats supported by stb_image.h, which
includes BMP, JPG, PNG and TGA.
Cheers,
Daniel
I can highly recommend SDL_stbimage, having recently started using it. But it’s worth noting that those functions which return a texture can only be called from the GUI thread. If you’re trying to farm out the image decoding to multiple threads you have to use the functions which return a surface and then call SDL_CreateTextureFromSurface in your main thread.
If I may make a request for a future release of SDL_stbimage it’s that it exposes the stbi_set_flip_vertically_on_load functionality (either directly or via a ‘flip’ flag) because I need bottom-up surfaces for OpenGL. Currently I’ve edited stb_image.h to export it.
I’m glad you like it!
SDL_stbimage.h does not set STB_IMAGE_STATIC so most stbi_*() functions should be exported [*] and you should be able to just call stbi_set_flip_vertically_on_load(1); directly without modifying anything.
[*] except for the ones using stdio, i.e. the ones taking a filename or a FILE* as an argument. except if you #define SDL_STBIMG_ALLOW_STDIO before #define SDL_STBIMAGE_IMPLEMENTATION and #include "SDL_stbimage.h"
Sorry, I wasn’t sufficiently clear. I need to export it so that I can get its address at run-time (e.g. using dlsym). I can do that with the functions in SDL_stbimage.h by using SDL_STBIMG_DEF:
#define SDL_STBIMG_DEF __attribute__ ((visibility ("default")))
but I don’t get the same opportunity with stbi_set_flip_vertically_on_load() because it’s governed by STBIDEF rather than SDL_STBIMG_DEF. I know I could create my own wrapper function, but it would be nice if it was included in SDL_stbimage.h as STBIMG_SetFlip or something, if only to draw attention to the existence of this very useful functionality.
Oh right, I somehow assumed that one can freely #define STBIDEF, but apparently it’s set to either static or extern depending on STB_IMAGE_STATIC - maybe this used to be different in the past, no idea… whatever.
Yeah, I guess I can provide a wrapper for that function.