godot icon indicating copy to clipboard operation
godot copied to clipboard

Godot Editor and Project Manager steals focus on a window manager on Linux.

Open nargacu83 opened this issue 3 years ago • 51 comments


Bugsquad note: This issue has been confirmed several times already. No need to confirm it further.


Godot version

v4.0.beta4.official [e6751549c] (any 4.0 version)

System information

Arch Linux with kernel 6.0.7-arch1-1 on AwesomeWM, X11

Issue description

The project manager and the editor steals the focus on Window Managers on Linux with X11 when opening other programs or when changing layouts. It doesn't happen on Godot 3.x.

I tested it on AwesomeWM, Hypr, LeftWM and DWM. Only LeftWM and DWM doesn't have the issue but i'm pretty sure they don't "implement" ConfigureNotify but i don't know much in the display server area.

However i managed to "fix" the issue by commenting out this if statement. https://github.com/godotengine/godot/blob/6882890a34528e056f13020798f4a473046990bb/platform/linuxbsd/x11/display_server_x11.cpp#L3855-L3857

Steps to reproduce

  1. Login with AwesomeWM
  2. Be on the awful.layout.suit.tile layout
  3. Launch any version of Godot 4.0
  4. Launch another program

Minimal reproduction project

You can use the default configuration of AwesomeWM. No project needed.

nargacu83 avatar Nov 05 '22 21:11 nargacu83

I can also confirm this bug for Fluxbox on Debian 10.

LXDE on Debian 11 works as expected.

The fastest way to reproduce it is opening Godot with an empty project list. The project list window and the "open assets store" popup fight for the focus.

fjfnaranjo avatar Nov 07 '22 10:11 fjfnaranjo

I can confirm this issue for Godot 4.0-beta10 with XMonad 0.17.1 on NixOS 2.11.

Expected and actual behavior is shown below. I have configured XMonad to draw a bright green border around the currently focused window.

Expected Behavior (Godot 3.4-stable) Actual Behavior (Godot 4.0-beta10)
focus-right focus-wrong

As can be seen above, the correct window is focused (terminal window has a bright green border), but keyboard input is going to the unfocused Godot window.

The issue also occurs when launching Godot with the --single-window option.

lihop avatar Dec 27 '22 21:12 lihop

Some other impacts of this issue:

  • Input focus is stolen from newly created windows (for example, when opening a new terminal window in the workspace the new terminal window should have focus). In this case the new window has focus, but input goes to Godot.

  • Input focus is stolen when resizing windows. Again, the window that had focus before resizing still has focus, but input goes to Godot.

    Expected (Godot 3.5) Actual (Godot 4.0-beta12)
    good bad

In both cases, clicking the window that should have focus does not allow it to regain input focus as the window manager already considers the window to be focused. Instead you have to click another window so that it loses focus and then click back to the target window so it regains full focus (including input focus).

lihop avatar Jan 14 '23 21:01 lihop

After studying the problem a little bit, it seems like the if i commented out doesn't impact any editor behavior neither the game windows behavior. By looking at the XSetInputFocus function definition and comments, it only ensure that the focus isn't lost. I have no clue what really that means in the context of most Desktop Environments or Window Managers, maybe you can lose the focus on a more barebone WM or DE that doesn't have it's focus system or something like it?

The problem doesn't happen on KDE Plasma with the Bismuth addon, so that's my temporary solution for now.

I'm trying to do a fix for it but unsure how to produce a test for this or if it's even necessary to have this piece of code in here, since the ConfigureNotify event mostly informs godot that it's geometry has changed, which is normal in a non floating window manager i guess. After testing it on AwesomeWM and KDE Plasma without the addon, it seems to work fine.

If we can't just remove it, maybe we can fix this by making sure one of the parent windows doesn't already have the focus before executing the function?

nargacu83 avatar Jan 16 '23 22:01 nargacu83

I want to report the issue still happens in the RC1 for 4.0. I will keep track of this issue and check again in 4.1. I will stick to the latest stable (3.x) for now and if I need 4.x for a project I already checked the next patch fixes the issue for me:

diff --git a/platform/linuxbsd/x11/display_server_x11.cpp b/platform/linuxbsd/x11/display_server_x11.cpp
index 525c62fbf2..0f741ef035 100644
--- a/platform/linuxbsd/x11/display_server_x11.cpp
+++ b/platform/linuxbsd/x11/display_server_x11.cpp
@@ -4215,7 +4215,7 @@ void DisplayServerX11::process_events() {
 				// RevertToPointerRoot is used to make sure we don't lose all focus in case
 				// a subwindow and its parent are both destroyed.
 				if ((xwa.map_state == IsViewable) && !wd.no_focus && !wd.is_popup) {
-					XSetInputFocus(x11_display, wd.x11_window, RevertToPointerRoot, CurrentTime);
+					//XSetInputFocus(x11_display, wd.x11_window, RevertToPointerRoot, CurrentTime);
 				}
 
 				_window_changed(&event);

As @nargacu83 pointed out, the challenge for this is understanding whats that line trying to do exactly and why it was necessary in the first place. I worry removing it may produce a regression if we don't try at least the 5/6 most common WM for X11.

fjfnaranjo avatar Feb 09 '23 19:02 fjfnaranjo

Reporting that this bug is present on:

  • Godot version: 4.1.dev.custom_build.4c677c88e
  • Arch Linux version: Linux version 6.1.7-arch1-1 (linux@archlinux) (gcc (GCC) 12.2.1 20230111, GNU ld (GNU Binutils) 2.40)
  • With a Fluxbox-only X11 launched via xinit.

For me, this issue is a hard blocker for using Godot4 because even in single-window mode, the editor and the debugger fight for focus when running the game via F5, which is a core part of my workflow when prototyping in GDScript.

Also, while the two windows are fighting for focus, they steal all input, so I can't even use an xbindkeys binding to kill the engine or Fluxbox's keybindings to kill X11, and I have to force shutdown my computer.

Currently using this shell script to test:

!#/bin/sh
godot4 &
sleep 60
pkill -x godot4

Commenting out the conditional in display_server_x11.cpp that @nargacu83 identified appears to fix the problem, but I agree that there must be a reason why it's there, so I second the desire for more thorough testing.

I don't know enough C++ to contribute any code, but I would be happy to help with regression testing if someone else submits a PR.

Let me know if there is anything specific that I can help test. :)

GrammAcc avatar May 25 '23 22:05 GrammAcc

Seeing that @bruvzg is assigned, if you need further details on this, feel free to ask. I'm also available on Discord if you need to discuss it.

If you want to get to an old commit when the majority of the design for the definition file is done, you are probably looking at 4396e98834f159da59ec790f2ff64fb65dacd9ce .

DEBUG_LOG_X11 can help you a lot. Adding more lines to that log will probably help to future developers.

You are also probably interested in an application called xev. Is a simple X11 test program that opens a window and logs to the standard output its X11 events as sent by the window manager.

@reduz deserves a shout out for that implementation. It works very well, handles lots of edge cases and it looks like he was losing his mind in the process:

https://github.com/godotengine/godot/blob/2eec9a67d564b11326f44c5ef8b6b6f9aec251b9/platform/linuxbsd/x11/display_server_x11.cpp#LL4322C7-L4322C85

fjfnaranjo avatar May 26 '23 07:05 fjfnaranjo

Still breaks everything. Arch Linux, X11, Fluxbox Godot 3+ and 4.1

I didn't open a new report for 4.1

br3eze avatar Jul 08 '23 08:07 br3eze

Godot 3+ and 4.1

Looks like a different bug. Godot 3+ was fine in my system (also X11+Fluxbox). Maybe they backported the changes from 4, but I don't think so. Are you experiencing the same behavior? The easiest way to reproduce this is opening the editor with a empty project list.

fjfnaranjo avatar Jul 08 '23 11:07 fjfnaranjo

I'm sorry, it's not about 3.5.2 (checked). My mistake.

Tried: Godot_v4.0.3-stable_linux.x86_64 Godot_v4.1-stable_linux.x86_64

