Skip to content

Commit

Permalink
Clean up any renderer in SDL_DestroyWindowSurface()
Browse files Browse the repository at this point in the history 
Also added an automated test to verify window surface functionality.

Fixes libsdl-org#10133
  • Loading branch information
@slouken
slouken committed Jun 29, 2024
1 parent e47179c commit 9eecc2e
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 49 deletions.
92 changes: 48 additions & 44 deletions src/video/SDL_video.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2442,12 +2442,6 @@ int SDL_RecreateWindow(SDL_Window *window, SDL_WindowFlags flags)
/* Tear down the old native window */
SDL_DestroyWindowSurface(window);

if (_this->checked_texture_framebuffer) { /* never checked? No framebuffer to destroy. Don't risk calling the wrong implementation. */
if (_this->DestroyWindowFramebuffer) {
_this->DestroyWindowFramebuffer(_this, window);
}
}

if ((window->flags & SDL_WINDOW_OPENGL) != (flags & SDL_WINDOW_OPENGL)) {
if (flags & SDL_WINDOW_OPENGL) {
need_gl_load = SDL_TRUE;
Expand Down Expand Up @@ -3251,39 +3245,43 @@ static SDL_Surface *SDL_CreateWindowFramebuffer(SDL_Window *window)
using a GPU texture through the 2D render API, if we think this would
be more efficient. This only checks once, on demand. */
if (!_this->checked_texture_framebuffer) {
SDL_bool attempt_texture_framebuffer = SDL_TRUE;
SDL_bool attempt_texture_framebuffer;

/* See if the user or application wants to specifically disable the framebuffer */
const char *hint = SDL_GetHint(SDL_HINT_FRAMEBUFFER_ACCELERATION);
if (hint) {
if (hint && *hint) {
if ((*hint == '0') || (SDL_strcasecmp(hint, "false") == 0) || (SDL_strcasecmp(hint, SDL_SOFTWARE_RENDERER) == 0)) {
attempt_texture_framebuffer = SDL_FALSE;
} else {
attempt_texture_framebuffer = SDL_TRUE;
}
}
} else {
attempt_texture_framebuffer = SDL_TRUE;

if (_this->is_dummy) { /* dummy driver never has GPU support, of course. */
attempt_texture_framebuffer = SDL_FALSE;
}
if (_this->is_dummy) { /* dummy driver never has GPU support, of course. */
attempt_texture_framebuffer = SDL_FALSE;
}

#ifdef SDL_PLATFORM_LINUX
/* On WSL, direct X11 is faster than using OpenGL for window framebuffers, so try to detect WSL and avoid texture framebuffer. */
else if ((_this->CreateWindowFramebuffer) && (SDL_strcmp(_this->name, "x11") == 0)) {
struct stat sb;
if ((stat("/proc/sys/fs/binfmt_misc/WSLInterop", &sb) == 0) || (stat("/run/WSL", &sb) == 0)) { /* if either of these exist, we're on WSL. */
attempt_texture_framebuffer = SDL_FALSE;
/* On WSL, direct X11 is faster than using OpenGL for window framebuffers, so try to detect WSL and avoid texture framebuffer. */
else if ((_this->CreateWindowFramebuffer) && (SDL_strcmp(_this->name, "x11") == 0)) {
struct stat sb;
if ((stat("/proc/sys/fs/binfmt_misc/WSLInterop", &sb) == 0) || (stat("/run/WSL", &sb) == 0)) { /* if either of these exist, we're on WSL. */
attempt_texture_framebuffer = SDL_FALSE;
}
}
}
#endif
#if defined(SDL_PLATFORM_WIN32) || defined(SDL_PLATFORM_WINGDK) /* GDI BitBlt() is way faster than Direct3D dynamic textures right now. (!!! FIXME: is this still true?) */
else if ((_this->CreateWindowFramebuffer) && (SDL_strcmp(_this->name, "windows") == 0)) {
attempt_texture_framebuffer = SDL_FALSE;
}
else if ((_this->CreateWindowFramebuffer) && (SDL_strcmp(_this->name, "windows") == 0)) {
attempt_texture_framebuffer = SDL_FALSE;
}
#endif
#ifdef SDL_PLATFORM_EMSCRIPTEN
else {
attempt_texture_framebuffer = SDL_FALSE;
}
else {
attempt_texture_framebuffer = SDL_FALSE;
}
#endif
}

if (attempt_texture_framebuffer) {
if (SDL_CreateWindowTexture(_this, window, &format, &pixels, &pitch) < 0) {
Expand Down Expand Up @@ -3319,6 +3317,7 @@ static SDL_Surface *SDL_CreateWindowFramebuffer(SDL_Window *window)
}

if (window->surface) {
/* We may have gone recursive and already created the surface */
return window->surface;
}

Expand All @@ -3337,7 +3336,12 @@ SDL_Surface *SDL_GetWindowSurface(SDL_Window *window)
CHECK_WINDOW_MAGIC(window, NULL);

if (!window->surface_valid) {
SDL_DestroyWindowSurface(window);
if (window->surface) {
window->surface->flags &= ~SDL_DONTFREE;
SDL_DestroySurface(window->surface);
window->surface = NULL;
}

window->surface = SDL_CreateWindowFramebuffer(window);
if (window->surface) {
window->surface_valid = SDL_TRUE;
Expand Down Expand Up @@ -3394,6 +3398,25 @@ int SDL_UpdateWindowSurfaceRects(SDL_Window *window, const SDL_Rect *rects,
return _this->UpdateWindowFramebuffer(_this, window, rects, numrects);
}

int SDL_DestroyWindowSurface(SDL_Window *window)
{
CHECK_WINDOW_MAGIC(window, -1);

if (window->surface) {
window->surface->flags &= ~SDL_DONTFREE;
SDL_DestroySurface(window->surface);
window->surface = NULL;
window->surface_valid = SDL_FALSE;
}

if (_this->checked_texture_framebuffer) { /* never checked? No framebuffer to destroy. Don't risk calling the wrong implementation. */
if (_this->DestroyWindowFramebuffer) {
_this->DestroyWindowFramebuffer(_this, window);
}
}
return 0;
}

int SDL_SetWindowOpacity(SDL_Window *window, float opacity)
{
int retval;
Expand All @@ -3417,19 +3440,6 @@ int SDL_SetWindowOpacity(SDL_Window *window, float opacity)
return retval;
}

int SDL_DestroyWindowSurface(SDL_Window *window)
{
CHECK_WINDOW_MAGIC(window, -1);

if (window->surface) {
window->surface->flags &= ~SDL_DONTFREE;
SDL_DestroySurface(window->surface);
window->surface = NULL;
window->surface_valid = SDL_FALSE;
}
return 0;
}

int SDL_GetWindowOpacity(SDL_Window *window, float *out_opacity)
{
CHECK_WINDOW_MAGIC(window, -1);
Expand Down Expand Up @@ -3922,12 +3932,6 @@ void SDL_DestroyWindow(SDL_Window *window)

SDL_DestroyWindowSurface(window);

if (_this->checked_texture_framebuffer) { /* never checked? No framebuffer to destroy. Don't risk calling the wrong implementation. */
if (_this->DestroyWindowFramebuffer) {
_this->DestroyWindowFramebuffer(_this, window);
}
}

/* Make no context current if this is the current context window */
if (window->flags & SDL_WINDOW_OPENGL) {
if (_this->current_glwin == window) {
Expand Down
79 changes: 74 additions & 5 deletions test/testautomation_video.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static SDL_Window *createVideoSuiteTestWindow(const char *title)

window = SDL_CreateWindow(title, w, h, flags);
SDLTest_AssertPass("Call to SDL_CreateWindow('Title',%d,%d,%" SDL_PRIu64 ")", w, h, flags);
SDLTest_AssertCheck(window != NULL, "Validate that returned window struct is not NULL");
SDLTest_AssertCheck(window != NULL, "Validate that returned window is not NULL");

/* Check the window is available in the window list */
windows = SDL_GetWindows(&count);
Expand Down Expand Up @@ -190,7 +190,7 @@ static int video_createWindowVariousSizes(void *arg)

window = SDL_CreateWindow(title, w, h, 0);
SDLTest_AssertPass("Call to SDL_CreateWindow('Title',%d,%d,SHOWN)", w, h);
SDLTest_AssertCheck(window != NULL, "Validate that returned window struct is not NULL");
SDLTest_AssertCheck(window != NULL, "Validate that returned window is not NULL");

/* Clean up */
destroyVideoSuiteTestWindow(window);
Expand Down Expand Up @@ -265,7 +265,7 @@ static int video_createWindowVariousFlags(void *arg)

window = SDL_CreateWindow(title, w, h, flags);
SDLTest_AssertPass("Call to SDL_CreateWindow('Title',%d,%d,%" SDL_PRIu64 ")", w, h, flags);
SDLTest_AssertCheck(window != NULL, "Validate that returned window struct is not NULL");
SDLTest_AssertCheck(window != NULL, "Validate that returned window is not NULL");

/* Clean up */
destroyVideoSuiteTestWindow(window);
Expand Down Expand Up @@ -1732,7 +1732,7 @@ static int video_setWindowCenteredOnDisplay(void *arg)
window = SDL_CreateWindowWithProperties(props);
SDL_DestroyProperties(props);
SDLTest_AssertPass("Call to SDL_CreateWindow('Title',%d,%d,%d,%d,SHOWN)", x, y, w, h);
SDLTest_AssertCheck(window != NULL, "Validate that returned window struct is not NULL");
SDLTest_AssertCheck(window != NULL, "Validate that returned window is not NULL");

/* Wayland windows require that a frame be presented before they are fully mapped and visible onscreen. */
if (video_driver_is_wayland) {
Expand Down Expand Up @@ -2238,6 +2238,71 @@ static int video_getSetWindowState(void *arg)
return skipFlags != (SDL_WINDOW_MAXIMIZED | SDL_WINDOW_MINIMIZED) ? TEST_COMPLETED : TEST_SKIPPED;
}

/**
* Tests window surface functionality
*/
static int video_getWindowSurface(void *arg)
{
const char *title = "video_getWindowSurface Test Window";
SDL_Window *window;
SDL_Surface *surface;
SDL_Renderer *renderer;
int result;

/* Make sure we're testing interaction with an accelerated renderer */
SDL_SetHint(SDL_HINT_FRAMEBUFFER_ACCELERATION, "1");

window = SDL_CreateWindow(title, 320, 320, 0);
SDLTest_AssertPass("Call to SDL_CreateWindow('Title',320,320,0)");
SDLTest_AssertCheck(window != NULL, "Validate that returned window is not NULL");

surface = SDL_GetWindowSurface(window);
SDLTest_AssertPass("Call to SDL_GetWindowSurface(window)");
SDLTest_AssertCheck(surface != NULL, "Validate that returned surface is not NULL");
SDLTest_AssertCheck(SDL_WindowHasSurface(window), "Validate that window has a surface");

result = SDL_UpdateWindowSurface(window);
SDLTest_AssertPass("Call to SDL_UpdateWindowSurface(window)");
SDLTest_AssertCheck(result == 0, "Verify return value; expected: 0, got: %d", result);

/* We shouldn't be able to create a renderer on a window with a surface */
renderer = SDL_CreateRenderer(window, NULL);
SDLTest_AssertPass("Call to SDL_CreateRenderer(window, NULL)");
SDLTest_AssertCheck(renderer == NULL, "Validate that returned renderer is NULL");

result = SDL_DestroyWindowSurface(window);
SDLTest_AssertPass("Call to SDL_DestroyWindowSurface(window)");
SDLTest_AssertCheck(result == 0, "Verify return value; expected: 0, got: %d", result);
SDLTest_AssertCheck(!SDL_WindowHasSurface(window), "Validate that window does not have a surface");

/* We should be able to create a renderer on the window now */
renderer = SDL_CreateRenderer(window, NULL);
SDLTest_AssertPass("Call to SDL_CreateRenderer(window, NULL)");
SDLTest_AssertCheck(renderer != NULL, "Validate that returned renderer is not NULL");

/* We should not be able to create a window surface now, unless it was created by the renderer */
if (!SDL_WindowHasSurface(window)) {
surface = SDL_GetWindowSurface(window);
SDLTest_AssertPass("Call to SDL_GetWindowSurface(window)");
SDLTest_AssertCheck(surface == NULL, "Validate that returned surface is NULL");
}

SDL_DestroyRenderer(renderer);
SDLTest_AssertPass("Call to SDL_DestroyRenderer(renderer)");
SDLTest_AssertCheck(!SDL_WindowHasSurface(window), "Validate that window does not have a surface");

/* We should be able to create a window surface again */
surface = SDL_GetWindowSurface(window);
SDLTest_AssertPass("Call to SDL_GetWindowSurface(window)");
SDLTest_AssertCheck(surface != NULL, "Validate that returned surface is not NULL");
SDLTest_AssertCheck(SDL_WindowHasSurface(window), "Validate that window has a surface");

/* Clean up */
SDL_DestroyWindow(window);

return TEST_COMPLETED;
}

/* ================= Test References ================== */

/* Video test cases */
Expand Down Expand Up @@ -2317,12 +2382,16 @@ static const SDLTest_TestCaseReference videoTest19 = {
(SDLTest_TestCaseFp)video_getSetWindowState, "video_getSetWindowState", "Checks transitioning between windowed, minimized, maximized, and fullscreen states", TEST_ENABLED
};

static const SDLTest_TestCaseReference videoTest20 = {
(SDLTest_TestCaseFp)video_getWindowSurface, "video_getWindowSurface", "Checks window surface functionality", TEST_ENABLED
};

/* Sequence of Video test cases */
static const SDLTest_TestCaseReference *videoTests[] = {
&videoTest1, &videoTest2, &videoTest3, &videoTest4, &videoTest5, &videoTest6,
&videoTest7, &videoTest8, &videoTest9, &videoTest10, &videoTest11, &videoTest12,
&videoTest13, &videoTest14, &videoTest15, &videoTest16, &videoTest17,
&videoTest18, &videoTest19, NULL
&videoTest18, &videoTest19, &videoTest20, NULL
};

/* Video test suite (global) */
Expand Down

0 comments on commit 9eecc2e

Please sign in to comment.