linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
       [not found] <kRKTNDYigUSblpNgSuZ2H4dX03Of1yD4j_L9GgbyKXcDqZ67yh5HOQfcd7_83U3jZuQzxpKT3L6FXcRkkZIGdl_-PQF14oIB0QmRSfvpc2k=@protonmail.com>
@ 2023-11-01 19:37 ` Jiri Kosina
  2023-11-02  0:44   ` Bagas Sanjaya
                     ` (2 more replies)
  0 siblings, 3 replies; 40+ messages in thread
From: Jiri Kosina @ 2023-11-01 19:37 UTC (permalink / raw)
  To: David Revoy
  Cc: jason.gerecke, jose.exposito89, ilya.ostapyshyn, Nils Fuhler,
	Ping Cheng, Peter Hutterer, Benjamin Tissoires, linux-kernel,
	linux-input

On Wed, 1 Nov 2023, David Revoy wrote:

> Hi Jason Gerecke, José Expósito, Jiri Kosina and Illia Ostapyshyn,
> 
> I am emailing to draw your attention and expertise to a problem I had 
> earlier this week with my Xp-Pen Artist 24 Pro display tablet under 
> Fedora Linux 38 KDE after a kernel update.
> 
> The second button on my stylus changed from a right-click (which I could 
> customise with xsetwacom or any GUI like kcm-tablet) to a button that 
> feels 'hardcoded' and now switches the whole device to an eraser mode. 
> This makes my main tool unusable.
> 
> I don't have the skills to write a proper kernel bug report, workaround 
> or even identify the exact source of the issue. I have written a blog 
> post about this with more details here: 
> https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help 
> , contacting you was something suggested by the comments.
> 
> Thank you very much if you can help me.

CCing a couple more people involved both in 276e14e6c3 and 87562fcd1342, 
and mailinglists.

This is almost certainly the behavior introduced by 276e14e6c3, where 
previously the button was mapped to BTN_TOUCH, but now it's mapped to 
BTN_TOOL_RUBBER, causing user-visible change in behavior.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-01 19:37 ` Requesting your attention and expertise regarding a Tablet/Kernel issue Jiri Kosina
@ 2023-11-02  0:44   ` Bagas Sanjaya
  2023-11-02  6:31     ` Linux regression tracking (Thorsten Leemhuis)
  2023-11-02  7:44   ` Linux regression tracking #adding (Thorsten Leemhuis)
  2023-11-03 20:05   ` Illia Ostapyshyn
  2 siblings, 1 reply; 40+ messages in thread
From: Bagas Sanjaya @ 2023-11-02  0:44 UTC (permalink / raw)
  To: Jiri Kosina, David Revoy
  Cc: jason.gerecke, jose.exposito89, ilya.ostapyshyn, Nils Fuhler,
	Ping Cheng, Peter Hutterer, Benjamin Tissoires,
	Linux Kernel Mailing List, Linux Input Devices,
	Linux Regressions

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

On Wed, Nov 01, 2023 at 08:37:40PM +0100, Jiri Kosina wrote:
> On Wed, 1 Nov 2023, David Revoy wrote:
> 
> > Hi Jason Gerecke, José Expósito, Jiri Kosina and Illia Ostapyshyn,
> > 
> > I am emailing to draw your attention and expertise to a problem I had 
> > earlier this week with my Xp-Pen Artist 24 Pro display tablet under 
> > Fedora Linux 38 KDE after a kernel update.
> > 
> > The second button on my stylus changed from a right-click (which I could 
> > customise with xsetwacom or any GUI like kcm-tablet) to a button that 
> > feels 'hardcoded' and now switches the whole device to an eraser mode. 
> > This makes my main tool unusable.
> > 
> > I don't have the skills to write a proper kernel bug report, workaround 
> > or even identify the exact source of the issue. I have written a blog 
> > post about this with more details here: 
> > https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help 
> > , contacting you was something suggested by the comments.
> > 
> > Thank you very much if you can help me.
> 
> CCing a couple more people involved both in 276e14e6c3 and 87562fcd1342, 
> and mailinglists.
> 
> This is almost certainly the behavior introduced by 276e14e6c3, where 
> previously the button was mapped to BTN_TOUCH, but now it's mapped to 
> BTN_TOOL_RUBBER, causing user-visible change in behavior.
> 

Thanks for the report.

David, can you resend the regression report as plain text email (preferably
with 276e14e6c3 people and regressions@lists.linux.dev Cc'ed)? You may need to
see kernel documentation [1] for how to configure your email client to send
plain text emails. Also, include in your report details from your blog post.

Thanks.

[1]: https://docs.kernel.org/process/email-clients.html

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-02  0:44   ` Bagas Sanjaya
@ 2023-11-02  6:31     ` Linux regression tracking (Thorsten Leemhuis)
  2023-11-02  7:51       ` Bagas Sanjaya
  0 siblings, 1 reply; 40+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-11-02  6:31 UTC (permalink / raw)
  To: Bagas Sanjaya, Jiri Kosina, David Revoy
  Cc: jason.gerecke, jose.exposito89, ilya.ostapyshyn, Nils Fuhler,
	Ping Cheng, Peter Hutterer, Benjamin Tissoires,
	Linux Kernel Mailing List, Linux Input Devices,
	Linux Regressions

On 02.11.23 01:44, Bagas Sanjaya wrote:
> On Wed, Nov 01, 2023 at 08:37:40PM +0100, Jiri Kosina wrote:
>> On Wed, 1 Nov 2023, David Revoy wrote:
>>
>>> Hi Jason Gerecke, José Expósito, Jiri Kosina and Illia Ostapyshyn,
>>>
>>> I am emailing to draw your attention and expertise to a problem I had 
>>> earlier this week with my Xp-Pen Artist 24 Pro display tablet under 
>>> Fedora Linux 38 KDE after a kernel update.
>>>
> […]

>>> Thank you very much if you can help me.
> […]
> Thanks for the report.
> 
> David, can you resend the regression report as plain text email (preferably
> with 276e14e6c3 people and regressions@lists.linux.dev Cc'ed)? You may need to
> see kernel documentation [1] for how to configure your email client to send
> plain text emails. Also, include in your report details from your blog post.

Bagas, I know you mean well, but I think you are making things
unnecessarily complicated for both David and the developers here. Sure,
the mail Jiri quoted did not make it to lore, but whatever, for him it
was apparently good enough; and I suspect this "quote forwarding to
others" is good enough for the people he brought into the loop as well.
Yes, there are developers that don't want to go to a website for
details, but if that's the case it's likely better to ask for details in
this thread instead of opening a second one. And regression tracking can
work without a separate mail as well.

Ciao, Thorsten

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-01 19:37 ` Requesting your attention and expertise regarding a Tablet/Kernel issue Jiri Kosina
  2023-11-02  0:44   ` Bagas Sanjaya
@ 2023-11-02  7:44   ` Linux regression tracking #adding (Thorsten Leemhuis)
  2023-12-01  8:24     ` Linux regression tracking #update (Thorsten Leemhuis)
  2023-11-03 20:05   ` Illia Ostapyshyn
  2 siblings, 1 reply; 40+ messages in thread
From: Linux regression tracking #adding (Thorsten Leemhuis) @ 2023-11-02  7:44 UTC (permalink / raw)
  To: Jiri Kosina, David Revoy
  Cc: jason.gerecke, jose.exposito89, ilya.ostapyshyn, Nils Fuhler,
	Ping Cheng, Peter Hutterer, Benjamin Tissoires, linux-kernel,
	linux-input, Linux kernel regressions list

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]

On 01.11.23 20:37, Jiri Kosina wrote:
> On Wed, 1 Nov 2023, David Revoy wrote:
> 
>> Hi Jason Gerecke, José Expósito, Jiri Kosina and Illia Ostapyshyn,
>>
>> I am emailing to draw your attention and expertise to a problem I had 
>> earlier this week with my Xp-Pen Artist 24 Pro display tablet under 
>> Fedora Linux 38 KDE after a kernel update.
>>
>> The second button on my stylus changed from a right-click (which I could 
>> customise with xsetwacom or any GUI like kcm-tablet) to a button that 
>> feels 'hardcoded' and now switches the whole device to an eraser mode. 
>> This makes my main tool unusable.
>>
>> I don't have the skills to write a proper kernel bug report, workaround 
>> or even identify the exact source of the issue. I have written a blog 
>> post about this with more details here: 
>> https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help 
>> , contacting you was something suggested by the comments.
>>
>> Thank you very much if you can help me.

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced 276e14e6c3
https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help

#regzbot title HID: input: stylus of Xp-Pen Artist 24 Pro display tablet
changed behavior
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-02  6:31     ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-11-02  7:51       ` Bagas Sanjaya
  0 siblings, 0 replies; 40+ messages in thread
From: Bagas Sanjaya @ 2023-11-02  7:51 UTC (permalink / raw)
  To: Linux regressions mailing list, Jiri Kosina, David Revoy
  Cc: jason.gerecke, jose.exposito89, ilya.ostapyshyn, Nils Fuhler,
	Ping Cheng, Peter Hutterer, Benjamin Tissoires,
	Linux Kernel Mailing List, Linux Input Devices

On 02/11/2023 13:31, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 02.11.23 01:44, Bagas Sanjaya wrote:
>> On Wed, Nov 01, 2023 at 08:37:40PM +0100, Jiri Kosina wrote:
>>> On Wed, 1 Nov 2023, David Revoy wrote:
>>>
>>>> Hi Jason Gerecke, José Expósito, Jiri Kosina and Illia Ostapyshyn,
>>>>
>>>> I am emailing to draw your attention and expertise to a problem I had 
>>>> earlier this week with my Xp-Pen Artist 24 Pro display tablet under 
>>>> Fedora Linux 38 KDE after a kernel update.
>>>>
>> […]
> 
>>>> Thank you very much if you can help me.
>> […]
>> Thanks for the report.
>>
>> David, can you resend the regression report as plain text email (preferably
>> with 276e14e6c3 people and regressions@lists.linux.dev Cc'ed)? You may need to
>> see kernel documentation [1] for how to configure your email client to send
>> plain text emails. Also, include in your report details from your blog post.
> 
> Bagas, I know you mean well, but I think you are making things
> unnecessarily complicated for both David and the developers here. Sure,
> the mail Jiri quoted did not make it to lore, but whatever, for him it
> was apparently good enough; and I suspect this "quote forwarding to
> others" is good enough for the people he brought into the loop as well.
> Yes, there are developers that don't want to go to a website for
> details, but if that's the case it's likely better to ask for details in
> this thread instead of opening a second one. And regression tracking can
> work without a separate mail as well.
> 

OK, thanks!

-- 
An old man doll... just what I always wanted! - Clara


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-01 19:37 ` Requesting your attention and expertise regarding a Tablet/Kernel issue Jiri Kosina
  2023-11-02  0:44   ` Bagas Sanjaya
  2023-11-02  7:44   ` Linux regression tracking #adding (Thorsten Leemhuis)
@ 2023-11-03 20:05   ` Illia Ostapyshyn
  2023-11-04  0:46     ` Bagas Sanjaya
  2023-11-06 13:17     ` David Revoy
  2 siblings, 2 replies; 40+ messages in thread
From: Illia Ostapyshyn @ 2023-11-03 20:05 UTC (permalink / raw)
  To: jkosina
  Cc: benjamin.tissoires, davidrevoy, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme, Illia Ostapyshyn

Hello David, Hello Jiri,

The XP-Pen hardware reports the Eraser usage for the upper stylus button.
Generally, styli report Invert usages when erasing, as described in [1].
XP-Pen digitizers, however, tend to omit them.

The generic driver maps the Eraser usage to BTN_TOUCH and the Invert
usage to BTN_TOOL_RUBBER.  Pens conforming to [1] send the Invert usage
first (switching the tool to BTN_TOOL_RUBBER) followed by Eraser, which
appears in userspace as a BTN_TOUCH event with the rubber tool set.

Due to an oversight, devices not reporting Invert had the BTN_TOOL_RUBBER
event masked.  This has caused the kernel to send only BTN_TOUCH events
without the tool switch when erasing.

The situation got worse with refactoring done in 87562fcd1342.  An eraser
without Invert caused the hidinput_hid_event state machine to get stuck
with BTN_TOOL_RUBBER internally (due to it being masked).  For the
userspace, this looked as if the pen was never hovering again, rendering
it unusable until the next reset.  276e14e6c3 fixes this by adding
support for digitizers that do not report Invert usages when erasing.

---

David, we are sorry that our patch broke your workflow.  However,
forwarding hardware events *as-is* to the userspace has always been the
intended behavior, with a kernel bug preventing it so far.  You can still
remap the eraser button to a right click using xsetwacom:

xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"

Replace the device name with the corresponding *eraser* device from
"xsetwacom list devices".  You can also do this with "xinput set-button-map",
which works for libinput as well.  We have tested this with several
XP-Pen devices, including Artist 24.

[1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-03 20:05   ` Illia Ostapyshyn
@ 2023-11-04  0:46     ` Bagas Sanjaya
  2023-11-06 13:17     ` David Revoy
  1 sibling, 0 replies; 40+ messages in thread
From: Bagas Sanjaya @ 2023-11-04  0:46 UTC (permalink / raw)
  To: Illia Ostapyshyn, jkosina
  Cc: benjamin.tissoires, davidrevoy, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

