[PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Rafi Rubin]

Added a quirk to enable distinct input devices.  The digitizer utilizes
three inputs to represent pen, multitouch and a normal touch screen.

With the Pen partitioned, it behaves well and does not need special
handling.

Also, I set names to the input devices to clarify the functions of the
various inputs.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |   46 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 49ce69d..4bb5ed0 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -42,6 +42,10 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	/* No special mappings needed for the pen */
+	if(field->application == HID_DG_PEN)
+		return 0;
+
 	switch (usage->hid & HID_USAGE_PAGE) {
 
 	case HID_UP_GENDESK:
@@ -104,6 +108,9 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	/* No special mappings needed for the pen */
+	if(field->application == HID_DG_PEN)
+		return 0;
 	if (usage->type == EV_KEY || usage->type == EV_REL
 			|| usage->type == EV_ABS)
 		clear_bit(usage->code, *bit);
@@ -123,6 +130,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 	struct input_dev *input = field->hidinput->input;
 	struct ntrig_data *nd = hid_get_drvdata(hid);
 
+	/* No special handling needed for the pen */
+	if(field->application == HID_DG_PEN)
+		return 0;
+
         if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
 
@@ -241,6 +252,11 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
 	struct ntrig_data *nd;
+	struct hid_input *hidinput;
+	struct input_dev *input;
+
+	if (id->driver_data)
+		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
 
 	nd = kmalloc(sizeof(struct ntrig_data), GFP_KERNEL);
 	if (!nd) {
@@ -255,14 +271,36 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (!ret)
 		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 
-	if (ret)
+	if (ret) {
 		kfree (nd);
+		return ret;
+	}
+
+	list_for_each_entry(hidinput, &hdev->inputs, list) {
+		input = hidinput->input;
+		switch (hidinput->report->field[0]->application) {
+		case HID_DG_PEN:
+			input->name = "N-Trig Pen";
+			break;
+		case HID_DG_TOUCHSCREEN:
+			/* 
+			 * The physical touchscreen (single touch) input
+			 * has a value for physical, whereas the multitouch
+			 * only has logical input fields.
+			 */
+			if(hidinput->report->field[0]->physical) {
+				input->name = "N-Trig Touchscreen";
+			} else {
+				input->name = "N-Trig MultiTouch";
+			}
+			break;
+		}
+	}
 
 	return ret;
 }
 
-static void ntrig_remove(struct hid_device *hdev)
-{
+static void ntrig_remove(struct hid_device *hdev) {
 	hid_hw_stop(hdev);
 	kfree(hid_get_drvdata(hdev));
 }
@@ -276,7 +314,7 @@ MODULE_DEVICE_TABLE(hid, ntrig_devices);
 
 static const struct hid_usage_id ntrig_grabbed_usages[] = {
 	{ HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
-	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
+	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1 }
 };
 
 static struct hid_driver ntrig_driver = {
-- 
1.6.6.1

[PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching

[Rafi Rubin]

With the pen and touch split apart, we no longer need to inject
additional tool switching events.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |   41 +----------------------------------------
 1 files changed, 1 insertions(+), 40 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 4bb5ed0..6464d15 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -27,9 +27,6 @@
 struct ntrig_data {
 	__s32 x, y, id, w, h;
 	char reading_a_point, found_contact_id;
-	char pen_active;
-	char finger_active;
-	char inverted;
 };
 
 /*
@@ -47,7 +44,6 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		return 0;
 
 	switch (usage->hid & HID_USAGE_PAGE) {
-
 	case HID_UP_GENDESK:
 		switch (usage->hid) {
 		case HID_GD_X:
@@ -111,6 +107,7 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 	/* No special mappings needed for the pen */
 	if(field->application == HID_DG_PEN)
 		return 0;
+
 	if (usage->type == EV_KEY || usage->type == EV_REL
 			|| usage->type == EV_ABS)
 		clear_bit(usage->code, *bit);
@@ -136,18 +133,6 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
         if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
-
-		case HID_DG_INRANGE:
-			if (field->application & 0x3)
-				nd->pen_active = (value != 0);
-			else
-				nd->finger_active = (value != 0);
-			return 0;
-
-		case HID_DG_INVERT:
-			nd->inverted = value;
-			return 0;
-
 		case HID_GD_X:
 			nd->x = value;
 			nd->reading_a_point = 1;
@@ -171,32 +156,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * to emit a normal (X, Y) position
 			 */
 			if (!nd->found_contact_id) {
-				if (nd->pen_active && nd->finger_active) {
-					input_report_key(input, BTN_TOOL_DOUBLETAP, 0);
-					input_report_key(input, BTN_TOOL_DOUBLETAP, 1);
-				}
-				input_event(input, EV_ABS, ABS_X, nd->x);
-				input_event(input, EV_ABS, ABS_Y, nd->y);
-			}
-			break;
-		case HID_DG_TIPPRESSURE:
-			/*
-			 * when in single touch mode, this is the last
-			 * report received in a pen event. We want
-			 * to emit a normal (X, Y) position
-			 */
-			if (! nd->found_contact_id) {
-				if (nd->pen_active && nd->finger_active) {
-					input_report_key(input,
-							nd->inverted ? BTN_TOOL_RUBBER : BTN_TOOL_PEN
-							, 0);
-					input_report_key(input,
-							nd->inverted ? BTN_TOOL_RUBBER : BTN_TOOL_PEN
-							, 1);
-				}
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
-				input_event(input, EV_ABS, ABS_PRESSURE, value);
 			}
 			break;
 		case 0xff000002:
-- 
1.6.6.1

[PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Rafi Rubin]

This cleans up the identification of multitouch groups.  When two touch
input devices are available, single and multi touch streams are
partitioned to the appropriate device.

Added triple and quad tap to the single touch device for the benefit of
tools that recognize different tap types but do not have full
multi touch support.

Capture a full multitouch group before emitting events.  This allows for
additional processing, though takes little advantage of that capability.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |  231 +++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 195 insertions(+), 36 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 6464d15..9f46a3d 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -20,13 +20,56 @@
 #include "hid-ids.h"
 
 #define NTRIG_DUPLICATE_USAGES	0x001
+#define NTRIG_MAX_CONTACTS	7
 
 #define nt_map_key_clear(c)	hid_map_usage_clear(hi, usage, bit, max, \
 					EV_KEY, (c))
 
+/*
+ * The relavent data needed to store events from a contact to
+ * delay transmittion.
+ */
+struct ntrig_contact {
+	char active;
+	__s8  logical_id;
+	__u16 x, y;
+	__u8 confidence;
+
+	/* height and width transformed */
+	char orientation;
+	__u16 touch_major;
+	__u16 touch_minor;
+};
+
+
 struct ntrig_data {
-	__s32 x, y, id, w, h;
-	char reading_a_point, found_contact_id;
+	/* Incoming raw values for a single contact */
+	__u16 x, y, w, h;
+	__u16 id;
+	__u8 confidence;
+
+	int max_width;
+	int max_height;
+	int max_contacts;
+
+	unsigned char reading_mt;
+
+	/* Collected state for 2 full sets of contacts */
+	struct ntrig_contact contacts[NTRIG_MAX_CONTACTS];
+	__u8 contact_count;
+
+	__u8 mt_footer[4];
+	__u8 mt_foot_count;
+
+	/* options */
+	unsigned char emit_ghosts;
+
+	/*
+	 * Keep a pointer to the single touch input device to enable
+	 * the multitouch handler to redirect the older style single
+	 * touch events.
+	 */
+	struct input_dev * st_input;
 };
 
 /*
@@ -39,8 +82,8 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen */
-	if(field->application == HID_DG_PEN)
+	/* No special mappings needed for the pen and single touch */
+	if(field->physical)
 		return 0;
 
 	switch (usage->hid & HID_USAGE_PAGE) {
@@ -104,8 +147,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen */
-	if(field->application == HID_DG_PEN)
+	/* No special mappings needed for the pen and single touch */
+	if(field->physical)
 		return 0;
 
 	if (usage->type == EV_KEY || usage->type == EV_REL
@@ -115,6 +158,73 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 	return 0;
 }
 
+static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
+{
+	int i;
+	struct ntrig_contact *contact = &nd->contacts[0];
+
+	/* Emit single touch events */
+	if(contact->confidence) {
+		switch (nd->contact_count) {
+			case 0:	/* for single touch devices */
+			case 1:
+				input_report_key(nd->st_input,
+						BTN_TOOL_DOUBLETAP, 1);
+				break;
+			case 2:
+				input_report_key(nd->st_input,
+						BTN_TOOL_TRIPLETAP, 1);
+				break;
+			case 3:
+			default:
+				input_report_key(nd->st_input,
+						BTN_TOOL_QUADTAP, 1);
+		}
+		input_report_key(nd->st_input, BTN_TOUCH, 1);
+		input_event(nd->st_input, EV_ABS, ABS_X, contact->x);
+		input_event(nd->st_input, EV_ABS, ABS_Y, contact->y);
+		input_sync(nd->st_input);
+	} else {
+		input_report_key(nd->st_input, BTN_TOUCH, 0);
+		input_report_key(nd->st_input, BTN_TOOL_DOUBLETAP, 0);
+		input_report_key(nd->st_input, BTN_TOOL_TRIPLETAP, 0);
+		input_report_key(nd->st_input, BTN_TOOL_QUADTAP, 0);
+		input_sync(nd->st_input);
+	}
+
+	/* Multitouch doesn't need to send an end of activity notice. */
+	if(!(nd->contact_count && nd->contacts[0].confidence))
+		return;
+
+	/* Emit multitouch events */
+	for (i = 0; i <= nd->max_contacts && i < NTRIG_MAX_CONTACTS; i++) {
+		if (nd->contacts[i].confidence) {
+			struct ntrig_contact *contact = &nd->contacts[i];
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
+			input_event(input, EV_ABS, ABS_MT_POSITION_X,
+					contact->x);
+			input_event(input, EV_ABS, ABS_MT_POSITION_Y,
+					contact->y);
+			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
+					contact->orientation);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR,
+					contact->touch_major);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR,
+					contact->touch_minor);
+			input_mt_sync(input);
+		} else if (nd->emit_ghosts) {
+			/* emit filler points if so desired */
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
+			input_event(input, EV_ABS, ABS_MT_POSITION_X, 0);
+			input_event(input, EV_ABS, ABS_MT_POSITION_Y, 0);
+			input_event(input, EV_ABS, ABS_MT_ORIENTATION, 0);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
+			input_mt_sync(input);
+		}
+	}
+}
+
 /*
  * this function is called upon all reports
  * so that we can filter contact point information,
@@ -133,17 +243,23 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
         if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
+		case 0xff000001:
+			/* Tag indicating the start of a multitouch group */
+			nd->reading_mt = 1;
+			break;
+		case HID_DG_CONFIDENCE:
+			nd->confidence = value;
+			break;
 		case HID_GD_X:
 			nd->x = value;
-			nd->reading_a_point = 1;
+			/* Clear the contact footer */
+			nd->mt_foot_count = 0;
 			break;
 		case HID_GD_Y:
 			nd->y = value;
 			break;
 		case HID_DG_CONTACTID:
 			nd->id = value;
-			/* we receive this only when in multitouch mode */
-			nd->found_contact_id = 1;
 			break;
 		case HID_DG_WIDTH:
 			nd->w = value;
@@ -155,7 +271,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * report received in a finger event. We want
 			 * to emit a normal (X, Y) position
 			 */
-			if (!nd->found_contact_id) {
+			if (!nd->reading_mt) {
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -167,33 +283,58 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * this usage tells if the contact point is real
 			 * or a placeholder
 			 */
-			if (!nd->reading_a_point || value != 1)
+
+			/* Shouldn't get more than 4 footer packets, so skip */
+			if (nd->mt_foot_count >= 4)
+				break;
+
+			nd->mt_footer[nd->mt_foot_count++] = value;
+
+			/* if the footer isn't complete break */
+			if (nd->mt_foot_count != 4)
+				break;
+
+			/* Pen activity signal, trigger end of touch. */
+			if (nd->mt_footer[2]) {
+				nd->contacts[nd->id].x = 0;
+				nd->contacts[nd->id].y = 0;
+				nd->contacts[nd->id].confidence = 0;
 				break;
-			/* emit a normal (X, Y) for the first point only */
-			if (nd->id == 0) {
-				input_event(input, EV_ABS, ABS_X, nd->x);
-				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
-			input_event(input, EV_ABS, ABS_MT_POSITION_X, nd->x);
-			input_event(input, EV_ABS, ABS_MT_POSITION_Y, nd->y);
+
+			/* If the contact was invalid */
+			if (!(nd->confidence && nd->mt_footer[0])
+					|| nd->w <= 250
+					|| nd->h <= 190) {
+				nd->contacts[nd->id].x = 0;
+				nd->contacts[nd->id].y = 0;
+				nd->contacts[nd->id].confidence = 0;
+				break;
+			}
+
+			nd->contacts[nd->id].logical_id = -1;
+			nd->contacts[nd->id].confidence = nd->confidence;
+			nd->contacts[nd->id].x = nd->x;
+			nd->contacts[nd->id].y = nd->y;
+
 			if (nd->w > nd->h) {
-				input_event(input, EV_ABS,
-						ABS_MT_ORIENTATION, 1);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MAJOR, nd->w);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MINOR, nd->h);
+				nd->contacts[nd->id].orientation = 1;
+				nd->contacts[nd->id].touch_major = nd->w;
+				nd->contacts[nd->id].touch_minor = nd->h;
 			} else {
-				input_event(input, EV_ABS,
-						ABS_MT_ORIENTATION, 0);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MAJOR, nd->h);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MINOR, nd->w);
+				nd->contacts[nd->id].orientation = 0;
+				nd->contacts[nd->id].touch_major = nd->h;
+				nd->contacts[nd->id].touch_minor = nd->w;
+			}
+			break;
+
+		case HID_DG_CONTACTCOUNT:
+			/* This marks the end of the multitouch group */
+			nd->contact_count = value;
+			if(nd->reading_mt) {
+				nd->reading_mt = 0;
+				ntrig_conclude_mt(input, nd);
 			}
-			input_mt_sync(field->hidinput->input);
-			nd->reading_a_point = 0;
-			nd->found_contact_id = 0;
 			break;
 
 		default:
@@ -203,8 +344,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 	}
 
 	/* we have handled the hidinput part, now remains hiddev */
-        if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
-                hid->hiddev_hid_event(hid, field, usage, value);
+	if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
+		hid->hiddev_hid_event(hid, field, usage, value);
 
 	return 1;
 }
@@ -224,8 +365,16 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		dev_err(&hdev->dev, "cannot allocate N-Trig data\n");
 		return -ENOMEM;
 	}
-	nd->reading_a_point = 0;
-	nd->found_contact_id = 0;
+	
+	/* Initialize the driver data to sane values */
+	nd->id = 0;
+	nd->reading_mt = 0;
+	nd->contact_count = 0;
+	nd->max_width = 0x500;
+	nd->max_height = 0x500;
+	nd->max_contacts = 5;
+	nd->emit_ghosts = 0;
+
 	hid_set_drvdata(hdev, nd);
 
 	ret = hid_parse(hdev);
@@ -251,6 +400,15 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			 */
 			if(hidinput->report->field[0]->physical) {
 				input->name = "N-Trig Touchscreen";
+				nd->st_input = input;
+
+				/*
+				 * A little something special to enable
+				 * two and three finger taps.
+				 */ 
+				set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
+				set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
+				set_bit(BTN_TOOL_QUADTAP, input->keybit);
 			} else {
 				input->name = "N-Trig MultiTouch";
 			}
@@ -271,6 +429,7 @@ static const struct hid_device_id ntrig_devices[] = {
 		.driver_data = NTRIG_DUPLICATE_USAGES },
 	{ }
 };
