Fix issues related to delay when processing events on X11 display server #41910
Conversation
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Do you not need some sort of locking to protect from calls to x11_display in the main thread? like for setting window size and other stuff
There was a problem hiding this comment.
XLib calls generally use XLockDisplay/XUnlockDisplay internally, so it should be safe as long as they don't poll events from the event queue while our thread process is doing it as well. I've used our own mutex when this case happens.
There was a problem hiding this comment.
Maybe @hpvb can validate that I'm not overlooking anything about thread safety.
pouleyKetchoupp
force-pushed
the
x11-inputs-lag-fix
branch
from
1278ee1 to
b9ff982
Compare
This change makes keyboard inputs more responsive on Linux, especially when the FPS is lower on slower configurations. Polling events from the x server is done on a separate thread to avoid a frame delay with inputs, due to first sending the event to the input manager with XFilterEvent then processing the new event only on the next frame. Calls to Input Manager functions like XSetICFocus, XUnsetICFocus and XSetICValues use a mutex, because they are polling events internally and would otherwise interfere with our own thread process for polling events which can cause a deadlock in some cases. XUnsetICFocus is called instead of XSetICFocus on FocusOut events, so the input manager can be properly notified of focus changes. clipboard_get now uses a blocking call to poll for a specific event type when waiting for a SelectionNotify event, instead of polling all events and filtering them afterwards.
When pasting clipboard content from Godot to other applications, multiple SelectionRequest events are sent to Godot in order to access the data. It could take a long time before the data is ready for the other app because events were processed one by one on the main thread, especially when Godot is unfocused and runs at low frequency. With this change, SelectionRequest events are directly handled on the separate event polling thread to minimize this delay. This change also replaces clipboard_get() calls in SelectionRequest with a direct access to internal_clipboard, since in this case we know Godot is the owner of the clipboard content and it's not necessary to query the x server for it.
pouleyKetchoupp
force-pushed
the
x11-inputs-lag-fix
branch
from
b9ff982 to
5a0376f
Compare
pouleyKetchoupp
changed the title
|
Just pushed an extra commit to fix a issue with clipboard contents on top of the delay with keyboard inputs, since both of these changes need events to be processed on a separate thread. @hpvb I know you have a plan for refactoring x11 events processing, but I figured that for now it doesn't seem too difficult to selectively process certain events on the separate thread to fix specific issues. It should be even possible to backport these changes to the 3.2 branch, and it's possible to refactor more things later for 4.0. |
3.2 backport of PR godotengine#41910: Fix general keyboard input lag on X11 display server Fix delay to process clipboard content from Godot in other programs
|
Thanks! (Note: approved on IRC by TMM) |
Edit 09/25/2020: Second commit added to fix clipboard issues.
This change moves the handling of events from x server to a separate thread to fix specific issues due to the delay caused by processing them in the main loop, which didn't allow for asynchronous communication with the x server.
Fix general keyboard input lag on X11 display server
The first commit makes keyboard inputs more responsive on Linux, especially when the FPS is lower on slower configurations.
Polling events from the x server is done on a separate thread to avoid a frame delay with inputs, due to first sending the event to the input manager with
XFilterEventthen processing the new event only on the next frame.Calls to Input Manager functions like
XSetICFocus,XUnsetICFocusandXSetICValuesuse a mutex, because they are polling events internally and would otherwise interfere with our own thread process for polling events which can cause a deadlock in some cases.XUnsetICFocusis called instead ofXSetICFocusonFocusOutevents, so the input manager can be properly notified of focus changes.clipboard_getnow uses a blocking call to poll for a specific event type when waiting for aSelectionNotifyevent, instead of polling all events and filtering them afterwards.Fixes #31177
Fix delay to process clipboard content from Godot in other programs
The second commit adds extra changes to fix issues with clipboard handling.
When pasting clipboard content from Godot to other applications, multiple SelectionRequest events are sent to Godot in order to access the data. It could take a long time before the data is ready for the other app because events were processed one by one on the main thread, especially when Godot is unfocused and runs at low frequency.
With this change, SelectionRequest events are directly handled on the separate event polling thread to minimize this delay.
This change also replaces clipboard_get() calls in SelectionRequest with a direct access to internal_clipboard, since in this case we know Godot is the owner of the clipboard content and it's not necessary to query the x server for it.
Fixes #39422