On Mon, Oct 9, 2017 at 11:57 PM, Henrik Rydberg <rydberg@xxxxxxxxxxx> wrote:
> Hi Wei-Ning,
>
>> The current hid-multitouch driver only allow the report of two
>> orientations, vertical and horizontal. We use the Azimuth orientation
>> usage 0x3F under the Digitizer usage page to report orientation if the
>> device supports it.
>>
>> Changelog:
>> v1 -> v2:
>> - Fix commit message.
>> - Remove resolution reporting for ABS_MT_ORIENTATION.
>> v2 -> v3:
>> - Fix commit message.
>> v3 -> v4:
>> - Fix ABS_MT_ORIENTATION ABS param range.
>> - Don't set ABS_MT_ORIENTATION in ABS_DG_HEIGHT when it is already
>> set by ABS_DG_AZIMUTH.
>>
>> Signed-off-by: Wei-Ning Huang <wnhuang@xxxxxxxxxxxx>
>> Reviewed-by: Dmitry Torokhov <dtor@xxxxxxxxxxxx>
>> ---
>> drivers/hid/hid-multitouch.c | 52 ++++++++++++++++++++++++++++++++++++++++----
>> include/linux/hid.h | 1 +
>> 2 files changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 440b999304a5..3317dae64ef7 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
>> #define MT_IO_FLAGS_PENDING_SLOTS 2
>>
>> struct mt_slot {
>> - __s32 x, y, cx, cy, p, w, h;
>> + __s32 x, y, cx, cy, p, w, h, a;
>> __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? */
>> bool confidence_state; /* is the touch made by a finger? */
>> + bool has_azimuth; /* the contact reports azimuth */
>> };
>>
>> struct mt_class {
>> @@ -571,8 +572,15 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> if (!(cls->quirks & MT_QUIRK_NO_AREA)) {
>> set_abs(hi->input, ABS_MT_TOUCH_MINOR, field,
>> cls->sn_height);
>> - input_set_abs_params(hi->input,
>> - ABS_MT_ORIENTATION, 0, 1, 0, 0);
>> +
>> + /*
>> + * Only set ABS_MT_ORIENTATION if it is not
>> + * already set by the HID_DG_AZIMUTH usage.
>> + */
>> + if (!test_bit(ABS_MT_ORIENTATION,
>> + hi->input->absbit))
>> + input_set_abs_params(hi->input,
>> + ABS_MT_ORIENTATION, 0, 1, 0, 0);
>> }
>> mt_store_field(usage, td, hi);
>> return 1;
>> @@ -591,6 +599,21 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> td->cc_index = field->index;
>> td->cc_value_index = usage->usage_index;
>> return 1;
>> + case HID_DG_AZIMUTH:
>> + hid_map_usage(hi, usage, bit, max,
>> + EV_ABS, ABS_MT_ORIENTATION);
>> + /*
>> + * Azimuth has the range of [0, MAX) representing a full
>> + * revolution. Set ABS_MT_ORIENTATION to a quarter of
>> + * MAX according the definition of ABS_MT_ORIENTATION
>> + */
>> + input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
>> + -field->logical_maximum / 4,
>> + field->logical_maximum / 4,
So do we expect userspace to have special handling for the range when
"min" is negative? I.e. when range is [0,1] it is understood that we
are reporting the first quadrant only, with 0 reported when finger is
roughly vertical and 1 is when it is horizontal. Similarly, the range
[0-90] would describe the 1st quadrant as well. This matches the
in-kernel documentation. However here we have [-90, 90] that describes
2 quadrants. Do we want to keep it as is and have userspace adapt (we
probably will need a patch to multi-touch-protocol.txt), or should
this be:
input_set_abs_params(hi->input, ABS_MT_ORIENTATION, 0,
field->logical_maximum / 4, ...);
and userspace should be able to handle reported negative events and
have them being understood as going counter-clockwise into the 4th and
then 3rd quadrant?
Thanks,
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
