Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pen: Rework public API. #9938

Merged
merged 2 commits into from
Aug 10, 2024
Merged

Conversation

icculus
Copy link
Collaborator

@icculus icculus commented May 31, 2024

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 icculus added this to the 3.0 ABI milestone May 31, 2024
include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
@slouken
Copy link
Collaborator

slouken commented May 31, 2024

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...?

@icculus
Copy link
Collaborator Author

icculus commented May 31, 2024

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

@icculus
Copy link
Collaborator Author

icculus commented May 31, 2024

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.

@slouken
Copy link
Collaborator

slouken commented May 31, 2024

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.

@icculus icculus force-pushed the sdl3-review-pen-api branch from 7429f7a to 283f633 Compare June 18, 2024 16:36
@icculus
Copy link
Collaborator Author

icculus commented Jun 21, 2024

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...

SDL/src/events/SDL_pen.c

Lines 920 to 1021 in 6e53a36

/* Returns a suitable pen name string from default_pen_names on success, otherwise NULL. */
static const char *pen_wacom_identify_tool(Uint32 requested_wacom_id, int *num_buttons, int *tool_type, int *axes)
{
int i;
/* List of known Wacom pens, extracted from libwacom.stylus and wacom_wac.c in the Linux kernel.
Could be complemented by dlopen()ing libwacom, in the future (if new pens get added). */
struct
{
/* Compress properties to 8 bytes per device in order to keep memory cost well below 1k.
Could be compressed further with more complex code. */
Uint32 wacom_id; /* Must be != PEN_WACOM_ID_INVALID */
Uint32 properties;
} tools[] = {
{ 0x0001, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0011, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0019, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0021, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0031, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0039, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0049, PEN_SPEC(PEN_NAME_GENERAL, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0071, PEN_SPEC(PEN_NAME_GENERAL, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0200, PEN_SPEC(PEN_NAME_PRO3, 3, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0221, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0231, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0271, PEN_SPEC(PEN_NAME_GENERAL, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0421, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0431, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0621, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0631, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK) },
{ 0x0801, PEN_SPEC(PEN_NAME_INKING, 0, SDL_PEN_TYPE_PENCIL, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0802, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0804, PEN_SPEC(PEN_NAME_ART, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK | SDL_PEN_AXIS_ROTATION_MASK) },
{ 0x080a, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x080c, PEN_SPEC(PEN_NAME_ART, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0812, PEN_SPEC(PEN_NAME_INKING, 0, SDL_PEN_TYPE_PENCIL, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0813, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x081b, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0822, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0823, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x082a, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x082b, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0832, PEN_SPEC(PEN_NAME_STROKE, 0, SDL_PEN_TYPE_BRUSH, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0842, PEN_SPEC(PEN_NAME_PRO2, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x084a, PEN_SPEC(PEN_NAME_PRO2, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0852, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x085a, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0862, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0885, PEN_SPEC(PEN_NAME_ART, 0, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK | SDL_PEN_AXIS_ROTATION_MASK) },
{ 0x08e2, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0902, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_AIRBRUSH, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK | SDL_PEN_AXIS_SLIDER_MASK) },
{ 0x090a, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0912, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_AIRBRUSH, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK | SDL_PEN_AXIS_SLIDER_MASK) },
{ 0x0913, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_AIRBRUSH, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x091a, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x091b, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x0d12, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_AIRBRUSH, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK | SDL_PEN_AXIS_SLIDER_MASK) },
{ 0x0d1a, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x8051, PEN_SPEC(PEN_NAME_AES, 0, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK) },
{ 0x805b, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK) },
{ 0x806b, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK) },
{ 0x807b, PEN_SPEC(PEN_NAME_GENERAL, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK) },
{ 0x826b, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK) },
{ 0x846b, PEN_SPEC(PEN_NAME_AES, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK) },
{ 0x2802, PEN_SPEC(PEN_NAME_INKING, 0, SDL_PEN_TYPE_PENCIL, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x4200, PEN_SPEC(PEN_NAME_PRO3, 3, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x4802, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x480a, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x8842, PEN_SPEC(PEN_NAME_PRO3D, 3, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x10802, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x10804, PEN_SPEC(PEN_NAME_ART, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK | SDL_PEN_AXIS_ROTATION_MASK) },
{ 0x1080a, PEN_SPEC(PEN_NAME_GRIP, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x1080c, PEN_SPEC(PEN_NAME_ART, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x10842, PEN_SPEC(PEN_NAME_PRO_SLIM, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x1084a, PEN_SPEC(PEN_NAME_PRO_SLIM, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x10902, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_AIRBRUSH, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK | SDL_PEN_AXIS_SLIDER_MASK) },
{ 0x1090a, PEN_SPEC(PEN_NAME_AIRBRUSH, 1, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x12802, PEN_SPEC(PEN_NAME_INKING, 0, SDL_PEN_TYPE_PENCIL, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x14802, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x1480a, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x16802, PEN_SPEC(PEN_NAME_PRO, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x1680a, PEN_SPEC(PEN_NAME_PRO, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x18802, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_PEN, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0x1880a, PEN_SPEC(PEN_NAME_GENERAL, 2, SDL_PEN_TYPE_ERASER, SDL_PEN_AXIS_PRESSURE_MASK | SDL_PEN_AXIS_XTILT_MASK | SDL_PEN_AXIS_YTILT_MASK | SDL_PEN_AXIS_DISTANCE_MASK) },
{ 0, 0 }
};
/* The list of pens is sorted, so we could do binary search, but this call should be pretty rare. */
for (i = 0; tools[i].wacom_id; ++i) {
if (tools[i].wacom_id == requested_wacom_id) {
Uint32 properties = tools[i].properties;
int name_index = (properties & PEN_SPEC_NAME_MASK) >> PEN_SPEC_NAME_SHIFT;
*num_buttons = (properties & PEN_SPEC_BUTTONS_MASK) >> PEN_SPEC_BUTTONS_SHIFT;
*tool_type = (properties & PEN_SPEC_TYPE_MASK) >> PEN_SPEC_TYPE_SHIFT;
*axes = (properties & PEN_SPEC_AXES_MASK) >> PEN_SPEC_AXES_SHIFT;
return default_pen_names[name_index];
}
}
return NULL;
}

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!)

@whot
Copy link

whot commented Jun 21, 2024

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 libwacom2 🤡

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.

Footnotes

  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

@icculus
Copy link
Collaborator Author

icculus commented Jun 21, 2024

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

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

@icculus
Copy link
Collaborator Author

icculus commented Jun 23, 2024

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.

@whot
Copy link

whot commented Jun 23, 2024

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.

@icculus
Copy link
Collaborator Author

icculus commented Jun 24, 2024

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.

@whot
Copy link

whot commented Jun 24, 2024

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.

Footnotes

  1. and to deal with the tablets that have information hidden in multiple places.

@icculus icculus force-pushed the sdl3-review-pen-api branch from 283f633 to 48505d3 Compare June 26, 2024 17:11
@icculus
Copy link
Collaborator Author

icculus commented Jun 26, 2024

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?

@whot
Copy link

whot commented Jun 26, 2024

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).

@icculus icculus force-pushed the sdl3-review-pen-api branch from 48505d3 to 6c73af3 Compare July 20, 2024 16:40
@icculus icculus force-pushed the sdl3-review-pen-api branch from 08fd099 to 26d443f Compare August 8, 2024 01:11
@icculus
Copy link
Collaborator Author

icculus commented Aug 8, 2024

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

@slouken
Copy link
Collaborator

slouken commented Aug 8, 2024

Woohoo! :)

include/SDL3/SDL_events.h Outdated Show resolved Hide resolved
@icculus
Copy link
Collaborator Author

icculus commented Aug 8, 2024

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 icculus force-pushed the sdl3-review-pen-api branch from 624f06e to 8176a59 Compare August 8, 2024 06:45
@icculus
Copy link
Collaborator Author

icculus commented Aug 9, 2024

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
Copy link
Collaborator Author

icculus commented Aug 9, 2024

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

@slouken
Copy link
Collaborator

slouken commented Aug 9, 2024

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

This changes the API in various ways, and updates the backends for this.

Overall, this is a massive simplification of the API, as most future backends
can't support the previously-offered API.

This also removes the testautomation pen code (not only did these interfaces
change completely, it also did something no other test did: mock the internal
API), and replaces testpen.c with a different implementation (the existing
code was fine, it was just easier to start from scratch than update it).
@icculus icculus force-pushed the sdl3-review-pen-api branch from fba47ba to b2cf6d7 Compare August 9, 2024 23:56
@icculus
Copy link
Collaborator Author

icculus commented Aug 9, 2024

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 icculus mentioned this pull request Aug 10, 2024
5 tasks
@icculus icculus merged commit b4ca15b into libsdl-org:main Aug 10, 2024
39 checks passed
@icculus icculus deleted the sdl3-review-pen-api branch August 10, 2024 02:09
@icculus
Copy link
Collaborator Author

icculus commented Aug 10, 2024

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

@sezero
Copy link
Contributor

sezero commented Aug 10, 2024

@madebr
Copy link
Contributor

madebr commented Aug 10, 2024

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.

@sezero
Copy link
Contributor

sezero commented Aug 10, 2024

OK, pushed f93920a to work around it.

@icculus: revise it as you see fit.

@slouken
Copy link
Collaborator

slouken commented Aug 10, 2024

OK, pushed f93920a to work around it.

@icculus: revise it as you see fit.

This looks fine to me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants