Skip to content

Wayland editor documentation popups appear on screen's edge and generate errors #107438

Closed

Description

@Cykyrios

Tested versions

  • Reproducible in v4.5.dev.custom_build [3b963ab]
  • Not reproducible in v4.5-dev5

System information

Godot v4.5.dev (d9cd011) - 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:

Image

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

Activity

  1. added this to the 4.5 milestone on Jun 12, 2025
  2. akien-mga commented on Jun 12, 2025

    @akien-mga
    Member
  3. ArchercatNEO commented on Jun 12, 2025

    @ArchercatNEO
    Contributor

    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

  4. deralmas commented on Jun 12, 2025

    @deralmas
    Member

    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.

  5. deralmas commented on Jun 12, 2025

    @deralmas
    Member

    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.

  6. 3 remaining items

  7. bruvzg commented on Jun 12, 2025

    @bruvzg
    Member

    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.

  8. bruvzg commented on Jun 12, 2025

    @bruvzg
    Member

    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.

  9. self-assigned this
    on Jul 3, 2025
  10. deralmas commented on Jul 11, 2025

    @deralmas
    Member

    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.

  11. Cykyrios commented on Jul 11, 2025

    @Cykyrios
    ContributorAuthor

    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...

    Image Image
  12. deralmas commented on Jul 11, 2025

    @deralmas
    Member

    @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.

  13. Cykyrios commented on Jul 11, 2025

    @Cykyrios
    ContributorAuthor

    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.

    debug.log

  14. deralmas commented on Jul 11, 2025

    @deralmas
    Member

    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.

  15. deralmas commented on Jul 14, 2025

    @deralmas
    Member

    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();
  16. moved this from For team assessment to In progress / Assigned in Platforms Issue Triageon Jul 14, 2025
  17. Cykyrios commented on Jul 14, 2025

    @Cykyrios
    ContributorAuthor

    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.

  18. deralmas commented on Jul 14, 2025

    @deralmas
    Member

    Yay, thank you for testing! I'll whip up a quick PR soon with both fixes.

  19. moved this from In progress / Assigned to Done in Platforms Issue Triageon Jul 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions