Popup in 2D editor teleports for one frame

[KoBeWi]

Godot version

fb28025

System information

W10

Issue description

When clicking outside menu, it changes position to cursor and then gets closed immediately. Happens also on events that should open it again, e.g. right-clicking in another place will not prevent closing the popup.

It might have similar cause to #58726

Steps to reproduce

1. Right-click in 2D editor
2. Right-click somewhere else

Minimal reproduction project

No response

[taigi100]

I've looked over the issue a bit. I managed to find the line actually causing it, however - I have no clue how to fix this one:

Line 419 in scene_tree.cpp: MessageQueue::get_singleton()->flush(); //small little hack

It seems that the popup disappears during the Main::iteration. It stays on screen till then during debugging and disappears at the previously mentioned line. Now, I don't know why it disappears only when another popup is up. Maybe they have the same reference, and it's trying to delete it since it received a new similar event?

Edit: Actually, this explanation doesn't really make a lot of sense. It uses the same node add_node_menu which it clears and redraws on right click. The clearing line only appears once (on right mouse button pressed).

[KoBeWi]

Reopening, the PR only fixed right-click. It can still happen with the list tool.

[derammo]

This no longer reproduces for me with #60894.

However, I still observe #60921, which I will investigate.

[derammo]

By the way, it would make sense this would be affected by my changes in that PR. In the 4.0 code without --single-window, each popup is a window. And the window message handler was discarding the first 6-8 messages for each new window. Now it discards 0.

It is not surprising if fixing that makes some other problems go away.

[derammo]

I am testing with the right mouse button re-enabled, reverting the PR that took it out.

This seems like a sequence problem, with a deferred operation triggering itself in the popup code. The first popup gets closed twice, but in the meantime, it has been reused and moved to the new location. Like so (not all frames shown):

OS_Windows::run
    process messages
        MouseProc right click
            _send_window_event WINDOW_EVENT_CLOSE_REQUEST
                Window::_propagate_window_notification NOTIFICATION_WM_CLOSE_REQUEST
                    PopupMenu::_notification
                        Popup::_notification
                            Popup::_close_pressed
                                call_deferred this->Window::hide
    Main::iteration
        Scene_tree::physics_process
            flush dispatches Window::hide
                Window::set_visible (false)
                    Window::_clear_window
                        DisplayServerWindows::delete_sub_window
                            DisplayServerWindows::popup_close
                                _send_window_event WINDOW_EVENT_CLOSE_REQUEST
                                    Window::_propagate_window_notification NOTIFICATION_WM_CLOSE_REQUEST
                                        PopupMenu::_notification
                                            Popup::_notification
                                                Popup::_close_pressed
                                                    call_deferred this->Window::hide

OS_Windows::run
    process messages
        WndProc right click
            Input::parse_input_event
                buffered_events.push_back
    Main::iteration
        Input::flush_buffered_events
            long way through the gui ...
                PopupMenu::popup
                    Window::popup
                        Window::set_visible (true)
                            window set visible in new location
        Scene_tree::physics_process
            flush dispatches Window::hide from previous sequence
                Window::set_visible (false)
                    window was reused in the time we were async, close it again

Now to find a clean fix. The challenge is that there are multiple ways that popups are called, sometimes the window is set to hidden from code, other times the MouseProc calls directly into it. With the current structure, I haven't found a clean way to prevent the asynchronous recursion. The popup code doesn't really know in which context it is used, whether it is responsible for scheduling the Window::hide. I am trying to find a fix for that without restructuring.

[derammo]

I have a fix, just waiting on PR in the same area to go in first for clean rebase.