libsdl-org / SDL

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

Touchpad Mouse Conflation Problem #5511

Closed fhoenig closed 2 years ago

fhoenig commented 2 years ago

I've tried to get touch events from my Mac touchpad and by default no SDL_FINGER* nor SDL_MULTIGESTURE event are generated.

int td = SDL_GetNumTouchDevices(); // td is 1 here in my case So I do have one touch device, which is correct.

Now I found that the only way to generate touch event from my touch device is to tell SDL that it is in fact a mouse. SDL_SetHint(SDL_HINT_MOUSE_TOUCH_EVENTS, "1");

However this now also generates SDL_FINGER* events from the actual mouse I have connected, which isn't a touch device.

How can I tell SDL that I want the finger events only from the touch device?

slouken commented 2 years ago

The Mac touchpad is treated as a mouse, because that matches general user expectations.

Here's the relevant code in SDL_cocoawindow.m:

- (void)handleTouches:(NSTouchPhase) phase withEvent:(NSEvent *) theEvent
{
    NSSet *touches = [theEvent touchesMatchingPhase:phase inView:nil];

    /* probably a MacBook trackpad; make this look like a synthesized event.
       This is backwards from reality, but better matches user expectations. */
    BOOL istrackpad = NO;
    @try {
        istrackpad = ([theEvent subtype] == NSEventSubtypeMouseEvent);
    }
slouken commented 2 years ago

We could potentially add a hint to allow applications to change how this is handled. Adding @icculus for more discussion.

icculus commented 2 years ago

Yeah, this has been an ongoing thing...the hint is probably the right thing to do here. I'll fix that up for 2.0.24.

icculus commented 2 years ago

Just pasting this untested thing here until I can sit at the desk where the Mac Mini is.

(But feedback is welcome in the meantime!)

diff --git a/include/SDL_hints.h b/include/SDL_hints.h
index 368cb0a3c..9da512683 100644
--- a/include/SDL_hints.h
+++ b/include/SDL_hints.h
@@ -2175,6 +2175,24 @@ extern "C" {
 #define SDL_HINT_KMSDRM_DEVICE_INDEX "SDL_KMSDRM_DEVICE_INDEX"

+/**
+ *  \brief  A variable that treats MacBook trackpads as touch devices.
+ *
+ *  On macOS, SDL will report touches on the trackpad as mouse input, which
+ *  is generally what users expect from this device; however, these are
+ *  actually full multitouch-capable touch devices, so it might be
+ *  preferable to some apps to treat them as such.
+ *
+ *  Setting this hint to true will make the trackpad input report as a
+ *  multitouch device instead of a mouse. The default is false.
+ *
+ *  This hint is checked during SDL_Init and can not be changed after.
+ *
+ *  This hint is available since SDL 2.24.0.
+ */
+#define SDL_HINT_MAC_TRACKPAD_IS_TOUCH_ONLY "SDL_MAC_TRACKPAD_IS_TOUCH_ONLY"
+
+
 /**
  *  \brief  An enumeration of hint priorities
  */
diff --git a/src/video/cocoa/SDL_cocoavideo.h b/src/video/cocoa/SDL_cocoavideo.h
index 4de8511a3..8f4a413e9 100644
--- a/src/video/cocoa/SDL_cocoavideo.h
+++ b/src/video/cocoa/SDL_cocoavideo.h
@@ -99,6 +99,7 @@ DECLARE_ALERT_STYLE(Critical);

 @interface SDL_VideoData : NSObject
     @property (nonatomic) int allow_spaces;
+    @property (nonatomic) int trackpad_is_touch_only;
     @property (nonatomic) unsigned int modifierFlags;
     @property (nonatomic) void *key_layout;
     @property (nonatomic) SDLTranslatorResponder *fieldEdit;
diff --git a/src/video/cocoa/SDL_cocoavideo.m b/src/video/cocoa/SDL_cocoavideo.m
index 987fa2d4d..94c1b4020 100644
--- a/src/video/cocoa/SDL_cocoavideo.m
+++ b/src/video/cocoa/SDL_cocoavideo.m
@@ -197,6 +197,7 @@ Cocoa_VideoInit(_THIS)
     }

     data.allow_spaces = SDL_GetHintBoolean(SDL_HINT_VIDEO_MAC_FULLSCREEN_SPACES, SDL_TRUE);
+    data.trackpad_is_touch_only = SDL_GetHintBoolean(SDL_HINT_MAC_TRACKPAD_IS_TOUCH_ONLY, SDL_FALSE);

     data.swaplock = SDL_CreateMutex();
     if (!data.swaplock) {
diff --git a/src/video/cocoa/SDL_cocoawindow.h b/src/video/cocoa/SDL_cocoawindow.h
index 9691fa3f1..ec25e1f57 100644
--- a/src/video/cocoa/SDL_cocoawindow.h
+++ b/src/video/cocoa/SDL_cocoawindow.h
@@ -56,6 +56,7 @@ typedef enum
     BOOL isDragAreaRunning;
 }

+-(BOOL) isTouchFromTrackpad:(NSEvent *)theEvent;
 -(void) listen:(SDL_WindowData *) data;
 -(void) pauseVisibleObservation;
 -(void) resumeVisibleObservation;
diff --git a/src/video/cocoa/SDL_cocoawindow.m b/src/video/cocoa/SDL_cocoawindow.m
index 1e47390be..83209b802 100644
--- a/src/video/cocoa/SDL_cocoawindow.m
+++ b/src/video/cocoa/SDL_cocoawindow.m
@@ -1364,26 +1364,38 @@ Cocoa_SendMouseButtonClicks(SDL_Mouse * mouse, NSEvent *theEvent, SDL_Window * w
     Cocoa_HandleMouseWheel(_data.window, theEvent);
 }

+
+- (BOOL)isTouchFromTrackpad:(NSEvent *)theEvent
+{
+    SDL_VideoData *videodata = ((__bridge SDL_WindowData *) window->driverdata).videodata;
+
+    /* if this a MacBook trackpad, we'll make input look like a synthesized
+       event. This is backwards from reality, but better matches user
+       expectations. You can make it look like a generic touch device instead
+       with the SDL_HINT_MAC_TRACKPAD_IS_TOUCH_ONLY hint. */
+    BOOL istrackpad = NO;
+    if (!videodata.trackpad_is_touch_only) {
+        @try {
+            istrackpad = ([theEvent subtype] == NSEventSubtypeMouseEvent);
+        }
+        @catch (NSException *e) {
+            /* if NSEvent type doesn't have subtype, such as NSEventTypeBeginGesture on
+             * macOS 10.5 to 10.10, then NSInternalInconsistencyException is thrown.
+             * This still prints a message to terminal so catching it's not an ideal solution.
+             *
+             * *** Assertion failure in -[NSEvent subtype]
+             */
+        }
+    }
+    return istrackpad;
+}
+
 - (void)touchesBeganWithEvent:(NSEvent *) theEvent
 {
     NSSet *touches;
     SDL_TouchID touchID;
     int existingTouchCount;
-
-    /* probably a MacBook trackpad; make this look like a synthesized event.
-       This is backwards from reality, but better matches user expectations. */
-    BOOL istrackpad = NO;
-    @try {
-        istrackpad = ([theEvent subtype] == NSEventSubtypeMouseEvent);
-    }
-    @catch (NSException *e) {
-        /* if NSEvent type doesn't have subtype, such as NSEventTypeBeginGesture on
-         * macOS 10.5 to 10.10, then NSInternalInconsistencyException is thrown.
-         * This still prints a message to terminal so catching it's not an ideal solution.
-         *
-         * *** Assertion failure in -[NSEvent subtype]
-         */
-    }
+    const BOOL istrackpad = [self isTouchFromTrackpad:theEvent]

     touches = [theEvent touchesMatchingPhase:NSTouchPhaseAny inView:nil];
     touchID = istrackpad ? SDL_MOUSE_TOUCHID : (SDL_TouchID)(intptr_t)[[touches anyObject] device];
@@ -1431,24 +1443,10 @@ Cocoa_SendMouseButtonClicks(SDL_Mouse * mouse, NSEvent *theEvent, SDL_Window * w
 - (void)handleTouches:(NSTouchPhase) phase withEvent:(NSEvent *) theEvent
 {
     NSSet *touches = [theEvent touchesMatchingPhase:phase inView:nil];
+    const BOOL istrackpad = [self isTouchFromTrackpad:theEvent]
     SDL_FingerID fingerId;
     float x, y;

-    /* probably a MacBook trackpad; make this look like a synthesized event.
-       This is backwards from reality, but better matches user expectations. */
-    BOOL istrackpad = NO;
-    @try {
-        istrackpad = ([theEvent subtype] == NSEventSubtypeMouseEvent);
-    }
-    @catch (NSException *e) {
-        /* if NSEvent type doesn't have subtype, such as NSEventTypeBeginGesture on
-         * macOS 10.5 to 10.10, then NSInternalInconsistencyException is thrown.
-         * This still prints a message to terminal so catching it's not an ideal solution.
-         *
-         * *** Assertion failure in -[NSEvent subtype]
-         */
-    }
-
     for (NSTouch *touch in touches) {
         const SDL_TouchID touchId = istrackpad ? SDL_MOUSE_TOUCHID : (SDL_TouchID)(intptr_t)[touch device];
         SDL_TouchDeviceType devtype = SDL_TOUCH_DEVICE_INDIRECT_ABSOLUTE;
cgutman commented 2 years ago

Let’s do a non-platform-specific hint because we’ll need the same option for Evdev trackpad support too.

icculus commented 2 years ago

My understanding is that this is a special case in our macOS handling...are you sure?

cgutman commented 2 years ago

Right now all evdev multi-touch trackpads are reporting touch events rather than mouse events. We'll probably want to change it to synthesize mouse events like macOS and provide an opt-out for apps that want the raw touch events back.

icculus commented 2 years ago

But for 2.24.0, you just want the word "MAC" out of the hint, so we can later apply it to evdev, right? If so, I'm fine with that.

cgutman commented 2 years ago

Yeah, just wanted to avoid painting ourselves into a corner and needing a second hint.