Re: Requesting your attention and expertise regarding a Tablet/Kernel issue

From: Illia Ostapyshyn
Date: Tue Nov 07 2023 - 08:40:46 EST


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

Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> 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