[RFC libinput 1/2] Add a "buttonset" interface for button-only devices

Peter Hutterer peter.hutterer at who-t.net
Thu Feb 12 23:42:08 PST 2015


On 13/02/2015 14:32 , Peter Hutterer wrote:
> On Thu, Feb 12, 2015 at 12:37:12PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 12-02-15 08:33, Peter Hutterer wrote:
>>> A generic interface for devices that provide buttons and axes, but don't
>>> control the pointer. This caters for the Pad part of Wacom graphics tablets
>>> but could eventually also deal with remote controls, 3D mice and other
>>> devices.
>>>
>>> This patch adds a new interface "buttonset" with a new capability and two
>>> events AXIS and BUTTON. The interface is largely modelled after the
>>> tablet interface, but with different axes and a few different behaviors.
>>> The button events do the obvious, axis events provide 5 hooks:
>>>
>>> libinput_event_buttonset_get_axis_value() -> axis value in physical dimensions
>>> libinput_event_buttonset_get_axis_value_transformed()
>>> libinput_event_buttonset_get_axis_delta() -> axis delta in physical dimensions
>>> libinput_event_buttonset_get_axis_delta_transformed()
>>> libinput_event_buttonset_get_axis_delta_discrete() -> axis delta in clicks
>>>
>>> The latter is similar after the scroll click counts.
>>>
>>> Currently implemented are two ring and two strip axes. The axis use depends a
>>> lot on the device and is hard to predict. e.g. strips have apparently been
>>> used for physical positioning (like a sliding actuator) whereas the wheel
>>> usually only cares about deltas. The above set should cater for the common
>>> use-cases.
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>
>> <snip>
>>
>>> diff --git a/src/libinput.h b/src/libinput.h
>>> index a8146c0..d777730 100644
>>> --- a/src/libinput.h
>>> +++ b/src/libinput.h
>>> @@ -55,7 +55,8 @@ enum libinput_device_capability {
>>>  	LIBINPUT_DEVICE_CAP_KEYBOARD = 0,
>>>  	LIBINPUT_DEVICE_CAP_POINTER = 1,
>>>  	LIBINPUT_DEVICE_CAP_TOUCH = 2,
>>> -	LIBINPUT_DEVICE_CAP_TABLET = 3
>>> +	LIBINPUT_DEVICE_CAP_TABLET = 3,
>>> +	LIBINPUT_DEVICE_CAP_BUTTONSET = 4
>>>  };
>>>
>>>  /**
>>> @@ -148,6 +149,34 @@ enum libinput_tablet_axis {
>>>  /**
>>>   * @ingroup device
>>>   *
>>> + * Available axis types for a buttonset device. It must have the @ref
>>> + * LIBINPUT_DEVICE_CAP_BUTTONSET capability.
>>> + */
>>> +enum libinput_buttonset_axis {
>>> +	LIBINPUT_BUTTONSET_AXIS_RING = 1,
>>> +	LIBINPUT_BUTTONSET_AXIS_RING2 = 2,
>>> +	LIBINPUT_BUTTONSET_AXIS_STRIP = 3,
>>> +	LIBINPUT_BUTTONSET_AXIS_STRIP2 = 4,
>>> +};
>>
>> Hmm, I thought that the idea was in the end to be able to use the
>> button set interface with any random input device, with any random
>> amount of inputs? Limiting things to just 2 of each type sounds wrong
>> in that light, I think it would be better to just number axis 0 - #
>> without looking at their type, and add a separate type field, which
>> can be: RING, STRIP or UNKNOWN. Axis should probably also have flags
>> to indicate if they generate abs events, deltas or both.
> 
> hmm, good point, I honestly didn't think of that. I did a quick edit of the
> header file to see what the API would look like, see the diff below.
> The changes are axis from the enum to uint32_t and a few extra calls:
>     libinput_event_buttonset_has_absolute_value(event, axisnumber) 
>     libinput_device_buttonset_get_num_axes(device)
>     libinput_device_buttonset_get_axis_type(device, axisnumber)
> 
> The latter two take a device. I'm not sure I like the API more though, on
> the caller side it becomes a lot less expressive: before every call into the
> API would carry enough information to know what you're getting, now it's
> just an arbitrary number. It's more flexible this way though, that's for
> sure.
> 
> As for an UNKNOWN axis: I'd rather avoid this altogether. if we don't know
> what an axis is we should ignore it since we can't normalize the events or
> even guarantee that any event sequence is correct.
> 
> Thinking aloud: and that "no unknown axes" approach is also why I still like
> the original diff: it guarantees that any axis we export is known to
> libinput. We don't use the axis type as mask (in the API) so we have a _lot_
> of axes we can add eventually. I don't think having, eventually,
> LIBINPUT_BUTTONSET_AXIS_STRIP14 is the worst of
> all things. We could group them if we really wanted to as well, similar to
> the events, i.e. 
>  LIBINPUT_BUTTONSET_AXIS_RING = 1
>  LIBINPUT_BUTTONSET_AXIS_RING2,
>  ... empty space
>  LIBINPUT_BUTTONSET_AXIS_STRIP = 50,
>  ...
> 
> So that some procedural looping through a specific axis type is possible. If
> we ever have a device with more than 50 axis of the same type, libinput is
> probably not the best piece of code to handle it anyway :)

