-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix touch input on Linux #14117
Fix touch input on Linux #14117
Conversation
The code relied on touch IDs being consecutive. This is true on Android, but not on Linux. Therefore, touch input on Linux was broken since 53886dc.
The issue has been resolved by this patch. Thank you. |
The issue with this PR's approach is that each touch-pointer can no longer be independently moved.
Minetest-5.8.0-Android-place-one.mp4This PR: Minetest-14117-Android-place-one.mp4In my opinion, your original approach in the touch-screen support for SDL (minetest/irrlicht PR 262) is good. For now, that approach can be applied for Linux/X11 (my attempt). |
This works for me in Linux. (not saying SDL isn't the right way) 😉 |
Sorry, which this did you mean? |
I tried to reproduce what you did on the video and it worked with patched 5.8.0 build on Linux phone. |
@@ -273,7 +273,7 @@ bool GUIModalMenu::preprocessEvent(const SEvent &event) | |||
irr_ptr<GUIModalMenu> holder; | |||
holder.grab(this); // keep this alive until return (it might be dropped downstream [?]) | |||
|
|||
if (event.TouchInput.ID == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ouch, this shouldn't have passed code review. easy to miss however.
It sounds like what the code should be doing is remember the ID of the first and second touch when they begin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not realise that the ID assignment is different for each device. I just checked on my phone and thought that it resets to zero when there is no touch. My bad.
Another possibility: Add a pointer index (as opposed to pointer ID) field to Irrlicht's The question is: Which solution do we choose? |
How about a global helper that "remember" the order/index of each active touch? It can be in Minetest or Irrlicht (for each device type). If done this way, the other part does not have to check for the index. |
After thinking about this some more, I believe this PR should be merged as is. The limitation @srifqi is describing - that moving the first pointer doesn't work if a second pointer exists - only applies in formspecs and has existed approximately forever. Nobody ever complained about it AFAIK - probably because there is only a use case for tapping the screen with a second finger, not for keeping a second finger on the screen. This PR just restores the state before the regression. More improvements would be nice, but should not be expected from a PR that is supposed to fix a regression. Merging this soon would also be good so as not to block further work on the touchscreen code. |
Fixes #14111.
The code relied on touch IDs being consecutive. This is true on Android, but not on Linux. Therefore, touch input on Linux was broken since 53886dc.
To do
This PR is a Ready for Review.
How to test
Play Minetest on Android. Verify that touchscreen input, and especially stack splitting, still work.
Play Minetest on Linux with a touchscreen. Verify that touchscreen input, and especially stack splitting, work again.