+
 MODULE_DEVICE_TABLE(hid, ntrig_devices);
 
 static const struct hid_usage_id ntrig_grabbed_usages[] = {
-- 
1.6.6.1

[PATCH 4/4] hid-ntrig: Contact tracking

[Rafi Rubin]

Added contact tracking to compensate for the lack of tracking in
hardware.  The approach is simplistic and leaves considerable
room for improvement.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |  222 +++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 185 insertions(+), 37 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 9f46a3d..fde8ac0 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -56,7 +56,9 @@ struct ntrig_data {
 
 	/* Collected state for 2 full sets of contacts */
 	struct ntrig_contact contacts[NTRIG_MAX_CONTACTS];
+	struct ntrig_contact prev_contacts[NTRIG_MAX_CONTACTS];
 	__u8 contact_count;
+	__u8 prev_contact_count;
 
 	__u8 mt_footer[4];
 	__u8 mt_foot_count;
@@ -109,18 +111,16 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_DIGITIZER:
 		switch (usage->hid) {
 		/* we do not want to map these for now */
-		case HID_DG_CONTACTID: /* value is useless */
 		case HID_DG_INPUTMODE:
 		case HID_DG_DEVICEINDEX:
 		case HID_DG_CONTACTCOUNT:
 		case HID_DG_CONTACTMAX:
 			return -1;
 
-		/* original mapping by Rafi Rubin */
-		case HID_DG_CONFIDENCE:
-			nt_map_key_clear(BTN_TOOL_DOUBLETAP);
+		case HID_DG_CONTACTID:
+			hid_map_usage(hi, usage, bit, max, EV_ABS,
+					ABS_MT_TRACKING_ID);
 			return 1;
-
 		/* width/height mapped on TouchMajor/TouchMinor/Orientation */
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
@@ -158,48 +158,190 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 	return 0;
 }
 
+static void ntrig_single_touch_emit(struct input_dev *input,
+				    struct ntrig_data *nd)
+{
+	if (nd->confidence) {
+		switch (nd->contact_count) {
+		case 0:	/* for single touch devices */
+		case 1:
+			input_report_key(input, BTN_TOOL_DOUBLETAP, 1);
+			break;
+		case 2:
+			input_report_key(input, BTN_TOOL_TRIPLETAP, 1);
+			break;
+		case 3:
+		default:
+			input_report_key(input, BTN_TOOL_QUADTAP, 1);
+		}
+		input_report_key(input, BTN_TOUCH, 1);
+		input_event(input, EV_ABS, ABS_X, nd->x);
+		input_event(input, EV_ABS, ABS_Y, nd->y);
+	} else {
+		/* No active fingers, clear all state */
+		input_report_key(input, BTN_TOUCH, 0);
+		input_report_key(input, BTN_TOOL_DOUBLETAP, 0);
+		input_report_key(input, BTN_TOOL_TRIPLETAP, 0);
+		input_report_key(input, BTN_TOOL_QUADTAP, 0);
+	}
+	input_sync(input);
+}
+/* 
+ * Spatial comparison of two points.  If the difference
+ * is within the given thresholds they are treated as the
+ * same point.
+ */
+#define nt_same_point(a, b, max_dx, max_dy) ( \
+		(abs(a.x - b.x) <= max_dx) && \
+		(abs(a.y - b.y) <= max_dy))
+
+/*
+ * To verify a new contact matches a contact in the previous
+ * group, ensure both are valid then check spatial correlation.
+ */
+#define nt_match_points(nd, new, old) (nd->contacts[new].confidence && \
+		nd->prev_contacts[old].confidence && \
+		nt_same_point(nd->contacts[new], nd->prev_contacts[old], \
+			nd->max_width, nd->max_height))
+
+/*
+ * After an older contact is identified as a match nt_map_match updates
+ * the newer point as well as the contact map
+ */
+#define nt_map_match(nd, contact_map, new, old) \
+	nd->contacts[new].logical_id = nd->prev_contacts[old].logical_id; \
+	contact_map[nd->contacts[new].logical_id] = new;
+
 static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
 {
-	int i;
-	struct ntrig_contact *contact = &nd->contacts[0];
+	__s8 contact_map[nd->max_contacts];
+	int i, j, k;
+	int matched = 0;
+	int first_free_id = 0;
+	int count = nd->contact_count;
+	int prev_count = nd->prev_contact_count;
+	struct ntrig_contact *contact;
+
+	/* If the previous state is corrupted, discard it. */
+	if (nd->prev_contact_count >= NTRIG_MAX_CONTACTS) {
+		printk(KERN_ERR
+		       "N-Trig previous state corrupted, discarding\n");
+		nd->prev_contact_count = 0;
+		prev_count = 0;
+	}
 
-	/* Emit single touch events */
-	if(contact->confidence) {
-		switch (nd->contact_count) {
-			case 0:	/* for single touch devices */
-			case 1:
-				input_report_key(nd->st_input,
-						BTN_TOOL_DOUBLETAP, 1);
-				break;
-			case 2:
-				input_report_key(nd->st_input,
-						BTN_TOOL_TRIPLETAP, 1);
+	/* Under some circumstances an empty group is emitted with an invalid
+	 * contact 0 and contact count of 1. */
+	if (count && (!nd->contacts[0].confidence)) {
+		count = 0;
+		nd->contact_count = 0;
+	}
+
+	/* Contact tracking logic */
+
+	/* Initialize and empty logical id map */
+	for (i = 0; i < nd->max_contacts; i++) {
+		contact_map[i] = -1;
+	}
+
+	/* 
+	 * Phase 1: Identify which contacts seem to match
+	 * those with the same physical id from the previous group.
+	 * This should be the most common case during long touch
+	 * action. */
+	for (i = 0; i < count && i < prev_count; i++) {
+		if (nt_match_points(nd, i, i)) {
+			nt_map_match(nd, contact_map, i, i);
+			matched++;
+		} else
+			nd->contacts[i].logical_id = -1;
+	}
+
+	/* 
+	 * Phase 2: Find corresponding contacts when the incoming
+	 * order has changed.
+	 */
+	for (i = 0; i < count && matched < count; i++) {
+		for (j = 0; j < prev_count &&
+		     (nd->contacts[i].logical_id < 0); j++) {
+
+			/* Check the map to avoid reusing an old contact
+			 * for multiple current contacts */
+			if ((contact_map[nd->prev_contacts[j].logical_id] < 0)
+			    && nt_match_points(nd, i, j)) {
+				nt_map_match(nd, contact_map, i, j);
+				matched++;
+			}
+		}
+	}
+
+	/*
+	 * Phase 3: New or unidentied contacts are assigned logical ids.
+	 */
+	for (i = 0; i < count && matched < count; i++) {
+		/* Ignore points that are already mapped */
+		if ((nd->contacts[i].confidence
+		     && nd->contacts[i].logical_id < 0)) {
+			/* find the first available logical id */
+			while (contact_map[first_free_id] >= 0
+			       && first_free_id < nd->max_contacts)
+				first_free_id++;
+			if (first_free_id >= nd->max_contacts) {
+				printk(KERN_ERR
+				       "hid-ntrig: exceeded contacts limit\n");
 				break;
-			case 3:
-			default:
-				input_report_key(nd->st_input,
-						BTN_TOOL_QUADTAP, 1);
+			}
+			nd->contacts[i].logical_id = first_free_id;
+			contact_map[first_free_id++] = i;
+			matched++;
 		}
-		input_report_key(nd->st_input, BTN_TOUCH, 1);
-		input_event(nd->st_input, EV_ABS, ABS_X, contact->x);
-		input_event(nd->st_input, EV_ABS, ABS_Y, contact->y);
-		input_sync(nd->st_input);
+	}
+
+	k = -1;			/* Lowest id contact */
+	j = -1;			/* Highest id contact */
+	if(nd->emit_ghosts < 2) {
+		for (i = 0; i < nd->max_contacts; i++)
+			if (contact_map[i] >= 0) {
+				j = i;
+				if (k < 0)
+					k = i;
+			}
+	} else {
+		j = nd->max_contacts-1;
+		for (i = 0; i < nd->max_contacts; i++)
+			if (contact_map[i] >= 0) {
+				k = i;
+				break;
+			}
+	}
+
+	/* Update the classic touchscreen state */
+	if (count) {
+		nd->x = nd->contacts[contact_map[k]].x;
+		nd->y = nd->contacts[contact_map[k]].y;
+		nd->confidence =
+			nd->contacts[contact_map[k]].confidence;
+		ntrig_single_touch_emit(nd->st_input, nd);
 	} else {
-		input_report_key(nd->st_input, BTN_TOUCH, 0);
-		input_report_key(nd->st_input, BTN_TOOL_DOUBLETAP, 0);
-		input_report_key(nd->st_input, BTN_TOOL_TRIPLETAP, 0);
-		input_report_key(nd->st_input, BTN_TOOL_QUADTAP, 0);
-		input_sync(nd->st_input);
+		/* Hit the end of activity, clear state */
+		nd->confidence = 0;
+		ntrig_single_touch_emit(nd->st_input, nd);
 	}
 
 	/* Multitouch doesn't need to send an end of activity notice. */
-	if(!(nd->contact_count && nd->contacts[0].confidence))
+	if(!(count && nd->contacts[0].confidence)) {
+		/* Only need to tag this group as empty, don't have
+		 * to worry about preserving all the empty state */
+		nd->prev_contact_count = 0;
 		return;
+	}
 
 	/* Emit multitouch events */
-	for (i = 0; i <= nd->max_contacts && i < NTRIG_MAX_CONTACTS; i++) {
-		if (nd->contacts[i].confidence) {
-			struct ntrig_contact *contact = &nd->contacts[i];
+	for (i = 0; i <= j; i++) {
+		/* Valid contact, send real values */
+		if (contact_map[i] >= 0
+		    && nd->contacts[contact_map[i]].confidence) {
+			contact = &nd->contacts[contact_map[i]];
 			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
 			input_event(input, EV_ABS, ABS_MT_POSITION_X,
 					contact->x);
@@ -223,6 +365,12 @@ static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
 			input_mt_sync(input);
 		}
 	}
+
+	/* Age the current state to previous. */
+	for (i = 0; i < count; i++) {
+		nd->prev_contacts[i] = nd->contacts[i];
+	}
+	nd->prev_contact_count = nd->contact_count;
 }
 
 /*
@@ -272,8 +420,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * to emit a normal (X, Y) position
 			 */
 			if (!nd->reading_mt) {
-				input_event(input, EV_ABS, ABS_X, nd->x);
-				input_event(input, EV_ABS, ABS_Y, nd->y);
+				ntrig_single_touch_emit(input, nd);
 			}
 			break;
 		case 0xff000002:
@@ -370,6 +517,7 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	nd->id = 0;
 	nd->reading_mt = 0;
 	nd->contact_count = 0;
+	nd->prev_contact_count = 0;
 	nd->max_width = 0x500;
 	nd->max_height = 0x500;
 	nd->max_contacts = 5;
-- 
1.6.6.1

Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Dmitry Torokhov]

On Thu, Feb 11, 2010 at 07:19:05PM -0500, Rafi Rubin wrote:
> This cleans up the identification of multitouch groups.  When two touch
> input devices are available, single and multi touch streams are
> partitioned to the appropriate device.
> 
> Added triple and quad tap to the single touch device for the benefit of
> tools that recognize different tap types but do not have full
> multi touch support.
> 
> Capture a full multitouch group before emitting events.  This allows for
> additional processing, though takes little advantage of that capability.
> 
> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
> ---
>  drivers/hid/hid-ntrig.c |  231 +++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 195 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 6464d15..9f46a3d 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -20,13 +20,56 @@
>  #include "hid-ids.h"
>  
>  #define NTRIG_DUPLICATE_USAGES	0x001
> +#define NTRIG_MAX_CONTACTS	7
>  
>  #define nt_map_key_clear(c)	hid_map_usage_clear(hi, usage, bit, max, \
>  					EV_KEY, (c))
>  
> +/*
> + * The relavent data needed to store events from a contact to
> + * delay transmittion.
> + */
> +struct ntrig_contact {
> +	char active;

Bool?

> +	__s8  logical_id;
> +	__u16 x, y;
> +	__u8 confidence;
> +
> +	/* height and width transformed */
> +	char orientation;

Make an enum?

> +	__u16 touch_major;
> +	__u16 touch_minor;
> +};
> +
> +
>  struct ntrig_data {
> -	__s32 x, y, id, w, h;
> -	char reading_a_point, found_contact_id;
> +	/* Incoming raw values for a single contact */
> +	__u16 x, y, w, h;
> +	__u16 id;
> +	__u8 confidence;
> +
> +	int max_width;
> +	int max_height;
> +	int max_contacts;
> +
> +	unsigned char reading_mt;
> +
> +	/* Collected state for 2 full sets of contacts */
> +	struct ntrig_contact contacts[NTRIG_MAX_CONTACTS];
> +	__u8 contact_count;
> +
> +	__u8 mt_footer[4];
> +	__u8 mt_foot_count;
> +
> +	/* options */
> +	unsigned char emit_ghosts;

bool?

> +
> +	/*
> +	 * Keep a pointer to the single touch input device to enable
> +	 * the multitouch handler to redirect the older style single
> +	 * touch events.
> +	 */
> +	struct input_dev * st_input;
>  };
>  
>  /*
> @@ -39,8 +82,8 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> -	/* No special mappings needed for the pen */
> -	if(field->application == HID_DG_PEN)
> +	/* No special mappings needed for the pen and single touch */
> +	if(field->physical)
>  		return 0;
>  
>  	switch (usage->hid & HID_USAGE_PAGE) {
> @@ -104,8 +147,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> -	/* No special mappings needed for the pen */
> -	if(field->application == HID_DG_PEN)
> +	/* No special mappings needed for the pen and single touch */
> +	if(field->physical)
>  		return 0;
>  
>  	if (usage->type == EV_KEY || usage->type == EV_REL
> @@ -115,6 +158,73 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  	return 0;
>  }
>  
> +static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
> +{
> +	int i;
> +	struct ntrig_contact *contact = &nd->contacts[0];
> +
> +	/* Emit single touch events */
> +	if(contact->confidence) {
> +		switch (nd->contact_count) {
> +			case 0:	/* for single touch devices */
> +			case 1:
> +				input_report_key(nd->st_input,
> +						BTN_TOOL_DOUBLETAP, 1);
> +				break;
> +			case 2:
> +				input_report_key(nd->st_input,
> +						BTN_TOOL_TRIPLETAP, 1);
> +				break;
> +			case 3:
> +			default:
> +				input_report_key(nd->st_input,
> +						BTN_TOOL_QUADTAP, 1);

Formatting: switch and case start in the same column.

> +		}
> +		input_report_key(nd->st_input, BTN_TOUCH, 1);
> +		input_event(nd->st_input, EV_ABS, ABS_X, contact->x);
> +		input_event(nd->st_input, EV_ABS, ABS_Y, contact->y);
> +		input_sync(nd->st_input);
> +	} else {
> +		input_report_key(nd->st_input, BTN_TOUCH, 0);
> +		input_report_key(nd->st_input, BTN_TOOL_DOUBLETAP, 0);
> +		input_report_key(nd->st_input, BTN_TOOL_TRIPLETAP, 0);
> +		input_report_key(nd->st_input, BTN_TOOL_QUADTAP, 0);
> +		input_sync(nd->st_input);
> +	}
> +
> +	/* Multitouch doesn't need to send an end of activity notice. */
> +	if(!(nd->contact_count && nd->contacts[0].confidence))
> +		return;
> +
> +	/* Emit multitouch events */
> +	for (i = 0; i <= nd->max_contacts && i < NTRIG_MAX_CONTACTS; i++) {
> +		if (nd->contacts[i].confidence) {
> +			struct ntrig_contact *contact = &nd->contacts[i];
> +			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
> +			input_event(input, EV_ABS, ABS_MT_POSITION_X,
> +					contact->x);
> +			input_event(input, EV_ABS, ABS_MT_POSITION_Y,
> +					contact->y);
> +			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
> +					contact->orientation);
> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR,
> +					contact->touch_major);
> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR,
> +					contact->touch_minor);
> +			input_mt_sync(input);
> +		} else if (nd->emit_ghosts) {
> +			/* emit filler points if so desired */
> +			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
> +			input_event(input, EV_ABS, ABS_MT_POSITION_X, 0);
> +			input_event(input, EV_ABS, ABS_MT_POSITION_Y, 0);
> +			input_event(input, EV_ABS, ABS_MT_ORIENTATION, 0);
> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
> +			input_mt_sync(input);
> +		}
> +	}

Where's the final input_sync()? And if hid does it for us (I think it does)
then syncs above are not needed.

> +}
> +
>  /*
>   * this function is called upon all reports
>   * so that we can filter contact point information,
> @@ -133,17 +243,23 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  
>          if (hid->claimed & HID_CLAIMED_INPUT) {
>  		switch (usage->hid) {
> +		case 0xff000001:
> +			/* Tag indicating the start of a multitouch group */
> +			nd->reading_mt = 1;
> +			break;
> +		case HID_DG_CONFIDENCE:
> +			nd->confidence = value;
> +			break;
>  		case HID_GD_X:
>  			nd->x = value;
> -			nd->reading_a_point = 1;
> +			/* Clear the contact footer */
> +			nd->mt_foot_count = 0;
>  			break;
>  		case HID_GD_Y:
>  			nd->y = value;
>  			break;
>  		case HID_DG_CONTACTID:
>  			nd->id = value;
> -			/* we receive this only when in multitouch mode */
> -			nd->found_contact_id = 1;
>  			break;
>  		case HID_DG_WIDTH:
>  			nd->w = value;
> @@ -155,7 +271,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  			 * report received in a finger event. We want
>  			 * to emit a normal (X, Y) position
>  			 */
> -			if (!nd->found_contact_id) {
> +			if (!nd->reading_mt) {
>  				input_event(input, EV_ABS, ABS_X, nd->x);
>  				input_event(input, EV_ABS, ABS_Y, nd->y);
>  			}
> @@ -167,33 +283,58 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  			 * this usage tells if the contact point is real
>  			 * or a placeholder
>  			 */
> -			if (!nd->reading_a_point || value != 1)
> +
> +			/* Shouldn't get more than 4 footer packets, so skip */
> +			if (nd->mt_foot_count >= 4)
> +				break;
> +
> +			nd->mt_footer[nd->mt_foot_count++] = value;
> +
> +			/* if the footer isn't complete break */
> +			if (nd->mt_foot_count != 4)
> +				break;
> +
> +			/* Pen activity signal, trigger end of touch. */
> +			if (nd->mt_footer[2]) {
> +				nd->contacts[nd->id].x = 0;
> +				nd->contacts[nd->id].y = 0;
> +				nd->contacts[nd->id].confidence = 0;
>  				break;
> -			/* emit a normal (X, Y) for the first point only */
> -			if (nd->id == 0) {
> -				input_event(input, EV_ABS, ABS_X, nd->x);
> -				input_event(input, EV_ABS, ABS_Y, nd->y);
>  			}
> -			input_event(input, EV_ABS, ABS_MT_POSITION_X, nd->x);
> -			input_event(input, EV_ABS, ABS_MT_POSITION_Y, nd->y);
> +
> +			/* If the contact was invalid */
> +			if (!(nd->confidence && nd->mt_footer[0])
> +					|| nd->w <= 250
> +					|| nd->h <= 190) {
> +				nd->contacts[nd->id].x = 0;
> +				nd->contacts[nd->id].y = 0;
> +				nd->contacts[nd->id].confidence = 0;
> +				break;
> +			}
> +
> +			nd->contacts[nd->id].logical_id = -1;
> +			nd->contacts[nd->id].confidence = nd->confidence;
> +			nd->contacts[nd->id].x = nd->x;
> +			nd->contacts[nd->id].y = nd->y;
> +
>  			if (nd->w > nd->h) {
> -				input_event(input, EV_ABS,
> -						ABS_MT_ORIENTATION, 1);
> -				input_event(input, EV_ABS,
> -						ABS_MT_TOUCH_MAJOR, nd->w);
> -				input_event(input, EV_ABS,
> -						ABS_MT_TOUCH_MINOR, nd->h);
> +				nd->contacts[nd->id].orientation = 1;
> +				nd->contacts[nd->id].touch_major = nd->w;
> +				nd->contacts[nd->id].touch_minor = nd->h;
>  			} else {
> -				input_event(input, EV_ABS,
> -						ABS_MT_ORIENTATION, 0);
> -				input_event(input, EV_ABS,
> -						ABS_MT_TOUCH_MAJOR, nd->h);
> -				input_event(input, EV_ABS,
> -						ABS_MT_TOUCH_MINOR, nd->w);
> +				nd->contacts[nd->id].orientation = 0;
> +				nd->contacts[nd->id].touch_major = nd->h;
> +				nd->contacts[nd->id].touch_minor = nd->w;
> +			}
> +			break;
> +
> +		case HID_DG_CONTACTCOUNT:
> +			/* This marks the end of the multitouch group */
> +			nd->contact_count = value;
> +			if(nd->reading_mt) {
> +				nd->reading_mt = 0;
> +				ntrig_conclude_mt(input, nd);
>  			}
> -			input_mt_sync(field->hidinput->input);
> -			nd->reading_a_point = 0;
> -			nd->found_contact_id = 0;
>  			break;
>  
>  		default:
> @@ -203,8 +344,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  	}
>  
>  	/* we have handled the hidinput part, now remains hiddev */
> -        if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
> -                hid->hiddev_hid_event(hid, field, usage, value);
> +	if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)

I prefer adding () around bitwise ops, just so I don't doubt which has
hgher perecdence.

> +		hid->hiddev_hid_event(hid, field, usage, value);
>  
>  	return 1;
>  }
> @@ -224,8 +365,16 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		dev_err(&hdev->dev, "cannot allocate N-Trig data\n");
>  		return -ENOMEM;
>  	}
> -	nd->reading_a_point = 0;
> -	nd->found_contact_id = 0;
> +	
> +	/* Initialize the driver data to sane values */
> +	nd->id = 0;
> +	nd->reading_mt = 0;
> +	nd->contact_count = 0;
> +	nd->max_width = 0x500;
> +	nd->max_height = 0x500;
> +	nd->max_contacts = 5;
> +	nd->emit_ghosts = 0;
> +
>  	hid_set_drvdata(hdev, nd);
>  
>  	ret = hid_parse(hdev);
> @@ -251,6 +400,15 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  			 */
>  			if(hidinput->report->field[0]->physical) {
>  				input->name = "N-Trig Touchscreen";
> +				nd->st_input = input;
> +
> +				/*
> +				 * A little something special to enable
> +				 * two and three finger taps.
> +				 */ 
> +				set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
> +				set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
> +				set_bit(BTN_TOOL_QUADTAP, input->keybit);

__set_bit() - no need to lock the bus.

>  			} else {
>  				input->name = "N-Trig MultiTouch";
>  			}
> @@ -271,6 +429,7 @@ static const struct hid_device_id ntrig_devices[] = {
>  		.driver_data = NTRIG_DUPLICATE_USAGES },
>  	{ }
>  };
> +
>  MODULE_DEVICE_TABLE(hid, ntrig_devices);
>  
>  static const struct hid_usage_id ntrig_grabbed_usages[] = {
> -- 
> 1.6.6.1
> 

-- 
Dmitry

Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
>> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
>> +			input_mt_sync(input);
>> +		}
>> +	}
> 
> Where's the final input_sync()? And if hid does it for us (I think it does)
> then syncs above are not needed.

It seems the normal input_sync calls are unnecessary for the single touch device.

The mt syncs are necessary to separate each contact.  And since we are caching the
contacts we need to emit those.  The final normal input_sync is handled by hid.

On a side note, once I pulled the st input syncs, the mt device stopped getting them.
I had to undo the blocking of CONTACTCOUNT which restores the normal syncs from hid.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt0xqgACgkQwuRiAT9o6081+wCfe5K5LRJXr0jORoTJPOVwCrXT
V7YAn1ijdSwMSyenvPU9wy3QfJyodDJS
=UZmW
-----END PGP SIGNATURE-----

Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Dmitry Torokhov]

On Thu, Feb 11, 2010 at 10:10:35PM -0500, Rafi Rubin wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> >> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
> >> +			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
> >> +			input_mt_sync(input);
> >> +		}
> >> +	}
> > 
> > Where's the final input_sync()? And if hid does it for us (I think it does)
> > then syncs above are not needed.
> 
> It seems the normal input_sync calls are unnecessary for the single touch device.
> 
> The mt syncs are necessary to separate each contact.  And since we are caching the
> contacts we need to emit those.  The final normal input_sync is handled by hid.
> 
> On a side note, once I pulled the st input syncs, the mt device stopped getting them.
> I had to undo the blocking of CONTACTCOUNT which restores the normal syncs from hid.

Ah, these are 2 different devices... I need to increase my coffee
intake.

OK, so I re-read the patch again and it you are splittig the one
physical device in 2 - one legacy single tap and another multi-touch.
If I understand it correctly that means that the same gesture can be
potentially sent through bnoth devices. So in this case how system
should decide which device to use and which device to ignore?

-- 
Dmitry

Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> Ah, these are 2 different devices... I need to increase my coffee
> intake.
> 
> OK, so I re-read the patch again and it you are splittig the one
> physical device in 2 - one legacy single tap and another multi-touch.
> If I understand it correctly that means that the same gesture can be
> potentially sent through bnoth devices. So in this case how system
> should decide which device to use and which device to ignore?

Though I can't say for sure that the pen and capacitive touch sensors are
separate circuitry, it certainly looks that way to the kernel.  The part
I find particularly strange is that there is a second hid device that does
nothing and it puts both physicals on a single hid.

Anyway, the physical pen and finger sensors do not interact.  Events from
tapping the screen with a finger end up going to one device, and using the
pen on the screen goes to the other.  From what I've seen userspace tools
recognize which is which by the tools supported (PEN and DOUBLETAP).


The third input is a logical multi touch device.  Yes there is definitely
potential for confusion when both MT X and normal ABS_X are sent.  That's
an ongoing discussion on the input list and I'm mostly trying to follow
the current preference which is to send both and let the user space
programs decide which they care about.  I'm not sure if the consensus was
to use separate input devices, but since one was already being allocated
by hid I decided to use it.

The two touch devices do not get the same output events.  ABS_X goes only
to the single touch and ABS_MT_POSITION_X to the multi.  The mt device
gets only details about the contact: id, x, y, and size.  It also sends
no touch or other tool up/down or on/off events.

Rafi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt1sl8ACgkQwuRiAT9o609hugCdHIqF30DTTXSZraCTICKSZ9C3
6jgAmwYGfVxUMFnQZw7n02lF8o5zdED8
=ERtj
-----END PGP SIGNATURE-----

Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Jiri Kosina]

On Thu, 11 Feb 2010, Dmitry Torokhov wrote:

> > +		} else if (nd->emit_ghosts) {
> > +			/* emit filler points if so desired */
> > +			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
> > +			input_event(input, EV_ABS, ABS_MT_POSITION_X, 0);
> > +			input_event(input, EV_ABS, ABS_MT_POSITION_Y, 0);
> > +			input_event(input, EV_ABS, ABS_MT_ORIENTATION, 0);
> > +			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
> > +			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
> > +			input_mt_sync(input);
> > +		}
> > +	}
> 
> Where's the final input_sync()? And if hid does it for us (I think it does)
> then syncs above are not needed.

Yes, it does, in hidinput_report_event(), after the custom ->event() 
callback has finished.

Thanks a lot Dmitry for beating me looking at the patches, I really 
appreciate that.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Dmitry Torokhov]

Hi Rafi,

On Thu, Feb 11, 2010 at 07:19:03PM -0500, Rafi Rubin wrote:
> Added a quirk to enable distinct input devices.  The digitizer utilizes
> three inputs to represent pen, multitouch and a normal touch screen.
> 
> With the Pen partitioned, it behaves well and does not need special
> handling.
> 
> Also, I set names to the input devices to clarify the functions of the
> various inputs.
> 

Overall looks much nicer (goes for all 4 patches) and reader should be
able to see what is going on much easier now.

Couple of formatting nits still (I am pretty sure scripts/checkpatch.pl
warns about these as well):

> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
> ---
>  drivers/hid/hid-ntrig.c |   46 ++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 49ce69d..4bb5ed0 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -42,6 +42,10 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> +	/* No special mappings needed for the pen */
> +	if(field->application == HID_DG_PEN)
> +		return 0;

Please add space after keyword and before opening paren ("if (...)",
"While (...)", etc) here and elsewhere.

> +
>  	switch (usage->hid & HID_USAGE_PAGE) {
>  
>  	case HID_UP_GENDESK:
> @@ -104,6 +108,9 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> +	/* No special mappings needed for the pen */
> +	if(field->application == HID_DG_PEN)
> +		return 0;
>  	if (usage->type == EV_KEY || usage->type == EV_REL
>  			|| usage->type == EV_ABS)
>  		clear_bit(usage->code, *bit);
> @@ -123,6 +130,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  	struct input_dev *input = field->hidinput->input;
>  	struct ntrig_data *nd = hid_get_drvdata(hid);
>  
> +	/* No special handling needed for the pen */
> +	if(field->application == HID_DG_PEN)
> +		return 0;
> +
>          if (hid->claimed & HID_CLAIMED_INPUT) {
>  		switch (usage->hid) {
>  
> @@ -241,6 +252,11 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>  	int ret;
>  	struct ntrig_data *nd;
> +	struct hid_input *hidinput;
> +	struct input_dev *input;
> +
> +	if (id->driver_data)
> +		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
>  
>  	nd = kmalloc(sizeof(struct ntrig_data), GFP_KERNEL);
>  	if (!nd) {
> @@ -255,14 +271,36 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	if (!ret)
>  		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>  
> -	if (ret)
> +	if (ret) {
>  		kfree (nd);

What about hid_hw_stop()? Overall I'd rather see the standard error
unwinding path with gotos.

> +		return ret;
> +	}
> +
> +	list_for_each_entry(hidinput, &hdev->inputs, list) {
> +		input = hidinput->input;
> +		switch (hidinput->report->field[0]->application) {
> +		case HID_DG_PEN:
> +			input->name = "N-Trig Pen";
> +			break;
> +		case HID_DG_TOUCHSCREEN:
> +			/* 
> +			 * The physical touchscreen (single touch) input
> +			 * has a value for physical, whereas the multitouch
> +			 * only has logical input fields.
> +			 */
> +			if(hidinput->report->field[0]->physical) {
> +				input->name = "N-Trig Touchscreen";
> +			} else {
> +				input->name = "N-Trig MultiTouch";
> +			}

No need for curly braces for single-line statements. Maybe compress to
?: statement?

-- 
Dmitry

Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/11/10 19:36, Dmitry Torokhov wrote:
> Hi Rafi,
> 
> On Thu, Feb 11, 2010 at 07:19:03PM -0500, Rafi Rubin wrote:
>> Added a quirk to enable distinct input devices.  The digitizer utilizes
>> three inputs to represent pen, multitouch and a normal touch screen.
>>
>> With the Pen partitioned, it behaves well and does not need special
>> handling.
>>
>> Also, I set names to the input devices to clarify the functions of the
>> various inputs.
>>
> 
> Overall looks much nicer (goes for all 4 patches) and reader should be
> able to see what is going on much easier now.
> 
> Couple of formatting nits still (I am pretty sure scripts/checkpatch.pl
> warns about these as well):

Fair enough.  I ran it through scripts/Lindent for the 4th patch.  I guess
that's not as thorough and I should have checked earlier.

>> -	if (ret)
>> +	if (ret) {
>>  		kfree (nd);
> 
> What about hid_hw_stop()? Overall I'd rather see the standard error
> unwinding path with gotos.

Ok.

>> +			if(hidinput->report->field[0]->physical) {
>> +				input->name = "N-Trig Touchscreen";
>> +			} else {
>> +				input->name = "N-Trig MultiTouch";
>> +			}
> 
> No need for curly braces for single-line statements. Maybe compress to
> ?: statement?

That's an artifact of staging the changes.  Oddly enough my original version was just ?:

Rafi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt0pL8ACgkQwuRiAT9o6098awCeLFRkVATyURHEBszR63dvzfWK
B7gAoNevD3DmWK3qgCEGIGgqKGY3f8aU
=2z1B
-----END PGP SIGNATURE-----

Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>>  	nd = kmalloc(sizeof(struct ntrig_data), GFP_KERNEL);
>>  	if (!nd) {
>> @@ -255,14 +271,36 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>  	if (!ret)
>>  		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>>  
>> -	if (ret)
>> +	if (ret) {
>>  		kfree (nd);
> 
> What about hid_hw_stop()? Overall I'd rather see the standard error
> unwinding path with gotos.

Cleaning up the probe function.  But it looks like the last caught error is from start, so is it fair to leave out stop?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt0qPUACgkQwuRiAT9o60+REACg6T73GpC3jz5dkAW+CFsAPLsz
w4sAn3ttnpvH85bECwYxe8WPhJ77Zs2z
=xRv3
-----END PGP SIGNATURE-----

Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Dmitry Torokhov]

On Thu, Feb 11, 2010 at 08:03:52PM -0500, Rafi Rubin wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> >>  	nd = kmalloc(sizeof(struct ntrig_data), GFP_KERNEL);
> >>  	if (!nd) {
> >> @@ -255,14 +271,36 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >>  	if (!ret)
> >>  		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> >>  
> >> -	if (ret)
> >> +	if (ret) {
> >>  		kfree (nd);
> > 
> > What about hid_hw_stop()? Overall I'd rather see the standard error
> > unwinding path with gotos.
> 
> Cleaning up the probe function.  But it looks like the last caught error is from start, so is it fair to leave out stop?

Ah, yes, my bad.

-- 
Dmitry

Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This patch set is a rethinking of my earlier mess.  I've broken it down into 4 conceptual steps, and a little extra which I've left
off for now.

The first three are basically clean up from the not handling the input devices correctly and leaving them a multiplexed stream.  In
userspace the only changes should be the need to open different devices and the loss of unnecessary events that indicated which
sensor a point came from.

The fourth adds the missing contact id event and implements basic finger tracking.  Stephane's code did send tracking id's, but it
was masked off.  The benefits of finger tracking should be pretty clear.  The most recent firmware seems to sort contacts by Y position.


Rafi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt0otAACgkQwuRiAT9o6098XgCg8P+m7mf4PT5ufZ0tsmsNd4av
ehoAoPXbgcm9zFOS4EpTZe5BPi3euzNN
=jCM6
-----END PGP SIGNATURE-----

[PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Rafi Rubin]

Added a quirk to enable distinct input devices.  The digitizer utilizes
three inputs to represent pen, multitouch and a normal touch screen.

With the Pen partitioned, it behaves well and does not need special
handling.

Also, I set names to the input devices to clarify the functions of the
various inputs.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |   69 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 49ce69d..38b2364 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -26,10 +26,10 @@
 
 struct ntrig_data {
 	__s32 x, y, id, w, h;
-	char reading_a_point, found_contact_id;
-	char pen_active;
-	char finger_active;
-	char inverted;
+	bool reading_a_point, found_contact_id;
+	bool pen_active;
+	bool finger_active;
+	bool inverted;
 };
 
 /*
@@ -42,6 +42,10 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	/* No special mappings needed for the pen */
+	if (field->application == HID_DG_PEN)
+		return 0;
+
 	switch (usage->hid & HID_USAGE_PAGE) {
 
 	case HID_UP_GENDESK:
@@ -104,6 +108,9 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
+	/* No special mappings needed for the pen */
+	if (field->application == HID_DG_PEN)
+		return 0;
 	if (usage->type == EV_KEY || usage->type == EV_REL
 			|| usage->type == EV_ABS)
 		clear_bit(usage->code, *bit);
@@ -123,6 +130,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 	struct input_dev *input = field->hidinput->input;
 	struct ntrig_data *nd = hid_get_drvdata(hid);
 
+	/* No special handling needed for the pen */
+	if (field->application == HID_DG_PEN)
+		return 0;
+
         if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
 
@@ -231,8 +242,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 	}
 
 	/* we have handled the hidinput part, now remains hiddev */
-        if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
-                hid->hiddev_hid_event(hid, field, usage, value);
+	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_hid_event)
+		hid->hiddev_hid_event(hid, field, usage, value);
 
 	return 1;
 }
@@ -241,6 +252,11 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
 	struct ntrig_data *nd;
+	struct hid_input *hidinput;
+	struct input_dev *input;
+
+	if (id->driver_data)
+		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
 
 	nd = kmalloc(sizeof(struct ntrig_data), GFP_KERNEL);
 	if (!nd) {
@@ -252,12 +268,43 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	hid_set_drvdata(hdev, nd);
 
 	ret = hid_parse(hdev);
-	if (!ret)
-		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret) {
+		dev_err(&hdev->dev, "parse failed\n");
+		goto err_free;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
+	if (ret) {
+		dev_err(&hdev->dev, "hw start failed\n");
+		goto err_free;
+	}
 
-	if (ret)
-		kfree (nd);
 
+	list_for_each_entry(hidinput, &hdev->inputs, list) {
+		input = hidinput->input;
+		switch (hidinput->report->field[0]->application) {
+		case HID_DG_PEN:
+			input->name = "N-Trig Pen";
+			break;
+		case HID_DG_TOUCHSCREEN:
+			/*
+			 * The physical touchscreen (single touch)
+			 * input has a value for physical, whereas
+			 * the multitouch only has logical input
+			 * fields.
+			 */
+			input->name =
+				(hidinput->report->field[0]
+				 ->physical) ?
+				"N-Trig Touchscreen" :
+				"N-Trig MultiTouch";
+			break;
+		}
+	}
+
+	return 0;
+err_free:
+	kfree(nd);
 	return ret;
 }
 
@@ -276,7 +323,7 @@ MODULE_DEVICE_TABLE(hid, ntrig_devices);
 
 static const struct hid_usage_id ntrig_grabbed_usages[] = {
 	{ HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
-	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
+	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1 }
 };
 
 static struct hid_driver ntrig_driver = {
-- 
1.6.6.1

[PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching

[Rafi Rubin]

With the pen and touch split apart, we no longer need to inject
additional tool switching events.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |   41 +----------------------------------------
 1 files changed, 1 insertions(+), 40 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 38b2364..1bda3a4 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -27,9 +27,6 @@
 struct ntrig_data {
 	__s32 x, y, id, w, h;
 	bool reading_a_point, found_contact_id;
-	bool pen_active;
-	bool finger_active;
-	bool inverted;
 };
 
 /*
@@ -47,7 +44,6 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		return 0;
 
 	switch (usage->hid & HID_USAGE_PAGE) {
-
 	case HID_UP_GENDESK:
 		switch (usage->hid) {
 		case HID_GD_X:
@@ -111,6 +107,7 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 	/* No special mappings needed for the pen */
 	if (field->application == HID_DG_PEN)
 		return 0;
+
 	if (usage->type == EV_KEY || usage->type == EV_REL
 			|| usage->type == EV_ABS)
 		clear_bit(usage->code, *bit);
@@ -136,18 +133,6 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
         if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
-
-		case HID_DG_INRANGE:
-			if (field->application & 0x3)
-				nd->pen_active = (value != 0);
-			else
-				nd->finger_active = (value != 0);
-			return 0;
-
-		case HID_DG_INVERT:
-			nd->inverted = value;
-			return 0;
-
 		case HID_GD_X:
 			nd->x = value;
 			nd->reading_a_point = 1;
@@ -171,32 +156,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * to emit a normal (X, Y) position
 			 */
 			if (!nd->found_contact_id) {
-				if (nd->pen_active && nd->finger_active) {
-					input_report_key(input, BTN_TOOL_DOUBLETAP, 0);
-					input_report_key(input, BTN_TOOL_DOUBLETAP, 1);
-				}
-				input_event(input, EV_ABS, ABS_X, nd->x);
-				input_event(input, EV_ABS, ABS_Y, nd->y);
-			}
-			break;
-		case HID_DG_TIPPRESSURE:
-			/*
-			 * when in single touch mode, this is the last
-			 * report received in a pen event. We want
-			 * to emit a normal (X, Y) position
-			 */
-			if (! nd->found_contact_id) {
-				if (nd->pen_active && nd->finger_active) {
-					input_report_key(input,
-							nd->inverted ? BTN_TOOL_RUBBER : BTN_TOOL_PEN
-							, 0);
-					input_report_key(input,
-							nd->inverted ? BTN_TOOL_RUBBER : BTN_TOOL_PEN
-							, 1);
-				}
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
-				input_event(input, EV_ABS, ABS_PRESSURE, value);
 			}
 			break;
 		case 0xff000002:
-- 
1.6.6.1

[PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Rafi Rubin]

This cleans up the identification of multitouch groups.  When two touch
input devices are available, single and multi touch streams are
partitioned to the appropriate device.

Added triple and quad tap to the single touch device for the benefit of
tools that recognize different tap types but do not have full
multi touch support.

Capture a full multitouch group before emitting events.  This allows for
additional processing, though takes little advantage of that capability.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |  239 +++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 200 insertions(+), 39 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 1bda3a4..3da7287 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -20,13 +20,64 @@
 #include "hid-ids.h"
 
 #define NTRIG_DUPLICATE_USAGES	0x001
+#define NTRIG_MAX_CONTACTS	7
 
 #define nt_map_key_clear(c)	hid_map_usage_clear(hi, usage, bit, max, \
 					EV_KEY, (c))
+enum mt_orientation {
+	MT_VERTICAL   = 0,
+	MT_HORIZONTAL = 1,
+};
+
+enum ntrig_mt_emit_ghosts {
+	NTRIG_MT_EMIT_GHOSTS_NONE	  = 0,
+	NTRIG_MT_EMIT_GHOSTS_LOW_IDS_ONLY = 1,
+	NTRIG_MT_EMIT_GHOSTS_ALL	  = 2,
+};
+
+/*
+ * The relavent data needed to store events from a contact to
+ * delay transmittion.
+ */
+struct ntrig_contact {
+	__s8 logical_id;
+	__u16 x, y;
+	__u8 confidence;
+
+	/* height and width transformed */
+	enum mt_orientation orientation;
+	__u16 touch_major;
+	__u16 touch_minor;
+};
 
 struct ntrig_data {
-	__s32 x, y, id, w, h;
-	bool reading_a_point, found_contact_id;
+	/* Incoming raw values for a single contact */
+	__u16 x, y, w, h;
+	__u16 id;
+	__u8 confidence;
+
+	int max_width;
+	int max_height;
+	int max_contacts;
+
+	bool reading_mt;
+
+	/* Collected state for 2 full sets of contacts */
+	struct ntrig_contact contacts[NTRIG_MAX_CONTACTS];
+	__u8 contact_count;
+
+	__u8 mt_footer[4];
+	__u8 mt_foot_count;
+
+	/* options */
+	enum ntrig_mt_emit_ghosts emit_ghosts;
+
+	/*
+	 * Keep a pointer to the single touch input device to enable
+	 * the multitouch handler to redirect the older style single
+	 * touch events.
+	 */
+	struct input_dev *st_input;
 };
 
 /*
@@ -39,8 +90,8 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen */
-	if (field->application == HID_DG_PEN)
+	/* No special mappings needed for the pen and single touch */
+	if (field->physical)
 		return 0;
 
 	switch (usage->hid & HID_USAGE_PAGE) {
@@ -104,8 +155,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen */
-	if (field->application == HID_DG_PEN)
+	/* No special mappings needed for the pen and single touch */
+	if (field->physical)
 		return 0;
 
 	if (usage->type == EV_KEY || usage->type == EV_REL
@@ -115,6 +166,68 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 	return 0;
 }
 
+static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
+{
+	int i;
+	struct ntrig_contact *contact = &nd->contacts[0];
+
+	/* Emit single touch events */
+	if (contact->confidence) {
+		switch (nd->contact_count) {
+		case 0:	/* for single touch devices */
+		case 1:
+			input_report_key(nd->st_input, BTN_TOOL_DOUBLETAP, 1);
+			break;
+		case 2:
+			input_report_key(nd->st_input, BTN_TOOL_TRIPLETAP, 1);
+			break;
+		case 3:
+		default:
+			input_report_key(nd->st_input, BTN_TOOL_QUADTAP, 1);
+		}
+		input_report_key(nd->st_input, BTN_TOUCH, 1);
+		input_event(nd->st_input, EV_ABS, ABS_X, contact->x);
+		input_event(nd->st_input, EV_ABS, ABS_Y, contact->y);
+	} else {
+		input_report_key(nd->st_input, BTN_TOUCH, 0);
+		input_report_key(nd->st_input, BTN_TOOL_DOUBLETAP, 0);
+		input_report_key(nd->st_input, BTN_TOOL_TRIPLETAP, 0);
+		input_report_key(nd->st_input, BTN_TOOL_QUADTAP, 0);
+	}
+
+	/* Multitouch doesn't need to send an end of activity notice. */
+	if (!(nd->contact_count && nd->contacts[0].confidence))
+		return;
+
+	/* Emit multitouch events */
+	for (i = 0; i <= nd->max_contacts && i < NTRIG_MAX_CONTACTS; i++) {
+		if (nd->contacts[i].confidence) {
+			struct ntrig_contact *contact = &nd->contacts[i];
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
+			input_event(input, EV_ABS, ABS_MT_POSITION_X,
+					contact->x);
+			input_event(input, EV_ABS, ABS_MT_POSITION_Y,
+					contact->y);
+			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
+					contact->orientation);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR,
+					contact->touch_major);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR,
+					contact->touch_minor);
+			input_mt_sync(input);
+		} else if (nd->emit_ghosts) {
+			/* emit filler points if so desired */
+			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
+			input_event(input, EV_ABS, ABS_MT_POSITION_X, 0);
+			input_event(input, EV_ABS, ABS_MT_POSITION_Y, 0);
+			input_event(input, EV_ABS, ABS_MT_ORIENTATION, 0);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, 0);
+			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, 0);
+			input_mt_sync(input);
+		}
+	}
+}
+
 /*
  * this function is called upon all reports
  * so that we can filter contact point information,
@@ -133,17 +246,23 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
         if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
+		case 0xff000001:
+			/* Tag indicating the start of a multitouch group */
+			nd->reading_mt = 1;
+			break;
+		case HID_DG_CONFIDENCE:
+			nd->confidence = value;
+			break;
 		case HID_GD_X:
 			nd->x = value;
-			nd->reading_a_point = 1;
+			/* Clear the contact footer */
+			nd->mt_foot_count = 0;
 			break;
 		case HID_GD_Y:
 			nd->y = value;
 			break;
 		case HID_DG_CONTACTID:
 			nd->id = value;
-			/* we receive this only when in multitouch mode */
-			nd->found_contact_id = 1;
 			break;
 		case HID_DG_WIDTH:
 			nd->w = value;
@@ -155,7 +274,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * report received in a finger event. We want
 			 * to emit a normal (X, Y) position
 			 */
-			if (!nd->found_contact_id) {
+			if (!nd->reading_mt) {
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -167,33 +286,58 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * this usage tells if the contact point is real
 			 * or a placeholder
 			 */
-			if (!nd->reading_a_point || value != 1)
+
+			/* Shouldn't get more than 4 footer packets, so skip */
+			if (nd->mt_foot_count >= 4)
+				break;
+
+			nd->mt_footer[nd->mt_foot_count++] = value;
+
+			/* if the footer isn't complete break */
+			if (nd->mt_foot_count != 4)
+				break;
+
+			/* Pen activity signal, trigger end of touch. */
+			if (nd->mt_footer[2]) {
+				nd->contacts[nd->id].x = 0;
+				nd->contacts[nd->id].y = 0;
+				nd->contacts[nd->id].confidence = 0;
 				break;
-			/* emit a normal (X, Y) for the first point only */
-			if (nd->id == 0) {
-				input_event(input, EV_ABS, ABS_X, nd->x);
-				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
-			input_event(input, EV_ABS, ABS_MT_POSITION_X, nd->x);
-			input_event(input, EV_ABS, ABS_MT_POSITION_Y, nd->y);
+
+			/* If the contact was invalid */
+			if (!(nd->confidence && nd->mt_footer[0])
+					|| nd->w <= 250
+					|| nd->h <= 190) {
+				nd->contacts[nd->id].x = 0;
+				nd->contacts[nd->id].y = 0;
+				nd->contacts[nd->id].confidence = 0;
+				break;
+			}
+
+			nd->contacts[nd->id].logical_id = -1;
+			nd->contacts[nd->id].confidence = nd->confidence;
+			nd->contacts[nd->id].x = nd->x;
+			nd->contacts[nd->id].y = nd->y;
+
 			if (nd->w > nd->h) {
-				input_event(input, EV_ABS,
-						ABS_MT_ORIENTATION, 1);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MAJOR, nd->w);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MINOR, nd->h);
+				nd->contacts[nd->id].orientation = 1;
+				nd->contacts[nd->id].touch_major = nd->w;
+				nd->contacts[nd->id].touch_minor = nd->h;
 			} else {
-				input_event(input, EV_ABS,
-						ABS_MT_ORIENTATION, 0);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MAJOR, nd->h);
-				input_event(input, EV_ABS,
-						ABS_MT_TOUCH_MINOR, nd->w);
+				nd->contacts[nd->id].orientation = 0;
+				nd->contacts[nd->id].touch_major = nd->h;
+				nd->contacts[nd->id].touch_minor = nd->w;
+			}
+			break;
+
+		case HID_DG_CONTACTCOUNT:
+			/* This marks the end of the multitouch group */
+			nd->contact_count = value;
+			if (nd->reading_mt) {
+				nd->reading_mt = 0;
+				ntrig_conclude_mt(input, nd);
 			}
-			input_mt_sync(field->hidinput->input);
-			nd->reading_a_point = 0;
-			nd->found_contact_id = 0;
 			break;
 
 		default:
@@ -224,8 +368,16 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		dev_err(&hdev->dev, "cannot allocate N-Trig data\n");
 		return -ENOMEM;
 	}
-	nd->reading_a_point = 0;
-	nd->found_contact_id = 0;
+
+	/* Initialize the driver data to sane values */
+	nd->id = 0;
+	nd->reading_mt = 0;
+	nd->contact_count = 0;
+	nd->max_width = 0x500;
+	nd->max_height = 0x500;
+	nd->max_contacts = 5;
+	nd->emit_ghosts = 0;
+
 	hid_set_drvdata(hdev, nd);
 
 	ret = hid_parse(hdev);
@@ -254,11 +406,19 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			 * the multitouch only has logical input
 			 * fields.
 			 */
-			input->name =
-				(hidinput->report->field[0]
-				 ->physical) ?
-				"N-Trig Touchscreen" :
-				"N-Trig MultiTouch";
+			if (hidinput->report->field[0]->physical) {
+				input->name = "N-Trig Touchscreen";
+				nd->st_input = input;
+
+				/*
+				 * A little something special to enable
+				 * two and three finger taps.
+				 */
+				set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
+				set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
+				set_bit(BTN_TOOL_QUADTAP, input->keybit);
+			} else
+				input->name = "N-Trig MultiTouch";
 			break;
 		}
 	}
@@ -280,6 +440,7 @@ static const struct hid_device_id ntrig_devices[] = {
 		.driver_data = NTRIG_DUPLICATE_USAGES },
 	{ }
 };
+
 MODULE_DEVICE_TABLE(hid, ntrig_devices);
 
 static const struct hid_usage_id ntrig_grabbed_usages[] = {
-- 
1.6.6.1

[PATCH 4/4] hid-ntrig: Contact tracking

[Rafi Rubin]

Added contact tracking to compensate for the lack of tracking in
hardware.  The approach is simplistic and leaves considerable
room for improvement.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |  216 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 183 insertions(+), 33 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 3da7287..ba092f6 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -64,7 +64,9 @@ struct ntrig_data {
 
 	/* Collected state for 2 full sets of contacts */
 	struct ntrig_contact contacts[NTRIG_MAX_CONTACTS];
+	struct ntrig_contact prev_contacts[NTRIG_MAX_CONTACTS];
 	__u8 contact_count;
+	__u8 prev_contact_count;
 
 	__u8 mt_footer[4];
 	__u8 mt_foot_count;
@@ -117,18 +119,15 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_DIGITIZER:
 		switch (usage->hid) {
 		/* we do not want to map these for now */
-		case HID_DG_CONTACTID: /* value is useless */
 		case HID_DG_INPUTMODE:
 		case HID_DG_DEVICEINDEX:
-		case HID_DG_CONTACTCOUNT:
 		case HID_DG_CONTACTMAX:
 			return -1;
 
-		/* original mapping by Rafi Rubin */
-		case HID_DG_CONFIDENCE:
-			nt_map_key_clear(BTN_TOOL_DOUBLETAP);
+		case HID_DG_CONTACTID:
+			hid_map_usage(hi, usage, bit, max, EV_ABS,
+					ABS_MT_TRACKING_ID);
 			return 1;
-
 		/* width/height mapped on TouchMajor/TouchMinor/Orientation */
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
@@ -166,43 +165,191 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 	return 0;
 }
 
-static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
+static void ntrig_single_touch_emit(struct input_dev *input,
+				    struct ntrig_data *nd)
 {
-	int i;
-	struct ntrig_contact *contact = &nd->contacts[0];
-
-	/* Emit single touch events */
-	if (contact->confidence) {
+	if (nd->confidence) {
 		switch (nd->contact_count) {
 		case 0:	/* for single touch devices */
 		case 1:
-			input_report_key(nd->st_input, BTN_TOOL_DOUBLETAP, 1);
+			input_report_key(input, BTN_TOOL_DOUBLETAP, 1);
 			break;
 		case 2:
-			input_report_key(nd->st_input, BTN_TOOL_TRIPLETAP, 1);
+			input_report_key(input, BTN_TOOL_TRIPLETAP, 1);
 			break;
 		case 3:
 		default:
-			input_report_key(nd->st_input, BTN_TOOL_QUADTAP, 1);
+			input_report_key(input, BTN_TOOL_QUADTAP, 1);
+		}
+		input_report_key(input, BTN_TOUCH, 1);
+		input_event(input, EV_ABS, ABS_X, nd->x);
+		input_event(input, EV_ABS, ABS_Y, nd->y);
+	} else {
+		/* No active fingers, clear all state */
+		input_report_key(input, BTN_TOUCH, 0);
+		input_report_key(input, BTN_TOOL_DOUBLETAP, 0);
+		input_report_key(input, BTN_TOOL_TRIPLETAP, 0);
+		input_report_key(input, BTN_TOOL_QUADTAP, 0);
+	}
+}
+
+/*
+ * Spatial comparison of two points.  If the difference
+ * is within the given thresholds they are treated as the
+ * same point.
+ */
+#define nt_same_point(a, b, max_dx, max_dy) ( \
+		(abs(a.x - b.x) <= max_dx) && \
+		(abs(a.y - b.y) <= max_dy))
+
+/*
+ * To verify a new contact matches a contact in the previous
+ * group, ensure both are valid then check spatial correlation.
+ */
+#define nt_match_points(nd, new, old) (nd->contacts[new].confidence && \
+		nd->prev_contacts[old].confidence && \
+		nt_same_point(nd->contacts[new], nd->prev_contacts[old], \
+			nd->max_width, nd->max_height))
+
+/*
+ * After an older contact is identified as a match nt_map_match updates
+ * the newer point as well as the contact map
+ */
+static inline void nt_map_match(struct ntrig_data *nd, __s8 *contact_map,
+		int new, int old)
+{
+	nd->contacts[new].logical_id = nd->prev_contacts[old].logical_id;
+	contact_map[nd->contacts[new].logical_id] = new;
+}
+
+static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
+{
+	__s8 contact_map[nd->max_contacts];
+	int i, j, k;
+	int matched = 0;
+	int first_free_id = 0;
+	int count = nd->contact_count;
+	int prev_count = nd->prev_contact_count;
+	struct ntrig_contact *contact;
+
+	/* If the previous state is corrupted, discard it. */
+	if (nd->prev_contact_count >= NTRIG_MAX_CONTACTS) {
+		printk(KERN_ERR
+		       "N-Trig previous state corrupted, discarding\n");
+		nd->prev_contact_count = 0;
+		prev_count = 0;
+	}
+
+	/* Under some circumstances an empty group is emitted with an invalid
+	 * contact 0 and contact count of 1. */
+	if (count && (!nd->contacts[0].confidence)) {
+		count = 0;
+		nd->contact_count = 0;
+	}
+
+	/* Contact tracking logic */
+
+	/* Initialize and empty logical id map */
+	for (i = 0; i < nd->max_contacts; i++)
+		contact_map[i] = -1;
+
+	/*
+	 * Phase 1: Identify which contacts seem to match
+	 * those with the same physical id from the previous group.
+	 * This should be the most common case during long touch
+	 * action. */
+	for (i = 0; i < count && i < prev_count; i++) {
+		if (nt_match_points(nd, i, i)) {
+			nt_map_match(nd, contact_map, i, i);
+			matched++;
+		} else
+			nd->contacts[i].logical_id = -1;
+	}
+
+	/*
+	 * Phase 2: Find corresponding contacts when the incoming
+	 * order has changed.
+	 */
+	for (i = 0; i < count && matched < count; i++) {
+		for (j = 0; j < prev_count &&
+		     (nd->contacts[i].logical_id < 0); j++) {
+
+			/* Check the map to avoid reusing an old contact
+			 * for multiple current contacts */
+			if ((contact_map[nd->prev_contacts[j].logical_id] < 0)
+			    && nt_match_points(nd, i, j)) {
+				nt_map_match(nd, contact_map, i, j);
+				matched++;
+			}
 		}
-		input_report_key(nd->st_input, BTN_TOUCH, 1);
-		input_event(nd->st_input, EV_ABS, ABS_X, contact->x);
-		input_event(nd->st_input, EV_ABS, ABS_Y, contact->y);
+	}
+
+	/*
+	 * Phase 3: New or unidentied contacts are assigned logical ids.
+	 */
+	for (i = 0; i < count && matched < count; i++) {
+		/* Ignore points that are already mapped */
+		if ((nd->contacts[i].confidence
+		     && nd->contacts[i].logical_id < 0)) {
+			/* find the first available logical id */
+			while (contact_map[first_free_id] >= 0
+			       && first_free_id < nd->max_contacts)
+				first_free_id++;
+			if (first_free_id >= nd->max_contacts) {
+				printk(KERN_ERR
+				       "hid-ntrig: exceeded contacts limit\n");
+				break;
+			}
+			nd->contacts[i].logical_id = first_free_id;
+			contact_map[first_free_id++] = i;
+			matched++;
+		}
+	}
+
+	k = -1;			/* Lowest id contact */
+	j = -1;			/* Highest id contact */
+	if (nd->emit_ghosts < 2) {
+		for (i = 0; i < nd->max_contacts; i++)
+			if (contact_map[i] >= 0) {
+				j = i;
+				if (k < 0)
+					k = i;
+			}
 	} else {
-		input_report_key(nd->st_input, BTN_TOUCH, 0);
-		input_report_key(nd->st_input, BTN_TOOL_DOUBLETAP, 0);
-		input_report_key(nd->st_input, BTN_TOOL_TRIPLETAP, 0);
-		input_report_key(nd->st_input, BTN_TOOL_QUADTAP, 0);
+		j = nd->max_contacts - 1;
+		for (i = 0; i < nd->max_contacts; i++)
+			if (contact_map[i] >= 0) {
+				k = i;
+				break;
+			}
+	}
+
+	/* Update the classic touchscreen state */
+	if (count) {
+		nd->x = nd->contacts[contact_map[k]].x;
+		nd->y = nd->contacts[contact_map[k]].y;
+		nd->confidence = nd->contacts[contact_map[k]].confidence;
+		ntrig_single_touch_emit(nd->st_input, nd);
+	} else {
+		/* Hit the end of activity, clear state */
+		nd->confidence = 0;
+		ntrig_single_touch_emit(nd->st_input, nd);
 	}
 
 	/* Multitouch doesn't need to send an end of activity notice. */
-	if (!(nd->contact_count && nd->contacts[0].confidence))
+	if (!(count && nd->contacts[0].confidence)) {
+		/* Only need to tag this group as empty, don't have
+		 * to worry about preserving all the empty state */
+		nd->prev_contact_count = 0;
 		return;
+	}
 
 	/* Emit multitouch events */
-	for (i = 0; i <= nd->max_contacts && i < NTRIG_MAX_CONTACTS; i++) {
-		if (nd->contacts[i].confidence) {
-			struct ntrig_contact *contact = &nd->contacts[i];
+	for (i = 0; i <= j; i++) {
+		/* Valid contact, send real values */
+		if (contact_map[i] >= 0
+		    && nd->contacts[contact_map[i]].confidence) {
+			contact = &nd->contacts[contact_map[i]];
 			input_event(input, EV_ABS, ABS_MT_TRACKING_ID, i);
 			input_event(input, EV_ABS, ABS_MT_POSITION_X,
 					contact->x);
@@ -226,6 +373,11 @@ static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
 			input_mt_sync(input);
 		}
 	}
+
+	/* Age the current state to previous. */
+	for (i = 0; i < count; i++)
+		nd->prev_contacts[i] = nd->contacts[i];
+	nd->prev_contact_count = nd->contact_count;
 }
 
 /*
@@ -234,7 +386,7 @@ static void ntrig_conclude_mt(struct input_dev *input, struct ntrig_data *nd)
  * decide whether we are in multi or single touch mode
  * and call input_mt_sync after each point if necessary
  */
-static int ntrig_event (struct hid_device *hid, struct hid_field *field,
+static int ntrig_event(struct hid_device *hid, struct hid_field *field,
 		                        struct hid_usage *usage, __s32 value)
 {
 	struct input_dev *input = field->hidinput->input;
@@ -244,7 +396,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 	if (field->application == HID_DG_PEN)
 		return 0;
 
-        if (hid->claimed & HID_CLAIMED_INPUT) {
+	if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
 		case 0xff000001:
 			/* Tag indicating the start of a multitouch group */
@@ -275,8 +427,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * to emit a normal (X, Y) position
 			 */
 			if (!nd->reading_mt) {
-				input_event(input, EV_ABS, ABS_X, nd->x);
-				input_event(input, EV_ABS, ABS_Y, nd->y);
+				ntrig_single_touch_emit(input, nd);
 			}
 			break;
 		case 0xff000002:
@@ -307,8 +458,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
 			/* If the contact was invalid */
 			if (!(nd->confidence && nd->mt_footer[0])
-					|| nd->w <= 250
-					|| nd->h <= 190) {
+					|| nd->w <= 250 || nd->h <= 190) {
 				nd->contacts[nd->id].x = 0;
 				nd->contacts[nd->id].y = 0;
 				nd->contacts[nd->id].confidence = 0;
@@ -373,6 +523,7 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	nd->id = 0;
 	nd->reading_mt = 0;
 	nd->contact_count = 0;
+	nd->prev_contact_count = 0;
 	nd->max_width = 0x500;
 	nd->max_height = 0x500;
 	nd->max_contacts = 5;
@@ -392,7 +543,6 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_free;
 	}
 
-
 	list_for_each_entry(hidinput, &hdev->inputs, list) {
 		input = hidinput->input;
 		switch (hidinput->report->field[0]->application) {
-- 
1.6.6.1

Re: [PATCH 4/4] hid-ntrig: Contact tracking

[Mohamed Ikbel Boulabiar]

Hi,

What happens when two fingers approaches then separates ?

Re: [PATCH 4/4] hid-ntrig: Contact tracking

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/20/10 03:25, Mohamed Ikbel Boulabiar wrote:
> Hi,
> 
> What happens when two fingers approaches then separates ?

Given the way the hardware numbers the contacts and the way that code processes
them, the persistent id would go to the finger closer to the bottom of the
screen and the other one would get assigned an inactive id.

Similarly I think the lower finger would most likely determine the id of the
merged contact.  Not entirely certain.

Longer term tracking for a more satisfying and reliable tracking takes more
processing.  I think that's why the protocol doc suggests that userspace should
be responsible for hardware that doesn't track.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkuAIHkACgkQwuRiAT9o609k+QCcCHXx/DRuhEOWpcWaZrTLfubh
cPAAoL+TMEiNtil9hh/KX5oJIKejchNT
=oQbI
-----END PGP SIGNATURE-----

Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Dmitry Torokhov]

On Thu, Feb 11, 2010 at 10:14:07PM -0500, Rafi Rubin wrote:
> +
> +	/* options */
> +	enum ntrig_mt_emit_ghosts emit_ghosts;

How can user change this option? Why would he want to do it?

> +	nd->emit_ghosts = 0;

That should be "nd->emit_ghosts = NTRIG_MT_EMIT_GHOSTS_NONE;"

The same goes for your other patch comparing emit_ghosts with 2.

-- 
Dmitry

Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch.

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/12/10 03:13, Dmitry Torokhov wrote:
> On Thu, Feb 11, 2010 at 10:14:07PM -0500, Rafi Rubin wrote:
>> +
>> +	/* options */
>> +	enum ntrig_mt_emit_ghosts emit_ghosts;
> 
> How can user change this option? Why would he want to do it?

If I move it to a module parameter is there some particular style to handle
enumerations or should I change it back to an int?  Or even a bool.

And if I also wanted to make max contacts a parameter is there any reason to
worry about people putting in multiple devices handled by this driver and
wanting different caps for each?


Anyway, the reason I have the max_contacts and 2 ghost options is a bit of
uncertainty about the protocol.  Perhaps we should clear that up first.

Henrik: you wrote the protocol doc, perhaps you can clarify this.

My understanding is that mt contacts should be emitted sparsely.  And we should
not worry about emitting ghosts.  If so I'd be happy to pull that.

The document also states that the kernel should not do finger tracking.  In this
case, the hardware is reporting tracking id's even though it is not actually
performing the tracking.  Given the history of behavior change on each firmware
update, it would not surprise me if newer firmware and newer devices might
actually start sending valid id's.  Is it better to track or to block emission
of the id's to userspace?

Also, keep in mind that finger tracking is useful for the single touch emulation.


For now, I'll remove the ghost code and will write a new patch later if needed.

Rafi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt14UcACgkQwuRiAT9o60865ACg+MQta7w6FLqXO0lwDslGM79g
RdMAn2EjxFyK3KYmn+4uORvuHjvUI0aY
=V5aK
-----END PGP SIGNATURE-----

[PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

This cleans up the identification of multitouch groups and enables
the end of group sync.

Taps are now explicitly handled to adjust for the changes in the
event stream in multitouch mode.  Added triple and quad tap for the
benefit of tools that recognize different tap types but do not have
full multi touch support.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |  116 ++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 94 insertions(+), 22 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 1bda3a4..f6f882d 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -25,8 +25,16 @@
 					EV_KEY, (c))
 
 struct ntrig_data {
-	__s32 x, y, id, w, h;
-	bool reading_a_point, found_contact_id;
+	/* Incoming raw values for a single contact */
+	__u16 x, y, w, h;
+	__u16 id;
+	__u8 confidence;
+
+	bool reading_mt;
+	__u8 first_contact_confidence;
+
+	__u8 mt_footer[4];
+	__u8 mt_foot_count;
 };
 
 /*
@@ -39,8 +47,8 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen */
-	if (field->application == HID_DG_PEN)
+	/* No special mappings needed for the pen and single touch */
+	if (field->physical)
 		return 0;
 
 	switch (usage->hid & HID_USAGE_PAGE) {
@@ -66,18 +74,12 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_DIGITIZER:
 		switch (usage->hid) {
 		/* we do not want to map these for now */
-		case HID_DG_CONTACTID: /* value is useless */
+		case HID_DG_CONTACTID: /* Not trustworthy, squelch for now */
 		case HID_DG_INPUTMODE:
 		case HID_DG_DEVICEINDEX:
-		case HID_DG_CONTACTCOUNT:
 		case HID_DG_CONTACTMAX:
 			return -1;
 
-		/* original mapping by Rafi Rubin */
-		case HID_DG_CONFIDENCE:
-			nt_map_key_clear(BTN_TOOL_DOUBLETAP);
-			return 1;
-
 		/* width/height mapped on TouchMajor/TouchMinor/Orientation */
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
@@ -104,8 +106,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/* No special mappings needed for the pen */
-	if (field->application == HID_DG_PEN)
+	/* No special mappings needed for the pen and single touch */
+	if (field->physical)
 		return 0;
 
 	if (usage->type == EV_KEY || usage->type == EV_REL
@@ -133,17 +135,24 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 
         if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
+		case 0xff000001:
+			/* Tag indicating the start of a multitouch group */
+			nd->reading_mt = 1;
+			nd->first_contact_confidence = 0;
+			break;
+		case HID_DG_CONFIDENCE:
+			nd->confidence = value;
+			break;
 		case HID_GD_X:
 			nd->x = value;
-			nd->reading_a_point = 1;
+			/* Clear the contact footer */
+			nd->mt_foot_count = 0;
 			break;
 		case HID_GD_Y:
 			nd->y = value;
 			break;
 		case HID_DG_CONTACTID:
 			nd->id = value;
-			/* we receive this only when in multitouch mode */
-			nd->found_contact_id = 1;
 			break;
 		case HID_DG_WIDTH:
 			nd->w = value;
@@ -155,7 +164,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * report received in a finger event. We want
 			 * to emit a normal (X, Y) position
 			 */
-			if (!nd->found_contact_id) {
+			if (!nd->reading_mt) {
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -167,10 +176,34 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			 * this usage tells if the contact point is real
 			 * or a placeholder
 			 */
-			if (!nd->reading_a_point || value != 1)
+
+			/* Shouldn't get more than 4 footer packets, so skip */
+			if (nd->mt_foot_count >= 4)
+				break;
+
+			nd->mt_footer[nd->mt_foot_count++] = value;
+
+			/* if the footer isn't complete break */
+			if (nd->mt_foot_count != 4)
+				break;
+
+			/* Pen activity signal, trigger end of touch. */
+			if (nd->mt_footer[2]) {
+				nd->confidence = 0;
+				break;
+			}
+
+			/* If the contact was invalid */
+			if (!(nd->confidence && nd->mt_footer[0])
+					|| nd->w <= 250
+					|| nd->h <= 190) {
+				nd->confidence = 0;
 				break;
+			}
+
 			/* emit a normal (X, Y) for the first point only */
 			if (nd->id == 0) {
+				nd->first_contact_confidence = nd->confidence;
 				input_event(input, EV_ABS, ABS_X, nd->x);
 				input_event(input, EV_ABS, ABS_Y, nd->y);
 			}
@@ -192,8 +225,39 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 						ABS_MT_TOUCH_MINOR, nd->w);
 			}
 			input_mt_sync(field->hidinput->input);
-			nd->reading_a_point = 0;
-			nd->found_contact_id = 0;
+			break;
+
+		case HID_DG_CONTACTCOUNT: /* End of a multitouch group */
+			if (!nd->reading_mt)
+				break;
+
+			nd->reading_mt = 0;
+
+			if (nd->first_contact_confidence) {
+				switch (value) {
+				case 0:	/* for single touch devices */
+				case 1:
+					input_report_key(input,
+							BTN_TOOL_DOUBLETAP, 1);
+					break;
+				case 2:
+					input_report_key(input,
+							BTN_TOOL_TRIPLETAP, 1);
+					break;
+				case 3:
+				default:
+					input_report_key(input,
+							BTN_TOOL_QUADTAP, 1);
+				}
+				input_report_key(input, BTN_TOUCH, 1);
+			} else {
+				input_report_key(input,
+						BTN_TOOL_DOUBLETAP, 0);
+				input_report_key(input,
+						BTN_TOOL_TRIPLETAP, 0);
+				input_report_key(input,
+						BTN_TOOL_QUADTAP, 0);
+			}
 			break;
 
 		default:
@@ -224,8 +288,8 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		dev_err(&hdev->dev, "cannot allocate N-Trig data\n");
 		return -ENOMEM;
 	}
-	nd->reading_a_point = 0;
-	nd->found_contact_id = 0;
+
+	nd->reading_mt = 0;
 	hid_set_drvdata(hdev, nd);
 
 	ret = hid_parse(hdev);
@@ -248,6 +312,14 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			input->name = "N-Trig Pen";
 			break;
 		case HID_DG_TOUCHSCREEN:
+			__clear_bit(BTN_TOOL_PEN, input->keybit);
+			/*
+			 * A little something special to enable
+			 * two and three finger taps.
+			 */
+			__set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
+			__set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
+			__set_bit(BTN_TOOL_QUADTAP, input->keybit);
 			/*
 			 * The physical touchscreen (single touch)
 			 * input has a value for physical, whereas
-- 
1.6.6.1

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This patch builds on the second of the 4 previous patches.  The latter 2 should
be ignored for now.

This cleans up the behavior particularly for the latest firmware.  That firmware
really doesn't work well with the version of the driver in the linus tree, hence
my push to fix that and more.

I will leave the more aggressive alterations until we get some feedback on
proper handling of multitouch and the question of redirecting single touch to a
separate device.

In this form, when multitouch is active, both mt and st events will come out of
the "N-Trig MultiTouch" device.  And when its not st events will come out of
"N-Trig Touchscreen".

Rafi

On 02/12/10 21:13, Rafi Rubin wrote:
> This cleans up the identification of multitouch groups and enables
> the end of group sync.
> 
> Taps are now explicitly handled to adjust for the changes in the
> event stream in multitouch mode.  Added triple and quad tap for the
> benefit of tools that recognize different tap types but do not have
> full multi touch support.
> 
> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
> ---
>  drivers/hid/hid-ntrig.c |  116 ++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 94 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 1bda3a4..f6f882d 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -25,8 +25,16 @@
>  					EV_KEY, (c))
>  
>  struct ntrig_data {
> -	__s32 x, y, id, w, h;
> -	bool reading_a_point, found_contact_id;
> +	/* Incoming raw values for a single contact */
> +	__u16 x, y, w, h;
> +	__u16 id;
> +	__u8 confidence;
> +
> +	bool reading_mt;
> +	__u8 first_contact_confidence;
> +
> +	__u8 mt_footer[4];
> +	__u8 mt_foot_count;
>  };
>  
>  /*
> @@ -39,8 +47,8 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> -	/* No special mappings needed for the pen */
> -	if (field->application == HID_DG_PEN)
> +	/* No special mappings needed for the pen and single touch */
> +	if (field->physical)
>  		return 0;
>  
>  	switch (usage->hid & HID_USAGE_PAGE) {
> @@ -66,18 +74,12 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  	case HID_UP_DIGITIZER:
>  		switch (usage->hid) {
>  		/* we do not want to map these for now */
> -		case HID_DG_CONTACTID: /* value is useless */
> +		case HID_DG_CONTACTID: /* Not trustworthy, squelch for now */
>  		case HID_DG_INPUTMODE:
>  		case HID_DG_DEVICEINDEX:
> -		case HID_DG_CONTACTCOUNT:
>  		case HID_DG_CONTACTMAX:
>  			return -1;
>  
> -		/* original mapping by Rafi Rubin */
> -		case HID_DG_CONFIDENCE:
> -			nt_map_key_clear(BTN_TOOL_DOUBLETAP);
> -			return 1;
> -
>  		/* width/height mapped on TouchMajor/TouchMinor/Orientation */
>  		case HID_DG_WIDTH:
>  			hid_map_usage(hi, usage, bit, max,
> @@ -104,8 +106,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> -	/* No special mappings needed for the pen */
> -	if (field->application == HID_DG_PEN)
> +	/* No special mappings needed for the pen and single touch */
> +	if (field->physical)
>  		return 0;
>  
>  	if (usage->type == EV_KEY || usage->type == EV_REL
> @@ -133,17 +135,24 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  
>          if (hid->claimed & HID_CLAIMED_INPUT) {
>  		switch (usage->hid) {
> +		case 0xff000001:
> +			/* Tag indicating the start of a multitouch group */
> +			nd->reading_mt = 1;
> +			nd->first_contact_confidence = 0;
> +			break;
> +		case HID_DG_CONFIDENCE:
> +			nd->confidence = value;
> +			break;
>  		case HID_GD_X:
>  			nd->x = value;
> -			nd->reading_a_point = 1;
> +			/* Clear the contact footer */
> +			nd->mt_foot_count = 0;
>  			break;
>  		case HID_GD_Y:
>  			nd->y = value;
>  			break;
>  		case HID_DG_CONTACTID:
>  			nd->id = value;
> -			/* we receive this only when in multitouch mode */
> -			nd->found_contact_id = 1;
>  			break;
>  		case HID_DG_WIDTH:
>  			nd->w = value;
> @@ -155,7 +164,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  			 * report received in a finger event. We want
>  			 * to emit a normal (X, Y) position
>  			 */
> -			if (!nd->found_contact_id) {
> +			if (!nd->reading_mt) {
>  				input_event(input, EV_ABS, ABS_X, nd->x);
>  				input_event(input, EV_ABS, ABS_Y, nd->y);
>  			}
> @@ -167,10 +176,34 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  			 * this usage tells if the contact point is real
>  			 * or a placeholder
>  			 */
> -			if (!nd->reading_a_point || value != 1)
> +
> +			/* Shouldn't get more than 4 footer packets, so skip */
> +			if (nd->mt_foot_count >= 4)
> +				break;
> +
> +			nd->mt_footer[nd->mt_foot_count++] = value;
> +
> +			/* if the footer isn't complete break */
> +			if (nd->mt_foot_count != 4)
> +				break;
> +
> +			/* Pen activity signal, trigger end of touch. */
> +			if (nd->mt_footer[2]) {
> +				nd->confidence = 0;
> +				break;
> +			}
> +
> +			/* If the contact was invalid */
> +			if (!(nd->confidence && nd->mt_footer[0])
> +					|| nd->w <= 250
> +					|| nd->h <= 190) {
> +				nd->confidence = 0;
>  				break;
> +			}
> +
>  			/* emit a normal (X, Y) for the first point only */
>  			if (nd->id == 0) {
> +				nd->first_contact_confidence = nd->confidence;
>  				input_event(input, EV_ABS, ABS_X, nd->x);
>  				input_event(input, EV_ABS, ABS_Y, nd->y);
>  			}
> @@ -192,8 +225,39 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  						ABS_MT_TOUCH_MINOR, nd->w);
>  			}
>  			input_mt_sync(field->hidinput->input);
> -			nd->reading_a_point = 0;
> -			nd->found_contact_id = 0;
> +			break;
> +
> +		case HID_DG_CONTACTCOUNT: /* End of a multitouch group */
> +			if (!nd->reading_mt)
> +				break;
> +
> +			nd->reading_mt = 0;
> +
> +			if (nd->first_contact_confidence) {
> +				switch (value) {
> +				case 0:	/* for single touch devices */
> +				case 1:
> +					input_report_key(input,
> +							BTN_TOOL_DOUBLETAP, 1);
> +					break;
> +				case 2:
> +					input_report_key(input,
> +							BTN_TOOL_TRIPLETAP, 1);
> +					break;
> +				case 3:
> +				default:
> +					input_report_key(input,
> +							BTN_TOOL_QUADTAP, 1);
> +				}
> +				input_report_key(input, BTN_TOUCH, 1);
> +			} else {
> +				input_report_key(input,
> +						BTN_TOOL_DOUBLETAP, 0);
> +				input_report_key(input,
> +						BTN_TOOL_TRIPLETAP, 0);
> +				input_report_key(input,
> +						BTN_TOOL_QUADTAP, 0);
> +			}
>  			break;
>  
>  		default:
> @@ -224,8 +288,8 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		dev_err(&hdev->dev, "cannot allocate N-Trig data\n");
>  		return -ENOMEM;
>  	}
> -	nd->reading_a_point = 0;
> -	nd->found_contact_id = 0;
> +
> +	nd->reading_mt = 0;
>  	hid_set_drvdata(hdev, nd);
>  
>  	ret = hid_parse(hdev);
> @@ -248,6 +312,14 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  			input->name = "N-Trig Pen";
>  			break;
>  		case HID_DG_TOUCHSCREEN:
> +			__clear_bit(BTN_TOOL_PEN, input->keybit);
> +			/*
> +			 * A little something special to enable
> +			 * two and three finger taps.
> +			 */
> +			__set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
> +			__set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
> +			__set_bit(BTN_TOOL_QUADTAP, input->keybit);
>  			/*
>  			 * The physical touchscreen (single touch)
>  			 * input has a value for physical, whereas
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt2DV0ACgkQwuRiAT9o60+LjACgrDC+ZKOVTwuQOek8DSA6erCI
uvUAni5AqKR58xdzFNLFnDNK2CueR4a3
=cbvj
-----END PGP SIGNATURE-----

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Jiri Kosina]

On Fri, 12 Feb 2010, Rafi Rubin wrote:

> This patch builds on the second of the 4 previous patches.  The latter 2 should
> be ignored for now.
> 
> This cleans up the behavior particularly for the latest firmware.  That firmware
> really doesn't work well with the version of the driver in the linus tree, hence
> my push to fix that and more.
> 
> I will leave the more aggressive alterations until we get some feedback on
> proper handling of multitouch and the question of redirecting single touch to a
> separate device.
> 
> In this form, when multitouch is active, both mt and st events will come out of
> the "N-Trig MultiTouch" device.  And when its not st events will come out of
> "N-Trig Touchscreen".

Thanks for clarification. I have updated the changelog to provide a little 
bit more reasoning for the changes being made, and applied the patch.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

Jiri Kosina wrote:
> On Fri, 12 Feb 2010, Rafi Rubin wrote:
> 
>> This patch builds on the second of the 4 previous patches.  The latter 2 should
>> be ignored for now.
>>
>> This cleans up the behavior particularly for the latest firmware.  That firmware
>> really doesn't work well with the version of the driver in the linus tree, hence
>> my push to fix that and more.
>>
>> I will leave the more aggressive alterations until we get some feedback on
>> proper handling of multitouch and the question of redirecting single touch to a
>> separate device.
>>
>> In this form, when multitouch is active, both mt and st events will come out of
>> the "N-Trig MultiTouch" device.  And when its not st events will come out of
>> "N-Trig Touchscreen".
> 
> Thanks for clarification. I have updated the changelog to provide a little 
> bit more reasoning for the changes being made, and applied the patch.
> 

In hid-ntrig.c, I see BTN_TOUCH set to one in one place in the code, but no
place where it is set to zero.

Incidently, I have a documentation patch pending which further specifies how to
handle the fingers-off-screen events. Perhaps that is the issue here?

Henrik

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

On 03/09/2010 04:01 PM, Henrik Rydberg wrote:
> In hid-ntrig.c, I see BTN_TOUCH set to one in one place in the code, but no
> place where it is set to zero.

Oops.  I removed that accidentally when I backed off on some of the more 
aggressive ideas.

> Incidently, I have a documentation patch pending which further specifies how to
> handle the fingers-off-screen events. Perhaps that is the issue here?

Nope just purely a mistake.


Since you're considering protocol clarification, what's your opinion on 
splitting the multi-touch and single touch (possibly emulated) to 
separate input devices?

Also BTN_0 or BTN_TOOL_DOUBLETAP for single touch (or both)?  It seems 
to me Wacom uses doubletap, whereas my hardware's rdesc defaults to btn0 
and evdev prefers btn0 as well.


Rafi

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Jiri Kosina]

On Tue, 9 Mar 2010, Rafi Rubin wrote:

> Since you're considering protocol clarification, what's your opinion on 
> splitting the multi-touch and single touch (possibly emulated) to 
> separate input devices?

What would be the advantages?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Stéphane Chatty]

Le 9 mars 10 à 22:19, Jiri Kosina a écrit :

> On Tue, 9 Mar 2010, Rafi Rubin wrote:
>
>> Since you're considering protocol clarification, what's your  
>> opinion on
>> splitting the multi-touch and single touch (possibly emulated) to
>> separate input devices?
>
> What would be the advantages?

One would be separation of concerns. If I'm interested in single  
touch events, I'd be better off with no "multitouch noise". If I'm  
interested in low level events, I'd be better off without the  
interference created by all sorts of "clever" event emulation in  
drivers. An easy way to do this is to have a different input node for  
each protocol.

Dmitry has already replied to this that if protocols are independent  
there is no problem with multiplexing them on the same node (I'm  
rephrasing heavily, here). Still, with multiplexing things are a bit  
less clear for programmers; this is not a clean object interface  
protocol to say the least ;-) And sometimes there even are  
interferences between protocols...


On another note, having multiple input nodes is a relevant question  
when dealing with multitouch anyway. Let me take an example: consider  
two users, each interacting with their own application in its own  
window but on the same display. Now, consider these two input  
possibilities: either each has their own mouse, or they both use the  
same dual-touch panel. In the first case, each app can open its own  
input node; in the second, they need to share the same device and  
perform some filtering to extract the events they want (user1 or  
user2). The symmetry breaking between the two situations is not  
satisfactory: you need to change the code in the apps.

With this regard, I am a big fan of the idea of having hierarchical  
devices, just like with have hierarchical file systems. Each finger  
on the dual-touch panel would be a device, child of the panel device.  
Each would be equivalent to a mouse, and voila, the symmetry is  
restored. Conceptually, saying (panel/finger1, any event) or (panel,  
finger1 events) are equivalent; but in the first case the routing is  
done by the OS and in the second case it has to be done by the app,  
which breaks reusability. There are other interesting perspectives,  
but I don't want to get carried away too much.

St.



--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

> One would be separation of concerns. If I'm interested in single touch
> events, I'd be better off with no "multitouch noise". If I'm interested
> in low level events, I'd be better off without the interference created
> by all sorts of "clever" event emulation in drivers. An easy way to do
> this is to have a different input node for each protocol.

Keeping backward compatibility with single-touch userland does not really
clutter the driver -- adding single-touch events to a clean multi-touch driver
is just a handful of lines.

[snip]

> On another note, having multiple input nodes is a relevant question when
> dealing with multitouch anyway. Let me take an example: consider two
> users, each interacting with their own application in its own window but
> on the same display. Now, consider these two input possibilities: either
> each has their own mouse, or they both use the same dual-touch panel. In
> the first case, each app can open its own input node; in the second,
> they need to share the same device and perform some filtering to extract
> the events they want (user1 or user2). The symmetry breaking between the
> two situations is not satisfactory: you need to change the code in the
> apps.

Symmetry is also restored by passing all single-input devices as MT packets.

> With this regard, I am a big fan of the idea of having hierarchical
> devices, just like with have hierarchical file systems. Each finger on
> the dual-touch panel would be a device, child of the panel device. Each
> would be equivalent to a mouse, and voila, the symmetry is restored.
> Conceptually, saying (panel/finger1, any event) or (panel, finger1
> events) are equivalent; but in the first case the routing is done by the
> OS and in the second case it has to be done by the app, which breaks
> reusability. There are other interesting perspectives, but I don't want
> to get carried away too much.

A hierarchy is imposing an unnecessary restriction on the graph of possible
relations between point devices. Consider for instance the case of two people,
each with one finger on the panel. The hierarchy says panel-person1-finger1 and
panel-person2-finger1. Now have them move close enough for the fingers to touch.
The hierarchy now says panel-person-(finger1, finger2). Symmetry breaking once more.

The main point here is that however the data reaches userland, it will have to
be processed intelligibly and collectively. The point of processing could be an
MT X Driver, it could be some other input section, but it is has to be done
somewhere.

Henrik

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Mohamed Ikbel Boulabiar]

On Tue, Mar 9, 2010 at 11:25 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> One would be separation of concerns. If I'm interested in single touch
>> events, I'd be better off with no "multitouch noise". If I'm interested
>> in low level events, I'd be better off without the interference created
>> by all sorts of "clever" event emulation in drivers. An easy way to do
>> this is to have a different input node for each protocol.
>
> Keeping backward compatibility with single-touch userland does not really
> clutter the driver -- adding single-touch events to a clean multi-touch driver
> is just a handful of lines.
>
> [snip]
>
>> On another note, having multiple input nodes is a relevant question when
>> dealing with multitouch anyway. Let me take an example: consider two
>> users, each interacting with their own application in its own window but
>> on the same display. Now, consider these two input possibilities: either
>> each has their own mouse, or they both use the same dual-touch panel. In
>> the first case, each app can open its own input node; in the second,
>> they need to share the same device and perform some filtering to extract
>> the events they want (user1 or user2). The symmetry breaking between the
>> two situations is not satisfactory: you need to change the code in the
>> apps.
>
> Symmetry is also restored by passing all single-input devices as MT packets.
>
>> With this regard, I am a big fan of the idea of having hierarchical
>> devices, just like with have hierarchical file systems. Each finger on
>> the dual-touch panel would be a device, child of the panel device. Each
>> would be equivalent to a mouse, and voila, the symmetry is restored.
>> Conceptually, saying (panel/finger1, any event) or (panel, finger1
>> events) are equivalent; but in the first case the routing is done by the
>> OS and in the second case it has to be done by the app, which breaks
>> reusability. There are other interesting perspectives, but I don't want
>> to get carried away too much.
>
> A hierarchy is imposing an unnecessary restriction on the graph of possible
> relations between point devices. Consider for instance the case of two people,
> each with one finger on the panel. The hierarchy says panel-person1-finger1 and
> panel-person2-finger1. Now have them move close enough for the fingers to touch.
> The hierarchy now says panel-person-(finger1, finger2). Symmetry breaking once more.
>
> The main point here is that however the data reaches userland, it will have to
> be processed intelligibly and collectively. The point of processing could be an
> MT X Driver, it could be some other input section, but it is has to be done
> somewhere.
>
> Henrik


The hierarchy applied on multitouch isn't the best example to prove
benefits of it.
Hierarchy is useful with some complex input devices that have many
axes, many buttons some accelerometers, but that are hierarchical from
the source (integrality/separability ?).
Then providing them as hierarchy can be useful.


For multitouch devices, we don't need to make separation inside the
multitouch protocol itself even for "simpler" devices like "double
touch".

The solution maybe to have other handlers to show virtual hierarchical
devices in another virtual devices folder in addition to the old way.
The handler read from the usual device file and provide other sources.

Kernel modules will be then simple providing necessary input. And
complex handling will be in an additional layer.
User then will chose from where read the input : the old way or the
dynamic with handler special ways.


It should not also be in X.
If things aren't in the kernel, they shouldn't so be in X by obligation.

ik.

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

Mohamed Ikbel Boulabiar wrote:

> The hierarchy applied on multitouch isn't the best example to prove
> benefits of it.
> Hierarchy is useful with some complex input devices that have many
> axes, many buttons some accelerometers, but that are hierarchical from
> the source (integrality/separability ?).
> Then providing them as hierarchy can be useful.

I believe I see your's and Stephane's point. But while you argue about how to
get the devices to userspace in a clean fashion, I argue about how to process
arbitrary collection of devices in a clean fashion, once they get there.

> The solution maybe to have other handlers to show virtual hierarchical
> devices in another virtual devices folder in addition to the old way.
> The handler read from the usual device file and provide other sources.
> 
> Kernel modules will be then simple providing necessary input. And
> complex handling will be in an additional layer.
> User then will chose from where read the input : the old way or the
> dynamic with handler special ways.

I still have a feeling that the most workable, generic structure to impose on
input devices in general, is the empty set.

> It should not also be in X.
> If things aren't in the kernel, they shouldn't so be in X by obligation.

I completely agree -- the recent movement around xorg input, hal and udev
clearly shows how badly input needs its own layer.

Henrik

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Dmitry Torokhov]

On Wed, Mar 10, 2010 at 12:08:51AM +0100, Henrik Rydberg wrote:
> I completely agree -- the recent movement around xorg input, hal and udev
> clearly shows how badly input needs its own layer.
> 

Software is like an onion - hundreds of layers and all of them smell ;P

-- 
Dmitry

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

Dmitry Torokhov wrote:
> On Wed, Mar 10, 2010 at 12:08:51AM +0100, Henrik Rydberg wrote:
>> I completely agree -- the recent movement around xorg input, hal and udev
>> clearly shows how badly input needs its own layer.
>>
> 
> Software is like an onion - hundreds of layers and all of them smell ;P
> 

Comprised wisdom :-P

Good night,
Henrik

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Peter Hutterer]

On Tue, Mar 09, 2010 at 11:42:34PM +0100, Mohamed Ikbel Boulabiar wrote:
> > A hierarchy is imposing an unnecessary restriction on the graph of possible
> > relations between point devices. Consider for instance the case of two people,
> > each with one finger on the panel. The hierarchy says panel-person1-finger1 and
> > panel-person2-finger1. Now have them move close enough for the fingers to touch.
> > The hierarchy now says panel-person-(finger1, finger2). Symmetry breaking once more.
> >
> > The main point here is that however the data reaches userland, it will have to
> > be processed intelligibly and collectively. The point of processing could be an
> > MT X Driver, it could be some other input section, but it is has to be done
> > somewhere.
> >
> > Henrik
> 
> 
> The hierarchy applied on multitouch isn't the best example to prove
> benefits of it.
> Hierarchy is useful with some complex input devices that have many
> axes, many buttons some accelerometers, but that are hierarchical from
> the source (integrality/separability ?).
> Then providing them as hierarchy can be useful.

Are we talking about real input devices here or a hypothetical device?
If the former, what are examples for such an input device?

> For multitouch devices, we don't need to make separation inside the
> multitouch protocol itself even for "simpler" devices like "double
> touch".
> 
> The solution maybe to have other handlers to show virtual hierarchical
> devices in another virtual devices folder in addition to the old way.
> The handler read from the usual device file and provide other sources.
> 
> Kernel modules will be then simple providing necessary input. And
> complex handling will be in an additional layer.
> User then will chose from where read the input : the old way or the
> dynamic with handler special ways.
> 
> 
> It should not also be in X.
> If things aren't in the kernel, they shouldn't so be in X by obligation.

Don't forget that X' main functionality aside from displaying wobbly windows
is to be an input multiplexer. If some additional management layer is
needed, why should it be another layer on top of or below X instead of a
part of X itself?

Cheers,
  Peter

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Mohamed Ikbel Boulabiar]

On Thu, Mar 11, 2010 at 5:30 AM, Peter Hutterer
<peter.hutterer@who-t.net> wrote:
> On Tue, Mar 09, 2010 at 11:42:34PM +0100, Mohamed Ikbel Boulabiar wrote:
>> > A hierarchy is imposing an unnecessary restriction on the graph of possible
>> > relations between point devices. Consider for instance the case of two people,
>> > each with one finger on the panel. The hierarchy says panel-person1-finger1 and
>> > panel-person2-finger1. Now have them move close enough for the fingers to touch.
>> > The hierarchy now says panel-person-(finger1, finger2). Symmetry breaking once more.
>> >
>> > The main point here is that however the data reaches userland, it will have to
>> > be processed intelligibly and collectively. The point of processing could be an
>> > MT X Driver, it could be some other input section, but it is has to be done
>> > somewhere.
>> >
>> > Henrik
>>
>>
>> The hierarchy applied on multitouch isn't the best example to prove
>> benefits of it.
>> Hierarchy is useful with some complex input devices that have many
>> axes, many buttons some accelerometers, but that are hierarchical from
>> the source (integrality/separability ?).
>> Then providing them as hierarchy can be useful.
>
> Are we talking about real input devices here or a hypothetical device?
> If the former, what are examples for such an input device?

What's the meaning of a hypothetical device ?
Please note that here I speak about former input as to be handled by the kernel.
If there is more complex handling it can be treated outside and
virtual device files are created (to handle these hierarchical
concepts).
To eliminate misunderstanding, virtual device files represent what
Stéphane first suggested as moving multitouch input (as reported from
the kernel) to 2 mouse-like input reporting files.


>> For multitouch devices, we don't need to make separation inside the
>> multitouch protocol itself even for "simpler" devices like "double
>> touch".
>>
>> The solution maybe to have other handlers to show virtual hierarchical
>> devices in another virtual devices folder in addition to the old way.
>> The handler read from the usual device file and provide other sources.
>>
>> Kernel modules will be then simple providing necessary input. And
>> complex handling will be in an additional layer.
>> User then will chose from where read the input : the old way or the
>> dynamic with handler special ways.
>>
>>
>> It should not also be in X.
>> If things aren't in the kernel, they shouldn't so be in X by obligation.
>
> Don't forget that X' main functionality aside from displaying wobbly windows
> is to be an input multiplexer. If some additional management layer is
> needed, why should it be another layer on top of or below X instead of a
> part of X itself?


Ok, I know you represent now all the input handling in X.
So I should have predicted such questions. :)

The X evidence that it works as input multiplexer can't stay until the
end of time.
The number of projects emerging and that only use the Linux kernel
without X aren't few.
Why I should have access to advanced multi-touch handling ONLY in X
meaning I should install also all the package ?
Meaning in another software that should take care "mainly" of graphics.

I will here cite some things about the past and what's is being done now :
The graphics handling itself, wasn't done before in X ? And now isn't
everybody tried to pull it out of it and putting it in the Linux
kernel ? KMS: Kernel Mode Setting ? Isn't now better to have such
handling done in kernel and have early access to graphics card without
having the screen flashing many times disturbing users ? Isn't the
switching to VT now better ?

It's true that when X was first designed, its main functionalities
were handling input&graphics in 'all' unix systems (which by case
still remembers me Multics cause of failure...).
Now the world changed and we should think about what system would be
at least in the next 5 years.

Many projects tried to replace X, from Framebuffer to Wayland without
forgetting all other dead projects.
Why ? ;-)

Please don't see this as an attack to X.org, but otherwise a different
point of view.

When I've proposed, I have said that it can't be in X "by obligation".
I have thoughts that is may be an additional layer, until many people
decide to include it in the kernel if things become working great.
This can help much embedded application that use FB or a display
server but needs multi-touch and advanced input handling.
Specially when comparing the number of people developing X, to those
developing the Kernel.
In other non-Linux systems, multi-touch is handled by a completely new
input server.




Anyway, forget all what I have said.
How do you want to deal with multitouch and new input handling in X and why ?
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Peter Hutterer]

On Thu, Mar 11, 2010 at 06:36:26AM +0100, Mohamed Ikbel Boulabiar wrote:
> On Thu, Mar 11, 2010 at 5:30 AM, Peter Hutterer <peter.hutterer@who-t.net>
> wrote:
> > On Tue, Mar 09, 2010 at 11:42:34PM +0100, Mohamed Ikbel Boulabiar wrote:
> >> > A hierarchy is imposing an unnecessary restriction on the graph of
> >> > possible relations between point devices. Consider for instance the
> >> > case of two people, each with one finger on the panel. The hierarchy
> >> > says panel-person1-finger1 and panel-person2-finger1. Now have them
> >> > move close enough for the fingers to touch.  The hierarchy now says
> >> > panel-person-(finger1, finger2). Symmetry breaking once more.
> >> >
> >> > The main point here is that however the data reaches userland, it
> >> > will have to be processed intelligibly and collectively. The point of
> >> > processing could be an MT X Driver, it could be some other input
> >> > section, but it is has to be done somewhere.
> >> >
> >> > Henrik
> >>
> >>
> >> The hierarchy applied on multitouch isn't the best example to prove
> >> benefits of it.  Hierarchy is useful with some complex input devices
> >> that have many axes, many buttons some accelerometers, but that are
> >> hierarchical from the source (integrality/separability ?).  Then
> >> providing them as hierarchy can be useful.
> >
> > Are we talking about real input devices here or a hypothetical device?
> > If the former, what are examples for such an input device?
> 
> What's the meaning of a hypothetical device ?  Please note that here I
> speak about former input as to be handled by the kernel.  If there is more
> complex handling it can be treated outside and virtual device files are
> created (to handle these hierarchical concepts).  To eliminate
> misunderstanding, virtual device files represent what Stéphane first
> suggested as moving multitouch input (as reported from the kernel) to 2
> mouse-like input reporting files.

Ah, I think I remember now. The use-case here is something like having to
physical devices but the buttons of the second device are to be used as
buttons on the first device instead and it should all be transparent. Right?
I think we exchanged emails about this once.

> > Don't forget that X' main functionality aside from displaying wobbly
> > windows is to be an input multiplexer. If some additional management
> > layer is needed, why should it be another layer on top of or below X
> > instead of a part of X itself?
> 
> 
> Ok, I know you represent now all the input handling in X.  So I should
> have predicted such questions. :)
> 
> The X evidence that it works as input multiplexer can't stay until the end
> of time.  The number of projects emerging and that only use the Linux
> kernel without X aren't few.  Why I should have access to advanced
> multi-touch handling ONLY in X meaning I should install also all the
> package ?  Meaning in another software that should take care "mainly" of
> graphics.
> 
> I will here cite some things about the past and what's is being done now :
> The graphics handling itself, wasn't done before in X ? And now isn't
> everybody tried to pull it out of it and putting it in the Linux kernel ?
> KMS: Kernel Mode Setting ? Isn't now better to have such handling done in
> kernel and have early access to graphics card without having the screen
> flashing many times disturbing users ? Isn't the switching to VT now
> better ?
> 
> It's true that when X was first designed, its main functionalities were
> handling input&graphics in 'all' unix systems (which by case still
> remembers me Multics cause of failure...).  Now the world changed and we
> should think about what system would be at least in the next 5 years.
> 
> Many projects tried to replace X, from Framebuffer to Wayland without
> forgetting all other dead projects.  Why ? ;-)

The same argument is true for input of course. Since switching to evdev
we've removed the actual hardware handling into the kernel and don't do much
hardware-dependent stuff in the driver anymore. That didn't change the role
of the server as input multiplexer though, this is a different role. Let me
explain: A main role of the server is still to handle keyboard focus,
deliver events to the windows, handle multiple clients in a reasonably sane
manner, etc.  It's still the role of the server that if you move the mouse
and then the touchpad, the same cursor moves on the screen. This is what I
mean by input multiplexer, it converts abstracted hardware-events into
information that represent the use of this hardware in a graphical
interface.

So I don't worry at all about the kernel multitouch drivers because I know
I'll only ever see the event API and that's a good thing. We need to
consider that X still needs to manage your focus and other things - unless
you want to route around it completely (obviously, cases where X isn't in
use at all are different, they don't worry my too much though :)

So if you have a managing layer that couples devices together in a dynamic
manner, you need it below X (above X is _really_ hard). Which means the
clients that configure things need to talk to it in some way. If that way is
through X, then X needs to know about it somehow. Otherwise, you need some
configuration channel that goes around X. That aside, you still need the
drivers and the server be able to reconfigure themselves when this layer
modifies input devices.

It's easy enough to slot something in underneath, but you're likely to have
to have some additional stuff in the server anyway to manage it
appropriately. And what's "appropriate" is likely defined by some
application that has the context. And often enough, getting the context
without having access to the window hierarchy can be tricky.

> Please don't see this as an attack to X.org, but otherwise a different
> point of view.

no worries, I neither meant it as an attack on your approach nor felt you
were attacking X (not that I'd really care much about the latter, I've
worked with it for too long :).

> When I've proposed, I have said that it can't be in X "by obligation".
> I have thoughts that is may be an additional layer, until many people
> decide to include it in the kernel if things become working great.
> This can help much embedded application that use FB or a display
> server but needs multi-touch and advanced input handling.
> Specially when comparing the number of people developing X, to those
> developing the Kernel.
> In other non-Linux systems, multi-touch is handled by a completely new
> input server.
> 
> Anyway, forget all what I have said.
> How do you want to deal with multitouch and new input handling in X and why ?

I don't know yet. as it turns out, real multi-touch is tricky. so far none
of the approaches we've come up with survived the scrutiny. We're still
trying to find approaches that work for the generic case though.

Unfortunately, sidestepping the server by adding another communication
channel to the input layer would suffer from the same issues we've found so
far - usually the killer was the lack of context without active client
interaction.

Cheers,
  Peter

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Stéphane Chatty]

Le 11 mars 10 à 05:30, Peter Hutterer a écrit :

> On Tue, Mar 09, 2010 at 11:42:34PM +0100, Mohamed Ikbel Boulabiar  
> wrote:
>>> A hierarchy is imposing an unnecessary restriction on the graph  
>>> of possible
>>> relations between point devices. Consider for instance the case  
>>> of two people,
>>> each with one finger on the panel. The hierarchy says panel- 
>>> person1-finger1 and
>>> panel-person2-finger1. Now have them move close enough for the  
>>> fingers to touch.
>>> The hierarchy now says panel-person-(finger1, finger2). Symmetry  
>>> breaking once more.
>>>
>>> The main point here is that however the data reaches userland, it  
>>> will have to
>>> be processed intelligibly and collectively. The point of  
>>> processing could be an
>>> MT X Driver, it could be some other input section, but it is has  
>>> to be done
>>> somewhere.
>>>
>>> Henrik
>>
>>
>> The hierarchy applied on multitouch isn't the best example to prove
>> benefits of it.
>> Hierarchy is useful with some complex input devices that have many
>> axes, many buttons some accelerometers, but that are hierarchical  
>> from
>> the source (integrality/separability ?).
>> Then providing them as hierarchy can be useful.
>
> Are we talking about real input devices here or a hypothetical device?
> If the former, what are examples for such an input device?

A Wacom Cintiq is made of a touch panel, a bunch of buttons and two  
sliders. Wait, there also are the thousands of pens that Wacom  
shipped, each with a unique ID! If I own three of them, Wacom allows  
my app to be interested only in the events from pen number 2. So, in  
my mind, the Cintiq's touch panel is "made" of three pens. Two levels  
of hierarchy.

OK, I admit that there is some multitouch in this example. Let's take  
one without multitouch. I have an RFID reader and dozens of RFID tags  
embedded in identification badges. Say my app is only interested in  
badge #123 entering or leaving the proximity area of my reader. It's  
useful to consider badge #123 as a device with two events (enter/ 
leave), subdevice of the RFID system.

The left button of my mouse is a button. Why am I not allowed to  
focus on it as a device?

Today's application programmers have to bear with a definition of  
"device" that is centered on the hardware USB architecture that some  
electronic company has deemed optimal for their purposes. That  
definition has very little to do with any kind of semantic  
equivalence between device (same behaviour, same protocol), and even  
less to do with any kind of usage pattern. This was bearable when  
there were only mice, keyboards, and a few exotic devices; it will  
become more and more of a problem with ubiquitous computing on the  
horizon.

St.


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Dmitry Torokhov]

On Tue, Mar 09, 2010 at 11:03:07PM +0100, Stéphane Chatty wrote:
> 
> Le 9 mars 10 à 22:19, Jiri Kosina a écrit :
> 
> >On Tue, 9 Mar 2010, Rafi Rubin wrote:
> >
> >>Since you're considering protocol clarification, what's your
> >>opinion on
> >>splitting the multi-touch and single touch (possibly emulated) to
> >>separate input devices?
> >
> >What would be the advantages?
> 
> One would be separation of concerns. If I'm interested in single
> touch events, I'd be better off with no "multitouch noise". If I'm
> interested in low level events, I'd be better off without the
> interference created by all sorts of "clever" event emulation in
> drivers. An easy way to do this is to have a different input node
> for each protocol.
> 
> Dmitry has already replied to this that if protocols are independent
> there is no problem with multiplexing them on the same node (I'm
> rephrasing heavily, here). Still, with multiplexing things are a bit
> less clear for programmers; this is not a clean object interface
> protocol to say the least ;-) And sometimes there even are
> interferences between protocols...

One of the main problems with splitting data into several input devices
is that it is still the same physical device, but now is potentially
handled by 2 separate drivers (unless we manage to fold entire
userspaceinto a single driver which I doubt its a good idea - I think
synaptics/evdev split worked well for non-multitouch devices) causing
double events to be reported to userspace. It is /dev/input/mice all
over again.

> 
> 
> On another note, having multiple input nodes is a relevant question
> when dealing with multitouch anyway. Let me take an example:
> consider two users, each interacting with their own application in
> its own window but on the same display. Now, consider these two
> input possibilities: either each has their own mouse, or they both
> use the same dual-touch panel. In the first case, each app can open
> its own input node; in the second, they need to share the same
> device and perform some filtering to extract the events they want
> (user1 or user2). The symmetry breaking between the two situations
> is not satisfactory: you need to change the code in the apps.
> 
> With this regard, I am a big fan of the idea of having hierarchical
> devices, just like with have hierarchical file systems. Each finger
> on the dual-touch panel would be a device, child of the panel
> device. Each would be equivalent to a mouse, and voila, the symmetry
> is restored. Conceptually, saying (panel/finger1, any event) or
> (panel, finger1 events) are equivalent; but in the first case the
> routing is done by the OS and in the second case it has to be done
> by the app, which breaks reusability. There are other interesting
> perspectives, but I don't want to get carried away too much.

Theoretically it is nice but it practice the cases are differemt: with
mice you are dealing with 2 separate devices whereas with touchscreen
there is one and it is mater of interpretation whether 2 touches should
be taken as independent events or a complex gestures.


-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

>> With this regard, I am a big fan of the idea of having hierarchical
>> devices, just like with have hierarchical file systems. Each finger
>> on the dual-touch panel would be a device, child of the panel
>> device. Each would be equivalent to a mouse, and voila, the symmetry
>> is restored. Conceptually, saying (panel/finger1, any event) or
>> (panel, finger1 events) are equivalent; but in the first case the
>> routing is done by the OS and in the second case it has to be done
>> by the app, which breaks reusability. There are other interesting
>> perspectives, but I don't want to get carried away too much.
>
> Theoretically it is nice but it practice the cases are differemt: with
> mice you are dealing with 2 separate devices whereas with touchscreen
> there is one and it is mater of interpretation whether 2 touches should
> be taken as independent events or a complex gestures.

I see your point, reporting the position on separate axes (ABS_X and 
ABS_MT_X) isn't as problematic as seeing TOUCH turn on twice.

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Stéphane Chatty]

Le 9 mars 10 à 23:27, Dmitry Torokhov a écrit :
>> With this regard, I am a big fan of the idea of having hierarchical
>> devices, just like with have hierarchical file systems. Each finger
>> on the dual-touch panel would be a device, child of the panel
>> device. Each would be equivalent to a mouse, and voila, the symmetry
>> is restored. Conceptually, saying (panel/finger1, any event) or
>> (panel, finger1 events) are equivalent; but in the first case the
>> routing is done by the OS and in the second case it has to be done
>> by the app, which breaks reusability. There are other interesting
>> perspectives, but I don't want to get carried away too much.
>
> Theoretically it is nice but it practice the cases are differemt: with
> mice you are dealing with 2 separate devices whereas with touchscreen
> there is one and it is mater of interpretation whether 2 touches  
> should
> be taken as independent events or a complex gestures.

Yes, but the whole point is that relying on the apps for the  
interpretation is not always the right approach. In my "two users"  
scenario, say one is using OpenOffice and the other is using a  
browser. There's no reason why OpenOffice and the browser should  
bother with multitouch. It is rather up to the OS (not necessarily  
the kernel, probably rather X) to provide for the appropriate  
configuration choice.

In other words: seen from inside the apps, the difference between  
having one hardware device (the touchscreen) vs two hardware devices  
(two mice) just does not make any sense :-)

Another example: NTrig's panel has two sensors, one that detects a  
pen and on that detects fingers; they have chosen to multiplex the  
two sensors through one USB interface; but is it one device or two  
devices? As a programmer I don't know and I should not care.  
Hierarchical devices would be a way for me to forget about this  
irrelevant issue. Leave it to the OS and the user to choose which is  
the right configuration.

St.

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

On 03/09/2010 04:19 PM, Jiri Kosina wrote:
> On Tue, 9 Mar 2010, Rafi Rubin wrote:
>
>> Since you're considering protocol clarification, what's your opinion on
>> splitting the multi-touch and single touch (possibly emulated) to
>> separate input devices?
>
> What would be the advantages?

With the N-Trig device it would more closely match the device rdesc.  At 
least for the firmwares I've had a chance to examine.  hid device 
describes 3 input devices: 1. pen, 2. multi touch, 3. single touch.

Depending on the mode, the hardware sends single touch events to either 
the mt report (mt modes) or single touch report.

If mt/st multiplexing is preferred, perhaps it would be better to adjust 
the device naming, and possibly collapse the second and third devices 
into a single device.

Either way, I think it would be more aesthetically pleasing to keep the 
behavior of the input devices as consistent as possible and not having 
events jump from dev to dev based on the direction of the wind.


The only real reason I see to argue in favor of splitting the two 
streams is as a crutch to inadequate user space tools.  So perhaps 
that's not even worth discussing.


So how can I prevent the third input from being allocated without 
eliminating the second?

Rafi

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

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

Dmitry,

Since the topic is being discussed right now, I might as well send the mt
documentation patches prematurely (I first wanted to hear Mika's view on this,
since he pointed out the problem).

Henrik

[-- Attachment #2: 0001-input-mt-clarify-the-no-finger-event.patch --]
[-- Type: text/x-diff, Size: 1455 bytes --]

>From ac8fda4c30f6ac682755151e23f4002a4ad8c1c5 Mon Sep 17 00:00:00 2001
From: Henrik Rydberg <rydberg@euromail.se>
Date: Tue, 9 Mar 2010 20:53:26 +0100
Subject: [PATCH] input: mt: clarify the no-finger event

The current documentation does not explicitly specify how to report
zero fingers, leaving a potential problem in the driver implementations
and giving no parsing directive to userland. This patch defines two
equally valid ways.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 Documentation/input/multi-touch-protocol.txt |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/Documentation/input/multi-touch-protocol.txt b/Documentation/input/multi-touch-protocol.txt
index 8490480..eacb226 100644
--- a/Documentation/input/multi-touch-protocol.txt
+++ b/Documentation/input/multi-touch-protocol.txt
@@ -68,6 +68,22 @@ like:
    SYN_MT_REPORT
    SYN_REPORT
 
+Here is the sequence after lifting one of the fingers:
+
+   ABS_MT_POSITION_X
+   ABS_MT_POSITION_Y
+   SYN_MT_REPORT
+   SYN_REPORT
+
+And here is the sequence after lifting the remaining finger:
+
+   SYN_MT_REPORT
+   SYN_REPORT
+
+If the driver reports one of BTN_TOUCH or ABS_PRESSURE in addition to the
+ABS_MT events, the last SYN_MT_REPORT event may be omitted. Otherwise, the
+last SYN_REPORT will be dropped by the input core, resulting in no
+zero-finger event reaching userland.
 
 Event Semantics
 ---------------
-- 
1.6.3.3


[-- Attachment #3: 0001-input-mt-update-the-status-of-the-Multitouch-X-drive.patch --]
[-- Type: text/x-diff, Size: 1518 bytes --]

>From d6c3b34c78f442a3d27ce99a5f12bb49ddab71b5 Mon Sep 17 00:00:00 2001
From: Henrik Rydberg <rydberg@euromail.se>
Date: Tue, 9 Mar 2010 21:21:31 +0100
Subject: [PATCH] input: mt: update the status of the Multitouch X driver project

The Multitouch X driver project has moved to alpha status.
This patch updates the documentation accordingly.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 Documentation/input/multi-touch-protocol.txt |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/Documentation/input/multi-touch-protocol.txt b/Documentation/input/multi-touch-protocol.txt
index eacb226..c0fc1c7 100644
--- a/Documentation/input/multi-touch-protocol.txt
+++ b/Documentation/input/multi-touch-protocol.txt
@@ -233,11 +233,6 @@ where examples can be found.
 difference between the contact position and the approaching tool position
 could be used to derive tilt.
 [2] The list can of course be extended.
-[3] The multi-touch X driver is currently in the prototyping stage. At the
-time of writing (April 2009), the MT protocol is not yet merged, and the
-prototype implements finger matching, basic mouse support and two-finger
-scrolling. The project aims at improving the quality of current multi-touch
-functionality available in the Synaptics X driver, and in addition
-implement more advanced gestures.
+[3] Multitouch X driver project: http://bitmath.org/code/multitouch/.
 [4] See the section on event computation.
 [5] See the section on finger tracking.
-- 
1.6.3.3

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

Rafi Rubin wrote:
> Since you're considering protocol clarification, what's your opinion on
> splitting the multi-touch and single touch (possibly emulated) to
> separate input devices?

The multi-touch protocol was designed to complement single-touch, not replace
it. The MT events were specifically designed to be ignored by current userland
applications, so there is no need to separate them into different devices.

On the contrary, the MT protocol is dependent on one of the single-touch events
BTN_TOUCH or ABS_PRESSURE to be present in the event stream for proper device
identification and for correct handling of zero-finger events. The latter
because at least one value (BTN_TOUCH or ABS_PRESSURE) must change state to pass
through input filtering when no fingers are present. This is what needs
clarification.

> Also BTN_0 or BTN_TOOL_DOUBLETAP for single touch (or both)?  It seems
> to me Wacom uses doubletap, whereas my hardware's rdesc defaults to btn0
> and evdev prefers btn0 as well.

Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
BTN_TOOL_DOUBLETAP is a two-finger event.

Henrik

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Stéphane Chatty]

Le 9 mars 10 à 22:59, Henrik Rydberg a écrit :
>
> On the contrary, the MT protocol is dependent on one of the single- 
> touch events
> BTN_TOUCH or ABS_PRESSURE to be present in the event stream for  
> proper device
> identification and for correct handling of zero-finger events. The  
> latter
> because at least one value (BTN_TOUCH or ABS_PRESSURE) must change  
> state to pass
> through input filtering when no fingers are present. This is what  
> needs
> clarification.

Someone has pointed this to me a few weeks ago and tbh I had the  
feeling that this is a bug in the protocol that we failed to notice  
last summer. I agree with you that BTN_TOUCH plugs the hole, but I  
don't find it very satisfactory that we need one protocol to inform  
another.

St.

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

Stéphane Chatty wrote:
> 
> Le 9 mars 10 à 22:59, Henrik Rydberg a écrit :
>>
>> On the contrary, the MT protocol is dependent on one of the
>> single-touch events
>> BTN_TOUCH or ABS_PRESSURE to be present in the event stream for proper
>> device
>> identification and for correct handling of zero-finger events. The latter
>> because at least one value (BTN_TOUCH or ABS_PRESSURE) must change
>> state to pass
>> through input filtering when no fingers are present. This is what needs
>> clarification.
> 
> Someone has pointed this to me a few weeks ago and tbh I had the feeling
> that this is a bug in the protocol that we failed to notice last summer.
> I agree with you that BTN_TOUCH plugs the hole, but I don't find it very
> satisfactory that we need one protocol to inform another.

Frankly, I never even imagined there would be pure MT devices without support
for the existing single-touch applications. But sure, it was an oversight at the
time.

Henrik

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Stéphane Chatty]

Le 9 mars 10 à 23:29, Henrik Rydberg a écrit :

> Stéphane Chatty wrote:
>>
>> Le 9 mars 10 à 22:59, Henrik Rydberg a écrit :
>>>
>>> On the contrary, the MT protocol is dependent on one of the
>>> single-touch events
>>> BTN_TOUCH or ABS_PRESSURE to be present in the event stream for  
>>> proper
>>> device
>>> identification and for correct handling of zero-finger events.  
>>> The latter
>>> because at least one value (BTN_TOUCH or ABS_PRESSURE) must change
>>> state to pass
>>> through input filtering when no fingers are present. This is what  
>>> needs
>>> clarification.
>>
>> Someone has pointed this to me a few weeks ago and tbh I had the  
>> feeling
>> that this is a bug in the protocol that we failed to notice last  
>> summer.
>> I agree with you that BTN_TOUCH plugs the hole, but I don't find  
>> it very
>> satisfactory that we need one protocol to inform another.
>
> Frankly, I never even imagined there would be pure MT devices  
> without support
> for the existing single-touch applications. But sure, it was an  
> oversight at the
> time.

In my case, not detecting it was a pure mistake, nothing else :-) And  
I've worked with formal models of input devices for decades, which  
makes it really embarrassing.

St.

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

On 03/09/2010 04:59 PM, Henrik Rydberg wrote:
> Rafi Rubin wrote:
>> Since you're considering protocol clarification, what's your opinion on
>> splitting the multi-touch and single touch (possibly emulated) to
>> separate input devices?
>
> The multi-touch protocol was designed to complement single-touch, not replace
> it. The MT events were specifically designed to be ignored by current userland
> applications, so there is no need to separate them into different devices.
>
> On the contrary, the MT protocol is dependent on one of the single-touch events
> BTN_TOUCH or ABS_PRESSURE to be present in the event stream for proper device
> identification and for correct handling of zero-finger events. The latter
> because at least one value (BTN_TOUCH or ABS_PRESSURE) must change state to pass
> through input filtering when no fingers are present. This is what needs
> clarification.
>
>> Also BTN_0 or BTN_TOOL_DOUBLETAP for single touch (or both)?  It seems
>> to me Wacom uses doubletap, whereas my hardware's rdesc defaults to btn0
>> and evdev prefers btn0 as well.
>
> Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
> BTN_TOOL_DOUBLETAP is a two-finger event.

That definitely makes sense to me.  I really don't know why wacom uses 
double tap there.

Is there any harm in letting through the BTN_0 events?  Is the "rdesc" 
mapping defined by the hardware or hid?

For my device touch contacts look like this:

Digitizers.TipSwitch ---> Key.Touch
Digitizers.InRange ---> Key.ToolPen
Digitizers.Confidence ---> Key.Btn0
Digitizers.ContactID ---> Sync.Report
GenericDesktop.X ---> Absolute.MTPositionX
GenericDesktop.Y ---> Absolute.MTPositionY
Digitizers.Width ---> Absolute.MTMajor
Digitizers.Height ---> Absolute.MTMinor
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report
ff00.0002 ---> Sync.Report

Hence some of my confusion about setting both Touch and another button 
(the ToolPen bit seemed less pertinent).


Rafi

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Henrik Rydberg]

Rafi Rubin wrote:
>> Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
>> BTN_TOOL_DOUBLETAP is a two-finger event.
> 
> That definitely makes sense to me.  I really don't know why wacom uses
> double tap there.
> 
> Is there any harm in letting through the BTN_0 events?  Is the "rdesc"
> mapping defined by the hardware or hid?

I cannot imagine BTN_0 will do any harm. In synaptics, the BTN_X buttons are
mapped to multibuttons without clear-cut usage. The button events that carry
well-defined semantic meaning with regard to touch screens are BTN_TOUCH (any
finger), BTN_TOOL_FINGER (one finger), BTN_TOOL_PEN (pen), BTN_TOOL_DOUBLETAP
(two fingers) and BTN_TOOL_TRIPLETAP (three fingers or more).

Henrik

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

On 03/09/2010 06:23 PM, Henrik Rydberg wrote:
> Rafi Rubin wrote:
>>> Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
>>> BTN_TOOL_DOUBLETAP is a two-finger event.
>>
>> That definitely makes sense to me.  I really don't know why wacom uses
>> double tap there.
>>
>> Is there any harm in letting through the BTN_0 events?  Is the "rdesc"
>> mapping defined by the hardware or hid?
>
> I cannot imagine BTN_0 will do any harm. In synaptics, the BTN_X buttons are
> mapped to multibuttons without clear-cut usage. The button events that carry
> well-defined semantic meaning with regard to touch screens are BTN_TOUCH (any
> finger), BTN_TOOL_FINGER (one finger), BTN_TOOL_PEN (pen), BTN_TOOL_DOUBLETAP
> (two fingers) and BTN_TOOL_TRIPLETAP (three fingers or more).
>
> Henrik
>

Perhaps we should have a general touch device document.

I looked at the evdev code.  And as you say for single touch devices, 
BTN_TOUCH is the bit that matters.  It only broken because I pulled the 
TOUCH <- 0 when I removed BTN_0.

So why does evdev associate BTN_TOOL_FINGER with touchpads (resulting in 
the wrong set of assumptions and a poorly calibrated touch screen)?  I 
suppose that's really a discussion for another list.


For what its worth, onions are tasty.

Rafi

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Dmitry Torokhov]

On Tue, Mar 09, 2010 at 06:38:22PM -0500, Rafi Rubin wrote:
> On 03/09/2010 06:23 PM, Henrik Rydberg wrote:
> >Rafi Rubin wrote:
> >>>Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
> >>>BTN_TOOL_DOUBLETAP is a two-finger event.
> >>
> >>That definitely makes sense to me.  I really don't know why wacom uses
> >>double tap there.
> >>
> >>Is there any harm in letting through the BTN_0 events?  Is the "rdesc"
> >>mapping defined by the hardware or hid?
> >
> >I cannot imagine BTN_0 will do any harm. In synaptics, the BTN_X buttons are
> >mapped to multibuttons without clear-cut usage. The button events that carry
> >well-defined semantic meaning with regard to touch screens are BTN_TOUCH (any
> >finger), BTN_TOOL_FINGER (one finger), BTN_TOOL_PEN (pen), BTN_TOOL_DOUBLETAP
> >(two fingers) and BTN_TOOL_TRIPLETAP (three fingers or more).
> >
> >Henrik
> >
> 
> Perhaps we should have a general touch device document.
> 
> I looked at the evdev code.  And as you say for single touch
> devices, BTN_TOUCH is the bit that matters.  It only broken because
> I pulled the TOUCH <- 0 when I removed BTN_0.
> 
> So why does evdev associate BTN_TOOL_FINGER with touchpads
> (resulting in the wrong set of assumptions and a poorly calibrated
> touch screen)?  I suppose that's really a discussion for another
> list.
> 

Historic reasons. When Synaptics driver was in works we needed an event
that is different from BTN_TOUCH to differentiate from touchscreens.

-- 
Dmitry

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

On 03/09/2010 06:42 PM, Dmitry Torokhov wrote:
> On Tue, Mar 09, 2010 at 06:38:22PM -0500, Rafi Rubin wrote:
>> On 03/09/2010 06:23 PM, Henrik Rydberg wrote:
>>> Rafi Rubin wrote:
>>>>> Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
>>>>> BTN_TOOL_DOUBLETAP is a two-finger event.
>>>>
>>>> That definitely makes sense to me.  I really don't know why wacom uses
>>>> double tap there.
>>>>
>>>> Is there any harm in letting through the BTN_0 events?  Is the "rdesc"
>>>> mapping defined by the hardware or hid?
>>>
>>> I cannot imagine BTN_0 will do any harm. In synaptics, the BTN_X buttons are
>>> mapped to multibuttons without clear-cut usage. The button events that carry
>>> well-defined semantic meaning with regard to touch screens are BTN_TOUCH (any
>>> finger), BTN_TOOL_FINGER (one finger), BTN_TOOL_PEN (pen), BTN_TOOL_DOUBLETAP
>>> (two fingers) and BTN_TOOL_TRIPLETAP (three fingers or more).
>>>
>>> Henrik
>>>
>>
>> Perhaps we should have a general touch device document.
>>
>> I looked at the evdev code.  And as you say for single touch
>> devices, BTN_TOUCH is the bit that matters.  It only broken because
>> I pulled the TOUCH<- 0 when I removed BTN_0.
>>
>> So why does evdev associate BTN_TOOL_FINGER with touchpads
>> (resulting in the wrong set of assumptions and a poorly calibrated
>> touch screen)?  I suppose that's really a discussion for another
>> list.
>>
>
> Historic reasons. When Synaptics driver was in works we needed an event
> that is different from BTN_TOUCH to differentiate from touchscreens.

So should I clear the BTN_TOOL_FINGER bit for ntrig, or push to clean up 
evdev?

At the moment nothing is actually expecting or emitting the finger 
events.  Given the move to actual mt support does it make sense to use 
finger, double and triple for single touch events?  I'd vote for letting 
the user space mt tools handle more advanced button clicks.

Rafi

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Dmitry Torokhov]

On Tue, Mar 09, 2010 at 07:32:57PM -0500, Rafi Rubin wrote:
> On 03/09/2010 06:42 PM, Dmitry Torokhov wrote:
> >On Tue, Mar 09, 2010 at 06:38:22PM -0500, Rafi Rubin wrote:
> >>On 03/09/2010 06:23 PM, Henrik Rydberg wrote:
> >>>Rafi Rubin wrote:
> >>>>>Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
> >>>>>BTN_TOOL_DOUBLETAP is a two-finger event.
> >>>>
> >>>>That definitely makes sense to me.  I really don't know why wacom uses
> >>>>double tap there.
> >>>>
> >>>>Is there any harm in letting through the BTN_0 events?  Is the "rdesc"
> >>>>mapping defined by the hardware or hid?
> >>>
> >>>I cannot imagine BTN_0 will do any harm. In synaptics, the BTN_X buttons are
> >>>mapped to multibuttons without clear-cut usage. The button events that carry
> >>>well-defined semantic meaning with regard to touch screens are BTN_TOUCH (any
> >>>finger), BTN_TOOL_FINGER (one finger), BTN_TOOL_PEN (pen), BTN_TOOL_DOUBLETAP
> >>>(two fingers) and BTN_TOOL_TRIPLETAP (three fingers or more).
> >>>
> >>>Henrik
> >>>
> >>
> >>Perhaps we should have a general touch device document.
> >>
> >>I looked at the evdev code.  And as you say for single touch
> >>devices, BTN_TOUCH is the bit that matters.  It only broken because
> >>I pulled the TOUCH<- 0 when I removed BTN_0.
> >>
> >>So why does evdev associate BTN_TOOL_FINGER with touchpads
> >>(resulting in the wrong set of assumptions and a poorly calibrated
> >>touch screen)?  I suppose that's really a discussion for another
> >>list.
> >>
> >
> >Historic reasons. When Synaptics driver was in works we needed an event
> >that is different from BTN_TOUCH to differentiate from touchscreens.
> 
> So should I clear the BTN_TOOL_FINGER bit for ntrig, or push to
> clean up evdev?
> 
> At the moment nothing is actually expecting or emitting the finger
> events.  Given the move to actual mt support does it make sense to
> use finger, double and triple for single touch events?  I'd vote for
> letting the user space mt tools handle more advanced button clicks.
> 

I'd opt for this as well; let's leave BTN_TOOL_FINGER to touchpads.

-- 
Dmitry

Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix

[Rafi Rubin]

On 03/09/2010 10:47 PM, Dmitry Torokhov wrote:
> On Tue, Mar 09, 2010 at 07:32:57PM -0500, Rafi Rubin wrote:
>> On 03/09/2010 06:42 PM, Dmitry Torokhov wrote:
>>> On Tue, Mar 09, 2010 at 06:38:22PM -0500, Rafi Rubin wrote:
>>>> On 03/09/2010 06:23 PM, Henrik Rydberg wrote:
>>>>> Rafi Rubin wrote:
>>>>>>> Single touch should be goverened by one of BTN_TOUCH or ABS_PRESSURE.
>>>>>>> BTN_TOOL_DOUBLETAP is a two-finger event.
>>>>>>
>>>>>> That definitely makes sense to me.  I really don't know why wacom uses
>>>>>> double tap there.
>>>>>>
>>>>>> Is there any harm in letting through the BTN_0 events?  Is the "rdesc"
>>>>>> mapping defined by the hardware or hid?
>>>>>
>>>>> I cannot imagine BTN_0 will do any harm. In synaptics, the BTN_X buttons are
>>>>> mapped to multibuttons without clear-cut usage. The button events that carry
>>>>> well-defined semantic meaning with regard to touch screens are BTN_TOUCH (any
>>>>> finger), BTN_TOOL_FINGER (one finger), BTN_TOOL_PEN (pen), BTN_TOOL_DOUBLETAP
>>>>> (two fingers) and BTN_TOOL_TRIPLETAP (three fingers or more).
>>>>>
>>>>> Henrik
>>>>>
>>>>
>>>> Perhaps we should have a general touch device document.
>>>>
>>>> I looked at the evdev code.  And as you say for single touch
>>>> devices, BTN_TOUCH is the bit that matters.  It only broken because
>>>> I pulled the TOUCH<- 0 when I removed BTN_0.
>>>>
>>>> So why does evdev associate BTN_TOOL_FINGER with touchpads
>>>> (resulting in the wrong set of assumptions and a poorly calibrated
>>>> touch screen)?  I suppose that's really a discussion for another
>>>> list.
>>>>
>>>
>>> Historic reasons. When Synaptics driver was in works we needed an event
>>> that is different from BTN_TOUCH to differentiate from touchscreens.
>>
>> So should I clear the BTN_TOOL_FINGER bit for ntrig, or push to
>> clean up evdev?
>>
>> At the moment nothing is actually expecting or emitting the finger
>> events.  Given the move to actual mt support does it make sense to
>> use finger, double and triple for single touch events?  I'd vote for
>> letting the user space mt tools handle more advanced button clicks.
>>
>
> I'd opt for this as well; let's leave BTN_TOOL_FINGER to touchpads.

Ok. I'll clear FINGER.  For now, I'll leave the incorrect DOUBLETAP as 
is to avoid breaking compatibility with the wacom driver.  Ugly or not, 
most people I've heard from who use this device use that driver.

What's the proper style for labeling code as deprecated and to be 
removed after viable alternatives are well established?

Rafi

[PATCH] hid: ntrig touch events

[rafi]

From: rafi <rafi@hex.seas.upenn.edu>

This reinstates the lost unpressing of BTN_TOUCH.  To prevent undesireably
touch toggles this also deals with tip switch events.

Added a trap to prevent going out of bounds for hidinputs with empty reports.

Clear bits of unused buttons which result in misidentification.

Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
---
 drivers/hid/hid-ntrig.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 3234c72..edcc0c4 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -140,6 +140,9 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 			nd->reading_mt = 1;
 			nd->first_contact_confidence = 0;
 			break;
+		case HID_DG_TIPSWITCH:
+			/* Prevent emission of touch until validated */
+			return 1;
 		case HID_DG_CONFIDENCE:
 			nd->confidence = value;
 			break;
@@ -259,6 +262,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
 						BTN_TOOL_TRIPLETAP, 0);
 				input_report_key(input,
 						BTN_TOOL_QUADTAP, 0);
+				input_report_key(input, BTN_TOUCH, 0);
 			}
 			break;
 
@@ -308,13 +312,20 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 
 	list_for_each_entry(hidinput, &hdev->inputs, list) {
+		if (hidinput->report->maxfield < 1)
+			continue;
+
 		input = hidinput->input;
 		switch (hidinput->report->field[0]->application) {
 		case HID_DG_PEN:
 			input->name = "N-Trig Pen";
 			break;
 		case HID_DG_TOUCHSCREEN:
+			/* These keys are redundant for fingers, clear them
+			 * to prevent incorrect identification */
 			__clear_bit(BTN_TOOL_PEN, input->keybit);
+			__clear_bit(BTN_TOOL_FINGER, input->keybit);
+			__clear_bit(BTN_0, input->keybit);
 			/*
 			 * A little something special to enable
 			 * two and three finger taps.
-- 
1.7.0

Re: [PATCH] hid: ntrig touch events

[Jiri Kosina]

On Wed, 10 Mar 2010, rafi@seas.upenn.edu wrote:

> This reinstates the lost unpressing of BTN_TOUCH.  To prevent undesireably
> touch toggles this also deals with tip switch events.
> 
> Added a trap to prevent going out of bounds for hidinputs with empty reports.
> 
> Clear bits of unused buttons which result in misidentification.
> 
> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
> ---
>  drivers/hid/hid-ntrig.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 3234c72..edcc0c4 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -140,6 +140,9 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  			nd->reading_mt = 1;
>  			nd->first_contact_confidence = 0;
>  			break;
> +		case HID_DG_TIPSWITCH:
> +			/* Prevent emission of touch until validated */
> +			return 1;
>  		case HID_DG_CONFIDENCE:
>  			nd->confidence = value;
>  			break;
> @@ -259,6 +262,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  						BTN_TOOL_TRIPLETAP, 0);
>  				input_report_key(input,
>  						BTN_TOOL_QUADTAP, 0);
> +				input_report_key(input, BTN_TOUCH, 0);
>  			}
>  			break;
>  
> @@ -308,13 +312,20 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  
>  
>  	list_for_each_entry(hidinput, &hdev->inputs, list) {
> +		if (hidinput->report->maxfield < 1)
> +			continue;
> +
>  		input = hidinput->input;
>  		switch (hidinput->report->field[0]->application) {
>  		case HID_DG_PEN:
>  			input->name = "N-Trig Pen";
>  			break;
>  		case HID_DG_TOUCHSCREEN:
> +			/* These keys are redundant for fingers, clear them
> +			 * to prevent incorrect identification */
>  			__clear_bit(BTN_TOOL_PEN, input->keybit);
> +			__clear_bit(BTN_TOOL_FINGER, input->keybit);
> +			__clear_bit(BTN_0, input->keybit);
>  			/*
>  			 * A little something special to enable
>  			 * two and three finger taps.

Thanks for the fix. I have altered changelog a little bit, and applied.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] hid: ntrig touch events

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/10/10 10:04, Jiri Kosina wrote:
> On Wed, 10 Mar 2010, rafi@seas.upenn.edu wrote:
> 
>> This reinstates the lost unpressing of BTN_TOUCH.  To prevent undesireably
>> touch toggles this also deals with tip switch events.
>>
>> Added a trap to prevent going out of bounds for hidinputs with empty reports.
>>
>> Clear bits of unused buttons which result in misidentification.
>>
>> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
>> ---
>>  drivers/hid/hid-ntrig.c |   11 +++++++++++
>>  1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
>> index 3234c72..edcc0c4 100644
>> --- a/drivers/hid/hid-ntrig.c
>> +++ b/drivers/hid/hid-ntrig.c
>> @@ -140,6 +140,9 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>>  			nd->reading_mt = 1;
>>  			nd->first_contact_confidence = 0;
>>  			break;
>> +		case HID_DG_TIPSWITCH:
>> +			/* Prevent emission of touch until validated */
>> +			return 1;
>>  		case HID_DG_CONFIDENCE:
>>  			nd->confidence = value;
>>  			break;
>> @@ -259,6 +262,7 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>>  						BTN_TOOL_TRIPLETAP, 0);
>>  				input_report_key(input,
>>  						BTN_TOOL_QUADTAP, 0);
>> +				input_report_key(input, BTN_TOUCH, 0);
>>  			}
>>  			break;
>>  
>> @@ -308,13 +312,20 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>  
>>  
>>  	list_for_each_entry(hidinput, &hdev->inputs, list) {
>> +		if (hidinput->report->maxfield < 1)
>> +			continue;
>> +
>>  		input = hidinput->input;
>>  		switch (hidinput->report->field[0]->application) {
>>  		case HID_DG_PEN:
>>  			input->name = "N-Trig Pen";
>>  			break;
>>  		case HID_DG_TOUCHSCREEN:
>> +			/* These keys are redundant for fingers, clear them
>> +			 * to prevent incorrect identification */
>>  			__clear_bit(BTN_TOOL_PEN, input->keybit);
>> +			__clear_bit(BTN_TOOL_FINGER, input->keybit);
>> +			__clear_bit(BTN_0, input->keybit);
>>  			/*
>>  			 * A little something special to enable
>>  			 * two and three finger taps.
> 
> Thanks for the fix. I have altered changelog a little bit, and applied.

Is this patch likely to get promoted soon (days, weeks)?

Rafi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkuiitMACgkQwuRiAT9o608cYgCgv9Rj4eDXJYVuW8tGZUfQ+U8W
y0EAnjZj21Cdlj6dvHvdceOXQunnhqiX
=dibf
-----END PGP SIGNATURE-----

Re: [PATCH] hid: ntrig touch events

[Jiri Kosina]

On Thu, 18 Mar 2010, Rafi Rubin wrote:

> Is this patch likely to get promoted soon (days, weeks)?

It's in Linus' tree already, commit 2886539d5e64.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] hid: ntrig touch events

[Rafi Rubin]

On 03/19/10 04:44, Jiri Kosina wrote:
> On Thu, 18 Mar 2010, Rafi Rubin wrote:
> 
>> Is this patch likely to get promoted soon (days, weeks)?
> 
> It's in Linus' tree already, commit 2886539d5e64.

Great, and such timing too.  :)

Re: [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching

[Jiri Kosina]

On Thu, 11 Feb 2010, Rafi Rubin wrote:

> With the pen and touch split apart, we no longer need to inject
> additional tool switching events.

Applied, thanks!

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Rafi Rubin]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This set is a resubmission of the previous with Dmitry's requests integrated.

Rafi

On 02/11/10 22:14, Rafi Rubin wrote:
> Added a quirk to enable distinct input devices.  The digitizer utilizes
> three inputs to represent pen, multitouch and a normal touch screen.
> 
> With the Pen partitioned, it behaves well and does not need special
> handling.
> 
> Also, I set names to the input devices to clarify the functions of the
> various inputs.
> 
> Signed-off-by: Rafi Rubin <rafi@seas.upenn.edu>
> ---
>  drivers/hid/hid-ntrig.c |   69 +++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 58 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 49ce69d..38b2364 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -26,10 +26,10 @@
>  
>  struct ntrig_data {
>  	__s32 x, y, id, w, h;
> -	char reading_a_point, found_contact_id;
> -	char pen_active;
> -	char finger_active;
> -	char inverted;
> +	bool reading_a_point, found_contact_id;
> +	bool pen_active;
> +	bool finger_active;
> +	bool inverted;
>  };
>  
>  /*
> @@ -42,6 +42,10 @@ static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> +	/* No special mappings needed for the pen */
> +	if (field->application == HID_DG_PEN)
> +		return 0;
> +
>  	switch (usage->hid & HID_USAGE_PAGE) {
>  
>  	case HID_UP_GENDESK:
> @@ -104,6 +108,9 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
>  		struct hid_field *field, struct hid_usage *usage,
>  		unsigned long **bit, int *max)
>  {
> +	/* No special mappings needed for the pen */
> +	if (field->application == HID_DG_PEN)
> +		return 0;
>  	if (usage->type == EV_KEY || usage->type == EV_REL
>  			|| usage->type == EV_ABS)
>  		clear_bit(usage->code, *bit);
> @@ -123,6 +130,10 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  	struct input_dev *input = field->hidinput->input;
>  	struct ntrig_data *nd = hid_get_drvdata(hid);
>  
> +	/* No special handling needed for the pen */
> +	if (field->application == HID_DG_PEN)
> +		return 0;
> +
>          if (hid->claimed & HID_CLAIMED_INPUT) {
>  		switch (usage->hid) {
>  
> @@ -231,8 +242,8 @@ static int ntrig_event (struct hid_device *hid, struct hid_field *field,
>  	}
>  
>  	/* we have handled the hidinput part, now remains hiddev */
> -        if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
> -                hid->hiddev_hid_event(hid, field, usage, value);
> +	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_hid_event)
> +		hid->hiddev_hid_event(hid, field, usage, value);
>  
>  	return 1;
>  }
> @@ -241,6 +252,11 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>  	int ret;
>  	struct ntrig_data *nd;
> +	struct hid_input *hidinput;
> +	struct input_dev *input;
> +
> +	if (id->driver_data)
> +		hdev->quirks |= HID_QUIRK_MULTI_INPUT;
>  
>  	nd = kmalloc(sizeof(struct ntrig_data), GFP_KERNEL);
>  	if (!nd) {
> @@ -252,12 +268,43 @@ static int ntrig_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	hid_set_drvdata(hdev, nd);
>  
>  	ret = hid_parse(hdev);
> -	if (!ret)
> -		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> +	if (ret) {
> +		dev_err(&hdev->dev, "parse failed\n");
> +		goto err_free;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> +	if (ret) {
> +		dev_err(&hdev->dev, "hw start failed\n");
> +		goto err_free;
> +	}
>  
> -	if (ret)
> -		kfree (nd);
>  
> +	list_for_each_entry(hidinput, &hdev->inputs, list) {
> +		input = hidinput->input;
> +		switch (hidinput->report->field[0]->application) {
> +		case HID_DG_PEN:
> +			input->name = "N-Trig Pen";
> +			break;
> +		case HID_DG_TOUCHSCREEN:
> +			/*
> +			 * The physical touchscreen (single touch)
> +			 * input has a value for physical, whereas
> +			 * the multitouch only has logical input
> +			 * fields.
> +			 */
> +			input->name =
> +				(hidinput->report->field[0]
> +				 ->physical) ?
> +				"N-Trig Touchscreen" :
> +				"N-Trig MultiTouch";
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +err_free:
> +	kfree(nd);
>  	return ret;
>  }
>  
> @@ -276,7 +323,7 @@ MODULE_DEVICE_TABLE(hid, ntrig_devices);
>  
>  static const struct hid_usage_id ntrig_grabbed_usages[] = {
>  	{ HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
> -	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
> +	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1 }
>  };
>  
>  static struct hid_driver ntrig_driver = {
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt0x9oACgkQwuRiAT9o60+eTwCdG+bbQ1MWV8QG5fQol7/eZyx4
omoAmwU3W1Prsx/JDIpc9IRrcBDUDjGB
=0On1
-----END PGP SIGNATURE-----

Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up

[Jiri Kosina]

On Thu, 11 Feb 2010, Rafi Rubin wrote:

> Added a quirk to enable distinct input devices.  The digitizer utilizes
> three inputs to represent pen, multitouch and a normal touch screen.
> 
> With the Pen partitioned, it behaves well and does not need special
> handling.
> 
> Also, I set names to the input devices to clarify the functions of the
> various inputs.

Applied, thank you.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.