[PATCH 1/2] touchpad: when clearing the touchpad state, release fake touches too

[Hans de Goede]

Hi,

On 01-05-15 04:48, Peter Hutterer wrote:
> On Thu, Apr 30, 2015 at 09:49:30AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 30-04-15 07:59, Peter Hutterer wrote:
>>> Causes an error message in the device_disable_release_tap_n_drag test. When
>>> the touchpad is suspended, all touches are ended in tp_clear_state. Since the
>>> hovering support was added, this returns the touches to TOUCH_HOVERING, a
>>> subsequent tp_handle_state() will turn them back into TOUCH_BEGIN based on
>>> BTN_TOUCH and BTN_TOOL_FINGER still being down.
>>>
>>> Clear the fake touch buttons as well after ending the touches, this way the
>>> touch points are reset to TOUCH_NONE as intended.
>>> Once we do that we don't need to manually change the tap finger count when
>>> releasing taps, we can just let the count reset naturally.
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>>   src/evdev-mt-touchpad-tap.c | 1 -
>>>   src/evdev-mt-touchpad.c     | 7 +++++++
>>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
>>> index 0f25e26..bd58bb1 100644
>>> --- a/src/evdev-mt-touchpad-tap.c
>>> +++ b/src/evdev-mt-touchpad-tap.c
>>> @@ -844,7 +844,6 @@ tp_release_all_taps(struct tp_dispatch *tp, uint64_t now)
>>>   	}
>>>
>>>   	tp->tap.state = tp->nfingers_down ? TAP_STATE_DEAD : TAP_STATE_IDLE;
>>> -	tp->tap.tap_finger_count = 0;
>>>   }
>>>
>>>   void
>>
>> We still need this ("tp->tap.tap_finger_count = 0;") for the
>> tp_tap_enabled_update() call, I know you're trying to fix this in the next
>> commit, but then at least this belongs in the next commit....
>
> I don't think that's quite correct. The two places this gets called from is:
> * tp_suspend->tp_clear_state where all touches are first released, so the
>    finger count goes to 0 later when we loop through the touch points

Hmm, right here it is wrong, because finger count would become < 0 in that
case (assuming tapping is enabled).

> * tp_tap_enabled_update() when we're _disabling_ tapping. in that case too
>    it is enough to reset the state to DEAD, at which point no further tap
>    events will be generated. The tap finger count can reduce to 0 normally
>    when they're lifted.

Here however since tapping is disabled, tp_tap_handle_state() will be short
circuited so if we do not set finger-count to 0 on tp_release_all_taps we
will restart with it being non 0.

I really believe we should just kill of finger-count, see the my reply to
the next patch in this set.

Regards,

Hans

>
> fwiw, the test suite does pass with this patch applied, and that includes
> tests of "tapping is enabled, finger down, now suspend" and similar.
> (and it also passes with 2/2 applied)
>
> Cheers,
>     Peter
>
>
>> Also see my comments on the next commit.
>>
>>> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
>>> index d5ce880..e017559 100644
>>> --- a/src/evdev-mt-touchpad.c
>>> +++ b/src/evdev-mt-touchpad.c
>>> @@ -750,6 +750,12 @@ tp_destroy(struct evdev_dispatch *dispatch)
>>>   }
>>>
>>>   static void
>>> +tp_release_fake_touches(struct tp_dispatch *tp)
>>> +{
>>> +	tp->fake_touches = 0;
>>> +}
>>> +
>>> +static void
>>>   tp_clear_state(struct tp_dispatch *tp)
>>>   {
>>>   	uint64_t now = libinput_now(tp->device->base.seat->libinput);
>>> @@ -772,6 +778,7 @@ tp_clear_state(struct tp_dispatch *tp)
>>>   	tp_for_each_touch(tp, t) {
>>>   		tp_end_sequence(tp, t, now);
>>>   	}
>>> +	tp_release_fake_touches(tp);
>>>
>>>   	tp_handle_state(tp, now);
>>>   }
>>>
>>
>> Regards,
>>
>> Hans