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 <[email protected]>
> >---
> > 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
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to