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

sdl_glimp: also recreate window on mode change #644

Closed
wants to merge 1 commit into from

Conversation

illwieckz
Copy link
Member

Also recreate window on mode change. Especially when the mode is reset to 3 because initialization failed.

This is a fix up for #478. Forgetting to also compare mode to decide to recreate window or not introduced a regression.

I caught it because it looks like the current SDL version on focal is buggy and fails to initialize correctly when the window size is larger than the screen (the released 0.52.1 compiled against another SDL doesn't suffer from that). Also, it may not be the same issue (I got mine in windowed mode), but dhewm3 is also experiencing the same initialization problem with some fullscreen configuration and recent SDL: dhewm/dhewm3#466

When I implemented the reuse of already existing context and window if there was no change to speed-up things, I forgot to also compare the mode. It means that when the SDL bug occurs, instead of resetting to a configuration that works, it just exits the game:

--- Common Initialization Complete --- 
Calling GetRefAPI… 
SDL_Init( SDL_INIT_VIDEO )...  
Using SDL version 2.0.10 
SDL using driver "x11" 
Initializing OpenGL display 
Using GLEW version 2.1.0 
Display aspect: 1.778 
...setting mode -1: 2560×1440 
Using preferred context - 24-bit OpenGL 3.2 core 
OpenGL Renderer: Quadro K1100M/PCIe/SSE2 
Using GL3 Renderer in OpenGL 3.x mode... 
Warn: Couldn't get window display mode: Couldn't find display mode match 
Warn: GLimp: Unknown error for mode -1 
Setting r_mode -1 failed, falling back on r_mode 3 
Initializing OpenGL display 
Using GLEW version 2.1.0 
Display aspect: 1.778 
...setting mode 3: 640×480 
Using preferred context - 24-bit OpenGL 3.2 core 
OpenGL Renderer: Quadro K1100M/PCIe/SSE2 
Using GL3 Renderer in OpenGL 3.x mode... 
Warn: Couldn't get window display mode: Couldn't find display mode match 
Warn: GLimp: Unknown error for mode 3 
Warn: Couldn't load a renderer. 

Note: I just reproduced the “cannot open a window larger than screen” bug with an Nvidia Quadro K1100M on nvidia 418 driver, but I also reproduced it some months ago with an ATi r300 card on current Mesa (at the time), so that looks really unrelated to the driver and it is very likely an SDL issue.

@slipher
Copy link
Member

slipher commented Jul 17, 2022

I don't get it. What is the use of "mode" here? r_mode is a setting that determines how the requested resolution is calculated, but AFAIK isn't a part of the final parameters to window creation.

@illwieckz
Copy link
Member Author

What I understand is that you can't reuse a window that is broken for whatever reason. If a previous mode produced an unusable window, then the window should be recreated when trying the next mode. I don't know how mode works, but well, given this is resolution-specific, it may not be a bad idea to recreate the window. In all cases that fixes the bug of an early failure making fail subsequent tries.

@illwieckz
Copy link
Member Author

illwieckz commented Jul 17, 2022

We may otherwise chose to call GLimp_DestroyWindowIfExists() before retrying with the fallback mode, but this looks more solid to me: as everything else changing requires a recreation anyway, this doesn't sound bad to do the same with mode.

@slipher
Copy link
Member

slipher commented Jul 17, 2022

r_mode determines whether to take the resolution from the screen size, user-provided values, or one of a hard-coded list of resolutions. As long as you are detecting changes in the requested resolution, there is no use in checking r_mode.

@illwieckz
Copy link
Member Author

@illwieckz illwieckz closed this Jul 26, 2022
@illwieckz illwieckz mentioned this pull request Jul 27, 2022
@illwieckz
Copy link
Member Author

illwieckz commented Jul 27, 2022

While recreating the window on mode/resolution change was needed, the issue described in first post was just a way to reproduce a need for using the fallback resolution.

The issue isn't an SDL issue, and we can display windows larger than the screen with the same SDL if we ignore the “Couldn't find display mode match” error, when the hardware supports it.

One problem is that the code was actually modified to load the fallback resolution on “Couldn't find display mode match” to prevent the engine to segfault when the hardware doesn't support the given resolution.

Basically there are two needs there:

  • Open a window larger than the screen when the hardware supports it, that's even useful for making large screenshots or for testing resolutions, see UI scaling is broken Unvanquished/Unvanquished#1922), we can continue.
  • Open a window larger than what the hardware support, we must catch that and load a safe resolution to prevent an engine segfault later.

The problem is that in both cases the exact same error is caught, I don't see a way to know if the error happens because the window is larger than the screen or because the hardware doesn't support the size.

See also this comment:

@ghost
Copy link

ghost commented Jul 27, 2022

Open a window larger than the screen when the hardware supports it, that's even useful for making large screenshots or for testing resolutions, see UI scaling is broken Unvanquished/Unvanquished#1922), we can continue.

This hardly looks like a normal use case, so I would only allow that if user toggles an option like "experimental.allow.wider.resolutions" and have a popup at each run (or maybe when the error pops out if possible) remembering them that they have this dangerous option turned on.

This is like using /devteam [ah] and spawning in a wall: it's a contributor option, and those should be able to read and understand what they do.

@illwieckz
Copy link
Member Author

Opening a window larger than the screen is not a normal use case and it's fine to hide it behind a cvar.

Unfortunately opening a window of arbitrary 900×700 size while your screen only supports 800×600 and 1600×1200 fullscreen resolutions is perfectly OK to do. We have to make sure this is possible to do.

@illwieckz
Copy link
Member Author

There is one legit usage of a window larger than one screen: having 3 screens and spanning the window on all of them (I even did that with quake3 in the early 2000's).

@illwieckz illwieckz deleted the illwieckz/mode branch July 30, 2022 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants