Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration

2014-06-04 Thread Jonas Ådahl
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

2014-06-04 Thread Jonas Ådahl
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Peter Hutterer

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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Jonas Ådahl
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Sjoerd Simons
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

2014-06-04 Thread Hans de Goede
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

2014-06-04 Thread Claudiu Lupu
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/

2014-06-04 Thread Jonas Ådahl
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

2014-06-04 Thread Jonas Ådahl
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/

2014-06-04 Thread Peter Hutterer
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

2014-06-04 Thread Jonas Ådahl
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/

2014-06-04 Thread Jonas Ådahl
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/

2014-06-04 Thread Peter Hutterer
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

2014-06-04 Thread Peter Hutterer
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

2014-06-04 Thread Peter Hutterer
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