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

XP-Pen Artist 12 2nd Gen: Spurious Tip Down #26

Closed
Grissess opened this issue Nov 10, 2021 · 23 comments · Fixed by #49
Closed

XP-Pen Artist 12 2nd Gen: Spurious Tip Down #26

Grissess opened this issue Nov 10, 2021 · 23 comments · Fixed by #49
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Grissess
Copy link

Hello!

Following up from DIGImend/digimend-kernel-drivers#578: thanks for implementing support! I gave your driver a test with the hardware, and it seemed to have recognized it just fine:

Listening on socket /home/grissess/.local/var/run/userspace_tablet_driver_daemon.sock
No existing config so we will be creating a new one
xp_pen_handler initialized
huion_handler initialized
Got hotplug event
Handling XP-Pen Artist 12 (2nd Gen)
Setup completed on interface 0
Setup completed on interface 1
Attached to interface 2
Sending init key on endpont 3
Setup completed on interface 2
Set up config for device 2378

But something in the tablet state handling is off; whenever the pen is close enough to cause a proximity event, it also sends a tip-down event (this according to libinput debug-events, with me "dipping" the tool several times briefly into proximity):

-event18  TABLET_TOOL_PROXIMITY   +0.000s		1.42*/4.73*	tilt: 3.20*/23.47*	pressure: 1.00*	pen      (0, id 0) proximity-in 	axes:pt	btn:SS2
 event18  TABLET_TOOL_TIP         +0.000s		1.42/4.73	tilt: 3.20/23.47	pressure: 1.00 down
 event18  TABLET_TOOL_TIP         +0.044s		1.42/4.73	tilt: 3.20*/23.47*	pressure: 1.00 up
 event18  TABLET_TOOL_PROXIMITY   +0.044s		1.42/4.73	tilt: 3.20/23.47	pressure: 1.00	pen      (0, id 0) proximity-out
 event18  TABLET_TOOL_PROXIMITY   +4.028s		1.66*/4.82*	tilt: 3.20*/35.20*	pressure: 1.00*	pen      (0, id 0) proximity-in 	axes:pt	btn:SS2
 event18  TABLET_TOOL_TIP         +4.028s		1.66/4.82	tilt: 3.20/35.20	pressure: 1.00 down
 event18  TABLET_TOOL_AXIS        +4.038s		1.66/4.82	tilt: 2.93*/34.93*	pressure: 1.00
 event18  TABLET_TOOL_AXIS        +4.042s		1.66/4.82	tilt: 2.67/34.40*	pressure: 1.00
 event18  TABLET_TOOL_AXIS        +4.046s		1.66/4.82	tilt: 2.40/33.33*	pressure: 1.00
 event18  TABLET_TOOL_TIP         +4.100s		1.66/4.82	tilt: 2.13*/30.93*	pressure: 1.00 up
 event18  TABLET_TOOL_PROXIMITY   +4.100s		1.66/4.82	tilt: 2.13/30.93	pressure: 1.00	pen      (0, id 0) proximity-out
 event18  TABLET_TOOL_PROXIMITY   +5.012s		1.68*/4.79*	tilt: 2.13*/30.93*	pressure: 1.00*	pen      (0, id 0) proximity-in 	axes:pt	btn:SS2
 event18  TABLET_TOOL_TIP         +5.012s		1.68/4.79	tilt: 2.13/30.93	pressure: 1.00 down
 event18  TABLET_TOOL_AXIS        +5.022s		1.68/4.79	tilt: 1.60*/39.20*	pressure: 1.00
 event18  TABLET_TOOL_AXIS        +5.032s		1.68/4.79	tilt: 1.60*/46.67*	pressure: 1.00
 event18  TABLET_TOOL_AXIS        +5.036s		1.68/4.79*	tilt: 1.60/54.13	pressure: 1.00
 event18  TABLET_TOOL_AXIS        +5.042s		1.68*/4.78*	tilt: 1.60/60.00*	pressure: 1.00
 event18  TABLET_TOOL_TIP         +5.094s		1.69/4.77	tilt: 2.13*/54.40*	pressure: 1.00 up
 event18  TABLET_TOOL_PROXIMITY   +5.094s		1.69/4.77	tilt: 2.13/54.40	pressure: 1.00	pen      (0, id 0) proximity-out

If I recall, libinput likes to clamp minimum pressure while the tip is down--and to 0 while it's up--which may also be interfering with the pressure readouts. For your ranging, the maximum pressure I observed was 2.05.

Let me know if you need further diagnostics :)

@kurikaesu
Copy link
Owner

kurikaesu commented Nov 10, 2021

Thanks for the report! I actually opened an issue to ask for some testing myself. I will close that out hahaha.

Does the tablet still draw correctly even with the tip in and out? While I can't find the documentation on hand at the moment, libinput uses the BTN_TOOL_PEN to indicate if the pen is in proximity or not and actual ABS_PRESSURE to say if the stylus is actually touching the digitizer as it should have even a little bit of pressure at that time. This is how it is implemented in the code: https://github.com/kurikaesu/userspace-tablet-driver-daemon/blob/main/src/xp_pen_unified_device.cpp#L33-L46

The pressure reporting on XP-Pens in my driver code is done by actual pressure levels (8192 for the XP-Pen Artist 12) and will set itself to 0 when the pen leaves proximity. it will not set pressure to 0 when the pen leaves proximity.

@kurikaesu
Copy link
Owner

Found some more documentation about it and it seems like spbnick of Digimend also replied to the post: http://who-t.blogspot.com/2019/06/libinput-and-tablet-proximity-handling.html

@Grissess
Copy link
Author

Apologies for the delay! The middle of the week was busy :) . I just found time to check out the most recent tip--aside from the issue above, which persists, the axis mapping has gotten a little stranger (I haven't restarted my WM, so testing seems to affirm a belief that the input is mapped to the right output). In particular, the Y axis travel is doubled from the device origin, while the X axis is... 1+1/8 or so? It's not particularly clear...

@Grissess
Copy link
Author

Oh, let me answer the questions as well...

Does the tablet still draw correctly even with the tip in and out?

Well, aside from the odd mapping at this point, it behaves in most applications as if I'm holding the tip down at full pressure. It seems to me Krita in particular tries to make sense of the stylus suddenly going to 100% pressure while jumping across the screen, and makes an awkwardly-interpolated stroke.

While I can't find the documentation on hand at the moment, libinput uses the BTN_TOOL_PEN to indicate if the pen is in proximity or not and actual ABS_PRESSURE to say if the stylus is actually touching the digitizer as it should have even a little bit of pressure at that time. This is how it is implemented in the code: https://github.com/kurikaesu/userspace-tablet-driver-daemon/blob/main/src/xp_pen_unified_device.cpp#L33-L46

The pressure reporting on XP-Pens in my driver code is done by actual pressure levels (8192 for the XP-Pen Artist 12) and will set itself to 0 when the pen leaves proximity. it will not set pressure to 0 when the pen leaves proximity.

Oddly enough, in at least my rush right now, I can't find anything not supporting your arguments. Nonetheless, my proof-of-concept doesn't send any kind of BTN_TOOL_PEN event (it could possibly stand to--I'll check later), and has the desired result: https://github.com/Grissess/xpptd/blob/main/read_usb.py#L146-L163 . More to come!

@kurikaesu
Copy link
Owner

Interesting. I wonder if it has anything to do with the X3 stylus that the tablet comes with.
If that is the case I am considering picking up another tablet with the X3 (The innovator 16) and testing with that.

I'll run tests on my end with the tablets I have to see if the libinput debug-events show similar behavior.

@Grissess
Copy link
Author

Grissess commented Nov 12, 2021

Tested the PoC with BTN_TOOL_PEN: https://github.com/Grissess/xpptd/blob/main/read_usb.py#L149-L151

Interestingly enough, I was already getting TABLET_TOOL_PROXIMITY on my libinput debug-events without this change; based on your linked sources, I have to believe that libinput outright ignores these events these days and depends on (as the blog says) the fact that inputs are sent at scanrate, which my program passes on to the input subsystem at the same rate.

Looking at it now, when holding the pen still enough, I get spurious proximity-outs even with this change, and after confirming (with evtest) that the program is properly generating BTN_TOOL_PEN events.

(I think the overarching reason I never implemented that in the first place was because I wasn't quite sure which tool this was--there are a number of BTN_TOOL_* constants, including PENCIL, BRUSH, ...)

Edit: well, if that isn't odd... only after the evtest harness, libtool debug-events stopped showing the spurious proximity for a still pen. I've never accused my system of sensibility :)

@kurikaesu
Copy link
Owner

Here's a proximity test I did with my tablet while running Wayland-gnome:

https://vimeo.com/manage/videos/645360584

Definitely don't see any weird tip down.
The TABLET_TOOL_TIP down message does happen when I touch the digitizer with the stylus.

Do you have a USB stream dump of the messages being generated when the stylus is going in & out of proximity as well as when it touches the digitizer? The messages may be formatted differently and my driver needs to be updated to handle the new style

@Grissess
Copy link
Author

I've built the current tip and collected a capture while UTDD was running: utdd.pcapng.gz . (That probably could have been done via usbhid-dump, but I was more comfortable with using this tool to get the setup handshake. Connection starts at packet 229.)

Here too is an event log of libinput: events.log. It looks like it got truncated at the end because of the pipe--oops. Still, there's plenty of data in there; it looks like the timestamps of the packet capture are ~25.335s ahead of the libinput log timetamps. (I'm having trouble with time shifts, but setting packet 288 as a reference lines up the timestamps pretty well.)

Let me know if you need anything more!

@kurikaesu
Copy link
Owner

Thanks, having a look through the dumps. So far I don't see anything weird but I'll probably need to extract all of the messages and line them up.

@kurikaesu
Copy link
Owner

Looks like I made a mistake on tablet models. I ordered the Innovator 16 instead of the Pro 16. Only the Pro 16 has the x3 chip stylus.

@kurikaesu
Copy link
Owner

I'll have to defer this issue to someone who has the tablet and is able to dedicate some time debugging the issue as I can't reproduce the problem locally.

If at any point I end up with this tablet in my possession then I'll take a look again. Until then though, this issue is up for grabs.

@kurikaesu kurikaesu added bug Something isn't working help wanted Extra attention is needed labels Nov 27, 2021
@mincrmatt12
Copy link

mincrmatt12 commented Jan 29, 2022

Hey, I've got one of these tablets and I'm experiencing similar behaviour to what's described here, if you've got any pointers on how I might help debug it that'd be great!

Just from playing around with the source a bit, applying these ugly hacks gets it working a lot better:

diff --git a/src/transfer_handler.cpp b/src/transfer_handler.cpp
index cc51ca8..8346627 100644
--- a/src/transfer_handler.cpp
+++ b/src/transfer_handler.cpp
@@ -507,6 +507,7 @@ void transfer_handler::handlePenLeftProximity(libusb_device_handle* handle) {
 }
 
 void transfer_handler::handlePenTouchingDigitizer(libusb_device_handle *handle, int pressure) {
+       uinput_send(uinputPens[handle], EV_KEY, BTN_TOUCH, (pressure != 0) ? 1 : 0);
     uinput_send(uinputPens[handle], EV_ABS, ABS_PRESSURE, pressure);
 }
 
diff --git a/src/xp_pen_unified_device.cpp b/src/xp_pen_unified_device.cpp
index 0663c8a..ac8cad2 100644
--- a/src/xp_pen_unified_device.cpp
+++ b/src/xp_pen_unified_device.cpp
@@ -57,7 +57,7 @@ bool xp_pen_unified_device::attachDevice(libusb_device_handle *handle, int inter
     padNameBuilder << " Pad";
     std::string padName = padNameBuilder.str();
 
-    std::cout << "Device: " << deviceName << " - Probed maxWidth: (" << maxWidth << ") maxHeight: (" << maxHeight << ") resolution: (" << resolution << ")" << std::endl;
+    std::cout << "Device: " << deviceName << " - Probed maxWidth: (" << maxWidth << ") maxHeight: (" << maxHeight << ") resolution: (" << resolution << ")" <<  "maxPresure: " << maxPressure << std::endl;
 
     unsigned short vendorId = 0x28bd;
     unsigned short aliasedProductId = productId + 0xf000;
@@ -110,6 +110,7 @@ void xp_pen_unified_device::handleDigitizerEvent(libusb_device_handle *handle, u
         // Check to see if the pen is touching
         std::bitset<sizeof(data)> stylusTipAndButton(data[1]);
         int pressure = (data[7] << 8) + data[6];
+               pressure -= 8192;
 
         // Handle pen coming into/out of proximity
         if (stylusTipAndButton.test(5)) {
@@ -119,9 +120,12 @@ void xp_pen_unified_device::handleDigitizerEvent(libusb_device_handle *handle, u
         }
 
         // Handle actual stylus to digitizer contact
-        if (stylusTipAndButton.test(0) | stylusTipAndButton.test(5)) {
+        if (stylusTipAndButton.test(0) && stylusTipAndButton.test(5)) {
             handlePenTouchingDigitizer(handle, applyPressureCurve(pressure));
         }
+        else {
+            handlePenTouchingDigitizer(handle, 0);
+               }
 
         // Grab the tilt values
         short tiltx = (char)data[8];

@kurikaesu
Copy link
Owner

Thanks for looking into this. Its quite interesting that the pressure is inverted. I'm wondering if it is being represented bytewise differently to how the XP-Pen devices I've had and seen have been serving them up until now.

With those changes does the tablet stop giving random events completely or are there still things that need to be solved?

@mincrmatt12
Copy link

Its quite interesting that the pressure is inverted. I'm wondering if it is being represented bytewise differently to how the XP-Pen devices I've had and seen have been serving them up until now.

I think you may have misread the patch, it's not inverted, there's just a constant offset of 8192 added to the pressure (so maybe something needs to be masked out or something). The device reports a maxPressure of 8192, but then sends values from 8192-16383.

With those changes does the tablet stop giving random events completely or are there still things that need to be solved?

With these small changes everything seems to work fine.

I had noticed while doing some print debugging that the low bit of stylusTipAndButton is set exactly when contact is made between the stylus and tablet, so I changed the || to an &&. That still didn't quite fix it, so I made the driver actually send some BTN_TOUCH events to indicate when actual contact is being made vs. proximity (based on the advice in the linux kernel docs for BTN_TOUCH)

All the rest of the data coming from the tablet (pad buttons, tilt, stylus buttons) seems to be accurate.

@Grissess
Copy link
Author

Ah, yes, I observed that offset too; cf. https://github.com/Grissess/xpptd/blob/main/read_usb.py#L38 . I guess I wouldn't have known if that was unusual :)

That same source agrees with your bit positions for touch/buttons, I believe, as well as events to send. I don't know if there's anything else you can find of use in there, but you're welcome to it!

@kurikaesu
Copy link
Owner

Thanks for the info! I'll make some changes in a branch and we can test to see if how I've interpreted it works for Artist 12 without breaking the other devices.

@kurikaesu
Copy link
Owner

Could either of you try this branch? https://github.com/kurikaesu/userspace-tablet-driver-daemon/tree/artist-12-pro-gen2-fixes

I've incorporated the fixes above. Let me know if it works correctly on your tablets.

@mincrmatt12
Copy link

mincrmatt12 commented Feb 4, 2022

I think you might have forgot to apply the offset to the non-pro version (which is what I have), but if I add

diff --git a/src/artist_12.cpp b/src/artist_12.cpp
index 81d53ab..dcabcee 100644
--- a/src/artist_12.cpp
+++ b/src/artist_12.cpp
@@ -61,7 +61,11 @@ void artist_12::setConfig(nlohmann::json config) {
 bool artist_12::handleTransferData(libusb_device_handle *handle, unsigned char *data, size_t dataLen, int productId) {
     switch (data[0]) {
         case 0x02:
-            handleDigitizerEvent(handle, data, dataLen);
+            if (productId == 0x094a) {
+                handleDigitizerEvent(handle, data, dataLen, -8192);
+            } else {
+                handleDigitizerEvent(handle, data, dataLen);
+            }
             handleFrameEvent(handle, data, dataLen);
             break;

everything works fine.

@kurikaesu
Copy link
Owner

Oh I must have missed the non-pro version to not do that. I'll fix that up once the others confirm the pro version.

@kurikaesu
Copy link
Owner

Looks like I made a mistake there with which device to set up. I've taken your changes and reverted the one in the pro version.

@kurikaesu
Copy link
Owner

Please check out the PR or grab the updated branch and test. I'll merge this once I get either of your confirmation messages or after 7 days, whichever is earlier.

@mincrmatt12
Copy link

Been running that branch for a few hours and it seems to be working fine.

@kurikaesu
Copy link
Owner

Alright, I'll take this as a confirmation and do the merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants