Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popup menu does not popup at indicated location #95151

Closed
blackears opened this issue Aug 5, 2024 · 6 comments · Fixed by #95164
Closed

Popup menu does not popup at indicated location #95151

blackears opened this issue Aug 5, 2024 · 6 comments · Fixed by #95164

Comments

@blackears
Copy link

Tested versions

v4.2.1.stable.official [b09f793]

System information

v4.2.1.stable.official [b09f793]

Issue description

I am trying to write a script that causes a PopupMenu to open at the location where the user right clicks the mouse. This is an editor addon, and the popup menu is opening nowhere near the point I'm indicating. In my original program, it wasn't even opening on the correct monitor. In the test case I'm attaching, it is opening on the correct monitor, but no where near the point where the mouse is located.

Steps to reproduce

Install the attached addon to the Godot editor. Expand the Popup Test tab in the bottom center dock and right click in the window area. My PopupMenu will be displayed, but in the top left hand corner of the screen. I've tried lots of combinations of popup(), popup_on_parent() and show(), but the popup menu always opens relative to the top left corner of the screen, not the control.

Minimal reproduction project (MRP)

popup_location.zip

@AThousandShips
Copy link
Member

Please try with 4.2.2 to make sure this hasn't already been fixed in a supported version

@detomon
Copy link
Contributor

detomon commented Aug 5, 2024

Events in _gui_input are in local coordinates. PopupMenu.popup() expects global coordinates. Changing the code to something like this should work:

%PopupMenu.popup(Rect2i(get_global_transform() * e.position, Vector2i.ZERO))

@Calinou
Copy link
Member

Calinou commented Aug 5, 2024

PopupMenu.popup() expects global coordinates. Changing the code to something like this should work:

Note that this varies depending on whether embedded subwindows are enabled in the Project Settings (or single-window mode in the Editor Settings). This can certainly catch users off-guard, which makes me wonder if we should have a popup_relative() method that always makes the coordinates relative to the main window's top-left corner. This way, you can have consistent operation regardless of whether single-window mode is enabled.

@blackears
Copy link
Author

I've upgraded to 4.2.2 and am now using:

%PopupMenu.popup(Rect2i(get_global_transform() * e.position, Vector2i.ZERO))

While this is now giving me the correct vertical position on my popped up window, the popup is on the far left of the monitor regardless of the X position of the mouse. And in my other project, the get_global_transform() * is not doing anything and the popup is still displaying on the wrong monitor.

@blackears
Copy link
Author

blackears commented Aug 5, 2024

I've modified the code to be

func _on_gui_input(event):
	if event is InputEventMouseButton:
		var e:InputEventMouseButton = event
		
		if e.button_index == MOUSE_BUTTON_RIGHT:
			if e.pressed:
				print("get_global_transform() ", get_global_transform())
				print("e.position ", e.position)
				%PopupMenu.popup(Rect2i(get_global_transform() * e.position, Vector2i.ZERO))

I am getting this output

get_global_transform() [X: (1, 0), Y: (0, 1), O: (290, 758)]
e.position (572, 81)

The popup menu is popping up on the left side of the screen:

image

@kleonc
Copy link
Member

kleonc commented Aug 5, 2024

I've upgraded to 4.2.2 and am now using:

%PopupMenu.popup(Rect2i(get_global_transform() * e.position, Vector2i.ZERO))

AFAICT it should be:

%PopupMenu.popup(Rect2i(get_screen_transform() * e.position, Vector2i.ZERO))

(at least seems to work)


Regarding event locality:

  • InputEventMouse.position docs are clear/precise about this:

    <member name="position" type="Vector2" setter="set_position" getter="get_position" default="Vector2(0, 0)">
    When received in [method Node._input] or [method Node._unhandled_input], returns the mouse's position in the [Viewport] this [Node] is in using the coordinate system of this [Viewport].
    When received in [method Control._gui_input], returns the mouse's position in the [Control] using the local coordinate system of the [Control].
    </member>

  • Control._gui_input docs mention this (line 151):

    <method name="_gui_input" qualifiers="virtual">
    <return type="void" />
    <param index="0" name="event" type="InputEvent" />
    <description>
    Virtual method to be implemented by the user. Use this method to process and accept inputs on UI elements. See [method accept_event].
    [b]Example usage for clicking a control:[/b]
    [codeblocks]
    [gdscript]
    func _gui_input(event):
    if event is InputEventMouseButton:
    if event.button_index == MOUSE_BUTTON_LEFT and event.pressed:
    print("I've been clicked D:")
    [/gdscript]
    [csharp]
    public override void _GuiInput(InputEvent @event)
    {
    if (@event is InputEventMouseButton mb)
    {
    if (mb.ButtonIndex == MouseButton.Left &amp;&amp; mb.Pressed)
    {
    GD.Print("I've been clicked D:");
    }
    }
    }
    [/csharp]
    [/codeblocks]
    The event won't trigger if:
    * clicking outside the control (see [method _has_point]);
    * control has [member mouse_filter] set to [constant MOUSE_FILTER_IGNORE];
    * control is obstructed by another [Control] on top of it, which doesn't have [member mouse_filter] set to [constant MOUSE_FILTER_IGNORE];
    * control's parent has [member mouse_filter] set to [constant MOUSE_FILTER_STOP] or has accepted the event;
    * it happens outside the parent's rectangle and the parent has either [member clip_contents] enabled.
    [b]Note:[/b] Event position is relative to the control origin.
    </description>

    but could be improved as Control's origin is just a single point, so "Event position is relative to the control origin" doesn't say anything about the orientation/scale. It says to what point/position it's relative to but not how. Like two different Controls can have the same origin but completely different scale/rotation, and thus they would get completely different event positions.

  • Control.gui_input (used in the MRP) docs don't mention it at all. Probably need adding a note about locality of the event, or a "See also: ..." link to some more detailed docs shown above:

    godot/doc/classes/Control.xml

    Lines 1094 to 1099 in 3978628

    <signal name="gui_input">
    <param index="0" name="event" type="InputEvent" />
    <description>
    Emitted when the node receives an [InputEvent].
    </description>
    </signal>


