godot
godot copied to clipboard
Wayland editor documentation popups appear on screen's edge and generate errors
Tested versions
- Reproducible in v4.5.dev.custom_build [3b963ab8b]
- Not reproducible in v4.5-dev5
System information
Godot v4.5.dev (d9cd011e2) - Manjaro Linux #1 SMP PREEMPT_DYNAMIC Wed, 14 May 2025 14:56:34 +0000 on Wayland - Wayland display driver, Multi-window, 2 monitors - Vulkan (Forward+) - dedicated AMD Radeon RX 7900 XT (RADV NAVI31) - AMD Ryzen 9 7900X 12-Core Processor (24 threads) - 30.49 GiB memory
Issue description
Every time a class or function is hovered and its documentation popup appears, the popup is placed against the screen's right edge, and the following error is displayed in the output panel:
ERROR: platform/linuxbsd/wayland/display_server_wayland.cpp:1052 - Condition "p_window_id != MAIN_WINDOW_ID" is true. Returning: INVALID_SCREEN
This error points to this function.
In the following screenshot, I am hovering the rotated method:
Steps to reproduce
Open any script, hover a documented class or function (custom or built-in).
Note that disabling multi-window support (or enabling single window mode) causes the popup to appear at the correct location, but still generates the error.
Minimal reproduction project (MRP)
N/A
CC @Riteo
Popup windows rely excessively on looking up what the screen which contains the window is. On Wayland as you'll have noticed this fails because we don't know which screen the window is mostly in so we just say nothing.
Besides, the popup code has a lot of duplicated code to get the "parent" rect, I will try to make a PR where reliance on screen is reduced enough to not have this by replacing a bunch of calls to window_get_current_screen to a helper method which hopefully behaves better
It's probably related to window fitting, which should be checked with the new feature flag. I think I can take a look soon.
I'm sure that this is simply related to #104907. For consistency with other platforms it returns INVALID_SCREEN for anything but the main window. I don't know the details but IMO we should either always return a "valid" value or simply make sure that no code hard depends on the current screen.
Given that Wayland never had reliable way of detecting the current screen I'd argue that we should always return a fixed value and never error out, while making sure that all users of similar methods always have a fall-back where possible.
Oh right. @bruvzg, since you made the PR in question, do you have some tips on what would be the best approach?
I think that it only showed the underlying issue. I clearly did not find all fitting attempts and disable them but I wonder if we could somehow bake a fall-back or something. You know the screen API best though so that's why I'm pinging you.
In order to try something I have just trivially reordered some logic so that when trying to get the parent rect (I assume this means the area the popup can expand into) we check if SELF_FITTING_WINDOWS is implemented before trying the screen.
This is not a solution for every other place where we try to use the screen for something so the fallback would be helpful in every other case where either we (or a godot user) haven't checked against SELF_FITTING_WINDOWS.
I don't know the details but IMO we should either always return a "valid" value or simply make sure that no code hard depends on the current screen.
Whatever is using is probably should check for INVALID_SCREEN and ignore it. Simply returning 0 for all windows is not good, since code calling it can use real screen zero size for some calculation which might be completely wrong.
Also, Wayland seems to have both window_get_position and screen_get_position/screen_get_size implemented, isn't it enough to infer current window screen?
Whatever is using is probably should check for INVALID_SCREEN and ignore it. Simply returning 0 for all windows is not good, since code calling it can use real screen zero size for some calculation which might be completely wrong.
That makes sense. Although I think that in Wayland's case we should remove the ERR_FAIL_COND_V macro then.
Also, Wayland seems to have both window_get_position and screen_get_position/screen_get_size implemented, isn't it enough to infer current window screen?
Window position is "relative" in a way. window_get_position returns (0,0) for toplevels but its position relative to the parent for popups. You can know the size of the screens and what screens you're on but not onto which you're on the most. I tried a lot in the past but it looks like that's limited by design.
Although I think that in Wayland's case we should remove the ERR_FAIL_COND_V macro then.
Yes, this make sense to remove it, since it's not an error, just indeterminate state.
Window position is "relative" in a way. window_get_position returns (0,0) for toplevels but its position relative to the parent for popups.
This should be mentioned in the docs, since on all other platforms it's in absolute screen coordinates.
All right I finally took a look.
I can't seem to replicate the popup appearing on the screen's edge neither on sway, weston or kwin. I think that I might need some more info, such as your monitor setup and the compositor you're using.
When it comes to the error, it's apparently very simple. The Wayland backend got multiwindow support basically at the same time as #104907 got merged. The result is that we now have a single-window-only check (p_window_id != MAIN_WINDOW_ID) while instead it should have !windows.has(p_window_id). The fix is quite trivial (will file a PR soon):
Patch
diff --git a/platform/linuxbsd/wayland/display_server_wayland.cpp b/platform/linuxbsd/wayland/display_server_wayland.cpp
index 3739ba0ff1..39f87e020e 100644
--- a/platform/linuxbsd/wayland/display_server_wayland.cpp
+++ b/platform/linuxbsd/wayland/display_server_wayland.cpp
@@ -1049,7 +1049,7 @@ void DisplayServerWayland::window_set_drop_files_callback(const Callable &p_call
}
int DisplayServerWayland::window_get_current_screen(DisplayServer::WindowID p_window_id) const {
- ERR_FAIL_COND_V(p_window_id != MAIN_WINDOW_ID, INVALID_SCREEN);
+ ERR_FAIL_COND_V(!windows.has(p_window_id), INVALID_SCREEN);
// Standard Wayland APIs don't support getting the screen of a window.
return 0;
}
Hopefully that fixes the issue (please lemme know), although I have a feeling that something more complicated might be going on. The current behavior is obviously wrong but it's kinda weird that it broke out of the blue. If we can figure out what's going on, we can at least hopefully whip a quick workaround in the short term.
In the meanwhile I'll look into returning and handling INVALID_SCREEN properly everywhere but that will require some elbow grease since it's a new API which must be retrofitted everywhere.
I can confirm this patch removes the error, that's a first step :)
I of course still have the popups appearing against the right edge of the screen, with the following setup:
- KDE Plasma on Wayland, KWin compositor
- 2 monitors (primary monitor on the right), Godot on the right monitor (but even if I move it to the left monitor, popups appear on the right edge of that screen)
- Issue does not occur in Single Window Mode, but otherwise does occur regardless of Multi Window state (which seems to be different from what I originally reported about Multi Window).
- If I make the editor a floating window instead of maximized, the popup will still go all the way to the screen's right edge.
Actually, I tested a bit more with a floating window: when I place the window almost centered between the 2 screens, if it's considered to be on the left screen, the popup appears on the edge of that screen, but if it's considered to be on the right screen, the popup doesn't go all the way to the edge, I'm not sure what dictates how far it goes in that situation...
@Cykyrios that's really weird, we can't spawn popups that far and we have no screen awareness. I'm wondering if this is a compositor thing.
I think I might need a full debug log. Could you please replicate the issue while running the following command and send to me the resulting file?
WAYLAND_DEBUG=1 path/to/godot --verbose 2>&1 | tee debug.log
Please note that this is a full protocol dump and will include any keystrokes and other input received only by the editor (not the whole OS - clipboard data is also out-of-band). Shouldn't matter in this case but I gotta let you know for transparency.
Here's the debug log, I tried to be quick to keep the size reasonable, but it's still 25k lines. I made the window non-maximized in the previous session to gain some time, hovered a function as soon as the editor opened (with the window centered), and the popup appeared on the edge of the screen; I then moved the window so it was on both screens, but still considered on the right screen, and hovered a function again (popup offset but not against the edge of the screen), then I exited the editor.
Thank you! Unfortunately I only see a single "making popup" entry but that's enough luckily. It really looks like it's setting an horizontal offset of 1920 to the right, for some reason. I can only assume that kwin is clamping it to the current screen, hence why it "snaps" on the right side in the left screen and doesn't quite do that when done in the middle.
It looks like we both have a dual 1920x1080 display setup yet I can't replicate it, that's so weird... I'll take a closer look later.
Hi, I'm still somewhat unsure on why exactly that is happening. That said, at this point I'm going to just fix the root cause by adapting the editor to accept the new INVALID_SCREEN constant.
In the meantime, I think I found a workaround. Could you please test it?
diff --git a/editor/editor_help.cpp b/editor/editor_help.cpp
index 14c1e6630d..3226de1971 100644
--- a/editor/editor_help.cpp
+++ b/editor/editor_help.cpp
@@ -4656,26 +4656,28 @@ void EditorHelpBitTooltip::popup_under_cursor() {
vr = window->get_usable_parent_rect();
}
- if (r.size.x + r.position.x > vr.size.x + vr.position.x) {
- // Place it in the opposite direction. If it fails, just hug the border.
- r.position.x = mouse_pos.x - r.size.x - tooltip_offset.x;
+ if (!DisplayServer::get_singleton()->has_feature(DisplayServer::FEATURE_SELF_FITTING_WINDOWS)) {
+ if (r.size.x + r.position.x > vr.size.x + vr.position.x) {
+ // Place it in the opposite direction. If it fails, just hug the border.
+ r.position.x = mouse_pos.x - r.size.x - tooltip_offset.x;
- if (r.position.x < vr.position.x) {
- r.position.x = vr.position.x + vr.size.x - r.size.x;
+ if (r.position.x < vr.position.x) {
+ r.position.x = vr.position.x + vr.size.x - r.size.x;
+ }
+ } else if (r.position.x < vr.position.x) {
+ r.position.x = vr.position.x;
}
- } else if (r.position.x < vr.position.x) {
- r.position.x = vr.position.x;
- }
- if (r.size.y + r.position.y > vr.size.y + vr.position.y) {
- // Same as above.
- r.position.y = mouse_pos.y - r.size.y - tooltip_offset.y;
+ if (r.size.y + r.position.y > vr.size.y + vr.position.y) {
+ // Same as above.
+ r.position.y = mouse_pos.y - r.size.y - tooltip_offset.y;
- if (r.position.y < vr.position.y) {
- r.position.y = vr.position.y + vr.size.y - r.size.y;
+ if (r.position.y < vr.position.y) {
+ r.position.y = vr.position.y + vr.size.y - r.size.y;
+ }
+ } else if (r.position.y < vr.position.y) {
+ r.position.y = vr.position.y;
}
- } else if (r.position.y < vr.position.y) {
- r.position.y = vr.position.y;
}
// When `FLAG_POPUP` is false, it prevents the editor from losing focus when displaying the tooltip.
diff --git a/scene/main/viewport.cpp b/scene/main/viewport.cpp
index 3d2dd6dcc7..1aa5093a74 100644
--- a/scene/main/viewport.cpp
+++ b/scene/main/viewport.cpp
@@ -1678,26 +1678,28 @@ void Viewport::_gui_show_tooltip_at(const Point2i &p_pos) {
r.size = r.size.ceil();
r.size = r.size.min(panel->get_max_size());
- if (r.size.x + r.position.x > vr.size.x + vr.position.x) {
- // Place it in the opposite direction. If it fails, just hug the border.
- r.position.x = gui.tooltip_pos.x - r.size.x - tooltip_offset.x;
+ if (!DisplayServer::get_singleton()->has_feature(DisplayServer::FEATURE_SELF_FITTING_WINDOWS)) {
+ if (r.size.x + r.position.x > vr.size.x + vr.position.x) {
+ // Place it in the opposite direction. If it fails, just hug the border.
+ r.position.x = gui.tooltip_pos.x - r.size.x - tooltip_offset.x;
- if (r.position.x < vr.position.x) {
- r.position.x = vr.position.x + vr.size.x - r.size.x;
+ if (r.position.x < vr.position.x) {
+ r.position.x = vr.position.x + vr.size.x - r.size.x;
+ }
+ } else if (r.position.x < vr.position.x) {
+ r.position.x = vr.position.x;
}
- } else if (r.position.x < vr.position.x) {
- r.position.x = vr.position.x;
- }
- if (r.size.y + r.position.y > vr.size.y + vr.position.y) {
- // Same as above.
- r.position.y = gui.tooltip_pos.y - r.size.y - tooltip_offset.y;
+ if (r.size.y + r.position.y > vr.size.y + vr.position.y) {
+ // Same as above.
+ r.position.y = gui.tooltip_pos.y - r.size.y - tooltip_offset.y;
- if (r.position.y < vr.position.y) {
- r.position.y = vr.position.y + vr.size.y - r.size.y;
+ if (r.position.y < vr.position.y) {
+ r.position.y = vr.position.y + vr.size.y - r.size.y;
+ }
+ } else if (r.position.y < vr.position.y) {
+ r.position.y = vr.position.y;
}
- } else if (r.position.y < vr.position.y) {
- r.position.y = vr.position.y;
}
DisplayServer::WindowID active_popup = DisplayServer::get_singleton()->window_get_active_popup();
Looks like the popups are behaving now :) and not just in the script editor, all popups are now properly appearing next to the cursor; they're also still properly hugging the edge when too far to the right.
Yay, thank you for testing! I'll whip up a quick PR soon with both fixes.