SDL icon indicating copy to clipboard operation
SDL copied to clipboard

pen: Rework public API.

Open icculus opened this issue 1 year ago • 13 comments

This changes the API in various ways, and changes the implementation code just enough to support the API changes. A more thorough going over of the implementation and backends would be extremely useful, though, especially before we add more backends!

  • Cleans up bitflags so they aren't reused between unrelated functions.
  • Moves most of the SDL_GetPen* functions into a single SDL_GetPenInfo() function. (SDL_GetPenName() remains).
  • Adds documentation.

Untangling the bitflags was messy. I don't love the SDL_PEN_INPUT_SOMETHING define that is going to cause problems when the user tries to bitwise-AND it against event->ptip.pen_state, because they were supposed to use SDL_PEN_INPUT_SOMETHING_MASK instead, but this matches how we do mouse buttons. I feel like we should change one or both of these.

Reference Issue #9871.

icculus avatar May 31 '24 18:05 icculus

Silly question, why do we try to provide a stable GUID for pens? We don't do this for joysticks where theoretically it would be more useful...?

slouken avatar May 31 '24 18:05 slouken

I have no idea. I was wondering that, too.

icculus avatar May 31 '24 18:05 icculus

Honestly I want to do a massive simplification to this code, but I don't want to get bogged down in it for a long time.

icculus avatar May 31 '24 18:05 icculus

Honestly I want to do a massive simplification to this code, but I don't want to get bogged down in it for a long time.

Now's the time, before the ABI lock.

slouken avatar May 31 '24 19:05 slouken

Paging @whot (who I think is the right person to ask; sorry to bother if not)...

SDL3 is discovering tablets through Xinput on X11, and Wayland's zwp_tablet_manager_v2 protocol, and part of our existing code extracts a "wacom id" from these interfaces for a given tool, and uses it to decide some truths about the tool (name, tool type, number of buttons and axes) ...

Which gives us this wild and probably always-out-of-date table...

https://github.com/libsdl-org/SDL/blob/6e53a364141d6f5e95bfd805982a9b137bd5463d/src/events/SDL_pen.c#L920-L1021

My question: do we need this table? Surely XInput and zwp_tablet_manager_v2 are going to supply this same information on any reasonable system, since they're probably talking to libinput and libwacom behind the scenes and will have more robust and well-maintained device data available to them...right?

Am I totally wrong about this? Is there some situation where Wayland or XInput is going to say "this is a generic pen with no features" but we can figure out it has more from its magic serial number like this?

(Thanks!)

icculus avatar Jun 21 '24 04:06 icculus

uhm. sort-of, yes, no, all of the above :)

libinput uses libwacom (if built-in, let's assume that) to get information about the tablet. Any tool will thus have the right bits set, i.e. you won't get tilt on a tool that doesn't do tilt. Parts of that will be kernel (ABS_TILT_X), other come parts from libwacom but generally you should assume libinput provides you a correct tool and where it doesn't it's a bug (or some restriction that would prevent you from knowing the correct value as well).

The Wayland protocol is effectively equivalent to libinput's API, so the above is true for Wayland too.

For X11 - we have three possible tablet drivers:

  • xf86-input-evdev: bare minimum, let's ignore, shouldn't ever handle tablets unless the other drivers are missing
  • xf86-input-libinput: has good enough tablet support but will sort lower than...
  • xf86-input-wacom: the reference driver, for historical reasons.

xf86-input-libinput will model tablet support after libinput (i.e. you won't see an X11 device for your pen until it comes into proximity first and you'll see different devices for different tools) but it's not bug-for-bug compatible with xf86-input-wacom. We decided it's not worth the effort and are generally recommending xf86-input-wacom because applications are used to that one. And xf86-input-wacom does not do any of the nice features above, you get an X device that is generic enough for any tool and the only way you'll know there's no tilt is because you won't see axis updates for it.

So in that case - yes, you will find more information about the tool using the tool id that xf86-input-wacom shoves into an (asynchronously updated) device property and then querying libwacom for that. Except that you only get tool ids on Wacom devices, which means everyone else is going to have a "generic pen" and libwacom will tell you that does tilt + pressure even though the tablet in question may not have tilt (all of them have pressure). Rather a lot of those devices also re-use VID/PIDs so a static lookup table won't work anyway. And button counts on tablet pads are woefully wrong thanks to re-used firmware, we're fixing them one by one as people get upset. libwacom should give you the right number of buttons, provided you get the right device which for Huion/Gaomon/XP-Pen devices is really only possible if you use libwacom_new_from_path()[^1] and a 6.11 kernel with the updated digimend drivers that export the firmware ID as uniq.

So the answer to "do I get all the information to drop that table" is:

Wacom Not Wacom
Wayland yes yes
X11 via libwacom[^2] :clown_face:

Oh, and distance has never been supported by the xf86-input-wacom driver anyway (xf86-input-libinput doesn't have it either). Wayland has it.

[^1]: you get the device node from the "Device Node" X11 property or through zwp_tablet_v2.path [^2]: linuxwacom/libwacom#714 may be interested but realistically you could generate that table at build-time too, see e.g. the debug-device utility for a base

whot avatar Jun 21 '24 06:06 whot

Wow, this is more complicated than I expected. :)

Thanks for the extremely-detailed explanation! This was very helpful!

icculus avatar Jun 21 '24 14:06 icculus

Okay, the more I'm looking at this, the more I'm realizing that unless we're going to build something like we did for gamepads, we're not going to get meaningful device info or hotplugging out of most platforms. Even the Wayland interface declines to tell you how many buttons are on a stylus (even though libwacom offers this info), and that's by far the nicest target platform of them all for tablet support. Windows's RealTimeStylus can't tell you basic truths about most stylus details, and Cocoa doesn't enumerate the devices at all; you have to wait for an input event to find out there's a tablet connected!

So maybe we're overthinking this, and we should remove almost all of the Pen API, and just report pen input like we do for touch: here's an event because the pen is touching/moving/clicking...if it has five buttons, you'll find out when the user presses all five of them, but like any number of fingers, they don't exist until they come in contact and we can track them.

icculus avatar Jun 23 '24 20:06 icculus

and that's by far the nicest target platform of them all for tablet support

I think I'm going to frame this, thanks :)

Come to think of it: libwacom has a glib dependency and libudev (which we could make compile-time optional) but it should in theory should work under windows/macos too. It's not like a C wrapper around some text files is particularly platform-specific.

whot avatar Jun 23 '24 23:06 whot

Come to think of it: libwacom has a glib dependency and libudev (which we could make compile-time optional) but it should in theory should work under windows/macos too. It's not like a C wrapper around some text files is particularly platform-specific.

Yes, but I'm not sure we can reasonably enumerate the devices (and/or map them to the same device IDs as libwacom...?) to make doing so worth it.

If we were to do this, I'd probably take the time to write a json/whatever export tool from libwacom, like you mentioned, but I think for now, we'll just approach pen support humbly and simplify SDL's interface more.

icculus avatar Jun 24 '24 03:06 icculus

libwacom 2.12 has a new libwacom_new_from_builder() API so you give it as much information as you have (vid+pid is sufficient for Wacom (and other sane) devices, the rest is more complicated) and that's it. You don't actually need a local path or anything, libwacom_new_from_path() which everyone uses is for convenience on linux [^1], libwacom is just a quirky database. So as long as you can get vid/pid, you're 80% there.

[^1]: and to deal with the tablets that have information hidden in multiple places.

whot avatar Jun 24 '24 03:06 whot

One more thing, @whot: there doesn't appear to be a way in the Wayland protocol to decide if a tool is using the eraser or pen tip, for styluses that allow the user to flip them over to use different functionality...does Wayland treat these as two separate tools on the same tablet, or did I miss something really obvious here?

icculus avatar Jun 26 '24 17:06 icculus

yes, they're two separate tools and right now the only way to know whether it's a tip eraser or button eraser is checking with libwacom (which for styli will only be accurate for wacom pens that send a tool id, everyone else just says "I have a stylus"). See the hardware_id_wacom event in the tablet protocol.

Unfortunately, what I think you really want to know is somewhat impossible because MS in their infinite wisdom decreed that the eraser button at the firmware level must emulate the pen going out of proximity and then going back into proximity as eraser - as if you had flipped the pen around. This has been the case ever since the Surface 3 10 years ago, possibly longer - that was just the first time I grumbled about it :)

libinput won't work around this because the user-space heuristics are too unreliable but we may allow emulating an eraser through a button in the future because we can actually disable this behaviour in udev-hid-bpf and make eraser buttons behave like normal buttons there.

Anyway, unless you're presenting a configuration GUI where you need to label things this may not matter anyway - erasers are always a different tool, regardless whether they're triggered by a button or the other tip. So any functionality you can attach to the tool, it's not really different to having multiple styli.

Oh, and in case that wasn't clear yet: styli in wayland are independent objects so you can track them across multiple tablets (now there's a niche case waiting to happen ;) and, if they have a serial number (wacom only) that configuration can be persistent since you'll know it was the same stylus next time you use it. That's different to the xf86-input-wacom driver where the serial number is just shoved along in a property but it's otherwise all mangled into the same X device by default (unless you configure it otherwise).

whot avatar Jun 26 '24 22:06 whot

Finally managed to untangle the X11 code (still untested, but I'm reworking testpen,c right now).

icculus avatar Aug 08 '24 01:08 icculus

Woohoo! :)

slouken avatar Aug 08 '24 01:08 slouken

Okay, we've got X11 again. Here's a test app that tracks the pen position, with the box growing as I press down harder.

Screencast from 2024-08-08 02-29-08.webm

This is on a low-end Wacom tablet, so it doesn't have tilt/rotation/etc, but those should work, too.

icculus avatar Aug 08 '24 06:08 icculus

macOS support went in last night. Unlike Linux, you have to install a driver from Wacom, which feels like garbageware and needs all sorts of scary system permissions in the accessibility section to work. I'm genuinely surprised this feels so junky and user-hostile on a platform ostensibly for creatives, but okay, it works in SDL3 now. :)

Other platforms that need to be done still:

iOS can work with Apple Pencil (and probably nothing else, but that's okay), using the same UITouch interface we already use; you just check the event's "type" to see if it came from a pencil instead of a finger, then metrics like rotation and stuff will be available on the event. I have an Apple Pencil and an iPad here, so if I can coerce the thing to work with my developer account, I'll try to implement this.

Android supports styluses (and apparently can also use a USB tablet plugged into the device in some cases). Most Chromebooks with touchscreens and some phones/tablets can use a standard pen interface called "USI 2.0" ... the pens themself aren't too expensive. If my tablet will work with something here, I'll give it a try. Android takes the same approach as iOS: you get the same MotionEvent, but it has extra info for stylus input.

Emscripten offers pointer events, which are widely supported, but I haven't actually tried this, so it might be the APIs are available but never work with tablets on most systems...I'll find some example page and see what happens here.

Windows has several APIs for this, but I'm pretty sure the widest supported is RealTimeStylus, which goes back to Windows XP Tablet Edition (APIs included by default for Vista and later). There's also WinTab, which is installed when installing Wacom's drivers on Windows, but I'd probably avoid this. Windows Ink is a Windows 10 thing and probably higher level than we want. I don't know what WinRT offers, but surely something.

Of all these, I want to get Windows up and running in SDL3, and then merge this PR. After ABI lock, we can return to fill in other platforms if there's time and usable hardware.

icculus avatar Aug 09 '24 22:08 icculus

(Alternately, we can merge this now, since Windows or not, I'm pretty comfortable that the SDL API isn't going to change.)

icculus avatar Aug 09 '24 22:08 icculus

Let's merge it now and bang on the Windows implementation.

slouken avatar Aug 09 '24 23:08 slouken

Okay, this is rebased to the latest in main and flattened down into two commits: the API rework (with the existing X11, Wayland, and test app reworked with it), and the addition of the Cocoa backend.

No changes were made beyond smushing everything into less commits and rebasing.

Once the builders are green, I'm clicking merge!

icculus avatar Aug 09 '24 23:08 icculus

Okay, we're live. Thanks again to @whot for all the insight he provided here!

icculus avatar Aug 10 '24 02:08 icculus

How did this happen: https://github.com/libsdl-org/sdl2-compat/actions/runs/10331541340/job/28601815563

CC: @madebr

sezero avatar Aug 10 '24 12:08 sezero

How did this happen: https://github.com/libsdl-org/sdl2-compat/actions/runs/10331541340/job/28601815563

CC: @madebr

sdl2-compat's ci does not install the development dependencies of SDL3. When configuring SDL with -DSDL_X11_XINPUT=OFF, I can reproduce.

madebr avatar Aug 10 '24 12:08 madebr

OK, pushed https://github.com/libsdl-org/SDL/commit/f93920a4f16c4d16f6cd3797191e7894542dac4b to work around it.

@icculus: revise it as you see fit.

sezero avatar Aug 10 '24 12:08 sezero

OK, pushed f93920a to work around it.

@icculus: revise it as you see fit.

This looks fine to me. Thanks!

slouken avatar Aug 10 '24 14:08 slouken