From d86a5312783e721b19be026913d0b901ee2d3822 Mon Sep 17 00:00:00 2001
From: Simon McVittie <[EMAIL REDACTED]>
Date: Thu, 20 Jul 2023 14:20:33 +0100
Subject: [PATCH] Allocate video surface object statically as a global
The SDL Perl bindings incorrectly call SDL_FreeSurface() on the result
of functions that return a "borrowed" pointer to the video surface,
namely SDL_SetVideoMode() and SDL_GetVideoSurface().
(See https://github.com/PerlGameDev/SDL/issues/305)
When we would previously have allocated or freed the video surface
wrapper object, instead allocate or free its contents in-place.
When checking whether the video surface exists, because we never destroy
it, we must now also check whether its underlying SDL2 video surface
exists.
Resolves: https://github.com/libsdl-org/sdl12-compat/issues/305
Signed-off-by: Simon McVittie <smcv@debian.org>
---
src/SDL12_compat.c | 48 +++++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/src/SDL12_compat.c b/src/SDL12_compat.c
index 6b58078d4..0c1c4fb86 100644
--- a/src/SDL12_compat.c
+++ b/src/SDL12_compat.c
@@ -980,6 +980,7 @@ static SDL_Window *VideoWindow20 = NULL;
static SDL_Renderer *VideoRenderer20 = NULL;
static SDL_mutex *VideoRendererLock = NULL;
static SDL_Texture *VideoTexture20 = NULL;
+static SDL12_Surface VideoSurface12Location;
static SDL12_Surface *VideoSurface12 = NULL;
static SDL_Palette *VideoPhysicalPalette20 = NULL;
static Uint32 VideoSurfacePresentTicks = 0;
@@ -4714,7 +4715,7 @@ EventFilter20to12(void *data, SDL_Event *event20)
}
case SDL_MOUSEMOTION:
- if (!VideoSurface12) {
+ if (!VideoSurface12 || !VideoSurface12->surface20) {
return 1; /* we don't have a screen surface yet? Don't send this on to the app. */
}
@@ -5569,11 +5570,9 @@ EndVidModeCreate(void)
VideoPhysicalPalette20 = NULL;
}
if (VideoSurface12) {
- SDL12_Surface *screen12 = VideoSurface12;
SDL20_free(VideoSurface12->pixels);
VideoSurface12->pixels = NULL;
- VideoSurface12 = NULL; /* SDL_FreeSurface will ignore the screen surface, so NULL the global variable out. */
- SDL_FreeSurface(screen12);
+ FreeSurfaceContents(VideoSurface12);
}
if (VideoConvertSurface20) {
SDL20_FreeSurface(VideoConvertSurface20);
@@ -5607,16 +5606,27 @@ EndVidModeCreate(void)
return NULL;
}
-
-static SDL12_Surface *
-CreateSurface12WithFormat(const int w, const int h, const Uint32 fmt)
+/* Essentially the same as SDL_CreateRGBSurface, but in-place */
+static void
+CreateVideoSurface(const Uint32 fmt)
{
Uint32 rmask, gmask, bmask, amask;
int bpp;
+ SDL_Surface *surface20;
+
if (!SDL20_PixelFormatEnumToMasks(fmt, &bpp, &rmask, &gmask, &bmask, &amask)) {
- return NULL;
+ return;
}
- return SDL_CreateRGBSurface(0, w, h, bpp, rmask, gmask, bmask, amask);
+
+ SDL20_zerop(VideoSurface12);
+ surface20 = CreateRGBSurface(0, 0, 0, bpp, rmask, gmask, bmask, amask);
+
+ if (!Surface20to12InPlace(surface20, VideoSurface12)) {
+ FreeSurfaceContents(VideoSurface12);
+ return;
+ }
+
+ Surface12SetMasks(VideoSurface12, rmask, gmask, bmask, amask);
}
static SDL_Surface *
@@ -5953,6 +5963,8 @@ SetVideoModeImpl(int width, int height, int bpp, Uint32 flags12)
int scaled_height = height;
const char *fromwin_env = NULL;
+ VideoSurface12 = &VideoSurface12Location;
+
if (flags12 & SDL12_OPENGL) {
/* For now we default GL scaling to ENABLED. If an app breaks or is linked directly
to glBindFramebuffer, they'll need to turn it off with this environment variable.
@@ -6042,11 +6054,11 @@ SetVideoModeImpl(int width, int height, int bpp, Uint32 flags12)
default: SDL20_SetError("Unsupported bits-per-pixel"); return NULL;
}
- SDL_assert((VideoSurface12 != NULL) == (VideoWindow20 != NULL));
+ SDL_assert((VideoSurface12->surface20 != NULL) == (VideoWindow20 != NULL));
- if (VideoSurface12 && ((VideoSurface12->flags & SDL12_OPENGL) != (flags12 & SDL12_OPENGL))) {
+ if (VideoSurface12->surface20 && ((VideoSurface12->flags & SDL12_OPENGL) != (flags12 & SDL12_OPENGL))) {
EndVidModeCreate(); /* rebuild the window if moving to/from a GL context */
- } else if (VideoSurface12 && (VideoSurface12->surface20->format->format != appfmt)) {
+ } else if (VideoSurface12->surface20 && (VideoSurface12->surface20->format->format != appfmt)) {
EndVidModeCreate(); /* rebuild the window if changing pixel format */
} else if (VideoGLContext20) {
/* SDL 1.2 (infuriatingly!) destroys the GL context on each resize in some cases, on various platforms. Try to match that. */
@@ -6188,11 +6200,11 @@ SetVideoModeImpl(int width, int height, int bpp, Uint32 flags12)
SDL20_SetWindowResizable(VideoWindow20, (flags12 & SDL12_RESIZABLE) ? SDL_TRUE : SDL_FALSE);
}
- if (VideoSurface12) {
+ if (VideoSurface12->surface20) {
SDL20_free(VideoSurface12->pixels);
} else {
- VideoSurface12 = CreateSurface12WithFormat(0, 0, appfmt);
- if (!VideoSurface12) {
+ CreateVideoSurface(appfmt);
+ if (!VideoSurface12->surface20) {
return EndVidModeCreate();
}
}
@@ -6708,7 +6720,7 @@ DECLSPEC12 SDL12_Surface * SDLCALL
SDL_DisplayFormat(SDL12_Surface *surface12)
{
const Uint32 flags = surface12->flags & (SDL12_SRCCOLORKEY|SDL12_SRCALPHA|SDL12_RLEACCELOK);
- if (!VideoSurface12) {
+ if (!VideoSurface12 || !VideoSurface12->surface20) {
SDL20_SetError("No video mode has been set");
return NULL;
}
@@ -6724,7 +6736,7 @@ SDL_DisplayFormatAlpha(SDL12_Surface *surface12)
SDL_PixelFormat *fmt20 = NULL;
SDL12_PixelFormat fmt12;
- if (!VideoSurface12) {
+ if (!VideoSurface12 || !VideoSurface12->surface20) {
SDL20_SetError("No video mode has been set");
return NULL;
}
@@ -7309,7 +7321,7 @@ static void
HandleInputGrab(SDL12_GrabMode mode)
{
/* SDL 1.2 always grabbed input if the video mode was fullscreen. */
- const SDL_bool isfullscreen = (VideoSurface12 && (VideoSurface12->flags & SDL12_FULLSCREEN)) ? SDL_TRUE : SDL_FALSE;
+ const SDL_bool isfullscreen = (VideoSurface12 && VideoSurface12->surface20 && (VideoSurface12->flags & SDL12_FULLSCREEN)) ? SDL_TRUE : SDL_FALSE;
const SDL_bool wantgrab = (isfullscreen || (mode == SDL12_GRAB_ON)) ? SDL_TRUE : SDL_FALSE;
if (VideoWindowGrabbed != wantgrab) {
if (VideoWindow20) {