ihhub / fheroes2

fheroes2 is a recreation of Heroes of Might and Magic II game engine.
https://ihhub.github.io/fheroes2/
GNU General Public License v2.0
2.74k stars 377 forks source link

Rightclick emulation on touchscreen does not work as intended in Linux #8051

Open drevoborod opened 1 year ago

drevoborod commented 1 year ago

Preliminary checks

Platform

Linux

Describe the bug

on touch screens double finger tap does not work as intended.

Expected actions to emulate right mouse click:

  1. Tap and hold one finger on the object.
  2. Not releasing the first finger, tap another finger somewhere else. Context message appears.
  3. Release first finger to see the message beneath. The message stays visible.

Actual actions needed to emulate right click:

  1. Tap one finger on the object.
  2. Not releasing the first finger, tap another finger somewhere else. Context message appears.
  3. Release first finger to see the message beneath. The message disappears!.
  4. Tap first finger on the object again.
  5. Release the second finger (!)
  6. Tap the second finger again.
  7. Release the first finger, the second remains on the screen. And only in this case appeared message does not disappear.

Here is video reproducing the issue:

https://github.com/ihhub/fheroes2/assets/10282847/82d43d5a-335d-4d19-b3a7-e1490d969984

Save file

-

Additional info

No response

oleg-derevenetz commented 1 year ago

According to the description, I would say that on Linux (at least on that particular Linux) finger ids are messed up. Apparently on step 3 "finger up" event comes with wrong finger id - it comes with finger id of the second finger, and not the first finger (as it should actually be). The logic of the operation is based on the fact that the correct finger ids come from SDL (or from the touchscreen driver). If this is not the case, then, alas, of course, it will not work. Need to fix the driver (or the corresponding SDL build).

drevoborod commented 1 year ago

Need to fix the driver (or the corresponding SDL build).

Unfortunately, it's not possible for regular user like me, so I would appreciate if you could find some workaround.

oleg-derevenetz commented 1 year ago

Unfortunately, it's not possible for regular user like me, so I would appreciate if you could find some workaround.

You can start with a simple one - which SDL version is installed on that device? Have you tried to install the latest one (2.28.5) and build fheroes2 using the latest version?

drevoborod commented 1 year ago

Unfortunately, it's not possible for regular user like me, so I would appreciate if you could find some workaround.

You can start with a simple one - which SDL version is installed on that device? Have you tried to install the latest one (2.28.5) and build fheroes2 using the latest version?

Please tell me how to check it? And what exactly SDL is? Package "sdl2" in my system has version 2.28.5-1, so looks like it's correct version? Manjaro is actually a rolling distribution, so all software has versions close to latest. And I build fheroes2 locally without any customization, just using make -j7 command on master branch.

oleg-derevenetz commented 1 year ago

Please tell me how to check it? And what exactly SDL is? Package "sdl2" in my system has version 2.28.5-1, so looks like it's correct version?

Looks like it is. Well, then the things may be worse than I thought.

And I build fheroes2 locally without any customization, just using make -j7 command on master branch.

There is no customization in fheroes2 that can deal with wrong finger ids in touchscreen events.

drevoborod commented 1 year ago

So maybe I could collect some traces to check contents of actual events?

oleg-derevenetz commented 1 year ago

So maybe I could collect some traces to check contents of actual events?

Well, even if you add some debugging output in LocalEvent::HandleTouchEvent() method and it will confirm that finger id on "finger up" event is wrong (and this is the only way to explain the disappearance of the info window, there can be no other options here in theory), I have no idea what to do next. Well, you can add something like:

printf("%u %ld\n", event.type, event.fingerId);

at the beginning of that function (after opening curly brace at line 899 in src/engine/localevent.cpp), this should print event type and finger ID to the stdout in terminal, let's see.

drevoborod commented 1 year ago

Here are results of tapping with ONE finger at a time in main game menu. Each block between minuses is a result of ONE tap on some of main menu items - load game, cancel load dialogue, etc.

1792 1
1793 1
-----------------
1792 2
1794 2
1794 2
1793 2
--------------------
1792 3
1793 3
-------------------
1792 4
1793 4
---------------
1792 5
1793 5
----------------
1792 6
1793 6
-----------------

What seems interesting:

Next two blocks are results of following gestures sequence:

  1. Place and hold first finger.
  2. Place and hold second finger.
  3. Release first finger.
  4. Release second finger.
1792 16
1794 16
1794 16
1794 16
1792 17
1793 17
1792 17
1794 16
1793 16
1794 17
1794 17
1793 17
---------------------
1792 18
1794 18
1794 18
1792 19
1793 19
1792 19
1794 18
1794 18
1793 18
1793 19

And it looks like events are really messed up: here I changed numbers with words for better undestanding, and we see, that there is event in the middle of the sequence where second finger is released and placed again (but, of course, it's not true):

place first
hold first
hold first
place second
release second
place second
hold first
hold first
release first
release second

And here I tried to hold ONE finger for several seconds, so looks like 1794 is actually a "hold" event:

1792 20
1794 20
1794 20
1794 20
1794 20
1794 20
1794 20
1794 20
1794 20
1793 20

And finally, it's how actual movement looks like. Here I placed ONE finger on the screen, moved it several inches and released:

1792 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1794 22
1793 22

So seems like 1794 indicates that the finger is on the screen and is being sent more frequently when coordinates change.

oleg-derevenetz commented 1 year ago

there are always at least two events. Maybe they are "place" and "release"? In this case, 4 events block possibly includes also "hold" or "move" although I tried not moving my finger and release it instantly.

Yes, 1792 is "finger down" event, 1793 is "finger up" and 1794 is "finger motion" events.

fingerId seems to be increasing constantly. I wander if it's correct behavior?

There is nothing wrong with this as long as different fingers touching the touchscreen at the same time have unique IDs.

And it looks like events are really messed up: here I changed numbers with words for better understanding, and we see, that there is event in the middle of the sequence where second finger is released and placed again (but, of course, it's not true)

That was in fact the only explanation - something (SDL or, more likely, touchscreen driver) reports that the wrong finger was removed from the screen. This should not happen, this behavior is wrong.

drevoborod commented 1 year ago

By the way, are these events always stored in correct order? Couldn't there be several instances of events listener running in parralel or concurrently?

oleg-derevenetz commented 1 year ago

Couldn't there be several instances of events listener running in parralel or concurrently?

In fheroes2 there is single event loop, which asks SDL for the next event and processes it, that's all. External events of all types are processed via this single event loop, so no events can be reordered in our code. In any case, reporting of an incorrect finger ID would be difficult to attribute to some reordering.