Fix for Android double tap not triggering correctly #46100 and #46101

[Ongnissim]

onDoubleTap gets called for touch events, so event.getButtonState() returns
nothing. Instead, event.getPointerCount returns the number of fingers currently on the
screen, so doubleTap now returns the appropriate "button" for one to
three fingers.

This fixes the issues with FileDialog on Android as well.

There is still a button release event being sent after the double_click is triggered, and I believe this is happening due to a bit of code called ensure_touch_mouse_raised() in input.cpp that's called by the SceneTree when focus is changed.

It doesn't seem to break anything, so I didn't mess with it.

[akien-mga]

Thanks for contributing!

There are some issues with the commits in this PR which would need to be solved by rebasing, namely:

• The "Merge branch 'master'" commit and its two revert commits should be removed, as they add noise to the Git history and no function. See https://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html#the-interactive-rebase
• The first commit which actually implements a fix should have its commit message edited to be more explicit about what it does (specifically in its title). See https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind

[thebestnom]

Godot on android does support mouse buttons, and this is how we get which button was pressed

[thebestnom]

You should check event source before doing this 😅

[aaronfranke]

@Ongnissim You can't just revert. You have to rebase. See the article that Akien linked for more information.

[Ongnissim]

I had an issue with Github authorization. I'm new to this whole process, so please forgive me. Should be clean and ready to go now!

Godot on android does support mouse buttons, and this is how we get which button was pressed

Yes! But in this case, in the GestureHandler was trying to parse a screen touch event as a mouse button event, which lead to a double click happening on mouse button 0, which was off.

At least that's what it appears to have been doing. My tests show that it's now working as intended, and mouse input on Android is unaffected by this change. I definitely tested that before I put up a pull request. I really really don't want to break anything, haha!

[thebestnom]

Mouse click should be affected by it for right click or middle click as you put a constant there 😅

[Ongnissim]

This only affects touch screen input, it's only triggered when a double tap is done on screen, since this is in the gesture input event handler. Traditional mouse input is unaffected.

This then sends the correct button mask to the double click parser.

The original, being a screen touch event, came out to 0, which becomes mouse button none. With this, the double tap is parser correctly as either 1, 2, or 3 for a double tap that is mapped to the intended button (left, right, middle) by the code that already exists within the Android event handler.

I hope this makes sense

[thebestnom]

Traditional mouse input is unaffected.

I have tested it once on traditional mouse and it does catch that, can you test your claim 😅?

[Ongnissim]

Alright, I've spent a bit more time on this. When I'd first tested it, I'll admit, I only tested single-clicks under the assumption that the gesture handler couldn't possibly affect non-gesture inputs. (I was wrong :P). So:

If we run both getPointerCount and getButtonState through, with a boolean check on whether the input was a mouse, then it behaves as it should to my testing. I'm updating my commit now.

I cannot get the right mouse button to register a double click, and I can't figure out why only that button isn't registering. The LMB and MMB both double click as expected.

The code in Master as-is also misses double-clicks from RMB, at least in my testing. But I've only tested on a Samsung Galaxy Tab S7, and my research shows that Android has had varying behaviors in the past on how RMB is mapped, but my attempts to find out why have been fruitless so far.

[Ongnissim]

There's another potential issue I see here: double-taps will register as mouse double-clicks even if emulate_touch_as_mouse is off, which means this probably isn't the absolute best way to fix this, or that it's just a quick patch for a deeper underlying bug.

This does solve the problems for my needs, but there is clearly something else going on.

[m4gr3d]

@Ongnissim Is double tap triggered with multiple fingers? I tested double tapping with 2 and 3 fingers and the event didn't trigger.
If it's not supported, then we can simplify this logic.

[Ongnissim]

I don't think it is supported, but in the event that it would be supported, this returns a button mask of 0,1,2,3... for the number of fingers. I did this so that touch input would have parity with mouse input. I never got a response so I stopped working on it.

Are you saying that onDoubleTap doesn't trigger at all on double tapping with multiple fingers? I also couldn't get this to trigger how I expected, and would like to know how to make this work.

This is my first PR ever, so I won't be offended if this isn't the correct way to implement this, and just want the bug fixed :).

[Ongnissim]

I'm going to clone this again and test to see if getPointerIndex() does what I anticipated.
The idea is that you'd be able to have one finger down, double-tap a second finger, and this would register as a right-mouse double-click. Three for MMB.

[m4gr3d]

The idea is that you'd be able to have one finger down, double-tap a second finger, and this would register as a right-mouse double-click. Three for MMB.

@Ongnissim I tested that, and yes onDoubleTap doesn't trigger in this scenario.

This is my first PR ever, so I won't be offended if this isn't the correct way to implement this, and just want the bug fixed :).

The approach is pretty close and can be improved by incorporating some of the logic in #59760.

I think the proper approach here would be to pass the tooltype value alongside the button mask.
In the native code in AndroidInputHandler::process_double_tap, when the tooltype is MotionEvent.TOOL_TYPE_FINGER, then disregard the button mask and use the logic in #59760.
Otherwise, use the current logic that takes into account the button mask.

@madmiraal @thebestnom Thoughts?

[madmiraal]

Yes, to properly fix #46100 and #46101 (beyond #59760) would require separating out the mouse double-clicks from the double-taps. So that the start of the second tap is not also detected as the start of a second single click.
Note: The main challenge is ensuring the release of the second click releases the double tap too.

[m4gr3d]

So that the start of the second tap is not also detected as the start of a second single click.

@madmiraal That's part of a larger issue that also affects other gestures besides double-taps. Our current input logic sends both the raw input events (down, move, up) and the processed input events (detected gestures) to the native layer, so in scenarios where a gesture is detected, we end double-counting.

We need to update the logic so that we only sends raw input events when a gesture is not detected; it's part of a larger fix I'll be looking at unless one of you is up to the task :)

For this issue, we can scope the fix to re-enabling finger double-taps using the proposal I highlighted above since at the moment they're disabled because of the button mask value.
On top of fixing finger double-taps, this will allow to leverage the current logic which provide information regarding the source of the event when it's not a finger.

[madmiraal]

The problem isn't just with the Android platform. It was thinking through this issue that led me to create godotengine/godot-proposals#4340

[m4gr3d]

Superseded by #65434