-
-
Notifications
You must be signed in to change notification settings - Fork 21.9k
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
[X11] Fix Godot stealing focus on alternative window managers #86441
Conversation
750a252
to
fcbbf9a
Compare
fcbbf9a
to
2c5ab53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me (have not tested it so far). I guess it can be combined with #86671, to only grab re-focus if it's not already focused and some other window of the same app have focus (with exception for ButtonPress which should only check the current window focus).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me (have not tested it so far). I guess it can be combined with #86671, to only grab re-focus if it's not already focused and some other window of the same app have focus (with exception for ButtonPress which should only check the current window focus).
2c5ab53
to
40d69c2
Compare
Thanks for the review!
Yes, I don't think the two PRs should conflict functionality-wise (there may end up being a Git conflict after one of them is merged, because of the proximity of the changed lines) although logically they are working in kind of inverse directions of each other. Personally, when I test this PR with i3, it doesn't fix any issues for me, but it also doesn't appear to break anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally (Fedora 39 KDE with X11, rebased against 96296e4), it works as expected.
This fixes a longstanding issue in Godot 4 where focus is stolen while the editor loads (or when a project starts), which causes some keypresses to be released if you're currently focusing on another window. I initially thought this was due to use of Window.request_attention()
somewhere, but it's not – it still occurs if you make Window.request_attention()
a no-op in the engine source code.
A perfect example of this is when you try to run diagonally in a game while the Godot editor loads, but one of the movement keypresses will be released after the editor is done loading. This PR fixes this issue completely. Previously, I could reproduce this pretty much every time, but I've tried 10+ times with this PR and couldn't reproduce the issue once.
I tested the editor quickly and couldn't notice any regressions in editor popup behavior.
Before
On both videos, I'm not releasing any keys while running.
simplescreenrecorder-2024-01-17_19.54.53.mp4
After (this PR)
simplescreenrecorder-2024-01-17_19.54.31.mp4
Thanks! |
Cherry-picked for 4.2.2. |
@YuriSizov God's work cherry picking this commit before 4.3. Thank you!! |
This is taking the changes from https://github.com/Mequam/funky-godot/tree/mequam/feat/xfce4_focus_grab and turning them into a PR (as @Mequam, the original author, requested on #74378)
Unfortunately, these changes don't fix the issue I am experiencing, but since others (including the original author) wanted to see these changes as a PR, I've gone ahead and created one. :-) It's my understanding that these changes do fix some of the problems from that issue.