Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Wed, Jun 04, 2014 at 08:00:17AM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 10:42:50PM +0200, Jonas Ådahl wrote: On Tue, Jun 03, 2014 at 03:34:56PM +1000, Peter Hutterer wrote: Now that we have run-time changes of the tap.enabled state move the check to the IDLE state only. Otherwise the tap machine may hang if tapping is disabled while a gesture is in progress. Two basic tests are added to check for the tap default setting - which is now tap disabled by default, because a bike shed's correct color is green. Say what? I might have missed some joke reference, but why would you want to disable tapping by default? dates back a couple of years where we had some amusing back/forth in the synaptics driver in regards to tapping enabled or disabled by default. long story short, we ended up disabling it by default for two reasons: * if you don't know that tapping is a thing (or enabled by default), you get spurious mouse events that make the desktop feel buggy. * if you do know what tapping is and you want it, you usually know where to enable it, or at least you can search for it. Of course, there is merry disagreement on this but I still think those two reasons above justify disabling tapping by default. Fair enough. I won't start such a thread again, even though it means one of my touchpads will not work by default (as the physical buttons are broken, although reported existing :P). I suppose however that certain models are designed to be used for tapping having it enabled in the preinstalled system, but I wouldn't know how to get that information. Jonas Cheers, Peter Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-tap.c | 56 +++-- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 32 ++ 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index eee334f..24ea8c3 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -438,13 +438,13 @@ static void tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) { enum tp_tap_state current; - if (!tp-tap.enabled) - return; current = tp-tap.state; switch(tp-tap.state) { case TAP_STATE_IDLE: + if (!tp-tap.enabled) + break; tp_tap_idle_handle_event(tp, event, time); break; case TAP_STATE_TOUCH: @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data) unsigned int tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) { - if (!tp-tap.enabled) - return 0; - if (tp-tap.timeout tp-tap.timeout = time) { tp_tap_clear_timer(tp); tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) return tp-tap.timeout; } +static int +tp_tap_config_count(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return min(tp-ntouches, 3); /* we only do up to 3 finger tap */ +} + +static enum libinput_config_status +tp_tap_config_enable(struct libinput_device *device, int enabled) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; nit: (struct bla *) var, and 80 chars line limit (here and below, and in patch 5). + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + if (tp_tap_config_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + tp-tap.enabled = enabled; + + return LIBINPUT_CONFIG_STATUS_SUCCESS; +} + +static int +tp_tap_config_is_enabled(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return tp-tap.enabled; +} + +static void +tp_tap_config_reset(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + tp-tap.enabled = false; +} + int tp_init_tap(struct tp_dispatch *tp) { + tp-tap.config.count = tp_tap_config_count; + tp-tap.config.enable = tp_tap_config_enable; + tp-tap.config.is_enabled = tp_tap_config_is_enabled; + tp-tap.config.reset = tp_tap_config_reset; + tp-device-base.config.tap = tp-tap.config; + tp-tap.state = TAP_STATE_IDLE; tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); @@ -603,8 +647,6 @@ tp_init_tap(struct
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
On Wed, Jun 04, 2014 at 11:17:58AM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 10:41:16PM +0200, Jonas Ådahl wrote: On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; Shouldn't this be returned also if !enable? It would be a bit confusing if disabling is allowed, but enabling is not. This was a conscious decision that applies to the other functions as well: if you're doing something that's not technically invalid, we're not reporting an error. This means that an error only needs to be handled if something really is off. I'd argue that disabling tapping on a device that doesn't support tapping is a technically invalid operation that is simply handled in the cases where it makes no sense. Also, in what circumstances will config.tap be set, but finger count returning 0? What type of devices would have tapping configuration enabled with no tapping possible? I expect none, this approach was just the safest and most copy-paste-proof way of checking. + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. Should it be libinput_device_config_tap_enable() here instead? ah, thanks. one always escapes the refacturing... + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +int +libinput_device_config_tap_get_finger_count(struct libinput_device *device); I wonder where doing things like this, and libinput_device_config_scroll_get_methods() will lead us. It'd mean we expose some device information via the config API, and some via direct device getters, which might not be the best thing for consistency. Just to be clear - you're talking about ditching
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
Hi, On 06/04/2014 10:21 AM, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 11:17:58AM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 10:41:16PM +0200, Jonas Ådahl wrote: On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, +int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, +int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; Shouldn't this be returned also if !enable? It would be a bit confusing if disabling is allowed, but enabling is not. This was a conscious decision that applies to the other functions as well: if you're doing something that's not technically invalid, we're not reporting an error. This means that an error only needs to be handled if something really is off. I'd argue that disabling tapping on a device that doesn't support tapping is a technically invalid operation that is simply handled in the cases where it makes no sense. I'm with Peter here, that its easier for our API consumers if they caan always safely call disable. Also, in what circumstances will config.tap be set, but finger count returning 0? What type of devices would have tapping configuration enabled with no tapping possible? I expect none, this approach was just the safest and most copy-paste-proof way of checking. + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. Should it be libinput_device_config_tap_enable() here instead? ah, thanks. one always escapes the refacturing... + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +int +libinput_device_config_tap_get_finger_count(struct libinput_device *device); I wonder where doing things like this, and libinput_device_config_scroll_get_methods() will lead us. It'd mean we expose some device information via the config API, and some via direct device getters, which might not be the best thing for consistency. Just to be clear - you're talking about ditching the _config_ part of the interface? If so, yeah, doable but I
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} Didn't we agree in an off-list discussion to add a get_default instead, so to that ie a cmdline app for querying config info can not only display the current value but also the device default value for a config option ? This get_default would replace the reset, apps can simple implement that functionality themselves through the get_default. Regards, Hans diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +int +libinput_device_config_tap_get_finger_count(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable tap-to-click on this device, with a default mapping of 1, 2, 3 + * finger tap mapping to left, right, middle click, respectively. + * Tapping is limited by the number of simultaneous touches + * supported by the device, see + * libinput_device_config_tap_get_finger_count(). + * + * @param device The device to configure + * @param enable Non-zero to enable, zero to disable + * + * @return A config status code. Disabling tapping on a device that does not + * support tapping always succeeds. + * + * @see libinput_device_config_tap_get_finger_count + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable); + +/** + * @ingroup config + * + * Check if tap-to-click is enabled on this device. If the device does not + * support tapping, this function always returns 0. + * + * @param device The device to configure + * + * @return 1 if enabled, 0 otherwise. + * + * @see
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
On 4/06/2014 18:40 , Hans de Goede wrote: Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} Didn't we agree in an off-list discussion to add a get_default instead, so to that ie a cmdline app for querying config info can not only display the current value but also the device default value for a config option ? This get_default would replace the reset, apps can simple implement that functionality themselves through the get_default. huh, yes, now I remember. I knew something was missing. I'll fix this series up with this in mind, sorry about that. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Now that we have run-time changes of the tap.enabled state move the check to the IDLE state only. Otherwise the tap machine may hang if tapping is disabled while a gesture is in progress. Two basic tests are added to check for the tap default setting - which is now tap disabled by default, because a bike shed's correct color is green. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-tap.c | 56 +++-- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 32 ++ 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index eee334f..24ea8c3 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -438,13 +438,13 @@ static void tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) { enum tp_tap_state current; - if (!tp-tap.enabled) - return; current = tp-tap.state; switch(tp-tap.state) { case TAP_STATE_IDLE: + if (!tp-tap.enabled) + break; tp_tap_idle_handle_event(tp, event, time); break; case TAP_STATE_TOUCH: @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data) unsigned int tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) { - if (!tp-tap.enabled) - return 0; - if (tp-tap.timeout tp-tap.timeout = time) { tp_tap_clear_timer(tp); tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) return tp-tap.timeout; } +static int +tp_tap_config_count(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return min(tp-ntouches, 3); /* we only do up to 3 finger tap */ +} + +static enum libinput_config_status +tp_tap_config_enable(struct libinput_device *device, int enabled) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; Please user container_of here for proper type checking rather then just a blatant cast of $random_type_foo to $random_type_bar. Same for all the other occurrences of the same pattern below. + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + if (tp_tap_config_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; This is a repeat of the test done by the higher level wrapper, as such this seems unnecessary by me. Regards, Hans + + tp-tap.enabled = enabled; + + return LIBINPUT_CONFIG_STATUS_SUCCESS; +} + +static int +tp_tap_config_is_enabled(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return tp-tap.enabled; +} + +static void +tp_tap_config_reset(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + tp-tap.enabled = false; +} + int tp_init_tap(struct tp_dispatch *tp) { + tp-tap.config.count = tp_tap_config_count; + tp-tap.config.enable = tp_tap_config_enable; + tp-tap.config.is_enabled = tp_tap_config_is_enabled; + tp-tap.config.reset = tp_tap_config_reset; + tp-device-base.config.tap = tp-tap.config; + tp-tap.state = TAP_STATE_IDLE; tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); @@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp) return -1; } - tp-tap.enabled = 1; /* FIXME */ - return 0; } diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index d514ed6..0b31e9a 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -203,6 +203,7 @@ struct tp_dispatch { struct { bool enabled; + struct libinput_device_config_tap config; int timer_fd; struct libinput_source *source; unsigned int timeout; diff --git a/test/touchpad.c b/test/touchpad.c index 35781c3..060b529 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap) struct libinput *li = dev-libinput; struct libinput_event *event; + libinput_device_config_tap_enable(dev-libinput_device, 1); + litest_drain_events(li); litest_touch_down(dev, 0, 50, 50); @@ -141,6 +143,8 @@ START_TEST(touchpad_1fg_tap_n_drag) struct libinput *li = dev-libinput; struct libinput_event *event; +
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net I see that in all the other config interfaces you use get / set, yet here you use enable / is_enabled. I think it would be more consistent to use get / set with a bool return / argument here too. This would also mix better with the get_default I've suggested. Regards, Hans --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +int +libinput_device_config_tap_get_finger_count(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable tap-to-click on this device, with a default mapping of 1, 2, 3 + * finger tap mapping to left, right, middle click, respectively. + * Tapping is limited by the number of simultaneous touches + * supported by the device, see + * libinput_device_config_tap_get_finger_count(). + * + * @param device The device to configure + * @param enable Non-zero to enable, zero to disable + * + * @return A config status code. Disabling tapping on a device that does not + * support tapping always succeeds. + * + * @see libinput_device_config_tap_get_finger_count + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable); + +/** + * @ingroup config + * + * Check if tap-to-click is enabled on this device. If the device does not + * support tapping, this function always returns 0. + * + * @param device The device to configure + * + * @return 1 if enabled, 0 otherwise. + * + * @see libinput_device_config_tap_get_finger_count + * @see libinput_device_config_tap_enable + * @see
Re: [PATCH libinput 04/10] Add a config interface for scroll methods
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: --- src/libinput-private.h | 9 ++ src/libinput.c | 35 src/libinput.h | 74 ++ 3 files changed, 118 insertions(+) Looks good to me. Regards, Hans diff --git a/src/libinput-private.h b/src/libinput-private.h index 020167e..d3570a4 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -77,8 +77,17 @@ struct libinput_device_config_tap { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_scroll { + int (*methods)(struct libinput_device *device); + enum libinput_config_status (*set)(struct libinput_device *device, +enum libinput_scroll_method method); + enum libinput_scroll_method (*get)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; + struct libinput_device_config_scroll *scroll; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 6a713bb..b2388e6 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1215,3 +1215,38 @@ libinput_device_config_tap_reset(struct libinput_device *device) if (device-config.tap) device-config.tap-reset(device); } + +LIBINPUT_EXPORT unsigned int +libinput_device_config_scroll_get_methods(struct libinput_device *device) +{ + return device-config.scroll ? + device-config.scroll-methods(device) : + LIBINPUT_SCROLL_METHOD_NONE; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_scroll_set_method(struct libinput_device *device, + enum libinput_scroll_method method) +{ + if ((method libinput_device_config_scroll_get_methods(device)) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.scroll-set(device, method); +} + +LIBINPUT_EXPORT enum libinput_scroll_method +libinput_device_config_scroll_get_method(struct libinput_device *device) +{ + if (libinput_device_config_scroll_get_methods(device) == + LIBINPUT_SCROLL_METHOD_NONE) + return LIBINPUT_SCROLL_METHOD_NONE; + + return device-config.scroll-get(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_scroll_reset(struct libinput_device *device) +{ + if (device-config.scroll) + device-config.scroll-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 0c84547..571f7ba 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1434,6 +1434,80 @@ libinput_device_config_tap_is_enabled(struct libinput_device *device); void libinput_device_config_tap_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Devices without a physical scroll wheel (such as touchpads) may emulate + * scroll events in software through one or more methods. + */ +enum libinput_scroll_method { + /** + * No scroll method available or selected. + */ + LIBINPUT_SCROLL_METHOD_NONE = 0, + /** + * Scrolling is triggered by moving a finger at the edge of the + * touchpad. + */ + LIBINPUT_SCROLL_METHOD_EDGE = (1 0), + /** + * Scrolling is triggered by moving two fingers simultaneously. + */ + LIBINPUT_SCROLL_METHOD_TWOFINGER = (1 1), +}; + +/** + * @ingroup config + * + * Check the available scroll methods on this device. + * + * @param device The device to configure + * + * @return A bitmask of the available scroll methods + */ +unsigned int +libinput_device_config_scroll_get_methods(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the scroll method on this device. Only one method at a time may be + * chosen for each device. + * + * @param device The device to configure + * @param method The scroll method to chose + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_scroll_set_method(struct libinput_device *device, + enum libinput_scroll_method method); + +/** + * @ingroup config + * + * Get the currently selected scroll method on this device. If a device does + * not support configurable scroll methods, the return value is always + * LIBINPUT_SCROLL_METHOD_NONE. + * + * @param device The device to configure + * + * @return The currently selected scroll method + */ +enum libinput_scroll_method +libinput_device_config_scroll_get_method(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset to the default scroll method for this device, if any. If the device + * does not support configurable scroll methods this function does nothing. + * + * @param device The device to configure + */ +void
Re: [PATCH libinput 05/10] touchpad: hook up scroll config
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 43 +++ src/evdev-mt-touchpad.h | 1 + 2 files changed, 44 insertions(+) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 26d5f7d..c1c994a 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -737,8 +737,51 @@ tp_init_accel(struct tp_dispatch *touchpad, double diagonal) } static int +tp_config_scroll_methods(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; As with previous patches, please user container_of here for proper type checking. Same for all the other occurrences of the same pattern below. Otherwise this looks good. Regards, Hans + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return LIBINPUT_SCROLL_METHOD_TWOFINGER; +} + +static enum libinput_config_status +tp_config_scroll_set(struct libinput_device *device, + enum libinput_scroll_method method) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + if (method != LIBINPUT_SCROLL_METHOD_TWOFINGER) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return LIBINPUT_CONFIG_STATUS_SUCCESS; +} + +static enum libinput_scroll_method +tp_config_scroll_get(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return LIBINPUT_SCROLL_METHOD_TWOFINGER; +} + +static void +tp_config_scroll_reset(struct libinput_device *device) +{ + /* two-finger scrolling is hardcoded, nothing to do */ +} + +static int tp_init_scroll(struct tp_dispatch *tp) { + tp-scroll.config.methods = tp_config_scroll_methods; + tp-scroll.config.set = tp_config_scroll_set; + tp-scroll.config.get = tp_config_scroll_get; + tp-scroll.config.reset = tp_config_scroll_reset; + tp-device-base.config.scroll = tp-scroll.config; + tp-scroll.direction = 0; tp-scroll.state = SCROLL_STATE_NONE; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 0b31e9a..d89d74c 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -195,6 +195,7 @@ struct tp_dispatch { } buttons; /* physical buttons */ struct { + struct libinput_device_config_scroll config; enum scroll_state state; enum libinput_pointer_axis direction; } scroll; ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 06/10] Add a config interface to change device rotation
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 9 +++ src/libinput.c | 33 src/libinput.h | 69 ++ 3 files changed, 111 insertions(+) Looks good. Regards, Hans diff --git a/src/libinput-private.h b/src/libinput-private.h index d3570a4..0d2a1b1 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -85,9 +85,18 @@ struct libinput_device_config_scroll { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_rotation { + int (*increment)(struct libinput_device *device); + enum libinput_config_status (*set)(struct libinput_device *device, +int degrees_cw); + int (*get)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; + struct libinput_device_config_rotation *rotation; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index b2388e6..2572f5b 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1250,3 +1250,36 @@ libinput_device_config_scroll_reset(struct libinput_device *device) if (device-config.scroll) device-config.scroll-reset(device); } + +LIBINPUT_EXPORT int +libinput_device_config_rotation_get_increment(struct libinput_device *device) +{ + return device-config.rotation ? + device-config.rotation-increment(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_rotation_set(struct libinput_device *device, + int degrees_cw) +{ + if (libinput_device_config_rotation_get_increment(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.rotation-set(device, degrees_cw); +} + +LIBINPUT_EXPORT int +libinput_device_config_rotation_get(struct libinput_device *device) +{ + if (libinput_device_config_rotation_get_increment(device) == 0) + return 0; + + return device-config.rotation-get(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_rotation_reset(struct libinput_device *device) +{ + if (libinput_device_config_rotation_get_increment(device) != 0) + device-config.rotation-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 571f7ba..328d050 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1508,6 +1508,75 @@ libinput_device_config_scroll_get_method(struct libinput_device *device); void libinput_device_config_scroll_reset(struct libinput_device *device); + +/** + * @ingroup config + * + * Query the rotation increment for this device, if any. The return value is + * the increment in degrees. For example, a device that returns 90 may only + * be rotated in 90-degree increments. + * + * @param device The device to configure + * + * @return The rotation increment in degrees, or 0 if the device cannot be + * rotated + */ +int +libinput_device_config_rotation_get_increment(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the rotation for this device, in degrees clockwise. This rotation + * applies to the physical orientation of the device, i.e. if a tablet is + * moved from landscape to portrait format, clockwise, this represents a + * 90-degree rotation. In the diagram below, if a is the device origin 0/0, + * after the rotation the coordinate b sends 0/0 coordinates and a sends + * xmax/0. + * + * @code + * +-++-+ + * |a||b a| + * | | - | | + * |b|| | + * +-+| | + * | | + * +-+ + * @endcode + * + * @param device The device to configure + * @param degrees_cw The number of degrees to rotate clockwise + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_rotation_set(struct libinput_device *device, + int degrees_cw); + +/** + * @ingroup config + * + * Get the current rotation for this device, in degrees clockwise. If the + * device does not support rotation, this function always returns 0. + * + * @param device The device to configure + * + * @return The rotation in degrees clockwise + */ +int +libinput_device_config_rotation_get(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset the rotation to the device's default setting. If thd evice does not + * support rotation, this function does nothing. + * + * @param device The device to configure + */ +void
Re: [PATCH libinput 07/10] Add a basic pointer acceleration API
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: Only exposes two knobs - speed and precision which have historically been the only two knobs exposed anyway on most UIs. We could go for something fancier but really, I think this will be enough. The only open question is whether speed will be enough for high-dpi devices. I would like to see an implementation of this before adding this API. Regards, Hans Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 12 src/libinput.c | 53 + src/libinput.h | 80 ++ 3 files changed, 145 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 0d2a1b1..85113bd 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -93,10 +93,22 @@ struct libinput_device_config_rotation { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_accel { + int (*available)(struct libinput_device *device); + enum libinput_config_status (*set_speed)(struct libinput_device *device, + unsigned int speed); + enum libinput_config_status (*set_precision)(struct libinput_device *device, + unsigned int precision); + int (*get_speed)(struct libinput_device *device); + int (*get_precision)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; struct libinput_device_config_rotation *rotation; + struct libinput_device_config_accel *accel; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 2572f5b..5a068f1 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1283,3 +1283,56 @@ libinput_device_config_rotation_reset(struct libinput_device *device) if (libinput_device_config_rotation_get_increment(device) != 0) device-config.rotation-reset(device); } + + +LIBINPUT_EXPORT int +libinput_device_config_accel_is_available(struct libinput_device *device) +{ + return device-config.accel ? + device-config.accel-available(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_accel_set_speed(struct libinput_device *device, +unsigned int speed) +{ + if (!libinput_device_config_accel_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.accel-set_speed(device, speed); +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_accel_set_precision(struct libinput_device *device, +unsigned int precision) +{ + if (!libinput_device_config_accel_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.accel-set_precision(device, precision); +} + +LIBINPUT_EXPORT unsigned int +libinput_device_config_accel_get_speed(struct libinput_device *device) +{ + if (!libinput_device_config_accel_is_available(device)) + return 0; + + return device-config.accel-get_speed(device); +} + +LIBINPUT_EXPORT unsigned int +libinput_device_config_accel_get_precision(struct libinput_device *device) +{ + if (!libinput_device_config_accel_is_available(device)) + return 0; + + return device-config.accel-get_precision(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_accel_reset(struct libinput_device *device) +{ + if (device-config.accel) + device-config.accel-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 328d050..1b6207c 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1577,6 +1577,86 @@ libinput_device_config_rotation_get(struct libinput_device *device); void libinput_device_config_rotation_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Check if a device uses libinput-internal pointer-acceleration. + * + * @param device The device to configure + * + * @return 0 if the device is not accelerated, nonzero if it is accelerated + */ +int +libinput_device_config_accel_is_available(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the speed of this pointer device, where 0% is the minimum pointer + * acceleration to be applied (none or slowed down, depending on the device) + * and 100% is the maximum amount of acceleration to be applied. + * + * @param device The device to configure + * @param speed The abstract speed identifier, ranged 0% to 100%. + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_accel_set_speed(struct
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
On Wed, Jun 04, 2014 at 10:38:19AM +0200, Hans de Goede wrote: Hi, On 06/04/2014 10:21 AM, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 11:17:58AM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 10:41:16PM +0200, Jonas Ådahl wrote: On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { +int (*count)(struct libinput_device *device); +enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); +int (*is_enabled)(struct libinput_device *device); +void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { +struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; +struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ +return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable) +{ +if (enable +libinput_device_config_tap_get_finger_count(device) == 0) +return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; Shouldn't this be returned also if !enable? It would be a bit confusing if disabling is allowed, but enabling is not. This was a conscious decision that applies to the other functions as well: if you're doing something that's not technically invalid, we're not reporting an error. This means that an error only needs to be handled if something really is off. I'd argue that disabling tapping on a device that doesn't support tapping is a technically invalid operation that is simply handled in the cases where it makes no sense. I'm with Peter here, that its easier for our API consumers if they caan always safely call disable. Who would want to disable tapping on a device that doesn't support it? Anyway, seems its 2-1 on this one so I'll stop now. Also, in what circumstances will config.tap be set, but finger count returning 0? What type of devices would have tapping configuration enabled with no tapping possible? I expect none, this approach was just the safest and most copy-paste-proof way of checking. + +return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ +if (libinput_device_config_tap_get_finger_count(device) == 0) +return 0; + +return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ +if (device-config.tap) +device-config.tap-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. Should it be libinput_device_config_tap_enable() here instead? ah, thanks. one always escapes the refacturing... + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see
Re: [PATCH libinput 09/10] Add config API for pointer modes
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 9 ++ src/libinput.c | 37 src/libinput.h | 77 ++ 3 files changed, 123 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 9a3e629..0de 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -112,12 +112,21 @@ struct libinput_device_config_disable_while_typing { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_pointer_mode { + int (*modes)(struct libinput_device *device); + enum libinput_config_status (*set)(struct libinput_device *device, +enum libinput_device_pointer_mode); + enum libinput_device_pointer_mode (*get)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; struct libinput_device_config_rotation *rotation; struct libinput_device_config_accel *accel; struct libinput_device_config_disable_while_typing *dwt; + struct libinput_device_config_pointer_mode *mode; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 33a8e90..5324407 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1369,3 +1369,40 @@ libinput_device_config_disable_while_typing_reset(struct libinput_device *device if (device-config.dwt) device-config.dwt-reset(device); } + +LIBINPUT_EXPORT unsigned int +libinput_device_config_pointer_mode_get_modes(struct libinput_device *device) +{ + return device-config.mode ? + device-config.mode-modes(device) : + LIBINPUT_POINTER_MODE_NATIVE_ONLY; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_pointer_mode_set_mode(struct libinput_device *device, + enum libinput_device_pointer_mode mode) +{ + if (libinput_device_config_pointer_mode_get_modes(device) == + LIBINPUT_POINTER_MODE_NATIVE_ONLY + mode != LIBINPUT_POINTER_MODE_NATIVE_ONLY) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.mode-set(device, mode); +} + +LIBINPUT_EXPORT enum libinput_device_pointer_mode +libinput_device_config_pointer_mode_get_mode(struct libinput_device *device) +{ + if (libinput_device_config_pointer_mode_get_modes(device) == + LIBINPUT_POINTER_MODE_NATIVE_ONLY) + return LIBINPUT_POINTER_MODE_NATIVE_ONLY; + + return device-config.mode-get(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_pointer_mode_reset(struct libinput_device *device) +{ + if (device-config.mode) + device-config.mode-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 62d0c0f..1a51b82 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1713,6 +1713,83 @@ libinput_device_config_disable_while_typing_is_enabled(struct libinput_device *d void libinput_device_config_disable_while_typing_reset(struct libinput_device *device); +/** + * @ingroup config + */ +enum libinput_device_pointer_mode { + /** + * The device only works in native mode and does not support mode + * switching. Native mode may be absolute or relative, depending on + * the device. + */ + LIBINPUT_POINTER_MODE_NATIVE_ONLY = 0, + /** + * The device behaves like an absolute input device, e.g. like a + * touchscreen. + */ + LIBINPUT_POINTER_MODE_ABSOLUTE = (1 0), + /** + * The device behaves like a relative input device, e.g. like a + * touchpad. + */ + LIBINPUT_POINTER_MODE_RELATIVE = (1 1), +}; + +/** + * @ingroup config + * + * Get the supported device modes for this device. Absolute pointer devices + * such as graphics tablet may be used in absolute mode or relative mode. + * + * @note A device that supports relative and absolute mode may be + * pointer-accelerated in relative mode. How will these interact, what will the available method of the pointer accel return when in absolute mode ? What will the set / get methods return ? Regards, Hans + * + * @param device The device to configure + * + * @return A bitmask of the available pointer modes, or + * POINTER_MODE_NATIVE_ONLY if the device does not allow mode switching + */ +unsigned int +libinput_device_config_pointer_mode_get_modes(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the pointer mode for this device. + * + * @param device The device to configure + * @param mode The pointer mode to switch the device to + * + * @return A config
Re: [PATCH libinput 10/10] Add config api for middle button emulation
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: --- src/libinput-private.h | 8 src/libinput.c | 33 ++ src/libinput.h | 54 ++ 3 files changed, 95 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 0de..28f071a 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -120,6 +120,13 @@ struct libinput_device_config_pointer_mode { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_middlebutton_emulation { + int (*available)(struct libinput_device *device); + int (*enable)(struct libinput_device *device, int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + Again set / get / get_default ? Other then that this looks good. Regards, Hans struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; @@ -127,6 +134,7 @@ struct libinput_device_config { struct libinput_device_config_accel *accel; struct libinput_device_config_disable_while_typing *dwt; struct libinput_device_config_pointer_mode *mode; + struct libinput_device_config_middlebutton_emulation *mbemu; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 5324407..bd06960 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1406,3 +1406,36 @@ libinput_device_config_pointer_mode_reset(struct libinput_device *device) if (device-config.mode) device-config.mode-reset(device); } + +LIBINPUT_EXPORT int +libinput_device_config_middlebutton_emulation_is_available(struct libinput_device *device) +{ + return device-config.mbemu ? + device-config.mbemu-available(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_middlebutton_emulation_enable(struct libinput_device *device, + int enable) +{ + if (!libinput_device_config_middlebutton_emulation_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.mbemu-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_middlebutton_emulation_is_enabled(struct libinput_device *device) +{ + if (!libinput_device_config_middlebutton_emulation_is_available(device)) + return 0; + + return device-config.mbemu-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_middlebutton_emulation_reset(struct libinput_device *device) +{ + if (device-config.mbemu) + device-config.mbemu-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 1a51b82..b78283f 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1789,6 +1789,60 @@ libinput_device_config_pointer_mode_get_mode(struct libinput_device *device); void libinput_device_config_pointer_mode_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Devices without a physical middle button may provide middle-button + * emulation by pressing the left and the right button simultaneously. + * + * @param device The device to configure + * + * @return 1 if available, 0 if not available + */ +int +libinput_device_config_middlebutton_emulation_is_available(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable or disable middle button emulation on this device. Note that + * enabling middle button emulation causes a delay in the delivery of button + * events. + * + * @param device The device to configure + * @param enable 1 to enable, 0 to disable + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_middlebutton_emulation_enable(struct libinput_device *device, + int enable); + +/** + * @ingroup config + * + * Check if middle button emulation is enabled on this device. If the device + * does not support middle button emulation, this function returns 0. + * + * @param device The device to configure + * + * @return 0 if disabled, 1 if enabled + */ +int +libinput_device_config_middlebutton_emulation_is_enabled(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset to the default emulation status. If the device does not support + * middle button emulation, this function does nothing. + * + * @param device The device to configure + * + */ +void +libinput_device_config_middlebutton_emulation_reset(struct libinput_device *device); + #ifdef __cplusplus } ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] touchpad: Drop the scroll direction lock, increase threshold instead
Hi, On 06/04/2014 06:09 AM, Peter Hutterer wrote: The direction lock was intended to avoid erroneous horizontal scroll events when scrolling vertically (and vice versa). Some testing on my touchpad here shows that it is too easy to accidentally lock the direction when no lock is intended (e.g. moving around an image). And quite hard to figure out what a pure vertical gesture is. I get movements from 90 degrees to 70 degrees for something my brain would consider vertical scrolling. Depending on the hand position, the fingers actually perform a slight curve, not a straight line. Hence - drop the direction lock, but increase the threshold a little. It doesn't totally avoid horizontal scroll events but keeps them minimal. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Ok, I give up. I thought I could find a simple way of locking the scroll directions but I ran out of time and the simplest approach didn't work that greatly. This patch replaces 5 and 6 of this series (the ones I never pushed) Thanks, looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans src/evdev-mt-touchpad.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 26d5f7d..466cf5e 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -463,21 +463,18 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) tp_filter_motion(tp, dx, dy, time); - if (tp-scroll.state == SCROLL_STATE_NONE) { - /* Require at least one px scrolling to start */ - if (dx = -1.0 || dx = 1.0) { - tp-scroll.state = SCROLL_STATE_SCROLLING; - tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); - } - - if (dy = -1.0 || dy = 1.0) { - tp-scroll.state = SCROLL_STATE_SCROLLING; - tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL); - } - - if (tp-scroll.state == SCROLL_STATE_NONE) - return; + /* Require at least three px scrolling to start */ + if (dy = -3.0 || dy = 3.0) { + tp-scroll.state = SCROLL_STATE_SCROLLING; + tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL); } + if (dx = -3.0 || dx = 3.0) { + tp-scroll.state = SCROLL_STATE_SCROLLING; + tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); + } + + if (tp-scroll.state == SCROLL_STATE_NONE) + return; /* Stop spurious MOTION events at the end of scrolling */ tp_for_each_touch(tp, t) ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 08/10] Add a config API for disable-while-typing
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: --- src/libinput-private.h | 9 src/libinput.c | 33 + src/libinput.h | 56 ++ 3 files changed, 98 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 85113bd..9a3e629 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -104,11 +104,20 @@ struct libinput_device_config_accel { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_disable_while_typing { + int (*available)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + set / get / get_default ? Regards, Hans struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; struct libinput_device_config_rotation *rotation; struct libinput_device_config_accel *accel; + struct libinput_device_config_disable_while_typing *dwt; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 5a068f1..33a8e90 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1336,3 +1336,36 @@ libinput_device_config_accel_reset(struct libinput_device *device) if (device-config.accel) device-config.accel-reset(device); } + +LIBINPUT_EXPORT int +libinput_device_config_disable_while_typing_is_available(struct libinput_device *device) +{ + return device-config.dwt ? + device-config.dwt-available(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_disable_while_typing_enable(struct libinput_device *device, +int enable) +{ + if (!libinput_device_config_disable_while_typing_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.dwt-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_disable_while_typing_is_enabled(struct libinput_device *device) +{ + if (!libinput_device_config_disable_while_typing_is_available(device)) + return 0; + + return device-config.dwt-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_disable_while_typing_reset(struct libinput_device *device) +{ + if (device-config.dwt) + device-config.dwt-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 1b6207c..62d0c0f 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1657,6 +1657,62 @@ libinput_device_config_accel_get_precision(struct libinput_device *device); void libinput_device_config_accel_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Check if this device supports a disable-while-typing feature. This + * feature is usually available on built-in touchpads where hand placement + * may cause erroneous events on the touchpad while typing. + * + * This feature is available on the device that is being disabled (i.e. the + * touchpad), not on the device causing the device to be disabled. Which + * devices trigger this feature is implementation-dependent. + * + * @param device The device to configure + * @return non-zero if disable while typing is available for this device + */ +int +libinput_device_config_disable_while_typing_is_available(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable or disable the disable-while-typing feature. When enabled, the + * device will not send events while and shortly after another device + * generates events. + * + * @param device The device to configure + * @param enable 0 to disable, 1 to enable + * + * @return 0 on success or a negative errno on failure + * @retval A config status code + */ +enum libinput_config_status +libinput_device_config_disable_while_typing_enable(struct libinput_device *device, +int enable); + +/** + * @ingroup config + * + * Check if the feature is currently enabled. + * + * @param device The device to configure + * + * @return 0 if the feature is disabled, non-zero if the feature is enabled + */ +int +libinput_device_config_disable_while_typing_is_enabled(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset to the default settings. + * + * @param device The device to configure + */ +void +libinput_device_config_disable_while_typing_reset(struct libinput_device *device); + #ifdef __cplusplus } #endif ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org
[PATCH] shell: Destroy the shell_surface when the xdg_shell interface gets destroyed
When running gtk3-demo under weston opening comboboxes a second time causes the program to bail due to weston returning an error. The relevant client trace in this case is: - xdg_shell@15.get_xdg_popup(new id xdg_popup@12, wl_surface@28, - xdg_popup@12.destroy() - xdg_shell@15.get_xdg_popup(new id xdg_popup@19, wl_surface@28, wl_display@1.error(wl_surface@28, 0, xdg_shell::get_xdg_popup already requested) However, according to the xdg_shell protocol this should be valid as calling destroy should remove the xdg_popup interface from the wl_surface object. Fix this by ensuring the internal shell surface also gets destroyed when the relevant xdg interface is destroyed and not just when the underlying wl_surface gets destroyed. This patch applies both to Weston 1.5 and master, please apply to both if correct. --- desktop-shell/shell.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 46c6e18..45a3321 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -3153,6 +3153,7 @@ destroy_shell_surface(struct shell_surface *shsurf) /* As destroy_resource() use wl_list_for_each_safe(), * we can always remove the listener. */ + wl_list_remove(shsurf-resource_destroy_listener.link); wl_list_remove(shsurf-surface_destroy_listener.link); shsurf-surface-configure = NULL; free(shsurf-title); @@ -3175,6 +3176,7 @@ shell_destroy_shell_surface(struct wl_resource *resource) if (!wl_list_empty(shsurf-popup.grab_link)) remove_popup_grab(shsurf); shsurf-resource = NULL; + destroy_shell_surface(shsurf); } static void -- 2.0.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 3/3] evdev-mt-touchpad-tap: Switch over to new timer subsystem
Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-tap.c | 76 ++--- src/evdev-mt-touchpad.c | 1 - src/evdev-mt-touchpad.h | 7 + 3 files changed, 10 insertions(+), 74 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index 2b0d21c..588570c 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -30,9 +30,7 @@ #include stdbool.h #include stdio.h #include string.h -#include time.h #include unistd.h -#include sys/timerfd.h #include evdev-mt-touchpad.h @@ -120,22 +118,13 @@ tp_tap_notify(struct tp_dispatch *tp, static void tp_tap_set_timer(struct tp_dispatch *tp, uint64_t time) { - uint64_t timeout = time + DEFAULT_TAP_TIMEOUT_PERIOD; - struct itimerspec its; - - its.it_interval.tv_sec = 0; - its.it_interval.tv_nsec = 0; - its.it_value.tv_sec = timeout / 1000; - its.it_value.tv_nsec = (timeout % 1000) * 1000 * 1000; - timerfd_settime(tp-tap.timer_fd, TFD_TIMER_ABSTIME, its, NULL); - - tp-tap.timeout = timeout; + libinput_timer_set(tp-tap.timer, time + DEFAULT_TAP_TIMEOUT_PERIOD); } static void tp_tap_clear_timer(struct tp_dispatch *tp) { - tp-tap.timeout = 0; + libinput_timer_cancel(tp-tap.timer); } static void @@ -548,60 +537,21 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time) } static void -tp_tap_timeout_handler(void *data) +tp_tap_handle_timeout(uint64_t time, void *data) { - struct tp_dispatch *touchpad = data; - uint64_t expires; - int len; - struct timespec ts; - uint64_t now; - - len = read(touchpad-tap.timer_fd, expires, sizeof expires); - if (len != sizeof expires) - /* This will only happen if the application made the fd -* non-blocking, but this function should only be called -* upon the timeout, so lets continue anyway. */ - log_error(timerfd read error: %s\n, strerror(errno)); - - clock_gettime(CLOCK_MONOTONIC, ts); - now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; - - tp_tap_handle_timeout(touchpad, now); -} - -unsigned int -tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) -{ - if (!tp-tap.enabled) - return 0; - - if (tp-tap.timeout tp-tap.timeout = time) { - tp_tap_clear_timer(tp); - tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); - } + struct tp_dispatch *tp = data; - return tp-tap.timeout; + tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); } int tp_init_tap(struct tp_dispatch *tp) { tp-tap.state = TAP_STATE_IDLE; - tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); - if (tp-tap.timer_fd == -1) - return -1; - - tp-tap.source = - libinput_add_fd(tp-device-base.seat-libinput, - tp-tap.timer_fd, - tp_tap_timeout_handler, - tp); - - if (tp-tap.source == NULL) { - close(tp-tap.timer_fd); - return -1; - } + libinput_timer_init(tp-tap.timer, + tp-device-base.seat-libinput, + tp_tap_handle_timeout, tp); tp-tap.enabled = 1; /* FIXME */ @@ -611,13 +561,5 @@ tp_init_tap(struct tp_dispatch *tp) void tp_destroy_tap(struct tp_dispatch *tp) { - if (tp-tap.source) { - libinput_remove_source(tp-device-base.seat-libinput, - tp-tap.source); - tp-tap.source = NULL; - } - if (tp-tap.timer_fd -1) { - close(tp-tap.timer_fd); - tp-tap.timer_fd = -1; - } + libinput_timer_cancel(tp-tap.timer); } diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index ba02122..d749a29 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -754,7 +754,6 @@ tp_init(struct tp_dispatch *tp, tp-base.interface = tp_interface; tp-device = device; - tp-tap.timer_fd = -1; if (tp_init_slots(tp, device) != 0) return -1; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 1749a55..0b1457d 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -200,9 +200,7 @@ struct tp_dispatch { struct { bool enabled; - int timer_fd; - struct libinput_source *source; - unsigned int timeout; + struct libinput_timer timer; enum tp_tap_state state; } tap; }; @@ -219,9 +217,6 @@ tp_set_pointer(struct tp_dispatch *tp, struct tp_touch *t); int tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time); -unsigned int -tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time); - int tp_init_tap(struct
Wayland and Android Emulator
Hello, I am running 2 operating systems in paralel on a Panda Board, Android and Linux with Weston on it. Since Android doesn't have access to the GPU (it's one OS or the other) I created proxy libs for EGL, GLESv1 and GLESv2 + gralloc(emulator part). So far all I was able to see on linux side was a colorful white noise rendered on wayland window. I am unable to find any issues in the emulator gralloc code or in my proxy libs. If I run a simple examples(using pvrShell on client plus my proxies) I get everything drawn ok. Are there any incompatibilities between pbuffers and wayland(I read some posts but did't answer my questions)? Is it a good approach to get emulator code and hack it to run on Wayland or I can just ignore gralloc and just render on a wayland surface directly? Any ideas appreciated... Thank you!___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] s/libinput_pointer_button_state/libinput_button_state/
On Wed, Jun 04, 2014 at 12:54:58PM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 08:08:02PM -0400, Stephen Chandler Paul wrote: Button states are applicable to more then just the pointer, so having a non-generic name name for a generic enumerator value like libinput_pointer_button_state doesn't make sense. Changing it to something generic like libinput_button_state allows it to be reused by other devices that may potentially be added to libinput in the future. Signed-off-by: Stephen Chandler Paul thatsly...@gmail.com this looks good to me, and saves us from introducing a separate but same enum for the tablet buttons. Looks good to me too, but would like to wait with pushing until I have patches to avoid breaking at least weston and GNOME builders. Jonas Reviewed-by: Peter Hutterer peter.hutte...@who-t.net Cheers, Peter --- src/evdev-mt-touchpad-buttons.c | 18 +- src/evdev-mt-touchpad-tap.c | 34 +- src/evdev.c | 4 ++-- src/libinput-private.h | 2 +- src/libinput.c | 12 ++-- src/libinput.h | 8 test/pointer.c | 8 test/touchpad.c | 30 +++--- tools/event-debug.c | 4 ++-- 9 files changed, 60 insertions(+), 60 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index faf04b7..7680334 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -687,7 +687,7 @@ static int tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; - enum libinput_pointer_button_state state; + enum libinput_button_state state; current = tp-buttons.state; old = tp-buttons.old_state; @@ -704,11 +704,11 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) return 0; } tp-buttons.active = button; - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; } else { button = tp-buttons.active; tp-buttons.active = 0; - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; } if (button) @@ -729,13 +729,13 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) button = BTN_LEFT; while (current || old) { - enum libinput_pointer_button_state state; + enum libinput_button_state state; if ((current 0x1) ^ (old 0x1)) { if (!!(current 0x1)) - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; else - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; pointer_notify_button(tp-device-base, time, @@ -755,7 +755,7 @@ static int tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; - enum libinput_pointer_button_state state; + enum libinput_button_state state; enum { AREA = 0x01, LEFT = 0x02, MIDDLE = 0x04, RIGHT = 0x08 }; current = tp-buttons.state; @@ -803,11 +803,11 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) button = BTN_LEFT; tp-buttons.active = button; - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; } else { button = tp-buttons.active; tp-buttons.active = 0; - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; } tp-buttons.click_pending = false; diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index eee334f..69829bb 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -99,7 +99,7 @@ static void tp_tap_notify(struct tp_dispatch *tp, uint64_t time, int nfingers, - enum libinput_pointer_button_state state) + enum libinput_button_state state) { int32_t button; @@ -170,7 +170,7 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t break; case TAP_EVENT_RELEASE: tp-tap.state = TAP_STATE_TAPPED; - tp_tap_notify(tp, time, 1, LIBINPUT_POINTER_BUTTON_STATE_PRESSED); + tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_PRESSED); tp_tap_set_timer(tp, time);
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote: Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Now that we have run-time changes of the tap.enabled state move the check to the IDLE state only. Otherwise the tap machine may hang if tapping is disabled while a gesture is in progress. Two basic tests are added to check for the tap default setting - which is now tap disabled by default, because a bike shed's correct color is green. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-tap.c | 56 +++-- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 32 ++ 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index eee334f..24ea8c3 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -438,13 +438,13 @@ static void tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) { enum tp_tap_state current; - if (!tp-tap.enabled) - return; current = tp-tap.state; switch(tp-tap.state) { case TAP_STATE_IDLE: + if (!tp-tap.enabled) + break; tp_tap_idle_handle_event(tp, event, time); break; case TAP_STATE_TOUCH: @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data) unsigned int tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) { - if (!tp-tap.enabled) - return 0; - if (tp-tap.timeout tp-tap.timeout = time) { tp_tap_clear_timer(tp); tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) return tp-tap.timeout; } +static int +tp_tap_config_count(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return min(tp-ntouches, 3); /* we only do up to 3 finger tap */ +} + +static enum libinput_config_status +tp_tap_config_enable(struct libinput_device *device, int enabled) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; Please user container_of here for proper type checking rather then just a blatant cast of $random_type_foo to $random_type_bar. Same for all the other occurrences of the same pattern below. Would just like to point out that there are several places this cast is already in place, and starting to use container_of would introduce inconsistencies. I think this kind of casting isn't necessarily bad for single (or primary) inheritance types of objects. Jonas + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + if (tp_tap_config_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; This is a repeat of the test done by the higher level wrapper, as such this seems unnecessary by me. Regards, Hans + + tp-tap.enabled = enabled; + + return LIBINPUT_CONFIG_STATUS_SUCCESS; +} + +static int +tp_tap_config_is_enabled(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return tp-tap.enabled; +} + +static void +tp_tap_config_reset(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + tp-tap.enabled = false; +} + int tp_init_tap(struct tp_dispatch *tp) { + tp-tap.config.count = tp_tap_config_count; + tp-tap.config.enable = tp_tap_config_enable; + tp-tap.config.is_enabled = tp_tap_config_is_enabled; + tp-tap.config.reset = tp_tap_config_reset; + tp-device-base.config.tap = tp-tap.config; + tp-tap.state = TAP_STATE_IDLE; tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); @@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp) return -1; } - tp-tap.enabled = 1; /* FIXME */ - return 0; } diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index d514ed6..0b31e9a 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -203,6 +203,7 @@ struct tp_dispatch { struct { bool enabled; + struct libinput_device_config_tap config; int timer_fd; struct libinput_source *source; unsigned int timeout; diff --git a/test/touchpad.c b/test/touchpad.c index 35781c3..060b529 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap)
Re: [PATCH] s/libinput_pointer_button_state/libinput_button_state/
On Wed, Jun 04, 2014 at 10:40:10PM +0200, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 12:54:58PM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 08:08:02PM -0400, Stephen Chandler Paul wrote: Button states are applicable to more then just the pointer, so having a non-generic name name for a generic enumerator value like libinput_pointer_button_state doesn't make sense. Changing it to something generic like libinput_button_state allows it to be reused by other devices that may potentially be added to libinput in the future. Signed-off-by: Stephen Chandler Paul thatsly...@gmail.com this looks good to me, and saves us from introducing a separate but same enum for the tablet buttons. Looks good to me too, but would like to wait with pushing until I have patches to avoid breaking at least weston and GNOME builders. can you take this one into your tree and push it together with the fixed - double changes? Then do a 0.3 release, that reduces the pain for everyone involved. Cheers, Peter Reviewed-by: Peter Hutterer peter.hutte...@who-t.net Cheers, Peter --- src/evdev-mt-touchpad-buttons.c | 18 +- src/evdev-mt-touchpad-tap.c | 34 +- src/evdev.c | 4 ++-- src/libinput-private.h | 2 +- src/libinput.c | 12 ++-- src/libinput.h | 8 test/pointer.c | 8 test/touchpad.c | 30 +++--- tools/event-debug.c | 4 ++-- 9 files changed, 60 insertions(+), 60 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index faf04b7..7680334 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -687,7 +687,7 @@ static int tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; - enum libinput_pointer_button_state state; + enum libinput_button_state state; current = tp-buttons.state; old = tp-buttons.old_state; @@ -704,11 +704,11 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) return 0; } tp-buttons.active = button; - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; } else { button = tp-buttons.active; tp-buttons.active = 0; - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; } if (button) @@ -729,13 +729,13 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) button = BTN_LEFT; while (current || old) { - enum libinput_pointer_button_state state; + enum libinput_button_state state; if ((current 0x1) ^ (old 0x1)) { if (!!(current 0x1)) - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; else - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; pointer_notify_button(tp-device-base, time, @@ -755,7 +755,7 @@ static int tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; - enum libinput_pointer_button_state state; + enum libinput_button_state state; enum { AREA = 0x01, LEFT = 0x02, MIDDLE = 0x04, RIGHT = 0x08 }; current = tp-buttons.state; @@ -803,11 +803,11 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) button = BTN_LEFT; tp-buttons.active = button; - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; } else { button = tp-buttons.active; tp-buttons.active = 0; - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; } tp-buttons.click_pending = false; diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index eee334f..69829bb 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -99,7 +99,7 @@ static void tp_tap_notify(struct tp_dispatch *tp, uint64_t time, int nfingers, - enum libinput_pointer_button_state state) + enum libinput_button_state state) { int32_t button; @@ -170,7 +170,7 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t break; case TAP_EVENT_RELEASE:
Re: [PATCH libinput 1/3] Add a timer subsystem
On Wed, Jun 04, 2014 at 05:26:34PM +0200, Hans de Goede wrote: Currently we are using DIY timers in the touchpad softbutton and tap handling code, and at least the softbutton code gets its wrong. It uses one timer-fd per touchpad to set a timeout per touch, which means that if a timeout is set for 100ms from now for touch 1, and then a timeout gets set 50 ms later for 200 ms from now, then the timeout for touch 1 will come 150 ms too late. This commits adds a proper timer subsystem so that we've one place to deal with timer handling, and so that we can only get it wrong (well hopefully we get it right) in one place. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/Makefile.am| 2 + src/libinput-private.h | 9 ++- src/libinput.c | 11 +++- src/timer.c| 149 + src/timer.h| 56 +++ 5 files changed, 224 insertions(+), 3 deletions(-) create mode 100644 src/timer.c create mode 100644 src/timer.h diff --git a/src/Makefile.am b/src/Makefile.am index efb089a..8b56348 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -22,6 +22,8 @@ libinput_la_SOURCES = \ path.c \ udev-seat.c \ udev-seat.h \ + timer.c \ + timer.h \ ../include/linux/input.h libinput_la_LIBADD = $(MTDEV_LIBS) \ diff --git a/src/libinput-private.h b/src/libinput-private.h index 0e92e68..ed8190d 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -28,6 +28,8 @@ #include libinput.h #include libinput-util.h +struct libinput_source; + struct libinput_interface_backend { int (*resume)(struct libinput *libinput); void (*suspend)(struct libinput *libinput); @@ -40,6 +42,12 @@ struct libinput { struct list seat_list; + struct { + struct list list; + struct libinput_source *source; + int fd; + } timer; + struct libinput_event **events; size_t events_count; size_t events_len; @@ -79,7 +87,6 @@ struct libinput_device { typedef void (*libinput_source_dispatch_t)(void *data); -struct libinput_source; #define log_debug(...) log_msg(LIBINPUT_LOG_PRIORITY_DEBUG, __VA_ARGS__) #define log_info(...) log_msg(LIBINPUT_LOG_PRIORITY_INFO, __VA_ARGS__) diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..e74d29f 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -33,6 +33,7 @@ #include libinput.h #include libinput-private.h #include evdev.h +#include timer.h struct libinput_source { libinput_source_dispatch_t dispatch; @@ -485,6 +486,12 @@ libinput_init(struct libinput *libinput, list_init(libinput-source_destroy_list); list_init(libinput-seat_list); + if (libinput_timer_subsys_init(libinput) != 0) { + free(libinput-events); + close(libinput-epoll_fd); + return -1; + } + return 0; } @@ -521,8 +528,6 @@ libinput_destroy(struct libinput *libinput) while ((event = libinput_get_event(libinput))) libinput_event_destroy(event); - libinput_drop_destroyed_sources(libinput); - free(libinput-events); list_for_each_safe(seat, next_seat, libinput-seat_list, link) { @@ -534,6 +539,8 @@ libinput_destroy(struct libinput *libinput) libinput_seat_destroy(seat); } + libinput_timer_subsys_exit(libinput); + libinput_drop_destroyed_sources(libinput); close(libinput-epoll_fd); free(libinput); } diff --git a/src/timer.c b/src/timer.c new file mode 100644 index 000..349a6c9 --- /dev/null +++ b/src/timer.c @@ -0,0 +1,149 @@ +/* + * 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,
Re: [PATCH] s/libinput_pointer_button_state/libinput_button_state/
On Thu, Jun 05, 2014 at 07:45:00AM +1000, Peter Hutterer wrote: On Wed, Jun 04, 2014 at 10:40:10PM +0200, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 12:54:58PM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 08:08:02PM -0400, Stephen Chandler Paul wrote: Button states are applicable to more then just the pointer, so having a non-generic name name for a generic enumerator value like libinput_pointer_button_state doesn't make sense. Changing it to something generic like libinput_button_state allows it to be reused by other devices that may potentially be added to libinput in the future. Signed-off-by: Stephen Chandler Paul thatsly...@gmail.com this looks good to me, and saves us from introducing a separate but same enum for the tablet buttons. Looks good to me too, but would like to wait with pushing until I have patches to avoid breaking at least weston and GNOME builders. can you take this one into your tree and push it together with the fixed - double changes? Then do a 0.3 release, that reduces the pain for everyone involved. That has been my plan indeed. We could wait a short while with 0.3 and just bump the version to 0.2.99, if there are any other API breaks you can think of, otherwise a 0.3 sounds good. Jonas Cheers, Peter Reviewed-by: Peter Hutterer peter.hutte...@who-t.net Cheers, Peter --- src/evdev-mt-touchpad-buttons.c | 18 +- src/evdev-mt-touchpad-tap.c | 34 +- src/evdev.c | 4 ++-- src/libinput-private.h | 2 +- src/libinput.c | 12 ++-- src/libinput.h | 8 test/pointer.c | 8 test/touchpad.c | 30 +++--- tools/event-debug.c | 4 ++-- 9 files changed, 60 insertions(+), 60 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index faf04b7..7680334 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -687,7 +687,7 @@ static int tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; - enum libinput_pointer_button_state state; + enum libinput_button_state state; current = tp-buttons.state; old = tp-buttons.old_state; @@ -704,11 +704,11 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) return 0; } tp-buttons.active = button; - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; } else { button = tp-buttons.active; tp-buttons.active = 0; - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; } if (button) @@ -729,13 +729,13 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) button = BTN_LEFT; while (current || old) { - enum libinput_pointer_button_state state; + enum libinput_button_state state; if ((current 0x1) ^ (old 0x1)) { if (!!(current 0x1)) - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; else - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED; pointer_notify_button(tp-device-base, time, @@ -755,7 +755,7 @@ static int tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; - enum libinput_pointer_button_state state; + enum libinput_button_state state; enum { AREA = 0x01, LEFT = 0x02, MIDDLE = 0x04, RIGHT = 0x08 }; current = tp-buttons.state; @@ -803,11 +803,11 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) button = BTN_LEFT; tp-buttons.active = button; - state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + state = LIBINPUT_BUTTON_STATE_PRESSED; } else { button = tp-buttons.active; tp-buttons.active = 0; - state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + state = LIBINPUT_BUTTON_STATE_RELEASED;
Re: [PATCH] s/libinput_pointer_button_state/libinput_button_state/
On Thu, Jun 05, 2014 at 12:21:52AM +0200, Jonas Ådahl wrote: On Thu, Jun 05, 2014 at 07:45:00AM +1000, Peter Hutterer wrote: On Wed, Jun 04, 2014 at 10:40:10PM +0200, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 12:54:58PM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 08:08:02PM -0400, Stephen Chandler Paul wrote: Button states are applicable to more then just the pointer, so having a non-generic name name for a generic enumerator value like libinput_pointer_button_state doesn't make sense. Changing it to something generic like libinput_button_state allows it to be reused by other devices that may potentially be added to libinput in the future. Signed-off-by: Stephen Chandler Paul thatsly...@gmail.com this looks good to me, and saves us from introducing a separate but same enum for the tablet buttons. Looks good to me too, but would like to wait with pushing until I have patches to avoid breaking at least weston and GNOME builders. can you take this one into your tree and push it together with the fixed - double changes? Then do a 0.3 release, that reduces the pain for everyone involved. That has been my plan indeed. We could wait a short while with 0.3 and just bump the version to 0.2.99, if there are any other API breaks you can think of, otherwise a 0.3 sounds good. I don't have any API _breaks_ coming up, just additions, so I'd be fine with 0.3 directly. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Wed, Jun 04, 2014 at 10:57:36PM +0200, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote: On 06/03/2014 07:34 AM, Peter Hutterer wrote: [...] @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) return tp-tap.timeout; } +static int +tp_tap_config_count(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return min(tp-ntouches, 3); /* we only do up to 3 finger tap */ +} + +static enum libinput_config_status +tp_tap_config_enable(struct libinput_device *device, int enabled) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; Please user container_of here for proper type checking rather then just a blatant cast of $random_type_foo to $random_type_bar. Same for all the other occurrences of the same pattern below. Would just like to point out that there are several places this cast is already in place, and starting to use container_of would introduce inconsistencies. I think this kind of casting isn't necessarily bad for single (or primary) inheritance types of objects. I don't mind switching this to container_of, but I won't do this for this patch. I'll switch the whole of libinput over in one go instead. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/3] Add a timer subsystem
On Wed, Jun 04, 2014 at 05:26:34PM +0200, Hans de Goede wrote: Currently we are using DIY timers in the touchpad softbutton and tap handling code, and at least the softbutton code gets its wrong. It uses one timer-fd per touchpad to set a timeout per touch, which means that if a timeout is set for 100ms from now for touch 1, and then a timeout gets set 50 ms later for 200 ms from now, then the timeout for touch 1 will come 150 ms too late. This commits adds a proper timer subsystem so that we've one place to deal with timer handling, and so that we can only get it wrong (well hopefully we get it right) in one place. Signed-off-by: Hans de Goede hdego...@redhat.com --- [...] +static void +libinput_timer_handler(void *data) +{ + struct libinput *libinput = data; + struct libinput_timer *timer, *tmp; + struct timespec ts; + uint64_t now; + int r; + + r = clock_gettime(CLOCK_MONOTONIC, ts); + if (r) { + log_error(clock_gettime error: %s\n, strerror(errno)); + return; + } + + now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; + + list_for_each_safe(timer, tmp, libinput-timer.list, list) { + if (timer-expire = now) { + /* Clear the timer before calling timer_func, +as timer_func may re-arm it */ + libinput_timer_cancel(timer); + timer-timer_func(now, timer-timer_func_data); would it make sense to pass the timer itself into the timer func, for the use-cases where the func handles multiple timers and only needs to e.g. print and re-arm that timer? this is an internal API so we can add it when needed later. + } + } +} + +int +libinput_timer_subsys_init(struct libinput *libinput) +{ + libinput-timer.fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); + if (libinput-timer.fd 0) + return -1; + + list_init(libinput-timer.list); + + libinput-timer.source = libinput_add_fd(libinput, + libinput-timer.fd, + libinput_timer_handler, + libinput); + if (!libinput-timer.source) { + close(libinput-timer.fd); + return -1; + } + + return 0; +} + +void +libinput_timer_subsys_exit(struct libinput *libinput) +{ + /* All timer uses should have destroyed their timers now */ typo: users +struct libinput_timer { + struct libinput *libinput; + struct list list; + uint64_t expire; do me a favour and add a comment here and in libinput_timer_set that this is in abstime, not reltime. + void (*timer_func)(uint64_t now, void *timer_func_data); + void *timer_func_data; +}; series: Reviewed-by: Peter Hutterer peter.hutte...@who-t.net with Jonas' comments and the above addressed. two other things that you may or may not want to add because they help debugging but require some more effort: - put a log_bug_libinput() into libinput_timer_set if the timer is current time - put a log_bug_libinput() into libinput_timer_set if the timer is more than say 5s into the future. this is arbitrary but a good safety guard that can save on some debugging time. Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel