[PATCH libinput 1/2] evdev: Query mouse DPI from udev

[Peter Hutterer]

On Mon, Nov 24, 2014 at 03:53:36PM -0600, Derek Foreman wrote:
> Instead of using a hard coded mouse DPI value, we query it from udev.
> If it's not present or the property is obviously broken we fall back
> to default.

for the archives: the thing we're struggling with on high-DPI mice is that
our pointer acceleration becomes rather useless. the DPI setting is
something we can't get from the device though, so our goal is to build a
udev database of "all" devices and go from there. once that is in place, we
expect the MOUSE_DPI property to be set and use that to calibrate our
pointer acceleration.

> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>  src/evdev.c         | 18 ++++++++++++++++++
>  src/libinput-util.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  src/libinput-util.h |  2 ++
>  3 files changed, 61 insertions(+)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 36c6859..a2547c7 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1218,8 +1218,11 @@ evdev_need_mtdev(struct evdev_device *device)
>  static void
>  evdev_tag_device(struct evdev_device *device)
>  {
> +	struct libinput *libinput = device->base.seat->libinput;
> +	const char *mouse_dpi;
>  	struct udev *udev;
>  	struct udev_device *udev_device = NULL;
> +	int dpi;
>  
>  	udev = udev_new();
>  	if (!udev)
> @@ -1229,6 +1232,21 @@ evdev_tag_device(struct evdev_device *device)
>  	if (udev_device) {
>  		if (device->dispatch->interface->tag_device)
>  			device->dispatch->interface->tag_device(device, udev_device);
> +		mouse_dpi = udev_device_get_property_value(udev_device, "MOUSE_DPI");
> +		if (mouse_dpi) {
> +			dpi = udev_parse_mouse_dpi_property(mouse_dpi);
> +			if (dpi)
> +				device->dpi = dpi;
> +			else {
> +				log_error(libinput, "Mouse DPI property for"
> +						    "  '%s' is present but "
> +						    "invalid, using %d DPI "
> +						    "instead\n",
> +						    device->devname,
> +						    DEFAULT_MOUSE_DPI);
> +				device->dpi = DEFAULT_MOUSE_DPI;
> +			}
> +		}
>  		udev_device_unref(udev_device);
>  	}
>  	udev_unref(udev);
> diff --git a/src/libinput-util.c b/src/libinput-util.c
> index 34d5549..5600424 100644
> --- a/src/libinput-util.c
> +++ b/src/libinput-util.c
> @@ -28,7 +28,9 @@
>  
>  #include "config.h"
>  
> +#include <ctype.h>
>  #include <stdarg.h>
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  
> @@ -113,3 +115,42 @@ ratelimit_test(struct ratelimit *r)
>  
>  	return RATELIMIT_EXCEEDED;
>  }
> +
> +int
> +udev_parse_mouse_dpi_property(const char *prop)

I'd prefer this not be named udev_... if it's a general utility. just drop
udev_ would be enough.


> +{
> +	bool is_default = false;
> +	int rd, dpi;

this is nitpicking, but I think we can afford the extra character storage to
have 'nread' instead of the harder-to-interpret 'rd' :)

> +
> +	/* When parsing the mouse DPI property, if we find an error we
> +	 * just return 0 since it's obviously invalid, the caller will
> +	 * treat that as an error and use a reasonable default instead.
> +	 * If the property contains multiple DPI settings but none flagged
> +	 * as default, we return the last because we're lazy and that's
> +	 * a silly way to set the property anyway.
> +	 */

it'd be nice to add a comment here on how the property _should_ look like.

otherwise, looks good, thanks. I'll get the udev hwdb patches to the list
asap, just a heads up: I'm going to wait until those are merged before we
merge this just in case there are any last-minute changes.

Cheers,
   Peter

> +	while (*prop != 0) {
> +		if (*prop == ' ') {
> +			prop++;
> +			continue;
> +		}
> +		if (*prop == '*') {
> +			prop++;
> +			is_default = true;
> +			if (!isdigit(prop[0]))
> +				return 0;
> +		}
> +
> +		rd = 0;
> +		sscanf(prop, "%d@%*d%n", &dpi, &rd);
> +		if (!rd)
> +			sscanf(prop, "%d%n", &dpi, &rd);
> +		if (!rd || dpi == 0 || prop[rd] == '@')
> +			return 0;

check for negative dpi/frequency here pls

otherwise, looks good, thanks.

Cheers,
   Peter

> +
> +		if (is_default)
> +			break;
> +		prop += rd;
> +	}
> +	return dpi;
> +}
> diff --git a/src/libinput-util.h b/src/libinput-util.h
> index 909c9db..62d7ac1 100644
> --- a/src/libinput-util.h
> +++ b/src/libinput-util.h
> @@ -296,4 +296,6 @@ struct ratelimit {
>  void ratelimit_init(struct ratelimit *r, uint64_t ival_ms, unsigned int burst);
>  enum ratelimit_state ratelimit_test(struct ratelimit *r);
>  
> +int udev_parse_mouse_dpi_property(const char *prop);
> +
>  #endif /* LIBINPUT_UTIL_H */
> -- 
> 2.1.3
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>