[PATCH v2 libinput 2/3] Add an interface to enable/disable tapping
jadahl at gmail.com
jadahl at gmail.com
Tue Jul 1 22:58:12 PDT 2014
On Wed, Jul 02, 2014 at 01:38:19PM +1000, Peter Hutterer wrote:
> On Tue, Jul 01, 2014 at 08:23:23PM +0200, jadahl at gmail.com wrote:
> > On Tue, Jul 01, 2014 at 03:56:20PM +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.hutterer at who-t.net>
> > > ---
> > > Changes to v1:
> > > - change to a simple enabled/disabled config
> > >
> > > src/libinput-private.h | 13 +++++++++
> > > src/libinput.c | 35 ++++++++++++++++++++++++
> > > src/libinput.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > > 3 files changed, 120 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/libinput-private.h b/src/libinput-private.h
> > > index 00901b4..23c5140 100644
> > > --- a/src/libinput-private.h
> > > +++ b/src/libinput-private.h
> > > @@ -81,12 +81,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 (*set_enabled)(struct libinput_device *device,
> > > + int enable);
> > > + int (*get_enabled)(struct libinput_device *device);
> > > + int (*get_default)(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 3888f43..58ea9d5 100644
> > > --- a/src/libinput.c
> > > +++ b/src/libinput.c
> > > @@ -1257,3 +1257,38 @@ libinput_config_status_to_str(enum libinput_config_status status)
> > >
> > > return str;
> > > }
> > > +
> > > +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_set_enabled(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->set_enabled(device, enable);
> > > +}
> > > +
> > > +LIBINPUT_EXPORT int
> > > +libinput_device_config_tap_get_enabled(struct libinput_device *device)
> > > +{
> > > + if (libinput_device_config_tap_get_finger_count(device) == 0)
> > > + return 0;
> > > +
> > > + return device->config.tap->get_enabled(device);
> > > +}
> > > +
> > > +LIBINPUT_EXPORT int
> > > +libinput_device_config_tap_get_default_enabled(struct libinput_device *device)
> > > +{
> > > + if (libinput_device_config_tap_get_finger_count(device) == 0)
> > > + return 0;
> > > +
> > > + return device->config.tap->get_default(device);
> > > +}
> > > diff --git a/src/libinput.h b/src/libinput.h
> > > index 3eaea91..8a1a9d3 100644
> > > --- a/src/libinput.h
> > > +++ b/src/libinput.h
> > > @@ -1394,8 +1394,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_enabled() 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_set_enabled
> > > + * @see libinput_device_config_tap_get_enabled
> > > + * @see libinput_device_config_tap_set_enabled_get_default
> > > + */
> > > +int
> > > +libinput_device_config_tap_get_finger_count(struct libinput_device *device);
> >
> > We discussed dropping the _config prefix of these types of feature
> > related information retrieval functions during the previous version. Did
> > you think anything more about that? The question is rather if a
> > configurable feature should only be exposed by its configuration
> > interface or if it should both have a general scope where details about
> > it can be retrieved, and a configuration scope, where details can be
> > configured.
> >
> > Anyhow, I still can't think of a use case where the user would care
> > about the features we are going to be exposing configuration interfaces
> > for in a context that is not directly related to configuring it, so I'm
> > not convinced myself its the best way to split them up like that.
>
> Yeah, I thought about it but I intentionally left the config prefix in,
> mainly for the expressiveness of a "device_config" prefix which leaves no
> questions on what the function does.
>
> Example in this case:
> libinput_device_tap_get_finger_count() looks like it returns the number of
> fingers currently tapping. Which coincidentally _could_ be a function called
> on a button event if we ever need to distinguish between tapping/clicking.
Fair enough. That ambiguity could be avoided by adding a _max_, but
indeed they will look less associated. Lets hope the above scenario wont
happen. With the nits below addressed, consider this
Reviewed-by: Jonas Ådahl <jadahl at gmail.com> .
>
> > > +/**
> > > + * @ingroup config
> > > + *
> > > + * Enable or disable 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_get_enabled
> > > + * @see libinput_device_config_tap_set_enabled_get_default
> > > + */
> > > +enum libinput_config_status
> > > +libinput_device_config_tap_set_enabled(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_set_enabled
> > > + * @see libinput_device_config_tap_set_enabledd_get_default
> >
> > s/enabledd/enabled/
>
> doh, thought I caught all of those...
>
> > > + */
> > > +int
> > > +libinput_device_config_tap_get_enabled(struct libinput_device *device);
> > > +
> > > +/**
> > > + * @ingroup config
> > > + *
> > > + * Return the default setting for whether tapping is enabled on this device.
> > > + *
> > > + * @param device The device to configure
> > > + * @return 1 if tapping is enabled by default, or 0 otherwise
> > > + *
> > > + * @see libinput_device_config_tap_get_finger_count
> > > + * @see libinput_device_config_tap_set_enabled
> > > + * @see libinput_device_config_tap_get_enabled
> > > + */
> > > +int
> > > +libinput_device_config_tap_get_default_enabled(struct libinput_device *device);
> >
> > Should this be _set_enabled_get_default here? Or did you mean
> > get_default_enabled on all document the references?
>
> the latter, thanks. Fixed locally.
>
> Cheers,
> Peter
>
More information about the wayland-devel
mailing list