[PATCH libinput 6/6] touchpad: pretend the jumpy semi-mt touchpad is a single-touch touchpad

[Hans de Goede]

Hi,

On 30-07-15 14:33, Hans de Goede wrote:
> Hi,
>
> On 30-07-15 08:11, Peter Hutterer wrote:
>> The first finger is accurate, it's just the second finger that is imprecise,
>> so we can't handle it as a true touch. Instead, revert the device back to
>> being a single-touch touchpad and use the fake touch bits for second finger
>> handling.
>>
>> Two-finger scrolling thus becomes usable though we will lose out on
>> other features like thumb detection. Useful scrolling trumps that though.
>
> I think we should wait with doing this until the scroll situation is more
> clear, see my last comment here:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1235175

Ok so Benjamin responded in the bug report, these touchpads
report accurate single touch data in the old-style single-touch
events, and inaccurate data in the mt events when 2 fingers are down.

So this patch which switches things over to using st coordinates
is the right thing todo. Review inline.


>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>> ---
>>   src/evdev-mt-touchpad.c | 19 ++++++++++++++-----
>>   test/touchpad-tap.c     |  3 +++
>>   test/touchpad.c         |  9 +++------
>>   3 files changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
>> index af1cd47..54ea819 100644
>> --- a/src/evdev-mt-touchpad.c
>> +++ b/src/evdev-mt-touchpad.c
>> @@ -1465,6 +1465,19 @@ tp_init_slots(struct tp_dispatch *tp,
>>
>>       tp->semi_mt = libevdev_has_property(device->evdev, INPUT_PROP_SEMI_MT);
>>
>> +    /* This device has a terrible 2fg resolution, but only for the
>> +     * second finger, causing scroll jumps when we use the touch points
>> +     * properly. The first finger resolution is accurate though, so
>> +     * we simply pretend it's a single touch touchpad with the BTN_TOOL
>> +     * bits.
>> +     */

This comment is wrong, the old-style st coordinates report accurate
data where as the mt data reports inaccurate data for both fingers
when 2 fingers are down. What is happening is that when a single finger
is down the st data is copied over to the mt data slot 0, so as to
have something to report to user-space which only listens to mt events,
and as soon as a second finger comes down the driver starts reporting
the bounding box limits in the mt data, making *both* slots inaccurate.

So it is not "only for the second finger" nor is "The first finger
resolution is accurate" really accurate to say :) The first finger as
reported in the st events is accurate, the first finger reported in the
mt events is still no good, otherwise we would not need this commit at
all.

The rest of the patch looks good and is:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans


>> +    if (tp->semi_mt &&
>> +        (device->model_flags & EVDEV_MODEL_JUMPING_SEMI_MT)) {
>> +        tp->num_slots = 1;
>> +        tp->slot = 0;
>> +        tp->has_mt = false;
>> +    }
>> +
>>       ARRAY_FOR_EACH(max_touches, m) {
>>           if (libevdev_has_event_code(device->evdev,
>>                           EV_KEY,
>> @@ -1526,11 +1539,7 @@ tp_scroll_get_methods(struct tp_dispatch *tp)
>>   {
>>       uint32_t methods = LIBINPUT_CONFIG_SCROLL_EDGE;
>>
>> -    /* some Synaptics semi-mt touchpads have a terrible 2fg resolution,
>> -     * causing scroll jumps. For all other 2fg touchpads, we enable 2fg
>> -     * scrolling */
>> -    if (tp->ntouches >= 2 &&
>> -        (tp->device->model_flags & EVDEV_MODEL_JUMPING_SEMI_MT) == 0)
>> +    if (tp->ntouches >= 2)
>>           methods |= LIBINPUT_CONFIG_SCROLL_2FG;
>>
>>       return methods;
>> diff --git a/test/touchpad-tap.c b/test/touchpad-tap.c
>> index 00afcdb..62c7a5c 100644
>> --- a/test/touchpad-tap.c
>> +++ b/test/touchpad-tap.c
>> @@ -241,6 +241,9 @@ START_TEST(touchpad_1fg_multitap_n_drag_2fg)
>>       int range = _i,
>>           ntaps;
>>
>> +    if (litest_is_synaptics_semi_mt(dev))
>> +        return;
>> +
>>       litest_enable_tap(dev->libinput_device);
>>
>>       litest_drain_events(li);
>> diff --git a/test/touchpad.c b/test/touchpad.c
>> index a6989e7..77c1d2d 100644
>> --- a/test/touchpad.c
>> +++ b/test/touchpad.c
>> @@ -402,14 +402,12 @@ START_TEST(touchpad_scroll_defaults)
>>
>>       method = libinput_device_config_scroll_get_methods(device);
>>       ck_assert(method & LIBINPUT_CONFIG_SCROLL_EDGE);
>> -    if (libevdev_get_num_slots(evdev) > 1 &&
>> -        !litest_is_synaptics_semi_mt(dev))
>> +    if (libevdev_get_num_slots(evdev) > 1)
>>           ck_assert(method & LIBINPUT_CONFIG_SCROLL_2FG);
>>       else
>>           ck_assert((method & LIBINPUT_CONFIG_SCROLL_2FG) == 0);
>>
>> -    if (libevdev_get_num_slots(evdev) > 1 &&
>> -        !litest_is_synaptics_semi_mt(dev))
>> +    if (libevdev_get_num_slots(evdev) > 1)
>>           expected = LIBINPUT_CONFIG_SCROLL_2FG;
>>       else
>>           expected = LIBINPUT_CONFIG_SCROLL_EDGE;
>> @@ -425,8 +423,7 @@ START_TEST(touchpad_scroll_defaults)
>>       status = libinput_device_config_scroll_set_method(device,
>>                         LIBINPUT_CONFIG_SCROLL_2FG);
>>
>> -    if (libevdev_get_num_slots(evdev) > 1 &&
>> -        !litest_is_synaptics_semi_mt(dev))
>> +    if (libevdev_get_num_slots(evdev) > 1)
>>           ck_assert_int_eq(status, LIBINPUT_CONFIG_STATUS_SUCCESS);
>>       else
>>           ck_assert_int_eq(status, LIBINPUT_CONFIG_STATUS_UNSUPPORTED);
>>