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

[Peter Hutterer]

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.

Cheers,
   Peter