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

Prevent SDL_WaitEvent loop from running before wlserver has finished initializing #1078

Conversation

sharkautarch
Copy link

@sharkautarch sharkautarch commented Dec 30, 2023

(hopefully) fixes issue #661

Uses C++11 compatible atomic and condition variable (reusing the steamcompmgr "poor man's semaphore" class)
This PR only adds synchronization during startup, so not much overhead IMO

I had noticed that issue #661 would be triggered 50% of the time when running ValveSoftware/gamescope/master under intel VTune.
Compiling and running gamescope w/ this PR under intel VTune -> haven't seen any occurrences of the issue after running gamescope under VTune 4 different times.

@sharkautarch sharkautarch force-pushed the gs-fix-wlserver-wlr-virtual_keyboard_device_nullptr_assert_fail branch from 6a1e5af to 538fc83 Compare December 30, 2023 01:34
@misyltoad
Copy link
Collaborator

You don't need to use the semaphore + condition variable, etc -- you can just use a std::atomic<bool> + .wait + .notify_all on it.

@sharkautarch
Copy link
Author

You don't need to use the semaphore + condition variable, etc -- you can just use a std::atomic<bool> + .wait + .notify_all on it.

I was originally thinking of using atomic wait, but I realized that atomic wait is actually only in c++20 and up. Plus I don’t think gcc will compile with c++20 support by default (correct me if I am wrong on that).
Wasn’t sure if you wanted gamescope to be able to be c++11 compatible or not.

@misyltoad
Copy link
Collaborator

Gamescope already requires C++20

@sharkautarch
Copy link
Author

Gamescope already requires C++20

Ah i see, ok I will revise this PR to use atomic + atomic wait instead of the ‘semaphore’

@sharkautarch sharkautarch force-pushed the gs-fix-wlserver-wlr-virtual_keyboard_device_nullptr_assert_fail branch 4 times, most recently from c3aa2f7 to 4cd1c10 Compare January 1, 2024 19:57
@sharkautarch
Copy link
Author

You don't need to use the semaphore + condition variable, etc -- you can just use a std::atomic<bool> + .wait + .notify_all on it.

Ok, I've now rewritten the PR to just use std::atomic<bool> + .wait + .notify_one

src/main.cpp Outdated Show resolved Hide resolved
@sharkautarch sharkautarch force-pushed the gs-fix-wlserver-wlr-virtual_keyboard_device_nullptr_assert_fail branch 3 times, most recently from 553fc35 to 64aedf9 Compare January 1, 2024 23:13
src/sdlwindow.cpp Outdated Show resolved Hide resolved
@sharkautarch sharkautarch force-pushed the gs-fix-wlserver-wlr-virtual_keyboard_device_nullptr_assert_fail branch from 30b3e9a to a3bda5f Compare January 6, 2024 16:37
@sharkautarch sharkautarch force-pushed the gs-fix-wlserver-wlr-virtual_keyboard_device_nullptr_assert_fail branch 2 times, most recently from a91ea0e to a9914cf Compare February 20, 2024 15:57
@sharkautarch sharkautarch force-pushed the gs-fix-wlserver-wlr-virtual_keyboard_device_nullptr_assert_fail branch from 9d81a47 to 7e2bc21 Compare February 28, 2024 14:52
@sharkautarch
Copy link
Author

@Joshua-Ashton Could you please re-review this PR?

@sharkautarch sharkautarch force-pushed the gs-fix-wlserver-wlr-virtual_keyboard_device_nullptr_assert_fail branch from 7e2bc21 to d2b8fa0 Compare February 28, 2024 15:23
@sharkautarch
Copy link
Author

Superseded by commit be20af6 in upstream gamescope

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