[PATCH] Input: wacom - report hardware provided MT_TRACKING_IDs

[PATCH] Input: wacom - report hardware provided MT_TRACKING_IDs

[Ping Cheng]

The "Multi-touch (MT) Protocol" says:

"The slot protocol requires the use of the ABS_MT_TRACKING_ID, either
provided by the hardware or computed from the raw data".

We get the IDs from Bamboo touch device. Let's use them instead of
compute our own.

Signed-off-by: Ping Cheng <pingc@wacom.com>

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index b3252ef..76d4980 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -885,7 +885,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
 			input_report_abs(input, ABS_MT_POSITION_X, x);
 			input_report_abs(input, ABS_MT_POSITION_Y, y);
 			if (wacom->id[i] < 0)
-				wacom->id[i] = wacom->trk_id++ & MAX_TRACKING_ID;
+				wacom->id[i] = i;
 			if (!count++)
 				sp = p, sx = x, sy = y;
 		} else {
@@ -1283,7 +1283,7 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 					     0, features->pressure_max,
 					     features->pressure_fuzz, 0);
 			input_set_abs_params(input_dev, ABS_MT_TRACKING_ID, 0,
-					     MAX_TRACKING_ID, 0, 0);
+					     1, 0, 0);
 		} else if (features->device_type == BTN_TOOL_PEN) {
 			__set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
 			__set_bit(BTN_TOOL_PEN, input_dev->keybit);
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index 00ca015..b1310ec 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -42,9 +42,6 @@
 #define WACOM_QUIRK_MULTI_INPUT		0x0001
 #define WACOM_QUIRK_BBTOUCH_LOWRES	0x0002
 
-/* largest reported tracking id */
-#define MAX_TRACKING_ID			0xfff
-
 enum {
 	PENPARTNER = 0,
 	GRAPHIRE,
@@ -100,7 +97,6 @@ struct wacom_wac {
 	int id[3];
 	__u32 serial[2];
 	int last_finger;
-	int trk_id;
 	struct wacom_features features;
 	struct wacom_shared *shared;
 	struct input_dev *input;

Re: [PATCH] Input: wacom - report hardware provided MT_TRACKING_IDs

[Henrik Rydberg]

On 10/31/2010 05:00 AM, Ping Cheng wrote:

> The "Multi-touch (MT) Protocol" says:
> 
> "The slot protocol requires the use of the ABS_MT_TRACKING_ID, either
> provided by the hardware or computed from the raw data".
> 
> We get the IDs from Bamboo touch device. Let's use them instead of
> compute our own.
> 
> Signed-off-by: Ping Cheng <pingc@wacom.com>


Hi Ping,

I share the same concern as Dmitry here. This patch weakens the semantics of the
current tracking id definition. As it seems, only to allow the tracking id to be
used in a different, undocumented or hardware-specific way. This is not a good
way to go - neither from a kernel maintenance perspective, nor from a user
experience perspective. After all, the kernel is about unifying the experience
of different hardware.

> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index b3252ef..76d4980 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -885,7 +885,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
>  			input_report_abs(input, ABS_MT_POSITION_X, x);
>  			input_report_abs(input, ABS_MT_POSITION_Y, y);
>  			if (wacom->id[i] < 0)
> -				wacom->id[i] = wacom->trk_id++ & MAX_TRACKING_ID;
> +				wacom->id[i] = i;


This change will not allow for detection of a new finger between two frames, and
breaks applications that assume a new finger is detected by a new unique
tracking id. Besides, the internal contact ids are already passed on as slot
ids, so this change sends the same information in two different ways.

>  			if (!count++)
>  				sp = p, sx = x, sy = y;
>  		} else {
> @@ -1283,7 +1283,7 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>  					     0, features->pressure_max,
>  					     features->pressure_fuzz, 0);
>  			input_set_abs_params(input_dev, ABS_MT_TRACKING_ID, 0,
> -					     MAX_TRACKING_ID, 0, 0);
> +					     1, 0, 0);
>  		} else if (features->device_type == BTN_TOOL_PEN) {
>  			__set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
>  			__set_bit(BTN_TOOL_PEN, input_dev->keybit);
> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
> index 00ca015..b1310ec 100644
> --- a/drivers/input/tablet/wacom_wac.h
> +++ b/drivers/input/tablet/wacom_wac.h
> @@ -42,9 +42,6 @@
>  #define WACOM_QUIRK_MULTI_INPUT		0x0001
>  #define WACOM_QUIRK_BBTOUCH_LOWRES	0x0002
>  
> -/* largest reported tracking id */
> -#define MAX_TRACKING_ID			0xfff
> -
>  enum {
>  	PENPARTNER = 0,
>  	GRAPHIRE,
> @@ -100,7 +97,6 @@ struct wacom_wac {
>  	int id[3];
>  	__u32 serial[2];
>  	int last_finger;
> -	int trk_id;
>  	struct wacom_features features;
>  	struct wacom_shared *shared;
>  	struct input_dev *input;


Thanks,
Henrik

Re: [PATCH] Input: wacom - report hardware provided MT_TRACKING_IDs

[Ping Cheng]

On Mon, Nov 1, 2010 at 7:25 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> On 10/31/2010 05:00 AM, Ping Cheng wrote:
>
>> The "Multi-touch (MT) Protocol" says:
>>
>> "The slot protocol requires the use of the ABS_MT_TRACKING_ID, either
>> provided by the hardware or computed from the raw data".
>>
>> We get the IDs from Bamboo touch device. Let's use them instead of
>> compute our own.
>>
>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>
>
> Hi Ping,
>
> I share the same concern as Dmitry here. This patch weakens the semantics of the
> current tracking id definition.  As it seems, only to allow the tracking id to be
> used in a different, undocumented or hardware-specific way. This is not a good
> way to go - neither from a kernel maintenance perspective, nor from a user
> experience perspective. After all, the kernel is about unifying the experience
> of different hardware.

I think I got the point now. The tracking id is aimed at an unified
touch/finger tracking in the kernel. Not just to pass whatever
hardware provides.

I was stuck at the phrase of "provided by the hardware". With your
explanation, I see the rationale behind the use of "trk_id". This
discussion made it clear how I need to convince app developers. Please
ignore this patch.

I see a potential in merging type A into type B with the introduction
of trk_id. The type A drivers may need to tweak their x/y/..values a
bit to fit into the protocol. But the benefit of tracking ID should be
universal.

Trust me, this is all technical. Wacom does not even have a type A
device. It is not my job to care about type A.

>From a pure developer point of view, if our goal is to unify the
experience of different hardwares, merging type A to type B may not be
a bad idea. However, I do know there was issues preventing us from
merging them when we started in June. I bring it up now to see if
things has changed in our favor or not.

Thank you Henrik for your reply and comments. It helped me a lot. I
hope it does the same for others too.

Ping