[PATCH libinput] touchpad: ignore modifier key combos for dwt

Hans de Goede hdegoede at redhat.com
Thu Aug 4 08:23:15 UTC 2016


Hi,

On 04-08-16 06:24, Peter Hutterer wrote:
> Inspired by the syndaemon -K switch and Anton Lindqvist's patch.
> https://patchwork.freedesktop.org/patch/102417/
>
> We already ignored modifiers for dwt. Now we also ignore modifier + key
> combinations, i.e. hitting Ctrl+s to save does not trigger dwt, the touchpad
> remains immediately usable.
>
> However, if dwt is already active and a modifier combination is pressed, dwt
> remains active, i.e. while typing, a shift + key does not disable dwt.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

LGTM:

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

Regards,

Hans

> ---
>  src/evdev-mt-touchpad.c |  35 ++++++++--
>  src/evdev-mt-touchpad.h |   1 +
>  test/touchpad.c         | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 196 insertions(+), 6 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 357f4c5..2c73428 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -1344,7 +1344,7 @@ tp_keyboard_timeout(uint64_t now, void *data)
>  }
>
>  static inline bool
> -tp_key_ignore_for_dwt(unsigned int keycode)
> +tp_key_is_modifier(unsigned int keycode)
>  {
>  	switch (keycode) {
>  	/* Ignore modifiers to be responsive to ctrl-click, alt-tab, etc. */
> @@ -1362,16 +1362,21 @@ tp_key_ignore_for_dwt(unsigned int keycode)
>  	case KEY_LEFTMETA:
>  		return true;
>  	default:
> -		break;
> +		return false;
>  	}
> +}
>
> +static inline bool
> +tp_key_ignore_for_dwt(unsigned int keycode)
> +{
>  	/* Ignore keys not part of the "typewriter set", i.e. F-keys,
>  	 * multimedia keys, numpad, etc.
>  	 */
> -	if (keycode >= KEY_F1)
> -		return true;
>
> -	return false;
> +	if (tp_key_is_modifier(keycode))
> +		return false;
> +
> +	return keycode >= KEY_F1;
>  }
>
>  static void
> @@ -1381,6 +1386,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event *event, void *data)
>  	struct libinput_event_keyboard *kbdev;
>  	unsigned int timeout;
>  	unsigned int key;
> +	bool is_modifier;
>
>  	if (event->type != LIBINPUT_EVENT_KEYBOARD_KEY)
>  		return;
> @@ -1392,18 +1398,34 @@ tp_keyboard_event(uint64_t time, struct libinput_event *event, void *data)
>  	if (libinput_event_keyboard_get_key_state(kbdev) !=
>  	    LIBINPUT_KEY_STATE_PRESSED) {
>  		long_clear_bit(tp->dwt.key_mask, key);
> +		long_clear_bit(tp->dwt.mod_mask, key);
>  		return;
>  	}
>
>  	if (!tp->dwt.dwt_enabled)
>  		return;
>
> +	if (tp_key_ignore_for_dwt(key))
> +		return;
> +
>  	/* modifier keys don't trigger disable-while-typing so things like
>  	 * ctrl+zoom or ctrl+click are possible */
> -	if (tp_key_ignore_for_dwt(key))
> +	is_modifier = tp_key_is_modifier(key);
> +	if (is_modifier) {
> +		long_set_bit(tp->dwt.mod_mask, key);
>  		return;
> +	}
>
>  	if (!tp->dwt.keyboard_active) {
> +		/* This is the first non-modifier key press. Check if the
> +		 * modifier mask is set. If any modifier is down we don't
> +		 * trigger dwt because it's likely to be combination like
> +		 * Ctrl+S or similar */
> +
> +		if (long_any_bit_set(tp->dwt.mod_mask,
> +				     ARRAY_LENGTH(tp->dwt.mod_mask)))
> +		    return;
> +
>  		tp_edge_scroll_stop_events(tp, time);
>  		tp_gesture_cancel(tp, time);
>  		tp_tap_suspend(tp, time);
> @@ -1482,6 +1504,7 @@ tp_dwt_pair_keyboard(struct evdev_device *touchpad,
>  			return;
>
>  		memset(tp->dwt.key_mask, 0, sizeof(tp->dwt.key_mask));
> +		memset(tp->dwt.mod_mask, 0, sizeof(tp->dwt.mod_mask));
>  		libinput_device_remove_event_listener(&tp->dwt.keyboard_listener);
>  	}
>
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 86dd048..9c5a828 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -351,6 +351,7 @@ struct tp_dispatch {
>  		struct libinput_timer keyboard_timer;
>  		struct evdev_device *keyboard;
>  		unsigned long key_mask[NLONGS(KEY_CNT)];
> +		unsigned long mod_mask[NLONGS(KEY_CNT)];
>
>  		uint64_t keyboard_last_press_time;
>  	} dwt;
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 9f78ed7..b7c3c98 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -3000,6 +3000,169 @@ START_TEST(touchpad_dwt_modifier_no_dwt)
>  }
>  END_TEST
>
> +START_TEST(touchpad_dwt_modifier_combo_no_dwt)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +	unsigned int modifiers[] = {
> +		KEY_LEFTCTRL,
> +		KEY_RIGHTCTRL,
> +		KEY_LEFTALT,
> +		KEY_RIGHTALT,
> +		KEY_LEFTSHIFT,
> +		KEY_RIGHTSHIFT,
> +		KEY_FN,
> +		KEY_CAPSLOCK,
> +		KEY_TAB,
> +		KEY_COMPOSE,
> +		KEY_RIGHTMETA,
> +		KEY_LEFTMETA,
> +	};
> +	unsigned int *key;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	ARRAY_FOR_EACH(modifiers, key) {
> +		litest_keyboard_key(keyboard, *key, true);
> +		litest_keyboard_key(keyboard, KEY_A, true);
> +		litest_keyboard_key(keyboard, KEY_A, false);
> +		litest_keyboard_key(keyboard, KEY_B, true);
> +		litest_keyboard_key(keyboard, KEY_B, false);
> +		litest_keyboard_key(keyboard, *key, false);
> +		libinput_dispatch(li);
> +
> +		litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +
> +		litest_touch_down(touchpad, 0, 50, 50);
> +		litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +		litest_touch_up(touchpad, 0);
> +		litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> +	}
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_dwt_modifier_combo_dwt_after)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +	unsigned int modifiers[] = {
> +		KEY_LEFTCTRL,
> +		KEY_RIGHTCTRL,
> +		KEY_LEFTALT,
> +		KEY_RIGHTALT,
> +		KEY_LEFTSHIFT,
> +		KEY_RIGHTSHIFT,
> +		KEY_FN,
> +		KEY_CAPSLOCK,
> +		KEY_TAB,
> +		KEY_COMPOSE,
> +		KEY_RIGHTMETA,
> +		KEY_LEFTMETA,
> +	};
> +	unsigned int *key;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	ARRAY_FOR_EACH(modifiers, key) {
> +		litest_keyboard_key(keyboard, *key, true);
> +		litest_keyboard_key(keyboard, KEY_A, true);
> +		litest_keyboard_key(keyboard, KEY_A, false);
> +		litest_keyboard_key(keyboard, *key, false);
> +		libinput_dispatch(li);
> +
> +		litest_keyboard_key(keyboard, KEY_A, true);
> +		litest_keyboard_key(keyboard, KEY_A, false);
> +		libinput_dispatch(li);
> +		litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +
> +		litest_touch_down(touchpad, 0, 50, 50);
> +		litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +		litest_touch_up(touchpad, 0);
> +		litest_assert_empty_queue(li);
> +
> +		litest_timeout_dwt_long();
> +		libinput_dispatch(li);
> +	}
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_dwt_modifier_combo_dwt_remains)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +	unsigned int modifiers[] = {
> +		KEY_LEFTCTRL,
> +		KEY_RIGHTCTRL,
> +		KEY_LEFTALT,
> +		KEY_RIGHTALT,
> +		KEY_LEFTSHIFT,
> +		KEY_RIGHTSHIFT,
> +		KEY_FN,
> +		KEY_CAPSLOCK,
> +		KEY_TAB,
> +		KEY_COMPOSE,
> +		KEY_RIGHTMETA,
> +		KEY_LEFTMETA,
> +	};
> +	unsigned int *key;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	ARRAY_FOR_EACH(modifiers, key) {
> +		litest_keyboard_key(keyboard, KEY_A, true);
> +		litest_keyboard_key(keyboard, KEY_A, false);
> +		libinput_dispatch(li);
> +
> +		/* this can't really be tested directly. The above key
> +		 * should enable dwt, the next key continues and extends the
> +		 * timeout as usual (despite the modifier combo). but
> +		 * testing for timeout differences is fickle, so all we can
> +		 * test though is that dwt is still on after the modifier
> +		 * combo and does not get disabled immediately.
> +		 */
> +		litest_keyboard_key(keyboard, *key, true);
> +		litest_keyboard_key(keyboard, KEY_A, true);
> +		litest_keyboard_key(keyboard, KEY_A, false);
> +		litest_keyboard_key(keyboard, *key, false);
> +		libinput_dispatch(li);
> +
> +		litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +
> +		litest_touch_down(touchpad, 0, 50, 50);
> +		litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +		litest_touch_up(touchpad, 0);
> +		litest_assert_empty_queue(li);
> +
> +		litest_timeout_dwt_long();
> +		libinput_dispatch(li);
> +	}
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
>  START_TEST(touchpad_dwt_fkeys_no_dwt)
>  {
>  	struct litest_device *touchpad = litest_current_device();
> @@ -4308,6 +4471,9 @@ litest_setup_tests(void)
>  	litest_add("touchpad:dwt", touchpad_dwt_type, LITEST_TOUCHPAD, LITEST_ANY);
>  	litest_add("touchpad:dwt", touchpad_dwt_type_short_timeout, LITEST_TOUCHPAD, LITEST_ANY);
>  	litest_add("touchpad:dwt", touchpad_dwt_modifier_no_dwt, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_modifier_combo_no_dwt, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_modifier_combo_dwt_after, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_modifier_combo_dwt_remains, LITEST_TOUCHPAD, LITEST_ANY);
>  	litest_add("touchpad:dwt", touchpad_dwt_fkeys_no_dwt, LITEST_TOUCHPAD, LITEST_ANY);
>  	litest_add("touchpad:dwt", touchpad_dwt_tap, LITEST_TOUCHPAD, LITEST_ANY);
>  	litest_add("touchpad:dwt", touchpad_dwt_tap_drag, LITEST_TOUCHPAD, LITEST_ANY);
>


More information about the wayland-devel mailing list