Re: [PATCH libinput 1/4] evdev: Ignore key/button release events if key was never pressed
Hi Jonas,
On Sun, Jul 27, 2014 at 11:28:28PM +0200, Jonas Ådahl wrote:
> The kernel may send a 'release' event without ever having sent a key
> 'pressed' event in case the key was pressed before libinput was
> initiated. Ignore these events so that we always guarantee a release
> event always comes after a pressed event for any given key or button.
Would it be possible to describe in the docs the invariants that
libinput is keeping w.r.t. key press/release ordering and count?
If the user can rely on such invariants, it can simplify how he
interfaces with e.g. libxkbcommon, which expects a coherent key event
stream (things like modifier press with a missed release can cause some
fun).
Ran
> Signed-off-by: Jonas Ådahl
> ---
> src/evdev.c | 33 -
> src/evdev.h | 2 ++
> src/libinput-util.h | 29 +
> test/keyboard.c | 52
> 4 files changed, 115 insertions(+), 1 deletion(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index 5872856..0f4874c 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -47,6 +47,18 @@ enum evdev_key_type {
> EVDEV_KEY_TYPE_BUTTON,
> };
>
> +static void
> +set_key_pressed(struct evdev_device *device, int code, int pressed)
> +{
> + long_set_bit_state(device->key_mask, code, pressed);
> +}
> +
> +static int
> +is_key_pressed(struct evdev_device *device, int code)
> +{
> + return long_bit_is_set(device->key_mask, code);
> +}
> +
> void
> evdev_device_led_update(struct evdev_device *device, enum libinput_led leds)
> {
> @@ -294,6 +306,8 @@ static inline void
> evdev_process_key(struct evdev_device *device,
> struct input_event *e, uint64_t time)
> {
> + enum evdev_key_type type;
> +
> /* ignore kernel key repeat */
> if (e->value == 2)
> return;
> @@ -306,7 +320,24 @@ evdev_process_key(struct evdev_device *device,
>
> evdev_flush_pending_event(device, time);
>
> - switch (get_key_type(e->code)) {
> + type = get_key_type(e->code);
> +
> + /* Ignore key release events from the kernel for keys that libinput
> + * never got a pressed event for. */
> + if (e->value == 0) {
> + switch (type) {
> + case EVDEV_KEY_TYPE_NONE:
> + break;
> + case EVDEV_KEY_TYPE_KEY:
> + case EVDEV_KEY_TYPE_BUTTON:
> + if (!is_key_pressed(device, e->code))
> + return;
> + }
> + }
> +
> + set_key_pressed(device, e->code, e->value);
> +
> + switch (type) {
> case EVDEV_KEY_TYPE_NONE:
> break;
> case EVDEV_KEY_TYPE_KEY:
> diff --git a/src/evdev.h b/src/evdev.h
> index fad1f84..f71d387 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -94,6 +94,8 @@ struct evdev_device {
> struct {
> struct motion_filter *filter;
> } pointer;
> +
> + unsigned long key_mask[NLONGS(KEY_CNT)];
> };
>
> #define EVDEV_UNHANDLED_DEVICE ((struct evdev_device *) 1)
> diff --git a/src/libinput-util.h b/src/libinput-util.h
> index c0235ef..2f1a1db 100644
> --- a/src/libinput-util.h
> +++ b/src/libinput-util.h
> @@ -72,6 +72,8 @@ int list_empty(const struct list *list);
>pos = tmp, \
>tmp = container_of(pos->member.next, tmp, member))
>
> +#define LONG_BITS (sizeof(long) * 8)
> +#define NLONGS(x) (((x) + LONG_BITS - 1) / LONG_BITS)
> #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
> #define ARRAY_FOR_EACH(_arr, _elem) \
> for (size_t _i = 0; (_elem = &_arr[_i]) && _i < ARRAY_LENGTH(_arr);
> _i++)
> @@ -150,4 +152,31 @@ vector_get_direction(int dx, int dy)
> return dir;
> }
>
> +static inline int
> +long_bit_is_set(const unsigned long *array, int bit)
> +{
> +return !!(array[bit / LONG_BITS] & (1LL << (bit % LONG_BITS)));
> +}
> +
> +static inline void
> +long_set_bit(unsigned long *array, int bit)
> +{
> +array[bit / LONG_BITS] |= (1LL << (bit % LONG_BITS));
> +}
> +
> +static inline void
> +long_clear_bit(unsigned long *array, int bit)
> +{
> +array[bit / LONG_BITS] &= ~(1LL << (bit % LONG_BITS));
> +}
> +
> +static inline void
> +long_set_bit_state(unsigned long *array, int bit, int state)
> +{
> + if (state)
> + long_set_bit(array, bit);
> + else
> + long_clear_bit(array, bit);
> +}
> +
> #endif /* LIBINPUT_UTIL_H */
> diff --git a/test/keyboard.c b/test/keyboard.c
> index a55405c..a7f500c 100644
> --- a/test/keyboard.c
> +++ b/test/keyboard.c
> @@ -112,10 +112,62 @@ START_TEST(keyboard_seat_key_count)
> }
> END_TEST
>
> +START_TEST(keyboard_ignore_no_pressed_release)
> +{
> + struct litest_device *dev;
> + struct libinput *libinput;
> + struct libinput_event *event;
> + struct libinput_event_keyboard *kevent;
> + int events[] = {
> + EV_KE
Re: [PATCH V2] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
On 04.07.2014 18:17, Michel Dänzer wrote:
> On 04.07.2014 17:51, Ander Conselvan de Oliveira wrote:
>> On 07/04/2014 04:13 AM, Michel Dänzer wrote:
>>> On 03.07.2014 21:27, Ander Conselvan de Oliveira wrote:
On 06/25/2014 05:09 PM, Alvaro Fernando García wrote:
> Init cursor size to 64x64 if drmGetCap() fails.
>
> Use Mesa GBM_BO_USE_CURSOR define (which removes 64x64 restriction)
>
> Signed-off-by: Alvaro Fernando García
> ---
>src/compositor-drm.c | 43
> ---
>1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 7d514e4..61ddea1 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -55,6 +55,14 @@
>#define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
>#endif
>
> +#ifndef DRM_CAP_CURSOR_WIDTH
> +#define DRM_CAP_CURSOR_WIDTH 0x8
> +#endif
> +
> +#ifndef DRM_CAP_CURSOR_HEIGHT
> +#define DRM_CAP_CURSOR_HEIGHT 0x9
> +#endif
> +
>static int option_current_mode = 0;
>
>enum output_config {
[...]
>
> @@ -1554,15 +1577,21 @@ drm_output_init_egl(struct drm_output *output,
> struct drm_compositor *ec)
>return -1;
>}
>
> -flags = GBM_BO_USE_CURSOR_64X64 | GBM_BO_USE_WRITE;
> +#ifdef GBM_BO_USE_CURSOR
> +flags = GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE;
> +#else
> +flags = GBM_BO_USE_WRITE;
> +if (ec->cursor_width == 64 && ec->cursor_height == 64)
> +flags = GBM_BO_USE_CURSOR_64X64 | flags;
> +#endif
Do we really need this? GBM_BO_USE_CURSOR has the same value as the old
_64X64 flag. GBM will check if the dimensions are 64x64 and fail
otherwise.
>>>
>>> No, that check was removed when adding GBM_BO_USE_CURSOR.
>>>
>>>
So this could just be
flags = GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE;
and a
#ifndef GBM_BO_USE_CURSOR
#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
#endif
earlier in the file.
>>>
>>> No, if GBM doesn't define GBM_BO_USE_CURSOR, it will likely fail if the
>>> dimensions are not 64x64.
>>
>> And that is what we expect. We shouldn't rely on a buffer allocated with
>> only GBM_BO_USE_WRITE to use as a hardware cursor.
>>
>> My point is that the code I proposed should work with both old and new
>> GBM. For old GBM, if cursor is not 64x64, gbm won't allocate the bo and
>> the check that follows the allocation will disable hardware cursors. If
>> GBM is new enough, everything works. On top of that, it avoids the ifdef
>> in the middle of a function.
>
> Hmm, yes, that makes sense.
Alvaro, can you simplify the patch according to Ander's suggestion?
--
Earthling Michel Dänzer| http://www.amd.com
Libre software enthusiast |Mesa and X developer
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 3/4] evdev: Release still pressed keys/buttons when removing device
When removing a device, its not guaranteed that all button or key
presses have been released, resulting in an invalid seat wide button
count.
Note that kernel devices normally will send release events when being
unplugged, but this won't happen when removing a device from the path
backend.
Signed-off-by: Jonas Ådahl
---
src/evdev.c | 47 +++
test/keyboard.c | 116
test/pointer.c | 90 +++
3 files changed, 253 insertions(+)
diff --git a/src/evdev.c b/src/evdev.c
index f656a5e..ade7eec 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -60,6 +60,12 @@ is_key_pressed(struct evdev_device *device, int code)
}
static int
+get_key_pressed_count(struct evdev_device *device, int code)
+{
+ return device->key_count[code];
+}
+
+static int
update_key_pressed_count(struct evdev_device *device, int code, int pressed)
{
assert(code >= 0 && code < KEY_CNT);
@@ -331,6 +337,45 @@ get_key_type(uint16_t code)
}
static void
+release_pressed_keys(struct evdev_device *device)
+{
+ struct libinput *libinput = device->base.seat->libinput;
+ struct timespec ts;
+ uint64_t time;
+ int code;
+
+ if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
+ log_bug_libinput(libinput, "clock_gettime: %s\n",
strerror(errno));
+ return;
+ }
+
+ time = ts.tv_sec * 1000ULL + ts.tv_nsec / 100;
+
+ for (code = 0; code < KEY_CNT; code++) {
+ if (get_key_pressed_count(device, code) > 0) {
+ switch (get_key_type(code)) {
+ case EVDEV_KEY_TYPE_NONE:
+ break;
+ case EVDEV_KEY_TYPE_KEY:
+ evdev_keyboard_notify_key(
+ device,
+ time,
+ code,
+ LIBINPUT_KEY_STATE_RELEASED);
+ break;
+ case EVDEV_KEY_TYPE_BUTTON:
+ evdev_pointer_notify_button(
+ device,
+ time,
+ code,
+ LIBINPUT_BUTTON_STATE_RELEASED);
+ break;
+ }
+ }
+ }
+}
+
+static void
evdev_process_touch_button(struct evdev_device *device,
uint64_t time, int value)
{
@@ -1008,6 +1053,8 @@ evdev_device_remove(struct evdev_device *device)
libinput_remove_source(device->base.seat->libinput,
device->source);
+ release_pressed_keys(device);
+
if (device->mtdev)
mtdev_close_delete(device->mtdev);
close_restricted(device->base.seat->libinput, device->fd);
diff --git a/test/keyboard.c b/test/keyboard.c
index a7f500c..dc9c4ea 100644
--- a/test/keyboard.c
+++ b/test/keyboard.c
@@ -25,6 +25,7 @@
#include
#include
+#include "libinput-util.h"
#include "litest.h"
START_TEST(keyboard_seat_key_count)
@@ -163,11 +164,126 @@ START_TEST(keyboard_ignore_no_pressed_release)
}
END_TEST
+static void
+test_key_event(struct litest_device *dev, unsigned int key, int state)
+{
+ struct libinput *li = dev->libinput;
+ struct libinput_event *event;
+ struct libinput_event_keyboard *kevent;
+
+ litest_event(dev, EV_KEY, key, state);
+ litest_event(dev, EV_SYN, SYN_REPORT, 0);
+
+ libinput_dispatch(li);
+
+ event = libinput_get_event(li);
+ ck_assert(event != NULL);
+ ck_assert_int_eq(libinput_event_get_type(event),
LIBINPUT_EVENT_KEYBOARD_KEY);
+
+ kevent = libinput_event_get_keyboard_event(event);
+ ck_assert(kevent != NULL);
+ ck_assert_int_eq(libinput_event_keyboard_get_key(kevent), key);
+ ck_assert_int_eq(libinput_event_keyboard_get_key_state(kevent),
+state ? LIBINPUT_KEY_STATE_PRESSED :
+LIBINPUT_KEY_STATE_RELEASED);
+ libinput_event_destroy(event);
+}
+
+START_TEST(keyboard_key_auto_release)
+{
+ struct libinput *libinput;
+ struct litest_device *dev;
+ struct libinput_event *event;
+ enum libinput_event_type type;
+ struct libinput_event_keyboard *kevent;
+ struct {
+ int code;
+ int released;
+ } keys[] = {
+ { .code = KEY_A, },
+ { .code = KEY_S, },
+ { .code = KEY_D, },
+ { .code = KEY_G, },
+ { .code = KEY_Z, },
+ { .code = KEY_DELETE, },
+ { .code = KEY_F24, },
+ };
+ int events[2 * (ARRAY_LENGTH(keys) + 1)];
+ unsigned i;
+ int key;
+ int v
[PATCH libinput 0/4] Per device button/key counting
Hi, These 4 patch patches replaces 7 and 8 from the previous series. This series brings in, as suggested in the review, the bit mask operations for long's from libevdev, but uses them only for ignoring initial release events. For auto release a byte array for counting button presses is used. By counting presses even per device, we can, as in 4/4 easily decouple the tap FSM from physical separate key presses without queueing multiple pressed events. This enables a tap-drag to continue without interruption by pressing the equivalent physical button and then by motion events continue dragging. It could probably be made so that clickpads also have this behavior when having enabled tapping as well, more or less by removing the TAP_EVENT_BUTTON event from the tap FSM, but I'm not sure what is expected on such a device. Jonas Jonas Ådahl (4): evdev: Ignore key/button release events if key was never pressed evdev: Keep track of button/key press count per device evdev: Release still pressed keys/buttons when removing device touchpad: Only break out of tap FSM for clickpad button presses doc/touchpad-tap-state-machine.svg | 1134 ++-- src/evdev-mt-touchpad-buttons.c| 30 +- src/evdev-mt-touchpad-tap.c| 13 +- src/evdev.c| 129 +++- src/evdev.h| 15 + src/libinput-util.h| 29 + test/keyboard.c| 168 ++ test/pointer.c | 90 +++ test/touchpad.c| 40 +- 9 files changed, 1054 insertions(+), 594 deletions(-) -- 1.8.5.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput] evdev: Let dispatch instances set their own capabilities
It's up to a evdev device backend to configure seat capabilities it
supports. Even though it may be possible for a touchpad to have extra
keys, there is currently no support for sending keyboard events from the
touchpad driver, and if that would be implemented, it'd be a detail of
the touchpad driver, not the generic evdev device part.
Signed-off-by: Jonas Ådahl
---
On Thu, Jul 17, 2014 at 02:25:18PM +1000, Peter Hutterer wrote:
> On Wed, Jul 16, 2014 at 10:39:10PM +0200, Jonas Ådahl wrote:
> > The feature set configured otherwise would not work anyway as it
> > would need using the fallback dispatch to function.
> >
> > Signed-off-by: Jonas Ådahl
> > ---
> > src/evdev.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/src/evdev.c b/src/evdev.c
> > index f980812..fec24f5 100644
> > --- a/src/evdev.c
> > +++ b/src/evdev.c
> > @@ -700,9 +700,11 @@ evdev_configure_device(struct evdev_device *device)
> > !libevdev_has_event_code(evdev, EV_KEY, BTN_TOOL_PEN) &&
> > (has_abs || has_mt)) {
> > device->dispatch = evdev_mt_touchpad_create(device);
> > + device->seat_caps |= EVDEV_DEVICE_POINTER;
> > log_info(libinput,
> > "input device '%s', %s is a touchpad\n",
> > device->devname, device->devnode);
> > + return device->dispatch == NULL ? -1 : 0;
>
> for something that is a touchpad with extra keys you're losing the keyboard
> seat_cap here. same for touch, which iirc you can get for wireless receivers
> where you may have more than one device multiplexed. which the touchpad code
> probably won't handle yet anyway, but still.
That is more or less my point. I rewrote the patch a bit, making the
actual dispatch init function configure its own seat capabilities. If
we make the touchpad driver handle keyboard events, we should only
advertise such a capability when we actually know it will work.
Jonas
src/evdev-mt-touchpad.c | 2 ++
src/evdev.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index b58a6ca..d831b83 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -824,6 +824,8 @@ tp_init(struct tp_dispatch *tp,
if (tp_init_palmdetect(tp, device) != 0)
return -1;
+ device->seat_caps |= EVDEV_DEVICE_POINTER;
+
return 0;
}
diff --git a/src/evdev.c b/src/evdev.c
index f980812..aa4cfae 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -703,6 +703,7 @@ evdev_configure_device(struct evdev_device *device)
log_info(libinput,
"input device '%s', %s is a touchpad\n",
device->devname, device->devnode);
+ return device->dispatch == NULL ? -1 : 0;
}
for (i = KEY_ESC; i < KEY_MAX; i++) {
if (i >= BTN_MISC && i < KEY_OK)
--
1.8.5.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 2/4] evdev: Keep track of button/key press count per device
Keep track of the number of times a given button or key is pressed on a
device. For regular mouse devices or keyboard devices, such a count will
never exceed 1, but counting button presses could help when button
presses with the same code can originate from different sources. One could
for example implement overlapping tap-drags with button presses by
having them deal with their own life-time independently, sorting out
when the user should receive button presses or not depending on the
pressed count.
Signed-off-by: Jonas Ådahl
---
src/evdev-mt-touchpad-buttons.c | 30 +
src/evdev-mt-touchpad-tap.c | 8 +++
src/evdev.c | 49 +
src/evdev.h | 13 +++
4 files changed, 78 insertions(+), 22 deletions(-)
diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index fe33d0b..8a28054 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -606,11 +606,12 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp,
uint64_t time)
state = LIBINPUT_BUTTON_STATE_RELEASED;
}
- if (button)
- pointer_notify_button(&tp->device->base,
- time,
- button,
- state);
+ if (button) {
+ evdev_pointer_notify_button(tp->device,
+ time,
+ button,
+ state);
+ }
return 1;
}
@@ -632,10 +633,10 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t
time)
else
state = LIBINPUT_BUTTON_STATE_RELEASED;
- pointer_notify_button(&tp->device->base,
- time,
- button,
- state);
+ evdev_pointer_notify_button(tp->device,
+ time,
+ button,
+ state);
}
button++;
@@ -707,11 +708,12 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp,
uint64_t time)
tp->buttons.click_pending = false;
- if (button)
- pointer_notify_button(&tp->device->base,
- time,
- button,
- state);
+ if (button) {
+ evdev_pointer_notify_button(tp->device,
+ time,
+ button,
+ state);
+ }
return 1;
}
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 6008507..64801a8 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -113,10 +113,10 @@ tp_tap_notify(struct tp_dispatch *tp,
return;
}
- pointer_notify_button(&tp->device->base,
- time,
- button,
- state);
+ evdev_pointer_notify_button(tp->device,
+ time,
+ button,
+ state);
}
static void
diff --git a/src/evdev.c b/src/evdev.c
index 0f4874c..f656a5e 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -59,6 +59,47 @@ is_key_pressed(struct evdev_device *device, int code)
return long_bit_is_set(device->key_mask, code);
}
+static int
+update_key_pressed_count(struct evdev_device *device, int code, int pressed)
+{
+ assert(code >= 0 && code < KEY_CNT);
+
+ if (pressed) {
+ return ++device->key_count[code];
+ } else {
+ assert(device->key_count[code] > 0);
+ return --device->key_count[code];
+ }
+}
+
+void
+evdev_keyboard_notify_key(struct evdev_device *device,
+ uint32_t time,
+ int key,
+ enum libinput_key_state state)
+{
+ int pressed_count;
+
+ pressed_count = update_key_pressed_count(device, key, state);
+
+ if ((state && pressed_count == 1) || (!state && pressed_count == 0))
+ keyboard_notify_key(&device->base, time, key, state);
+}
+
+void
+evdev_pointer_notify_button(struct evdev_device *device,
+ uint32_t time,
+ int button,
+ enum libinput_button_state state)
+{
+ int pressed_count;
+
+ pressed_count = update_key_pressed_count(device, button, state);
+
+ if ((state && pressed_count == 1) || (!state
[PATCH libinput] test: Use only one test device for some udev and path tests
Some tests in test/path.c and test/udev.c are not dependent on
device behaviour but rather managing of device lifetime etc. Run those
tests only once with only one device, resulting more or less the same
code coverage but shorter run time.
Signed-off-by: Jonas Ådahl
---
> hmm, I think instead of litest_add_once which is pretty much unpredictable
> maybe just add a litest_add_for_device() call where you can pass the enum
> litest_device_type in. That way you get the same benefits as your patch
> here, with the benefit of knowing which device the test runs for on any
> given system.
>
> so instad of
> litest_add_once("udev:suspend", udev_double_suspend, LITEST_ANY,
> LITEST_ANY);
> you have
> litest_add_for_device("udev:suspend", udev_double_suspend,
> LITEST_SYNAPTICS_CLICKPAD);
>
Sure, works for me. Updated version of the patch:
test/litest.c | 45 +++--
test/litest.h | 4
test/path.c | 13 +++--
test/udev.c | 15 ---
4 files changed, 54 insertions(+), 23 deletions(-)
diff --git a/test/litest.c b/test/litest.c
index deab0cf..946a0d7 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -179,11 +179,8 @@ litest_add_no_device(const char *name, void *func)
litest_add(name, func, LITEST_DISABLE_DEVICE, LITEST_DISABLE_DEVICE);
}
-void
-litest_add(const char *name,
- void *func,
- enum litest_device_feature required,
- enum litest_device_feature excluded)
+static struct suite *
+get_suite(const char *name)
{
struct suite *s;
@@ -191,10 +188,8 @@ litest_add(const char *name,
list_init(&all_tests);
list_for_each(s, &all_tests, node) {
- if (strcmp(s->name, name) == 0) {
- litest_add_tcase(s, func, required, excluded);
- return;
- }
+ if (strcmp(s->name, name) == 0)
+ return s;
}
s = zalloc(sizeof(*s));
@@ -203,7 +198,37 @@ litest_add(const char *name,
list_init(&s->tests);
list_insert(&all_tests, &s->node);
- litest_add_tcase(s, func, required, excluded);
+
+ return s;
+}
+
+void
+litest_add(const char *name,
+ void *func,
+ enum litest_device_feature required,
+ enum litest_device_feature excluded)
+{
+ litest_add_tcase(get_suite(name), func, required, excluded);
+}
+
+void
+litest_add_for_device(const char *name,
+ void *func,
+ enum litest_device_type type)
+{
+ struct suite *s;
+ struct litest_test_device **dev = devices;
+
+ s = get_suite(name);
+ while (*dev) {
+ if ((*dev)->type == type) {
+ litest_add_tcase_for_device(s, func, *dev);
+ return;
+ }
+ dev++;
+ }
+
+ ck_abort_msg("Invalid test device type");
}
static int
diff --git a/test/litest.h b/test/litest.h
index 9a9d10a..c0035d3 100644
--- a/test/litest.h
+++ b/test/litest.h
@@ -76,6 +76,10 @@ struct libinput *litest_create_context(void);
void litest_add(const char *name, void *func,
enum litest_device_feature required_feature,
enum litest_device_feature excluded_feature);
+void
+litest_add_for_device(const char *name,
+ void *func,
+ enum litest_device_type type);
void litest_add_no_device(const char *name, void *func);
int litest_run(int argc, char **argv);
diff --git a/test/path.c b/test/path.c
index 475e125..f6f6b84 100644
--- a/test/path.c
+++ b/test/path.c
@@ -793,8 +793,9 @@ START_TEST(path_seat_recycle)
}
END_TEST
-int main (int argc, char **argv) {
-
+int
+main(int argc, char **argv)
+{
litest_add_no_device("path:create", path_create_NULL);
litest_add_no_device("path:create", path_create_invalid);
litest_add_no_device("path:create", path_create_destroy);
@@ -804,13 +805,13 @@ int main (int argc, char **argv) {
litest_add_no_device("path:suspend", path_add_device_suspend_resume);
litest_add_no_device("path:suspend",
path_add_device_suspend_resume_fail);
litest_add_no_device("path:suspend",
path_add_device_suspend_resume_remove_device);
- litest_add("path:seat events", path_added_seat, LITEST_ANY, LITEST_ANY);
+ litest_add_for_device("path:seat events", path_added_seat,
LITEST_CLICKPAD);
litest_add("path:device events", path_added_device, LITEST_ANY,
LITEST_ANY);
litest_add("path:device events", path_device_sysname, LITEST_ANY,
LITEST_ANY);
- litest_add("path:device events", path_add_device, LITEST_ANY,
LITEST_ANY);
+ litest_add_for_device("path:device events", path_add_device,
LITEST_CLICKPAD);
litest_add_no_device("path:device events", path_add_invalid_path);
- litest_add("path:device events", path_remove_device, LITEST_ANY,
LITEST_ANY);
- lite
[PATCH libinput 1/4] evdev: Ignore key/button release events if key was never pressed
The kernel may send a 'release' event without ever having sent a key
'pressed' event in case the key was pressed before libinput was
initiated. Ignore these events so that we always guarantee a release
event always comes after a pressed event for any given key or button.
Signed-off-by: Jonas Ådahl
---
src/evdev.c | 33 -
src/evdev.h | 2 ++
src/libinput-util.h | 29 +
test/keyboard.c | 52
4 files changed, 115 insertions(+), 1 deletion(-)
diff --git a/src/evdev.c b/src/evdev.c
index 5872856..0f4874c 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -47,6 +47,18 @@ enum evdev_key_type {
EVDEV_KEY_TYPE_BUTTON,
};
+static void
+set_key_pressed(struct evdev_device *device, int code, int pressed)
+{
+ long_set_bit_state(device->key_mask, code, pressed);
+}
+
+static int
+is_key_pressed(struct evdev_device *device, int code)
+{
+ return long_bit_is_set(device->key_mask, code);
+}
+
void
evdev_device_led_update(struct evdev_device *device, enum libinput_led leds)
{
@@ -294,6 +306,8 @@ static inline void
evdev_process_key(struct evdev_device *device,
struct input_event *e, uint64_t time)
{
+ enum evdev_key_type type;
+
/* ignore kernel key repeat */
if (e->value == 2)
return;
@@ -306,7 +320,24 @@ evdev_process_key(struct evdev_device *device,
evdev_flush_pending_event(device, time);
- switch (get_key_type(e->code)) {
+ type = get_key_type(e->code);
+
+ /* Ignore key release events from the kernel for keys that libinput
+* never got a pressed event for. */
+ if (e->value == 0) {
+ switch (type) {
+ case EVDEV_KEY_TYPE_NONE:
+ break;
+ case EVDEV_KEY_TYPE_KEY:
+ case EVDEV_KEY_TYPE_BUTTON:
+ if (!is_key_pressed(device, e->code))
+ return;
+ }
+ }
+
+ set_key_pressed(device, e->code, e->value);
+
+ switch (type) {
case EVDEV_KEY_TYPE_NONE:
break;
case EVDEV_KEY_TYPE_KEY:
diff --git a/src/evdev.h b/src/evdev.h
index fad1f84..f71d387 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -94,6 +94,8 @@ struct evdev_device {
struct {
struct motion_filter *filter;
} pointer;
+
+ unsigned long key_mask[NLONGS(KEY_CNT)];
};
#define EVDEV_UNHANDLED_DEVICE ((struct evdev_device *) 1)
diff --git a/src/libinput-util.h b/src/libinput-util.h
index c0235ef..2f1a1db 100644
--- a/src/libinput-util.h
+++ b/src/libinput-util.h
@@ -72,6 +72,8 @@ int list_empty(const struct list *list);
pos = tmp, \
tmp = container_of(pos->member.next, tmp, member))
+#define LONG_BITS (sizeof(long) * 8)
+#define NLONGS(x) (((x) + LONG_BITS - 1) / LONG_BITS)
#define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
#define ARRAY_FOR_EACH(_arr, _elem) \
for (size_t _i = 0; (_elem = &_arr[_i]) && _i < ARRAY_LENGTH(_arr);
_i++)
@@ -150,4 +152,31 @@ vector_get_direction(int dx, int dy)
return dir;
}
+static inline int
+long_bit_is_set(const unsigned long *array, int bit)
+{
+return !!(array[bit / LONG_BITS] & (1LL << (bit % LONG_BITS)));
+}
+
+static inline void
+long_set_bit(unsigned long *array, int bit)
+{
+array[bit / LONG_BITS] |= (1LL << (bit % LONG_BITS));
+}
+
+static inline void
+long_clear_bit(unsigned long *array, int bit)
+{
+array[bit / LONG_BITS] &= ~(1LL << (bit % LONG_BITS));
+}
+
+static inline void
+long_set_bit_state(unsigned long *array, int bit, int state)
+{
+ if (state)
+ long_set_bit(array, bit);
+ else
+ long_clear_bit(array, bit);
+}
+
#endif /* LIBINPUT_UTIL_H */
diff --git a/test/keyboard.c b/test/keyboard.c
index a55405c..a7f500c 100644
--- a/test/keyboard.c
+++ b/test/keyboard.c
@@ -112,10 +112,62 @@ START_TEST(keyboard_seat_key_count)
}
END_TEST
+START_TEST(keyboard_ignore_no_pressed_release)
+{
+ struct litest_device *dev;
+ struct libinput *libinput;
+ struct libinput_event *event;
+ struct libinput_event_keyboard *kevent;
+ int events[] = {
+ EV_KEY, KEY_A,
+ -1, -1,
+ };
+ enum libinput_key_state *state;
+ enum libinput_key_state expected_states[] = {
+ LIBINPUT_KEY_STATE_PRESSED,
+ LIBINPUT_KEY_STATE_RELEASED,
+ };
+
+
+ libinput = litest_create_context();
+ dev = litest_add_device_with_overrides(libinput,
+ LITEST_KEYBOARD,
+ "Generic keyboard",
+ NULL, NULL, events);
+
+ litest_drain_events(libinput);
+
+ litest_keyboard_key(dev,
Re: wayland: how should dispatch the default wayland display queue?
Hi Daniel, thanks, i understood we should add the wl_display_dispatch_pending call in the application there is currently no way to avoid this and basically it does not harm. I only wanted to understand if there is something missing. 2014-07-27 22:41 GMT+02:00 Daniel Stone : > Hi Eugen, > > > On Sunday, July 27, 2014, Eugen Friedrich wrote: >> >> Our graphics stack creates it's own wayland queue and uses this queue for >> all wayland objects and only this queue will be dispatched. Our graphics >> stack calls also wl_display_sync api and the issue is that the wayland >> library will send a delete event which will go to the default queue of >> wayland display but this is never dispatched from the driver and the memory >> consumption increases. >> >> So should the graphics stack dispatch also the default queue or should >> application call somewhere wl_display_dispatch_pending? >> > > The EGL implementation must never dispatch the default queue. > > There is a new convenience call in development to fix this, but for now, > you have to call wl_display_roundtrip, wl_callback_set_queue, and > wl_callback_add_listener. You can see this used in Mesa. > > Hope this helps. > > Cheers, > Daniel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] test: add a semi-mt Alps test device
Hi,
On 07/24/2014 08:53 AM, Peter Hutterer wrote:
> Provides the bounding box only, with slot 0 always being the upper/left, slot
> 1 being the lower-right touch. This needs to use the touch_down etc. litest
> interfaces, which are now widened to double (leftover from 489630f58) and a
> device-specific private pointer in the litest device.
>
> New device feature for litest: LITEST_SEMI_MT
>
> Signed-off-by: Peter Hutterer
Thanks for working on this!
A few very small nitpicks, see below.
> ---
> test/Makefile.am | 1 +
> test/litest-alps-semi-mt.c | 258
> +
> test/litest-int.h | 4 +-
> test/litest.c | 17 +--
> test/litest.h | 6 ++
> 5 files changed, 277 insertions(+), 9 deletions(-)
> create mode 100644 test/litest-alps-semi-mt.c
>
> diff --git a/test/Makefile.am b/test/Makefile.am
> index c3c293a..35c3bf8 100644
> --- a/test/Makefile.am
> +++ b/test/Makefile.am
> @@ -15,6 +15,7 @@ liblitest_la_SOURCES = \
> ../src/libinput-util.c \
> litest.h \
> litest-int.h \
> + litest-alps-semi-mt.c \
> litest-bcm5974.c \
> litest-keyboard.c \
> litest-mouse.c \
> diff --git a/test/litest-alps-semi-mt.c b/test/litest-alps-semi-mt.c
> new file mode 100644
> index 000..2e85540
> --- /dev/null
> +++ b/test/litest-alps-semi-mt.c
> @@ -0,0 +1,258 @@
> +/*
> + * Copyright © 2014 Red Hat, Inc.
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and
> its
> + * documentation for any purpose is hereby granted without fee, provided that
> + * the above copyright notice appear in all copies and that both that
> copyright
> + * notice and this permission notice appear in supporting documentation, and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission. The copyright holders make no representations
> + * about the suitability of this software for any purpose. It is provided
> "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF
> USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#if HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include
> +
> +#include "libinput-util.h"
> +
> +#include "litest.h"
> +#include "litest-int.h"
> +
> +static int tracking_id;
> +
> +/* this is a semi-mt device, so we keep track of the touches that the tests
> + * send and modify them so that the first touch is always slot 0 and sends
> + * the top-left of the bounding box, the second is always slot 1 and sends
> + * the bottom-right of the bounding box.
> + * Lifting any of two fingers terminates slot 1
> + */
> +struct alps {
> + /* The actual touches requested by the test for the two slots
> + * in the 0..100 range used by litest */
> + struct {
> + double x, y;
> + } touches[2];
> +};
> +
> +static void alps_create(struct litest_device *d);
> +
> +static void
> +litest_alps_setup(void)
> +{
> + struct litest_device *d = litest_create_device(LITEST_ALPS_SEMI_MT);
> + litest_set_current_device(d);
> +}
> +
> +static void
> +send_abs_xy(struct litest_device *d, double x, double y)
> +{
> + struct input_event e;
> + int val;
> +
> + e.type = EV_ABS;
> + e.code = ABS_X;
> + e.value = LITEST_AUTO_ASSIGN;
> + val = litest_auto_assign_value(d, &e, 0, x, y);
> + litest_event(d, EV_ABS, ABS_X, val);
> +
> + e.code = ABS_Y;
> + val = litest_auto_assign_value(d, &e, 0, x, y);
> + litest_event(d, EV_ABS, ABS_Y, val);
> +}
> +
> +static void
> +send_abs_mt_xy(struct litest_device *d, double x, double y)
> +{
> + struct input_event e;
> + int val;
> +
> + e.type = EV_ABS;
> + e.code = ABS_MT_POSITION_X;
> + e.value = LITEST_AUTO_ASSIGN;
> + val = litest_auto_assign_value(d, &e, 0, x, y);
> + litest_event(d, EV_ABS, ABS_MT_POSITION_X, val);
> +
> + e.code = ABS_MT_POSITION_Y;
> + e.value = LITEST_AUTO_ASSIGN;
> + val = litest_auto_assign_value(d, &e, 0, x, y);
> + litest_event(d, EV_ABS, ABS_MT_POSITION_Y, val);
> +}
> +
> +static void
> +alps_touch_down(struct litest_device *d, unsigned int slot, double x, double
> y)
> +{
> + struct alps *alps = d->private;
> + double t, l, r, b; /* top, left, right, bottom */
> +
> + if (d->ntouches_down > 2 || slot > 1)
> + return;
> +
> + if (d->ntouches_down == 1) {
> + l = x;
Re: wayland: how should dispatch the default wayland display queue?
Hi Eugen, On Sunday, July 27, 2014, Eugen Friedrich wrote: > > Our graphics stack creates it's own wayland queue and uses this queue for > all wayland objects and only this queue will be dispatched. Our graphics > stack calls also wl_display_sync api and the issue is that the wayland > library will send a delete event which will go to the default queue of > wayland display but this is never dispatched from the driver and the memory > consumption increases. > > So should the graphics stack dispatch also the default queue or should > application call somewhere wl_display_dispatch_pending? > The EGL implementation must never dispatch the default queue. There is a new convenience call in development to fix this, but for now, you have to call wl_display_roundtrip, wl_callback_set_queue, and wl_callback_add_listener. You can see this used in Mesa. Hope this helps. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wayland: how should dispatch the default wayland display queue?
O thanks for the hint about mailing list, and for clarification on the dispatching logic(I'm not original English speaker and assume "havok" is like behavior? ) 2014-07-27 21:33 GMT+02:00 Giulio Camuffo : > FIY, You dropped the mailing list in your reply. > > 2014-07-27 22:15 GMT+03:00 Eugen Friedrich : > > The problem is the app never calls wl_display_distpath(_pending) > > it just creates native handles and then uses EGL for rendering and the > > events on the main wayland display queue are never dispatched > > > > I think if EGL triggers the event(ba calling wl_diplay_sync) it should be > > also responsible for dispatching > > EGL should not dispatch the main queue, that would break havok in the > apps. What you should do is dispatch the main queue in your app's main > loop. > > > > > > > > > 2014-07-27 20:42 GMT+02:00 Giulio Camuffo : > > > >> 2014-07-27 21:14 GMT+03:00 Eugen Friedrich : > >> > Hi all, > >> > > >> > after debugging a memory consumption issue in simple wayland > application > >> > i > >> > found a issue in our wayland graphics backend. Then i took a look into > >> > the > >> > mesa implementation and found the quite similar implementation there. > >> > > >> > Our graphics stack creates it's own wayland queue and uses this queue > >> > for > >> > all wayland objects and only this queue will be dispatched. Our > graphics > >> > stack calls also wl_display_sync api and the issue is that the wayland > >> > library will send a delete event which will go to the default queue of > >> > wayland display but this is never dispatched from the driver and the > >> > memory > >> > consumption increases. > >> > > >> > So should the graphics stack dispatch also the default queue or should > >> > application call somewhere wl_display_dispatch_pending? > >> > >> I don't see why the memory consumption should increase. The app should > >> call wl_display_distpath(_pending) in its main loop, but I'm afraid i > >> don't quite understand your problem. > >> > >> -- > >> Giulio > >> > >> > >> > > >> > ___ > >> > wayland-devel mailing list > >> > wayland-devel@lists.freedesktop.org > >> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > >> > > > > > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wayland: how should dispatch the default wayland display queue?
FIY, You dropped the mailing list in your reply. 2014-07-27 22:15 GMT+03:00 Eugen Friedrich : > The problem is the app never calls wl_display_distpath(_pending) > it just creates native handles and then uses EGL for rendering and the > events on the main wayland display queue are never dispatched > > I think if EGL triggers the event(ba calling wl_diplay_sync) it should be > also responsible for dispatching EGL should not dispatch the main queue, that would break havok in the apps. What you should do is dispatch the main queue in your app's main loop. > > > > 2014-07-27 20:42 GMT+02:00 Giulio Camuffo : > >> 2014-07-27 21:14 GMT+03:00 Eugen Friedrich : >> > Hi all, >> > >> > after debugging a memory consumption issue in simple wayland application >> > i >> > found a issue in our wayland graphics backend. Then i took a look into >> > the >> > mesa implementation and found the quite similar implementation there. >> > >> > Our graphics stack creates it's own wayland queue and uses this queue >> > for >> > all wayland objects and only this queue will be dispatched. Our graphics >> > stack calls also wl_display_sync api and the issue is that the wayland >> > library will send a delete event which will go to the default queue of >> > wayland display but this is never dispatched from the driver and the >> > memory >> > consumption increases. >> > >> > So should the graphics stack dispatch also the default queue or should >> > application call somewhere wl_display_dispatch_pending? >> >> I don't see why the memory consumption should increase. The app should >> call wl_display_distpath(_pending) in its main loop, but I'm afraid i >> don't quite understand your problem. >> >> -- >> Giulio >> >> >> > >> > ___ >> > wayland-devel mailing list >> > wayland-devel@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel >> > > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wayland: how should dispatch the default wayland display queue?
2014-07-27 21:14 GMT+03:00 Eugen Friedrich : > Hi all, > > after debugging a memory consumption issue in simple wayland application i > found a issue in our wayland graphics backend. Then i took a look into the > mesa implementation and found the quite similar implementation there. > > Our graphics stack creates it's own wayland queue and uses this queue for > all wayland objects and only this queue will be dispatched. Our graphics > stack calls also wl_display_sync api and the issue is that the wayland > library will send a delete event which will go to the default queue of > wayland display but this is never dispatched from the driver and the memory > consumption increases. > > So should the graphics stack dispatch also the default queue or should > application call somewhere wl_display_dispatch_pending? I don't see why the memory consumption should increase. The app should call wl_display_distpath(_pending) in its main loop, but I'm afraid i don't quite understand your problem. -- Giulio > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
wayland: how should dispatch the default wayland display queue?
Hi all, after debugging a memory consumption issue in simple wayland application i found a issue in our wayland graphics backend. Then i took a look into the mesa implementation and found the quite similar implementation there. Our graphics stack creates it's own wayland queue and uses this queue for all wayland objects and only this queue will be dispatched. Our graphics stack calls also wl_display_sync api and the issue is that the wayland library will send a delete event which will go to the default queue of wayland display but this is never dispatched from the driver and the memory consumption increases. So should the graphics stack dispatch also the default queue or should application call somewhere wl_display_dispatch_pending? ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
