[SDL3] SDL_WindowRestore does not restore the original window size when maximize->full-screen(true)->minize->restore->fullscreen(false)->restore()

[abdes]

Using SDL3 from main, on Windows Platform.

The following sequence of calls produces the following results:

• create window size 800x600 on a display of size 3840x2160 => OK
• maximize => window size 3840x2036 => OK
• full-screen(true)=> window size 3840x2160 => OK
• minimize => window size 3840x2160 => OK
• restore => window size 3840x2160 => OK
• full-screen(false)=> window size 3840x2036 => OK
• restore => window size 3816x2096 => NOT OK - expected 800x600

Note that the following sequence produces the final result of window size = 800x600

• create window size 800x600 on a display of size 3840x2160 => OK
• maximize => window size 3840x2036 => OK
• full-screen(true)=> window size 3840x2160 => OK
• full-screen(false)=> window size 3840x2036 => OK
• restore => window size 800x600

[slouken]

What happens if you do this sequence?

• create window size 800x600 on a display of size 3840x2160
• maximize
• minimize
• restore
• restore

[abdes]

I20230526 21:45:17.808161 10408 window.cpp:92] SDL3 Window[oxy-window-playground] created I20230526 21:45:20.084039 10408 main.cpp:111] Maximize() I20230526 21:45:20.087045 10408 main.cpp:113] w: 3840, h: 2036 I20230526 21:45:20.087045 10408 main.cpp:114] x: 0, y: 40 I20230526 21:45:21.513334 10408 main.cpp:117] Minimize() I20230526 21:45:21.524124 10408 main.cpp:119] w: 3840, h: 2036 I20230526 21:45:21.525131 10408 main.cpp:120] x: 0, y: 40 I20230526 21:45:21.525131 10408 main.cpp:121] Restore() I20230526 21:45:21.536581 10408 main.cpp:123] w: 3840, h: 2036 I20230526 21:45:21.536581 10408 main.cpp:124] x: 0, y: 40 I20230526 21:45:23.237475 10408 main.cpp:127] Restore() I20230526 21:45:23.239476 10408 main.cpp:129] w: 800, h: 600 I20230526 21:45:23.240476 10408 main.cpp:130] x: 1520, y: 780

As expected behavior.

[slouken]

Ah, okay, so switching fullscreen and back is seen as an explicit window size change (because it is) and changes the restore flow. I'm not sure what the best fix is here, as any change to the way restore works is likely to introduce unexpected behaviors.

You're welcome to submit a PR that makes it work the way you'd expect and get community feedback.

[abdes]

It's actually way more broken than just this scenario. I've made a test application that can allow me to trigger the full-screen, maximize, minimize and restore actions with key events.

Running it and randomly triggering these actions really produces bizarre results such as the window going full-screen on the second display instead of maximizing on the current display after some combos, or restoring to its maximized size when it should not.

I need to spend some time thoroughly reviewing the code and doing some specific unit tests to check the invariants before I suggest a resolution to my issue. I'll try to do that soon. Thanks for taking the time to reply.

[slouken]

You're welcome, good luck!

[abdes]

Actually, looking at the code for SDL_MaximizeWindow, which I think is the culprit, in the scenario above, we can already see the existing doubts in the mind of whoever wrote that code 😄 .

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

    if (window->flags & SDL_WINDOW_MAXIMIZED) {
        return 0;
    }

    if (window->flags & SDL_WINDOW_HIDDEN) {
        window->pending_flags |= SDL_WINDOW_MAXIMIZED;
        return 0;
    }

    /* !!! FIXME: should this check if the window is resizable? */

    if (_this->MaximizeWindow) {
        _this->MaximizeWindow(_this, window);
    }
    return 0;
}

First, calling maximize on a full-screen window does not switch the window out of full-screen mode. Second, yeah, it maybe worth checking that the window is resizable before accepting a maximize request, and a full-screen window is not resizable (logically).

So maybe just /* !!! FIXME: should this check if the window is resizable? */ with the interpretation of resizable being having the flag SDL_WINDOW_RESIZABLE false or being in full-screen mode, is enough 😄

Your opinion and others would be helpful.

Same thing for SDL_RestoreWindow, it has no visual effect on a full-screen window, but still does restore the window size when not in full-screen, which is counter-intuitive and misleading. It should become a no-op for full-screen windows.

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

    if (!(window->flags & (SDL_WINDOW_MAXIMIZED | SDL_WINDOW_MINIMIZED))) {
        return 0;
    }

    if (_this->RestoreWindow) {
        _this->RestoreWindow(_this, window);
    }
    return 0;
}

Additionally, a BORDERLESS window is also not resizable.