On Fri, Nov 03, 2023 at 09:05:25PM +0100, Illia Ostapyshyn wrote:
> Hello David, Hello Jiri,
> 
> The XP-Pen hardware reports the Eraser usage for the upper stylus button.
> Generally, styli report Invert usages when erasing, as described in [1].
> XP-Pen digitizers, however, tend to omit them.
> 
> The generic driver maps the Eraser usage to BTN_TOUCH and the Invert
> usage to BTN_TOOL_RUBBER.  Pens conforming to [1] send the Invert usage
> first (switching the tool to BTN_TOOL_RUBBER) followed by Eraser, which
> appears in userspace as a BTN_TOUCH event with the rubber tool set.
> 
> Due to an oversight, devices not reporting Invert had the BTN_TOOL_RUBBER
> event masked.  This has caused the kernel to send only BTN_TOUCH events
> without the tool switch when erasing.
> 
> The situation got worse with refactoring done in 87562fcd1342.  An eraser
> without Invert caused the hidinput_hid_event state machine to get stuck
> with BTN_TOOL_RUBBER internally (due to it being masked).  For the
> userspace, this looked as if the pen was never hovering again, rendering
> it unusable until the next reset.  276e14e6c3 fixes this by adding
> support for digitizers that do not report Invert usages when erasing.
> 
> ---
> 
> David, we are sorry that our patch broke your workflow.  However,
> forwarding hardware events *as-is* to the userspace has always been the
> intended behavior, with a kernel bug preventing it so far.  You can still
> remap the eraser button to a right click using xsetwacom:
> 
> xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
> 
> Replace the device name with the corresponding *eraser* device from
> "xsetwacom list devices".  You can also do this with "xinput set-button-map",
> which works for libinput as well.  We have tested this with several
> XP-Pen devices, including Artist 24.
> 
> [1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states

Thanks for the explanation!

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-03 20:05   ` Illia Ostapyshyn
  2023-11-04  0:46     ` Bagas Sanjaya
@ 2023-11-06 13:17     ` David Revoy
  2023-11-06 16:59       ` Benjamin Tissoires
  1 sibling, 1 reply; 40+ messages in thread
From: David Revoy @ 2023-11-06 13:17 UTC (permalink / raw)
  To: Illia Ostapyshyn
  Cc: jkosina, benjamin.tissoires, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

Hi Illia, Jiri, Bagas, 

Thanks to Jiri for forwarding my request for help to this mailing list.

I apologise in advance to Bagas and you all for not being able to properly configure my email client to follow the correct etiquette (Protonmail, unsuitable for kernel development, but we've made some progress with them on Mastodon on the encryption issue [1]).

Thanks to Illia for the detailed explanation. Thanks also for fixing it, and for explaining how the fix broke my workflow, and for your kind words about the situation. I appreciate it.

However, after reading your reply, I still have my doubts about the preference to put an eraser on the top button of the pen by default. But after a few days and re-reading your first sentence "The XP-Pen hardware reports the Eraser usage for the upper stylus button.", I *think* I understood it: this is an internal signal that is sent by the device itself, and is therefore a specification that has to be followed, right? In this case, it makes sense for a generic driver to follow such a signal if it is sent by the hardware.

Having said that, behaving like this still makes no sense in one case: I'm thinking of my other display tablet, the XPPEN 16 Artist Pro (Gen2). In this case, the stylus has the top button as an eraser, and an eraser tip as well, as you can see in this photo [2]. Was it also a decision by XPPEN to include this signal in the hardware, knowing that they already had an eraser tip? In this case, please let me know, as it sounds like a hardware problem at the design stage and I have probably a way of passing on the information to their technical team.

> You can still remap the eraser button to a right click using xsetwacom:
> xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"

Unfortunately, it doesn't work. 

Firstly, such tablets are often connected to a computer that already has a display. So each device (pen/eraser) needs to be mapped to the correct display and set to the correct 'Area' for calibration. A better syntax in my case to test your workaround is written like this:

tableteraser="UGTABLET 24 inch PenDisplay eraser"
xsetwacom set "$tableteraser" MapToOutput "DisplayPort-2"
xsetwacom set "$tableteraser" Area 100 120 32794 32797
xsetwacom set "$tableteraser" "Button" "1" "3"

Secondly, forwarding the eraser button 1 to button 3 (a right click) in xsetwacom doesn't work. 

Pressing the button switches the device to an eraser and no right click happens. You'll need to hold down the button and click with the tip of the pen to create a right-click event. 

In a digital painting session in Krita, the software will switch your device to an eraser tool preset while you hold down the button, and the feedback on the canvas will be the eraser cursor. You'll then have to click "with the eraser" to get the right-click that triggers the pop-up palette [3].

I'd also like to mention that if you haven't correctly mapped and calibrated your eraser device with xsetwacom, as I did in the code snippet above, the cursor will by default jump to a different location when you hold down the eraser button. It can be on a different display.

Finally, in the case of the XPPEN 16 Artist Pro (Gen2) with a real eraser device (Photo, [2]), the setting 'xsetwacom set "$tableteraser" "Button" "1" "3"' will affect both erasers. Flipping the stylus on the eraser side and entering in 'hover' mode will return a correct eraser, but as soon as you start erasing with the tip, a right click will also be entered.

> You can also do this with "xinput set-button-map", which works for libinput as well.

$ xinput list
⎡ Virtual core pointer    
⎜   ↳ UGTABLET 24 inch PenDisplay Mouse         id=8    [slave  pointer  (2)]
⎜   ↳ UGTABLET 24 inch PenDisplay stylus        id=10   [slave  pointer  (2)]
⎜   ↳ UGTABLET 24 inch PenDisplay eraser        id=17   [slave  pointer  (2)]
[...]
$ xinput get-button-map 17
1 2 3 4 5 6 7 8
$ xinput set-button-map 17 3 3 3 3 3 3 3 3
$ xinput get-button-map 17
3 3 3 3 3 3 3 3

Even after that, it doesn't work. My original article [4] mentions in the last paragraph that I have tested almost all methods, and this was one of them. Even a udev rule doesn't fix it. For reference, xinput produces the same behaviour as xsetwacom: you have to hold the button (it triggers an eraser device switch) then click with the tip to get a right-click...

> We have tested this with several XP-Pen devices, including Artist 24.

Sorry if my tests show something different. Maybe there is something wrong with my GNU/Linux operating system (Fedora 38 KDE spin). Let me know if you have any idea why I get these results and guide me with what distro I should switch to get a similar obsevation than you do (and also to report the issue to the Fedora team).

---

All in all (and in the case that my observations and tests are correct), I still stand by the points that I made in my blog post:

- I don't have any way to customise the hardcoded eraser buttons in userspace and **it is a problem**: for devices without a touchscreen or mouse, not having access to a right-click by default on the device is a handicap. Many actions on the D.E. and applications use the right click. The preference to replace it with an 'eraser switch' action by default is IMHO highly debatable here.
- In the case of a pen that already has an eraser tip on the other side of the device [2], it makes no sense to exchange the right-click top button for another way to erase. Also in userspace, there seems to be no way to distinguish between the two buttons (the eraser tip and the eraser button). All actions from xsetwacom, xinput/libinput target the two eraser devices, and they target them unsuccessfully too.

[1]: https://mastodon.social/@protonmail/111346195283677411
[2]: https://www.davidrevoy.com/data/images/blog/2023/2023-11-01_linux-kernel-broke-my-stylus_xp-pen-artist-pro-16-gen2_net.jpg
[3]: https://docs.krita.org/en/reference_manual/popup-palette.html
[4]: https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help


------- Original Message -------
On Friday, November 3rd, 2023 at 21:05, Illia Ostapyshyn <ostapyshyn@sra.uni-hannover.de> wrote:


> Hello David, Hello Jiri,
> 
> The XP-Pen hardware reports the Eraser usage for the upper stylus button.
> Generally, styli report Invert usages when erasing, as described in [1].
> XP-Pen digitizers, however, tend to omit them.
> 
> The generic driver maps the Eraser usage to BTN_TOUCH and the Invert
> usage to BTN_TOOL_RUBBER. Pens conforming to [1] send the Invert usage
> first (switching the tool to BTN_TOOL_RUBBER) followed by Eraser, which
> appears in userspace as a BTN_TOUCH event with the rubber tool set.
> 
> Due to an oversight, devices not reporting Invert had the BTN_TOOL_RUBBER
> event masked. This has caused the kernel to send only BTN_TOUCH events
> without the tool switch when erasing.
> 
> The situation got worse with refactoring done in 87562fcd1342. An eraser
> without Invert caused the hidinput_hid_event state machine to get stuck
> with BTN_TOOL_RUBBER internally (due to it being masked). For the
> userspace, this looked as if the pen was never hovering again, rendering
> it unusable until the next reset. 276e14e6c3 fixes this by adding
> support for digitizers that do not report Invert usages when erasing.
> 
> ---
> 
> David, we are sorry that our patch broke your workflow. However,
> forwarding hardware events as-is to the userspace has always been the
> intended behavior, with a kernel bug preventing it so far. You can still
> remap the eraser button to a right click using xsetwacom:
> 
> xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
> 
> Replace the device name with the corresponding eraser device from
> "xsetwacom list devices". You can also do this with "xinput set-button-map",
> which works for libinput as well. We have tested this with several
> XP-Pen devices, including Artist 24.
> 
> [1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-06 13:17     ` David Revoy
@ 2023-11-06 16:59       ` Benjamin Tissoires
  2023-11-06 20:06         ` Illia Ostapyshyn
  2023-11-08 22:28         ` David Revoy
  0 siblings, 2 replies; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-06 16:59 UTC (permalink / raw)
  To: David Revoy
  Cc: Illia Ostapyshyn, jkosina, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

Hi David,

On Mon, Nov 6, 2023 at 2:18 PM David Revoy <davidrevoy@protonmail.com> wrote:
>
> Hi Illia, Jiri, Bagas,
>
> Thanks to Jiri for forwarding my request for help to this mailing list.
>
> I apologise in advance to Bagas and you all for not being able to properly configure my email client to follow the correct etiquette (Protonmail, unsuitable for kernel development, but we've made some progress with them on Mastodon on the encryption issue [1]).

Hopefully you'll be able to figure out a way to have your emails
posted to the lkml soon. It's very valuable to have them in
lore.kernel.org to get the thread context.

>
> Thanks to Illia for the detailed explanation. Thanks also for fixing it, and for explaining how the fix broke my workflow, and for your kind words about the situation. I appreciate it.
>
> However, after reading your reply, I still have my doubts about the preference to put an eraser on the top button of the pen by default. But after a few days and re-reading your first sentence "The XP-Pen hardware reports the Eraser usage for the upper stylus button.", I *think* I understood it: this is an internal signal that is sent by the device itself, and is therefore a specification that has to be followed, right? In this case, it makes sense for a generic driver to follow such a signal if it is sent by the hardware.

Yes, you are correct. The device talks a given protocol (HID) and is
supposed to use the specification from Microsoft[0] on how to behave
properly. As such, it has to convey the "eraser" state by adding
dedicated information in the event stream.

In your case, I think we (the kernel and your stylus) simply talk past
each other and we lose information.

>
> Having said that, behaving like this still makes no sense in one case: I'm thinking of my other display tablet, the XPPEN 16 Artist Pro (Gen2). In this case, the stylus has the top button as an eraser, and an eraser tip as well, as you can see in this photo [2]. Was it also a decision by XPPEN to include this signal in the hardware, knowing that they already had an eraser tip? In this case, please let me know, as it sounds like a hardware problem at the design stage and I have probably a way of passing on the information to their technical team.

If the pen has 2 buttons, and an eraser side, it would be a serious
design flow for XPPEN to report both as eraser.

Could you please use sudo hid-recorder from hid-tools[1] on any kernel
version and send us the logs here?
I'll be able to replay the events locally, and understand why the
kernel doesn't work properly.

And if there is a design flaw that can be fixed, we might even be able
to use hid-bpf to change it :)

>
> > You can still remap the eraser button to a right click using xsetwacom:
> > xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
>
> Unfortunately, it doesn't work.
>
> Firstly, such tablets are often connected to a computer that already has a display. So each device (pen/eraser) needs to be mapped to the correct display and set to the correct 'Area' for calibration. A better syntax in my case to test your workaround is written like this:
>
> tableteraser="UGTABLET 24 inch PenDisplay eraser"
> xsetwacom set "$tableteraser" MapToOutput "DisplayPort-2"
> xsetwacom set "$tableteraser" Area 100 120 32794 32797
> xsetwacom set "$tableteraser" "Button" "1" "3"
>
> Secondly, forwarding the eraser button 1 to button 3 (a right click) in xsetwacom doesn't work.
>
> Pressing the button switches the device to an eraser and no right click happens. You'll need to hold down the button and click with the tip of the pen to create a right-click event.
>
> In a digital painting session in Krita, the software will switch your device to an eraser tool preset while you hold down the button, and the feedback on the canvas will be the eraser cursor. You'll then have to click "with the eraser" to get the right-click that triggers the pop-up palette [3].
>
> I'd also like to mention that if you haven't correctly mapped and calibrated your eraser device with xsetwacom, as I did in the code snippet above, the cursor will by default jump to a different location when you hold down the eraser button. It can be on a different display.
>
> Finally, in the case of the XPPEN 16 Artist Pro (Gen2) with a real eraser device (Photo, [2]), the setting 'xsetwacom set "$tableteraser" "Button" "1" "3"' will affect both erasers. Flipping the stylus on the eraser side and entering in 'hover' mode will return a correct eraser, but as soon as you start erasing with the tip, a right click will also be entered.
>
> > You can also do this with "xinput set-button-map", which works for libinput as well.
>
> $ xinput list
> ⎡ Virtual core pointer
> ⎜   ↳ UGTABLET 24 inch PenDisplay Mouse         id=8    [slave  pointer  (2)]
> ⎜   ↳ UGTABLET 24 inch PenDisplay stylus        id=10   [slave  pointer  (2)]
> ⎜   ↳ UGTABLET 24 inch PenDisplay eraser        id=17   [slave  pointer  (2)]
> [...]
> $ xinput get-button-map 17
> 1 2 3 4 5 6 7 8
> $ xinput set-button-map 17 3 3 3 3 3 3 3 3
> $ xinput get-button-map 17
> 3 3 3 3 3 3 3 3
>
> Even after that, it doesn't work. My original article [4] mentions in the last paragraph that I have tested almost all methods, and this was one of them. Even a udev rule doesn't fix it. For reference, xinput produces the same behaviour as xsetwacom: you have to hold the button (it triggers an eraser device switch) then click with the tip to get a right-click...

Generally speaking, relying on X to fix your hardware is going to be a
dead end. When you switch to wayland, you'll lose all of your fixes,
which isn't great.

>
> > We have tested this with several XP-Pen devices, including Artist 24.
>
> Sorry if my tests show something different. Maybe there is something wrong with my GNU/Linux operating system (Fedora 38 KDE spin). Let me know if you have any idea why I get these results and guide me with what distro I should switch to get a similar observation than you do (and also to report the issue to the Fedora team).
>
> ---
>
> All in all (and in the case that my observations and tests are correct), I still stand by the points that I made in my blog post:
>
> - I don't have any way to customise the hardcoded eraser buttons in userspace and **it is a problem**: for devices without a touchscreen or mouse, not having access to a right-click by default on the device is a handicap. Many actions on the D.E. and applications use the right click. The preference to replace it with an 'eraser switch' action by default is IMHO highly debatable here.

AFAIU, the kernel now "merges" both buttons, which is a problem. It
seems to be a serious regression. This case is also worrying because I
added regression tests on hid, but I don't have access to all of the
various tablets, so I implemented them from the Microsoft
specification[0]. We need a special case for you here.

> - In the case of a pen that already has an eraser tip on the other side of the device [2], it makes no sense to exchange the right-click top button for another way to erase. Also in userspace, there seems to be no way to distinguish between the two buttons (the eraser tip and the eraser button). All actions from xsetwacom, xinput/libinput target the two eraser devices, and they target them unsuccessfully too.

Again, we can't do more than what the device reports. Well, we can
always quirk it in some cases, but ideally we don't have to do
anything. MS spec [0], only shows a single button pen with an eraser
tail or a pen with 2 buttons, one of them being the eraser one. I'm
not sure if they decided to prevent dual button pens with eraser tail,
but Wacom definitely has some, and they work.

Without the actual data from your pen, I can not tell much, because I
don't follow which path we are taking on the kernel side. Please
report with your hid-recorder logs, so I can understand why this is
happening and how we can fix it.

>
> [1]: https://mastodon.social/@protonmail/111346195283677411
> [2]: https://www.davidrevoy.com/data/images/blog/2023/2023-11-01_linux-kernel-broke-my-stylus_xp-pen-artist-pro-16-gen2_net.jpg
> [3]: https://docs.krita.org/en/reference_manual/popup-palette.html
> [4]: https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help
>
>
> ------- Original Message -------
> On Friday, November 3rd, 2023 at 21:05, Illia Ostapyshyn <ostapyshyn@sra.uni-hannover.de> wrote:
>
>
> > Hello David, Hello Jiri,
> >
> > The XP-Pen hardware reports the Eraser usage for the upper stylus button.
> > Generally, styli report Invert usages when erasing, as described in [1].
> > XP-Pen digitizers, however, tend to omit them.
> >
> > The generic driver maps the Eraser usage to BTN_TOUCH and the Invert
> > usage to BTN_TOOL_RUBBER. Pens conforming to [1] send the Invert usage
> > first (switching the tool to BTN_TOOL_RUBBER) followed by Eraser, which
> > appears in userspace as a BTN_TOUCH event with the rubber tool set.
> >
> > Due to an oversight, devices not reporting Invert had the BTN_TOOL_RUBBER
> > event masked. This has caused the kernel to send only BTN_TOUCH events
> > without the tool switch when erasing.
> >
> > The situation got worse with refactoring done in 87562fcd1342. An eraser
> > without Invert caused the hidinput_hid_event state machine to get stuck
> > with BTN_TOOL_RUBBER internally (due to it being masked). For the
> > userspace, this looked as if the pen was never hovering again, rendering
> > it unusable until the next reset. 276e14e6c3 fixes this by adding
> > support for digitizers that do not report Invert usages when erasing.

AFAICT 276e14e6c3 already had the hid tests integrated at
tools/testing/selftests/hid/tests/test_tablet.py

It would have been great to add the cases you are mentioning here,
because there is a strong chance I'll break things once again when we
try to fix that regression.


> >
> > ---
> >
> > David, we are sorry that our patch broke your workflow. However,
> > forwarding hardware events as-is to the userspace has always been the
> > intended behavior, with a kernel bug preventing it so far. You can still
> > remap the eraser button to a right click using xsetwacom:
> >
> > xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
> >
> > Replace the device name with the corresponding eraser device from
> > "xsetwacom list devices". You can also do this with "xinput set-button-map",
> > which works for libinput as well. We have tested this with several
> > XP-Pen devices, including Artist 24.
> >
> > [1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
>

Cheers,
Benjamin

[0] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
[1] https://gitlab.freedesktop.org/libevdev/hid-tools


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-06 16:59       ` Benjamin Tissoires
@ 2023-11-06 20:06         ` Illia Ostapyshyn
  2023-11-07  7:59           ` Benjamin Tissoires
  2023-11-08 22:28         ` David Revoy
  1 sibling, 1 reply; 40+ messages in thread
From: Illia Ostapyshyn @ 2023-11-06 20:06 UTC (permalink / raw)
  To: Benjamin Tissoires, David Revoy
  Cc: jkosina, jason.gerecke, jose.exposito89, linux-input,
	linux-kernel, nils, peter.hutterer, ping.cheng, bagasdotme

On 11/6/23 17:59, Benjamin Tissoires wrote:

> If the pen has 2 buttons, and an eraser side, it would be a serious
> design flow for XPPEN to report both as eraser.
> 
> Could you please use sudo hid-recorder from hid-tools[1] on any kernel
> version and send us the logs here?
> I'll be able to replay the events locally, and understand why the
> kernel doesn't work properly.
> 
> And if there is a design flaw that can be fixed, we might even be able
> to use hid-bpf to change it :)

My wild guess is that XP-Pen 16 Artist Pro reports an Eraser usage 
without Invert for the upper button and Eraser with Invert for the 
eraser tip.  A device-specific driver could work with that, but there 
seems to be no way to incorporate two different erasers (thus, allowing 
userspace to map them to different actions arbitrarily) in the generic 
driver currently.

> Generally speaking, relying on X to fix your hardware is going to be a
> dead end. When you switch to wayland, you'll lose all of your fixes,
> which isn't great.

> AFAIU, the kernel now "merges" both buttons, which is a problem. It
> seems to be a serious regression. This case is also worrying because I
> added regression tests on hid, but I don't have access to all of the
> various tablets, so I implemented them from the Microsoft
> specification[0]. We need a special case for you here.

The issue preventing David from mapping HID_DG_ERASER to BTN_STYLUS2 is 
that the hidinput_hid_event is not compatible with hidinput_setkeycode. 
If usage->code is no longer BTN_TOUCH after remapping, it won't be 
released when Eraser reports 0.  A simple fix is:

--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1589,7 +1589,7 @@ void hidinput_hid_event(struct hid_device *hid, 
struct hid_field *field, struct
  			/* value is off, tool is not rubber, ignore */
  			return;
  		else if (*quirks & HID_QUIRK_NOINVERT &&
-			 !test_bit(BTN_TOUCH, input->key)) {
+			 !test_bit(usage->code, input->key)) {
  			/*
  			 * There is no invert to release the tool, let hid_input
  			 * send BTN_TOUCH with scancode and release the tool after.

This change alone fixes David's problem and the right-click mapping in 
hwdb works again.  However, the tool switches to rubber for the remapped 
eraser (here BTN_STYLUS2) events, both for devices with and without 
Invert.  This does no harm but is not useful either.  A cleaner solution 
for devices without Invert would be to omit the whole tool switching 
logic in this case:

@@ -1577,6 +1577,9 @@ void hidinput_hid_event(struct hid_device *hid, 
struct hid_field *field, struct

  	switch (usage->hid) {
  	case HID_DG_ERASER:
+		if (*quirks & HID_QUIRK_NOINVERT && usage->code != BTN_TOUCH)
+			break;
+
  		report->tool_active |= !!value;

Remapping Invert does not work anyway as the Invert tool is hardcoded in 
hidinput_hid_event.  Even worse, I guess (not tested) trying to do so 
would mask BTN_TOOL_RUBBER from dev->keybit and could cause weird 
behavior similar to one between 87562fcd1342 and 276e14e6c3.  This 
raises the question: should users be able to remap Invert after all?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-06 20:06         ` Illia Ostapyshyn
@ 2023-11-07  7:59           ` Benjamin Tissoires
  2023-11-07 13:40             ` Illia Ostapyshyn
                               ` (2 more replies)
  0 siblings, 3 replies; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-07  7:59 UTC (permalink / raw)
  To: Illia Ostapyshyn
  Cc: David Revoy, jkosina, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
<ostapyshyn@sra.uni-hannover.de> wrote:
>
> On 11/6/23 17:59, Benjamin Tissoires wrote:
>
> > If the pen has 2 buttons, and an eraser side, it would be a serious
> > design flow for XPPEN to report both as eraser.
> >
> > Could you please use sudo hid-recorder from hid-tools[1] on any kernel
> > version and send us the logs here?
> > I'll be able to replay the events locally, and understand why the
> > kernel doesn't work properly.
> >
> > And if there is a design flaw that can be fixed, we might even be able
> > to use hid-bpf to change it :)
>
> My wild guess is that XP-Pen 16 Artist Pro reports an Eraser usage
> without Invert for the upper button and Eraser with Invert for the
> eraser tip.  A device-specific driver could work with that, but there
> seems to be no way to incorporate two different erasers (thus, allowing
> userspace to map them to different actions arbitrarily) in the generic
> driver currently.

That's exactly why I want to see the exact event flow. We can not do
"wild guesses" unfortunately (not meaning any offenses).
And I am very suspicious about the fact that the stylus reports 2
identical erasers. Because in the past David seemed to be able to have
2 distincts behaviors for the 2 "buttons" (physical button and eraser
tail).

>
>
> > Generally speaking, relying on X to fix your hardware is going to be a
> > dead end. When you switch to wayland, you'll lose all of your fixes,
> > which isn't great.
>
> > AFAIU, the kernel now "merges" both buttons, which is a problem. It
> > seems to be a serious regression. This case is also worrying because I
> > added regression tests on hid, but I don't have access to all of the
> > various tablets, so I implemented them from the Microsoft
> > specification[0]. We need a special case for you here.
>
> The issue preventing David from mapping HID_DG_ERASER to BTN_STYLUS2 is
> that the hidinput_hid_event is not compatible with hidinput_setkeycode.
> If usage->code is no longer BTN_TOUCH after remapping, it won't be
> released when Eraser reports 0.  A simple fix is:

I must confess, being the one who refactored everything, I still don't
believe this is as simple as it may seem. I paged out all of the
special cases, and now, without seeing the event flow I just can not
understand why this would fix the situation.

And BTW, if you have a tool affected by 276e14e6c3, I'd be curious to
get a hid-recorder sample for it so I can get regression tests for it.

>
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1589,7 +1589,7 @@ void hidinput_hid_event(struct hid_device *hid,
> struct hid_field *field, struct
>                         /* value is off, tool is not rubber, ignore */
>                         return;
>                 else if (*quirks & HID_QUIRK_NOINVERT &&
> -                        !test_bit(BTN_TOUCH, input->key)) {
> +                        !test_bit(usage->code, input->key)) {

I don't want to be rude, but this feels very much like black magic,
especially because there is a comment just below and it is not
updated. So either the explanation was wrong, or it's not explaining
the situation (I also understand that this is not a formal submission,
so maybe that's the reason why the comment is not updated).

>                         /*
>                          * There is no invert to release the tool, let hid_input
>                          * send BTN_TOUCH with scancode and release the tool after.
>
> This change alone fixes David's problem and the right-click mapping in
> hwdb works again.  However, the tool switches to rubber for the remapped
> eraser (here BTN_STYLUS2) events, both for devices with and without
> Invert.  This does no harm but is not useful either.  A cleaner solution
> for devices without Invert would be to omit the whole tool switching
> logic in this case:
>
> @@ -1577,6 +1577,9 @@ void hidinput_hid_event(struct hid_device *hid,
> struct hid_field *field, struct
>
>         switch (usage->hid) {
>         case HID_DG_ERASER:
> +               if (*quirks & HID_QUIRK_NOINVERT && usage->code != BTN_TOUCH)
> +                       break;
> +
>                 report->tool_active |= !!value;
>
> Remapping Invert does not work anyway as the Invert tool is hardcoded in
> hidinput_hid_event.  Even worse, I guess (not tested) trying to do so
> would mask BTN_TOOL_RUBBER from dev->keybit and could cause weird
> behavior similar to one between 87562fcd1342 and 276e14e6c3.  This
> raises the question: should users be able to remap Invert after all?
>

The kernel is supposed to transfer what the device is. So if it says
this is an eraser, we should not try to change it. Users can then
tweak their own device if they wish through hid-bpf or through
libinput quirks, but when you install a fresh kernel without tweaks,
we should be as accurate as possible.

My main concern is that now we have a device which exports 2 different
interactions as being the same. So either the firmware is wrong, and
we need to quirk it, or the kernel is wrong and merges both, and this
needs fixes as well.

Once every interaction on the device gets its own behavior, userspace
can do whatever they want. It's not the kernel's concern anymore.

BTW, David, were you able to do a revert of 276e14e6c3?

Cheers,
Benjamin


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-07  7:59           ` Benjamin Tissoires
@ 2023-11-07 13:40             ` Illia Ostapyshyn
  2023-11-08  5:23             ` Eric GOUYER
  2023-11-08 23:18             ` David Revoy
  2 siblings, 0 replies; 40+ messages in thread
From: Illia Ostapyshyn @ 2023-11-07 13:40 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: David Revoy, jkosina, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

Sending again because the mail bounced from the mailing list due to the
attachment.

Benjamin Tissoires <benjamin.tissoires@redhat.com> writes:

> And BTW, if you have a tool affected by 276e14e6c3, I'd be curious to
> get a hid-recorder sample for it so I can get regression tests for it.

I have attached [3] the recording of me:

(1) Bringing the stylus in range, touching the screen with the tip and
    bringing the stylus out of range.

(2) Bringing the stylus in range, pressing the top barrel button and
    bringing the stylus out of range.

(3) Bringing the stylus in range, touching the screen with the tip again
    and bringing the stylus out of range.

The digitizer is the one of the two that David uses, XP-Pen Artist 24.
I don't have the other one with two erasers here, so we'd have to wait
for David's recording to investigate further.

If you revert 276e14e6c3, you can observe that after pressing the eraser
button, neither BTN_TOOL_PEN nor BTN_TOUCH events will appear in evdev
anymore for (3).

> I must confess, being the one who refactored everything, I still don't
> believe this is as simple as it may seem. I paged out all of the
> special cases, and now, without seeing the event flow I just can not
> understand why this would fix the situation.

David uses hwdb to remap Eraser (0xd0045) to BTN_STYLUS2 (0x14c) [1]:

evdev:input:b0003v28BDp092De0100-e0*
 KEYBOARD_KEY_d0045=0x14c

In the end, this translates to a hidinput_setkeycode with the respective
arguments, setting usage->code to BTN_STYLUS2.  In the current state,
doing so results in BTN_STYLUS2 being permanently set and never released
when pressing the top barrel switch.  You can replay and observe this
with the attached [3] recording.

The if statement [2] is there to release BTN_TOOL_RUBBER if the device
has no Invert, but only after BTN_TOUCH has been released.  Eraser with
value 0 releases BTN_TOUCH in the first iteration and BTN_TOOL_RUBBER in
the second (when BTN_TOUCH is not in input->key anymore).

The problem is that the condition assumes usage->code is BTN_TOUCH.
When this is not the case, (!test_bit(BTN_TOUCH, input->key)) is always
true, we release the tool and return prematurely.  Therefore,
usage->code is never released.

As such, BTN_TOOL_RUBBER is not the problem and does no harm (except for
maybe showing the rubber icon in Krita).  It is required, however, for a
functional eraser out of the box.  I think, in the HID_QUIRK_NOINVERT
case, BTN_TOOL_RUBBER should better be omitted if Eraser is remapped to
something else, like BTN_STYLUS2.  Hence the second proposal.

> So either the explanation was wrong, or it's not explaining the
> situation (I also understand that this is not a formal submission, so
> maybe that's the reason why the comment is not updated).

Right, the example was not meant as a formal submission, that's why I
didn't change the comment.  Sorry for that.  We should fix the comment
below it (line 1603) too after this is resolved.

Cheers,
Illia

[1]
https://www.davidrevoy.com/article842/review-xp-pen-artist-24-pro-on-linux
[2]
https://elixir.bootlin.com/linux/latest/source/drivers/hid/hid-input.c#L1594
[3] https://dl.uni-h.de/?t=dc4a5542a8e4d54964e298045a173049


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-07  7:59           ` Benjamin Tissoires
  2023-11-07 13:40             ` Illia Ostapyshyn
@ 2023-11-08  5:23             ` Eric GOUYER
  2023-11-08  9:04               ` Benjamin Tissoires
  2023-11-08 23:18             ` David Revoy
  2 siblings, 1 reply; 40+ messages in thread
From: Eric GOUYER @ 2023-11-08  5:23 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Illia Ostapyshyn, David Revoy, jkosina, jason.gerecke,
	jose.exposito89, linux-input, linux-kernel

Sorry, below I re-send my email which bounced due to exceeding 100k
(hid-recorder traces included), so I re-send it without them, for some
book-keeping.

I had originally replied-to-all, so mainteners involved should have
received those traces just before (I hope).

Best Regards,

-----8<-----8<-----8<-----8<

Hello, I have the same tablet than OP (David) :
- XP-Pen Artist Pro 16 Gen 2
- on Ubuntu 23.10 linux-image-generic 6.5.0.10.12

I am not (yet ?) encountering the problem described above since I guess
that my kernel is before the suspected regression.

Here below I included much detail, but you can TL;DR + jump at the five
hid-recorder traces below.

> On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> <ostapyshyn@sra.uni-hannover.de> wrote:
> >
> > On 11/6/23 17:59, Benjamin Tissoires wrote:
> >  
> > > If the pen has 2 buttons, and an eraser side, it would be a
> > > serious design flow for XPPEN to report both as eraser.
> > >
> > > Could you please use sudo hid-recorder from hid-tools[1] on any
> > > kernel version and send us the logs here?
> > > I'll be able to replay the events locally, and understand why the
> > > kernel doesn't work properly.
> > >
> > > And if there is a design flaw that can be fixed, we might even be
> > > able to use hid-bpf to change it :)  
> >
> > My wild guess is that XP-Pen 16 Artist Pro reports :
> > - (1) an Eraser usage without Invert for the upper button
> > - (2) and Eraser with Invert for the eraser tip.

I think you will agree with below traces that :
- (1) : correct (it reports invert=0 eraser=1 tipSwitch=1)
- (2) : no, for the rubber tip, it reports invert=1 eraser=0 tipSwitch=1

> > A device-specific driver could work with that, but
> > there seems to be no way to incorporate two different erasers
> > (thus, allowing userspace to map them to different actions
> > arbitrarily) in the generic driver currently.  
> 
> That's exactly why I want to see the exact event flow. We can not do
> "wild guesses" unfortunately (not meaning any offenses).
> And I am very suspicious about the fact that the stylus reports 2
> identical erasers. Because in the past David seemed to be able to have
> 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> tail).

The Pen, hardware-wise, has the following possibilities :
- The (main) pressure tip
- The "bottom" button (nearest of the main pressure tip)
- The "top" button (farthest of the main tip
- The "back" pressure tip (it has pressure!) somewhat called rubber

All of those works I think -natively- on my kernel version, without
external 3rd-party kernel modules.

It works especially on Blender, where I can :
- click (or paint) with main pressure-tip
- middle-click (to pan viewport) with "bottom" button + move main tip
- right-click with "top" button
- erase with backside pressure-tip

I installed a .deb from the official website [1] which does not contain
kernel modules, and seems to only contains :
- a udev rule to, I think, chmod 0666 to input character devices
- a Qt app to configure some behavior, only userland-side.

I think I observed that before running the Qt app, there was only 3
xinput devices, and after launching it, there were 4 more (7 total).

Blender would not receive pen input without lanching the Qt app.
Indeed, the "first" 3 /dev/input/event* associated to the pen are
chmod'ed too restrictively (0660) for an underprivileged user.

Anyway, after running the Qt App, 4 *ANOTHER* /dev/input/event* pops,
correctly chmod'ed (thanks to the udev rules), and Blender works.

My best guess would be that the first 3 very-native /dev/input/
devices are somewhat "raw", and that the Qt app feeds those inputs to
the Qt app's configured pressure curve + remap the button to the input
as chosen by the user.

Here is the dmesg before launching the Qt App :
-----8<-----8<-----8<-----8<
usb 3-9: new full-speed USB device number 64 using xhci_hcd
usb 3-9: New USB device found, idVendor=28bd, idProduct=095b,
bcdDevice= 0.00
usb 3-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 3-9: Product: Artist Pro 16 (Gen2)
usb 3-9: Manufacturer: UGTABLET
usb 3-9: SerialNumber: 00000
input: UGTABLET Artist Pro 16 (Gen2) Mouse as
/devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input196
input: UGTABLET Artist Pro 16 (Gen2) Keyboard as
/devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input197
hid-generic 0003:28BD:095B.009E: input,hidraw4: USB HID v1.00 Mouse
[UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input0
input: UGTABLET Artist Pro 16 (Gen2) as
/devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.1/0003:28BD:095B.009F/input/input198
hid-generic 0003:28BD:095B.009F: input,hidraw7: USB HID v1.00 Device
[UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input1
hid-generic 0003:28BD:095B.00A0: hiddev0,hidraw12: USB HID v1.00 Device
[UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input2
-----8<-----8<-----8<-----8<

This make "sudo xinput" reports 3 new devices :
UGTABLET Artist Pro 16 (Gen2) Mouse id=x [slave pointer (n)]
UGTABLET Artist Pro 16 (Gen2) Keyboard id=y [slave keyboard (n+1)]
UGTABLET Artist Pro 16 (Gen2) id=z [slave keyboard (n+1)]
(sorry I've masked the id to prevent confusion, I did not run "xinput"
at the same time/power cycle than "dmesg")

Running the Qt App makes "sudo xinput" report 4 new devices :
XP-Pen Mouse id=xc [slave pointer (n)]
XP-Pen Eraser id=xa [slave keyboard (n+1)]
XP-Pen Pen id=xb [slave keyboard (n+1)] << only this chmod'ed 0666
XP-Pen Mouse id=xd [slave keyboard (n+1)]

And... it suffices to make Blender works without configuring anything.

Anyway, besides this user experience / userland Qt app, here are some
five hid-recorder traces ;
I re-used the terminology appearing in the traces :
- "barrel" : the "bottom" button nearest of the main tip
- "erase" : the farther "top" button, just above "barrel"
- "back" : the "rubber" secondary pressure tip at the back of the pen

(1) hidraw7_inRange_contact_move_lift_outOfRange
I "contact" the main tip, move it, and lift it out or range.

(2) hidraw7_inRange_contact_barrelPress\
_move_barrelRelease_lift_outOfRange
I "contact" the main tip, press "barrel" (which is the button nearest
of the tip), move the pen, release the button, move it out of range.

(3) hidraw7_inRange_contact_erasePress\
_move_eraseRelease_lift_outOfRange
The same than above, except that I instead use the "erase" button,
i.e. the button just above ("vertically speaking") of the "main" button.

(4) hidraw7_inRange_contact_BOTHbarrelAnderasePress_move_\
BOTHbarrelAnderaseRelease_lift_outOfRange
That's just plain stupid, and uncomfortable to do, but it's just to
show an example of the capability of the hardware ;
In this one, I'm doing the same than above, except I stupidly press BOTH
buttons.
It means that I "contact" the main tip, press both barrel+erase, move
the pen, release both button, lift it out of range.

(5) hidraw7_inRange_contact_backPress_move_backRelease_lift_outOfRange
Back to non-stupid tests, here I am using the "rubber" (which has
pressure !), i.e. I use the back of the pen.
I contact it, press the rubber to have some pressure, move it, release
the pressure, move the pen ouf of contact and out of range.

Reading those traces, I think I observed some (to me) very logical
behavior ;

I agree that the Pen is possibly not acting in respect of the
specification, by the fact that... the spec does not differentiate the
"eraser" 2nd button, versus the "backside" rubber pressure tip.

The traces shows that :
- In range [0 - 1] is correctly reported, both for main tip + rubber tip
- Tip Pressure [0 - n] is correctly reported, for both pressure tips

- Tip Switch [0 - 1] is correctly set to ONE for all traces, when the
  in-range tip goes to contact. (either main tip or rubber tip)

- Barrel Switch [0 - 1] is correctly set to ONE only when pressing
  button down, and of course only when using the main tip

- Invert [0 - 1] is set to ONE when (and only) using the back rubber
  tip, and the ERASE button (2nd button) does NOT set it, so "INVERT"

- Eraser [0 - 1] is set to ONE ONLY when pressing the 2nd top button,
  and of :
  - ONLY when using the main itp
  - and if you were to instead use the "back" of the pen (rubber tip),
    then you will have invert=1 + eraser=0

The main extract of those is, I think, that :
- eraser "top" button is completely independent of the back "rubber"
- eraser "top" button NEVER concerns itself to set invert=1
- backside "rubber" pressure tip will set invert=1, but leave eraser=0

So, the behavior probably breaks the specs, but sincerely I'm happy to
have the "eraser" button independent of the "rubber eraser", which
makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
rubber), and I would like to keep this.

Best Regards,

[1] https://www.xp-pen.fr/download-1027.html ; the .deb Qt app

-- 
Eric GOUYER

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08  5:23             ` Eric GOUYER
@ 2023-11-08  9:04               ` Benjamin Tissoires
  2023-11-08  9:23                 ` José Expósito
  2023-11-08 19:40                 ` Nils Fuhler
  0 siblings, 2 replies; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-08  9:04 UTC (permalink / raw)
  To: Eric GOUYER
  Cc: Illia Ostapyshyn, David Revoy, jkosina, jason.gerecke,
	jose.exposito89, linux-input, linux-kernel

Thanks Eric and Illia for the logs and the explanations.

On Wed, Nov 8, 2023 at 6:23 AM Eric GOUYER <folays@gmail.com> wrote:
>
> Sorry, below I re-send my email which bounced due to exceeding 100k
> (hid-recorder traces included), so I re-send it without them, for some
> book-keeping.
>
> I had originally replied-to-all, so mainteners involved should have
> received those traces just before (I hope).
>
> Best Regards,
>
> -----8<-----8<-----8<-----8<
>
> Hello, I have the same tablet than OP (David) :
> - XP-Pen Artist Pro 16 Gen 2
> - on Ubuntu 23.10 linux-image-generic 6.5.0.10.12
>
> I am not (yet ?) encountering the problem described above since I guess
> that my kernel is before the suspected regression.
>
> Here below I included much detail, but you can TL;DR + jump at the five
> hid-recorder traces below.
>
> > On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> > <ostapyshyn@sra.uni-hannover.de> wrote:
> > >
> > > On 11/6/23 17:59, Benjamin Tissoires wrote:
> > >
> > > > If the pen has 2 buttons, and an eraser side, it would be a
> > > > serious design flow for XPPEN to report both as eraser.
> > > >
> > > > Could you please use sudo hid-recorder from hid-tools[1] on any
> > > > kernel version and send us the logs here?
> > > > I'll be able to replay the events locally, and understand why the
> > > > kernel doesn't work properly.
> > > >
> > > > And if there is a design flaw that can be fixed, we might even be
> > > > able to use hid-bpf to change it :)
> > >
> > > My wild guess is that XP-Pen 16 Artist Pro reports :
> > > - (1) an Eraser usage without Invert for the upper button
> > > - (2) and Eraser with Invert for the eraser tip.
>
> I think you will agree with below traces that :
> - (1) : correct (it reports invert=0 eraser=1 tipSwitch=1)
> - (2) : no, for the rubber tip, it reports invert=1 eraser=0 tipSwitch=1
>
> > > A device-specific driver could work with that, but
> > > there seems to be no way to incorporate two different erasers
> > > (thus, allowing userspace to map them to different actions
> > > arbitrarily) in the generic driver currently.
> >
> > That's exactly why I want to see the exact event flow. We can not do
> > "wild guesses" unfortunately (not meaning any offenses).
> > And I am very suspicious about the fact that the stylus reports 2
> > identical erasers. Because in the past David seemed to be able to have
> > 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> > tail).
>
> The Pen, hardware-wise, has the following possibilities :
> - The (main) pressure tip
> - The "bottom" button (nearest of the main pressure tip)
> - The "top" button (farthest of the main tip
> - The "back" pressure tip (it has pressure!) somewhat called rubber
>
> All of those works I think -natively- on my kernel version, without
> external 3rd-party kernel modules.
>
> It works especially on Blender, where I can :
> - click (or paint) with main pressure-tip
> - middle-click (to pan viewport) with "bottom" button + move main tip
> - right-click with "top" button
> - erase with backside pressure-tip
>
> I installed a .deb from the official website [1] which does not contain
> kernel modules, and seems to only contains :
> - a udev rule to, I think, chmod 0666 to input character devices
> - a Qt app to configure some behavior, only userland-side.
>
> I think I observed that before running the Qt app, there was only 3
> xinput devices, and after launching it, there were 4 more (7 total).
>
> Blender would not receive pen input without lanching the Qt app.
> Indeed, the "first" 3 /dev/input/event* associated to the pen are
> chmod'ed too restrictively (0660) for an underprivileged user.

This is wrong (not your fault, of course). There is no way Blender
should directly talk to the input nodes. It should use the events from
the compositor, not randomly getting values from input nodes. sigh...

>
> Anyway, after running the Qt App, 4 *ANOTHER* /dev/input/event* pops,
> correctly chmod'ed (thanks to the udev rules), and Blender works.

I got a quick look, and those events are coming from uinput. So their
Qt App is just a horrible user space driver to fix their tablet. Ouch.

>
> My best guess would be that the first 3 very-native /dev/input/
> devices are somewhat "raw", and that the Qt app feeds those inputs to
> the Qt app's configured pressure curve + remap the button to the input
> as chosen by the user.

The "raw" nodes are giving all of the information, but not correctly.
So they are using a separate userspace driver to "fix" it.

>
> Here is the dmesg before launching the Qt App :
> -----8<-----8<-----8<-----8<
> usb 3-9: new full-speed USB device number 64 using xhci_hcd
> usb 3-9: New USB device found, idVendor=28bd, idProduct=095b,
> bcdDevice= 0.00
> usb 3-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 3-9: Product: Artist Pro 16 (Gen2)
> usb 3-9: Manufacturer: UGTABLET
> usb 3-9: SerialNumber: 00000
> input: UGTABLET Artist Pro 16 (Gen2) Mouse as
> /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input196
> input: UGTABLET Artist Pro 16 (Gen2) Keyboard as
> /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input197
> hid-generic 0003:28BD:095B.009E: input,hidraw4: USB HID v1.00 Mouse
> [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input0
> input: UGTABLET Artist Pro 16 (Gen2) as
> /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.1/0003:28BD:095B.009F/input/input198
> hid-generic 0003:28BD:095B.009F: input,hidraw7: USB HID v1.00 Device
> [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input1
> hid-generic 0003:28BD:095B.00A0: hiddev0,hidraw12: USB HID v1.00 Device
> [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input2
> -----8<-----8<-----8<-----8<
>
> This make "sudo xinput" reports 3 new devices :
> UGTABLET Artist Pro 16 (Gen2) Mouse id=x [slave pointer (n)]
> UGTABLET Artist Pro 16 (Gen2) Keyboard id=y [slave keyboard (n+1)]
> UGTABLET Artist Pro 16 (Gen2) id=z [slave keyboard (n+1)]
> (sorry I've masked the id to prevent confusion, I did not run "xinput"
> at the same time/power cycle than "dmesg")
>
> Running the Qt App makes "sudo xinput" report 4 new devices :
> XP-Pen Mouse id=xc [slave pointer (n)]
> XP-Pen Eraser id=xa [slave keyboard (n+1)]
> XP-Pen Pen id=xb [slave keyboard (n+1)] << only this chmod'ed 0666
> XP-Pen Mouse id=xd [slave keyboard (n+1)]
>
> And... it suffices to make Blender works without configuring anything.
>
> Anyway, besides this user experience / userland Qt app, here are some
> five hid-recorder traces ;

Many thanks for attaching them and for digging into them.

> I re-used the terminology appearing in the traces :
> - "barrel" : the "bottom" button nearest of the main tip
> - "erase" : the farther "top" button, just above "barrel"
> - "back" : the "rubber" secondary pressure tip at the back of the pen
>
> (1) hidraw7_inRange_contact_move_lift_outOfRange
> I "contact" the main tip, move it, and lift it out or range.
>
> (2) hidraw7_inRange_contact_barrelPress\
> _move_barrelRelease_lift_outOfRange
> I "contact" the main tip, press "barrel" (which is the button nearest
> of the tip), move the pen, release the button, move it out of range.
>
> (3) hidraw7_inRange_contact_erasePress\
> _move_eraseRelease_lift_outOfRange
> The same than above, except that I instead use the "erase" button,
> i.e. the button just above ("vertically speaking") of the "main" button.
>
> (4) hidraw7_inRange_contact_BOTHbarrelAnderasePress_move_\
> BOTHbarrelAnderaseRelease_lift_outOfRange
> That's just plain stupid, and uncomfortable to do, but it's just to
> show an example of the capability of the hardware ;
> In this one, I'm doing the same than above, except I stupidly press BOTH
> buttons.
> It means that I "contact" the main tip, press both barrel+erase, move
> the pen, release both button, lift it out of range.
>
> (5) hidraw7_inRange_contact_backPress_move_backRelease_lift_outOfRange
> Back to non-stupid tests, here I am using the "rubber" (which has
> pressure !), i.e. I use the back of the pen.
> I contact it, press the rubber to have some pressure, move it, release
> the pressure, move the pen ouf of contact and out of range.
>
> Reading those traces, I think I observed some (to me) very logical
> behavior ;
>
> I agree that the Pen is possibly not acting in respect of the
> specification, by the fact that... the spec does not differentiate the
> "eraser" 2nd button, versus the "backside" rubber pressure tip.
>
> The traces shows that :
> - In range [0 - 1] is correctly reported, both for main tip + rubber tip
> - Tip Pressure [0 - n] is correctly reported, for both pressure tips

Yep, that part is correct and follows the specification

>
> - Tip Switch [0 - 1] is correctly set to ONE for all traces, when the
>   in-range tip goes to contact. (either main tip or rubber tip)

This is half wrong (spec-wise):
- tip switch should only be used when the actual tip is in contact.
- when using the rubber tip, we should get an eraser event :(

>
> - Barrel Switch [0 - 1] is correctly set to ONE only when pressing
>   button down, and of course only when using the main tip

This is correct (as in it follows the spec)

>
> - Invert [0 - 1] is set to ONE when (and only) using the back rubber
>   tip, and the ERASE button (2nd button) does NOT set it, so "INVERT"

In your case, the invert usage is correct.
In Illia's case, the invert usage is forwarded by the eraser usage, so
this is just plain wrong there.

>
> - Eraser [0 - 1] is set to ONE ONLY when pressing the 2nd top button,
>   and of :
>   - ONLY when using the main itp
>   - and if you were to instead use the "back" of the pen (rubber tip),
>     then you will have invert=1 + eraser=0

This is not correct (again, according to the specification). XP-Pen
should report SecondaryBarrelSwitch (0x5a in the HID usage table[0])
And again, in Illia's case, this button should be reported as an
invert, and eraser when the tip touches the tablet.

>
> The main extract of those is, I think, that :
> - eraser "top" button is completely independent of the back "rubber"

Yes

> - eraser "top" button NEVER concerns itself to set invert=1

yes

> - backside "rubber" pressure tip will set invert=1, but leave eraser=0

yes, but that's not correct, it should report eraser=1 when you touch
the rubber on the surface.

>
> So, the behavior probably breaks the specs, but sincerely I'm happy to
> have the "eraser" button independent of the "rubber eraser", which
> makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> rubber), and I would like to keep this.

Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
apparently broke it.

So, to me:
- 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
firmware bug (reporting invert through eraser) and should not be
tackled at the generic level, thus it should be reverted
- both of these tablets are forwarding the useful information, but not
correctly, which confuses the kernel
- I should now be able to write regression tests
- I should be able to provide HID-BPF fixes for those tablets so that
we can keep them working with or without
276e14e6c3993317257e1787e93b7166fbc30905
reverted (hopefully)
- problem is I still don't have the mechanics to integrate the HID-BPF
fixes directly in the kernel tree, so maybe I'll have to write a
driver for XP-Pen while these internals are set (it shouldn't
interfere with the HID-BPF out of the tree).

Cheers,
Benjamin

>
> Best Regards,
>
> [1] https://www.xp-pen.fr/download-1027.html ; the .deb Qt app
>
> --
> Eric GOUYER
>

[0] https://usb.org/document-library/hid-usage-tables-14


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08  9:04               ` Benjamin Tissoires
@ 2023-11-08  9:23                 ` José Expósito
  2023-11-08  9:34                   ` Benjamin Tissoires
  2023-11-08 19:40                 ` Nils Fuhler
  1 sibling, 1 reply; 40+ messages in thread
From: José Expósito @ 2023-11-08  9:23 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Eric GOUYER, Illia Ostapyshyn, David Revoy, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi Benjamin,

On Wed, Nov 08, 2023 at 10:04:30AM +0100, Benjamin Tissoires wrote:
> Thanks Eric and Illia for the logs and the explanations.
> 
> On Wed, Nov 8, 2023 at 6:23 AM Eric GOUYER <folays@gmail.com> wrote:
> >
> > Sorry, below I re-send my email which bounced due to exceeding 100k
> > (hid-recorder traces included), so I re-send it without them, for some
> > book-keeping.
> >
> > I had originally replied-to-all, so mainteners involved should have
> > received those traces just before (I hope).
> >
> > Best Regards,
> >
> > -----8<-----8<-----8<-----8<
> >
> > Hello, I have the same tablet than OP (David) :
> > - XP-Pen Artist Pro 16 Gen 2
> > - on Ubuntu 23.10 linux-image-generic 6.5.0.10.12
> >
> > I am not (yet ?) encountering the problem described above since I guess
> > that my kernel is before the suspected regression.
> >
> > Here below I included much detail, but you can TL;DR + jump at the five
> > hid-recorder traces below.
> >
> > > On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> > > <ostapyshyn@sra.uni-hannover.de> wrote:
> > > >
> > > > On 11/6/23 17:59, Benjamin Tissoires wrote:
> > > >
> > > > > If the pen has 2 buttons, and an eraser side, it would be a
> > > > > serious design flow for XPPEN to report both as eraser.
> > > > >
> > > > > Could you please use sudo hid-recorder from hid-tools[1] on any
> > > > > kernel version and send us the logs here?
> > > > > I'll be able to replay the events locally, and understand why the
> > > > > kernel doesn't work properly.
> > > > >
> > > > > And if there is a design flaw that can be fixed, we might even be
> > > > > able to use hid-bpf to change it :)
> > > >
> > > > My wild guess is that XP-Pen 16 Artist Pro reports :
> > > > - (1) an Eraser usage without Invert for the upper button
> > > > - (2) and Eraser with Invert for the eraser tip.
> >
> > I think you will agree with below traces that :
> > - (1) : correct (it reports invert=0 eraser=1 tipSwitch=1)
> > - (2) : no, for the rubber tip, it reports invert=1 eraser=0 tipSwitch=1
> >
> > > > A device-specific driver could work with that, but
> > > > there seems to be no way to incorporate two different erasers
> > > > (thus, allowing userspace to map them to different actions
> > > > arbitrarily) in the generic driver currently.
> > >
> > > That's exactly why I want to see the exact event flow. We can not do
> > > "wild guesses" unfortunately (not meaning any offenses).
> > > And I am very suspicious about the fact that the stylus reports 2
> > > identical erasers. Because in the past David seemed to be able to have
> > > 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> > > tail).
> >
> > The Pen, hardware-wise, has the following possibilities :
> > - The (main) pressure tip
> > - The "bottom" button (nearest of the main pressure tip)
> > - The "top" button (farthest of the main tip
> > - The "back" pressure tip (it has pressure!) somewhat called rubber
> >
> > All of those works I think -natively- on my kernel version, without
> > external 3rd-party kernel modules.
> >
> > It works especially on Blender, where I can :
> > - click (or paint) with main pressure-tip
> > - middle-click (to pan viewport) with "bottom" button + move main tip
> > - right-click with "top" button
> > - erase with backside pressure-tip
> >
> > I installed a .deb from the official website [1] which does not contain
> > kernel modules, and seems to only contains :
> > - a udev rule to, I think, chmod 0666 to input character devices
> > - a Qt app to configure some behavior, only userland-side.
> >
> > I think I observed that before running the Qt app, there was only 3
> > xinput devices, and after launching it, there were 4 more (7 total).
> >
> > Blender would not receive pen input without lanching the Qt app.
> > Indeed, the "first" 3 /dev/input/event* associated to the pen are
> > chmod'ed too restrictively (0660) for an underprivileged user.
> 
> This is wrong (not your fault, of course). There is no way Blender
> should directly talk to the input nodes. It should use the events from
> the compositor, not randomly getting values from input nodes. sigh...
> 
> >
> > Anyway, after running the Qt App, 4 *ANOTHER* /dev/input/event* pops,
> > correctly chmod'ed (thanks to the udev rules), and Blender works.
> 
> I got a quick look, and those events are coming from uinput. So their
> Qt App is just a horrible user space driver to fix their tablet. Ouch.
> 
> >
> > My best guess would be that the first 3 very-native /dev/input/
> > devices are somewhat "raw", and that the Qt app feeds those inputs to
> > the Qt app's configured pressure curve + remap the button to the input
> > as chosen by the user.
> 
> The "raw" nodes are giving all of the information, but not correctly.
> So they are using a separate userspace driver to "fix" it.
> 
> >
> > Here is the dmesg before launching the Qt App :
> > -----8<-----8<-----8<-----8<
> > usb 3-9: new full-speed USB device number 64 using xhci_hcd
> > usb 3-9: New USB device found, idVendor=28bd, idProduct=095b,
> > bcdDevice= 0.00
> > usb 3-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> > usb 3-9: Product: Artist Pro 16 (Gen2)
> > usb 3-9: Manufacturer: UGTABLET
> > usb 3-9: SerialNumber: 00000
> > input: UGTABLET Artist Pro 16 (Gen2) Mouse as
> > /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> > 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input196
> > input: UGTABLET Artist Pro 16 (Gen2) Keyboard as
> > /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> > 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input197
> > hid-generic 0003:28BD:095B.009E: input,hidraw4: USB HID v1.00 Mouse
> > [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input0
> > input: UGTABLET Artist Pro 16 (Gen2) as
> > /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> > 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.1/0003:28BD:095B.009F/input/input198
> > hid-generic 0003:28BD:095B.009F: input,hidraw7: USB HID v1.00 Device
> > [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input1
> > hid-generic 0003:28BD:095B.00A0: hiddev0,hidraw12: USB HID v1.00 Device
> > [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input2
> > -----8<-----8<-----8<-----8<
> >
> > This make "sudo xinput" reports 3 new devices :
> > UGTABLET Artist Pro 16 (Gen2) Mouse id=x [slave pointer (n)]
> > UGTABLET Artist Pro 16 (Gen2) Keyboard id=y [slave keyboard (n+1)]
> > UGTABLET Artist Pro 16 (Gen2) id=z [slave keyboard (n+1)]
> > (sorry I've masked the id to prevent confusion, I did not run "xinput"
> > at the same time/power cycle than "dmesg")
> >
> > Running the Qt App makes "sudo xinput" report 4 new devices :
> > XP-Pen Mouse id=xc [slave pointer (n)]
> > XP-Pen Eraser id=xa [slave keyboard (n+1)]
> > XP-Pen Pen id=xb [slave keyboard (n+1)] << only this chmod'ed 0666
> > XP-Pen Mouse id=xd [slave keyboard (n+1)]
> >
> > And... it suffices to make Blender works without configuring anything.
> >
> > Anyway, besides this user experience / userland Qt app, here are some
> > five hid-recorder traces ;
> 
> Many thanks for attaching them and for digging into them.
> 
> > I re-used the terminology appearing in the traces :
> > - "barrel" : the "bottom" button nearest of the main tip
> > - "erase" : the farther "top" button, just above "barrel"
> > - "back" : the "rubber" secondary pressure tip at the back of the pen
> >
> > (1) hidraw7_inRange_contact_move_lift_outOfRange
> > I "contact" the main tip, move it, and lift it out or range.
> >
> > (2) hidraw7_inRange_contact_barrelPress\
> > _move_barrelRelease_lift_outOfRange
> > I "contact" the main tip, press "barrel" (which is the button nearest
> > of the tip), move the pen, release the button, move it out of range.
> >
> > (3) hidraw7_inRange_contact_erasePress\
> > _move_eraseRelease_lift_outOfRange
> > The same than above, except that I instead use the "erase" button,
> > i.e. the button just above ("vertically speaking") of the "main" button.
> >
> > (4) hidraw7_inRange_contact_BOTHbarrelAnderasePress_move_\
> > BOTHbarrelAnderaseRelease_lift_outOfRange
> > That's just plain stupid, and uncomfortable to do, but it's just to
> > show an example of the capability of the hardware ;
> > In this one, I'm doing the same than above, except I stupidly press BOTH
> > buttons.
> > It means that I "contact" the main tip, press both barrel+erase, move
> > the pen, release both button, lift it out of range.
> >
> > (5) hidraw7_inRange_contact_backPress_move_backRelease_lift_outOfRange
> > Back to non-stupid tests, here I am using the "rubber" (which has
> > pressure !), i.e. I use the back of the pen.
> > I contact it, press the rubber to have some pressure, move it, release
> > the pressure, move the pen ouf of contact and out of range.
> >
> > Reading those traces, I think I observed some (to me) very logical
> > behavior ;
> >
> > I agree that the Pen is possibly not acting in respect of the
> > specification, by the fact that... the spec does not differentiate the
> > "eraser" 2nd button, versus the "backside" rubber pressure tip.
> >
> > The traces shows that :
> > - In range [0 - 1] is correctly reported, both for main tip + rubber tip
> > - Tip Pressure [0 - n] is correctly reported, for both pressure tips
> 
> Yep, that part is correct and follows the specification
> 
> >
> > - Tip Switch [0 - 1] is correctly set to ONE for all traces, when the
> >   in-range tip goes to contact. (either main tip or rubber tip)
> 
> This is half wrong (spec-wise):
> - tip switch should only be used when the actual tip is in contact.
> - when using the rubber tip, we should get an eraser event :(
> 
> >
> > - Barrel Switch [0 - 1] is correctly set to ONE only when pressing
> >   button down, and of course only when using the main tip
> 
> This is correct (as in it follows the spec)
> 
> >
> > - Invert [0 - 1] is set to ONE when (and only) using the back rubber
> >   tip, and the ERASE button (2nd button) does NOT set it, so "INVERT"
> 
> In your case, the invert usage is correct.
> In Illia's case, the invert usage is forwarded by the eraser usage, so
> this is just plain wrong there.
> 
> >
> > - Eraser [0 - 1] is set to ONE ONLY when pressing the 2nd top button,
> >   and of :
> >   - ONLY when using the main itp
> >   - and if you were to instead use the "back" of the pen (rubber tip),
> >     then you will have invert=1 + eraser=0
> 
> This is not correct (again, according to the specification). XP-Pen
> should report SecondaryBarrelSwitch (0x5a in the HID usage table[0])
> And again, in Illia's case, this button should be reported as an
> invert, and eraser when the tip touches the tablet.
> 
> >
> > The main extract of those is, I think, that :
> > - eraser "top" button is completely independent of the back "rubber"
> 
> Yes
> 
> > - eraser "top" button NEVER concerns itself to set invert=1
> 
> yes
> 
> > - backside "rubber" pressure tip will set invert=1, but leave eraser=0
> 
> yes, but that's not correct, it should report eraser=1 when you touch
> the rubber on the surface.
> 
> >
> > So, the behavior probably breaks the specs, but sincerely I'm happy to
> > have the "eraser" button independent of the "rubber eraser", which
> > makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> > rubber), and I would like to keep this.
> 
> Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
> apparently broke it.
> 
> So, to me:
> - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> firmware bug (reporting invert through eraser) and should not be
> tackled at the generic level, thus it should be reverted
> - both of these tablets are forwarding the useful information, but not
> correctly, which confuses the kernel
> - I should now be able to write regression tests
> - I should be able to provide HID-BPF fixes for those tablets so that
> we can keep them working with or without
> 276e14e6c3993317257e1787e93b7166fbc30905
> reverted (hopefully)
> - problem is I still don't have the mechanics to integrate the HID-BPF
> fixes directly in the kernel tree, so maybe I'll have to write a
> driver for XP-Pen while these internals are set (it shouldn't
> interfere with the HID-BPF out of the tree).

I already added support for a few XP-Pen devices on the UCLogic driver
and I was planning to start working on this one during the weekend in
my DIGImend fork (to simplify testing).

Let me know if you prefer to add it yourself or if you want me to ping
you in the DIGImend discussion.

José Expósito

> 
> Cheers,
> Benjamin
> 
> >
> > Best Regards,
> >
> > [1] https://www.xp-pen.fr/download-1027.html ; the .deb Qt app
> >
> > --
> > Eric GOUYER
> >
> 
> [0] https://usb.org/document-library/hid-usage-tables-14
> 

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08  9:23                 ` José Expósito
@ 2023-11-08  9:34                   ` Benjamin Tissoires
  2023-11-08 18:21                     ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-08  9:34 UTC (permalink / raw)
  To: José Expósito
  Cc: Eric GOUYER, Illia Ostapyshyn, David Revoy, jkosina,
	jason.gerecke, linux-input, linux-kernel

On Wed, Nov 8, 2023 at 10:23 AM José Expósito <jose.exposito89@gmail.com> wrote:
>
> Hi Benjamin,
>
> On Wed, Nov 08, 2023 at 10:04:30AM +0100, Benjamin Tissoires wrote:
[...]
> >
> > >
> > > So, the behavior probably breaks the specs, but sincerely I'm happy to
> > > have the "eraser" button independent of the "rubber eraser", which
> > > makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> > > rubber), and I would like to keep this.
> >
> > Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
> > apparently broke it.
> >
> > So, to me:
> > - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> > firmware bug (reporting invert through eraser) and should not be
> > tackled at the generic level, thus it should be reverted
> > - both of these tablets are forwarding the useful information, but not
> > correctly, which confuses the kernel
> > - I should now be able to write regression tests
> > - I should be able to provide HID-BPF fixes for those tablets so that
> > we can keep them working with or without
> > 276e14e6c3993317257e1787e93b7166fbc30905
> > reverted (hopefully)
> > - problem is I still don't have the mechanics to integrate the HID-BPF
> > fixes directly in the kernel tree, so maybe I'll have to write a
> > driver for XP-Pen while these internals are set (it shouldn't
> > interfere with the HID-BPF out of the tree).
>
> I already added support for a few XP-Pen devices on the UCLogic driver
> and I was planning to start working on this one during the weekend in
> my DIGImend fork (to simplify testing).
>
> Let me know if you prefer to add it yourself or if you want me to ping
> you in the DIGImend discussion.
>

So far, I really have to work on this now. It's a good use case for
HID-BPF and it's a regression that I'd like to be fixed ASAP.
I'd appreciate any reviews :)

Also, good to know that I can probably piggyback on hid-uclogic for
fixing those 2 devices in the kernel.

Cheers,
Benjamin


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08  9:34                   ` Benjamin Tissoires
@ 2023-11-08 18:21                     ` Benjamin Tissoires
  2023-11-09  0:32                       ` David Revoy
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-08 18:21 UTC (permalink / raw)
  To: José Expósito
  Cc: Eric GOUYER, Illia Ostapyshyn, David Revoy, jkosina,
	jason.gerecke, linux-input, linux-kernel

On Wed, Nov 8, 2023 at 10:34 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Wed, Nov 8, 2023 at 10:23 AM José Expósito <jose.exposito89@gmail.com> wrote:
> >
> > Hi Benjamin,
> >
> > On Wed, Nov 08, 2023 at 10:04:30AM +0100, Benjamin Tissoires wrote:
> [...]
> > >
> > > >
> > > > So, the behavior probably breaks the specs, but sincerely I'm happy to
> > > > have the "eraser" button independent of the "rubber eraser", which
> > > > makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> > > > rubber), and I would like to keep this.
> > >
> > > Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
> > > apparently broke it.
> > >
> > > So, to me:
> > > - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> > > firmware bug (reporting invert through eraser) and should not be
> > > tackled at the generic level, thus it should be reverted
> > > - both of these tablets are forwarding the useful information, but not
> > > correctly, which confuses the kernel
> > > - I should now be able to write regression tests
> > > - I should be able to provide HID-BPF fixes for those tablets so that
> > > we can keep them working with or without
> > > 276e14e6c3993317257e1787e93b7166fbc30905
> > > reverted (hopefully)
> > > - problem is I still don't have the mechanics to integrate the HID-BPF
> > > fixes directly in the kernel tree, so maybe I'll have to write a
> > > driver for XP-Pen while these internals are set (it shouldn't
> > > interfere with the HID-BPF out of the tree).
> >
> > I already added support for a few XP-Pen devices on the UCLogic driver
> > and I was planning to start working on this one during the weekend in
> > my DIGImend fork (to simplify testing).
> >
> > Let me know if you prefer to add it yourself or if you want me to ping
> > you in the DIGImend discussion.
> >
>
> So far, I really have to work on this now. It's a good use case for
> HID-BPF and it's a regression that I'd like to be fixed ASAP.
> I'd appreciate any reviews :)

Alright, I made quite some progress so far:
- regressions tests have been written (branch wip/xp-pen of my fork on
freedesktop[0])
  that branch can not go in directly as it just adds the tests, and
thus is failing
- I made the fixes through HID-BPF[1]

Anyone using those 2 tablets and using Fedora should be able to just
grab the artifact at [2], uncompress it and run `sudo ./install.sh
--verbose`.
This will install the bpf programs in /lib/firmware/hid/bpf/ and will
automatically load them when the device is connected.

For those not using Fedora, the binary might work (or not, not sure),
but you can always decompress it, and check if running
`udev-hid-bpf_0.1.0/bin/udev-hid-bpf --version` returns the correct
version or just fails. If you get "udev-hid-bpf 0.1.0", then running
`sudo ./install.sh --verbose` should work, as long as the kernel has
CONFIG_HID_BPF set to 'Y'.

>
> Also, good to know that I can probably piggyback on hid-uclogic for
> fixing those 2 devices in the kernel.
>

Next step will be to fix them using a kernel driver, but it seems that
the uclogic driver is doing more than just report descriptor fixups,
so maybe I'll have to use a different driver.
But the point is, in theory, if you are affected by those bugs, using
the udev-hid-bpf should fix the issue, and this should also be
resilient to further kernel updates.

I'd appreciate testing when I got the full kernel series up and ready,
of course.

Cheers,
Benjamin

[0] https://gitlab.freedesktop.org/bentiss/hid/-/tree/wip/xp-pen?ref_type=heads
[1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
[2] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/jobs/51350589/artifacts/raw/udev-hid-bpf_0.1.0.tar.xz


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08  9:04               ` Benjamin Tissoires
  2023-11-08  9:23                 ` José Expósito
@ 2023-11-08 19:40                 ` Nils Fuhler
  2023-11-08 20:34                   ` Benjamin Tissoires
  1 sibling, 1 reply; 40+ messages in thread
From: Nils Fuhler @ 2023-11-08 19:40 UTC (permalink / raw)
  To: benjamin.tissoires
  Cc: davidrevoy, folays, jason.gerecke, jkosina, jose.exposito89,
	linux-input, linux-kernel, ostapyshyn, nils

Benjamin Tissoires <benjamin.tissoires@redhat.com> writes:

> So, to me:
> - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> firmware bug (reporting invert through eraser) and should not be
> tackled at the generic level, thus it should be reverted

Surely that can't be the solution.  [1] is not a specification but a
suggestion that many manufacturers seem to follow.  Apparently, there
are devices that directly report "erasing" for the upper button,
skipping the whole "intent to erase" business.  A questionable decision,
but clearly intended.

The evdev input protocol represents the erasing action as BTN_TOUCH with
BTN_TOOL_RUBBER being active.  Previously, it was assumed that there is
an Invert usage that would toggle BTN_TOOL_RUBBER.  XP-Pen Artist 24
(and possibly other devices) does not have Invert in its report
descriptor, resulting in missing BTN_TOOL_RUBBER.  BTN_TOUCH without an
active tool has no meaning in the input stream.

276e14e6c3 adds a quirk for this and sends the required BTN_TOOL_RUBBER
events *only* for styli not doing it themselves via Invert.  In fact,
276e14e6c3 does not even affect the "two-eraser" XP-Pen Artist Pro 16
Gen 2 and all other devices having Invert in their report descriptors.
The quirk is not set, the behavior is unchanged [2].

As Illia already described, the *real problem* is the missing
compatibility of the whole hidinput_hid_event state machine with
hidinput_setkeycode, invoked from userspace via the EVIOCSKEYCODE ioctl.
In David's case, this is done by hwdb.

This has been the case at least since the refactoring and even affects
styli *completely* adhering to [1]: remapping Invert to something other
than BTN_TOOL_RUBBER borks the device.  If you replay the recording [3]
(with or without 276e14e6c3) and remap [4] 0xd003c (Invert) to something
other than BTN_TOOL_RUBBER, you can observe that:

(1) Remapped Invert does not trigger the event it was remapped to -- this
    is due to hardcoded BTN_TOOL_RUBBER and BTN_TOUCH in hidinput_hid_event

(2) After triggering Invert once, BTN_TOOL_PEN and BTN_TOUCH never appear
    in the event stream again -- remapping Invert masks BTN_TOOL_RUBBER,
    causing it to get permanently stuck in report->tool.

276e14e6c3 does extend this issue onto Eraser remappings for devices
without Invert, resulting in this regression.  However, the solution is
to fix 276e14e6c3 (and the whole function, while we're at it), not to
revert it.

> - both of these tablets are forwarding the useful information, but not
> correctly, which confuses the kernel
> - I should now be able to write regression tests
> - I should be able to provide HID-BPF fixes for those tablets so that
> we can keep them working with or without
> 276e14e6c3993317257e1787e93b7166fbc30905
> reverted (hopefully)
> - problem is I still don't have the mechanics to integrate the HID-BPF
> fixes directly in the kernel tree, so maybe I'll have to write a
> driver for XP-Pen while these internals are set (it shouldn't
> interfere with the HID-BPF out of the tree).

As you can see, there is no need for rewriting anything.  The generic
solution for "invertless" digitizers is already in place and has no
impact on other devices.  IMHO, it would be wiser to fix the regression
by making hidinput_hid_event compatible with EVIOCSKEYCODE, than to miss
the point of what is actually broken and write device-specific drivers.

[1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states

[2] David confirms it in his blog post: "I now know it is there for a
    long time, because even with the "old" kernel, my newer XPPen 16 Pro
    (gen2) also reacts this way."
    https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help

[3] https://dl.uni-h.de/?t=6b2cabd8f15ac8ff281b52e25920c0a9

[4] https://github.com/iostapyshyn/evmap is a handy EVIOCSKEYCODE wrapper
    for remapping.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08 19:40                 ` Nils Fuhler
@ 2023-11-08 20:34                   ` Benjamin Tissoires
  2023-11-08 22:31                     ` ostapyshyn
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-08 20:34 UTC (permalink / raw)
  To: Nils Fuhler
  Cc: davidrevoy, folays, jason.gerecke, jkosina, jose.exposito89,
	linux-input, linux-kernel, ostapyshyn

On Wed, Nov 8, 2023 at 8:41 PM Nils Fuhler <nils@nilsfuhler.de> wrote:
>
> Benjamin Tissoires <benjamin.tissoires@redhat.com> writes:
>
> > So, to me:
> > - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> > firmware bug (reporting invert through eraser) and should not be
> > tackled at the generic level, thus it should be reverted
>
> Surely that can't be the solution.  [1] is not a specification but a
> suggestion that many manufacturers seem to follow.  Apparently, there
> are devices that directly report "erasing" for the upper button,
> skipping the whole "intent to erase" business.  A questionable decision,
> but clearly intended.

How many of such devices do we have? Are they all UGTablet like the
XP-PEN? Are they behaving properly on Windows without a proprietary
driver?
We can try to make the code work based on suppositions, but this is
the first time I see such a behavior, so I'd prefer fix it when I see
it rather than adding complexity in the driver where changing anything
is hard. I'm writing regression tests for that exact same purpose.

>
> The evdev input protocol represents the erasing action as BTN_TOUCH with
> BTN_TOOL_RUBBER being active.  Previously, it was assumed that there is
> an Invert usage that would toggle BTN_TOOL_RUBBER.  XP-Pen Artist 24
> (and possibly other devices) does not have Invert in its report
> descriptor, resulting in missing BTN_TOOL_RUBBER.  BTN_TOUCH without an
> active tool has no meaning in the input stream.
>
> 276e14e6c3 adds a quirk for this and sends the required BTN_TOOL_RUBBER
> events *only* for styli not doing it themselves via Invert.  In fact,
> 276e14e6c3 does not even affect the "two-eraser" XP-Pen Artist Pro 16
> Gen 2 and all other devices having Invert in their report descriptors.
> The quirk is not set, the behavior is unchanged [2].

Yes. That quirk only affects the device clearly not following the
specification. Which is why it wasn't noticed that it was wrong.

>
> As Illia already described, the *real problem* is the missing
> compatibility of the whole hidinput_hid_event state machine with
> hidinput_setkeycode, invoked from userspace via the EVIOCSKEYCODE ioctl.
> In David's case, this is done by hwdb.

Sorry but I tend to disagree. Relying on the ioctl EVIOCSKEYCODE for
tuning the behavior of a state machine is just plain wrong. When
people do that, they are doing it at the wrong level and introducing
further bugs.

The whole pen and touch HID protocols rely on a state machine. You
just can not change the meaning of it because your hardware maker
produced a faulty hardware.
The correct solution is to submit a fix here, so that everyone gets
the benefit of it. But that fix can now be a HID-BPF program, which
will be eventually integrated in the kernel tree as soon as I manage
to get enough time to work on it.

>
> This has been the case at least since the refactoring and even affects
> styli *completely* adhering to [1]: remapping Invert to something other
> than BTN_TOOL_RUBBER borks the device.  If you replay the recording [3]
> (with or without 276e14e6c3) and remap [4] 0xd003c (Invert) to something
> other than BTN_TOOL_RUBBER, you can observe that:

In the same way, if you remap Tip Switch to KEY-A, you won't get
clicks from your pen. Assuming you can do that with any event on any
HID device is just plain wrong.
That ioctl is OK-ish for "remapping" simple things like keys. In our
case, the whole firmware follows a state machine, so we can not change
it. It has to be remapped in a later layer, in libinput, your
compositor, or in your end user application.

>
> (1) Remapped Invert does not trigger the event it was remapped to -- this
>     is due to hardcoded BTN_TOOL_RUBBER and BTN_TOUCH in hidinput_hid_event
>
> (2) After triggering Invert once, BTN_TOOL_PEN and BTN_TOUCH never appear
>     in the event stream again -- remapping Invert masks BTN_TOOL_RUBBER,
>     causing it to get permanently stuck in report->tool.
>
> 276e14e6c3 does extend this issue onto Eraser remappings for devices
> without Invert, resulting in this regression.  However, the solution is
> to fix 276e14e6c3 (and the whole function, while we're at it), not to
> revert it.

Again, you convinced me that this commit was wrong. If people needs to
also use an ioctl to "fix" it, then there is something wrong.

>
> > - both of these tablets are forwarding the useful information, but not
> > correctly, which confuses the kernel
> > - I should now be able to write regression tests
> > - I should be able to provide HID-BPF fixes for those tablets so that
> > we can keep them working with or without
> > 276e14e6c3993317257e1787e93b7166fbc30905
> > reverted (hopefully)
> > - problem is I still don't have the mechanics to integrate the HID-BPF
> > fixes directly in the kernel tree, so maybe I'll have to write a
> > driver for XP-Pen while these internals are set (it shouldn't
> > interfere with the HID-BPF out of the tree).
>
> As you can see, there is no need for rewriting anything.  The generic
> solution for "invertless" digitizers is already in place and has no
> impact on other devices.  IMHO, it would be wiser to fix the regression
> by making hidinput_hid_event compatible with EVIOCSKEYCODE, than to miss
> the point of what is actually broken and write device-specific drivers.

EVIOCSKEYCODE is called by userspace *after* the kernel parsed the
device and is ready to accept events. There is no way we can make this
consistent with the event stream. If you need to call that ioctl to
fix your device, it's a bug in the kernel, and it needs to be fixed
there before being presented to the userspace.

I might buy the "invertless" devices are a thing if I can get more
data about it. So far there are only 2 of them, and they add extra
complexity in the code when we can just patch the devices to do the
right thing.

>
> [1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
>
> [2] David confirms it in his blog post: "I now know it is there for a
>     long time, because even with the "old" kernel, my newer XPPen 16 Pro
>     (gen2) also reacts this way."
>     https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help

New hardware isn't supposed to be supported on an old kernel and is
not considered as a regression. However, David mentioned that the
device was "working" on 6.5.0 but broke in 6.5.8 with the patch
mentioned above. This is a regression that needs to be tackled.
Especially because it was introduced in 6.6 but backported into 6.5.

Cheers,
Benjamin

>
> [3] https://dl.uni-h.de/?t=6b2cabd8f15ac8ff281b52e25920c0a9
>
> [4] https://github.com/iostapyshyn/evmap is a handy EVIOCSKEYCODE wrapper
>     for remapping.
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-06 16:59       ` Benjamin Tissoires
  2023-11-06 20:06         ` Illia Ostapyshyn
@ 2023-11-08 22:28         ` David Revoy
  1 sibling, 0 replies; 40+ messages in thread
From: David Revoy @ 2023-11-08 22:28 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Illia Ostapyshyn, jkosina, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

Hi Benjamin,

Thanks for all the information.

> Could you please use sudo hid-recorder from hid-tools[1] on any kernel
> version and send us the logs here?
> I'll be able to replay the events locally, and understand why the
> kernel doesn't work properly.
> [1] https://gitlab.freedesktop.org/libevdev/hid-tools

Yes, I can. I've done it for the two tablets and you can find the file output hosted here:
https://www.peppercarrot.com/extras/?dir=mailing-list/hid-records

I followed the diagnostic method (for gestures) and file naming suggested in the short videos on this page:
https://digimend.github.io/support/howto/trbl/diagnostics/

I hope that it will be useful, even after the detailed feedback from Eric Gouyer (folays@gmail.com) on the ML (thank you).

> Generally speaking, relying on X to fix your hardware is going to be a
> dead end. When you switch to wayland, you'll lose all of your fixes,
> which isn't great.

You are right. I hope that the Wayland session will offer possible command line tools to customise devices. But that's another topic. :)


On Monday, November 6th, 2023 at 17:59, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:


> Hi David,
> 
> On Mon, Nov 6, 2023 at 2:18 PM David Revoy davidrevoy@protonmail.com wrote:
> 
> > Hi Illia, Jiri, Bagas,
> > 
> > Thanks to Jiri for forwarding my request for help to this mailing list.
> > 
> > I apologise in advance to Bagas and you all for not being able to properly configure my email client to follow the correct etiquette (Protonmail, unsuitable for kernel development, but we've made some progress with them on Mastodon on the encryption issue 1).
> 
> 
> Hopefully you'll be able to figure out a way to have your emails
> posted to the lkml soon. It's very valuable to have them in
> lore.kernel.org to get the thread context.
> 
> > Thanks to Illia for the detailed explanation. Thanks also for fixing it, and for explaining how the fix broke my workflow, and for your kind words about the situation. I appreciate it.
> > 
> > However, after reading your reply, I still have my doubts about the preference to put an eraser on the top button of the pen by default. But after a few days and re-reading your first sentence "The XP-Pen hardware reports the Eraser usage for the upper stylus button.", I think I understood it: this is an internal signal that is sent by the device itself, and is therefore a specification that has to be followed, right? In this case, it makes sense for a generic driver to follow such a signal if it is sent by the hardware.
> 
> 
> Yes, you are correct. The device talks a given protocol (HID) and is
> supposed to use the specification from Microsoft[0] on how to behave
> properly. As such, it has to convey the "eraser" state by adding
> dedicated information in the event stream.
> 
> In your case, I think we (the kernel and your stylus) simply talk past
> each other and we lose information.
> 
> > Having said that, behaving like this still makes no sense in one case: I'm thinking of my other display tablet, the XPPEN 16 Artist Pro (Gen2). In this case, the stylus has the top button as an eraser, and an eraser tip as well, as you can see in this photo 2. Was it also a decision by XPPEN to include this signal in the hardware, knowing that they already had an eraser tip? In this case, please let me know, as it sounds like a hardware problem at the design stage and I have probably a way of passing on the information to their technical team.
> 
> 
> If the pen has 2 buttons, and an eraser side, it would be a serious
> design flow for XPPEN to report both as eraser.
> 
> Could you please use sudo hid-recorder from hid-tools1 on any kernel
> version and send us the logs here?
> I'll be able to replay the events locally, and understand why the
> kernel doesn't work properly.
> 
> And if there is a design flaw that can be fixed, we might even be able
> to use hid-bpf to change it :)
> 
> > > You can still remap the eraser button to a right click using xsetwacom:
> > > xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
> > 
> > Unfortunately, it doesn't work.
> > 
> > Firstly, such tablets are often connected to a computer that already has a display. So each device (pen/eraser) needs to be mapped to the correct display and set to the correct 'Area' for calibration. A better syntax in my case to test your workaround is written like this:
> > 
> > tableteraser="UGTABLET 24 inch PenDisplay eraser"
> > xsetwacom set "$tableteraser" MapToOutput "DisplayPort-2"
> > xsetwacom set "$tableteraser" Area 100 120 32794 32797
> > xsetwacom set "$tableteraser" "Button" "1" "3"
> > 
> > Secondly, forwarding the eraser button 1 to button 3 (a right click) in xsetwacom doesn't work.
> > 
> > Pressing the button switches the device to an eraser and no right click happens. You'll need to hold down the button and click with the tip of the pen to create a right-click event.
> > 
> > In a digital painting session in Krita, the software will switch your device to an eraser tool preset while you hold down the button, and the feedback on the canvas will be the eraser cursor. You'll then have to click "with the eraser" to get the right-click that triggers the pop-up palette 3.
> > 
> > I'd also like to mention that if you haven't correctly mapped and calibrated your eraser device with xsetwacom, as I did in the code snippet above, the cursor will by default jump to a different location when you hold down the eraser button. It can be on a different display.
> > 
> > Finally, in the case of the XPPEN 16 Artist Pro (Gen2) with a real eraser device (Photo, 2), the setting 'xsetwacom set "$tableteraser" "Button" "1" "3"' will affect both erasers. Flipping the stylus on the eraser side and entering in 'hover' mode will return a correct eraser, but as soon as you start erasing with the tip, a right click will also be entered.
> > 
> > > You can also do this with "xinput set-button-map", which works for libinput as well.
> > 
> > $ xinput list
> > ⎡ Virtual core pointer
> > ⎜ ↳ UGTABLET 24 inch PenDisplay Mouse id=8 [slave pointer (2)]
> > ⎜ ↳ UGTABLET 24 inch PenDisplay stylus id=10 [slave pointer (2)]
> > ⎜ ↳ UGTABLET 24 inch PenDisplay eraser id=17 [slave pointer (2)]
> > [...]
> > $ xinput get-button-map 17
> > 1 2 3 4 5 6 7 8
> > $ xinput set-button-map 17 3 3 3 3 3 3 3 3
> > $ xinput get-button-map 17
> > 3 3 3 3 3 3 3 3
> > 
> > Even after that, it doesn't work. My original article 4 mentions in the last paragraph that I have tested almost all methods, and this was one of them. Even a udev rule doesn't fix it. For reference, xinput produces the same behaviour as xsetwacom: you have to hold the button (it triggers an eraser device switch) then click with the tip to get a right-click...
> 
> 
> Generally speaking, relying on X to fix your hardware is going to be a
> dead end. When you switch to wayland, you'll lose all of your fixes,
> which isn't great.
> 
> > > We have tested this with several XP-Pen devices, including Artist 24.
> > 
> > Sorry if my tests show something different. Maybe there is something wrong with my GNU/Linux operating system (Fedora 38 KDE spin). Let me know if you have any idea why I get these results and guide me with what distro I should switch to get a similar observation than you do (and also to report the issue to the Fedora team).
> > 
> > ---
> > 
> > All in all (and in the case that my observations and tests are correct), I still stand by the points that I made in my blog post:
> > 
> > - I don't have any way to customise the hardcoded eraser buttons in userspace and it is a problem: for devices without a touchscreen or mouse, not having access to a right-click by default on the device is a handicap. Many actions on the D.E. and applications use the right click. The preference to replace it with an 'eraser switch' action by default is IMHO highly debatable here.
> 
> 
> AFAIU, the kernel now "merges" both buttons, which is a problem. It
> seems to be a serious regression. This case is also worrying because I
> added regression tests on hid, but I don't have access to all of the
> various tablets, so I implemented them from the Microsoft
> specification[0]. We need a special case for you here.
> 
> > - In the case of a pen that already has an eraser tip on the other side of the device 2, it makes no sense to exchange the right-click top button for another way to erase. Also in userspace, there seems to be no way to distinguish between the two buttons (the eraser tip and the eraser button). All actions from xsetwacom, xinput/libinput target the two eraser devices, and they target them unsuccessfully too.
> 
> 
> Again, we can't do more than what the device reports. Well, we can
> always quirk it in some cases, but ideally we don't have to do
> anything. MS spec [0], only shows a single button pen with an eraser
> tail or a pen with 2 buttons, one of them being the eraser one. I'm
> not sure if they decided to prevent dual button pens with eraser tail,
> but Wacom definitely has some, and they work.
> 
> Without the actual data from your pen, I can not tell much, because I
> don't follow which path we are taking on the kernel side. Please
> report with your hid-recorder logs, so I can understand why this is
> happening and how we can fix it.
> 
> > ------- Original Message -------
> > On Friday, November 3rd, 2023 at 21:05, Illia Ostapyshyn ostapyshyn@sra.uni-hannover.de wrote:
> > 
> > > Hello David, Hello Jiri,
> > > 
> > > The XP-Pen hardware reports the Eraser usage for the upper stylus button.
> > > Generally, styli report Invert usages when erasing, as described in 1.
> > > XP-Pen digitizers, however, tend to omit them.
> > > 
> > > The generic driver maps the Eraser usage to BTN_TOUCH and the Invert
> > > usage to BTN_TOOL_RUBBER. Pens conforming to 1 send the Invert usage
> > > first (switching the tool to BTN_TOOL_RUBBER) followed by Eraser, which
> > > appears in userspace as a BTN_TOUCH event with the rubber tool set.
> > > 
> > > Due to an oversight, devices not reporting Invert had the BTN_TOOL_RUBBER
> > > event masked. This has caused the kernel to send only BTN_TOUCH events
> > > without the tool switch when erasing.
> > > 
> > > The situation got worse with refactoring done in 87562fcd1342. An eraser
> > > without Invert caused the hidinput_hid_event state machine to get stuck
> > > with BTN_TOOL_RUBBER internally (due to it being masked). For the
> > > userspace, this looked as if the pen was never hovering again, rendering
> > > it unusable until the next reset. 276e14e6c3 fixes this by adding
> > > support for digitizers that do not report Invert usages when erasing.
> 
> 
> AFAICT 276e14e6c3 already had the hid tests integrated at
> tools/testing/selftests/hid/tests/test_tablet.py
> 
> It would have been great to add the cases you are mentioning here,
> because there is a strong chance I'll break things once again when we
> try to fix that regression.
> 
> > > ---
> > > 
> > > David, we are sorry that our patch broke your workflow. However,
> > > forwarding hardware events as-is to the userspace has always been the
> > > intended behavior, with a kernel bug preventing it so far. You can still
> > > remap the eraser button to a right click using xsetwacom:
> > > 
> > > xsetwacom set "UGTABLET 24 inch PenDisplay eraser" "Button" "1" "3"
> > > 
> > > Replace the device name with the corresponding eraser device from
> > > "xsetwacom list devices". You can also do this with "xinput set-button-map",
> > > which works for libinput as well. We have tested this with several
> > > XP-Pen devices, including Artist 24.
> > > 
> > > 1 https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
> 
> 
> Cheers,
> Benjamin
> 
> [0] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
> 1 https://gitlab.freedesktop.org/libevdev/hid-tools

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08 20:34                   ` Benjamin Tissoires
@ 2023-11-08 22:31                     ` ostapyshyn
  2023-11-09 11:46                       ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: ostapyshyn @ 2023-11-08 22:31 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Nils Fuhler, davidrevoy, folays, jason.gerecke, jkosina,
	jose.exposito89, linux-input, linux-kernel, ostapyshyn

On 11/8/23 21:34, Benjamin Tissoires wrote:

> Again, you convinced me that this commit was wrong. If people needs to
> also use an ioctl to "fix" it, then there is something wrong.

I don't think we're on the same page here.  Nobody needs to use an ioctl
to fix 276e14e6c3.  Rather, the _exact opposite_: the bug reporter used
an ioctl to remap Eraser to BTN_STYLUS2.  It stopped working after
276e14e6c3 and broke his workflow.  He reported it as a regression,
starting this whole thread.

> Sorry but I tend to disagree. Relying on the ioctl EVIOCSKEYCODE for
> tuning the behavior of a state machine is just plain wrong. When
> people do that, they are doing it at the wrong level and introducing
> further bugs.
>
> The whole pen and touch HID protocols rely on a state machine. You
> just can not change the meaning of it because your hardware maker
> produced a faulty hardware.
>
> [...]
>
> In the same way, if you remap Tip Switch to KEY-A, you won't get
> clicks from your pen. Assuming you can do that with any event on any
> HID device is just plain wrong.
> That ioctl is OK-ish for "remapping" simple things like keys. In our
> case, the whole firmware follows a state machine, so we can not change
> it. It has to be remapped in a later layer, in libinput, your
> compositor, or in your end user application.

I don't disagree.  Forbidding EVIOCSKEYCODE ioctls for pen and touch HID
is a legitimate way to resolve this (an appealing one too -- accounting
for it in hidinput_hid_event might be a hellish task).

Should we forbid remapping Eraser too?  If your answer is yes, then we
can finish this conversation here and leave the code as it is now,
because __the regression__ is a user not being able to use an ioctl to
remap Eraser after 276e14e6c3.  Otherwise, if we do make an exemption for
David's Eraser, the fix is as simple as replacing BTN_TOUCH with
usage->code on line 1594 of hid-input.c.

> How many of such devices do we have? Are they all UGTablet like the
> XP-PEN? Are they behaving properly on Windows without a proprietary
> driver?
>
> [...]
>
> I might buy the "invertless" devices are a thing if I can get more
> data about it. So far there are only 2 of them, and they add extra
> complexity in the code when we can just patch the devices to do the
> right thing.

There might or might not be more devices like this in the wild.  It looks
like BarrelSwitch2 was added only 2013 [1], which is why so many styli
use Eraser for the second button.  Setting two bits for a single button
just to adhere to Microsoft's *recommendation* is nice for compatibility,
but I can imagine vendors taking a shortcut and omitting Invert
altogether.  The HID specification alone just lists the usages and says
nothing about how they relate to each other.

XP-Pen Artist 24 does work on Windows with the generic driver.  The
Eraser engages as soon as the button is pressed, without touching the
screen.

> New hardware isn't supposed to be supported on an old kernel and is
> not considered as a regression. However, David mentioned that the
> device was "working" on 6.5.0 but broke in 6.5.8 with the patch
> mentioned above. This is a regression that needs to be tackled.
> Especially because it was introduced in 6.6 but backported into 6.5.

To make sure we're talking about the same thing:

1. "Broke" in this context means that the ioctl remapping from Eraser to
   right-click stopped working.

2. XPPen 16 Pro Gen2 is a whole different topic, untouched by 276e14e6c3.

[1] https://www.usb.org/sites/default/files/hutrr46e.txt

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-07  7:59           ` Benjamin Tissoires
  2023-11-07 13:40             ` Illia Ostapyshyn
  2023-11-08  5:23             ` Eric GOUYER
@ 2023-11-08 23:18             ` David Revoy
  2023-11-09 11:47               ` Benjamin Tissoires
  2023-11-11 17:15               ` Thorsten Leemhuis
  2 siblings, 2 replies; 40+ messages in thread
From: David Revoy @ 2023-11-08 23:18 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Illia Ostapyshyn, jkosina, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

> BTW, David, were you able to do a revert of 276e14e6c3?

I'm sorry Benjamin: I did some research on how to build a kernel [1], on how to revert a commit (easy, I know a bit of Git), and started following it step by step. Result: I failed and concluded that it probably requires too much computer knowledge compared to what I can do now. I'm afraid I won't be able to build a custom kernel for testing.

[1] https://docs.fedoraproject.org/en-US/quick-docs/kernel-build-custom/#_building_a_vanilla_upstream_kernel


On Tuesday, November 7th, 2023 at 08:59, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:


> On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> ostapyshyn@sra.uni-hannover.de wrote:
> 
> > On 11/6/23 17:59, Benjamin Tissoires wrote:
> > 
> > > If the pen has 2 buttons, and an eraser side, it would be a serious
> > > design flow for XPPEN to report both as eraser.
> > > 
> > > Could you please use sudo hid-recorder from hid-tools[1] on any kernel
> > > version and send us the logs here?
> > > I'll be able to replay the events locally, and understand why the
> > > kernel doesn't work properly.
> > > 
> > > And if there is a design flaw that can be fixed, we might even be able
> > > to use hid-bpf to change it :)
> > 
> > My wild guess is that XP-Pen 16 Artist Pro reports an Eraser usage
> > without Invert for the upper button and Eraser with Invert for the
> > eraser tip. A device-specific driver could work with that, but there
> > seems to be no way to incorporate two different erasers (thus, allowing
> > userspace to map them to different actions arbitrarily) in the generic
> > driver currently.
> 
> 
> That's exactly why I want to see the exact event flow. We can not do
> "wild guesses" unfortunately (not meaning any offenses).
> And I am very suspicious about the fact that the stylus reports 2
> identical erasers. Because in the past David seemed to be able to have
> 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> tail).
> 
> > > Generally speaking, relying on X to fix your hardware is going to be a
> > > dead end. When you switch to wayland, you'll lose all of your fixes,
> > > which isn't great.
> > 
> > > AFAIU, the kernel now "merges" both buttons, which is a problem. It
> > > seems to be a serious regression. This case is also worrying because I
> > > added regression tests on hid, but I don't have access to all of the
> > > various tablets, so I implemented them from the Microsoft
> > > specification[0]. We need a special case for you here.
> > 
> > The issue preventing David from mapping HID_DG_ERASER to BTN_STYLUS2 is
> > that the hidinput_hid_event is not compatible with hidinput_setkeycode.
> > If usage->code is no longer BTN_TOUCH after remapping, it won't be
> > released when Eraser reports 0. A simple fix is:
> 
> 
> I must confess, being the one who refactored everything, I still don't
> believe this is as simple as it may seem. I paged out all of the
> special cases, and now, without seeing the event flow I just can not
> understand why this would fix the situation.
> 
> And BTW, if you have a tool affected by 276e14e6c3, I'd be curious to
> get a hid-recorder sample for it so I can get regression tests for it.
> 
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -1589,7 +1589,7 @@ void hidinput_hid_event(struct hid_device *hid,
> > struct hid_field field, struct
> > / value is off, tool is not rubber, ignore */
> > return;
> > else if (*quirks & HID_QUIRK_NOINVERT &&
> > - !test_bit(BTN_TOUCH, input->key)) {
> > + !test_bit(usage->code, input->key)) {
> 
> 
> I don't want to be rude, but this feels very much like black magic,
> especially because there is a comment just below and it is not
> updated. So either the explanation was wrong, or it's not explaining
> the situation (I also understand that this is not a formal submission,
> so maybe that's the reason why the comment is not updated).
> 
> > /*
> > * There is no invert to release the tool, let hid_input
> > * send BTN_TOUCH with scancode and release the tool after.
> > 
> > This change alone fixes David's problem and the right-click mapping in
> > hwdb works again. However, the tool switches to rubber for the remapped
> > eraser (here BTN_STYLUS2) events, both for devices with and without
> > Invert. This does no harm but is not useful either. A cleaner solution
> > for devices without Invert would be to omit the whole tool switching
> > logic in this case:
> > 
> > @@ -1577,6 +1577,9 @@ void hidinput_hid_event(struct hid_device *hid,
> > struct hid_field *field, struct
> > 
> > switch (usage->hid) {
> > case HID_DG_ERASER:
> > + if (*quirks & HID_QUIRK_NOINVERT && usage->code != BTN_TOUCH)
> > + break;
> > +
> > report->tool_active |= !!value;
> > 
> > Remapping Invert does not work anyway as the Invert tool is hardcoded in
> > hidinput_hid_event. Even worse, I guess (not tested) trying to do so
> > would mask BTN_TOOL_RUBBER from dev->keybit and could cause weird
> > behavior similar to one between 87562fcd1342 and 276e14e6c3. This
> > raises the question: should users be able to remap Invert after all?
> 
> 
> The kernel is supposed to transfer what the device is. So if it says
> this is an eraser, we should not try to change it. Users can then
> tweak their own device if they wish through hid-bpf or through
> libinput quirks, but when you install a fresh kernel without tweaks,
> we should be as accurate as possible.
> 
> My main concern is that now we have a device which exports 2 different
> interactions as being the same. So either the firmware is wrong, and
> we need to quirk it, or the kernel is wrong and merges both, and this
> needs fixes as well.
> 
> Once every interaction on the device gets its own behavior, userspace
> can do whatever they want. It's not the kernel's concern anymore.
> 
> BTW, David, were you able to do a revert of 276e14e6c3?
> 
> Cheers,
> Benjamin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08 18:21                     ` Benjamin Tissoires
@ 2023-11-09  0:32                       ` David Revoy
  2023-11-09 11:56                         ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: David Revoy @ 2023-11-09  0:32 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi Benjamin,

> Alright, I made quite some progress so far:
> - regressions tests have been written (branch wip/xp-pen of my fork on
> freedesktop[0])
> that branch can not go in directly as it just adds the tests, and
> thus is failing
> - I made the fixes through HID-BPF[1]
> 
> Anyone using those 2 tablets and using Fedora should be able to just
> grab the artifact at [2], uncompress it and run `sudo ./install.sh --verbose`.
> This will install the bpf programs in /lib/firmware/hid/bpf/ and will
> automatically load them when the device is connected.
> 
> For those not using Fedora, the binary might work (or not, not sure),
> but you can always decompress it, and check if running
> `udev-hid-bpf_0.1.0/bin/udev-hid-bpf --version` returns the correct
> version or just fails. If you get "udev-hid-bpf 0.1.0", then running
> `sudo ./install.sh --verbose` should work, as long as the kernel has
> CONFIG_HID_BPF set to 'Y'.
> [...]
> [0] https://gitlab.freedesktop.org/bentiss/hid/-/tree/wip/xp-pen?ref_type=heads
> [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
> [2] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/jobs/51350589/artifacts/raw/udev-hid-bpf_0.1.0.tar.xz

Thank you for this package. 

I was able to test it even though the link in (2) at the bottom of your email returned a blank page. I was able to find my way after manually visiting gitlab.freedesktop.org [1] and then manually downloading the article from 51350589. I unzipped it and ran `sudo ./install.sh --verbose`. Everything looks like it was successful [2]. I then rebooted my Fedora 38 'Linux workstation 6.5.8-200.fc38.x86_64' kernel (the one I blamed in my post) and tested both tablets. 

Here are my observation:

XPPEN Artist Pro 24
===================
Nothing changed for this device (it's the one with two buttons and no 'eraser tip'). Nor my hwdb/udev rules or `xsetwacom set "UGTABLET 24 inch PenDisplay eraser" button 1 3` affects the upper button of the stylus: if I hold it hover the canvas, Krita switch the tool and cursor for an eraser. If I click on the canvas with the pen tip while holding the upper button pressed, I get the right-click Pop-up Palette (but not all the time, probably Krita has hard time to triage Eraser or Right-click).

XPPEN Artist Pro 16 (Gen2)
==========================
Something changed. `xsetwacom set "UGTABLET Artist Pro 16 (Gen2) eraser" button 1 3` successfully affected the upper button of the stylus. Now if I click it while hovering the canvas, Krita shows the right click Pop-up Palette.
On the downside; the real eraser tip when I flip the stylus bugs. When I flip the stylus on eraser hovering the canvas, Krita shows the Eraser icon and switch tool. As soon as I draw with the eraser tip, Krita will also show a right-click color palette and with also not a 100% consistency, as if the event were mixed.

[1] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/artifacts  
[2] https://www.peppercarrot.com/extras/temp/2023-11-09_screenshot_005214_net.jpg


On Wednesday, November 8th, 2023 at 19:21, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:


> On Wed, Nov 8, 2023 at 10:34 AM Benjamin Tissoires
> benjamin.tissoires@redhat.com wrote:
> 
> > On Wed, Nov 8, 2023 at 10:23 AM José Expósito jose.exposito89@gmail.com wrote:
> > 
> > > Hi Benjamin,
> > > 
> > > On Wed, Nov 08, 2023 at 10:04:30AM +0100, Benjamin Tissoires wrote:
> > > [...]
> > > 
> > > > > So, the behavior probably breaks the specs, but sincerely I'm happy to
> > > > > have the "eraser" button independent of the "rubber eraser", which
> > > > > makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> > > > > rubber), and I would like to keep this.
> > > > 
> > > > Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
> > > > apparently broke it.
> > > > 
> > > > So, to me:
> > > > - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> > > > firmware bug (reporting invert through eraser) and should not be
> > > > tackled at the generic level, thus it should be reverted
> > > > - both of these tablets are forwarding the useful information, but not
> > > > correctly, which confuses the kernel
> > > > - I should now be able to write regression tests
> > > > - I should be able to provide HID-BPF fixes for those tablets so that
> > > > we can keep them working with or without
> > > > 276e14e6c3993317257e1787e93b7166fbc30905
> > > > reverted (hopefully)
> > > > - problem is I still don't have the mechanics to integrate the HID-BPF
> > > > fixes directly in the kernel tree, so maybe I'll have to write a
> > > > driver for XP-Pen while these internals are set (it shouldn't
> > > > interfere with the HID-BPF out of the tree).
> > > 
> > > I already added support for a few XP-Pen devices on the UCLogic driver
> > > and I was planning to start working on this one during the weekend in
> > > my DIGImend fork (to simplify testing).
> > > 
> > > Let me know if you prefer to add it yourself or if you want me to ping
> > > you in the DIGImend discussion.
> > 
> > So far, I really have to work on this now. It's a good use case for
> > HID-BPF and it's a regression that I'd like to be fixed ASAP.
> > I'd appreciate any reviews :)
> 
> 
> Alright, I made quite some progress so far:
> - regressions tests have been written (branch wip/xp-pen of my fork on
> freedesktop[0])
> that branch can not go in directly as it just adds the tests, and
> thus is failing
> - I made the fixes through HID-BPF[1]
> 
> Anyone using those 2 tablets and using Fedora should be able to just
> grab the artifact at [2], uncompress it and run `sudo ./install.sh --verbose`.
> This will install the bpf programs in /lib/firmware/hid/bpf/ and will
> automatically load them when the device is connected.
> 
> For those not using Fedora, the binary might work (or not, not sure),
> but you can always decompress it, and check if running
> `udev-hid-bpf_0.1.0/bin/udev-hid-bpf --version` returns the correct
> version or just fails. If you get "udev-hid-bpf 0.1.0", then running
> `sudo ./install.sh --verbose` should work, as long as the kernel has
> CONFIG_HID_BPF set to 'Y'.
> 
> > Also, good to know that I can probably piggyback on hid-uclogic for
> > fixing those 2 devices in the kernel.
> 
> 
> Next step will be to fix them using a kernel driver, but it seems that
> the uclogic driver is doing more than just report descriptor fixups,
> so maybe I'll have to use a different driver.
> But the point is, in theory, if you are affected by those bugs, using
> the udev-hid-bpf should fix the issue, and this should also be
> resilient to further kernel updates.
> 
> I'd appreciate testing when I got the full kernel series up and ready,
> of course.
> 
> Cheers,
> Benjamin
> 
> [0] https://gitlab.freedesktop.org/bentiss/hid/-/tree/wip/xp-pen?ref_type=heads
> [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
> [2] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/jobs/51350589/artifacts/raw/udev-hid-bpf_0.1.0.tar.xz

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08 22:31                     ` ostapyshyn
@ 2023-11-09 11:46                       ` Benjamin Tissoires
  0 siblings, 0 replies; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-09 11:46 UTC (permalink / raw)
  To: ostapyshyn
  Cc: Nils Fuhler, davidrevoy, folays, jason.gerecke, jkosina,
	jose.exposito89, linux-input, linux-kernel

On Wed, Nov 8, 2023 at 11:32 PM <ostapyshyn@sra.uni-hannover.de> wrote:
>
> On 11/8/23 21:34, Benjamin Tissoires wrote:
>
> > Again, you convinced me that this commit was wrong. If people needs to
> > also use an ioctl to "fix" it, then there is something wrong.
>
> I don't think we're on the same page here.  Nobody needs to use an ioctl
> to fix 276e14e6c3.  Rather, the _exact opposite_: the bug reporter used
> an ioctl to remap Eraser to BTN_STYLUS2.  It stopped working after
> 276e14e6c3 and broke his workflow.  He reported it as a regression,
> starting this whole thread.

After more thoughts about Niels' email, the whole thread and a
not-so-good night's sleep, I think I now understand what is the
problem.

And yes, most of the problem comes from that remap *after* the kernel
parsed the device and made a decision based on what was provided to
it.

>
> > Sorry but I tend to disagree. Relying on the ioctl EVIOCSKEYCODE for
> > tuning the behavior of a state machine is just plain wrong. When
> > people do that, they are doing it at the wrong level and introducing
> > further bugs.
> >
> > The whole pen and touch HID protocols rely on a state machine. You
> > just can not change the meaning of it because your hardware maker
> > produced a faulty hardware.
> >
> > [...]
> >
> > In the same way, if you remap Tip Switch to KEY-A, you won't get
> > clicks from your pen. Assuming you can do that with any event on any
> > HID device is just plain wrong.
> > That ioctl is OK-ish for "remapping" simple things like keys. In our
> > case, the whole firmware follows a state machine, so we can not change
> > it. It has to be remapped in a later layer, in libinput, your
> > compositor, or in your end user application.
>
> I don't disagree.  Forbidding EVIOCSKEYCODE ioctls for pen and touch HID
> is a legitimate way to resolve this (an appealing one too -- accounting
> for it in hidinput_hid_event might be a hellish task).

I think it would be best not to require the need for the ioctl in the
first place.

Looking at David's blog, I'm starting to wonder if we actually need to
report BTN_TOOL_RUBBER after all in the case where there is no Invert
usage.

>
> Should we forbid remapping Eraser too?  If your answer is yes, then we
> can finish this conversation here and leave the code as it is now,
> because __the regression__ is a user not being able to use an ioctl to
> remap Eraser after 276e14e6c3.  Otherwise, if we do make an exemption for
> David's Eraser, the fix is as simple as replacing BTN_TOUCH with
> usage->code on line 1594 of hid-input.c.
>
> > How many of such devices do we have? Are they all UGTablet like the
> > XP-PEN? Are they behaving properly on Windows without a proprietary
> > driver?
> >
> > [...]
> >
> > I might buy the "invertless" devices are a thing if I can get more
> > data about it. So far there are only 2 of them, and they add extra
> > complexity in the code when we can just patch the devices to do the
> > right thing.
>
> There might or might not be more devices like this in the wild.  It looks
> like BarrelSwitch2 was added only 2013 [1], which is why so many styli
> use Eraser for the second button.  Setting two bits for a single button
> just to adhere to Microsoft's *recommendation* is nice for compatibility,
> but I can imagine vendors taking a shortcut and omitting Invert
> altogether.  The HID specification alone just lists the usages and says
> nothing about how they relate to each other.

Right. So maybe instead of trying to force the "no Invert" pens into
the "oh, this looks like an eraser", maybe we should remap in that
case the eraser usage into a secondary barrel switch.
We then need to filter the proximity out event that is sent when the
user presses it, but all in all it should be doable (hopefully).


>
> XP-Pen Artist 24 does work on Windows with the generic driver.  The
> Eraser engages as soon as the button is pressed, without touching the
> screen.

OK, thanks for the confirmation.

I just had a meeting with Peter Hutterer, and he told me it would be
best if the kernel doesn't follow the entire "this button is an eraser
mode". But that requires some filtering of the events because some
hardware (like the Artist 24 here) partially implements the
"specification" by sending a proximity out event when the button is
pressed.

So my idea would be to do that change in HID-BPF, so that it's only
included when libinput supports it (no regressions then), and we can
actually change the heuristics more easily than having to patch the
kernel.

I'd also need to get the behavior of:
- stylus is in range -> second button is pressed -> stylus touches the
surface with the button still pressed -> button is released -> stylus
leaves the surface and goes out of proximity.
- stylus is in range -> stylus touches the surface without any button
pressed -> second button is pressed -> stylus leaves the surface and
goes out of proximity
- stylus is in range -> stylus touches the surface without any button
pressed -> second button is pressed -> second button is released ->
stylus leaves the surface and goes out of proximity

And probably some other weird corner cases.

If we get the "eraser" event being set to 0/1 when the button is
pressed whether the stylus touches the surface or not, it would be
simple enough to change the purpose of the button in HID-BPF and
filter the eventual prox out events.


>
> > New hardware isn't supposed to be supported on an old kernel and is
> > not considered as a regression. However, David mentioned that the
> > device was "working" on 6.5.0 but broke in 6.5.8 with the patch
> > mentioned above. This is a regression that needs to be tackled.
> > Especially because it was introduced in 6.6 but backported into 6.5.
>
> To make sure we're talking about the same thing:
>
> 1. "Broke" in this context means that the ioctl remapping from Eraser to
>    right-click stopped working.

Yeah, you're correct. This isn't a regression, it's a user tempering
with the kernel and the kernel can't deal with it.
But the use case is still valid. It's the way it was done that was wrong.

>
> 2. XPPen 16 Pro Gen2 is a whole different topic, untouched by 276e14e6c3.

I still need to figure out what is wrong after my HID-BPF changes. But
yeah, it is orthogonal.

Cheers,
Benjamin

>
> [1] https://www.usb.org/sites/default/files/hutrr46e.txt
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08 23:18             ` David Revoy
@ 2023-11-09 11:47               ` Benjamin Tissoires
  2023-11-11 17:15               ` Thorsten Leemhuis
  1 sibling, 0 replies; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-09 11:47 UTC (permalink / raw)
  To: David Revoy
  Cc: Illia Ostapyshyn, jkosina, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

On Thu, Nov 9, 2023 at 12:19 AM David Revoy <davidrevoy@protonmail.com> wrote:
>
> > BTW, David, were you able to do a revert of 276e14e6c3?
>
> I'm sorry Benjamin: I did some research on how to build a kernel [1], on how to revert a commit (easy, I know a bit of Git), and started following it step by step. Result: I failed and concluded that it probably requires too much computer knowledge compared to what I can do now. I'm afraid I won't be able to build a custom kernel for testing.

No worries. And I'm actually happy, because you definitely fit into
the HID-BPF model where I want to fix a user's device without
requiring kernel compilation, and fixing the device in a reliable way
that we can do the general fix without impacting the reporter.

Cheers,
Benjamin

>
> [1] https://docs.fedoraproject.org/en-US/quick-docs/kernel-build-custom/#_building_a_vanilla_upstream_kernel
>
>
> On Tuesday, November 7th, 2023 at 08:59, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>
>
> > On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> > ostapyshyn@sra.uni-hannover.de wrote:
> >
> > > On 11/6/23 17:59, Benjamin Tissoires wrote:
> > >
> > > > If the pen has 2 buttons, and an eraser side, it would be a serious
> > > > design flow for XPPEN to report both as eraser.
> > > >
> > > > Could you please use sudo hid-recorder from hid-tools[1] on any kernel
> > > > version and send us the logs here?
> > > > I'll be able to replay the events locally, and understand why the
> > > > kernel doesn't work properly.
> > > >
> > > > And if there is a design flaw that can be fixed, we might even be able
> > > > to use hid-bpf to change it :)
> > >
> > > My wild guess is that XP-Pen 16 Artist Pro reports an Eraser usage
> > > without Invert for the upper button and Eraser with Invert for the
> > > eraser tip. A device-specific driver could work with that, but there
> > > seems to be no way to incorporate two different erasers (thus, allowing
> > > userspace to map them to different actions arbitrarily) in the generic
> > > driver currently.
> >
> >
> > That's exactly why I want to see the exact event flow. We can not do
> > "wild guesses" unfortunately (not meaning any offenses).
> > And I am very suspicious about the fact that the stylus reports 2
> > identical erasers. Because in the past David seemed to be able to have
> > 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> > tail).
> >
> > > > Generally speaking, relying on X to fix your hardware is going to be a
> > > > dead end. When you switch to wayland, you'll lose all of your fixes,
> > > > which isn't great.
> > >
> > > > AFAIU, the kernel now "merges" both buttons, which is a problem. It
> > > > seems to be a serious regression. This case is also worrying because I
> > > > added regression tests on hid, but I don't have access to all of the
> > > > various tablets, so I implemented them from the Microsoft
> > > > specification[0]. We need a special case for you here.
> > >
> > > The issue preventing David from mapping HID_DG_ERASER to BTN_STYLUS2 is
> > > that the hidinput_hid_event is not compatible with hidinput_setkeycode.
> > > If usage->code is no longer BTN_TOUCH after remapping, it won't be
> > > released when Eraser reports 0. A simple fix is:
> >
> >
> > I must confess, being the one who refactored everything, I still don't
> > believe this is as simple as it may seem. I paged out all of the
> > special cases, and now, without seeing the event flow I just can not
> > understand why this would fix the situation.
> >
> > And BTW, if you have a tool affected by 276e14e6c3, I'd be curious to
> > get a hid-recorder sample for it so I can get regression tests for it.
> >
> > > --- a/drivers/hid/hid-input.c
> > > +++ b/drivers/hid/hid-input.c
> > > @@ -1589,7 +1589,7 @@ void hidinput_hid_event(struct hid_device *hid,
> > > struct hid_field field, struct
> > > / value is off, tool is not rubber, ignore */
> > > return;
> > > else if (*quirks & HID_QUIRK_NOINVERT &&
> > > - !test_bit(BTN_TOUCH, input->key)) {
> > > + !test_bit(usage->code, input->key)) {
> >
> >
> > I don't want to be rude, but this feels very much like black magic,
> > especially because there is a comment just below and it is not
> > updated. So either the explanation was wrong, or it's not explaining
> > the situation (I also understand that this is not a formal submission,
> > so maybe that's the reason why the comment is not updated).
> >
> > > /*
> > > * There is no invert to release the tool, let hid_input
> > > * send BTN_TOUCH with scancode and release the tool after.
> > >
> > > This change alone fixes David's problem and the right-click mapping in
> > > hwdb works again. However, the tool switches to rubber for the remapped
> > > eraser (here BTN_STYLUS2) events, both for devices with and without
> > > Invert. This does no harm but is not useful either. A cleaner solution
> > > for devices without Invert would be to omit the whole tool switching
> > > logic in this case:
> > >
> > > @@ -1577,6 +1577,9 @@ void hidinput_hid_event(struct hid_device *hid,
> > > struct hid_field *field, struct
> > >
> > > switch (usage->hid) {
> > > case HID_DG_ERASER:
> > > + if (*quirks & HID_QUIRK_NOINVERT && usage->code != BTN_TOUCH)
> > > + break;
> > > +
> > > report->tool_active |= !!value;
> > >
> > > Remapping Invert does not work anyway as the Invert tool is hardcoded in
> > > hidinput_hid_event. Even worse, I guess (not tested) trying to do so
> > > would mask BTN_TOOL_RUBBER from dev->keybit and could cause weird
> > > behavior similar to one between 87562fcd1342 and 276e14e6c3. This
> > > raises the question: should users be able to remap Invert after all?
> >
> >
> > The kernel is supposed to transfer what the device is. So if it says
> > this is an eraser, we should not try to change it. Users can then
> > tweak their own device if they wish through hid-bpf or through
> > libinput quirks, but when you install a fresh kernel without tweaks,
> > we should be as accurate as possible.
> >
> > My main concern is that now we have a device which exports 2 different
> > interactions as being the same. So either the firmware is wrong, and
> > we need to quirk it, or the kernel is wrong and merges both, and this
> > needs fixes as well.
> >
> > Once every interaction on the device gets its own behavior, userspace
> > can do whatever they want. It's not the kernel's concern anymore.
> >
> > BTW, David, were you able to do a revert of 276e14e6c3?
> >
> > Cheers,
> > Benjamin
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-09  0:32                       ` David Revoy
@ 2023-11-09 11:56                         ` Benjamin Tissoires
  2023-11-09 16:13                           ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-09 11:56 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi David,

On Thu, Nov 9, 2023 at 1:32 AM David Revoy <davidrevoy@protonmail.com> wrote:
>
> Hi Benjamin,
>
> > Alright, I made quite some progress so far:
> > - regressions tests have been written (branch wip/xp-pen of my fork on
> > freedesktop[0])
> > that branch can not go in directly as it just adds the tests, and
> > thus is failing
> > - I made the fixes through HID-BPF[1]
> >
> > Anyone using those 2 tablets and using Fedora should be able to just
> > grab the artifact at [2], uncompress it and run `sudo ./install.sh --verbose`.
> > This will install the bpf programs in /lib/firmware/hid/bpf/ and will
> > automatically load them when the device is connected.
> >
> > For those not using Fedora, the binary might work (or not, not sure),
> > but you can always decompress it, and check if running
> > `udev-hid-bpf_0.1.0/bin/udev-hid-bpf --version` returns the correct
> > version or just fails. If you get "udev-hid-bpf 0.1.0", then running
> > `sudo ./install.sh --verbose` should work, as long as the kernel has
> > CONFIG_HID_BPF set to 'Y'.
> > [...]
> > [0] https://gitlab.freedesktop.org/bentiss/hid/-/tree/wip/xp-pen?ref_type=heads
> > [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
> > [2] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/jobs/51350589/artifacts/raw/udev-hid-bpf_0.1.0.tar.xz
>
> Thank you for this package.
>
> I was able to test it even though the link in (2) at the bottom of your email returned a blank page. I was able to find my way after manually visiting gitlab.freedesktop.org [1] and then manually downloading the article from 51350589. I unzipped it and ran `sudo ./install.sh --verbose`. Everything looks like it was successful [2]. I then rebooted my Fedora 38 'Linux workstation 6.5.8-200.fc38.x86_64' kernel (the one I blamed in my post) and tested both tablets.

Weird that you had to manually retrieve it. It works here, but maybe
because I am logged in on gitlab.fd.o.

Also, just FYI, you shouldn't have to reboot. Just unplug/replug and
you are good. In the same way, if you uninstall the package, you can
just unplug/replug to not have the programs loaded.

>
> Here are my observation:
>
> XPPEN Artist Pro 24
> ===================
> Nothing changed for this device (it's the one with two buttons and no 'eraser tip'). Nor my hwdb/udev rules or `xsetwacom set "UGTABLET 24 inch PenDisplay eraser" button 1 3` affects the upper button of the stylus: if I hold it hover the canvas, Krita switch the tool and cursor for an eraser. If I click on the canvas with the pen tip while holding the upper button pressed, I get the right-click Pop-up Palette (but not all the time, probably Krita has hard time to triage Eraser or Right-click).

As I mentioned in another reply, the more I think of it, the more I
think I should get rid of the "eraser mode". In that Artist Pro 24 I
can detect it through the same mechanics as the HID_QUIRK_NOINVERT
from Illia's patch. But instead of trying to force the device into the
eraser mode, we should just say "this is actually BUTTON_STYLUS_2".

So I'm going to amend the bpf program to do this and hopefully you
won't need the hwdb/udev rule at all.

>
> XPPEN Artist Pro 16 (Gen2)
> ==========================
> Something changed. `xsetwacom set "UGTABLET Artist Pro 16 (Gen2) eraser" button 1 3` successfully affected the upper button of the stylus. Now if I click it while hovering the canvas, Krita shows the right click Pop-up Palette.

I'm surprised you need to teach the wacom driver that BTN_STYLUS_2 is
the right click.

> On the downside; the real eraser tip when I flip the stylus bugs. When I flip the stylus on eraser hovering the canvas, Krita shows the Eraser icon and switch tool. As soon as I draw with the eraser tip, Krita will also show a right-click color palette and with also not a 100% consistency, as if the event were mixed.

I'll investigate. Maybe I messed up with my event flow patch.

But just to be sure, you don't have a custom configuration in place
for that tablet device?

Cheers,
Benjamin

>
> [1] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/artifacts
> [2] https://www.peppercarrot.com/extras/temp/2023-11-09_screenshot_005214_net.jpg
>
>
> On Wednesday, November 8th, 2023 at 19:21, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>
>
> > On Wed, Nov 8, 2023 at 10:34 AM Benjamin Tissoires
> > benjamin.tissoires@redhat.com wrote:
> >
> > > On Wed, Nov 8, 2023 at 10:23 AM José Expósito jose.exposito89@gmail.com wrote:
> > >
> > > > Hi Benjamin,
> > > >
> > > > On Wed, Nov 08, 2023 at 10:04:30AM +0100, Benjamin Tissoires wrote:
> > > > [...]
> > > >
> > > > > > So, the behavior probably breaks the specs, but sincerely I'm happy to
> > > > > > have the "eraser" button independent of the "rubber eraser", which
> > > > > > makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> > > > > > rubber), and I would like to keep this.
> > > > >
> > > > > Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
> > > > > apparently broke it.
> > > > >
> > > > > So, to me:
> > > > > - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> > > > > firmware bug (reporting invert through eraser) and should not be
> > > > > tackled at the generic level, thus it should be reverted
> > > > > - both of these tablets are forwarding the useful information, but not
> > > > > correctly, which confuses the kernel
> > > > > - I should now be able to write regression tests
> > > > > - I should be able to provide HID-BPF fixes for those tablets so that
> > > > > we can keep them working with or without
> > > > > 276e14e6c3993317257e1787e93b7166fbc30905
> > > > > reverted (hopefully)
> > > > > - problem is I still don't have the mechanics to integrate the HID-BPF
> > > > > fixes directly in the kernel tree, so maybe I'll have to write a
> > > > > driver for XP-Pen while these internals are set (it shouldn't
> > > > > interfere with the HID-BPF out of the tree).
> > > >
> > > > I already added support for a few XP-Pen devices on the UCLogic driver
> > > > and I was planning to start working on this one during the weekend in
> > > > my DIGImend fork (to simplify testing).
> > > >
> > > > Let me know if you prefer to add it yourself or if you want me to ping
> > > > you in the DIGImend discussion.
> > >
> > > So far, I really have to work on this now. It's a good use case for
> > > HID-BPF and it's a regression that I'd like to be fixed ASAP.
> > > I'd appreciate any reviews :)
> >
> >
> > Alright, I made quite some progress so far:
> > - regressions tests have been written (branch wip/xp-pen of my fork on
> > freedesktop[0])
> > that branch can not go in directly as it just adds the tests, and
> > thus is failing
> > - I made the fixes through HID-BPF[1]
> >
> > Anyone using those 2 tablets and using Fedora should be able to just
> > grab the artifact at [2], uncompress it and run `sudo ./install.sh --verbose`.
> > This will install the bpf programs in /lib/firmware/hid/bpf/ and will
> > automatically load them when the device is connected.
> >
> > For those not using Fedora, the binary might work (or not, not sure),
> > but you can always decompress it, and check if running
> > `udev-hid-bpf_0.1.0/bin/udev-hid-bpf --version` returns the correct
> > version or just fails. If you get "udev-hid-bpf 0.1.0", then running
> > `sudo ./install.sh --verbose` should work, as long as the kernel has
> > CONFIG_HID_BPF set to 'Y'.
> >
> > > Also, good to know that I can probably piggyback on hid-uclogic for
> > > fixing those 2 devices in the kernel.
> >
> >
> > Next step will be to fix them using a kernel driver, but it seems that
> > the uclogic driver is doing more than just report descriptor fixups,
> > so maybe I'll have to use a different driver.
> > But the point is, in theory, if you are affected by those bugs, using
> > the udev-hid-bpf should fix the issue, and this should also be
> > resilient to further kernel updates.
> >
> > I'd appreciate testing when I got the full kernel series up and ready,
> > of course.
> >
> > Cheers,
> > Benjamin
> >
> > [0] https://gitlab.freedesktop.org/bentiss/hid/-/tree/wip/xp-pen?ref_type=heads
> > [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
> > [2] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/jobs/51350589/artifacts/raw/udev-hid-bpf_0.1.0.tar.xz
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-09 11:56                         ` Benjamin Tissoires
@ 2023-11-09 16:13                           ` Benjamin Tissoires
  2023-11-09 22:04                             ` David Revoy
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-09 16:13 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

On Thu, Nov 9, 2023 at 12:56 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi David,
>
> On Thu, Nov 9, 2023 at 1:32 AM David Revoy <davidrevoy@protonmail.com> wrote:
> >
> > Hi Benjamin,
> >
> > > Alright, I made quite some progress so far:
> > > - regressions tests have been written (branch wip/xp-pen of my fork on
> > > freedesktop[0])
> > > that branch can not go in directly as it just adds the tests, and
> > > thus is failing
> > > - I made the fixes through HID-BPF[1]
> > >
> > > Anyone using those 2 tablets and using Fedora should be able to just
> > > grab the artifact at [2], uncompress it and run `sudo ./install.sh --verbose`.
> > > This will install the bpf programs in /lib/firmware/hid/bpf/ and will
> > > automatically load them when the device is connected.
> > >
> > > For those not using Fedora, the binary might work (or not, not sure),
> > > but you can always decompress it, and check if running
> > > `udev-hid-bpf_0.1.0/bin/udev-hid-bpf --version` returns the correct
> > > version or just fails. If you get "udev-hid-bpf 0.1.0", then running
> > > `sudo ./install.sh --verbose` should work, as long as the kernel has
> > > CONFIG_HID_BPF set to 'Y'.
> > > [...]
> > > [0] https://gitlab.freedesktop.org/bentiss/hid/-/tree/wip/xp-pen?ref_type=heads
> > > [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
> > > [2] https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/jobs/51350589/artifacts/raw/udev-hid-bpf_0.1.0.tar.xz
> >
> > Thank you for this package.
> >
> > I was able to test it even though the link in (2) at the bottom of your email returned a blank page. I was able to find my way after manually visiting gitlab.freedesktop.org [1] and then manually downloading the article from 51350589. I unzipped it and ran `sudo ./install.sh --verbose`. Everything looks like it was successful [2]. I then rebooted my Fedora 38 'Linux workstation 6.5.8-200.fc38.x86_64' kernel (the one I blamed in my post) and tested both tablets.
>
> Weird that you had to manually retrieve it. It works here, but maybe
> because I am logged in on gitlab.fd.o.
>
> Also, just FYI, you shouldn't have to reboot. Just unplug/replug and
> you are good. In the same way, if you uninstall the package, you can
> just unplug/replug to not have the programs loaded.

I've pushed an update of the file[0], turns out I made several mistakes.
As a general rule of thumb, you can follow the MR I've opened at [1],
click on the pipeline, open the last job ("make release"), then browse
the artifacts and pull the file from there.

>
> >
> > Here are my observation:
> >
> > XPPEN Artist Pro 24
> > ===================
> > Nothing changed for this device (it's the one with two buttons and no 'eraser tip'). Nor my hwdb/udev rules or `xsetwacom set "UGTABLET 24 inch PenDisplay eraser" button 1 3` affects the upper button of the stylus: if I hold it hover the canvas, Krita switch the tool and cursor for an eraser. If I click on the canvas with the pen tip while holding the upper button pressed, I get the right-click Pop-up Palette (but not all the time, probably Krita has hard time to triage Eraser or Right-click).
>
> As I mentioned in another reply, the more I think of it, the more I
> think I should get rid of the "eraser mode". In that Artist Pro 24 I
> can detect it through the same mechanics as the HID_QUIRK_NOINVERT
> from Illia's patch. But instead of trying to force the device into the
> eraser mode, we should just say "this is actually BUTTON_STYLUS_2".
>
> So I'm going to amend the bpf program to do this and hopefully you
> won't need the hwdb/udev rule at all.

I've fixed that one normally. There were a couple of issues:
- the PID in use was the one from the pro 16 gen2, which explained why
no change was appearing
- I've now decided to not export the second button as an eraser, as
mentioned above.

>
> >
> > XPPEN Artist Pro 16 (Gen2)
> > ==========================
> > Something changed. `xsetwacom set "UGTABLET Artist Pro 16 (Gen2) eraser" button 1 3` successfully affected the upper button of the stylus. Now if I click it while hovering the canvas, Krita shows the right click Pop-up Palette.
>
> I'm surprised you need to teach the wacom driver that BTN_STYLUS_2 is
> the right click.
>
> > On the downside; the real eraser tip when I flip the stylus bugs. When I flip the stylus on eraser hovering the canvas, Krita shows the Eraser icon and switch tool. As soon as I draw with the eraser tip, Krita will also show a right-click color palette and with also not a 100% consistency, as if the event were mixed.
>
> I'll investigate. Maybe I messed up with my event flow patch.

Definitely my mistake: both the bpf programs I wrote were attached to
the same device. Thus, the 2 fixes were stacking on each other,
leading to some interesting side effects.

You can check that the bpf are properly loaded by having a look at the
report descriptor when you replug the device:
if you see "Secondary Barrel Switch" at offset 16 instead of "Eraser"
on both of your tablets (with hid-recorder), you should have
successfully patched your devices.

Cheers,
Benjamin

>
> But just to be sure, you don't have a custom configuration in place
> for that tablet device?
>

[0] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/51399392/artifacts/file/udev-hid-bpf_0.1.0.tar.xz
[1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-09 16:13                           ` Benjamin Tissoires
@ 2023-11-09 22:04                             ` David Revoy
  2023-11-10 10:19                               ` Benjamin Tissoires
  2023-11-11  8:52                               ` Benjamin Tissoires
  0 siblings, 2 replies; 40+ messages in thread
From: David Revoy @ 2023-11-09 22:04 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi Benjamin,

Thank you it works! 🎉 🎉 🎉 

> I've pushed an update of the file[0], turns out I made several mistakes.
> As a general rule of thumb, you can follow the MR I've opened at [1],
> click on the pipeline, open the last job ("make release"), then browse
> the artifacts and pull the file from there.
> [...]
> > But just to be sure, you don't have a custom configuration in place
> > for that tablet device?
> [...]
> [0] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/51399392/artifacts/file/udev-hid-bpf_0.1.0.tar.xz
> [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27

I tested the latest artifact on kernel 6.5.8-200.fc38.x86_64 and I also removed my custom configuration at startup (I had not much: an hwdb files for the 24 Pro −mainly for frame buttons− and an xsetwacom bash script for each tablet). 

During the tests, the styluses of both 24 Pro and 16 Pro Gen2 worked perfectly: right-click on upper button out-of-the-box, and the eraser tip of the 16 Pro Gen2 continued to erase as expected. 

I could also target with xsetwacom this 'button 3' of the styluses, and I tested random available shortcuts (but I'll keep default right-click).

So, good job, and many thanks!

I want now to write a follow-up after the first blog-post. I see it is a MR [1], maybe it means if it get merged it will be part of libevdev? What would you advice to write for the ones who want to benefit from the fix?

Thanks again,
David

[1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-09 22:04                             ` David Revoy
@ 2023-11-10 10:19                               ` Benjamin Tissoires
  2023-11-11  8:52                               ` Benjamin Tissoires
  1 sibling, 0 replies; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-10 10:19 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel, Peter Hutterer

Hi David,

On Thu, Nov 9, 2023 at 11:04 PM David Revoy <davidrevoy@protonmail.com> wrote:
>
> Hi Benjamin,
>
> Thank you it works! 🎉 🎉 🎉

\o/  /o\  \o/

>
> > I've pushed an update of the file[0], turns out I made several mistakes.
> > As a general rule of thumb, you can follow the MR I've opened at [1],
> > click on the pipeline, open the last job ("make release"), then browse
> > the artifacts and pull the file from there.
> > [...]
> > > But just to be sure, you don't have a custom configuration in place
> > > for that tablet device?
> > [...]
> > [0] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/51399392/artifacts/file/udev-hid-bpf_0.1.0.tar.xz
> > [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
>
> I tested the latest artifact on kernel 6.5.8-200.fc38.x86_64 and I also removed my custom configuration at startup (I had not much: an hwdb files for the 24 Pro −mainly for frame buttons− and an xsetwacom bash script for each tablet).
>
> During the tests, the styluses of both 24 Pro and 16 Pro Gen2 worked perfectly: right-click on upper button out-of-the-box, and the eraser tip of the 16 Pro Gen2 continued to erase as expected.
>
> I could also target with xsetwacom this 'button 3' of the styluses, and I tested random available shortcuts (but I'll keep default right-click).
>
> So, good job, and many thanks!
>
> I want now to write a follow-up after the first blog-post. I see it is a MR [1], maybe it means if it get merged it will be part of libevdev? What would you advice to write for the ones who want to benefit from the fix?

This MR will be merged into udev-hid-bpf. It's not "libevdev", the
library, per se, but we piggyback on the libevdev project some tools
that are more or less official. Right now, udev-hid-bpf is not
integrated in any distribution. It's a relatively new project.

As for the blog post, feel free to copy/extract/rewrite/link the
following. I've tried to address the couple of elements that worried
me on your initial blog post (mostly the why), and gave a rough
overview of what we have done here:

---

Here is a little bit of history of why you were encountering this bug:

First, HID is a quite old protocol and has the benefit of being "plug
and play" [0] [1].

But plug and play often means for a hardware maker: "let's do trial
and error until Windows seems to behave in a correct way".

In some other cases, Microsoft puts more restrictions on the HID part
(Windows 7 enforced touchscreens to follow a specific pattern, and
then Windows 8 did it for touchpads). And they also sometimes provide
a test suite that hardware makers have to pass to be "certified".
They have to pass the test suite by using the Windows provided generic
driver, but Windows also allows them to create a virtual HID device
and let a custom driver create that virtual HID device. Which means,
we sometimes have devices behaving badly but working perfectly fine on
Windows because there are bits missing in the device itself that are
fixed by adding an extra software layer. Sigh.

But I digress and we need to go back to the pen bits, and AFAIK, there
is no such test suite and certification.

So basically, hardware makers follow the empiric rule of "if Windows
is happy, I am too".

To do so, they have to use several events from HID [2] (quoting them):
- *Tip Switch* -> A switch located at the tip of a stylus, indicating
contact of the stylus with a surface. A pen-based system or system
extension would use this switch to enable the input of handwriting or
gesture data. The system typically maps Tip Switch to a primary button
in a non-pen context.
- *In Range* -> Indicates that a transducer is located within the
region where digitizing is possible. In Range is a bit quantity
- *Barrel Switch* -> A non-tip button located on the barrel of a
stylus. Its function is typically mapped to a system secondary button
or to a Shift key modifier that changes the Tip Switch function from
primary button to secondary button.
- *Secondary Barrel Switch* -> A second non-tip button located on the
barrel of a stylus further from the tip than the Barrel Switch. Its
function is typically mapped to a system secondary or tertiary button.
- *Eraser* -> This control is used for erasing objects. Following the
metaphor of a pencil, it is typically located opposite the writing end
of a stylus. It may be a bit switch or a pressure quantity.
- *Invert* -> A bit that indicates that the currently sensed position
originates from the end of a stylus opposite the tip.

I'm sure that by reading those, everybody should be able to
immediately know how to write a Pen HID device, and how the
interactions between the events should be. :) (If you are, please
contact me ASAP, we have plenty of kernel work to do).

So for years the state of pen devices in the Linux kernel was 2 fold:
- Wacom shipped an in-kernel driver for their own devices, that they
tested and that defined the de-facto "standard" in Linux
- the rest was trying to catch up by luck or with the help of projects
like DiGiMend, by relying on the generic HID processing of the Linux
kernel

However, they were no specification for how the events should come:
basically in the hid generic input processing each event was mapped to
a single input keycode and we had situations were we would get both
`BTN_TOOL_PEN` and `BTN_TOOL_ERASER` at the same time (because the `In
Range` and the `Eraser` bits were sent by the device at the same
time). Which means "hey, the user is interacting with a pen with both
the tail and the tip at the same time. Good luck with that!"

This led to a complex situation where userspace (libinput mostly) had
to do guesses on what is the intent of the user. But the problem is
that when you are in userspace, you don't know all of the events of
the device at the same time, so it was messy to deal with. Again,
Wacom devices were unaffected because they controlled all of the
stack: a kernel driver to fix/interpret their devices and a userspace
component, xf86-drv-wacom, in the X.org world.

Once, as you mentioned in your blog, Microsoft decided to use the
second barrel button as the "rubber" mode. The reason was practical:
people liked the rubber capability on the styluses, but they wanted to
have a separate button on the tail end of the styluses. And I suppose
that at the time, given that no other hardware vendors were capable of
having no-battery styluses but Wacom (IP protection and capabilities
of the hardware IIRC), you still had to put the battery somewhere. And
that tail end is convenient for storing such a battery. But that makes
it harder to have an eraser end because you need to link both ends of
the pen on the same IC, with a battery in the middle that is roughly
the same size as your pen's barrel. So having just 2 wires for the
battery allows you to have a separate bluetooth button on one end, and
the normal stylus on the other end, and keep the width of the pen
reasonable.

So that choice of using the second button as an eraser was made, and
the hardware makers followed: on the XP-Pen Artist Pro 24, the device
reports `Tip Switch`, `Barrel Switch`, `Eraser`, `In Range`.
Which is "correct" according to the HID Usage Table [2], but which
doesn't adhere to the recommendation Microsoft is doing [3]: the
device should report an extra `Invert` when the pen is in range with
the intent to erase...

But you can see that XP-Pen tried to adhere to it in some ways because
if you look carefully at the events coming from the device with
hid-recorder[4], you'll notice that when you are in range of the
sensor and press this button, you'll get an extra "In Range = 0" event
to notify that you went out of proximity of the sensor.

In kernel 5.18, with commit 87562fcd1342 ("HID: input: remove the need
for HID_QUIRK_INVERT"), I tried to remove those multiple tool states
to have a straightforward state provided by the kernel that userspace
can deal with easily. However, given that there were no regression
tests at the time for generic tablets, I wrote some based on
Microsoft's recommendation [3] and also tested on a Huion device I
have locally. And it was working fine. But I didn't have the devices
that were not sending `Invert`, which explained why it was bogus on
those devices.

This was "fixed" in kernel 6.6 with commit 276e14e6c399 ("HID: input:
Support devices sending Eraser without Invert"). Putting quotes around
"fixed" because I'm still not happy about this patch.

But the point is, from kernel 5.18, the Pen processing in the kernel
became a state machine, which means we can not have external actors
tampering with it.


Why using the ioctl EVIOCSKEYCODE is bad to remap `Eraser` to
`BTN_STYLUS2` (through tools like evmap):

Having the ability to do something doesn't mean it's the right thing
to do. And in that case, this is definitely wrong because you have to
call the ioctl after the kernel presents the device to userspace.
Which means userspace (and the kernel) already made assumptions on the
device itself. There is a high chance libinput (or the Wacom driver)
opens the device before evmap, and that it is considering that the
device doesn't have `BTN_STYLUS2`. So sending that event would break
userspace.

And in our case here, the kernel expects some state between the input
layer and its internal HID state, and remapping that HID event to
something else confuses it.

There is another side effect of this: usually end users configuring
their devices with such tools do not report back their configuration
to the kernel community. In some cases this is valid (this is my
preference and my choice), but in other cases it's not (there is a bug
here and I'm papering over it).


So, what can be done?

Basically 2 options:
1. write a kernel patch to fix that problem once and for all
2. use the brand new HID-BPF[5] capability introduced in kernel v6.3
and send me back the BPF program so I can eventually integrate the
source in the kernel tree itself and fix that problem once and for all
as well

For 1., you need:
- to be able to dig into the kernel code
- to be able to write a patch with the correct kernel standard (with a
regression test in `tools/testing/selftests/hid`, please)
- to be able to compile your own kernel and test it
- to be able to submit your contribution by email (I can suggest using
b4 for that, very nice tool)
- to be able to take reviews into account, and learn `git rebase -i`
to submit v2, v3, and potentially v10 or more in some complex cases
- to wait for the patch to be integrated into Linus' tree
- to wait for Greg to backport your patch into a stable kernel tree
- to wait for your distribution to pick up the stable tree with your patch

That's relatively easy, no? :)

OTOTH, we have 2.: HID-BPF [5]

Very quickly, eBPF [6] is a state machine inside the kernel that
allows user space to include a verified code path in the kernel to
tweak its behavior. And I adapted this for HID so you can:
- change the report descriptor of the device: this
disconnects/reconnects the device, meaning the kernel works on the new
report descriptor and is consistent with its state
- change the event flow of the device: to fix the spurious out-of-prox
event for example
- more to come

What is interesting in BPF (or eBPF), is that nowadays, libbpf
implements something named CORE (Compile Once Run Everywhere). Which
means that if I compile locally an eBPF program on my machine with my
development kernel, as long as I only use functions available from
kernel v6.3 for instance, the same compilation output (that changes
the event flow of your HID device) will work on any kernel from v6.3
unless there are some later API breakages[7].

Which means, anybody can modify the event flow of an HID device, put
the file in the filesystem, and have the device still fixed even if
they upgrade their kernel.

In the long run, I intend to include those HID-BPF fixes in the kernel
tree to centralize them, but also to be able to automatically load
them from the kernel when those devices appear.

Which means, for the reporter of such a bug you:
- can now rely on someone else to write the code, compile it and
provide the compilation result [10]
- just put that result in the filesystem to have the device tested and fixed

Behind the scenes, that other knowledgeable person can take the heavy
task of submitting the kernel patch for you, but given that the code
has been tested, it's way easier to do (and to eventually re-test).

Currently, the "let's integrate that bpf program in the kernel" is not
completely done, so we use udev-hid-bpf[8][9] to give it a jump start.

And that's exactly what happened in your case David. Which is why I'm
so happy (also because I fixed the tools from an author I like and
already had the books at home :-P):

You want your device to be fixed now, but going through a regular
kernel patch means months before it's fixed in your distribution.
But with HID-BPF, I fixed it now, and you can safely upgrade the
kernel, because even if I do changes in the kernel, the HID-BPF fix
will still convert the device into something valid from the HID point
of view, and it has a regression test now. When your device will be
fixed in the future in the kernel, there is a high chance the `probe`
function of the HID-BPF program will say that it's not the correct
device, and so the program will not even load and rely on the fixed
kernel only. Transparently for you, without you having to change your
filesystem.


On my side, what's left to be done:

- First, I need to fix the tablets not sending the `Invert` usage.
Commit 276e14e6c399 ("HID: input: Support devices sending Eraser
without Invert") is IMO not good enough, and we might as well simply
say that if there is no `Invert` usage, we can convert the `Eraser`
usage into `Secondary Barrel Switch`
- then I need to fix the XP-Pen Artist Pro 16 gen 2 from the kernel
too, by replacing the `Eraser` usage with `Secondary Barrel Switch`.
Ideally I would just dump the HID-BPF program in the kernel, but this
is not available yet, so I'll probably write a small kernel driver
using the same code path as the HID-BPF program.
- then Peter and I need to write a more generic HID-BPF program to
convert "eraser mode buttons" into `Secondary Barrel Switch`,
basically unwinding what the hardware does. This can only happen when
libinput will be able to do the opposite transformation so we don't
regress. But we can rely on libwacom to tell us if this pen has a tail
end eraser or not, and then have userspace choose if they want the
button to be a button, or an eraser mode.

I think that's pretty much it.

Thanks for reading through everything :)


Cheers,
Benjamin


[0] https://who-t.blogspot.com/2018/12/understanding-hid-report-descriptors.html
[1] https://docs.kernel.org/hid/hidintro.html
[2] https://usb.org/sites/default/files/hut1_4.pdf
[3] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states
[4] https://gitlab.freedesktop.org/libevdev/hid-tools
[5] https://docs.kernel.org/hid/hid-bpf.html
[6] https://docs.kernel.org/bpf/index.html
[7] but if API breakage happens, all that will happen is that the
HID-BPF program will not be loaded. No kernel crash involved.
[8] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf
[9] https://libevdev.pages.freedesktop.org/udev-hid-bpf/tutorial.html
[10] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27



>
> Thanks again,
> David
>
> [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
>



^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-09 22:04                             ` David Revoy
  2023-11-10 10:19                               ` Benjamin Tissoires
@ 2023-11-11  8:52                               ` Benjamin Tissoires
  2023-11-13 22:08                                 ` David Revoy
  1 sibling, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-11  8:52 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi David,

On Thu, Nov 9, 2023 at 11:04 PM David Revoy <davidrevoy@protonmail.com> wrote:
>
> Hi Benjamin,
>
> Thank you it works! 🎉 🎉 🎉
>
> > I've pushed an update of the file[0], turns out I made several mistakes.
> > As a general rule of thumb, you can follow the MR I've opened at [1],
> > click on the pipeline, open the last job ("make release"), then browse
> > the artifacts and pull the file from there.
> > [...]
> > > But just to be sure, you don't have a custom configuration in place
> > > for that tablet device?
> > [...]
> > [0] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/51399392/artifacts/file/udev-hid-bpf_0.1.0.tar.xz
> > [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
>
> I tested the latest artifact on kernel 6.5.8-200.fc38.x86_64 and I also removed my custom configuration at startup (I had not much: an hwdb files for the 24 Pro −mainly for frame buttons− and an xsetwacom bash script for each tablet).
>
> During the tests, the styluses of both 24 Pro and 16 Pro Gen2 worked perfectly: right-click on upper button out-of-the-box, and the eraser tip of the 16 Pro Gen2 continued to erase as expected.

While polishing the patches yesterday I realized that there was a bug
on the 24 Pro: it would never send proximity out events unless on a
very specific case: if you hold the pen perfectly vertical. This might
have an effect on the session and the pen behavior.

Could you please once again attach the hid-recorder of the Pro 24
while doing the following sequence:
- touch with the tip of the stylus the tablet
- while touching, press the upper button (the problematic one)
- still while touching, release the button
- remove the stylus

I need it to check what the device is sending when an "eraser mode
button" is pressed while the tip is touching the surface. Because I
think that's the only one problematic proximity out event as it would
"break" the current touch.

I have created another MR[2] to fix that, and I would appreciate it if
you could also give a test of the artifacts of job 51469284[3].

The points to check are:
- if you right click while touching the surface, do you still get only
a right click or some weird glitches in addition to it?
- if you right click while not touching (hovering), no glitches?

Cheers,
Benjamin

>
> I could also target with xsetwacom this 'button 3' of the styluses, and I tested random available shortcuts (but I'll keep default right-click).
>
> So, good job, and many thanks!
>
> I want now to write a follow-up after the first blog-post. I see it is a MR [1], maybe it means if it get merged it will be part of libevdev? What would you advice to write for the ones who want to benefit from the fix?
>
> Thanks again,
> David
>
> [1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/27
>

[2] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/30
[3] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/51469284


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-08 23:18             ` David Revoy
  2023-11-09 11:47               ` Benjamin Tissoires
@ 2023-11-11 17:15               ` Thorsten Leemhuis
  1 sibling, 0 replies; 40+ messages in thread
From: Thorsten Leemhuis @ 2023-11-11 17:15 UTC (permalink / raw)
  To: David Revoy, Benjamin Tissoires
  Cc: Illia Ostapyshyn, jkosina, jason.gerecke, jose.exposito89,
	linux-input, linux-kernel, nils, peter.hutterer, ping.cheng,
	bagasdotme

On 09.11.23 00:18, David Revoy wrote:
>> BTW, David, were you able to do a revert of 276e14e6c3?
> 
> I'm sorry Benjamin: I did some research on how to build a kernel [1], on how to revert a commit (easy, I know a bit of Git), and started following it step by step. Result: I failed and concluded that it probably requires too much computer knowledge compared to what I can do now. I'm afraid I won't be able to build a custom kernel for testing.
> 
> [1] https://docs.fedoraproject.org/en-US/quick-docs/kernel-build-custom/#_building_a_vanilla_upstream_kernel

FWIW, in case you want to try again: give the upstream text on building
your own kernel a try, maybe it works better for your:

https://docs.kernel.org/admin-guide/quickly-build-trimmed-linux.html

It should work on Fedora (but you might want to install the latest
updates first, there recently was a problem).

Ciao, Thorsten

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-11  8:52                               ` Benjamin Tissoires
@ 2023-11-13 22:08                                 ` David Revoy
  2023-11-14 14:35                                   ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: David Revoy @ 2023-11-13 22:08 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi Benjamin,

> Here is a little bit of history of why you were encountering this bug [...]

Many thanks for all the details you wrote about the bug, I found 
your email so interesting that I couldn't resist to copy/paste it 
on my blog[1].  

> And that's exactly what happened in your case David. Which is why I'm
> so happy (also because I fixed the tools from an author I like and
> already had the books at home :-P):

Please send me an email in private with your adress; I'll be happy to
send you an original drawing. That will be my way to thank you :-) 

> Could you please once again attach the hid-recorder of the Pro 24
> while doing the following sequence:
> - touch with the tip of the stylus the tablet
> - while touching, press the upper button (the problematic one)
> - still while touching, release the button
> - remove the stylus

Sure, you'll find the action (repeated three times) recorded here [2]

> you could also give a test of the artifacts of job 51469284[3].
> 
> The points to check are:
> - if you right click while touching the surface, do you still get only
> a right click or some weird glitches in addition to it?
> - if you right click while not touching (hovering), no glitches?

I tested. It's a bit hard to tell if it causes glitches or if the
behavior is normal or not. I'm not using the right-click this way.
I always use it in "hover mode". With artifact or without, the 
behavior is while the tip is pressed, the right-click quickly 
'disapear' but it is probably normal because I test on contextual
menu and clicking somewhere else makes this type of menu disapear.

I hope this will help,
Cheers,

David

[1] https://www.davidrevoy.com/article1002/how-a-kernel-developer-made-my-styluses-work-again  
[2] https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-24-Pro/XPPEN-Artist-24-Pro_pen_tip-contact-and-action-press-release-upper-stylus-button-x3.txt

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-13 22:08                                 ` David Revoy
@ 2023-11-14 14:35                                   ` Benjamin Tissoires
  2023-11-15 15:14                                     ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-14 14:35 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi David,

On Mon, Nov 13, 2023 at 11:08 PM David Revoy <davidrevoy@protonmail.com> wrote:
>
> Hi Benjamin,
>
> > Here is a little bit of history of why you were encountering this bug [...]
>
> Many thanks for all the details you wrote about the bug, I found
> your email so interesting that I couldn't resist to copy/paste it
> on my blog[1].

Heh, glad you found it interesting. Too bad it was tough to understand :)

>
> > And that's exactly what happened in your case David. Which is why I'm
> > so happy (also because I fixed the tools from an author I like and
> > already had the books at home :-P):
>
> Please send me an email in private with your adress; I'll be happy to
> send you an original drawing. That will be my way to thank you :-)

Oh, that would be wonderful. Many thanks.
I'll send a separate email.

>
> > Could you please once again attach the hid-recorder of the Pro 24
> > while doing the following sequence:
> > - touch with the tip of the stylus the tablet
> > - while touching, press the upper button (the problematic one)
> > - still while touching, release the button
> > - remove the stylus
>
> Sure, you'll find the action (repeated three times) recorded here [2]

Thanks a lot. However, I realized this morning 2 issues (sorry):
- you made the recording while the HID-BPF program was attached, which
is now obvious that anyone would do that. But the program sometimes
discards events, so I am not sure now if sometimes the device is not
sending the spurious events, or if the filter was acting correctly.
(Note to self, next time, while in the testing phase, do not blindly
discard the events, but remap them to an ignored report)
- that device is really "interesting" in how it behaves with the
eraser mode emulation: when you press the second button while touching
the pen, we get a spurious release of the touch event... And this
leads me to think that I'm not sure about all of the transitions we
can have with buttons :(

TL;DR: there is still work to do for me and for you if you still agree
to send me more traces.

>
> > you could also give a test of the artifacts of job 51469284[3].
> >
> > The points to check are:
> > - if you right click while touching the surface, do you still get only
> > a right click or some weird glitches in addition to it?
> > - if you right click while not touching (hovering), no glitches?
>
> I tested. It's a bit hard to tell if it causes glitches or if the
> behavior is normal or not. I'm not using the right-click this way.
> I always use it in "hover mode". With artifact or without, the
> behavior is while the tip is pressed, the right-click quickly
> 'disapear' but it is probably normal because I test on contextual
> menu and clicking somewhere else makes this type of menu disapear.

AFAICT you used the artifacts from job 51469284. Which is good. But as
I mentioned above, the tablet is sending a spurious event I haven't
anticipated, which results in a left click (well release/click) from
the host point of view. And that very well explains the disappearance
of the right-click menu.

>
> I hope this will help,

It does, but there are glitches that I'd like to fix. I need to iron
out the bpf filter for those use cases. I rewrote the tests today to
take those into account (assuming I understand the HW enough) and I
think they are better now.

But I would also totally understand that you had enough debugging and
you would rather focus on using the tablets, instead of debugging
them. In which case, someone else from the community might help me.

Cheers,
Benjamin


> Cheers,
>
> David
>
> [1] https://www.davidrevoy.com/article1002/how-a-kernel-developer-made-my-styluses-work-again
> [2] https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-24-Pro/XPPEN-Artist-24-Pro_pen_tip-contact-and-action-press-release-upper-stylus-button-x3.txt
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-14 14:35                                   ` Benjamin Tissoires
@ 2023-11-15 15:14                                     ` Benjamin Tissoires
  2023-11-23 22:12                                       ` David Revoy
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-15 15:14 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi David,

On Tue, Nov 14, 2023 at 3:35 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
[...]
> > > Could you please once again attach the hid-recorder of the Pro 24
> > > while doing the following sequence:
> > > - touch with the tip of the stylus the tablet
> > > - while touching, press the upper button (the problematic one)
> > > - still while touching, release the button
> > > - remove the stylus
> >
> > Sure, you'll find the action (repeated three times) recorded here [2]
>
> Thanks a lot. However, I realized this morning 2 issues (sorry):
> - you made the recording while the HID-BPF program was attached, which
> is now obvious that anyone would do that. But the program sometimes
> discards events, so I am not sure now if sometimes the device is not
> sending the spurious events, or if the filter was acting correctly.
> (Note to self, next time, while in the testing phase, do not blindly
> discard the events, but remap them to an ignored report)
> - that device is really "interesting" in how it behaves with the
> eraser mode emulation: when you press the second button while touching
> the pen, we get a spurious release of the touch event... And this
> leads me to think that I'm not sure about all of the transitions we
> can have with buttons :(
>
> TL;DR: there is still work to do for me and for you if you still agree
> to send me more traces.
>

I managed to refine the bpf filter. Assuming I am correct in what I
understand from the device, of course.
Also this time I made sure the original events are still around but
unprocessed by the input layer :)

So it would be nice if you could try the artifacts of job 51600738[4].
Again, download them (udev-hid-bpf_0.1.0-4-g5ab02ec.tar.xz), unpack,
sudo ./install --verbose, then unplug/replug the artist Pro 24.

Then, I'll need the following sequence (ideally repeated twice or
three times, given that your last record show a slight difference in
the first and second attempt):

- outside of the proximity of the sensor, press the upper button
- approach the stylus to the surface keeping the upper button pressed
- touch the surface with the tip while holding the upper button pressed
- release the upper button while keeping the tip pressed (like previously)
- press once again the upper button while keeping the tip touching the
surface (like previously)
- lift of the pen, keeping the upper button pressed, and still in
range of the sensor
- remove the pen from the proximity of the sensor entirely (move away
20 cm or so), while still keeping the upper button pressed

It's actually longer to describe than to execute :)

> >
> > > you could also give a test of the artifacts of job 51469284[3].
> > >
> > > The points to check are:
> > > - if you right click while touching the surface, do you still get only
> > > a right click or some weird glitches in addition to it?
> > > - if you right click while not touching (hovering), no glitches?
> >
> > I tested. It's a bit hard to tell if it causes glitches or if the
> > behavior is normal or not. I'm not using the right-click this way.
> > I always use it in "hover mode". With artifact or without, the
> > behavior is while the tip is pressed, the right-click quickly
> > 'disapear' but it is probably normal because I test on contextual
> > menu and clicking somewhere else makes this type of menu disapear.
>
> AFAICT you used the artifacts from job 51469284. Which is good. But as
> I mentioned above, the tablet is sending a spurious event I haven't
> anticipated, which results in a left click (well release/click) from
> the host point of view. And that very well explains the disappearance
> of the right-click menu.

I think I managed to remove the spurious release/click with the latest
udev-hid-bpf pipeline.

>
> >
> > I hope this will help,
>
> It does, but there are glitches that I'd like to fix. I need to iron
> out the bpf filter for those use cases. I rewrote the tests today to
> take those into account (assuming I understand the HW enough) and I
> think they are better now.
>
> But I would also totally understand that you had enough debugging and
> you would rather focus on using the tablets, instead of debugging
> them. In which case, someone else from the community might help me.
>
> Cheers,
> Benjamin
>
>
> > Cheers,
> >
> > David
> >
> > [1] https://www.davidrevoy.com/article1002/how-a-kernel-developer-made-my-styluses-work-again
> > [2] https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-24-Pro/XPPEN-Artist-24-Pro_pen_tip-contact-and-action-press-release-upper-stylus-button-x3.txt
> >

Cheers,
Benjamin

[4] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/51600738


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-15 15:14                                     ` Benjamin Tissoires
@ 2023-11-23 22:12                                       ` David Revoy
  2023-11-24 17:18                                         ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: David Revoy @ 2023-11-23 22:12 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi Benjamin,

Sorry for late reply.

> So it would be nice if you could try the artifacts of job 51600738[4].
> Again, download them (udev-hid-bpf_0.1.0-4-g5ab02ec.tar.xz), unpack,
> sudo ./install --verbose, then unplug/replug the artist Pro 24.

Ok, the main change I experienced after installing is xsetwacom 
listing the ID name of the device with twice the name Stylus on 
"UGTABLET 24 inch PenDisplay Stylus stylus". Before it was only 
"UGTABLET 24 inch PenDisplay stylus".

$ xsetwacom --list
UGTABLET 24 inch PenDisplay Stylus stylus       id: 10  type: STYLUS 

Not a big deal, but I thought it was worth to mention it.

> Then, I'll need the following sequence (ideally repeated twice or
> three times, given that your last record show a slight difference in
> the first and second attempt):
> 
> - outside of the proximity of the sensor, press the upper button
> - approach the stylus to the surface keeping the upper button pressed
> - touch the surface with the tip while holding the upper button pressed
> - release the upper button while keeping the tip pressed (like previously)
> - press once again the upper button while keeping the tip touching the
> surface (like previously)
> - lift of the pen, keeping the upper button pressed, and still in
> range of the sensor
> - remove the pen from the proximity of the sensor entirely (move away
> 20 cm or so), while still keeping the upper button pressed
> 
> It's actually longer to describe than to execute :)
> 

Thank you for the detailed steps. True, it makes sens once 
practising it. I made the gesture three time on: 

https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-24-Pro/2023-11-23_XPPEN-Artist-24-Pro_pen_tip-contact-and-press-release-upper-stylus-button-while-pressed-x3.txt

> But I would also totally understand that you had enough debugging and
> you would rather focus on using the tablets, instead of debugging
> them. In which case, someone else from the community might help me.

No problem for continue testing, I'm around! Thank you again 
for improving the behavior of the tablets.

Cheers,
David

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-23 22:12                                       ` David Revoy
@ 2023-11-24 17:18                                         ` Benjamin Tissoires
  2023-11-29 15:32                                           ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-24 17:18 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi David,

On Thu, Nov 23, 2023 at 11:12 PM David Revoy <davidrevoy@protonmail.com> wrote:
>
> Hi Benjamin,
>
> Sorry for late reply.
>
> > So it would be nice if you could try the artifacts of job 51600738[4].
> > Again, download them (udev-hid-bpf_0.1.0-4-g5ab02ec.tar.xz), unpack,
> > sudo ./install --verbose, then unplug/replug the artist Pro 24.
>
> Ok, the main change I experienced after installing is xsetwacom
> listing the ID name of the device with twice the name Stylus on
> "UGTABLET 24 inch PenDisplay Stylus stylus". Before it was only
> "UGTABLET 24 inch PenDisplay stylus".
>
> $ xsetwacom --list
> UGTABLET 24 inch PenDisplay Stylus stylus       id: 10  type: STYLUS
>
> Not a big deal, but I thought it was worth to mention it.

Oh, this might be because I added a debug device. Given that there are
2 devices on the HID node, then one is tagged as Stylus by the kernel.
Nothing to worry about.

>
> > Then, I'll need the following sequence (ideally repeated twice or
> > three times, given that your last record show a slight difference in
> > the first and second attempt):
> >
> > - outside of the proximity of the sensor, press the upper button
> > - approach the stylus to the surface keeping the upper button pressed
> > - touch the surface with the tip while holding the upper button pressed
> > - release the upper button while keeping the tip pressed (like previously)
> > - press once again the upper button while keeping the tip touching the
> > surface (like previously)
> > - lift of the pen, keeping the upper button pressed, and still in
> > range of the sensor
> > - remove the pen from the proximity of the sensor entirely (move away
> > 20 cm or so), while still keeping the upper button pressed
> >
> > It's actually longer to describe than to execute :)
> >
>
> Thank you for the detailed steps. True, it makes sens once
> practising it. I made the gesture three time on:
>
> https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-24-Pro/2023-11-23_XPPEN-Artist-24-Pro_pen_tip-contact-and-press-release-upper-stylus-button-while-pressed-x3.txt

Thanks a lot. And of course this device doesn't react in the way I expected :)

Transitions from/to the tip touching the surface while the second
button is pressed are normal, there are no extra events...

But this also showed that the previous filter was better when pressing
the upper button while touching the tip on the surface, because now we
get another spurious event that was filtered before (and because it
was filtered, I thought it was not there).

Anyway, I couldn't rewrite the filter today, but I'll work on it next
week for sure.

>
> > But I would also totally understand that you had enough debugging and
> > you would rather focus on using the tablets, instead of debugging
> > them. In which case, someone else from the community might help me.
>
> No problem for continue testing, I'm around! Thank you again
> for improving the behavior of the tablets.
>

great!

Cheers,
Benjamin


> Cheers,
> David
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-24 17:18                                         ` Benjamin Tissoires
@ 2023-11-29 15:32                                           ` Benjamin Tissoires
  2023-11-30 22:25                                             ` David Revoy
  0 siblings, 1 reply; 40+ messages in thread
From: Benjamin Tissoires @ 2023-11-29 15:32 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

On Fri, Nov 24, 2023 at 6:18 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi David,
>
> On Thu, Nov 23, 2023 at 11:12 PM David Revoy <davidrevoy@protonmail.com> wrote:
> >
> > Hi Benjamin,
> >
> > Sorry for late reply.
> >
> > > So it would be nice if you could try the artifacts of job 51600738[4].
> > > Again, download them (udev-hid-bpf_0.1.0-4-g5ab02ec.tar.xz), unpack,
> > > sudo ./install --verbose, then unplug/replug the artist Pro 24.
> >
> > Ok, the main change I experienced after installing is xsetwacom
> > listing the ID name of the device with twice the name Stylus on
> > "UGTABLET 24 inch PenDisplay Stylus stylus". Before it was only
> > "UGTABLET 24 inch PenDisplay stylus".
> >
> > $ xsetwacom --list
> > UGTABLET 24 inch PenDisplay Stylus stylus       id: 10  type: STYLUS
> >
> > Not a big deal, but I thought it was worth to mention it.
>
> Oh, this might be because I added a debug device. Given that there are
> 2 devices on the HID node, then one is tagged as Stylus by the kernel.
> Nothing to worry about.
>
> >
> > > Then, I'll need the following sequence (ideally repeated twice or
> > > three times, given that your last record show a slight difference in
> > > the first and second attempt):
> > >
> > > - outside of the proximity of the sensor, press the upper button
> > > - approach the stylus to the surface keeping the upper button pressed
> > > - touch the surface with the tip while holding the upper button pressed
> > > - release the upper button while keeping the tip pressed (like previously)
> > > - press once again the upper button while keeping the tip touching the
> > > surface (like previously)
> > > - lift of the pen, keeping the upper button pressed, and still in
> > > range of the sensor
> > > - remove the pen from the proximity of the sensor entirely (move away
> > > 20 cm or so), while still keeping the upper button pressed
> > >
> > > It's actually longer to describe than to execute :)
> > >
> >
> > Thank you for the detailed steps. True, it makes sens once
> > practising it. I made the gesture three time on:
> >
> > https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-24-Pro/2023-11-23_XPPEN-Artist-24-Pro_pen_tip-contact-and-press-release-upper-stylus-button-while-pressed-x3.txt
>
> Thanks a lot. And of course this device doesn't react in the way I expected :)
>
> Transitions from/to the tip touching the surface while the second
> button is pressed are normal, there are no extra events...
>
> But this also showed that the previous filter was better when pressing
> the upper button while touching the tip on the surface, because now we
> get another spurious event that was filtered before (and because it
> was filtered, I thought it was not there).
>
> Anyway, I couldn't rewrite the filter today, but I'll work on it next
> week for sure.


I've updated the HID-BPF filter, and you can find it in the latest pipeline[0].
I've removed the extra "Stylus" and you should have a better
experience with the upper button now.


>
>
> >
> > > But I would also totally understand that you had enough debugging and
> > > you would rather focus on using the tablets, instead of debugging
> > > them. In which case, someone else from the community might help me.
> >
> > No problem for continue testing, I'm around! Thank you again
> > for improving the behavior of the tablets.
> >

I think we are done with the XP-Pen Pro 24. But now I wonder if the
Pro 16 (gen2) doesn't also have those glitches.

Could you send me the same debug sequence as the last time
(transitions from/to touching the surface while holding the upper
button) but on the 16 now?

There is a chance I'll need the same filter to remove the extra left
click that might appear when you press the upper button while touching
the surface.

Cheers,
Benjamin


[0] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/52148274


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-29 15:32                                           ` Benjamin Tissoires
@ 2023-11-30 22:25                                             ` David Revoy
  2023-12-01 15:41                                               ` Benjamin Tissoires
  0 siblings, 1 reply; 40+ messages in thread
From: David Revoy @ 2023-11-30 22:25 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi Benjamin,

> I've updated the HID-BPF filter, and you can find it in the latest pipeline[0].
> I've removed the extra "Stylus" and you should have a better
> experience with the upper button now.
> [0] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/52148274

Thanks for the update!

> I think we are done with the XP-Pen Pro 24. But now I wonder if the
> Pro 16 (gen2) doesn't also have those glitches.
> Could you send me the same debug sequence as the last time
> (transitions from/to touching the surface while holding the upper
> button) but on the 16 now?

Sure, here is the same action, three time but now on the Pro 16 (Gen2): 
https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-16-Pro-Gen2/2023-11-30_XPPEN-Artist-16-Pro-Gen2_pen_tip-contact-and-press-release-upper-stylus-button-while-pressed-x3.txt

Have a good end of week,
David.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-02  7:44   ` Linux regression tracking #adding (Thorsten Leemhuis)
@ 2023-12-01  8:24     ` Linux regression tracking #update (Thorsten Leemhuis)
  0 siblings, 0 replies; 40+ messages in thread
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-12-01  8:24 UTC (permalink / raw)
  To: Jiri Kosina, David Revoy
  Cc: jason.gerecke, jose.exposito89, ilya.ostapyshyn, Nils Fuhler,
	Ping Cheng, Peter Hutterer, Benjamin Tissoires, linux-kernel,
	linux-input, Linux kernel regressions list

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 02.11.23 08:44, Linux regression tracking #adding (Thorsten Leemhuis)
wrote:
> On 01.11.23 20:37, Jiri Kosina wrote:
>> On Wed, 1 Nov 2023, David Revoy wrote:
>>
>>> I am emailing to draw your attention and expertise to a problem I had 
>>> earlier this week with my Xp-Pen Artist 24 Pro display tablet under 
>>> Fedora Linux 38 KDE after a kernel update.
>>>
>>> The second button on my stylus changed from a right-click (which I could 
>>> customise with xsetwacom or any GUI like kcm-tablet) to a button that 
>>> feels 'hardcoded' and now switches the whole device to an eraser mode. 
>>> This makes my main tool unusable.
> [...]
> Thanks for the report. To be sure the issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
> tracking bot:
> 
> #regzbot ^introduced 276e14e6c3
> https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help

This thread got long, but from briefly skimming it, it seems like the
regressions was dealt with somehow and everybody is happy. Please holler
if I got it wrong, as I hereby remove this from the tracking:

#regzbot resolve: solution found
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.



^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
  2023-11-30 22:25                                             ` David Revoy
@ 2023-12-01 15:41                                               ` Benjamin Tissoires
  0 siblings, 0 replies; 40+ messages in thread
From: Benjamin Tissoires @ 2023-12-01 15:41 UTC (permalink / raw)
  To: David Revoy
  Cc: José Expósito, Eric GOUYER, Illia Ostapyshyn, jkosina,
	jason.gerecke, linux-input, linux-kernel

Hi David,

On Thu, Nov 30, 2023 at 11:25 PM David Revoy <davidrevoy@protonmail.com> wrote:
>
> Hi Benjamin,
>
> > I've updated the HID-BPF filter, and you can find it in the latest pipeline[0].
> > I've removed the extra "Stylus" and you should have a better
> > experience with the upper button now.
> > [0] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/jobs/52148274
>
> Thanks for the update!
>
> > I think we are done with the XP-Pen Pro 24. But now I wonder if the
> > Pro 16 (gen2) doesn't also have those glitches.
> > Could you send me the same debug sequence as the last time
> > (transitions from/to touching the surface while holding the upper
> > button) but on the 16 now?
>
> Sure, here is the same action, three time but now on the Pro 16 (Gen2):
> https://www.peppercarrot.com/extras/mailing-list/hid-records/XPPEN-Artist-16-Pro-Gen2/2023-11-30_XPPEN-Artist-16-Pro-Gen2_pen_tip-contact-and-press-release-upper-stylus-button-while-pressed-x3.txt

Thanks a lot.

And the good news is that we get correct events all the time, so
nothing more to fix here. The current filter is doing a good job
already :)

>
> Have a good end of week,
> David.

Cheers,
Benjamin

>


^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2023-12-01 15:42 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <kRKTNDYigUSblpNgSuZ2H4dX03Of1yD4j_L9GgbyKXcDqZ67yh5HOQfcd7_83U3jZuQzxpKT3L6FXcRkkZIGdl_-PQF14oIB0QmRSfvpc2k=@protonmail.com>
2023-11-01 19:37 ` Requesting your attention and expertise regarding a Tablet/Kernel issue Jiri Kosina
2023-11-02  0:44   ` Bagas Sanjaya
2023-11-02  6:31     ` Linux regression tracking (Thorsten Leemhuis)
2023-11-02  7:51       ` Bagas Sanjaya
2023-11-02  7:44   ` Linux regression tracking #adding (Thorsten Leemhuis)
2023-12-01  8:24     ` Linux regression tracking #update (Thorsten Leemhuis)
2023-11-03 20:05   ` Illia Ostapyshyn
2023-11-04  0:46     ` Bagas Sanjaya
2023-11-06 13:17     ` David Revoy
2023-11-06 16:59       ` Benjamin Tissoires
2023-11-06 20:06         ` Illia Ostapyshyn
2023-11-07  7:59           ` Benjamin Tissoires
2023-11-07 13:40             ` Illia Ostapyshyn
2023-11-08  5:23             ` Eric GOUYER
2023-11-08  9:04               ` Benjamin Tissoires
2023-11-08  9:23                 ` José Expósito
2023-11-08  9:34                   ` Benjamin Tissoires
2023-11-08 18:21                     ` Benjamin Tissoires
2023-11-09  0:32                       ` David Revoy
2023-11-09 11:56                         ` Benjamin Tissoires
2023-11-09 16:13                           ` Benjamin Tissoires
2023-11-09 22:04                             ` David Revoy
2023-11-10 10:19                               ` Benjamin Tissoires
2023-11-11  8:52                               ` Benjamin Tissoires
2023-11-13 22:08                                 ` David Revoy
2023-11-14 14:35                                   ` Benjamin Tissoires
2023-11-15 15:14                                     ` Benjamin Tissoires
2023-11-23 22:12                                       ` David Revoy
2023-11-24 17:18                                         ` Benjamin Tissoires
2023-11-29 15:32                                           ` Benjamin Tissoires
2023-11-30 22:25                                             ` David Revoy
2023-12-01 15:41                                               ` Benjamin Tissoires
2023-11-08 19:40                 ` Nils Fuhler
2023-11-08 20:34                   ` Benjamin Tissoires
2023-11-08 22:31                     ` ostapyshyn
2023-11-09 11:46                       ` Benjamin Tissoires
2023-11-08 23:18             ` David Revoy
2023-11-09 11:47               ` Benjamin Tissoires
2023-11-11 17:15               ` Thorsten Leemhuis
2023-11-08 22:28         ` David Revoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).