After a bit more thinking about this, I started liking this idea more
and more. The only change to the original patch would be spacing out the
enum, with calls to the API becoming both well defined and free-form and
forward-compatible.

If we know there are devices that use a specific axis we can add them to
the enum (RING2 for example), otherwise with the proper spacing all
other axes are still available:

To get the first ring value:
libinput_event_buttonset_get_axis_value(
         event,
         LIBINPUT_BUTTONSET_AXIS_RING);

To get the 14th ring value:

libinput_event_buttonset_get_axis_value(
         event,
         LIBINPUT_BUTTONSET_AXIS_RING + 13);

If we make the type spacing even, you can query all *potential* axes of
one type with:
  for (i = AXIS_RING; i < AXIS_RING + AXIS_TYPE_COUNT; i++)
       ....

I think that's a lot better than having just numbered axes, it adds the
a lot of semantics that are otherwise difficult to get.

Cheers,
  Peter

> diff --git a/src/libinput.h b/src/libinput.h
> index 651e823..9e26a8d 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -153,10 +153,9 @@ enum libinput_tablet_axis {
>   * LIBINPUT_DEVICE_CAP_BUTTONSET capability.
>   */
>  enum libinput_buttonset_axis_type {
> -	LIBINPUT_BUTTONSET_AXIS_RING = 1,
> -	LIBINPUT_BUTTONSET_AXIS_RING2 = 2,
> -	LIBINPUT_BUTTONSET_AXIS_STRIP = 3,
> -	LIBINPUT_BUTTONSET_AXIS_STRIP2 = 4,
> +	LIBINPUT_BUTTONSET_AXIS_TYPE_NONE,
> +	LIBINPUT_BUTTONSET_AXIS_TYPE_RING,
> +	LIBINPUT_BUTTONSET_AXIS_TYPE_STRIP,
>  };
>  
>  /**
> @@ -1319,24 +1318,39 @@ libinput_event_buttonset_get_time(struct libinput_event_buttonset *event);
>   */
>  int
>  libinput_event_buttonset_has_axis(struct libinput_event_buttonset *event,
> -				  enum libinput_buttonset_axis_type axis);
> +				  uint32_t axis);
>  
>  /**
>   * @ingroup event_buttonset
>   *
> + * Return 1 if the axis has an absolute value in this event or zero
> + * otherwise.
> + *
> + * An axis may provide an absolute value none of the time, all of the time
> + * or some of the time, depending on the axis type and device configuration
> + * settings.
> + *
> + * @param event The libinput buttonset event
> + * @param axis The axis to check for updates
> + * @return 1 if the axis provides an absolute value or 0 otherwise.
> + */
> +int
> +libinput_event_buttonset_has_absolute_value(struct libinput_event_buttonset *event,
> +					    uint32_t axis);
> +/**
> + * @ingroup event_buttonset
> + *
>   * Return the axis value of a given axis for a buttonset. The interpretation
> - * of the value is dependent on the axis:
> - * - @ref LIBINPUT_BUTTONSET_AXIS_RING and @ref
> - *   LIBINPUT_BUTTONSET_AXIS_RING2 - the absolute value of the wheel, in
> - *   degrees clockwise, 0 is the ring's northernmost point in the device's
> - *   current logical rotation.
> + * of the value is dependent on the axis type:
> + * - @ref LIBINPUT_BUTTONSET_AXIS_TYPE_RING- the absolute value of the
> + *   ring, in degrees clockwise, 0 is the ring's northernmost point in the
> + *   device's current logical rotation.
>   *   If the axis source is @ref LIBINPUT_BUTTONSET_AXIS_SOURCE_FINGER,
>   *   libinput will send an event with a value of -1 when the finger is
>   *   lifted.
> - * - @ref LIBINPUT_BUTTONSET_AXIS_STRIP and @ref
> - *   LIBINPUT_BUTTONSET_AXIS_STRIP2 - normalized to a range [0, 1] where 0 is
> - *   the strips top/left-most point in the device's current logical
> - *   rotation.
> + * - @ref LIBINPUT_BUTTONSET_AXIS_TYPE_STRIP - normalized to a range [0, 1]
> + *   where 0 is the strips top/left-most point in the device's current
> + *   logical rotation.
>   *   If the axis source is @ref LIBINPUT_BUTTONSET_AXIS_SOURCE_FINGER,
>   *   libinput will send an event with a value of -1 when the finger is
>   *   lifted.
> @@ -1348,7 +1362,7 @@ libinput_event_buttonset_has_axis(struct libinput_event_buttonset *event,
>   */
>  double
>  libinput_event_buttonset_get_axis_value(struct libinput_event_buttonset *event,
> -					enum libinput_buttonset_axis_type axis);
> +					uint32_t axis);
>  
>  /**
>   * @ingroup event_buttonset
> @@ -1382,7 +1396,7 @@ libinput_event_buttonset_get_axis_value_transformed(struct libinput_event_button
>   */
>  double
>  libinput_event_buttonset_get_axis_delta(struct libinput_event_buttonset *event,
> -				        enum libinput_buttonset_axis_type axis);
> +				        uint32_t axis);
>  
>  /**
>   * @ingroup event_buttonset
> @@ -1396,7 +1410,7 @@ libinput_event_buttonset_get_axis_delta(struct libinput_event_buttonset *event,
>   */
>  double
>  libinput_event_buttonset_get_axis_delta_discrete(struct libinput_event_buttonset *event,
> -						 enum libinput_buttonset_axis_type axis);
> +						 uint32_t axis);
>  
>  /**
>   * @ingroup event_buttonset
> @@ -1423,7 +1437,7 @@ libinput_event_buttonset_get_axis_delta_discrete(struct libinput_event_buttonset
>   */
>  enum libinput_buttonset_axis_source
>  libinput_event_buttonset_get_axis_source(struct libinput_event_buttonset *event,
> -					 enum libinput_buttonset_axis_type axis);
> +					 uint32_t axis);
>  
>  /**
>   * @ingroup event_buttonset
> @@ -2251,6 +2265,49 @@ libinput_device_has_button(struct libinput_device *device, uint32_t code);
>  /**
>   * @ingroup device
>   *
> + * Return the number of axes on this @ref LIBINPUT_DEVICE_CAP_BUTTONSET
> + * device.
> + *
> + * @note It is an application bug to call this function for devices without
> + * the @ref LIBINPUT_DEVICE_CAP_BUTTONSET capability. For those devices, this
> + * function returns 0.
> + *
> + * @param device A current input device with @ref
> + * LIBINPUT_DEVICE_CAP_BUTTONSET set
> + *
> + * @return The number of axes on this device.
> + */
> +size_t
> +libinput_device_buttonset_get_num_axes(struct libinput_device *device);
> +
> +/**
> + * @ingroup device
> + *
> + * Return the type of the given axis. Axis types remain constant for the
> + * lifetype of the device.
> + *
> + * @note It is an application bug to call this function for devices without
> + * the @ref LIBINPUT_DEVICE_CAP_BUTTONSET capability. For those devices, this
> + * function returns @ref LIBINPUT_BUTTONSET_AXIS_TYPE_NONE.
> + *
> + * @note It is an application bug to call this function for an axis that
> + * does not exist on a device. This function returns @ref
> + * LIBINPUT_BUTTONSET_AXIS_TYPE_NONE.
> + *
> + * @param device A current input device with @ref
> + * LIBINPUT_DEVICE_CAP_BUTTONSET set
> + * @return The axis type of this axis
> + * @retval LIBINPUT_BUTTONSET_AXIS_TYPE_NONE The axis does not exist on this
> + * device or this device does not have the @ref
> + * LIBINPUT_DEVICE_CAP_BUTTONSET capability.
> + */
> +enum libinput_buttonset_axis_type
> +libinput_device_buttonset_get_axis_type(struct libinput_event_buttonset *event,
> +					uint32_t axis);
> +
> +/**
> + * @ingroup device
> + *
>   * Increase the refcount of the device group. A device group will be freed
>   * whenever the refcount reaches 0. This may happen during dispatch if all
>   * devices of this group were removed from the system. A caller must ensure
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 



More information about the wayland-devel mailing list