window_get_popup_safe_rect error on popups in viewport

[essial]

Godot version

4.0.RC2

System information

Windows 10, M1 Macbook Air

Issue description

When closing a popup that was opened within a SubViewport, the following message appears:

E 0:00:05:0346   window_get_popup_safe_rect: Condition "!windows.has(p_window)" is true. Returning: Rect2i()
  <C++ Source>   platform/macos/display_server_macos.mm:3676 @ window_get_popup_safe_rect()

Note on windows, you get a similar error, but for display_server_windows.

Steps to reproduce

Put a button in a sub viewport.
Put a popup menu under the button.
Call popup() on the popup menu.
Click outside of the popup menu to close it.

Minimal reproduction project

PopupBug.zip

Upon running, click on the two buttons to show the two popups.
Notice the popup outside of the viewport doesn't throw an error, but the one inside it does.

[essial]

In the inner viewport, p_window = -1.
On line 255 of popup_menu.cpp, get_window_id() is returning this when called.
Inside of get_window_id(), embedder property is NULL, so it takes the value of window_id, which appears to be -1 in a subviewport.

For the normal popup (not inside the subviewport) the window id is 0.

[essial]

The only place window_id is set to anything other than -1 or 0 (main window) is in _make_window.

_make_window is called in _notification when the node enters the tree (NOTIFICATION_ENTER_TREE), but ONLY if it's not embedded.

Secondly however, if visible is set to true, the node is not currently visible, and the node is inside the tree, and it has an embedder (embedder_vp), and the window id is currently invalid(-1), it will generate a window ID.

I believe Window::_get_embedder() is the problem child here.
It calls get_parent_viewport(), and I think it's not working quite right for SubViewports.

So it looks like it's creating the OS window, but never setting the window ID.

[Sauermann]

While investigating PopupMenu::_parent_focused, I noticed multiple problems:

• *window_parent doesn't know what to do with SubViewports.
• window_get_popup_safe_rect only works for DisplayServer. For embedded Popups, this would need to be made available via Viewport, because Viewports now also perform the function of a Window-Manager.
• mouse_pos_adjusted doesn't account for Viewport::get_final_transform(), so Windows with a non-default transform will have problems with the positioning of the safe_area.

I believe Window::_get_embedder() is the problem child here.

I don't think, that this function has any problems related to this bugreport. It returns the Viewport, that this Window is embedded in, and nullptr, if there is no embedding Viewport.

[essial]

Ok sorry, first day in the codebase. Thanks so much for looking into it. Popups are used constantly (think Runescape) so it's a pretty bad error on our side.

If it's any help, getting the popup to show in the right spot (under the cursor) was also quite the challenge. This is called from a node inside of the subviewport (popup menu is a child of said node):

var viewport = get_viewport()
var camera = viewport.get_camera_2d()
popup_menu.set_position(
	(camera.get_global_mouse_position() * camera.zoom) + 
	(popup_menu.size / 2.0) + 
	viewport.canvas_transform.origin)