[RFC PATCH] HID: wacom: Stop mangling tool IDs

[RFC PATCH] HID: wacom: Stop mangling tool IDs

[Gerecke, Jason]

From: Jason Gerecke <jason.gerecke@wacom.com>

In ancient times, an off-by-one-nybble error resulted in the Wacom
driver sending "mangled" tool IDs to userspace. This mangling behavior
was later enshrined into a function so that devices using the then-new
generic codepath could share the same broken behavior. The mangled IDs
have not historically been a major problem for userspace since few
applications care about the exact numeric value of the IDs. As long as
the IDs were unique it didn't much matter. Some programs (cross-
platform and remote-desktop applications in particular) /do/ rely on
the IDs being correct, however.

This patch rids the driver of the mangling behavior.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2")
References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs")
---
I'd like to get the opinion of the kernel maintainers on making a
change like this at some point in the future. There are _very_ few
userspace uses of these IDs (primarily: drivers, compositors, and
tablet control panels) and my plan is to update those bits and then
give some time for the changes to roll out to users before re-
submitting this for real. I don't expect any kind of breakage since
we'll be taking our time with the rollout and userspace needs to
have handling for "unknown" IDs anyway (since Wacom periodically
releases new pens).
---
 drivers/hid/wacom_wac.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2541fa2e0fa3b..facab5bd37e41 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -671,11 +671,6 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
 	return 1;
 }
 
-static int wacom_intuos_id_mangle(int tool_id)
-{
-	return (tool_id & ~0xFFF) << 4 | (tool_id & 0xFFF);
-}
-
 static bool wacom_is_art_pen(int tool_id)
 {
 	bool is_art_pen = false;
@@ -1010,8 +1005,7 @@ static int wacom_intuos_general(struct wacom_wac *wacom)
 		break;
 	}
 
-	input_report_abs(input, ABS_MISC,
-			 wacom_intuos_id_mangle(wacom->id[idx])); /* report tool id */
+	input_report_abs(input, ABS_MISC, wacom->id[idx]); /* report tool id */
 	input_report_key(input, wacom->tool[idx], 1);
 	input_event(input, EV_MSC, MSC_SERIAL, wacom->serial[idx]);
 	wacom->reporting_data = true;
@@ -1379,8 +1373,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
 
 			input_report_key(pen_input, wacom->tool[0], prox);
 			input_event(pen_input, EV_MSC, MSC_SERIAL, wacom->serial[0]);
-			input_report_abs(pen_input, ABS_MISC,
-					 wacom_intuos_id_mangle(wacom->id[0])); /* report tool id */
+			input_report_abs(pen_input, ABS_MISC, wacom->id[0]); /* report tool id */
 		}
 
 		wacom->shared->stylus_in_proximity = prox;
@@ -2514,13 +2507,6 @@ static void wacom_wac_pen_report(struct hid_device *hdev,
 		input_report_key(input, BTN_STYLUS2, wacom_wac->hid_data.barrelswitch2);
 		input_report_key(input, BTN_STYLUS3, wacom_wac->hid_data.barrelswitch3);
 
-		/*
-		 * Non-USI EMR tools should have their IDs mangled to
-		 * match the legacy behavior of wacom_intuos_general
-		 */
-		if (wacom_wac->serial[0] >> 52 == 1)
-			id = wacom_intuos_id_mangle(id);
-
 		/*
 		 * To ensure compatibility with xf86-input-wacom, we should
 		 * report the BTN_TOOL_* event prior to the ABS_MISC or
-- 
2.46.0

Re: [RFC PATCH] HID: wacom: Stop mangling tool IDs

[Dmitry Torokhov]

On Tue, Sep 03, 2024 at 11:26:33AM -0700, Gerecke, Jason wrote:
> From: Jason Gerecke <jason.gerecke@wacom.com>
> 
> In ancient times, an off-by-one-nybble error resulted in the Wacom
> driver sending "mangled" tool IDs to userspace. This mangling behavior
> was later enshrined into a function so that devices using the then-new
> generic codepath could share the same broken behavior. The mangled IDs
> have not historically been a major problem for userspace since few
> applications care about the exact numeric value of the IDs. As long as
> the IDs were unique it didn't much matter. Some programs (cross-
> platform and remote-desktop applications in particular) /do/ rely on
> the IDs being correct, however.
> 
> This patch rids the driver of the mangling behavior.
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2")
> References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs")
> ---
> I'd like to get the opinion of the kernel maintainers on making a
> change like this at some point in the future. There are _very_ few
> userspace uses of these IDs (primarily: drivers, compositors, and
> tablet control panels) and my plan is to update those bits and then
> give some time for the changes to roll out to users before re-
> submitting this for real. I don't expect any kind of breakage since
> we'll be taking our time with the rollout and userspace needs to
> have handling for "unknown" IDs anyway (since Wacom periodically
> releases new pens).

I think if you take care of users of this data (it is Wacom-specific
anyway because ABS_MISC does not have a defined behavior) I'd be fine
changing the kernel.

Up to Jiri/Benjamin though.

Thanks.

-- 
Dmitry

Re: [RFC PATCH] HID: wacom: Stop mangling tool IDs

[Peter Hutterer]

On Tue, Sep 03, 2024 at 11:26:33AM -0700, Gerecke, Jason wrote:
> From: Jason Gerecke <jason.gerecke@wacom.com>
> 
> In ancient times, an off-by-one-nybble error resulted in the Wacom
> driver sending "mangled" tool IDs to userspace. This mangling behavior
> was later enshrined into a function so that devices using the then-new
> generic codepath could share the same broken behavior. The mangled IDs
> have not historically been a major problem for userspace since few
> applications care about the exact numeric value of the IDs. As long as
> the IDs were unique it didn't much matter. Some programs (cross-
> platform and remote-desktop applications in particular) /do/ rely on
> the IDs being correct, however.
> 
> This patch rids the driver of the mangling behavior.
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2")
> References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs")
> ---
> I'd like to get the opinion of the kernel maintainers on making a
> change like this at some point in the future. There are _very_ few
> userspace uses of these IDs (primarily: drivers, compositors, and
> tablet control panels) and my plan is to update those bits and then
> give some time for the changes to roll out to users before re-
> submitting this for real. I don't expect any kind of breakage since
> we'll be taking our time with the rollout and userspace needs to
> have handling for "unknown" IDs anyway (since Wacom periodically
> releases new pens).

Just to show my support :)
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>

Cheers,
  Peter

Re: [RFC PATCH] HID: wacom: Stop mangling tool IDs

[Jiri Kosina]

On Tue, 3 Sep 2024, Gerecke, Jason wrote:

> From: Jason Gerecke <jason.gerecke@wacom.com>
> 
> In ancient times, an off-by-one-nybble error resulted in the Wacom
> driver sending "mangled" tool IDs to userspace. This mangling behavior
> was later enshrined into a function so that devices using the then-new
> generic codepath could share the same broken behavior. The mangled IDs
> have not historically been a major problem for userspace since few
> applications care about the exact numeric value of the IDs. As long as
> the IDs were unique it didn't much matter. Some programs (cross-
> platform and remote-desktop applications in particular) /do/ rely on
> the IDs being correct, however.
> 
> This patch rids the driver of the mangling behavior.
> 
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2")
> References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs")
> ---
> I'd like to get the opinion of the kernel maintainers on making a
> change like this at some point in the future. There are _very_ few
> userspace uses of these IDs (primarily: drivers, compositors, and
> tablet control panels) and my plan is to update those bits and then
> give some time for the changes to roll out to users before re-
> submitting this for real. I don't expect any kind of breakage since
> we'll be taking our time with the rollout and userspace needs to
> have handling for "unknown" IDs anyway (since Wacom periodically
> releases new pens).

As you are in control of the whole ecosystem anyway (because it's Wacom 
specific), I don't see it as particularly problematic.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [RFC PATCH] HID: wacom: Stop mangling tool IDs

[Benjamin Tissoires]

On Sep 05 2024, Jiri Kosina wrote:
> On Tue, 3 Sep 2024, Gerecke, Jason wrote:
> 
> > From: Jason Gerecke <jason.gerecke@wacom.com>
> > 
> > In ancient times, an off-by-one-nybble error resulted in the Wacom
> > driver sending "mangled" tool IDs to userspace. This mangling behavior
> > was later enshrined into a function so that devices using the then-new
> > generic codepath could share the same broken behavior. The mangled IDs
> > have not historically been a major problem for userspace since few
> > applications care about the exact numeric value of the IDs. As long as
> > the IDs were unique it didn't much matter. Some programs (cross-
> > platform and remote-desktop applications in particular) /do/ rely on
> > the IDs being correct, however.
> > 
> > This patch rids the driver of the mangling behavior.
> > 
> > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> > References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2")
> > References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs")
> > ---
> > I'd like to get the opinion of the kernel maintainers on making a
> > change like this at some point in the future. There are _very_ few
> > userspace uses of these IDs (primarily: drivers, compositors, and
> > tablet control panels) and my plan is to update those bits and then
> > give some time for the changes to roll out to users before re-
> > submitting this for real. I don't expect any kind of breakage since
> > we'll be taking our time with the rollout and userspace needs to
> > have handling for "unknown" IDs anyway (since Wacom periodically
> > releases new pens).
> 
> As you are in control of the whole ecosystem anyway (because it's Wacom 
> specific), I don't see it as particularly problematic.
> 

Yeah, same here. And that change doesn't impact the basic functionality
of the styluses: they'll still be handled properly as pointers, but only
the configuration panel IIUC. Peter mentioned about some slight
differences in libinput which shouldn't impact the users as well IIRC.

Anyway, that's an OK from me too.

Cheers,
Benjamin

Re: [RFC PATCH] HID: wacom: Stop mangling tool IDs

[Peter Hutterer]

On Thu, Sep 05, 2024 at 11:01:58AM +0200, Benjamin Tissoires wrote:
> On Sep 05 2024, Jiri Kosina wrote:
> > On Tue, 3 Sep 2024, Gerecke, Jason wrote:
> > 
> > > From: Jason Gerecke <jason.gerecke@wacom.com>
> > > 
> > > In ancient times, an off-by-one-nybble error resulted in the Wacom
> > > driver sending "mangled" tool IDs to userspace. This mangling behavior
> > > was later enshrined into a function so that devices using the then-new
> > > generic codepath could share the same broken behavior. The mangled IDs
> > > have not historically been a major problem for userspace since few
> > > applications care about the exact numeric value of the IDs. As long as
> > > the IDs were unique it didn't much matter. Some programs (cross-
> > > platform and remote-desktop applications in particular) /do/ rely on
> > > the IDs being correct, however.
> > > 
> > > This patch rids the driver of the mangling behavior.
> > > 
> > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> > > References: 493630b20389 ("Input: wacom - fix serial number handling on Cintiq 21UX2")
> > > References: 82527da319ee ("HID: wacom: Read and internally use corrected Intuos tool IDs")
> > > ---
> > > I'd like to get the opinion of the kernel maintainers on making a
> > > change like this at some point in the future. There are _very_ few
> > > userspace uses of these IDs (primarily: drivers, compositors, and
> > > tablet control panels) and my plan is to update those bits and then
> > > give some time for the changes to roll out to users before re-
> > > submitting this for real. I don't expect any kind of breakage since
> > > we'll be taking our time with the rollout and userspace needs to
> > > have handling for "unknown" IDs anyway (since Wacom periodically
> > > releases new pens).
> > 
> > As you are in control of the whole ecosystem anyway (because it's Wacom 
> > specific), I don't see it as particularly problematic.
> > 
> 
> Yeah, same here. And that change doesn't impact the basic functionality
> of the styluses: they'll still be handled properly as pointers, but only
> the configuration panel IIUC. Peter mentioned about some slight
> differences in libinput which shouldn't impact the users as well IIRC.

Assuming libwacom is updated (and used) before this happens in the
kernel, then there won't be a difference in libinput behaviour at all.
And that's the plan for this anyway, have libwacom updated well before
the kernel update ships.

Otherwise, the only difference in libinput will be that it is treated as
an unknown stylus. Those default to capabilities matching the kernel
evdev codes (i.e. "everything") whereas a known stylus will only
initialize capabilities that the stylus actually has. IOW you may get a
libinput_tablet_tool that e.g. claims to support rotation and has a
wheel when it doesn't. Not the end of the world either.

Cheers,
  Peter