There were no problems a ~ week or two ago. But now I just can't create or open projects. Main window and dialog starts blinking. It hangs whole WM, until killall Godot.... Got all Arch updates.

br3eze avatar Jul 08 '23 12:07 br3eze

So, if it kills all the thing, why is it there??? Just can't understand.

XSetInputFocus(x11_display, wd.x11_window, RevertToPointerRoot, CurrentTime);

I mean 4.1.1 still has this issue..

br3eze avatar Jul 21 '23 07:07 br3eze

So, if it kills all the thing, why is it there???

Well, i may have an answer for that. If i understood correctly. It appears to handle an internal focus state. Godot will check if any window and it's parent should have the focus. The problem here, is this focus state is not in sync with the WM focus.

nargacu83 avatar Jul 21 '23 10:07 nargacu83

Just wanted to leave my two cents as this issue makes godot nearly unusable for me as someone who is reliant on window managers to be able to use my computer without my hands exploding in a comical puff of dust.

If the presence of lines 3855 ~ 3857 in display_server_x11.cpp causes these issues, and we aren't clear on if removing them is a regression, surely removing the lines and seeing what the bug reports that rise from that look like would be nominally more useful than blindly trying to figure out how to fix this?

Idk, definitely a small-scope take from me that likely won't work in a project as big as Godot. Just really hope to see this fixed and to not have to deal with using a bespoke fork on all my linux machines.

crestofthebeast avatar Jul 29 '23 07:07 crestofthebeast

surely removing the lines and seeing what the bug reports that rise from that look like would be nominally more useful than blindly trying to figure out how to fix this?

This is fine for any of us to do in our local builds. And it will certainly be one of the steps tried by whoever tackles the issue. But Godot is used by lots of people and using them as guinea pigs for debugging is not good engineering and it will hurt the project.

I automated the creation of the fork a long time ago and I suggest you do the same. This doesn't affects many users and looks insanely complex.

If they ever fix it they will be doing us a favor. With only the benefit of one of them training on X11. And X11 is dying (sadly, because my hands also explode if my carefully keyboard-based crafted environment changes).

There is also the option of the bounty... But I guess it will be also difficult to find someone for it.

fjfnaranjo avatar Jul 29 '23 07:07 fjfnaranjo

Just in case anyone finds it useful, I created a fork that includes the workaround patch so that affected users that aren't comfortable with editing the C++ code can start using Godot 4 without maintaining a patched version of the engine themselves.

https://github.com/GrammAcc/godot4-workaround-68305

It's in the README on the fork, but just for the sake of CYA, I'll reiterate it here: This fork is not intended for use in production. Since we don't know what kind of regressions the workaround might introduce, you should NOT export any projects to production using this fork.

I can't contribute much to the engine itself, but hopefully this is helpful. :)

GrammAcc avatar Jul 29 '23 17:07 GrammAcc

Just in case anyone finds it useful, I created a fork that includes the workaround patch so that affected users that aren't comfortable with editing the C++ code can start using Godot 4 without maintaining a patched version of the engine themselves.

https://github.com/GrammAcc/godot4-workaround-68305

It's in the README on the fork, but just for the sake of CYA, I'll reiterate it here: This fork is not intended for use in production. Since we don't know what kind of regressions the workaround might introduce, you should NOT export any projects to production using this fork.

I can't contribute much to the engine itself, but hopefully this is helpful. :)

Will try developing on this fork and report any unexpected behaviour in the issues there -- can hopefully be useful to the assignee of this issue.

crestofthebeast avatar Jul 29 '23 23:07 crestofthebeast

Reporting a possible regression with the workaround identified in this thread.

When running the project with F5, everything seems to work when the editor is on the 2D, 3D, or Script editor tabs, but when running with the editor on the Asset Lib tab, the editor main window steals focus from the launched game window.

This isn't a serious problem since the game window can still be manually focused after it opens, but this is an indication that something is not working correctly with this workaround.

Additionally, there is a noticeable flicker in the main editor window when running via F5 on the 2D, 3D, and Script tabs, so I think they might still be vying for focus when the game window is opened and it just happens that the Asset Lib tab ends up with the focus where the others don't.

