Keep input event as unhandled if they go through a control set to MOU…

[groud]

I had that issue where I wanted to be able to drag&drop things onto a game view. To do so, I tried put a control set to MOUSE_FILTER_PASS on top of my world, expecting that all unhandled mouse event would end up being processed by _unhandled_event, turned out it wasn't the case. Basically, as soon as an event is given to a Control somewhere, the event would be considered as handled if it went through a control with MOUSE_FILTER_PASS or MOUSE_FILTER_STOP.

However, I think it is not expected that an event which went through Controls set to MOUSE_FILTER_PASS ends up as handled if it wasn't explicitly set as handled.

As a consequence, what this PR does is simply set a mouse event as handled only if it it was stopped by a control. If the event reaches the root going only through controls set to MOUSE_FILTER_PASS or MOUSE_FILTER_IGNORE (and is not set as handled explicitly), then the event is now passed to the _unhandled_event processing.

Obsolete edit

while investigating this issue, I ended up with other problems that I tried to fix too:

1. There was a mistake in this line of _gui_call_input(...) :

bool ismouse = ev.is_valid() || Object::cast_to<InputEventMouseMotion>(*p_input) != nullptr;

The ev.is_valid() part was always true, which caused all events, even keyboard ones, to be stopped while climbing up the tree if the Control had the filter set to MOUSE_FILTER_STOP. This does not seemed like an intended behavior.

2. ~~Scroll wheel events were unstoppable. It was not much of an issue when those event were not forwarded to _unhandled_input, but with this PR, it became quite annoying. So I removed this special case (as ScrollContainer is already set to MOUSE_FILTER_PASS by default anyway). It is debatable whether or not this change is welcome, as it means that we would need all containers in a hierarchy to be set as to use the MOUSE_FILTER_IGNORE or MOUSE_FILTER_PASS modes so that the event can climb up the tree. Maybe an alternative would be to add another mode, set by default, that would allow those specific events to pass? Like MOUSE_FILTER_STOP_BUT_SCROLLS (bad name, I have to admit ^^)?

Alright, I had to modify a little bit the PR:

• Events that are not stopped still correctly go to _unhandled_input. This initial work caused issues, as scroll events were always passed, even with the MOUSE_FILTER_STOP filter. (I had no way to stop those events, thus scrolling in a ScrollContainer caused the event to be handled by _unhandled_input, and, in my case, it caused my camera to zoom in/out)
• To solve this, I added a mouse_force_pass_scroll_events. It allows you to still allow scroll events to be passed up the tree even when using the MOUSE_FILTER_STOP filter. It is enabled by default. The property is grayed out if the mouse filter is not set to MOUSE_FILTER_STOP as it is useless in those other cases.
• Still fixed the bug on that wrong line bool ismouse = ev.is_valid() || Object::cast_to<InputEventMouseMotion>(*p_input) != nullptr;

[groud]

Might fix #17326 and #57349

[groud]

Ah wait, it seems there's a bug with release events, which might not end up in _unhandled_input in some cases.

[groud]

Alright, I had to modify the PR to avoid some issues but it made me discovered other. See the original post.

[akien-mga]

Approved in PR review meeting. There's potential for regression but we'll find out in production ;) it's easy to see when input breaks.

[groud]

Alright, I updated the code with the suggested changes and updated the documentation accordingly.
It should be good to merge as soon as CI is happy.

[Sauermann]

Might fix #17326 and #57349

From what I can see from the code change, I am quite confident, that this PR will not fix #17326.

[groud]

From what I can see from the code change, I am quite confident, that this PR will not fix #17326.

Well, I relied on what you said on that PR: #57894, where you said that "Since the event is internally marked as handled, _unhandled_input is never called.". This PR does make it so that event is passed to to _unhandled_input correctly, I supposed it would solve the associated issue.

But maybe I am wrong, I don't know, I haven't checked whether or not the PR solved the issue.

Edit: to be clear, it does not mean your fix is not good. My PR may solve this specific issue but your changes might still be needed, I am not sure.

[Sauermann]

Boolean arithmetic ... You are right, I misread your code. This PR might change the event behavior, so that the symptom which causes #17326, is mitigated.

I believe, that #57894 is still necessary. With your changes applied I can provide a use-case where Mouse-clicks now are used up by a Control-Element, that is located behind a SubViewportContainer, while the user would expect, that it should be consumed inside the SubViewport. After this gets merged, I will have to update the description of my PR.

[groud]

I believe, that #57894 is still necessary. With your changes applied I can provide a use-case where Mouse-clicks now are used up by a Control-Element, that is located behind a SubViewportContainer, while the user would expect, that it should be consumed inside the SubViewport. After this gets merged, I will have to update the description of my PR.

Yeah that makes sense.

[akien-mga]

Thanks!

[kleonc]

Does this supersede #54656?

Does this solve any of: #55430, #55432, #54529?

CC @golddotasksquestions

[groud]

Does this solve any of: #55430, #55432, #54529?

I feel that this PR might fix all of them. Makes me wonder if it would be worth a cherry-pick for 3.x

[golddotasksquestions]

I just tested this with my latest problem example in Godot 4.0 Alpha 11 and the mouse_filter pass still does not seem to work.

Detailed issue description: #55432 (comment)
Minimal Godot 4.0 Alpha 11 Reproduction Project:
mouse_filer_pass_issue.zip