Gestures InputEvents

[Federico-Ciuffardi]

Addresses the issue #13139 (just for android) by implementing the following InputEvents from https://github.com/Federico-Ciuffardi/Godot-Touch-Input-Manager:

• InputEventScreenPinch
• InputEventMultiScreenDrag
• InputEventScreenTwist

[realkotob]

Looks good but I have a couple of points to discuss.

1. Does it make sense to add a length() and angle_to() function to the Android OS class? Can't we add them to the Point2 class directly?

2. If I understand correctly, it seems with these conditions, you can only get one gesture emitted at the same time, but often you want to pinch and twist at the same time.
   For example to rotate an image while zooming in/out.
   Edit: It seems this is done in the OSX code as well Native pan and zoom for macOS + InputEventGesture #12573, so I guess it's fine.

[Federico-Ciuffardi]

Looks good but I have a couple of points to discuss.

1. Does it make sense to add a length() and angle_to() function to the Android OS class? Can't we add them to the Point2 class directly?

2. If I understand correctly, it seems with these conditions, you can only get one gesture emitted at the same time, but often you want to pinch and twist at the same time.
   For example to rotate an image while zooming in/out.
   Edit: It seems this is done in the OSX code as well  #12573, so I guess it's fine.

1. I agree, it would be better to add those methods to the Point2 class directly.
2. Yes, you are understanding it correctly. To solve this the analysis that deduces what gesture is the most appropriate could be removed, and as just removing that analysis would cause unnecessary gesture InputEvents to trigger it would be necessary to apply a filter (for example, trigger if relative is greater than a certain threshold).

[nathanfranke]

Awesome!

InputEventMultiScreenDrag should be renamed to InputEventScreenPan since

• User can just check for multiple InputEventScreenDrag so no possibilities are lost
• Consistent naming style with InputEventScreen* for general camera methods

[Federico-Ciuffardi]

Awesome!

InputEventMultiScreenDrag should be renamed to InputEventScreenPan since

* User can just check for multiple `InputEventScreenDrag` so no possibilities are lost

* Consistent naming style with `InputEventScreen*` for general camera methods

I changed the names but forgot about it, here is how I changed them:

• InputEventScreenPinch -> InputEventPinchGesture
• InputEventScreenTwist -> InputEventTwistGesture
• InputEventMultiScreenDrag

I tried to keep the consistency with the already existing InputEvents (e.g. InputEventMagnifyGesture and InputEventScreenDrag).

[realkotob]

Removing that analysis would cause unnecessary gesture InputEvents to trigger it would be necessary to apply a filter (for example, trigger if relative is greater than a certain threshold).

I would definitely not suggest that we should force users to pick their own threshold values, but if we can find some very sane defaults I would prefer this option, and leave the option for users to modify them somewhere in the Android project settings.

Detecting pinch & twist at the same time is common in my opinion, but I'd like more discussion about this point from other developers who've worked with gestures (my experience might be biased to my use cases).

[nathanfranke]

Make sure you squash all the commits eventually

[jeremyz]

Wouldn't it be better to give more attributes and methods to InputEventScreenDrag class instead of having InputEventGesture, InputEventMultiScreenDrag, InputEventTwistGesture, InputEventPinchGesture, InputEventMagnifyGesture, InputEventPanGesture ?

Whatever the gesture is, the InputEventScreenDrag is always emitted.

[nathanfranke]

@jeremyz the gestures should happen alongside other events but not replace them, so the drag event is always emitted. Regardless, I don't think adding parameters to the drag event would be ideal since for example you would never have a pinch gesture with only one tap.

Anyways @Federico-Ciuffardi , in my opinion we should replace the multi drag event with a pan gesture. As it stands, I'm not even sure whether it does the averaging math to behave like a pan or just a useless wrapper for multiple events (it's not like we need InputEventMultiKey)

[jeremyz]

@nathanwfranke sure, I did rethink a bit my position since my writing.
Emitting a gesture event alongside other events, when conditions and a threshold is reached is correct.
I'm just a bit worried about all these InputEventGesture sub classes. If this nice PR is merged, we will end up with 2 classes emitted on OSX and 3 other classes emitted by Android ...

[nathanfranke]

Osx gestures should be removed since they are native only. This implementation should be multiplatform and replace it.

Edit: I see now that this pr implements os_android which is bad. All this math should be done the same on all platforms

[jeremyz]

I was at first a bit puzzled by your answer, but your Edit is spot on !
Some thinking must be done.

[Federico-Ciuffardi]

Anyways @Federico-Ciuffardi , in my opinion we should replace the multi drag event with a pan gesture. As it stands, I'm not even sure whether it does the averaging math to behave like a pan or just a useless wrapper for multiple events (it's not like we need InputEventMultiKey)

It does the averaging math to behave like a pan. With that out of the way I will try to explain the reason why I think the multi drag event is valuable an not just a "InputEventMultiKey".

When I was making games, many times I needed to move the camera while Interacting with the game (e.g. selecting and moving units on a RTS) so my solution to avoid having a "toggle between camera and interact mode" button and to be constantly pressing that button was to handle:

• Camera movement when dragging with multiple fingers
• Interaction when using one finger

So I found useful to have a InputEventMultiScreenDrag and a InputEventSingleScreenDrag, the latter different to the InputEventScreenDrag as it just triggers when using only one finger (not present in this PR). And even if you wanted a more general pan gesture that works also with just one drag as well you could use if event is InputEventMultiScreenDrag or event is InputEventSingleScreenDrag:.

If you find it useful, I (or anyone who wants to) could try to add the InputEventSingleScreenDrag to this PR. And if no one realy likes the idea of a InputEventMultiScreenDrag and a InputEventSingleScreenDrag, I (or again, anyone who wants to) could just merge inputs events InputEventMultiScreenDrag and InputEventSingleScreenDrag into a InputEventPanGesture and add an attribute for the number of fingers involved in it so the case I described before can be solved with just this input event.

So, what do you think about this?

Also remember that you could check out Godot Touch Input Manager to see InputEventMultiScreenDrag, InputEventSingleScreenDrag and many more InputEvents implemented in GDScript as well as some usages examples.

[jeremyz]

Hi @Federico-Ciuffardi,

I've checked your 3 repos and the way basic InputEvent* are generated from native code under platform/**/*.

It seems to me that your code in InputManager should merge into Inputfilter#parse_input_event_impl, it is a singleton class that deals with InputEvents, that way your event analysis and gesture event emission would be shared by all the supported platforms.

I think you should define 3 new InputEvent classes InputEventGestureTwist, InputEventGesturePinch and InputEventGesturePan, with the appropriate attributes.

Naming follows the InputEventMouse* and clearly defines 3 different types of gestures.

@nathanwfranke ?

[sairam4123]

It would have been cool to support windows and Linux as well, because it was supporting Windows' Gestures as well.

[jeremyz]

They are 2 pending PR, 3.2 branch #37754, master #39055.
They implement EventGesturePan,Pinch,Twist in godot core,
so this should work with any OS that emits proper InputEventTouch,Drag.
I've tested it on Android and Linux.