Using force_drag from a Control's gui_input signal crashes with no errors with multiple mouse input buttons.

[braydeejohnson]

Tested versions

• Reproduced in 4.2.2, 4.2.1, 4.2, 4.1.2, 4.1, and 4.0

System information

Godot v4.2.2.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 Ti (NVIDIA; 31.0.15.5222) - 11th Gen Intel(R) Core(TM) i9-11900K @ 3.50GHz (16 Threads)

Issue description

When using the gui_input signal of a Control node to start a drag event via force_drag, any additional MouseButton presses cause the project to crash immediately. I start the force drag with a Right Click and then any additional mouse press (Left, Middle, Mouse 4/5 etc) all cause it to occur. On my Windows machine it shows no errors, even when using --verbose to launch, but on my Steam Deck I can see it throws a crash with signal 11

Steps to reproduce

On a Control node ie Button connect the gui_input signal to a script that checks for any InputEventMouseButton and call the force_drag function to begin the drag.

extends Button

func _on_gui_input(event: InputEvent) -> void:
	if event is InputEventMouseButton:
		force_drag({}, duplicate())

While running the project, use any mouse button to start the drag, then press any additional MouseButton and it will crash entirely, no error message or anything to debug.

Minimal reproduction project (MRP)

button_force_drag-main.zip

Project code is also available on github

[braydeejohnson]

I spent some time getting the source code and compiler running on my machine and poked around in the Viewport.cpp file and noticed that the gui.mouse_focus is set to nullptr and commenting that out seems to fix this specific issue, though I don't know of what consequence as I'm not strong in C++ development nor the Godot source as a whole.

void Viewport::_gui_force_drag(Control *p_base, const Variant &p_data, Control *p_control) {
	ERR_FAIL_COND_MSG(p_data.get_type() == Variant::NIL, "Drag data must be a value.");

	gui.dragging = true;
	gui.drag_data = p_data;
	// gui.mouse_focus = nullptr;    # I'm sure that there is a reason to null the mouse_focus, but in doing so it causing an issue downstream.

	if (p_control) {
		_gui_set_drag_preview(p_base, p_control);
	}
	_propagate_viewport_notification(this, NOTIFICATION_DRAG_BEGIN);
}

[braydeejohnson]

I was able to reproduce the error with a compiled version of the engine with debugging enabled. The Viewport's _gui_input_event throws an exception:

Exception has occurred: W32/0xC0000005

- 

Unhandled exception thrown: read access violation. this->gui.mouse_focus was nullptr.

It looks like when we are setting the gui.mouse_focus to nullptr from the _gui_force_drag function, we aren't checking to prevent that condition from being the case here in the followup _gui_input_event calls when we are also clicking immediately afterwards.

void Viewport::_gui_input_event(Ref<InputEvent> p_event) {
	ERR_FAIL_COND(p_event.is_null());

	Ref<InputEventMouseButton> mb = p_event;
	if (mb.is_valid()) {
		gui.key_event_accepted = false;

		Point2 mpos = mb->get_position();
		if (mb->is_pressed()) {
			MouseButtonMask button_mask = mouse_button_to_mask(mb->get_button_index());
			if (!gui.mouse_focus_mask.is_empty() && !gui.mouse_focus_mask.has_flag(button_mask)) { 
				// Do not steal mouse focus and stuff while a focus mask without the current mouse button exists.
				gui.mouse_focus_mask.set_flag(button_mask);
			} else {
				gui.mouse_focus = gui_find_control(mpos);
				gui.last_mouse_focus = gui.mouse_focus;

				if (!gui.mouse_focus) {
					return;
				}

				gui.mouse_focus_mask.set_flag(mouse_button_to_mask(mb->get_button_index()));

				if (mb->get_button_index() == MouseButton::LEFT) {
					gui.drag_accum = Vector2();
					gui.drag_attempted = false;
				}
			}
			DEV_ASSERT(gui.mouse_focus);   

			mb = mb->xformed_by(Transform2D()); // Make a copy of the event.

			Point2 pos = gui.mouse_focus->get_global_transform_with_canvas().affine_inverse().xform(mpos); 

This DEV_ASSERT implies that this block of code must have a valid gui.mouse_focus for proper execution...

			DEV_ASSERT(gui.mouse_focus);   

When gui.mouse_focus is nullptr, we fatal at this point.

			Point2 pos = gui.mouse_focus->get_global_transform_with_canvas().affine_inverse().xform(mpos); 

This condition block is the only one not guarding directly against gui.mouse_focus being invalid, or fetching for other conditions, but assumes the button masking. Maybe this could guard against it with a simple conditional check?

			if (!gui.mouse_focus_mask.is_empty() && !gui.mouse_focus_mask.has_flag(button_mask)) { 
				// Do not steal mouse focus and stuff while a focus mask without the current mouse button exists.
				gui.mouse_focus_mask.set_flag(button_mask);
			} 

Not sure if the expectation of force dragging is that it always should clear the mosue_focus or if it's safe to remove the set to nullptr when a force_drag occurs. I could see for example if the force drag call makes the assumption that no mouse event caused it to occur, therefore it should be unset, but then you lose out on cases like mine where you want to trigger a force drag with alternative mouse buttons and those get unset when calling the force_drag.