| Message ID | 1352306256-12180-12-git-send-email-benjamin.tissoires@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Wed, Nov 07, 2012 at 05:37:34PM +0100, Benjamin Tissoires wrote: > Win8 devices supporting hovering must provides InRange HID field. provide the > The information that the finger is here but is not touching the surface > is sent to the user space through ABS_MT_DISTANCE as required by the > multitouch protocol. In the absence of more detailed information, use ABS_MT_DISTANCE with a [0,1] interval to distinguish between presence (1) and touch (0). > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> > --- > drivers/hid/hid-multitouch.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > index b393c6c..1f3d1e0 100644 > --- a/drivers/hid/hid-multitouch.c > +++ b/drivers/hid/hid-multitouch.c > @@ -59,6 +59,7 @@ struct mt_slot { > __s32 x, y, cx, cy, p, w, h; > __s32 contactid; /* the device ContactID assigned to this slot */ > bool touch_state; /* is the touch valid? */ > + bool inrange_state; /* is the finger in proximity of the sensor? */ > }; > > struct mt_class { > @@ -397,6 +398,12 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, > case HID_UP_DIGITIZER: > switch (usage->hid) { > case HID_DG_INRANGE: > + if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED) { > + hid_map_usage(hi, usage, bit, max, > + EV_ABS, ABS_MT_DISTANCE); > + input_set_abs_params(hi->input, > + ABS_MT_DISTANCE, -1, 1, 0, 0); Why [-1,1] here? > + } > mt_store_field(usage, td, hi); > td->last_field_index = field->index; > return 1; > @@ -516,6 +523,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) > if (slotnum < 0 || slotnum >= td->maxcontacts) > return; > > + if (!test_bit(ABS_MT_DISTANCE, input->absbit)) > + /* If InRange is not present, rely on TipSwitch */ > + s->inrange_state = s->touch_state; > + This could be skipped, see below. > if (td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES && > input_mt_is_active(&input->mt->slots[slotnum]) && > input_mt_is_used(input->mt, &input->mt->slots[slotnum])) > @@ -523,9 +534,9 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) > > input_mt_slot(input, slotnum); > input_mt_report_slot_state(input, MT_TOOL_FINGER, > - s->touch_state); > - if (s->touch_state) { > - /* this finger is on the screen */ > + s->inrange_state); > + if (s->inrange_state) { > + /* this finger is in proximity of the sensor */ Using (s->touch_state || s->inrange_state) here seems simpler. > int wide = (s->w > s->h); > /* divided by two to match visual scale of touch */ > int major = max(s->w, s->h) >> 1; > @@ -535,11 +546,14 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) > input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y); > input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx); > input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy); > + input_event(input, EV_ABS, ABS_MT_DISTANCE, > + !s->touch_state); > input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide); > input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p); > input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major); > input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor); > - } > + } else > + input_event(input, EV_ABS, ABS_MT_DISTANCE, -1); Just skip this, please. > } > > td->num_received++; > @@ -567,6 +581,8 @@ static int mt_event(struct hid_device *hid, struct hid_field *field, > case HID_DG_INRANGE: > if (quirks & MT_QUIRK_VALID_IS_INRANGE) > td->curvalid = value; > + if (quirks & MT_QUIRK_WIN_8_CERTIFIED) > + td->curdata.inrange_state = value; > break; > case HID_DG_TIPSWITCH: > if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) > -- > 1.7.11.7 > Thanks, 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
Hi Henrik, thanks for the review of the patchset. I'll do my best for the next version :) On Tue, Nov 13, 2012 at 5:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote: > On Wed, Nov 07, 2012 at 05:37:34PM +0100, Benjamin Tissoires wrote: >> Win8 devices supporting hovering must provides InRange HID field. > > provide the > >> The information that the finger is here but is not touching the surface >> is sent to the user space through ABS_MT_DISTANCE as required by the >> multitouch protocol. > > In the absence of more detailed information, use ABS_MT_DISTANCE with > a [0,1] interval to distinguish between presence (1) and touch (0). > >> >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> >> --- >> drivers/hid/hid-multitouch.c | 24 ++++++++++++++++++++---- >> 1 file changed, 20 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c >> index b393c6c..1f3d1e0 100644 >> --- a/drivers/hid/hid-multitouch.c >> +++ b/drivers/hid/hid-multitouch.c >> @@ -59,6 +59,7 @@ struct mt_slot { >> __s32 x, y, cx, cy, p, w, h; >> __s32 contactid; /* the device ContactID assigned to this slot */ >> bool touch_state; /* is the touch valid? */ >> + bool inrange_state; /* is the finger in proximity of the sensor? */ >> }; >> >> struct mt_class { >> @@ -397,6 +398,12 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, >> case HID_UP_DIGITIZER: >> switch (usage->hid) { >> case HID_DG_INRANGE: >> + if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED) { >> + hid_map_usage(hi, usage, bit, max, >> + EV_ABS, ABS_MT_DISTANCE); >> + input_set_abs_params(hi->input, >> + ABS_MT_DISTANCE, -1, 1, 0, 0); > > Why [-1,1] here? At first, I only used [0,1]. However, it's still the same problem with the information being kept between the touches: if you start an application after having touched your device, you normally have to ask for the different per-touche states to get x, y, distance, pressure, etc... However, this is not much mandatory because the protocol in its current form ensures that you will get the new states of the axes when a new touch occurs. ABS_MT_DISTANCE is a little bit different here because the protocol guarantees that once you get the "not in range" state through tracking_id == -1, distance should be 1. When the new touch of the very same slot occurs, you also have the guarantee that distance is at 1 too. So basically, if you don't ask for the slot states, you will never get that very first distance. I know that it's a user-space problem, but honestly, I don't want to fix several user-space problems when we could fix it through the protocol. I can see 2 solutions: - set the range to: [0,1] and still sending -1 (or 2) for the invalid distance value (if it's not clamped) - set the range to: [0,2] and having: 0 for touch, 1 for hovering, and 2 for not in range Does that make sense? > >> + } >> mt_store_field(usage, td, hi); >> td->last_field_index = field->index; >> return 1; >> @@ -516,6 +523,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) >> if (slotnum < 0 || slotnum >= td->maxcontacts) >> return; >> >> + if (!test_bit(ABS_MT_DISTANCE, input->absbit)) >> + /* If InRange is not present, rely on TipSwitch */ >> + s->inrange_state = s->touch_state; >> + > > This could be skipped, see below. > >> if (td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES && >> input_mt_is_active(&input->mt->slots[slotnum]) && >> input_mt_is_used(input->mt, &input->mt->slots[slotnum])) >> @@ -523,9 +534,9 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) >> >> input_mt_slot(input, slotnum); >> input_mt_report_slot_state(input, MT_TOOL_FINGER, >> - s->touch_state); >> - if (s->touch_state) { >> - /* this finger is on the screen */ >> + s->inrange_state); >> + if (s->inrange_state) { >> + /* this finger is in proximity of the sensor */ > > Using (s->touch_state || s->inrange_state) here seems simpler. agree. > >> int wide = (s->w > s->h); >> /* divided by two to match visual scale of touch */ >> int major = max(s->w, s->h) >> 1; >> @@ -535,11 +546,14 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) >> input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y); >> input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx); >> input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy); >> + input_event(input, EV_ABS, ABS_MT_DISTANCE, >> + !s->touch_state); >> input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide); >> input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p); >> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major); >> input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor); >> - } >> + } else >> + input_event(input, EV_ABS, ABS_MT_DISTANCE, -1); > > Just skip this, please. see above. Cheers, Benjamin > >> } >> >> td->num_received++; >> @@ -567,6 +581,8 @@ static int mt_event(struct hid_device *hid, struct hid_field *field, >> case HID_DG_INRANGE: >> if (quirks & MT_QUIRK_VALID_IS_INRANGE) >> td->curvalid = value; >> + if (quirks & MT_QUIRK_WIN_8_CERTIFIED) >> + td->curdata.inrange_state = value; >> break; >> case HID_DG_TIPSWITCH: >> if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) >> -- >> 1.7.11.7 >> > > Thanks, > 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
Hi Benjamin, > > Why [-1,1] here? > > At first, I only used [0,1]. However, it's still the same problem with > the information being kept between the touches: > if you start an application after having touched your device, you > normally have to ask for the different per-touche states to get x, y, > distance, pressure, etc... > However, this is not much mandatory because the protocol in its > current form ensures that you will get the new states of the axes when > a new touch occurs. Right, the stateful communication requires a full state read after opening the deivce, although in most cases this is not really necessary. This is no different for ABS_MT_DISTANCE, of course. > ABS_MT_DISTANCE is a little bit different here because the protocol > guarantees that once you get the "not in range" state through > tracking_id == -1, distance should be 1. > When the new touch of the very same slot occurs, you also have the > guarantee that distance is at 1 too. ABS_MT_DISTANCE is just another attribute of the slot, so it really is no different. > So basically, if you don't ask for the slot states, you will never get > that very first distance. Which will not be important either; as long as the slot is unused, it does not matter what the attributes of that slot are. > I know that it's a user-space problem, but honestly, I don't want to > fix several user-space problems when we could fix it through the > protocol. > > I can see 2 solutions: > - set the range to: [0,1] and still sending -1 (or 2) for the invalid > distance value (if it's not clamped) > - set the range to: [0,2] and having: 0 for touch, 1 for hovering, and > 2 for not in range > > Does that make sense? I just do not see what problem you want to solve here. Thanks, 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
On Tue, Nov 13, 2012 at 7:04 PM, Henrik Rydberg <rydberg@euromail.se> wrote: > Hi Benjamin, > >> > Why [-1,1] here? >> >> At first, I only used [0,1]. However, it's still the same problem with >> the information being kept between the touches: >> if you start an application after having touched your device, you >> normally have to ask for the different per-touche states to get x, y, >> distance, pressure, etc... >> However, this is not much mandatory because the protocol in its >> current form ensures that you will get the new states of the axes when >> a new touch occurs. > > Right, the stateful communication requires a full state read after > opening the deivce, although in most cases this is not really > necessary. This is no different for ABS_MT_DISTANCE, of course. > >> ABS_MT_DISTANCE is a little bit different here because the protocol >> guarantees that once you get the "not in range" state through >> tracking_id == -1, distance should be 1. >> When the new touch of the very same slot occurs, you also have the >> guarantee that distance is at 1 too. > > ABS_MT_DISTANCE is just another attribute of the slot, so it really is > no different. > >> So basically, if you don't ask for the slot states, you will never get >> that very first distance. > > Which will not be important either; as long as the slot is unused, it > does not matter what the attributes of that slot are. And if the slot is unused, but has been used since the plug of the device, the state is kept between the touch. > >> I know that it's a user-space problem, but honestly, I don't want to >> fix several user-space problems when we could fix it through the >> protocol. >> >> I can see 2 solutions: >> - set the range to: [0,1] and still sending -1 (or 2) for the invalid >> distance value (if it's not clamped) >> - set the range to: [0,2] and having: 0 for touch, 1 for hovering, and >> 2 for not in range >> >> Does that make sense? > > I just do not see what problem you want to solve here. I just intend to force the update of the distance value at the beginning of the touch. This is just to send a coherent state when the finger goes in range, and to make sure that user-space application do not rely on the undefined value (0) whereas the kernel thought it was set to 1. Cheers, Benjamin > > Thanks, > 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
> I just intend to force the update of the distance value at the > beginning of the touch. > This is just to send a coherent state when the finger goes in range, > and to make sure that user-space application do not rely on the > undefined value (0) whereas the kernel thought it was set to 1. Right, so we use EVIOCGMTSLOTS. This "problem" occurs with ABS_MT_ORIENTATION as well. Thanks, 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
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index b393c6c..1f3d1e0 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -59,6 +59,7 @@ struct mt_slot { __s32 x, y, cx, cy, p, w, h; __s32 contactid; /* the device ContactID assigned to this slot */ bool touch_state; /* is the touch valid? */ + bool inrange_state; /* is the finger in proximity of the sensor? */ }; struct mt_class { @@ -397,6 +398,12 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, case HID_UP_DIGITIZER: switch (usage->hid) { case HID_DG_INRANGE: + if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED) { + hid_map_usage(hi, usage, bit, max, + EV_ABS, ABS_MT_DISTANCE); + input_set_abs_params(hi->input, + ABS_MT_DISTANCE, -1, 1, 0, 0); + } mt_store_field(usage, td, hi); td->last_field_index = field->index; return 1; @@ -516,6 +523,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) if (slotnum < 0 || slotnum >= td->maxcontacts) return; + if (!test_bit(ABS_MT_DISTANCE, input->absbit)) + /* If InRange is not present, rely on TipSwitch */ + s->inrange_state = s->touch_state; + if (td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES && input_mt_is_active(&input->mt->slots[slotnum]) && input_mt_is_used(input->mt, &input->mt->slots[slotnum])) @@ -523,9 +534,9 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) input_mt_slot(input, slotnum); input_mt_report_slot_state(input, MT_TOOL_FINGER, - s->touch_state); - if (s->touch_state) { - /* this finger is on the screen */ + s->inrange_state); + if (s->inrange_state) { + /* this finger is in proximity of the sensor */ int wide = (s->w > s->h); /* divided by two to match visual scale of touch */ int major = max(s->w, s->h) >> 1; @@ -535,11 +546,14 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input) input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y); input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx); input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy); + input_event(input, EV_ABS, ABS_MT_DISTANCE, + !s->touch_state); input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide); input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p); input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major); input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor); - } + } else + input_event(input, EV_ABS, ABS_MT_DISTANCE, -1); } td->num_received++; @@ -567,6 +581,8 @@ static int mt_event(struct hid_device *hid, struct hid_field *field, case HID_DG_INRANGE: if (quirks & MT_QUIRK_VALID_IS_INRANGE) td->curvalid = value; + if (quirks & MT_QUIRK_WIN_8_CERTIFIED) + td->curdata.inrange_state = value; break; case HID_DG_TIPSWITCH: if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
Win8 devices supporting hovering must provides InRange HID field. The information that the finger is here but is not touching the surface is sent to the user space through ABS_MT_DISTANCE as required by the multitouch protocol. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> --- drivers/hid/hid-multitouch.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)