Proposal for SubViewportContainer input handling

[Sauermann]

This pull request intends to solve the problem, that SubViewportContainer does not send unhandled input events to the Viewport, which is tracked in #17326. The current situation makes it necessary to include code like this for each SubViewportContainer, that needs to send events to it's viewport:

extends SubViewportContainer

func _input(event):
	$Viewport.unhandled_input(event)

The reason for this problem is in my opinion, that events get sent to SubViewports during _input and also during _unhandled_input. This conflicts with Control Nodes (in this case the SubViewportContainer) setting the event to handled during _gui_input, so that _unhandled_input is not triggered at all.

My suggested solution to this problem is to switch from the current event-layer-first approach to a viewport-first approach: Handle events primarily within a Viewport and only in special cases jump to other viewports.
For SubViewportContainer, this special case should be within _gui_input, because this is where all other Control nodes handle their functionality.
This solution makes SubViewport event propagation work out of the box.

This change happens to resolve several other bugs as well. Including the one, that the pull request #55300 also tries to solve. Unfortunately the two pull requests are not compatible, because the other one extends on the event-layer-first approach. I have found no way to resolve the intended bug with that approach.

Here is a list of all resolved issues:

1. Unhandled events do not get sent to SubViewports
   fix Sub - viewports still don't handle _unhandled_input or CollisionObject._input_event() #17326

2. SubViewportContainer catches mouse events despite being under Control Nodes
   fix Control inside Viewport inside ViewportContainer ignores gui-event tree order and is in front #48401
   fix Viewport catches mouse events despite being under Canvas Layer #45400
   fix Click Events passing through Buttons if there is a Viewport underneath #39666

3. Clip Content does not work for SubViewportContainer
   fix Clicking outside of a SubVieportContainer with activated rect_clip_content can trigger a gui event on a Container's descendant Control-Node #57653
   fix Viewport not clip control node input #28833

4. Misaligned Event coordinates inside SubViewports for MouseEvents
   fix Button inside viewport not receiving input when Camera2D active #48368

5. Some Event notifications get dropped at SubViewportContainer border
   fix Notification problem at SubViewportContainer border #57762

It is related to

• Physics objects don't detect the mouse if inside a sub-viewport #31802
• Area2D mouse_entered and mouse_exited signals not called in Viewport #26181

I did not yet squash the commits together for a better understandability of the changes.

Concept

This chart shows the high level concept of Event handling in my proposed changes:

The basic idea of this patch is to replace input() with gui_input() in SubViewportContainer and to step through all event layers inside of Viewport.

Reasoning

The reasons for this approach are:

1. While _input() is used to filter out events of interest for user-convenience, _gui_input() usually handles the behavior of Control elements. Since SubViewportContainer is a Control element, _gui_input() should be the place, where it performs it's actions, in this case handling the interaction between parent Viewport and SubViewport. This includes handling of event-propagation.
2. Keeping the handling of the layers at one location within the viewport simplifies the whole procedure and makes it less error-prone. This pull request separates viewports as much as possible from each other. This is more similar to the viewport-separation idea described in the Using InputEvent Tutorial, than the current status of the implementation is.
3. Since these changes simplify the behavior of input propagation and reduce the deviation between the behavior of SubViewportContainer and Control nodes, event handling is simpler to document and I believe that it is easier for users to understand in comparison to the current implementation.

Introduced Changes

This approach changes the current behavior in the following ways:

1. SubViewportContainer sends received gui-events as input-events to the child SubViewport.
2. The order in which Nodes recieve an event is changed as shown in the flowchart above.
3. Key-Events are only delivered from a SubViewportContainer to the corresponding SubViewport, if the SubViewportContainer has active key_focus. This makes it consistent with the way other Control nodes treat Key-Events.
4. Propagation from Gui Event Layer to Unhandled Key Event Layer now happens in Viewport. SubViewportContainers no longer need to propagate unhandled_key_events and Window does not need to address unhandled_key_events separately. This also fixes the problem, that TouchScreenButton events do not get propagated correctly to unhandled_events.
5. Event propagation (e.g. Physics Collider with Mouse) now also work out of the box in SubViewports without the need for workarounds.
6. When setting key_focus, now only the key_focus of the current SubViewport is erased, instead of all viewports of the current window. This allows gui_events to travel through a sequence of viewport-transitions.
7. Input Events no longer get propagated by all SubViewportContainers to their SubViewports, but only by those that are active in regard to Gui Input Events. For example MouseClick Events no longer get delivered to SubViewports, which are away from the MousePointer, reducing unnecessary events. This also makes the behavior of SubViewportContainers consistent with the regular behavior of Control Nodes.
8. SubViewportContainer now sends relevant notifications to SubViewport so that focus is handled correctly.

Testing

Here is my project file, that I used for testing the implementation: SubViewportClick.zip

I performed the following steps during testing:

• Ensured that the MRP's of all mentioned issues work
• Mouse Event propagation with two levels of subviewports
• Keyboard Event propagation with two levels of subviewports
• Physics picking with two levels of subviewports
• Correctness of Viewport-Transformations with two levels of subviewports (moved & rotated)
• SubViewportContainer siblings
• 3D SubViewport at root level
• RootWindow -> Window -> SubViewportContainer

Known issues

• If a SubViewport with 2D/3D content contains a SubViewport with 3D/2D content, then the inner content is not visible - this is the same behavior as in master. This pull request doesn't try to fix this issue
• Area2D + CollisionShape2D: inconsistent mouse_enter & mouse_exited events at window border #57723 happens also at the border of SubViewportContainer
• An active Camera2D inside a viewport with a non default Transorm2D does not work correctly, just like in master.

Related

The suggested changes will interfere with #48800, because set_input_as_handled() is called immediately after physics_picking_events.push_back(ev);.
My reasoning is that if a viewport has physics_object_pickingenabled, the event should be considered as handled, no matter if the event actually finds a CollisionObject. Also the physics processing happen later and possibly within a different thread, so feedback is non reliable and for this implementation it is necessary to be reliable. If set_input_as_handled() is used within Physics-processing, it should be independent from the set_input_as_handled() of Event-processing.

If this pull request is accepted, Viewport::_gui_remove_focus_for_window is no longer used anywhere in the code. Should that function be accordingly removed in this pull request?

Should this pull request be accepted, I am willing to contribute to the InputEvent Tutorial.

[reduz]

I have the feeling that this may be too much of a complex solution, since maybe that would be needed here is a _gui_unhandled_input per viewport instead, which respects GUI order.

[Sauermann]

@reduz Thank you for your comment.

I will withdraw this pull request.

[reduz]

Well, I suppose we could still discuss this with other contributors in the area. I feel _unhandled_input and _key_input are maybe too broad notifications and maybe they should be only used in the context of GUI, unlike _input

[Sauermann]

I believe, you were right about this being too complex. I try to do too much in this single pull request. I would rather prefer to split up the changes into smaller portions, so that they are more easily understandable and verifiable. Two of my open pull requests already do exactly this. Since the changes build on top of each other, I started with the basic ones.

[Sauermann]

Closing this draft, because I have split up the changes into a few different PRs.