From 1413d6774806cca9e9f17552684f47791d7c9af3 Mon Sep 17 00:00:00 2001 From: "Ryan C. Gordon" Date: Thu, 18 Apr 2024 10:20:31 -0400 Subject: [PATCH] render: SDL_DestroyWindow hollows out its renderer but doesn't free it. 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 #10174. (cherry picked from commit cab3defc1826b299f55712beb6d6303186ddd8f0) --- src/render/SDL_render.c | 35 +++++++++++++++++++++++++++++------ src/render/SDL_sysrender.h | 5 +++++ src/video/SDL_video.c | 8 ++++++++ 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c index 643fdd59af..5afb5562c7 100644 --- a/src/render/SDL_render.c +++ b/src/render/SDL_render.c @@ -47,12 +47,19 @@ this should probably be removed at some point in the future. --ryan. */ #define SDL_WINDOWRENDERDATA "_SDL_WindowRenderData" -#define CHECK_RENDERER_MAGIC(renderer, retval) \ +#define CHECK_RENDERER_MAGIC_BUT_NOT_DESTROYED_FLAG(renderer, retval) \ if (!renderer || renderer->magic != &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 != &texture_magic) { \ SDL_InvalidParamError("texture"); \ @@ -4345,11 +4352,14 @@ void SDL_DestroyTexture(SDL_Texture *texture) SDL_free(texture); } -void SDL_DestroyRenderer(SDL_Renderer *renderer) +void SDL_DestroyRendererWithoutFreeing(SDL_Renderer *renderer) { SDL_RenderCommand *cmd; - CHECK_RENDERER_MAGIC(renderer, ); + SDL_assert(renderer != NULL); + SDL_assert(!renderer->destroyed); + + renderer->destroyed = SDL_TRUE; SDL_DelEventWatch(SDL_RendererEventWatch, renderer); @@ -4382,20 +4392,33 @@ void SDL_DestroyRenderer(SDL_Renderer *renderer) if (renderer->window) { SDL_SetWindowData(renderer->window, SDL_WINDOWRENDERDATA, NULL); + renderer->window = NULL; } - /* It's no longer magical... */ - renderer->magic = NULL; - /* Free the target mutex */ SDL_DestroyMutex(renderer->target_mutex); renderer->target_mutex = NULL; /* Free the renderer instance */ 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); } + int SDL_GL_BindTexture(SDL_Texture *texture, float *texw, float *texh) { SDL_Renderer *renderer; diff --git a/src/render/SDL_sysrender.h b/src/render/SDL_sysrender.h index e367b9cdb6..9fdc385f65 100644 --- a/src/render/SDL_sysrender.h +++ b/src/render/SDL_sysrender.h @@ -282,6 +282,8 @@ struct SDL_Renderer size_t vertex_data_used; size_t vertex_data_allocation; + SDL_bool destroyed; /* already destroyed by SDL_DestroyWindow; just free this struct in SDL_DestroyRenderer. */ + void *driverdata; }; @@ -324,6 +326,9 @@ extern void *SDL_AllocateRenderVertices(SDL_Renderer *renderer, const size_t num extern int SDL_PrivateLowerBlitScaled(SDL_Surface *src, SDL_Rect *srcrect, SDL_Surface *dst, SDL_Rect *dstrect, SDL_ScaleMode scaleMode); extern int SDL_PrivateUpperBlitScaled(SDL_Surface *src, const SDL_Rect *srcrect, SDL_Surface *dst, SDL_Rect *dstrect, SDL_ScaleMode scaleMode); +/* 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 } diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c index 7f93beb8ea..cc0e473b68 100644 --- a/src/video/SDL_video.c +++ b/src/video/SDL_video.c @@ -34,6 +34,8 @@ #include "SDL_syswm.h" +#include "../render/SDL_sysrender.h" + #ifdef SDL_VIDEO_OPENGL #include "SDL_opengl.h" #endif /* SDL_VIDEO_OPENGL */ @@ -3303,6 +3305,7 @@ SDL_Window *SDL_GetFocusWindow(void) void SDL_DestroyWindow(SDL_Window *window) { SDL_VideoDisplay *display; + SDL_Renderer *renderer; CHECK_WINDOW_MAGIC(window, ); @@ -3321,6 +3324,11 @@ void SDL_DestroyWindow(SDL_Window *window) SDL_SetMouseFocus(NULL); } + renderer = SDL_GetRenderer(window); + if (renderer) { + SDL_DestroyRendererWithoutFreeing(renderer); + } + SDL_DestroyWindowSurface(window); /* Make no context current if this is the current context window */