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

Crash when using display filters on some systems (fix included) #1433

Open
karadoc opened this issue Jan 12, 2024 · 4 comments
Open

Crash when using display filters on some systems (fix included) #1433

karadoc opened this issue Jan 12, 2024 · 4 comments

Comments

@karadoc
Copy link

karadoc commented Jan 12, 2024

Backstory (maybe not important):
In the past I've noticed intermittent crashes on Windows when setting some display filters. I mostly just ignored it. Recently I've started running openxcom on Linux, and I ran into a couple of problems:

  • When playing in full screen mode, the game cannot be minimised. (keyboard shortcuts such as alt+tab, super+d, ctrl+alt+right, etc have no effect); also, the screen resolution was not returned to previous settings after exiting.
  • Playing in 'borderless' (with full screen resolution), got around both those problems, but unfortunately the desktop task panel was still drawn over the bottom part of the screen, getting in the way of the game.

After some searching I worked out that both these problems are due to how sdl1 interacts with newer linux systems; and I was able to fix both of these problems by using sdl1.2-compat, which replace the normal sdl runtime with a version that uses sdl2 as a backend. (No recompiling required.) This worked well, except it that it introduced a new problem.

Main issue and solution:
The problem was that any time I set any display filter, the game would immediately crash. It would display a box saying "Use SDL_GL_SwapBuffers() on OpenGL surface", and then it would seg-fault. Restarting the game kept the same faulty display filter settings, so I had to delete the config file every time I wanted to start the game without crashing.

In any case, I took the advice of the error message and made a minor change to the code, which you can see here:
karadoc@e2d44cb

Essentially, I'm telling it to use SDL_GL_SwapBuffers instead of SDL_Flip when OpenGL is in use. I don't know much about OpenGL, but this change fixed the issue for me. I've tested it both with libsdl1.2debian (which is what I had originally) and libsdl1.2-compat (which is what I'm now using to fix the original problems). It works with and without display filters with both versions of the run time, with no crashes. I haven't yet tested on Windows though.

Note: this is probably the same issue as #1389.

@karadoc
Copy link
Author

karadoc commented Jan 13, 2024

Ok... I've just tested my 'fix' on Windows, and the results are not great. It's causing some crazy flickering weirdness. So I guess I haven't worked out a fix after all. Sorry. I'll let you know if I learn more.

@SupSuper
Copy link
Member

There's already a SDL_GL_SwapBuffers here: https://github.com/OpenXcom/OpenXcom/blob/master/src/Engine/Zoom.cpp#L680
Maybe it's in the wrong place?

@karadoc
Copy link
Author

karadoc commented Jan 13, 2024

Right. The name flipWithZoom does imply that it should be flipping the display - but by the looks of it, it only flips in OpenGL mode.

If I simply comment out SDL_GL_SwapBuffers() line in Zoom::flipWithZoom, so that it is only called in my modified version of Screen::flip(), then the flicking problem in Windows is fixed. (It makes sense, because if we flip, then immediately flip again we're probably going to get a blank screen, or perhaps just whatever the previous buffer use to have - which might be why it seemed to work in Linux.)

I'll do a bit more testing, but my current recommendation is to rename Zoom::flipWithZoom to Zoom::blitWithZoom or something like that, and get remove its call to SDL_GL_SwapBuffers(); then just keep the change that I made in Screen::flip() (which is to either use SDL_GL_SwapBuffers() for OpenGL mode, or SDL_Flip(_screen) for ordinary 2D SDL mode.)

There are still a few things I'm foggy on. For example, I don't fully understand the purpose of OpenGL::refresh(). The description says "make the buffer show up on screen"; which makes it sound exactly the same as Flip and SwapBuffers... so that's confusing to me.

@karadoc
Copy link
Author

karadoc commented Jan 13, 2024

Ok. I've amended my original fix to do what I said in the previous post. Here it is:
karadoc@7138594

(I figure that since no one relies on my repository, I can just force history changing commits for neatness.)

I've tested this (briefly) on both Windows and Linux, with and without shaders. I did not notice any problems related to that. So I think this time is is properly fixed!

That said, one time during my testing I did get a crash in Windows while switching shaders. It wasn't with the final code that I'm uploading now, but it was with a version without the Screen::flip(). Essentially what I'm saying is that I don't think my patch here is going to fix #1389 after all. I now believe that's something unrelated. I haven't been able to reliably reproduce the crash. It just seems to sometimes happen when switching between shaders.

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

No branches or pull requests

2 participants