Re: [PATCH libinput] touchpad: while a key is held down, don't disable dwt

2016-02-04 Thread Hans de Goede

Hi,

On 04-02-16 01:33, Peter Hutterer wrote:

If a key enables dwt and is held down when the timeout expires, re-issue the
timeout.

There is a corner case where dwt may not work as expected:
1. key down and held down
2. dwt timer expires, dwt is re-issued
3. touch starts
4. key is released
5. dwt timer expires
6. touch now starts moving the pointer

This is an effect of the smart touch detection. A touch starting after the
last key press is released for pointer motion once dwt turns off again. This
is what happens in the above case, the dwt timer expiring is the last virtual
key press. This is a corner case and likely hard to trigger by a real user.

https://bugs.freedesktop.org/show_bug.cgi?id=93984

Signed-off-by: Peter Hutterer 


Patch looks good to me: Reviewed-by: Hans de Goede 

Regards.

Hans



---
  src/evdev-mt-touchpad.c |  18 +-
  src/evdev-mt-touchpad.h |   1 +
  src/libinput-util.h |  14 +
  test/touchpad.c | 142 
  4 files changed, 173 insertions(+), 2 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index f249116..d0d1ddd 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1196,6 +1196,15 @@ tp_keyboard_timeout(uint64_t now, void *data)
  {
struct tp_dispatch *tp = data;

+   if (long_any_bit_set(tp->dwt.key_mask,
+ARRAY_LENGTH(tp->dwt.key_mask))) {
+   libinput_timer_set(&tp->dwt.keyboard_timer,
+  now + DEFAULT_KEYBOARD_ACTIVITY_TIMEOUT_2);
+   tp->dwt.keyboard_last_press_time = now;
+   log_debug(tp_libinput_context(tp), "palm: keyboard timeout 
refresh\n");
+   return;
+   }
+
tp_tap_resume(tp, now);

tp->dwt.keyboard_active = false;
@@ -1240,6 +1249,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
struct tp_dispatch *tp = data;
struct libinput_event_keyboard *kbdev;
unsigned int timeout;
+   unsigned int key;

if (!tp->dwt.dwt_enabled)
return;
@@ -1248,15 +1258,18 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
return;

kbdev = libinput_event_get_keyboard_event(event);
+   key = libinput_event_keyboard_get_key(kbdev);

/* Only trigger the timer on key down. */
if (libinput_event_keyboard_get_key_state(kbdev) !=
-   LIBINPUT_KEY_STATE_PRESSED)
+   LIBINPUT_KEY_STATE_PRESSED) {
+   long_clear_bit(tp->dwt.key_mask, 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(libinput_event_keyboard_get_key(kbdev)))
+   if (tp_key_ignore_for_dwt(key))
return;

if (!tp->dwt.keyboard_active) {
@@ -1270,6 +1283,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
}

tp->dwt.keyboard_last_press_time = time;
+   long_set_bit(tp->dwt.key_mask, key);
libinput_timer_set(&tp->dwt.keyboard_timer,
   time + timeout);
  }
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 0053122..87d34b2 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -343,6 +343,7 @@ struct tp_dispatch {
struct libinput_event_listener keyboard_listener;
struct libinput_timer keyboard_timer;
struct evdev_device *keyboard;
+   unsigned long key_mask[NLONGS(KEY_CNT)];

uint64_t keyboard_last_press_time;
} dwt;
diff --git a/src/libinput-util.h b/src/libinput-util.h
index 6adbbc9..522c19c 100644
--- a/src/libinput-util.h
+++ b/src/libinput-util.h
@@ -25,6 +25,7 @@
  #ifndef LIBINPUT_UTIL_H
  #define LIBINPUT_UTIL_H

+#include 
  #include 
  #include 
  #include 
@@ -171,6 +172,19 @@ long_set_bit_state(unsigned long *array, int bit, int 
state)
long_clear_bit(array, bit);
  }

+static inline int
+long_any_bit_set(unsigned long *array, size_t size)
+{
+   unsigned long i;
+
+   assert(size > 0);
+
+   for (i = 0; i < size; i++)
+   if (array[i] != 0)
+   return 1;
+   return 0;
+}
+
  struct matrix {
float val[3][3]; /* [row][col] */
  };
diff --git a/test/touchpad.c b/test/touchpad.c
index 9376cd5..0d3aa03 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -2438,6 +2438,145 @@ START_TEST(touchpad_dwt_key_hold)
  }
  END_TEST

+START_TEST(touchpad_dwt_key_hold_timeout)
+{
+   struct litest_device *touchpad = litest_current_device();
+   struct litest_device *keyboard;
+   struct libinput *li = touchpad->libinput;
+
+   if (!has_disable_while_typing(touchpad))
+   return;
+
+   keyboard = dwt_init_paired_keyboard(li, touchpad);
+

[PATCH libinput] touchpad: while a key is held down, don't disable dwt

2016-02-03 Thread Peter Hutterer
If a key enables dwt and is held down when the timeout expires, re-issue the
timeout.

There is a corner case where dwt may not work as expected:
1. key down and held down
2. dwt timer expires, dwt is re-issued
3. touch starts
4. key is released
5. dwt timer expires
6. touch now starts moving the pointer

This is an effect of the smart touch detection. A touch starting after the
last key press is released for pointer motion once dwt turns off again. This
is what happens in the above case, the dwt timer expiring is the last virtual
key press. This is a corner case and likely hard to trigger by a real user.

https://bugs.freedesktop.org/show_bug.cgi?id=93984

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad.c |  18 +-
 src/evdev-mt-touchpad.h |   1 +
 src/libinput-util.h |  14 +
 test/touchpad.c | 142 
 4 files changed, 173 insertions(+), 2 deletions(-)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index f249116..d0d1ddd 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1196,6 +1196,15 @@ tp_keyboard_timeout(uint64_t now, void *data)
 {
struct tp_dispatch *tp = data;
 
+   if (long_any_bit_set(tp->dwt.key_mask,
+ARRAY_LENGTH(tp->dwt.key_mask))) {
+   libinput_timer_set(&tp->dwt.keyboard_timer,
+  now + DEFAULT_KEYBOARD_ACTIVITY_TIMEOUT_2);
+   tp->dwt.keyboard_last_press_time = now;
+   log_debug(tp_libinput_context(tp), "palm: keyboard timeout 
refresh\n");
+   return;
+   }
+
tp_tap_resume(tp, now);
 
tp->dwt.keyboard_active = false;
@@ -1240,6 +1249,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
struct tp_dispatch *tp = data;
struct libinput_event_keyboard *kbdev;
unsigned int timeout;
+   unsigned int key;
 
if (!tp->dwt.dwt_enabled)
return;
@@ -1248,15 +1258,18 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
return;
 
kbdev = libinput_event_get_keyboard_event(event);
+   key = libinput_event_keyboard_get_key(kbdev);
 
/* Only trigger the timer on key down. */
if (libinput_event_keyboard_get_key_state(kbdev) !=
-   LIBINPUT_KEY_STATE_PRESSED)
+   LIBINPUT_KEY_STATE_PRESSED) {
+   long_clear_bit(tp->dwt.key_mask, 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(libinput_event_keyboard_get_key(kbdev)))
+   if (tp_key_ignore_for_dwt(key))
return;
 
if (!tp->dwt.keyboard_active) {
@@ -1270,6 +1283,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event 
*event, void *data)
}
 
tp->dwt.keyboard_last_press_time = time;
+   long_set_bit(tp->dwt.key_mask, key);
libinput_timer_set(&tp->dwt.keyboard_timer,
   time + timeout);
 }
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 0053122..87d34b2 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -343,6 +343,7 @@ struct tp_dispatch {
struct libinput_event_listener keyboard_listener;
struct libinput_timer keyboard_timer;
struct evdev_device *keyboard;
+   unsigned long key_mask[NLONGS(KEY_CNT)];
 
uint64_t keyboard_last_press_time;
} dwt;
diff --git a/src/libinput-util.h b/src/libinput-util.h
index 6adbbc9..522c19c 100644
--- a/src/libinput-util.h
+++ b/src/libinput-util.h
@@ -25,6 +25,7 @@
 #ifndef LIBINPUT_UTIL_H
 #define LIBINPUT_UTIL_H
 
+#include 
 #include 
 #include 
 #include 
@@ -171,6 +172,19 @@ long_set_bit_state(unsigned long *array, int bit, int 
state)
long_clear_bit(array, bit);
 }
 
+static inline int
+long_any_bit_set(unsigned long *array, size_t size)
+{
+   unsigned long i;
+
+   assert(size > 0);
+
+   for (i = 0; i < size; i++)
+   if (array[i] != 0)
+   return 1;
+   return 0;
+}
+
 struct matrix {
float val[3][3]; /* [row][col] */
 };
diff --git a/test/touchpad.c b/test/touchpad.c
index 9376cd5..0d3aa03 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -2438,6 +2438,145 @@ START_TEST(touchpad_dwt_key_hold)
 }
 END_TEST
 
+START_TEST(touchpad_dwt_key_hold_timeout)
+{
+   struct litest_device *touchpad = litest_current_device();
+   struct litest_device *keyboard;
+   struct libinput *li = touchpad->libinput;
+
+   if (!has_disable_while_typing(touchpad))
+   return;
+
+   keyboard = dwt_init_paired_keyboard(li, touchpad);
+   litest_disable_tap(touchpad->libinput_device);
+   litest_drain_events(li);
+
+   litest_keyboard_key(keyboard, KEY