API for pressure-sensitive pens + XInput2 & Wayland implementations #5481
Conversation
creichen
changed the title
|
For the Wayland side, see #5344. The protocol is in place for the bare minimum support, just needs to be fully implemented. |
|
Got the following build errors on an old setup: Easily fixed by: diff --git a/src/video/x11/SDL_x11pen.c b/src/video/x11/SDL_x11pen.c
index f95e647..5a95804 100644
--- a/src/video/x11/SDL_x11pen.c
+++ b/src/video/x11/SDL_x11pen.c
@@ -64,3 +64,3 @@ X11_InitPen(_THIS)
/* Check for pen or eraser and set properties suitably */
- bool is_pen_or_eraser = False;
+ SDL_bool is_pen_or_eraser = SDL_FALSE;
struct SDL_X11Pen pen_device;
@@ -85,3 +85,3 @@ X11_InitPen(_THIS)
/* pressure-sensitive is the sole requirement for being pen or eraser */
- is_pen_or_eraser = True;
+ is_pen_or_eraser = SDL_TRUE;
diff --git a/src/video/x11/SDL_x11xinput2.c b/src/video/x11/SDL_x11xinput2.c
index b402270..a5c3127 100644
--- a/src/video/x11/SDL_x11xinput2.c
+++ b/src/video/x11/SDL_x11xinput2.c
@@ -229,3 +229,3 @@ X11_HandleXinput2Event(SDL_VideoData *videodata,XGenericEventCookie *cookie)
const int button = xev->detail;
- const bool pressed = cookie->evtype == XI_ButtonPress;
+ const SDL_bool pressed = (cookie->evtype == XI_ButtonPress) ? SDL_TRUE : SDL_FALSE;
Additional note: your changes to configury, if any, are not understandable from the patchset. |
|
@sezero : Thank you! Tried out and pushed your patch. Can you elaborate on the configuration changes, please? I already pushed the (minor) I had changes to |
|
Yes: if there are no changes to configure.ac, please revert changes to generated configure. Thanks. |
creichen
force-pushed
the
pen-api
branch
2 times, most recently
from
c7b69ed to
dc30f5d
Compare
its still affected, but this PR doesn't currently do the tablet api for wayland. You can take inspiration from 9125b24 which does (as @flibitijibibo said) some basic functionality that tablets can act as mice input devices. |
|
I merged against |
Speaking for myself, I'd prefer a rebase against main + force-push. |
|
I started on some |
|
All squashed into a single commit, as per @sezero 's preference. I hope that the most recent changes also address the XWayland issue that @sp1ritCS ran into. I've kept the most recent history at https://github.com/creichen/SDL/tree/pen-api-history for those who want to check today's changes. Marking "Ready for review" now. If there is a good solution for the |
creichen
changed the title
|
Will let the others review the X side, but for an API like this it seems like a good idea to take a page from Khronos' book and require a second implementation before we lock this down. I'm biased and would like to see Wayland support but I know pen/tablet events are something that pop up in the existing Windows video backend as well. |
|
For most compatibility with many compilers, testpen.c can lose C99'isms --- testpen.c~
+++ testpen.c
@@ -46,7 +46,8 @@ DrawScreen(SDL_Renderer *renderer)
if (last_was_eraser) {
- SDL_Rect rect = {
- .x = last_x - 10,
- .y = last_y - 10,
- .w = 21,
- .h = 21 };
+ SDL_Rect rect;
+
+ rect.x = last_x - 10;
+ rect.y = last_y - 10;
+ rect.w = 21;
+ rect.h = 21;
@@ -128,3 +129,2 @@ dump_state()
-
guid2.data[15] = 0;
@@ -156,6 +156,4 @@ process_event(SDL_Event event)
#if VERBOSE
- int x, y;
-
+ { int x, y;
SDL_GetMouseState(&x, &y);
-
if (event.type == SDL_MOUSEMOTION) {
@@ -172,3 +170,3 @@ process_event(SDL_Event event)
}
-
+ }
#endif
--- test/Makefile.os2~
+++ test/Makefile.os2
@@ -25,3 +25,3 @@
testviewport.exe testwm2.exe torturethread.exe checkkeys.exe &
- checkkeysthreads.exe testmouse.exe &
+ checkkeysthreads.exe testmouse.exe testpen.exe &
controllermap.exe testhaptic.exe testqsort.exe testresample.exe & |
it did.
seconded, alternatively I'd biased towards an libinput/evdev implantation around the compositor for lower latency (assuming the user has permission to read the device) |
|
Thank you! Merged @sezero's patch. Let me know if you'd prefer that I split out Pen event reporting should work fine without it (with a couple of I'm not currently planning to add more features to this PR, unless I get my hands on new pen input hardware or find time to set up Wayland (unlikely for at least a couple of weeks). |
|
No worries on the timeline - I believe we're in the middle of a freeze since 2.0.22 is coming up, so it'll be a while before we start working on new features again. |
|
Let's get an initial Windows implementation in as well. |
|
@slouken, are you referring to you wanting to merge in @PJB3005's PR creichen#1 ? That one is promising but still WIP, as far as I know. |
|
@PJB3005, I'd like to include your implementation in this PR. Can you finish it up and add it here? |
|
Yeah I should get off my butt and finish this PR. Guess I'll look into it tonight/ |
|
This isn't going in until SDL3, so we have some time. |
Ah, didn't notice the milestone change. (Pity though.) |
|
"Alright time to get off my butt and get the Windows side over the finish line this weeke-"
oh god should I feel guilty about this |
|
No, just finish it and we'll merge it, if you're done before SDL4 😆 |
|
apologies for the delay, I was on a long leave away from the computer.
Enter events in X are tied to the cursor, so unless the cursor moves out of the window you won't get a leave event. In XI 1.x there was the |
|
@PJB3005 No worries from my side! :-) I will be too busy with work to merge/re-validate until mid-December anyway (and have been for a bit). |
|
@PJB3005, any update on this? |
|
Alright I'm finally gonna try to push this over now. Working on rebasing my first commit on top of the new branch and then seeing what issues I still had to fix. I think I'm probably gonna give up on the idea of getting Wacom GUIDs and distance values for now. In the future we could probably make a switchable wintab backend if those are deemed important enough. |
|
Currently hammering away at the flickering cursor problem in the test app. Sporadic mouse events erroneously sent into the app due to bugs cause this, as the test app re-shows the cursor on mouse events. The first issue I ran into with that was easy enough. Apparently pen input also fires The second problem however is a "god damn, Windows is just broken" scenario. There appear to just be bogus There also appears to be some sort of internal SDL2 bug where it keeps re-setting the cursor. This happens to me during testing: I have a button rigged to turn off the |
|
@PJB3005, if you are trying to grab the edge of the window to resize it on Windows, then it's most likely related to this issue: #1059 (comment) |
|
@PJB3005 Would it help you if I were to re-base the current patch onto the most recent SDL? As for messing with the cursor: This default behaviour is for backward compatibility; I'd prefer to have |
|
Pity that this had been neglected. |
|
Hmm, I think I messed up-- I wanted to rename the branch on my local fork to |
|
New PR would be easiest I'd guess |
|
If GitHub doesn't show a button to recreate the branch, a new PR is the way to go. |
|
#8058 replaces this PR (same original code base). |
Tools default to 1% lower threshold (tip up) and 5% upper threshold (tip down). But our distance vs pressure exclusion would reset the distance for *any* pressure value, regardless how low that value was and how high distance was in comparison. A very low pressure value of less than 1% would then result in a normalized pressure of 0, so we'd effectively just reset the distance to zero and do nothing with the pressure. This can cause distance jumps when the tool arbitrarily sends low pressure values while hovering as seen in libsdl-org/SDL#5481 (comment) Commit 61bdc05 from Dec 2017 "tablet: set the tip-up pressure threshold to 1%" was presumably to address this but no longer (?) works. Fix this by addressing multiple issues at the same time: - anything under that 1% threshold is now considered as zero pressure and any distance value is kept as-is. Once pressure reaches 1%, distance is always zero. - axis normalization is now from 1% to 100% (previously: upper threshold to 100%). So a tip down event should always have ~4% pressure and we may get tablet motion events with nonzero pressure before the tip down event. From memory, this was always intended anyway since a tip event should require some significant pressure, maybe too high compared to e.g. pressure-sensitive painting - where a tablet has an offset, add the same 1%/5% thresholds, on top of that offset. And keep adjusting those thresholds as we change the offset. Assuming that the offset is the absolute minimum a worn-out pen can reach, this gives us the same behaviour as a new pen. The calculation here uses a simple approach so the actual range is slightly larger than 5% but it'll do. Previously, the lower threshold for an offset pen was the axis minimum but that can never be reached. So there was probably an undiscovered bug in there. And fix a bunch of comments that were either wrong, confusing or incomplete, e.g. the pressure thresholds were already in device coordinates. Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This pull request adds support for pressure-sensitive pens to SDL. It is intended to maintain backward compatibility, and adds new events to provide more detailed information for pen movement.
(Last updated 2022-06-17)
Description
This pull request adds support for pressure-sensitive pens to SDL:
PenMotion,PenButton)float- typedx,ypositionsSDL_pen.h)testautomation_pen.c)testpen)SDL_HINT_PEN_NOT_MOUSEallows either entirely suppressing mouse events for pen events, or sending pen movement/button updates without moving the mouse position.SDL_HINT_PEN_DELAY_MOUSE_BUTTON(default) reports mouse button presses when the pen touches/leaves the tablet, with the mouse button determined by which pen button is pressed when the pen first touches the surface. If disabled, pen buttons translate to mouse buttons even if the pen is just hovering, and the pen tip acts as left mouse button.Design Rationale
Abstractly speaking, Pens are similar to mice, except that they provide additional information (pressure, tilt). In practice, they are particularly useful for writing / drawing, for which it is valuable to have sub-pixel position resolution.
To adapt to SDL conventions, the API reports normalised data:
0.0f..1.0f-1.0f..1.0f(0.0fas centre), reporting the tilt vector (rather than the tilt angle, as Wayland does)-1.0f..1.0f(0.0fas centre), representing barrel rotation. Multiply with 180 for degrees or pi to get radians.0.0f..1.0f-1.0f..1.0f(0.0fas centre) or0.0f..1.0f(untested)PenMotionwith pressure greater than0has button 1 pressed, which simulates currentMouseMotionbehaviourwhichasSDL_PEN_MOUSEID(analogously toSDL_TOUCH_MOUSEID)Design alternatives:
MouseMotionandMouseButtoninstead of introducing new eventsx,yfieldsSDL_PenID:SDL_PenGUID: unique across sessions, but more data to pass around, can't print / compare / hash as easilySDL_Pen*: largely the same asSDL_PenID, but requires clients to go through extra steps when making pen API calls during event processing, unless we pass pointers in event structures.Backward Compatibility
Pens still report mouse events via
which=SDL_PEN_MOUSEID. They should thus work the same as beforeto clients who are unaware of pen events.
[EDIT]: Updates. Next Steps, and Limitations (updates summarised here)
Changelog
cmakesupport not yet adpatedExtend(-> Add visualisation for mouse wheel testing to test/testmouse #5486 )testmousefor mouse wheels in preparation for the next itemXInput2: Doesn't currently filter out duplicate X11Motion/ButtonPress/ButtonReleaseeventsNot currently filtering out duplicateMouseMotionevents caused by pensHot-pluggingGeneral-purpose pen API, make events more easily extensible with more/flexible axesTest program runs in a tight loopXInput2: Implementation is currently limited to 4 pen and/or eraser devicesStop pen movement from updating default pointer position, toggled viaSDL_HINT_PEN_NOT_MOUSEAdditional axes: courtesy of detailed information from @Pinglinux at Wacom, the code might now work with (and auto-detect) most if not all Wacom pensSDL_HINT_PEN_DELAY_MOUSE_BUTTONto toggleNo plans to fix in this PR:
Missing testing
Below are scenarios that are difficult for me to test:
Existing Issue(s)
SDL_HINT_PEN_NOT_MOUSE=1could help fix mouse clicks reported for pen position issue from [Wayland] Add support for tablet-unstable-v2 protocol #5344Acknowledgements and Thanks