Editor: The popups selecting menu options on mouse move

[EmrysMyrddin]

Tested versions

Bug introduced by this commit: 06c2cda

System information

macOS 14.1, Apple M3 Pro

Issue description

When editing an exported Dictionary property, trying to change the type of a property opens a popup to chose the new type.

In fact, this seems to be the case of every popup menus. The right click menus for example have the same problem, if you maintain the right click and drag over a menu item, this menu item will be clicked once you start moving the mouse again after releasing the mouse button.

If the mouse doesn't immediatly move after clicking of the edit button, the popup will close itself when the mouse begins to move, using whatever type is under the mouse as the selected type.

Steps to reproduce

• Create a script with an exported dictionary
• Add this script to a node
• Open inspector for this Node
• Edit dictionary
• Click on the ✎ button
• Don't move the mouse for ~1s
• Move the mouse

Enregistrement.de.l.ecran.2024-02-14.a.14.43.47.mov

Minimal reproduction project (MRP)

N/A

[EmrysMyrddin]

I also notice a weird warning in the console, but this one was already showing up before the linked commit.

 scene/main/window.cpp:712 - Entering a window while a window is hovered should never happen in DisplayServer.

[KoBeWi]

Can't reproduce it on Windows.

[EmrysMyrddin]

After some tests:

• If the popup opens under the mouse (which is arguably a bad UI pattern but it exists in multiple places in Godot), moving the mouse after a 250ms delay will activate the menu item hovered by the mouse
• If the menu opens with a click (right or left) and the mouse button is not released before the mouse goes over the menu, the menu item hovered by the mouse after the button release will be activated on next mouse movement.
• The menu items are now activated on button down instead of button up, which means:
  • You can't click on an item and drag your mouse over another if you changed your mind
  • You can't cancel a click on a menu item by dragging outside of the menu popup

The 2 last items are very confusing since it's not how every native menus are working. It breaks users assumption about how to interact with the UI

[KoBeWi]

The first 2 are a bug. Items should activate only on button release, not mouse movement.

The last one is expected behavior. I don't know if it's OS difference, but that's how most apps work (e.g. VS Code, GIMP, Audacity etc.).

[AThousandShips]

If the popup opens under the mouse (which is arguably a bad UI pattern but it exists in multiple places in Godot)

See:

• No editor popup menus in the editor should popup with the cursor over an item godot-proposals#8883

[EmrysMyrddin]

@KoBeWi Hummm not sure I follow you ?

On Windows too, in a menu, if you maintain a mouse click on a menu item, you can either drag over another menu item, or outside the menu popup. Depending on the application, dragging outside cancel the click or closes the popup (I've seen both in multiple common app)

[ryevdokimov]

I just checked with a friend who uses mac and he says that this kind of functionality is used there as well, so this might have to do specifically with how Godot handles inputs on macOS. At least with the native mac menus this is how they work.

[KoBeWi]

Ah yes, dragging outside not closing menu is also a bug technically.
I meant that activating the menu (opening popup) on button down is intended, because that's what enables drag activation in the first place.

[EmrysMyrddin]

@KoBeWi Ah ok I see what you mean. But then this is only for buttons that opens a popup right ? Because other buttons, you can drag outside of the button to cancel click too.

[EmrysMyrddin]

I will try later if I can find out what differs between the macos and windows behavior. I have both at home, not at the office, so will check tonight or later this week.

[EmrysMyrddin]

If someone has a Linux to test if the bug is also there ?

[akien-mga]

I can confirm a regression / unexpected change in behavior on Linux with KDE Plasma on Wayland, using both touchpad and mouse.

When I right-click an element e.g. in the FileSystem dock (with the touchpad button), moving the touchpad triggers the selection of one of the item under the cursor, even though I did not tap/click. This is not the intended behavior for me when using a touchpad with tap-to-click enabled.

This is also reproducible with a mouse similarly: Right click on a folder, move the mouse to the right to attempt selecting an item, there's a high probability that one will be selected automatically.

It works if I hold right-click, but that's not feasible on a touchpad, and unconventional with a mouse, at least for me.

In either case, the item under the cursor is not selected on mouse release, but on the first movement after mouse release.

[ryevdokimov]

Strange. I just tested with a touchpad as well, and it works fine.

This will be difficult for me to debug since I don't have a mac or linux machine, although my theories based off this section of code in the PR: #86952

godot/scene/gui/popup_menu.cpp

Lines 574 to 576 in 06c2cda

	if (!initial_button_mask.has_flag(mouse_button_to_mask(MouseButton::LEFT)) && !initial_button_mask.has_flag(mouse_button_to_mask(MouseButton::RIGHT))) {
	mouse_is_pressed = false;
	}

Is that on macOS/linux something is different about how these flags or masks work. Maybe these flags are returning false when the cursor enters the popup menu element on these OS? I'd be curious at what point a breakpoint placed at line 575 would be triggered.

For me, it's only when left/right mouse is released regardless of where the cursor is (unless it's outside Godot, and which point it gets triggered immediately when the mouse reenters)

Edit: This is confirmed. When the breakpoint is triggered depends on the OS. Would need someone more familiar with the Input code to look at this if we want it resolved quickly.

[EmrysMyrddin]

Ok, I managed to compile Godot on my Windows machine, and I confirm that the original bug that I described in the issue is not reproduced for me.

The "one cycle click" feature is working both with menu opened via a button or via right click.

But I have the same behavior than on macOS concerning the trigger of action on click press instead of click release.
Perhaps I should update the body of the issue ? I think that those 2 bugs are tightly related.

[ryevdokimov]

I'm not sure I understand the 2nd issue. What is the undesired behavior found in the windows build?

[EmrysMyrddin]

@ryevdokimov Good catch, on Windows, we go into the if when the mouse button is released wherever the mouse is, as you said.

On macOS, we don't go into the if when the mouse button is released, but when go into it whenever the mouse enters the menu area.

[ryevdokimov]

Interesting.

Would need someone more familiar with the Input code to take a look at this if we want to solve this quickly. I am unfamiliar with that part of the code and couldn't guess why the discrepancy between OS would exist, and whether or not it's by design.

Did you see my question about the 2nd issue? Is there any behavior in the Windows build that is undesirable?

[EmrysMyrddin]

I'm not sure I understand the 2nd issue. What is the undesired behavior found in the windows build?

Since I'm not a native english person, it's a bit difficult for me to explain it clearly by text :-(

Let me try to make a video:

Enregistrement.de.l.ecran.2024-02-14.a.22.27.58.mov

Here, you can see the menu action is selected when I push the mouse button down.

The expected behavior is to trigger the action only on button up. And the selected menu action should then be the one under the mouse cursor when the button is released. If the cursor is out of the menu area when the button is released, it should close the menu (not every app do this, but I think it's actually a good idea).

Here an example of how VSCode menu is working:

Enregistrement.de.l.ecran.2024-02-14.a.22.32.48.mov

[ryevdokimov]

Ah I see. After you you've already clicked (but haven't selected an item), the behavior has regressed so that items are selected on mouse-down, which I agree is undesirable. This I can fix. Good catch.

[ryevdokimov]

Also, popup input handling seems to be extremely overcomplicated, not sure what it's trying to do at all, should not it just activate item on release?

Mouse up is not registered in the popup when mouse down is initiated to make the popup visible and then subsequently held. I wrote the code within that framework, but it now sounds like a Band-Aid to a bigger issue.

[bruvzg]

I guess the same code should be applied to mouse button events, since if mouse was pressed all events will be sent (at least by macOS) to the window that have received the press event, until its released. Otherwise, activation on release will never work.

[bruvzg]

A quick fix for macOS (and probably X11 as well, have not tested it) - #88366

I'll evaluate other event behavior on all desktop platforms tomorrow, it might be better to do the same redirection to all events.

[EmrysMyrddin]

In fact I do think that it makes sense to not get event happening outside of the pop up window.

But for me, the root problem is that the mouse button release event should be forwarded to the pop up window, even if the button down happened outside it.

Forwarding mouse mouvement events is just a workaround to make our feature work as it is, but we should not rely on mouse movement for this in the first place.

[kitbdev]

Mouse up is not registered in the popup when mouse down is initiated

This happens with all controls too, not just multi window stuff.

• related Mouse released event is called at the mouse press location instead of the release location #84466

[ryevdokimov]

This happens with all controls too, not just multi window stuff.

Good to know there is a rational for it at least.

I still don't fully understand why my PR doesn't fix this popup issue: #88342

Doesn't using DisplayServer check state regardless of what context the cursor is in? I hate to repeat this question, but trying to understand if there is another separate issue.

According to this: https://docs.godotengine.org/en/stable/classes/class_input.html#class-input-method-get-mouse-button-mask

This method in the Input class is equivalent, and the input class says that it checks global input state.

The reason I use DisplayServer in that PR is because I also check global position of the cursor using the same object, but yeah it shouldn't make a difference.

[EmrysMyrddin]

@ryevdokimov The problem is not the detection of left/right button state detection. This was already working fine actually :-)

The problem is that the mouse mouvement event are not received when the happen outside of the popup. So the problem is that the _input_from_window_internal function is not called until the mouse enters the popup area.

So what happens is:

• You mouse down on the button (the popup doesn't exists yet)
• The popup opens
• You release the mouse (no event is sent to the popup, so nothing happens)
• You move the mouse, you are still outside the popup since your mouse was on the button
• There, the behavior is not the same on all platforms:
  • Windows: an mouse movement event is sent to the popup, the _input_from_window_internal is called. We detect that the mouse moved and that left and right buttons are not pressed and the mouse position is outside the popup. This means that the "on click cycle" is disabled.
  • Unix: no mouse movement is sent, since it happened outside the popup.
• You continue to move the mouse and enter the popup area
• A mouse mouvement event is sent to the popup in all platforms, but since we are in 2 different states, different things happens:
  • Windows: We already have detected that we are not in a "one click cycle", the mouse movement event is ignored (just used for the hover effect)
  • Unix: We detect a movement with both left and right click released, and we are still waiting for the end of a potential "one click cycle". The code detects that we are inside the popup area, which is the condition to trigger the menu action. So the menu is selected.

Is it more clear ? I can try to make a video with some logs if it can help you visualize what happens in macOS :-) I know it's difficult to imagine this just from text

[akien-mga]

Please test #88392 and see if it works as expected on your platforms.

[ryevdokimov]

Please test #88392 and see if it works as expected on your platforms.

Works for me. All the issues it says it will fix seem to be fixed on Windows.

The problem is that the mouse mouvement event are not received when the happen outside of the popup. So the problem is that the _input_from_window_internal function is not called until the mouse enters the popup area.

The PR I was referring to doesn't check against mouse movement anymore to receive button state. It checks against the DisplayServer which I thought will allow capturing state globally from within the popup regardless of where the click was initiated. Since the problem seems to be resolved though I can ask about it separate from this post.

[EmrysMyrddin]

@ryevdokimov Yes, you don't rely on the event object to get the mouse state, but this doesn't change how the function _input_from_window_internal is called. It is called for any input events, including mouse mouvement events. And your were are relying on the fact the function is called on each mouse mouvement, regardless of how you detect mouse state during the event handling.

[EmrysMyrddin]

@akien-mga It is working on macOS :-)

But as I said, I'm not sure if it's a good fix? Is it expected to receive events that doesn't belongs to the window your control is in?

I do think that fixing #84466 is a better fix, it's a more expected behavior and would simplify the code of the "one cycle click" feature a lot :-)

[kitbdev]

I do think that fixing #84466 is a better fix, it's a more expected behavior and would simplify the code of the "one cycle click" feature a lot :-)

I tried to change mouse focus and fix #84466 for this, but it wouldn't work when single window mode was false. When holding the mouse down and moving it over the popup, it wouldn't receive the inputs. Mouse events just weren't being sent to the correct window from the DisplayServer, at least on Windows. See kitbdev@34a2f18

[DanielSnd]

Huh, I came here searching to see if there was already an issue for this, I see that other people aren't experiencing that on windows but I'm on Windows 11 and I'm experiencing it.

If I right click while moving my mouse to the right/down I reproduce it every time. Only way to properly use right clicks now is if I right click while moving my mouse to the left.

