[PATCH libinput] evdev: implement support for the MOUSE_WHEEL_CLICK_COUNT property

[Hans de Goede]

Hi,

On 03-11-16 03:16, Peter Hutterer wrote:
> On Fri, Oct 28, 2016 at 12:57:38PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 28-10-16 07:08, Peter Hutterer wrote:
>>> Not all mice have a click angle with integer degrees. The new
>>> MOUSE_WHEEL_CLICK_COUNT property specifies how many clicks per full rotation,
>>> the angle can be calculated from that.
>>>
>>> See https://github.com/systemd/systemd/pull/4440 for more information
>>>
>>> CLICK_COUNT overrides CLICK_ANGLE, so we check for the former first and then
>>> fall back to the angle if need be. No changes to the user-facing API.
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>> src/evdev.c                                  | 51 +++++++++++++++---
>>> src/libinput-private.h                       |  2 +-
>>> src/libinput-util.c                          | 32 ++++++++++++
>>> src/libinput-util.h                          |  1 +
>>> test/Makefile.am                             |  1 +
>>> test/litest-device-mouse-wheel-click-count.c | 77 ++++++++++++++++++++++++++++
>>> test/litest.c                                |  2 +
>>> test/litest.h                                |  1 +
>>> test/misc.c                                  | 30 +++++++++++
>>> test/pointer.c                               | 49 +++++++++++++++---
>>> 10 files changed, 229 insertions(+), 17 deletions(-)
>>> create mode 100644 test/litest-device-mouse-wheel-click-count.c
>>>
>>> diff --git a/src/evdev.c b/src/evdev.c
>>> index d49b391..1c46534 100644
>>> --- a/src/evdev.c
>>> +++ b/src/evdev.c
>>> @@ -2008,7 +2008,7 @@ evdev_device_init_pointer_acceleration(struct evdev_device *device,
>>> static inline bool
>>> evdev_read_wheel_click_prop(struct evdev_device *device,
>>> 			    const char *prop,
>>> -			    int *angle)
>>> +			    double *angle)
>>> {
>>> 	int val;
>>>
>>> @@ -2032,18 +2032,53 @@ evdev_read_wheel_click_prop(struct evdev_device *device,
>>> 	return false;
>>> }
>>>
>>> +static inline bool
>>> +evdev_read_wheel_click_count_prop(struct evdev_device *device,
>>> +				  const char *prop,
>>> +				  double *angle)
>>> +{
>>> +	int val;
>>> +
>>> +	prop = udev_device_get_property_value(device->udev_device, prop);
>>> +	if (!prop)
>>> +		return false;
>>> +
>>> +	val = parse_mouse_wheel_click_angle_property(prop);
>>> +	if (val) {
>>> +		*angle = 360.0/val;
>>> +		return true;
>>> +	}
>>> +
>>> +	log_error(evdev_libinput_context(device),
>>> +		  "Mouse wheel click count '%s' is present but invalid, "
>>> +		  "using %d degrees for angle instead instead\n",
>>> +		  device->devname,
>>> +		  DEFAULT_WHEEL_CLICK_ANGLE);
>>> +	*angle = DEFAULT_WHEEL_CLICK_ANGLE;
>>> +
>>> +	return false;
>>> +}
>>> +
>>
>> This is almost a 100% copy of evdev_read_wheel_click_prop
>> how about giving evdev_read_wheel_click_prop an extra
>> "bool count" argument and then doing:
>>
>> 	if (val) {
>> 		if (count)
>> 			*angle = 360.0 / val;
>> 		else
>> 			*angle = val;
>> 		return true;
>> 	}
>>
>> Then we do not need the almost identical function.
>
> I played around with this for a few days and tbh I'm not happy any
> incarnation that I tried. Any merging together means we have to have
> conditions for the error message (or a more generic and thus less useful
> one). And with the parsing functions themselves, yes, they're 100% the same
> but most of the info is contained in the comments above to explain what the
> property does. Merging those together is also a net loss in information.
>
> Given that they're small enough and they won't see much change over time,
> I'm gonna go with the current patch after all.

Ok.

Regards,

Hans