touch issues with sway nightly

[hrdl-github]

• KOReader version: 2023.04
• Device: Surface Go (mainline Linux), PineNote (both sway nightly with the wayland backend)

Issue

I've been using sway nightly as it features support for cancelling gestures. Before swaywm/sway@4666d17 touch events did not affect the pointer, which is consistent which e.g. plasma-wayland. Since swaywm/sway@4666d17 touch down events induce a cursor leave event if it was on the same surface, and the last touch up event induces a cursor enter event. See swaywm/sway#7577 for some sample wayland events.

When the cursor is on KOReader and a touch down and a touch up event are induced to initiate a tap, SDL also fires a TOUCHMOTION event, probably due to the cursor being restored. This results in not a tap gesture, but a pan gesture.

I can provide some logs if you think this makes sense, but I think my description covers everything of relevance. I saw that koreader/koreader-base#1599 introduced a non-working (and therefore commented out) check https://github.com/koreader/koreader-base/blob/66b21534def34109c478759adb75364ac13694a4/ffi/SDL2_0.lua#L369
The check does not work because event.motion.which == 0 (vs -1).

I'm not sure yet if this is an issue with KOReader, SDL, or sway. I'm filing an issue here because I'm beginning to think that sway's behaviour is not unreasonable despite differing from e.g. plasma-wayland.

Steps to reproduce

1. Install a recent nightly version of sway (and wlroots)
2. Launch KOReader. Place the cursor on KOReader
3. Initiate a touch event with the intention to tap

[hrdl-github]

This is where SDL creates the pointer motion event when processing the pointer enter event: https://github.com/libsdl-org/SDL/blob/237348c772b4ff0e758ace83f471dbf8570535e2/src/video/wayland/SDL_waylandevents.c#L452 This one does obviously not belong to the touch input device.

[Frenzie]

I saw that koreader/koreader-base#1599 introduced a non-working (and therefore commented out) check https://github.com/koreader/koreader-base/blob/66b21534def34109c478759adb75364ac13694a4/ffi/SDL2_0.lua#L369

Either the comment or it being there at all is a mistake.

[Frenzie]

I can provide some logs if you think this makes sense, but I think my description covers everything of relevance. I saw that koreader/koreader-base#1599 introduced a non-working (and therefore commented out) check https://github.com/koreader/koreader-base/blob/66b21534def34109c478759adb75364ac13694a4/ffi/SDL2_0.lua#L369
The check does not work because event.motion.which == 0 (vs -1).

You managed to confuse me with "non-working (and therefore commented out)". ^_^ It works perfectly; I'd merely accidentally left it commented for whatever reason because the overall effect is indistinguishable for our purposes. It's just an extra event saying effectively the same thing.

[Frenzie]

When the cursor is on KOReader and a touch down and a touch up event are induced to initiate a tap, SDL also fires a TOUCHMOTION event, probably due to the cursor being restored. This results in not a tap gesture, but a pan gesture.

I can provide some logs if you think this makes sense

This description doesn't make much sense to me actually, because you're simply describing a completely normal click or tap. There will often be a few pixels of movement.

[Frenzie]

In short, if you want to trigger an accidental swipe you have to give it a really enthusiastic try. An intentional tap rarely generates more than about 5-10 physical px of movement even at 300 DPI, and note the scaleBySize() on there meaning you'd have to go closer to 70. Which is simple to do, just a little flick, but if you're having a fake mouse movement at the same spot where you just lifted your finger, no matter how silly that might be for SDL to do, it doesn't seem that it should normally cause any issues.

https://github.com/koreader/koreader/blob/d8b3878e8afd700f138606e8e720611f289cc0e2/frontend/device/gesturedetector.lua#LL117C22-L117C22

[hrdl-github]

Let me try again: the TOUCHMOTION event has the coordinates of the cursor and not the touch point, which causes a very large pan (or gesture) instead of a click.

[hrdl-github]

This is what it looks like. The touch point has coordinates (753, 208), the pointer was resting at (66, 467) before the touch event occurred. I've added a few print statements to make my life easier.

down	1792	true	0	752.54689717293	207.96093463898
genTouchDownEvent	0	true	0	752.54689717293	207.96093463898
processing 	5	events, event.type=	1792
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 47 (ABS_MT_SLOT), value: 0, time: 8155.808939 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 57 (ABS_MT_TRACKING_ID), value: 0, time: 8155.808939 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 53 (ABS_MT_POSITION_X), value: 752.54689717293, time: 8155.808939 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 54 (ABS_MT_POSITION_Y), value: 207.96093463898, time: 8155.808939 
05/10/23-22:36:01 DEBUG input event => type: 0 (EV_SYN), code: 0 (SYN_REPORT), value: 0, time: 8155.808939 
05/10/23-22:36:01 DEBUG GestureDetector:probeClockSource: Touch event timestamps appear to use CLOCK_MONOTONIC 
05/10/23-22:36:01 DEBUG slot 0 in tap state... 
05/10/23-22:36:01 DEBUG handleNonTap new_tap true  hold timer  false 
05/10/23-22:36:01 DEBUG set up hold timer 
processing 	4	events, event.type=	1024
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 47 (ABS_MT_SLOT), value: 0, time: 8155.835575 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 53 (ABS_MT_POSITION_X), value: 66, time: 8155.835575 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 54 (ABS_MT_POSITION_Y), value: 467, time: 8155.835575 
05/10/23-22:36:01 DEBUG input event => type: 0 (EV_SYN), code: 0 (SYN_REPORT), value: 0, time: 8155.835575 
05/10/23-22:36:01 DEBUG slot 0 in tap state... 
05/10/23-22:36:01 DEBUG handleNonTap new_tap nil  hold timer  true 
05/10/23-22:36:01 DEBUG slot 0 in pan state... 
up	true	0	752.54689717293	207.96093463898
genTouchDownEvent	0	752.54689717293	207.96093463898
processing 	5	events, event.type=	1793
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 47 (ABS_MT_SLOT), value: 0, time: 8155.842234 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 57 (ABS_MT_TRACKING_ID), value: -1, time: 8155.842234 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 53 (ABS_MT_POSITION_X), value: 752.54689717293, time: 8155.842234 
05/10/23-22:36:01 DEBUG input event => type: 3 (EV_ABS), code: 54 (ABS_MT_POSITION_Y), value: 207.96093463898, time: 8155.842234 
05/10/23-22:36:01 DEBUG input event => type: 0 (EV_SYN), code: 0 (SYN_REPORT), value: 0, time: 8155.842234 
05/10/23-22:36:01 DEBUG slot 0 in pan state... 
05/10/23-22:36:01 DEBUG Contact:handlePanRelease: pan release detected 

[hrdl-github]

Both the first touch and the pointer are assigned ABS_MT_SLOT 0. This means that https://github.com/koreader/koreader-base/blob/66b21534def34109c478759adb75364ac13694a4/ffi/SDL2_0.lua#L378 succeeds, causing the TOUCHMOTION event to be accepted. When no slots are active the TOUCHMOTION event would be discarded.

This should be fixable by extending said check and saving some information to distinguish between touch and pointer events. What do you think?

[Frenzie]

There's nothing to fix. Or that's not true exactly, as I stated here a tester can easily trigger this scenario by using a mouse and a touchscreen simultaneously,[1] which could be avoided by using first come first serve, but the specific problem here is a bug in sway or SDL. That mouse movement should come after up, not before.

[1] Though much more easily in the reverse manner by clicking with a pointing device and doing a tap somewhere with a finger to trigger a swipe.

[Frenzie]

Anyway, I was planning to fix that actual issue eventually but you're welcome to submit a PR. ;-)

[hrdl-github]

Thanks -- I'll bring it up in my sway issue. I'm going to test the PR that I submitted just now. Thanks for looking into this!

[hrdl-github]

Fixed by swaywm/sway#7583.