* [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up @ 2010-02-12 0:19 Rafi Rubin 2010-02-12 0:19 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin ` (3 more replies) 0 siblings, 4 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 0:19 UTC (permalink / raw) To: linux-input, jkosina, dmitry.torokhov, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching 2010-02-12 0:19 [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin @ 2010-02-12 0:19 ` Rafi Rubin 2010-02-12 0:19 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin 2010-02-12 0:36 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Dmitry Torokhov ` (2 subsequent siblings) 3 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 0:19 UTC (permalink / raw) To: linux-input, jkosina, dmitry.torokhov, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 0:19 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin @ 2010-02-12 0:19 ` Rafi Rubin 2010-02-12 0:19 ` [PATCH 4/4] hid-ntrig: Contact tracking Rafi Rubin 2010-02-12 0:42 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov 0 siblings, 2 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 0:19 UTC (permalink / raw) To: linux-input, jkosina, dmitry.torokhov, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 4/4] hid-ntrig: Contact tracking 2010-02-12 0:19 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin @ 2010-02-12 0:19 ` Rafi Rubin 2010-02-12 0:42 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov 1 sibling, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 0:19 UTC (permalink / raw) To: linux-input, jkosina, dmitry.torokhov, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 0:19 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin 2010-02-12 0:19 ` [PATCH 4/4] hid-ntrig: Contact tracking Rafi Rubin @ 2010-02-12 0:42 ` Dmitry Torokhov 2010-02-12 3:10 ` Rafi Rubin 2010-02-12 10:41 ` Jiri Kosina 1 sibling, 2 replies; 62+ messages in thread From: Dmitry Torokhov @ 2010-02-12 0:42 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, jkosina, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 0:42 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov @ 2010-02-12 3:10 ` Rafi Rubin 2010-02-12 8:09 ` Dmitry Torokhov 2010-02-12 10:41 ` Jiri Kosina 1 sibling, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 3:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, jkosina, chatty -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 3:10 ` Rafi Rubin @ 2010-02-12 8:09 ` Dmitry Torokhov 2010-02-12 19:56 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Dmitry Torokhov @ 2010-02-12 8:09 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, jkosina, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 8:09 ` Dmitry Torokhov @ 2010-02-12 19:56 ` Rafi Rubin 0 siblings, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 19:56 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, jkosina, chatty -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 0:42 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov 2010-02-12 3:10 ` Rafi Rubin @ 2010-02-12 10:41 ` Jiri Kosina 1 sibling, 0 replies; 62+ messages in thread From: Jiri Kosina @ 2010-02-12 10:41 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Rafi Rubin, linux-input, chatty 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 0:19 [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin 2010-02-12 0:19 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin @ 2010-02-12 0:36 ` Dmitry Torokhov 2010-02-12 0:45 ` Rafi Rubin 2010-02-12 1:03 ` Rafi Rubin 2010-02-12 0:37 ` Rafi Rubin 2010-02-12 3:14 ` Rafi Rubin 3 siblings, 2 replies; 62+ messages in thread From: Dmitry Torokhov @ 2010-02-12 0:36 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, jkosina, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 0:36 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Dmitry Torokhov @ 2010-02-12 0:45 ` Rafi Rubin 2010-02-12 1:03 ` Rafi Rubin 1 sibling, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 0:45 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, jkosina, chatty -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 0:36 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Dmitry Torokhov 2010-02-12 0:45 ` Rafi Rubin @ 2010-02-12 1:03 ` Rafi Rubin 2010-02-12 1:20 ` Dmitry Torokhov 1 sibling, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 1:03 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, jkosina, chatty -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 1:03 ` Rafi Rubin @ 2010-02-12 1:20 ` Dmitry Torokhov 0 siblings, 0 replies; 62+ messages in thread From: Dmitry Torokhov @ 2010-02-12 1:20 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, jkosina, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 0:19 [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin 2010-02-12 0:19 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin 2010-02-12 0:36 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Dmitry Torokhov @ 2010-02-12 0:37 ` Rafi Rubin 2010-02-12 3:14 ` Rafi Rubin 3 siblings, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 0:37 UTC (permalink / raw) To: jkosina; +Cc: linux-input, dmitry.torokhov, chatty -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 0:19 [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin ` (2 preceding siblings ...) 2010-02-12 0:37 ` Rafi Rubin @ 2010-02-12 3:14 ` Rafi Rubin 2010-02-12 3:14 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin ` (2 more replies) 3 siblings, 3 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 3:14 UTC (permalink / raw) To: linux-input, dmitry.torokhov, jkosina, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching 2010-02-12 3:14 ` Rafi Rubin @ 2010-02-12 3:14 ` Rafi Rubin 2010-02-12 3:14 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin 2010-02-16 12:49 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Jiri Kosina 2010-02-12 3:15 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin 2010-02-16 12:49 ` Jiri Kosina 2 siblings, 2 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 3:14 UTC (permalink / raw) To: linux-input, dmitry.torokhov, jkosina, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 3:14 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin @ 2010-02-12 3:14 ` Rafi Rubin 2010-02-12 3:14 ` [PATCH 4/4] hid-ntrig: Contact tracking Rafi Rubin 2010-02-12 8:13 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov 2010-02-16 12:49 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Jiri Kosina 1 sibling, 2 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 3:14 UTC (permalink / raw) To: linux-input, dmitry.torokhov, jkosina, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH 4/4] hid-ntrig: Contact tracking 2010-02-12 3:14 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin @ 2010-02-12 3:14 ` Rafi Rubin 2010-02-20 8:29 ` Mohamed Ikbel Boulabiar [not found] ` <45cc95261002200025m378e1a80rec09bde5673a6060@mail.gmail.com> 2010-02-12 8:13 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov 1 sibling, 2 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 3:14 UTC (permalink / raw) To: linux-input, dmitry.torokhov, jkosina, chatty; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH 4/4] hid-ntrig: Contact tracking 2010-02-12 3:14 ` [PATCH 4/4] hid-ntrig: Contact tracking Rafi Rubin @ 2010-02-20 8:29 ` Mohamed Ikbel Boulabiar [not found] ` <45cc95261002200025m378e1a80rec09bde5673a6060@mail.gmail.com> 1 sibling, 0 replies; 62+ messages in thread From: Mohamed Ikbel Boulabiar @ 2010-02-20 8:29 UTC (permalink / raw) To: USB list Hi, What happens when two fingers approaches then separates ? ^ permalink raw reply [flat|nested] 62+ messages in thread
[parent not found: <45cc95261002200025m378e1a80rec09bde5673a6060@mail.gmail.com>]
* Re: [PATCH 4/4] hid-ntrig: Contact tracking [not found] ` <45cc95261002200025m378e1a80rec09bde5673a6060@mail.gmail.com> @ 2010-02-20 17:48 ` Rafi Rubin 0 siblings, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-20 17:48 UTC (permalink / raw) To: Mohamed Ikbel Boulabiar; +Cc: linux-input, dmitry.torokhov, jkosina, chatty -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 3:14 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin 2010-02-12 3:14 ` [PATCH 4/4] hid-ntrig: Contact tracking Rafi Rubin @ 2010-02-12 8:13 ` Dmitry Torokhov 2010-02-12 23:16 ` Rafi Rubin 1 sibling, 1 reply; 62+ messages in thread From: Dmitry Torokhov @ 2010-02-12 8:13 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, jkosina, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 3/4] hid-ntrig.c Split multi and single touch. 2010-02-12 8:13 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov @ 2010-02-12 23:16 ` Rafi Rubin 2010-02-13 2:13 ` [PATCH] hid-ntrig.c Multitouch cleanup and fix Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 23:16 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, jkosina, chatty, Henrik Rydberg -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-02-12 23:16 ` Rafi Rubin @ 2010-02-13 2:13 ` Rafi Rubin 2010-02-13 2:24 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-02-13 2:13 UTC (permalink / raw) To: linux-input, dmitry.torokhov, jkosina, chatty, rydberg; +Cc: 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-02-13 2:13 ` [PATCH] hid-ntrig.c Multitouch cleanup and fix Rafi Rubin @ 2010-02-13 2:24 ` Rafi Rubin 2010-02-16 12:50 ` Jiri Kosina 0 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-02-13 2:24 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, dmitry.torokhov, jkosina, chatty, rydberg -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-02-13 2:24 ` Rafi Rubin @ 2010-02-16 12:50 ` Jiri Kosina 2010-03-09 21:01 ` Henrik Rydberg 0 siblings, 1 reply; 62+ messages in thread From: Jiri Kosina @ 2010-02-16 12:50 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, dmitry.torokhov, chatty, rydberg 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-02-16 12:50 ` Jiri Kosina @ 2010-03-09 21:01 ` Henrik Rydberg 2010-03-09 21:17 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 21:01 UTC (permalink / raw) To: Jiri Kosina; +Cc: Rafi Rubin, linux-input, dmitry.torokhov, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 21:01 ` Henrik Rydberg @ 2010-03-09 21:17 ` Rafi Rubin 2010-03-09 21:19 ` Jiri Kosina 2010-03-09 21:59 ` Henrik Rydberg 0 siblings, 2 replies; 62+ messages in thread From: Rafi Rubin @ 2010-03-09 21:17 UTC (permalink / raw) To: Henrik Rydberg; +Cc: Jiri Kosina, linux-input, dmitry.torokhov, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 21:17 ` Rafi Rubin @ 2010-03-09 21:19 ` Jiri Kosina 2010-03-09 22:03 ` Stéphane Chatty 2010-03-09 22:12 ` Rafi Rubin 2010-03-09 21:59 ` Henrik Rydberg 1 sibling, 2 replies; 62+ messages in thread From: Jiri Kosina @ 2010-03-09 21:19 UTC (permalink / raw) To: Rafi Rubin; +Cc: Henrik Rydberg, linux-input, dmitry.torokhov, chatty 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 21:19 ` Jiri Kosina @ 2010-03-09 22:03 ` Stéphane Chatty 2010-03-09 22:25 ` Henrik Rydberg 2010-03-09 22:27 ` Dmitry Torokhov 2010-03-09 22:12 ` Rafi Rubin 1 sibling, 2 replies; 62+ messages in thread From: Stéphane Chatty @ 2010-03-09 22:03 UTC (permalink / raw) To: Jiri Kosina; +Cc: Rafi Rubin, Henrik Rydberg, linux-input, dmitry.torokhov 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:03 ` Stéphane Chatty @ 2010-03-09 22:25 ` Henrik Rydberg 2010-03-09 22:42 ` Mohamed Ikbel Boulabiar 2010-03-09 22:27 ` Dmitry Torokhov 1 sibling, 1 reply; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 22:25 UTC (permalink / raw) To: Stéphane Chatty Cc: Jiri Kosina, Rafi Rubin, linux-input, dmitry.torokhov > 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:25 ` Henrik Rydberg @ 2010-03-09 22:42 ` Mohamed Ikbel Boulabiar 2010-03-09 23:08 ` Henrik Rydberg 2010-03-11 4:30 ` Peter Hutterer 0 siblings, 2 replies; 62+ messages in thread From: Mohamed Ikbel Boulabiar @ 2010-03-09 22:42 UTC (permalink / raw) To: Henrik Rydberg Cc: Stéphane Chatty, Jiri Kosina, Rafi Rubin, linux-input, dmitry.torokhov 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:42 ` Mohamed Ikbel Boulabiar @ 2010-03-09 23:08 ` Henrik Rydberg 2010-03-09 23:17 ` Dmitry Torokhov 2010-03-11 4:30 ` Peter Hutterer 1 sibling, 1 reply; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 23:08 UTC (permalink / raw) To: Mohamed Ikbel Boulabiar Cc: Stéphane Chatty, Jiri Kosina, Rafi Rubin, linux-input, dmitry.torokhov 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 23:08 ` Henrik Rydberg @ 2010-03-09 23:17 ` Dmitry Torokhov 2010-03-09 23:26 ` Henrik Rydberg 0 siblings, 1 reply; 62+ messages in thread From: Dmitry Torokhov @ 2010-03-09 23:17 UTC (permalink / raw) To: Henrik Rydberg Cc: Mohamed Ikbel Boulabiar, Stéphane Chatty, Jiri Kosina, Rafi Rubin, linux-input 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 23:17 ` Dmitry Torokhov @ 2010-03-09 23:26 ` Henrik Rydberg 0 siblings, 0 replies; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 23:26 UTC (permalink / raw) To: Dmitry Torokhov Cc: Mohamed Ikbel Boulabiar, Stéphane Chatty, Jiri Kosina, Rafi Rubin, linux-input 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:42 ` Mohamed Ikbel Boulabiar 2010-03-09 23:08 ` Henrik Rydberg @ 2010-03-11 4:30 ` Peter Hutterer 2010-03-11 5:36 ` Mohamed Ikbel Boulabiar 2010-03-11 9:42 ` Stéphane Chatty 1 sibling, 2 replies; 62+ messages in thread From: Peter Hutterer @ 2010-03-11 4:30 UTC (permalink / raw) To: Mohamed Ikbel Boulabiar Cc: Henrik Rydberg, Stéphane Chatty, Jiri Kosina, Rafi Rubin, linux-input, dmitry.torokhov 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-11 4:30 ` Peter Hutterer @ 2010-03-11 5:36 ` Mohamed Ikbel Boulabiar 2010-03-11 6:25 ` Peter Hutterer 2010-03-11 9:42 ` Stéphane Chatty 1 sibling, 1 reply; 62+ messages in thread From: Mohamed Ikbel Boulabiar @ 2010-03-11 5:36 UTC (permalink / raw) To: Peter Hutterer Cc: Henrik Rydberg, Stéphane Chatty, Jiri Kosina, Rafi Rubin, linux-input, dmitry.torokhov 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-11 5:36 ` Mohamed Ikbel Boulabiar @ 2010-03-11 6:25 ` Peter Hutterer 0 siblings, 0 replies; 62+ messages in thread From: Peter Hutterer @ 2010-03-11 6:25 UTC (permalink / raw) To: Mohamed Ikbel Boulabiar Cc: Henrik Rydberg, Stéphane Chatty, Jiri Kosina, Rafi Rubin, linux-input, dmitry.torokhov 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-11 4:30 ` Peter Hutterer 2010-03-11 5:36 ` Mohamed Ikbel Boulabiar @ 2010-03-11 9:42 ` Stéphane Chatty 1 sibling, 0 replies; 62+ messages in thread From: Stéphane Chatty @ 2010-03-11 9:42 UTC (permalink / raw) To: Peter Hutterer Cc: Mohamed Ikbel Boulabiar, Henrik Rydberg, Jiri Kosina, Rafi Rubin, linux-input, dmitry.torokhov 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:03 ` Stéphane Chatty 2010-03-09 22:25 ` Henrik Rydberg @ 2010-03-09 22:27 ` Dmitry Torokhov 2010-03-09 22:32 ` Rafi Rubin 2010-03-09 22:54 ` Stéphane Chatty 1 sibling, 2 replies; 62+ messages in thread From: Dmitry Torokhov @ 2010-03-09 22:27 UTC (permalink / raw) To: Stéphane Chatty; +Cc: Jiri Kosina, Rafi Rubin, Henrik Rydberg, linux-input 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:27 ` Dmitry Torokhov @ 2010-03-09 22:32 ` Rafi Rubin 2010-03-09 22:54 ` Stéphane Chatty 1 sibling, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-03-09 22:32 UTC (permalink / raw) To: Dmitry Torokhov Cc: Stéphane Chatty, Jiri Kosina, Henrik Rydberg, linux-input >> 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:27 ` Dmitry Torokhov 2010-03-09 22:32 ` Rafi Rubin @ 2010-03-09 22:54 ` Stéphane Chatty 1 sibling, 0 replies; 62+ messages in thread From: Stéphane Chatty @ 2010-03-09 22:54 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Jiri Kosina, Rafi Rubin, Henrik Rydberg, linux-input 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 21:19 ` Jiri Kosina 2010-03-09 22:03 ` Stéphane Chatty @ 2010-03-09 22:12 ` Rafi Rubin 2010-03-09 22:39 ` Henrik Rydberg 1 sibling, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-03-09 22:12 UTC (permalink / raw) To: Jiri Kosina; +Cc: Henrik Rydberg, linux-input, dmitry.torokhov, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:12 ` Rafi Rubin @ 2010-03-09 22:39 ` Henrik Rydberg 0 siblings, 0 replies; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 22:39 UTC (permalink / raw) To: Rafi Rubin Cc: Jiri Kosina, linux-input, dmitry.torokhov, chatty, Mika.Kuoppala [-- 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 21:17 ` Rafi Rubin 2010-03-09 21:19 ` Jiri Kosina @ 2010-03-09 21:59 ` Henrik Rydberg 2010-03-09 22:11 ` Stéphane Chatty 2010-03-09 22:27 ` Rafi Rubin 1 sibling, 2 replies; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 21:59 UTC (permalink / raw) To: Rafi Rubin; +Cc: Jiri Kosina, linux-input, dmitry.torokhov, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 21:59 ` Henrik Rydberg @ 2010-03-09 22:11 ` Stéphane Chatty 2010-03-09 22:29 ` Henrik Rydberg 2010-03-09 22:27 ` Rafi Rubin 1 sibling, 1 reply; 62+ messages in thread From: Stéphane Chatty @ 2010-03-09 22:11 UTC (permalink / raw) To: Henrik Rydberg Cc: Rafi Rubin, Jiri Kosina, linux-input, Dmitry Torokhov, Peter Hutterer 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:11 ` Stéphane Chatty @ 2010-03-09 22:29 ` Henrik Rydberg 2010-03-09 22:44 ` Stéphane Chatty 0 siblings, 1 reply; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 22:29 UTC (permalink / raw) To: Stéphane Chatty Cc: Rafi Rubin, Jiri Kosina, linux-input, Dmitry Torokhov, Peter Hutterer 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:29 ` Henrik Rydberg @ 2010-03-09 22:44 ` Stéphane Chatty 0 siblings, 0 replies; 62+ messages in thread From: Stéphane Chatty @ 2010-03-09 22:44 UTC (permalink / raw) To: Henrik Rydberg Cc: Rafi Rubin, Jiri Kosina, linux-input, Dmitry Torokhov, Peter Hutterer 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 21:59 ` Henrik Rydberg 2010-03-09 22:11 ` Stéphane Chatty @ 2010-03-09 22:27 ` Rafi Rubin 2010-03-09 23:23 ` Henrik Rydberg 1 sibling, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-03-09 22:27 UTC (permalink / raw) To: Henrik Rydberg; +Cc: Jiri Kosina, linux-input, dmitry.torokhov, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 22:27 ` Rafi Rubin @ 2010-03-09 23:23 ` Henrik Rydberg 2010-03-09 23:38 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Henrik Rydberg @ 2010-03-09 23:23 UTC (permalink / raw) To: Rafi Rubin; +Cc: Jiri Kosina, linux-input, dmitry.torokhov, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 23:23 ` Henrik Rydberg @ 2010-03-09 23:38 ` Rafi Rubin 2010-03-09 23:42 ` Dmitry Torokhov 0 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-03-09 23:38 UTC (permalink / raw) To: Henrik Rydberg; +Cc: Jiri Kosina, linux-input, dmitry.torokhov, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 23:38 ` Rafi Rubin @ 2010-03-09 23:42 ` Dmitry Torokhov 2010-03-10 0:32 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Dmitry Torokhov @ 2010-03-09 23:42 UTC (permalink / raw) To: Rafi Rubin; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-09 23:42 ` Dmitry Torokhov @ 2010-03-10 0:32 ` Rafi Rubin 2010-03-10 3:47 ` Dmitry Torokhov 0 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-03-10 0:32 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-10 0:32 ` Rafi Rubin @ 2010-03-10 3:47 ` Dmitry Torokhov 2010-03-10 4:40 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Dmitry Torokhov @ 2010-03-10 3:47 UTC (permalink / raw) To: Rafi Rubin; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid-ntrig.c Multitouch cleanup and fix 2010-03-10 3:47 ` Dmitry Torokhov @ 2010-03-10 4:40 ` Rafi Rubin 2010-03-10 8:38 ` [PATCH] hid: ntrig touch events rafi 0 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-03-10 4:40 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Henrik Rydberg, Jiri Kosina, linux-input, chatty 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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] hid: ntrig touch events 2010-03-10 4:40 ` Rafi Rubin @ 2010-03-10 8:38 ` rafi 2010-03-10 15:04 ` Jiri Kosina 0 siblings, 1 reply; 62+ messages in thread From: rafi @ 2010-03-10 8:38 UTC (permalink / raw) To: Dmitry Torokhov, Henrik Rydberg, Jiri Kosina, linux-input, chatty Cc: rafi, Rafi Rubin 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] hid: ntrig touch events 2010-03-10 8:38 ` [PATCH] hid: ntrig touch events rafi @ 2010-03-10 15:04 ` Jiri Kosina 2010-03-18 20:19 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Jiri Kosina @ 2010-03-10 15:04 UTC (permalink / raw) To: Rafi Rubin; +Cc: Dmitry Torokhov, Henrik Rydberg, linux-input, chatty, rafi 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid: ntrig touch events 2010-03-10 15:04 ` Jiri Kosina @ 2010-03-18 20:19 ` Rafi Rubin 2010-03-19 8:44 ` Jiri Kosina 0 siblings, 1 reply; 62+ messages in thread From: Rafi Rubin @ 2010-03-18 20:19 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid: ntrig touch events 2010-03-18 20:19 ` Rafi Rubin @ 2010-03-19 8:44 ` Jiri Kosina 2010-03-19 14:12 ` Rafi Rubin 0 siblings, 1 reply; 62+ messages in thread From: Jiri Kosina @ 2010-03-19 8:44 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] hid: ntrig touch events 2010-03-19 8:44 ` Jiri Kosina @ 2010-03-19 14:12 ` Rafi Rubin 0 siblings, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-03-19 14:12 UTC (permalink / raw) To: Jiri Kosina; +Cc: linux-input 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. :) ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching 2010-02-12 3:14 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin 2010-02-12 3:14 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin @ 2010-02-16 12:49 ` Jiri Kosina 1 sibling, 0 replies; 62+ messages in thread From: Jiri Kosina @ 2010-02-16 12:49 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, dmitry.torokhov, chatty 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 3:14 ` Rafi Rubin 2010-02-12 3:14 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin @ 2010-02-12 3:15 ` Rafi Rubin 2010-02-16 12:49 ` Jiri Kosina 2 siblings, 0 replies; 62+ messages in thread From: Rafi Rubin @ 2010-02-12 3:15 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, dmitry.torokhov, jkosina, chatty -----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----- ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up 2010-02-12 3:14 ` Rafi Rubin 2010-02-12 3:14 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin 2010-02-12 3:15 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin @ 2010-02-16 12:49 ` Jiri Kosina 2 siblings, 0 replies; 62+ messages in thread From: Jiri Kosina @ 2010-02-16 12:49 UTC (permalink / raw) To: Rafi Rubin; +Cc: linux-input, dmitry.torokhov, chatty 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. ^ permalink raw reply [flat|nested] 62+ messages in thread
end of thread, other threads:[~2010-03-19 14:12 UTC | newest] Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-02-12 0:19 [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin 2010-02-12 0:19 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin 2010-02-12 0:19 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin 2010-02-12 0:19 ` [PATCH 4/4] hid-ntrig: Contact tracking Rafi Rubin 2010-02-12 0:42 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov 2010-02-12 3:10 ` Rafi Rubin 2010-02-12 8:09 ` Dmitry Torokhov 2010-02-12 19:56 ` Rafi Rubin 2010-02-12 10:41 ` Jiri Kosina 2010-02-12 0:36 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Dmitry Torokhov 2010-02-12 0:45 ` Rafi Rubin 2010-02-12 1:03 ` Rafi Rubin 2010-02-12 1:20 ` Dmitry Torokhov 2010-02-12 0:37 ` Rafi Rubin 2010-02-12 3:14 ` Rafi Rubin 2010-02-12 3:14 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Rafi Rubin 2010-02-12 3:14 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Rafi Rubin 2010-02-12 3:14 ` [PATCH 4/4] hid-ntrig: Contact tracking Rafi Rubin 2010-02-20 8:29 ` Mohamed Ikbel Boulabiar [not found] ` <45cc95261002200025m378e1a80rec09bde5673a6060@mail.gmail.com> 2010-02-20 17:48 ` Rafi Rubin 2010-02-12 8:13 ` [PATCH 3/4] hid-ntrig.c Split multi and single touch Dmitry Torokhov 2010-02-12 23:16 ` Rafi Rubin 2010-02-13 2:13 ` [PATCH] hid-ntrig.c Multitouch cleanup and fix Rafi Rubin 2010-02-13 2:24 ` Rafi Rubin 2010-02-16 12:50 ` Jiri Kosina 2010-03-09 21:01 ` Henrik Rydberg 2010-03-09 21:17 ` Rafi Rubin 2010-03-09 21:19 ` Jiri Kosina 2010-03-09 22:03 ` Stéphane Chatty 2010-03-09 22:25 ` Henrik Rydberg 2010-03-09 22:42 ` Mohamed Ikbel Boulabiar 2010-03-09 23:08 ` Henrik Rydberg 2010-03-09 23:17 ` Dmitry Torokhov 2010-03-09 23:26 ` Henrik Rydberg 2010-03-11 4:30 ` Peter Hutterer 2010-03-11 5:36 ` Mohamed Ikbel Boulabiar 2010-03-11 6:25 ` Peter Hutterer 2010-03-11 9:42 ` Stéphane Chatty 2010-03-09 22:27 ` Dmitry Torokhov 2010-03-09 22:32 ` Rafi Rubin 2010-03-09 22:54 ` Stéphane Chatty 2010-03-09 22:12 ` Rafi Rubin 2010-03-09 22:39 ` Henrik Rydberg 2010-03-09 21:59 ` Henrik Rydberg 2010-03-09 22:11 ` Stéphane Chatty 2010-03-09 22:29 ` Henrik Rydberg 2010-03-09 22:44 ` Stéphane Chatty 2010-03-09 22:27 ` Rafi Rubin 2010-03-09 23:23 ` Henrik Rydberg 2010-03-09 23:38 ` Rafi Rubin 2010-03-09 23:42 ` Dmitry Torokhov 2010-03-10 0:32 ` Rafi Rubin 2010-03-10 3:47 ` Dmitry Torokhov 2010-03-10 4:40 ` Rafi Rubin 2010-03-10 8:38 ` [PATCH] hid: ntrig touch events rafi 2010-03-10 15:04 ` Jiri Kosina 2010-03-18 20:19 ` Rafi Rubin 2010-03-19 8:44 ` Jiri Kosina 2010-03-19 14:12 ` Rafi Rubin 2010-02-16 12:49 ` [PATCH 2/4] hid-ntrig.c: removed unnecessary tool switching Jiri Kosina 2010-02-12 3:15 ` [PATCH 1/4] HID: hid-ntrig add multi input quirk and clean up Rafi Rubin 2010-02-16 12:49 ` Jiri Kosina
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).