Can't reproduce on 4.2. I'm compiled from master as of 2 days ago.

[EmrysMyrddin]

It seems from the screen record you have that the popup menu takes a bit of time to be actually initialiased (it stays grey for a bit of time) It think that if your mouse enters the popup area before it is fully initialised, you are experiencing the exact same problem we have on macOS. It just have to take more than 250ms to fully initialised (before that, the activation of menus are disabled to avoid missclicks)

This is why the proper fix to this is to rely on mouse up event and to not rely on mouse mouvement for this :-)

And i suppose @DanielSnd that #88392 doesn't fix your problem ?

[DanielSnd]

And i suppose @DanielSnd that #88392 doesn't fix your problem ?

I hadn't tried before, I have now tried #88392 and I can confirm it does indeed fix my problem :)

Thank you!

[EmrysMyrddin]

That's weird ^^' I was expecting it to not fix your problem

[DanielSnd]

That's weird ^^' I was expecting it to not fix your problem

I think it didn't entirely, but did in parts? I just tried again after seeing your comment and this is how it's looking for me at the moment. It doesn't "click" on the place the mouse is, before it would be opening the add child window and such, now it seems to be dismissing the popup but not clicking things on it. But also not every time like it was before, can see in the start of the gif that many times the menu shows up just fine, while before it was reproducible 100% of the time.

I do feel this behaviour is different from when I tried and commented yesterday. I'm assuming since this does depend on how long the popup takes to load, when I tried and assumed the behaviour was 100% fixed my popups were loading faster and hiding the issue.

[WhalesState]

The last one is expected behavior. I don't know if it's OS difference, but that's how most apps work (e.g. VS Code, GIMP, Audacity etc.).

In VS Code and Audacity the items are activated when mouse button is up [Windows 10].

[kitbdev]

• Yeah, it's an OS difference, see Make Right Mouse Button Behavior Consistent with the Platform Being Used godot-proposals#7463 and Right Mouse Button Behavior is Inconsistent with Standard Windows Behavior #80346

[WhalesState]

Those issues was caused by #86952, it was an attempt to fix opening the popup and triggering an action in a single mouse click which wasn't working for 4.x because it's no longer embedding the subwindows by default for the editor. merging this #87462 which reverts the changes and this #88392 will fix those issues if the second one will make the non-embedded PopupMenu recieves the initial mouse release event after opening the Menu, but one of them will need to be rebased to fix the conflict in the popup_menu.cpp input function.

I have reverted the changes because of those issues:

• The ScrollBar can't be clicked. Clicking it will activate an item under it.
• Menu items gets selected when the mouse event is pressed instead of released. After fixing those two issues i have found another issue which will lead to the same fix needed before merging the pull request.
• When opening the popup and triggering an action in a single mouse click over a non-clickable item like [submenu or separator or disabled item] the drag_to_press won't become false because the window don't recieve the initial mouse release event when disabling embedded subwindows and will select the first selectable item when the mouse enters it.

[akien-mga]

So given the discussion so far and the multiple related PRs, I think it might be best to start with reverting #86952 to go back to the previous status quo.

Then we can evaluate @bruvzg's and @WhalesState's PRs, and if needed other ones to re-enable what #86952 was trying to achieve without regression. WDYT?

For any such change, we'll need to ensure we test on all platforms with both single-window and multi-window modes.

[bruvzg]

Before #86952 it was fully usable, and issues were mostly visual, so reverting it should be good enough.

#88392 fixes more stuff, but break drag-drop (probably can be fixed in the drag-and-drop code).

But I feel like the whole mouse input should be re-evaluated and changed, and maybe send all event to all windows (the global mouse events filters can be integrated as well, to remove native popup code from DisplaServer), and filter them based on window_id in place as necessary, this way popups and DnD can use all events.

[donn-xx]

Just to subscribe and confirm that this is happening on my system. It's almost impossible to use Godot right now!

v4.3.dev.custom_build [fb10e67]
Godot v4.3.dev (fb10e67) - Ubuntu 22.04.3 LTS 22.04 - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 (nvidia; 535.154.05) - Intel(R) Core(TM) i5-9400F CPU @ 2.90GHz (6 Threads)