I'm not sure what the difference in the Asset Lib plugin and other editors is, but I wanted to pass this along since the difference in focus behavior between the different editor tabs might give a hint as to what is really going on here. :)

Tested this on versions: 4.1.1.stable.custom_build.d11b085c9 4.2.dev.custom_build.4714e9589 (using dev_mode=yes build option)

Linux version 6.4.10-arch1-1 (linux@archlinux) (gcc (GCC) 13.2.1 20230801, GNU ld (GNU Binutils) 2.41.0)

GrammAcc avatar Aug 19 '23 20:08 GrammAcc

Alright, i fixed the issue! Well mostly...

The editor and game window don't steal the focus anymore but it will sometimes. I can't figure out why exactly but that's all i got right now.

if you want to test it for yourself here's the commit: https://github.com/nargacu83/godot/commit/69fe46ee16a4caf23d7412f6b8807a6a88cdc997

nargacu83 avatar Sep 22 '23 20:09 nargacu83

Hi @nargacu83!

As your fix and issue is essentially the same as #74378 but the other author seems unresponsive I'll continue commenting here.

While this implementation also fixes my issue with moving and selecting windows in i3 (#80170), it does keep stealing focus occasionally when switching workspaces.

However it introduces another bug for me that is popups not correctly receiving focus.

For example Ctrl+P opens the search window, which seems correctly focused (based on i3wm border color) but still does not responds to keyboard input. I have to manually focus on the main window and back to the popup to have the keyboard work as it does without this patch.

unlessgames avatar Sep 26 '23 19:09 unlessgames

While this implementation also fixes my issue with moving and selecting windows in i3 (https://github.com/godotengine/godot/issues/80170), it does keep stealing focus occasionally when switching workspaces.

Thank you for taking the time to test it!

For example Ctrl+P opens the search window, which seems correctly focused (based on i3wm border color) but still does not responds to keyboard input. I have to manually focus on the main window and back to the popup to have the keyboard work as it does without this patch.

That's exactly the things i was talking about (minus the workspace switching, at least on my config).

I'm currently working on it, so expect a new version of the fix Soon™.

nargacu83 avatar Sep 26 '23 23:09 nargacu83

I haven't had a chance to test the latest patch by @nargacu83 yet, but I just wanted to give everyone a heads up that I just updated my fork with the original workaround included to version 4.1.2-stable if anyone is actually using it: https://github.com/GrammAcc/godot4-workaround-68305

To clarify, this only includes the original workaround of commenting out the couple lines originally identified in the display server implementation and not the patch that nargacu83 created a few weeks ago. :)

GrammAcc avatar Oct 11 '23 00:10 GrammAcc

Hi @GrammAcc, your workaround applied to the latest godot source fixes some of the focus steal issues on i3 for me while not introducing the same focus loss on popup windows, however a detached text editor wil stilll steal focus from a running game window and still occasionally prevents switching the workspace away from the one with godot.

I wonder about when can the case described above your edit happen?

// Set focus when menu window is re-used. // RevertToPointerRoot is used to make sure we don't lose all focus in case // a subwindow and its parent are both destroyed.

When is a window reused and when can a subwindow and its parent be destroyed? I'd appreciate any insight on this.

unlessgames avatar Oct 14 '23 11:10 unlessgames

I haven't got much time to work on it since last time, most of my attempts failed but i'm not familiar with C++ and with X11. If anyone wants to fix this issue, here's what i think.

I was trying to keep the focus order in sync without affecting the DE/WM focus. In my understanding, it does keep a focus order already but i wanted to check if the window wanting the focus is actually the correct one. Which could solve problems like @GrammAcc said, the editor can steal focus from other sub-windows like the game debug window.

This is just the one side of the problem since, as mentionned by @lihop @unlessgames, it also steals focus when changing tags/workspaces, that means implementing missing XDG event types or modify the implemented ones, so we may have more work to do in that area but who knows i may be overthinking this.

Also i recently went back to KDE Plasma on Wayland and noticed small focus issues too! It may not be only on Window Managers. (Could be a KDE issue as well). I'm really looking forward to see Godot Wayland to see if this issue will still be here though.

nargacu83 avatar Oct 22 '23 20:10 nargacu83

Sorry for ghosting this thread for a while. Been busy with work and internships. :)

@nargacu83 I'd be willing to take over on this one once I'm finished with the other issue I'm working on, but I'm not sure when that will be. I don't have a lot of free time these days, so I pretty much only work on open source stuff on Saturdays. It might be a couple more weeks before I can work on this one. Maybe even longer. :/

Hi @GrammAcc, your workaround applied to the latest godot source fixes some of the focus steal issues on i3 for me while not introducing the same focus loss on popup windows, however a detached text editor wil stilll steal focus from a running game window and still occasionally prevents switching the workspace away from the one with godot.

I wonder about when can the case described above your edit happen?

// Set focus when menu window is re-used. // RevertToPointerRoot is used to make sure we don't lose all focus in case // a subwindow and its parent are both destroyed.

When is a window reused and when can a subwindow and its parent be destroyed? I'd appreciate any insight on this.

I'm not sure what combination of events would result in that condition. I simply applied the workaround identified in this thread, so that other people wouldn't have to themselves. :sweat_smile: I'm also a C++ newbie and not very familiar with X11 either. That being said, I broke out the pickaxe and found the PR that adds the block in question: #41456, but it is from long before this issue was opened. I noticed these problems on my machine prior to this issue as well, but I wasn't using Godot4 until late 2022, so I'm not sure when this problem was actually introduced. When I get the chance, I will try building from the commit in that PR to see if this problem was introduced at that time, but I doubt it since it wasn't reported until recently. In any case, this should at least give us an idea of whether the root cause is the ConfigureNotify handler or if there is a deeper problem with the display server implementation on X11 causing events to get mishandled. :)

GrammAcc avatar Oct 23 '23 00:10 GrammAcc

Well I tried to test this out on https://github.com/godotengine/godot/pull/41456/commits/2b49cb0b7397e3ba14aede1d6d4d28bf17b816bd from #41456, but the third party libs in that commit aren't compiling on my machine. :upside_down_face:

The other issue I was working on got taken care of by another contributor, so I can start digging deeper into this one. It looks like there was quite a bit of discussion in #74378 regarding tiling WMs, and it seems the common ground between the two issues is that Godot steals focus when windows are resized or moved, even if window focus doesn't actually change. That would explain why commenting out the XSetInputFocus call in the ConfigureNotify event somewhat fixes the issue since that event is fired whenever the properties of a window change, which includes things like geometry and location xlib docs. Also, the problem that I have on Arch with fluxbox where the main editor window and any godot popup infinitely trade focus and lock my entire machine could also be caused by that due to the stacking order change being included in requests that trigger the ConfigureNotify handler.

The input focus section of the Xlib docs seems promising, but I don't have the time or energy to digest it right now.

One possibility for a root cause of all these issues that comes to mind is that the current implementation is actually correct as far as what should happen during each event, but due to the asynchronous nature of the client-server communication between X and each different UI application on the desktop, some events are being handled out of order resulting in the same kind of non-deterministic behavior that we get when we have race conditions in concurrent code. I will look into the way that X handles these different events, but if there is a sequence of actions that need to happen before an XSetInputFocus request would be safe to call, then possibly a simple 100-200 ms timer in the event handler could solve the problem, but I'm sure that's just wishful thinking. :laughing:

I will try to dig into this some more when I have more time, but it seems like an actual fix and not just a workaround will require digging deeper into how all of the events are being handled.

GrammAcc avatar Oct 30 '23 23:10 GrammAcc

Updated my workaround fork to latest stable version 4.1.3. It still works like the previous versions of the workaround on my machine. :)

I'm also reading up on the X Window System since I don't really understand how all the events are used or how they fit together yet. Will report back here if I figure out anything about the Godot implementation that might be useful.

GrammAcc avatar Nov 04 '23 19:11 GrammAcc

I have had very little time to work on Godot lately, and I've been preoccupied with a separate issue that was related to a PR I submitted previously, so I have not been able to look into this any more since my last comment at the beginning of November.

I'm starting to wonder if we should just submit the workaround as a PR and go from there since no one has reported any significant regressions with it and no one seems to have the time to really figure out the problem with X11 either?

It doesn't solve the workspace issues mentioned in https://github.com/godotengine/godot/issues/68305#issuecomment-1762851375, but without it, I can't even open Godot on my machine since any popup will cause a total lock of my machine and I have to force shutdown to fix it. I'm not sure if the bug is that severe on anyone else's setup, but considering the severity on some machines, I think it's probably worth it to just submit the workaround as a PR if we don't have the resources to research a full fix. I do worry that maybe there could be a regression on some other setup and we haven't heard about it since users on other setups aren't affected by the bug in the first place, but in the worst case, the PR reviewer will just reject the PR and we'll be back to scrounging for help with this bug.

@nargacu83 do you want to submit a PR for this since you originally identified the workaround of removing the XSetInputFocus call in the ConfigureNotify event handler? I can also submit the PR and credit you if you don't have the time/don't want to deal with the review and possible corrections. Just let me know. :)

GrammAcc avatar Dec 12 '23 02:12 GrammAcc

I will reply to this as I replied to: https://github.com/godotengine/godot/issues/68305#issuecomment-1656581417 ITT

We should not use neither the final Godot users nor the internal time of the reviewers to validate a workaround that we did not validated/understood ourselves.

I deal with this using a different WM when I work with Godot. Obviously is not ideal, but is not a "blocker" issue for that many systems if you add an alternative option to your DM/startx to launch LXDE (or any other working WM).

fjfnaranjo avatar Dec 12 '23 10:12 fjfnaranjo

I will reply to this as I replied to: #68305 (comment) ITT

We should not use neither the final Godot users nor the internal time of the reviewers to validate a workaround that we did not validated/understood ourselves.

I deal with this using a different WM when I work with Godot. Obviously is not ideal, but is not a "blocker" issue for that many systems if you add an alternative option to your DM/startx to launch LXDE (or any other working WM).

I was not trying to suggest that we use the users or maintainers to validate the workaround. I was under the assumption that all of the affected users in this thread are using this workaround, and none of them have reported any regressions for several months. I would consider this to be validation of the workaround in as much as I would consider a thorough fix validated. Ultimately, any fix we provide will need to be tested on other systems/setups to ensure it doesn't cause regressions.

I completely agree with you about using users/maintainers for validation, and I really want more testing before submitting something, but no one here has the time to really dig into this, and I'm not comfortable with the assumption that this critical bug is completely isolated from the rest of the system. This is a bug that causes total system lock on X11 with certain configurations. This might not affect users of Godot, but what about users of Godot games? We have to keep in mind that Godot games are the editor with some extra code included, and since it's a platform bug and not specifically an editor UI bug, it's entirely possible that someone loading up a popular title built with Godot4 on the Steam Deck or a Linux desktop could encounter the same bug when the game pops up a notification window about a Patreon or something.

This is a very contrived example, but I'm just trying to articulate my concerns with the current situation. I feel like sitting on this when we have a potential fix for the most critical portion of this bug is irresponsible, and I just don't have the time to properly investigate a full fix.

You make a completely valid point about not wasting the maintainers time though. This is a massive project, and they are very busy. I will ask about this in the Godot Contributors Chat to see what the experienced/core contributors think we should do.

I'll report back here once I get an answer. :)

GrammAcc avatar Dec 12 '23 14:12 GrammAcc

I agree, we should not do a PR about this unless we found and tested a real solution and not just a partial workaround on this issue. I'm sure we will find a suitable solution in the future. I will try to allocate some time to do a proper test on this issue to locate where and why the fix is needed. In the meantime my personal workaround has been to use a Wayland compositor or DE which the issue is isolated in XWayland (some annoyances with color picker or nodes selection modal for example).

nargacu83 avatar Dec 12 '23 14:12 nargacu83