Patch: some refactoring in evdev-mt-touchpad-tap.c
Peter Hutterer
peter.hutterer at who-t.net
Sun Apr 26 18:35:48 PDT 2015
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.
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.
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
More information about the wayland-devel
mailing list