libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.97k stars 1.84k forks source link

Proper way of picking up multitouch events from a tablet? #4819

Closed franciscod closed 3 years ago

franciscod commented 3 years ago

Hello, I've been trying to make SDL on Linux to pick up multitouch (finger) events from a Wacom tablet on a Linux machine.

After experimenting a bit, I came up with the following, which works! (see patch below)

A few questions: 1) Does the integration between the X11 and evdev stuff make sense? 2) Should I be changing the udev class of my device from outside somehow? (No, it has the FINGER flag) 3) Could any of this (probably after putting some more thought and work, for sure) be added into SDL? I'd love to help with it.

I guess that there's the unexpected situation where a touchpad in a laptop surprisingly acts as a multitouch source and that might be confusing... but would be cool to have support for this.

Thanks!


diff --git a/src/core/linux/SDL_evdev.c b/src/core/linux/SDL_evdev.c
index bd4fcf6cd..7558c85de 100644
--- a/src/core/linux/SDL_evdev.c
+++ b/src/core/linux/SDL_evdev.c
@@ -218,6 +218,10 @@ static void SDL_EVDEV_udev_callback(SDL_UDEV_deviceevent udev_event, int udev_cl

     switch(udev_event) {
     case SDL_UDEV_DEVICEADDED:
+        if (strstr(dev_path, "event19") != NULL) {
+            udev_class |= SDL_UDEV_DEVICE_TOUCHSCREEN;
+        }
+
         if (!(udev_class & (SDL_UDEV_DEVICE_MOUSE | SDL_UDEV_DEVICE_KEYBOARD |
             SDL_UDEV_DEVICE_TOUCHSCREEN)))
             return;
diff --git a/src/video/x11/SDL_x11events.c b/src/video/x11/SDL_x11events.c
index f91131305..4be5cab3a 100644
--- a/src/video/x11/SDL_x11events.c
+++ b/src/video/x11/SDL_x11events.c
@@ -22,6 +22,10 @@

 #if SDL_VIDEO_DRIVER_X11

+#ifdef SDL_INPUT_LINUXEV
+#include "../../core/linux/SDL_evdev.h"
+#endif
+
 #include <sys/types.h>
 #include <sys/time.h>
 #include <signal.h>
@@ -1640,6 +1644,10 @@ X11_PumpEvents(_THIS)
             X11_FlashWindow(_this, data->windowlist[i]->window, SDL_FLASH_CANCEL);
         }
     }
+
+#ifdef SDL_INPUT_LINUXEV
+    SDL_EVDEV_Poll();
+#endif
 }

diff --git a/src/video/x11/SDL_x11video.c b/src/video/x11/SDL_x11video.c
index f1d10410a..61c2c499b 100644
--- a/src/video/x11/SDL_x11video.c
+++ b/src/video/x11/SDL_x11video.c
@@ -22,6 +22,10 @@

 #if SDL_VIDEO_DRIVER_X11

+#ifdef SDL_INPUT_LINUXEV
+#include "../../core/linux/SDL_evdev.h"
+#endif
+
 #include <unistd.h> /* For getpid() and readlink() */

 #include "SDL_video.h"
@@ -453,6 +457,12 @@ X11_VideoInit(_THIS)

     X11_InitTouch(_this);

+#ifdef SDL_INPUT_LINUXEV    
+    if (SDL_EVDEV_Init() < 0) {
+        return -1;
+    }
+#endif    
+
     return 0;
 }

@@ -476,6 +486,10 @@ X11_VideoQuit(_THIS)
     X11_QuitKeyboard(_this);
     X11_QuitMouse(_this);
     X11_QuitTouch(_this);
+
+#ifdef SDL_INPUT_LINUXEV    
+    SDL_EVDEV_Quit();
+#endif    
 }

 SDL_bool
franciscod commented 3 years ago

A mega-cool guy helped me find a cleaner way of doing the detection. Looks like there's a placeholder for ID_INPUT_TOUCHPAD, and with the obvious patch, it works perfectly.

I'm still considering touchpads as touchscreens so I don't need to change every event handler.

This enables finger multitouch gestures both for an external tablet and a builtin synaptics touchpad.

diff --git a/src/core/linux/SDL_evdev.c b/src/core/linux/SDL_evdev.c
index 7558c85de..175f20f09 100644
--- a/src/core/linux/SDL_evdev.c
+++ b/src/core/linux/SDL_evdev.c
@@ -218,7 +218,7 @@ static void SDL_EVDEV_udev_callback(SDL_UDEV_deviceevent udev_event, int udev_cl

     switch(udev_event) {
     case SDL_UDEV_DEVICEADDED:
-        if(strstr(dev_path, "event19") != NULL) {
+        if (udev_class & SDL_UDEV_DEVICE_TOUCHPAD) {
             udev_class |= SDL_UDEV_DEVICE_TOUCHSCREEN;
         }

diff --git a/src/core/linux/SDL_evdev_capabilities.c b/src/core/linux/SDL_evdev_capabilities.c
index 1373f0e9b..72bdba17f 100644
--- a/src/core/linux/SDL_evdev_capabilities.c
+++ b/src/core/linux/SDL_evdev_capabilities.c
@@ -78,7 +78,7 @@ SDL_EVDEV_GuessDeviceClass(unsigned long bitmask_ev[NBITS(EV_MAX)],
         if (test_bit(BTN_STYLUS, bitmask_key) || test_bit(BTN_TOOL_PEN, bitmask_key)) {
             ; /* ID_INPUT_TABLET */
         } else if (test_bit(BTN_TOOL_FINGER, bitmask_key) && !test_bit(BTN_TOOL_PEN, bitmask_key)) {
-            ; /* ID_INPUT_TOUCHPAD */
+            devclass |= SDL_UDEV_DEVICE_TOUCHPAD; /* ID_INPUT_TOUCHPAD */
         } else if (test_bit(BTN_MOUSE, bitmask_key)) {
             devclass |= SDL_UDEV_DEVICE_MOUSE; /* ID_INPUT_MOUSE */
         } else if (test_bit(BTN_TOUCH, bitmask_key)) {
diff --git a/src/core/linux/SDL_evdev_capabilities.h b/src/core/linux/SDL_evdev_capabilities.h
index 6822425eb..6167ebbf8 100644
--- a/src/core/linux/SDL_evdev_capabilities.h
+++ b/src/core/linux/SDL_evdev_capabilities.h
@@ -38,7 +38,8 @@ typedef enum
     SDL_UDEV_DEVICE_JOYSTICK    = 0x0004,
     SDL_UDEV_DEVICE_SOUND       = 0x0008,
     SDL_UDEV_DEVICE_TOUCHSCREEN = 0x0010,
-    SDL_UDEV_DEVICE_ACCELEROMETER = 0x0020
+    SDL_UDEV_DEVICE_ACCELEROMETER = 0x0020,
+    SDL_UDEV_DEVICE_TOUCHPAD    = 0x0040
 } SDL_UDEV_deviceclass;

 #define BITS_PER_LONG           (sizeof(unsigned long) * 8)
slouken commented 3 years ago

XInput should be picking this up and providing events via X11. Are you building SDL with XInput support?

slouken commented 3 years ago

I'm not sure about the X11 portion, but the evdev changes look good to me, and I've added them in https://github.com/libsdl-org/SDL/commit/373216ae5be62b710ad68524777ae38ca712c53d

franciscod commented 3 years ago

XInput should be picking this up and providing events via X11. Are you building SDL with XInput support?

I think so: here's a snip from my CMake build dir:

//Enable XInput support
VIDEO_X11_XINPUT:BOOL=ON
--
//Use Xinput for Windows
XINPUT:BOOL=OFF
--
//Have include X11/extensions/XInput2.h
HAVE_XINPUT2_H:INTERNAL=1
//Test HAVE_XINPUT2_MULTITOUCH
HAVE_XINPUT2_MULTITOUCH:INTERNAL=1

the evdev changes look good to me, and I've added them in 373216a

Great, thanks! I'll try if that's enough. Maybe the X11 portion wasn't required...

franciscod commented 3 years ago

Seems like the XInput path provides SDL_FINGERMOTION events (individual fingers?), but not SDL_MULTIGESTURE (compound gestures like pinch zoom / rotate).

slouken commented 3 years ago

SDL_MULTIGESTURE is always available when finger motion events are delivered. However I seem to remember that evdev and X11 don't return the same coordinate system (one is normalized and the other is not), can you confirm?

slouken commented 3 years ago

I just confirmed that X11 returns normalized events and testgesture handles them properly. I also confirmed that SDL_MULTIGESTURE events are generated in this case. You can see this by setting the environment variable SDL_EVENT_LOGGING=1 and watching the debug output.

I'll go ahead and close this, but please let me know if you're seeing something different.

franciscod commented 3 years ago

OK! I think I didn't receive events without the evdev init code that I added (but haven't checked again). That environment variable sure looks handy. Thanks!