Patch: some refactoring in evdev-mt-touchpad-tap.c
Lucas Baudin
lucas.baudin at ens.fr
Mon Apr 27 03:36:20 PDT 2015
Hi,
Well, I was not aware of Hans de Goede's patchset, so I am not sure this
is still relevant, see in text replies.
On 27/04/2015 03:35, Peter Hutterer wrote:
> Hi Lucas,
>
> On Fri, Apr 24, 2015 at 11:50:58PM +0200, Lucas Baudin wrote:
>> Here is a patch that factorizes some code from edev-mt-touchpad-tap.c, that
>> uses more the structures that are already updated by libinput.
>>
>> This patch is a first step towards multi-touch touchpad event, such as three
>> finger scrolling or 4-5 fingers click.
> can you expand on why you'd want all this?
> we already have two-finger scrolling and that's unlikely to ever change,
> it's too ingrained in virtually everything today. So adding 3 finger
> scrolling doesn't seem to add much.
3 finger scrolling would be used as a gesture (to trigger a WM expose
event or whatever), but I suppose it should rather be a gesture in
Hans's patchset than in a different file.
>
> We don't have 4/5 finger tap because not only are they hard to trigger on
> many touchpads, it's also not clear what should happen when tapped. which
> button event should be generated, what will the caller do with this event,
> etc.
>
> In regards to the patch there are two comments: one is that the state
> machine diagram should be updated whenever you change the state machine.
> the second is: the state diagram is quite complex but in exchange the code
> itself contains virtually no logic other than posting the events and
> setting/clearing timers. That's intentional, it makes debugging a lot
> easier since there is no side effects. I'd prefer to keep it that way for 4
> and 5-finger taps.
In fact, there is no new side effects, since the fingers_count is
already used in some other part of the code, so it has to be accurate
anyway.
Regarding the state diagram, I am not sure, how is it built ? by hand in
Inkscape or something like that ?
Thanks for the comments,
-- Lucas
>
> Cheers,
> Peter
>
>> From 91b5c43fc43a6f12b86c00f55f57267a4e27c866 Mon Sep 17 00:00:00 2001
>> From: Lucas Baudin <lucas.baudin at ens.fr>
>> Date: Fri, 24 Apr 2015 23:37:19 +0200
>> Subject: [PATCH] Factorize the tap_touch*_handle_event
>>
>> ---
>> src/evdev-mt-touchpad-tap.c | 147 +++++---------------------------------------
>> src/evdev-mt-touchpad.h | 4 --
>> 2 files changed, 16 insertions(+), 135 deletions(-)
>>
>> diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
>> index 0f25e26..5ae8aab 100644
>> --- a/src/evdev-mt-touchpad-tap.c
>> +++ b/src/evdev-mt-touchpad-tap.c
>> @@ -66,10 +66,6 @@ tap_state_to_str(enum tp_tap_state state) {
>> CASE_RETURN_STRING(TAP_STATE_HOLD);
>> CASE_RETURN_STRING(TAP_STATE_TOUCH);
>> CASE_RETURN_STRING(TAP_STATE_TAPPED);
>> - CASE_RETURN_STRING(TAP_STATE_TOUCH_2);
>> - CASE_RETURN_STRING(TAP_STATE_TOUCH_2_HOLD);
>> - CASE_RETURN_STRING(TAP_STATE_TOUCH_3);
>> - CASE_RETURN_STRING(TAP_STATE_TOUCH_3_HOLD);
>> CASE_RETURN_STRING(TAP_STATE_DRAGGING);
>> CASE_RETURN_STRING(TAP_STATE_DRAGGING_WAIT);
>> CASE_RETURN_STRING(TAP_STATE_DRAGGING_OR_DOUBLETAP);
>> @@ -167,13 +163,22 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp,
>>
>> switch (event) {
>> case TAP_EVENT_TOUCH:
>> - tp->tap.state = TAP_STATE_TOUCH_2;
>> tp_tap_set_timer(tp, time);
>> break;
>> case TAP_EVENT_RELEASE:
>> - tp->tap.state = TAP_STATE_TAPPED;
>> - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_PRESSED);
>> - tp_tap_set_timer(tp, time);
>> + if (tp->tap.tap_finger_count == 0) {
>> + tp->tap.state = TAP_STATE_TAPPED;
>> + tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_PRESSED);
>> + tp_tap_set_timer(tp, time);
>> + }
>> + else if (tp->tap.tap_finger_count > 0) {
>> + tp->tap.state = TAP_STATE_HOLD;
>> + if (t->tap.state == TAP_TOUCH_STATE_TOUCH) {
>> + tp_tap_notify(tp, time, tp->tap.tap_finger_count + 1, LIBINPUT_BUTTON_STATE_PRESSED);
>> + tp_tap_notify(tp, time, tp->tap.tap_finger_count + 1, LIBINPUT_BUTTON_STATE_RELEASED);
>> + }
>> + tp_tap_clear_timer(tp);
>> + }
>> break;
>> case TAP_EVENT_TIMEOUT:
>> case TAP_EVENT_MOTION:
>> @@ -194,11 +199,12 @@ tp_tap_hold_handle_event(struct tp_dispatch *tp,
>>
>> switch (event) {
>> case TAP_EVENT_TOUCH:
>> - tp->tap.state = TAP_STATE_TOUCH_2;
>> tp_tap_set_timer(tp, time);
>> break;
>> case TAP_EVENT_RELEASE:
>> - tp->tap.state = TAP_STATE_IDLE;
>> + if (tp->tap.tap_finger_count <= 0) {
>> + tp->tap.state = TAP_STATE_IDLE;
>> + }
>> break;
>> case TAP_EVENT_MOTION:
>> case TAP_EVENT_TIMEOUT:
>> @@ -237,113 +243,6 @@ tp_tap_tapped_handle_event(struct tp_dispatch *tp,
>> }
>>
>> static void
>> -tp_tap_touch2_handle_event(struct tp_dispatch *tp,
>> - struct tp_touch *t,
>> - enum tap_event event, uint64_t time)
>> -{
>> -
>> - switch (event) {
>> - case TAP_EVENT_TOUCH:
>> - tp->tap.state = TAP_STATE_TOUCH_3;
>> - tp_tap_set_timer(tp, time);
>> - break;
>> - case TAP_EVENT_RELEASE:
>> - tp->tap.state = TAP_STATE_HOLD;
>> - if (t->tap.state == TAP_TOUCH_STATE_TOUCH) {
>> - tp_tap_notify(tp, time, 2, LIBINPUT_BUTTON_STATE_PRESSED);
>> - tp_tap_notify(tp, time, 2, LIBINPUT_BUTTON_STATE_RELEASED);
>> - }
>> - tp_tap_clear_timer(tp);
>> - break;
>> - case TAP_EVENT_MOTION:
>> - tp_tap_clear_timer(tp);
>> - /* fallthrough */
>> - case TAP_EVENT_TIMEOUT:
>> - tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
>> - break;
>> - case TAP_EVENT_BUTTON:
>> - tp->tap.state = TAP_STATE_DEAD;
>> - break;
>> - }
>> -}
>> -
>> -static void
>> -tp_tap_touch2_hold_handle_event(struct tp_dispatch *tp,
>> - struct tp_touch *t,
>> - enum tap_event event, uint64_t time)
>> -{
>> -
>> - switch (event) {
>> - case TAP_EVENT_TOUCH:
>> - tp->tap.state = TAP_STATE_TOUCH_3;
>> - tp_tap_set_timer(tp, time);
>> - break;
>> - case TAP_EVENT_RELEASE:
>> - tp->tap.state = TAP_STATE_HOLD;
>> - break;
>> - case TAP_EVENT_MOTION:
>> - case TAP_EVENT_TIMEOUT:
>> - tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
>> - break;
>> - case TAP_EVENT_BUTTON:
>> - tp->tap.state = TAP_STATE_DEAD;
>> - break;
>> - }
>> -}
>> -
>> -static void
>> -tp_tap_touch3_handle_event(struct tp_dispatch *tp,
>> - struct tp_touch *t,
>> - enum tap_event event, uint64_t time)
>> -{
>> -
>> - switch (event) {
>> - case TAP_EVENT_TOUCH:
>> - tp->tap.state = TAP_STATE_DEAD;
>> - tp_tap_clear_timer(tp);
>> - break;
>> - case TAP_EVENT_MOTION:
>> - case TAP_EVENT_TIMEOUT:
>> - tp->tap.state = TAP_STATE_TOUCH_3_HOLD;
>> - tp_tap_clear_timer(tp);
>> - break;
>> - case TAP_EVENT_RELEASE:
>> - tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
>> - if (t->tap.state == TAP_TOUCH_STATE_TOUCH) {
>> - tp_tap_notify(tp, time, 3, LIBINPUT_BUTTON_STATE_PRESSED);
>> - tp_tap_notify(tp, time, 3, LIBINPUT_BUTTON_STATE_RELEASED);
>> - }
>> - break;
>> - case TAP_EVENT_BUTTON:
>> - tp->tap.state = TAP_STATE_DEAD;
>> - break;
>> - }
>> -}
>> -
>> -static void
>> -tp_tap_touch3_hold_handle_event(struct tp_dispatch *tp,
>> - struct tp_touch *t,
>> - enum tap_event event, uint64_t time)
>> -{
>> -
>> - switch (event) {
>> - case TAP_EVENT_TOUCH:
>> - tp->tap.state = TAP_STATE_DEAD;
>> - tp_tap_set_timer(tp, time);
>> - break;
>> - case TAP_EVENT_RELEASE:
>> - tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
>> - break;
>> - case TAP_EVENT_MOTION:
>> - case TAP_EVENT_TIMEOUT:
>> - break;
>> - case TAP_EVENT_BUTTON:
>> - tp->tap.state = TAP_STATE_DEAD;
>> - break;
>> - }
>> -}
>> -
>> -static void
>> tp_tap_dragging_or_doubletap_handle_event(struct tp_dispatch *tp,
>> struct tp_touch *t,
>> enum tap_event event, uint64_t time)
>> @@ -560,18 +459,6 @@ tp_tap_handle_event(struct tp_dispatch *tp,
>> case TAP_STATE_TAPPED:
>> tp_tap_tapped_handle_event(tp, t, event, time);
>> break;
>> - case TAP_STATE_TOUCH_2:
>> - tp_tap_touch2_handle_event(tp, t, event, time);
>> - break;
>> - case TAP_STATE_TOUCH_2_HOLD:
>> - tp_tap_touch2_hold_handle_event(tp, t, event, time);
>> - break;
>> - case TAP_STATE_TOUCH_3:
>> - tp_tap_touch3_handle_event(tp, t, event, time);
>> - break;
>> - case TAP_STATE_TOUCH_3_HOLD:
>> - tp_tap_touch3_hold_handle_event(tp, t, event, time);
>> - break;
>> case TAP_STATE_DRAGGING_OR_DOUBLETAP:
>> tp_tap_dragging_or_doubletap_handle_event(tp, t, event, time);
>> break;
>> @@ -687,8 +574,6 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
>> case TAP_STATE_TOUCH:
>> case TAP_STATE_TAPPED:
>> case TAP_STATE_DRAGGING_OR_DOUBLETAP:
>> - case TAP_STATE_TOUCH_2:
>> - case TAP_STATE_TOUCH_3:
>> case TAP_STATE_MULTITAP_DOWN:
>> filter_motion = 1;
>> break;
>> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
>> index 97b17cd..1661c56 100644
>> --- a/src/evdev-mt-touchpad.h
>> +++ b/src/evdev-mt-touchpad.h
>> @@ -89,10 +89,6 @@ enum tp_tap_state {
>> TAP_STATE_TOUCH,
>> TAP_STATE_HOLD,
>> TAP_STATE_TAPPED,
>> - TAP_STATE_TOUCH_2,
>> - TAP_STATE_TOUCH_2_HOLD,
>> - TAP_STATE_TOUCH_3,
>> - TAP_STATE_TOUCH_3_HOLD,
>> TAP_STATE_DRAGGING_OR_DOUBLETAP,
>> TAP_STATE_DRAGGING,
>> TAP_STATE_DRAGGING_WAIT,
>> --
>> 2.3.5
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list