Linux: Force SDL2 video driver selection to X11

[nadiaholmquist]

It looks like RT64 does not currently support Wayland and unconditionally expects to receive an X11 window, but on some distros (namely, Fedora in my case) SDL2 will select the Wayland video driver by default.

While it'd of course be preferable to support both, this PR adds a small change to forcibly set the SDL2 video driver to X11 if the Wayland driver is present to prevent the game from crashing for people.

[boomshroom]

Related issue: rt64/rt64#19

[dcvz]

Related issue: rt64/rt64#19

boom shroom does this PR work for you as well?

[boomshroom]

Related issue: rt64/rt64#19

boom shroom does this PR work for you as well?

I don't currently have this exact patch, but I do have the same line in my local repo, just without the #if guard, and it only works if I explicitly unset the SDL_VIDEODRIVER environment variable. Commenting out the line actually does nothing.

Since I'm running Wayland normally, I automatically set SDL_VIDEODRIVER to wayland,x11 to avoid xwayland when possible. For some reason, that environment variable seems to override setting the hint in the source code. It seems that if the application actually depends on a particular video driver, the recommended actions are to either reset the environment variable within the application prior to initializing SDL, or to file a bug report.

[darksylinc]

Since I'm running Wayland normally, I automatically set SDL_VIDEODRIVER to wayland,x11 to avoid xwayland when possible. For some reason, that environment variable seems to override setting the hint in the source code.

Forcing an unsupported backend will end up in a crash. I'm not sure what you are expecting.

You should not try to force apps to use Wayland via SDL_VIDEODRIVER. On most distros SDL2 defaults to x11 because of frame pacing issues. SDL2 will only default to using Wayland if it determines there is something to gain (i.e. fifo-v1 and commit-timing-v1 protocols are both present).

This PR tells SDL2 to always prefer x11 (i.e. even if fifo-v1 and commit-timing-v1 are present). But you should absolutely not try to force it via environment variables. That env. variable should only be used to workaround specific problems with specific apps.

[boomshroom]

Forcing an unsupported backend will end up in a crash. I'm not sure what you are expecting.

What I was expecting was setting a preferred backend, and then a fallback in case the first is unsupported. (Alternatively, a crash saying "video driver not supported" rather than a segfault.) I was unaware that the environment variable, even when set with multiple backends, would always force the first. Until now, I hadn't noticed any issues with running SDL applications with the variable set.

[nadiaholmquist]

Would it be best if I adddd a check further down verifying that what SDL gave us is indeed an X11 window and exiting if it's not? That way it can catch a case like this of a user trying to override the preference instead of crashing.

[dcvz]

@nadiaholmquist would you mind doing an empty push to validate with the CI? Our move to an organization means secrets won't take until the next push (even if retrigger)

[nadiaholmquist]

Sorry, I'm not at my computer currently so I can't do that. I think any collaborator in the organization should be able to do it though.

[dcvz]

Sorry, I'm not at my computer currently so I can't do that. I think any collaborator in the organization should be able to do it though.

Didn’t want to invade the PR. If that’s ok with you, I’ll do that then

[nadiaholmquist]

Yeah that's alright.

[dcvz]

Looks like GitHub has failed us here. I’m gonna close and reopen and see if that takes.

[dcvz]

Turns out it’s an issue with forks and secrets, we’re working on a fix

[dcvz]

@nadiaholmquist I believe this is now fixed. Rebasing the branch against dev should allow us to validate your build via CI! Let me know if you want me to do that for you.

[nadiaholmquist]

bleh I didn't have all changes locally apparently and only noticed after force pushing. Let me just fix that real quick.

Edit: There we go.

[dcvz]

Thanks a lot for the fix @nadiaholmquist and for bearing with us as we sorted out the workflow issue!