Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix restoring window size when exiting fullscreen #3400

Merged
merged 2 commits into from Oct 10, 2021
Merged

Conversation

@criezy
Copy link
Member

@criezy criezy commented Oct 2, 2021

There were two different issues:

  • On macOS the window was always restored to the maximum window size and not to the previous window size.
  • The window was restored to the default size and not to the previous window size.

The first issue might be specific to macOS, for which fullscreen more or less means maximized, and in particular when requesting a SDL_WINDOW_FULLSCREEN_DESKTOP window, the SDL_WINDOW_MAXIMIZED flag gets also set. As a result, when we exit fullscreen, loadVideoMode() gets called while the window is still fullscreen, and checking the window flag indicate it is maximized. As a result it tries to restore the maximized size. Since we store in the config if the window was maximized, instead of looking at the SDL window flag we can use the flag we stored in the config.

That issue only occurred if maximized size was stored in the config file (i.e. if the window had been maximized by the user at least once in the past).

The second issue was more general. In loadVideoMode() when switching to fullscreen, it was storing the default size in the config, overwriting the current window size. So when exiting fullscreen, the size was restored to the default size (since this is what was stored in the config). I don't think this was intended, and at least it seems to me that restoring to the size the window had before entering fullscreen would be what is expected.

As the SDL behaviour may be different on other systems, it would be good to test that this works as expected on other systems (e.g. Windows and Linux) before merging this change.

Note: On macOS setting the SDL_WINDOW_MAXIMIZED does not quite work properly. The window is resized to fill the screen, but with the menubar and title bar still visible. That means that if we maximize the window (maximize button in window title bar), then go fullscreen (Alt+Enter) and then exit fullscreen (Alt+Enter), we get an almost-maximised window but not exactly what we had before entering fullscreen.

criezy added 2 commits Oct 2, 2021
… fullscreen

When loadVideoMode gets called to exit fullscreen, the window is
still fullscreen, which means that the SDL_WINDOW_MAXIMIZED is set
(at least on macOS). As a result the window was resized to the
stored maximized window size instead of the size it had before
entering fullscreen.
When entering fullscreen it was storing the default window size
in the config, which was overwriting the current window size. As
a result when exiting window size, instead of restoring to the
previous window size it was restoring to the default window size.
@@ -348,29 +348,30 @@ bool OpenGLSdlGraphicsManager::loadVideoMode(uint requestedWidth, uint requested
Common::Rect desktopRes = _window->getDesktopResolution();

#if SDL_VERSION_ATLEAST(2, 0, 0)
SDL_Window *window = _window->getSDLWindow();
bool isMaximized = window ? (SDL_GetWindowFlags(window) & SDL_WINDOW_MAXIMIZED) : false;
Copy link
Member

@lotharsm lotharsm Oct 3, 2021

I'm pretty sure we have to keep the isMaximized check intact, since this is not covered by _wantsFullScreen. At least on Win32 and Linux, fullscreen and maximized are not the same like on macOS, but two different modes.

Fetching the "isMaximized" directly from ConfMan never worked reliably unfortunately when I implemented this - adding this check was one of the keys to solve issues like having a restored window drawn way to big.

Copy link
Member

@lotharsm lotharsm Oct 3, 2021

Now that I think of it, maybe it's better to fix what causes my workaround to be necessary in the first place 😂

@bluegr
Copy link
Member

@bluegr bluegr commented Oct 10, 2021

Regarding the second issue, I can confirm that with the changes in this PR, the original window's dimensions are preserved correctly after switching from full screen under Windows 10.

@bluegr
Copy link
Member

@bluegr bluegr commented Oct 10, 2021

I have also tested this under *nix (Ubuntu 18.04.3 LTS), and the behavior is correct as well.

Thanks for your work! This is good to be merged now

@bluegr bluegr merged commit 594a81f into scummvm:master Oct 10, 2021
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants