Skip to content

Commit

Permalink
render: SDL_DestroyWindow hollows out its renderer but doesn't free it.
Browse files Browse the repository at this point in the history
This allows apps to destroy the window and renderer in either order, but
makes sure that the renderer can properly clean up its resources while OpenGL
contexts and libraries are still loaded, etc.

If the window is destroyed first, the renderer is (mostly) destroyed but its
pointer remains valid. Attempts to use the renderer will return an error,
but it can still be explicitly destroyed, at which time the struct is free'd.

If the renderer is destroyed first, everything works as before, and a new
renderer can still be created on the existing window.

Fixes libsdl-org#9540.
  • Loading branch information
icculus committed Apr 18, 2024
1 parent a9813f0 commit 657c317
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 7 deletions.
32 changes: 26 additions & 6 deletions src/render/SDL_render.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,19 @@ this should probably be removed at some point in the future. --ryan. */
#define SDL_PROP_WINDOW_RENDERER_POINTER "SDL.internal.window.renderer"
#define SDL_PROP_TEXTURE_PARENT_POINTER "SDL.internal.texture.parent"

#define CHECK_RENDERER_MAGIC(renderer, retval) \
#define CHECK_RENDERER_MAGIC_BUT_NOT_DESTROYED_FLAG(renderer, retval) \
if (!(renderer) || (renderer)->magic != &SDL_renderer_magic) { \
SDL_InvalidParamError("renderer"); \
return retval; \
}

#define CHECK_RENDERER_MAGIC(renderer, retval) \
CHECK_RENDERER_MAGIC_BUT_NOT_DESTROYED_FLAG(renderer, retval); \
if ((renderer)->destroyed) { \
SDL_SetError("Renderer's window has been destroyed, can't use further"); \
return retval; \
}

#define CHECK_TEXTURE_MAGIC(texture, retval) \
if (!(texture) || (texture)->magic != &SDL_texture_magic) { \
SDL_InvalidParamError("texture"); \
Expand Down Expand Up @@ -4517,9 +4524,12 @@ static void SDL_DiscardAllCommands(SDL_Renderer *renderer)
}
}

void SDL_DestroyRenderer(SDL_Renderer *renderer)
void SDL_DestroyRendererWithoutFreeing(SDL_Renderer *renderer)
{
CHECK_RENDERER_MAGIC(renderer,);
SDL_assert(renderer != NULL);
SDL_assert(!renderer->destroyed);

renderer->destroyed = SDL_TRUE;

SDL_DestroyProperties(renderer->props);

Expand All @@ -4540,15 +4550,25 @@ void SDL_DestroyRenderer(SDL_Renderer *renderer)
SDL_ClearProperty(SDL_GetWindowProperties(renderer->window), SDL_PROP_WINDOW_RENDERER_POINTER);
}

/* It's no longer magical... */
renderer->magic = NULL;

/* Free the target mutex */
SDL_DestroyMutex(renderer->target_mutex);
renderer->target_mutex = NULL;

/* Clean up renderer-specific resources */
renderer->DestroyRenderer(renderer);
}

void SDL_DestroyRenderer(SDL_Renderer *renderer)
{
CHECK_RENDERER_MAGIC_BUT_NOT_DESTROYED_FLAG(renderer,);

// if we've already destroyed the renderer through SDL_DestroyWindow, we just need
// to free the renderer pointer. This lets apps destroy the window and renderer
// in either order.
if (!renderer->destroyed) {
SDL_DestroyRendererWithoutFreeing(renderer);
renderer->magic = NULL; // It's no longer magical...
}

SDL_free(renderer);
}
Expand Down
5 changes: 5 additions & 0 deletions src/render/SDL_sysrender.h
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,8 @@ struct SDL_Renderer

SDL_PropertiesID props;

SDL_bool destroyed; // already destroyed by SDL_DestroyWindow; just free this struct in SDL_DestroyRenderer.

void *driverdata;
};

Expand Down Expand Up @@ -335,6 +337,9 @@ extern SDL_BlendOperation SDL_GetBlendModeAlphaOperation(SDL_BlendMode blendMode
the next call, because it might be in an array that gets realloc()'d. */
extern void *SDL_AllocateRenderVertices(SDL_Renderer *renderer, const size_t numbytes, const size_t alignment, size_t *offset);

// Let the video subsystem destroy a renderer without making its pointer invalid.
extern void SDL_DestroyRendererWithoutFreeing(SDL_Renderer *renderer);

/* Ends C function definitions when using C++ */
#ifdef __cplusplus
}
Expand Down
3 changes: 2 additions & 1 deletion src/video/SDL_video.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "../SDL_properties_c.h"
#include "../timer/SDL_timer_c.h"
#include "../camera/SDL_camera_c.h"
#include "../render/SDL_sysrender.h"

#ifdef SDL_VIDEO_OPENGL
#include <SDL3/SDL_opengl.h>
Expand Down Expand Up @@ -3649,7 +3650,7 @@ void SDL_DestroyWindow(SDL_Window *window)

SDL_Renderer *renderer = SDL_GetRenderer(window);
if (renderer) {
SDL_DestroyRenderer(renderer);
SDL_DestroyRendererWithoutFreeing(renderer);
}

SDL_DestroyProperties(window->props);
Expand Down

0 comments on commit 657c317

Please sign in to comment.