Regarding Window.popup{...} methods I'd say none of the docs are clear about in what coordinate spaces the arguments are meant to be, with regards to the 2D coordinate systems and 2D transforms docs:

<method name="popup">
<return type="void" />
<param index="0" name="rect" type="Rect2i" default="Rect2i(0, 0, 0, 0)" />
<description>
Shows the [Window] and makes it transient (see [member transient]). If [param rect] is provided, it will be set as the [Window]'s size. Fails if called on the main window.
</description>
</method>
<method name="popup_centered">
<return type="void" />
<param index="0" name="minsize" type="Vector2i" default="Vector2i(0, 0)" />
<description>
Popups the [Window] at the center of the current screen, with optionally given minimum size. If the [Window] is embedded, it will be centered in the parent [Viewport] instead.
[b]Note:[/b] Calling it with the default value of [param minsize] is equivalent to calling it with [member size].
</description>
</method>
<method name="popup_centered_clamped">
<return type="void" />
<param index="0" name="minsize" type="Vector2i" default="Vector2i(0, 0)" />
<param index="1" name="fallback_ratio" type="float" default="0.75" />
<description>
Popups the [Window] centered inside its parent [Window]. [param fallback_ratio] determines the maximum size of the [Window], in relation to its parent.
[b]Note:[/b] Calling it with the default value of [param minsize] is equivalent to calling it with [member size].
</description>
</method>
<method name="popup_centered_ratio">
<return type="void" />
<param index="0" name="ratio" type="float" default="0.8" />
<description>
If [Window] is embedded, popups the [Window] centered inside its embedder and sets its size as a [param ratio] of embedder's size.
If [Window] is a native window, popups the [Window] centered inside the screen of its parent [Window] and sets its size as a [param ratio] of the screen size.
</description>
</method>
<method name="popup_exclusive">
<return type="void" />
<param index="0" name="from_node" type="Node" />
<param index="1" name="rect" type="Rect2i" default="Rect2i(0, 0, 0, 0)" />
<description>
Attempts to parent this dialog to the last exclusive window relative to [param from_node], and then calls [method Window.popup] on it. The dialog must have no current parent, otherwise the method fails.
See also [method set_unparent_when_invisible] and [method Node.get_last_exclusive_window].
</description>
</method>
<method name="popup_exclusive_centered">
<return type="void" />
<param index="0" name="from_node" type="Node" />
<param index="1" name="minsize" type="Vector2i" default="Vector2i(0, 0)" />
<description>
Attempts to parent this dialog to the last exclusive window relative to [param from_node], and then calls [method Window.popup_centered] on it. The dialog must have no current parent, otherwise the method fails.
See also [method set_unparent_when_invisible] and [method Node.get_last_exclusive_window].
</description>
</method>
<method name="popup_exclusive_centered_clamped">
<return type="void" />
<param index="0" name="from_node" type="Node" />
<param index="1" name="minsize" type="Vector2i" default="Vector2i(0, 0)" />
<param index="2" name="fallback_ratio" type="float" default="0.75" />
<description>
Attempts to parent this dialog to the last exclusive window relative to [param from_node], and then calls [method Window.popup_centered_clamped] on it. The dialog must have no current parent, otherwise the method fails.
See also [method set_unparent_when_invisible] and [method Node.get_last_exclusive_window].
</description>
</method>
<method name="popup_exclusive_centered_ratio">
<return type="void" />
<param index="0" name="from_node" type="Node" />
<param index="1" name="ratio" type="float" default="0.8" />
<description>
Attempts to parent this dialog to the last exclusive window relative to [param from_node], and then calls [method Window.popup_centered_ratio] on it. The dialog must have no current parent, otherwise the method fails.
See also [method set_unparent_when_invisible] and [method Node.get_last_exclusive_window].
</description>
</method>
<method name="popup_exclusive_on_parent">
<return type="void" />
<param index="0" name="from_node" type="Node" />
<param index="1" name="parent_rect" type="Rect2i" />
<description>
Attempts to parent this dialog to the last exclusive window relative to [param from_node], and then calls [method Window.popup_on_parent] on it. The dialog must have no current parent, otherwise the method fails.
See also [method set_unparent_when_invisible] and [method Node.get_last_exclusive_window].
</description>
</method>
<method name="popup_on_parent">
<return type="void" />
<param index="0" name="parent_rect" type="Rect2i" />
<description>
Popups the [Window] with a position shifted by parent [Window]'s position. If the [Window] is embedded, has the same effect as [method popup].
</description>
</method>

@akien-mga akien-mga added this to the 4.3 milestone Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants