Browse Source

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)
Ryan C. Gordon 1 year ago
parent
commit
1413d67748
3 changed files with 42 additions and 6 deletions
  1. 29 6
      src/render/SDL_render.c
  2. 5 0
      src/render/SDL_sysrender.h
  3. 8 0
      src/video/SDL_video.c

+ 29 - 6
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;

+ 5 - 0
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
 }

+ 8 - 0
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 */