[PATCH 5/7] evdev: Normalize orientation and pressure values

Peter Hutterer peter.hutterer at who-t.net
Wed Jun 10 23:06:09 PDT 2015


On Wed, Jun 10, 2015 at 04:09:15PM +0200, Andreas Pokorny wrote:
> Pressure values will be mapped into the range [0;1] while orientation
> will be mapped to [-1;1] (or ]-1;1] or [0;1] depending on the how the
> underlying device configures the axis value ranges).

aargh. I guess there go my comments for the other patch. Please reshuffle
the patches so that the prep work goes in first and squash this one in with
the other one.

also I only just realised: 
    git config --add format.subjectprefix "PATCH `basename $PWD`"


> ---
>  src/evdev.c    |  6 ++++++
>  src/evdev.h    |  1 +
>  src/libinput.c | 14 ++++++++++++--
>  src/libinput.h | 11 ++++++-----
>  test/touch.c   |  8 ++++----
>  5 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 6f68554..f75e1c3 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1906,6 +1906,12 @@ evdev_configure_device(struct evdev_device *device)
>  			return -1;
>  		}
>  	}
> +	if (libevdev_has_event_code(evdev, EV_ABS, ABS_MT_ORIENTATION)) {
> +		device->abs.absinfo_orientation = libevdev_get_abs_info(evdev, ABS_MT_ORIENTATION);
> +	}
> +	if (libevdev_has_event_code(evdev, EV_ABS, ABS_MT_PRESSURE)) {
> +		device->abs.absinfo_pressure = libevdev_get_abs_info(evdev, ABS_MT_PRESSURE);
> +	}

no {} around single lines
libevdev returns NULL for non-existing codes, so you can skip the condition
anyway.

>  
>  	if (udev_tags & EVDEV_UDEV_TAG_TOUCHPAD) {
>  		device->dispatch = evdev_mt_touchpad_create(device);
> diff --git a/src/evdev.h b/src/evdev.h
> index 8e67e5d..c7d762b 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -128,6 +128,7 @@ struct evdev_device {
>  	int fd;
>  	struct {
>  		const struct input_absinfo *absinfo_x, *absinfo_y;
> +		const struct input_absinfo *absinfo_pressure, *absinfo_orientation;
>  		int fake_resolution;
>  
>  		struct device_coords point;
> diff --git a/src/libinput.c b/src/libinput.c
> index 92cc790..9357c01 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -667,25 +667,35 @@ libinput_event_touch_get_touch_minor(struct libinput_event_touch *event)
>  LIBINPUT_EXPORT double
>  libinput_event_touch_get_orientation(struct libinput_event_touch *event)
>  {
> +	struct evdev_device *device =
> +		(struct evdev_device *) event->base.device;
> +
>  	require_event_type(libinput_event_get_context(&event->base),
>  			   event->base.type,
>  			   0,
>  			   LIBINPUT_EVENT_TOUCH_DOWN,
>  			   LIBINPUT_EVENT_TOUCH_MOTION);
>  
> -	return event->orientation;
> +	if (device->abs.absinfo_orientation->minimum < 0)
> +		return evdev_scale_axis(device->abs.absinfo_orientation, event->orientation, 2.0) - 1.0;
> +	else /* this branch assume a simplisitic orientation calculation which either only yields 0 or 1
> +		or only values within the positive quadrant between y and x axis */
> +		return evdev_scale_axis(device->abs.absinfo_orientation, event->orientation, 1.0);

line width 80 please, and put the comment before the if rather than
half-nested in the else.

also, looking at the kernel's documentation for ABS_MT_ORIENTATION looks
like we need to be a bit more fine-grained here.

>  }
>  
>  LIBINPUT_EXPORT double
>  libinput_event_touch_get_pressure(struct libinput_event_touch *event)
>  {
> +	struct evdev_device *device =
> +		(struct evdev_device *) event->base.device;
> +
>  	require_event_type(libinput_event_get_context(&event->base),
>  			   event->base.type,
>  			   0,
>  			   LIBINPUT_EVENT_TOUCH_DOWN,
>  			   LIBINPUT_EVENT_TOUCH_MOTION);
>  
> -	return event->pressure;
> +	return evdev_scale_axis(device->abs.absinfo_pressure, event->pressure, 1.0);
>  }
>  
>  struct libinput_source *
> diff --git a/src/libinput.h b/src/libinput.h
> index deecdda..f6f51b5 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -950,8 +950,8 @@ libinput_event_touch_get_touch_major(struct libinput_event_touch *event);
>  /**
>   * @ingroup event_touch
>   *
> - * Return the current pressure value touch point as a device specific value.
> - * This value might not be measured by the device.
> + * Return the current pressure value touch point normalized to the range
> + * [0;1]. This value might not be measured by the device.
>   *
>   * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
>   * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> @@ -968,9 +968,10 @@ libinput_event_touch_get_pressure(struct libinput_event_touch *event);
>  /**
>   * @ingroup event_touch
>   *
> - * Return the orientation of the touch ellipse in device specific values.
> - * If the ellipse is aligned with the Y Axis of the screen 0 should be
> - * returned.
> + * Return the revolution of the touch ellipse against the Y axis within the
> + * range [-1;1]. The value 0 will be returned when the ellipse is aligned

rest of the file uses a comma, not a ; (i.e. [-1, 1])

s/revolution/orientation/ I think. also, there's a couple of problems here.
one is: if we know 1 is a 90 degree rotation, why not provide the value in
degrees?
second: a caller can't know if the device provides a 90 degree or 180 degree
rotation, so the value of 1 is ambiguous.

knowing what the use-case is you have in mind should help here.

Cheers,
   Peter

> + * with the Y Axis. If the ellipse is aligned with X Axis 1 will be returned.
> + * This value might not be measured by the device.
>   *
>   * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
>   * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> diff --git a/test/touch.c b/test/touch.c
> index 7a69bd7..10c4e2d 100644
> --- a/test/touch.c
> +++ b/test/touch.c
> @@ -674,8 +674,8 @@ START_TEST(touch_point_properties)
>  	tev = libinput_event_get_touch_event(ev);
>  
>  	ck_assert_int_eq(libinput_event_touch_get_slot(tev), 0);
> -	ck_assert_int_eq(libinput_event_touch_get_pressure(tev), 128);
> -	ck_assert_int_eq(libinput_event_touch_get_orientation(tev), 64);
> +	litest_assert_double_eq(libinput_event_touch_get_pressure(tev), 0.5);
> +	litest_assert_double_eq(libinput_event_touch_get_orientation(tev), 0.25);
>  	ck_assert_int_eq(libinput_event_touch_get_touch_major(tev), 14);
>  	ck_assert_int_eq(libinput_event_touch_get_touch_minor(tev), 8);
>  
> @@ -691,8 +691,8 @@ START_TEST(touch_point_properties)
>  	ev = libinput_get_event(li);
>  	tev = libinput_event_get_touch_event(ev);
>  
> -	ck_assert_int_eq(libinput_event_touch_get_pressure(tev), 128);
> -	ck_assert_int_eq(libinput_event_touch_get_orientation(tev), 128);
> +	litest_assert_double_eq(libinput_event_touch_get_pressure(tev), 0.5);
> +	litest_assert_double_eq(libinput_event_touch_get_orientation(tev), 0.5);
>  	ck_assert_int_eq(libinput_event_touch_get_touch_major(tev), 30);
>  	ck_assert_int_eq(libinput_event_touch_get_touch_minor(tev), 8);
>  
> -- 
> 2.1.4
> 


More information about the wayland-devel mailing list