SDL_CreateWindow position not applied when going from fullscreen to windowed

[eloj]

Platform: Linux/X11, SDL 2.23.0 @ 822cf0b34be5bc

The X and Y position originally passed to SDL_CreateWindow() does not seem to be retained/applied if the window is created with SDL_WINDOW_FULLSCREEN_DESKTOP, and you then toggle it to windowed mode with SDL_SetWindowFullscreen().

This may be by design, but I found it surprising. Perhaps the complexity of tracking if the user moved to window since is not worth it, which I can totally understand.

Scenario 1: Working as expected

1. SDL_CreateWindow(banner, SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, game->sw, game->sh, 0);
2. SDL_SetWindowFullscreen(win, SDL_WINDOW_FULLSCREEN_DESKTOP);
3. SDL_SetWindowFullscreen(win, 0);
4. The window is centered on screen.

Scenario 2: Surprising

1. SDL_CreateWindow(banner, SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, game->sw, game->sh, SDL_WINDOW_FULLSCREEN_DESKTOP);
2. SDL_SetWindowFullscreen(win, 0);
3. The window is NOT centered on screen.

Event logs

Starting fullscreen, then going to windowed mode:

Entering game loop.
INFO: Window 1 shown
INFO: Window 1 hidden
INFO: Window 1 shown
INFO: Window 1 gained keyboard focus
INFO: Window 1 is offered a focus
INFO: Window 1 moved to 0,0
INFO: Window 1 size changed to 2560x1440
INFO: Window 1 resized to 2560x1440
INFO: Window 1 exposed
INFO: Mouse entered window 1
INFO: Window 1 moved to 2,60  <--- not center
INFO: Window 1 size changed to 1280x800
INFO: Mouse left window 1
INFO: Window 1 exposed
Exited game loop.

Starting windowed, and going fullscreen (desktop), and then back.

Entering game loop.
INFO: Window 1 shown
INFO: Window 1 hidden
INFO: Window 1 shown
INFO: Window 1 moved to 641,350
INFO: Window 1 exposed
INFO: Mouse entered window 1
INFO: Window 1 gained keyboard focus
INFO: Window 1 is offered a focus
INFO: Window 1 moved to 0,0
INFO: Window 1 size changed to 2560x1440
INFO: Window 1 resized to 2560x1440
INFO: Mouse left window 1
INFO: Window 1 exposed
INFO: Mouse entered window 1
INFO: Window 1 moved to 641,350 <-- restored
INFO: Window 1 size changed to 1280x800
INFO: Window 1 exposed
Exited game loop.

[icculus]

Reproduction case. Change the #if 1 for how to create the window, compile, run on x11. Press 'f' to toggle fullscreen/windowed.

/* gcc -Wall -O0 -ggdb3 -o testwin test.c `sdl2-config --cflags --libs` */

#include "SDL.h"

#if 1
#define STARTING_STATE SDL_WINDOW_FULLSCREEN_DESKTOP
#else
#define STARTING_STATE 0
#endif

int main(int argc, char** argv)
{
    SDL_Init(SDL_INIT_VIDEO);
    SDL_Window* win = SDL_CreateWindow("Hello SDL", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 1024, 768, STARTING_STATE);
    SDL_Renderer *renderer = SDL_CreateRenderer(win, -1, 0);
    SDL_Event e;

    int done = 0;
    while (!done) {
        while (SDL_PollEvent(&e)) {
            if ((e.type == SDL_KEYDOWN) && (e.key.keysym.sym == SDLK_f)) {
                const SDL_bool onoff = (SDL_GetWindowFlags(win) & SDL_WINDOW_FULLSCREEN) ? SDL_FALSE : SDL_TRUE;
                SDL_Log("going %s", onoff ? "fullscreen" : "windowed");
                SDL_SetWindowFullscreen(win, onoff ? SDL_WINDOW_FULLSCREEN_DESKTOP : 0);
            } else if (e.type == SDL_QUIT) {
                done = 1;
                break;
            }
        }
        SDL_RenderClear(renderer);
        SDL_RenderPresent(renderer);
        SDL_Delay(100);
    }

    SDL_DestroyWindow(win);
    SDL_Quit();
    return 0;
}

[icculus]

Okay, the problem is, at least for X11, that we rely on the window manager to put this stuff back during SDL_SetWindowFullscreen(win, 0);, but it does it based on the window size/position at the point where we flagged it as fullscreen.

But we create the window with fullscreen values if created with SDL_WINDOW_FULLSCREEN_DESKTOP:

https://github.com/libsdl-org/SDL/blob/dd2e3182116c9b0e07a157adb0ef286a49708fda/src/video/SDL_video.c#L1653-L1656

So the fastest solution is to create new X11 windows with its windowed.* fields and let the window manager sort it out, which fixes the reproduction case, but I have no idea what other side effects this might cause.

@slouken, I'm going to need your opinion on applying this.

diff --git a/src/video/x11/SDL_x11window.c b/src/video/x11/SDL_x11window.c
index f7ec281b9..58d0a8750 100644
--- a/src/video/x11/SDL_x11window.c
+++ b/src/video/x11/SDL_x11window.c
@@ -526,8 +526,14 @@ X11_CreateWindow(_THIS, SDL_Window * window)
                             visual, AllocNone);
     }
 
+    /* Always create this with the window->windowed.* fields; if we're
+       creating a windowed mode window, that's fine. If we're creating a
+       fullscreen window, the window manager will want to know these values
+       so it can use them if we go _back_ to windowed mode. SDL manages
+       migration to fullscreen after CreateSDLWindow returns, which will
+       put all the SDL_Window fields and system state as expected. */
     w = X11_XCreateWindow(display, RootWindow(display, screen),
-                      window->x, window->y, window->w, window->h,
+                      window->windowed.x, window->windowed.y, window->windowed.w, window->windowed.h,
                       0, depth, InputOutput, visual,
                       (CWOverrideRedirect | CWBackPixmap | CWBorderPixel |
                        CWBackingStore | CWColormap), &xattr);

[slouken]

This seems like a really good change, but a little risky at the last minute. I'm going to move the milestone and let's put this in as soon as 2.26.0 is ready to go.