[PATCH libinput v5 5/5] libinput: add orientation and size of touch point and pressure to the API
Peter Hutterer
peter.hutterer at who-t.net
Tue Jul 7 16:03:11 PDT 2015
On Mon, Jul 06, 2015 at 05:15:01PM +0200, Andreas Pokorny wrote:
> This change adds four new properties to touch events.
> major: diameter of the touch ellipse along the major axis
> minor: diameter perpendicular to major axis
> pressure: a pressure value mapped into the range [0,1]
> orientation: the angle between major and the x axis
>
> Those values are optionally supported by multi-touch drivers, so
> zero as a default value is used if the information is missing.
>
> This change also correctly bumps the ABI version to 0.20.0 and
> adds the new libinput symbols there.
>
> Signed-off-by: Andreas Pokorny <andreas.pokorny at canonical.com>
> ---
> src/evdev.c | 192 +++++++++++++++++++++++++++++++++++++++++-------
> src/evdev.h | 24 ++++++
> src/libinput-private.h | 10 ++-
> src/libinput.c | 137 +++++++++++++++++++++++++++++++++-
> src/libinput.h | 194 +++++++++++++++++++++++++++++++++++++++++++++++++
> src/libinput.sym | 10 +++
> test/touch.c | 162 +++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 697 insertions(+), 32 deletions(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index 346f11a..1dbfc61 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -45,6 +45,10 @@
>
> #define DEFAULT_WHEEL_CLICK_ANGLE 15
> #define DEFAULT_MIDDLE_BUTTON_SCROLL_TIMEOUT 200
> +#define DEFAULT_TOUCH_PRESSURE 0
> +#define DEFAULT_TOUCH_ORIENTATION 0
> +#define DEFAULT_TOUCH_MAJOR 0
> +#define DEFAULT_TOUCH_MINOR 0
>
> enum evdev_key_type {
> EVDEV_KEY_TYPE_NONE,
> @@ -87,6 +91,12 @@ static const struct evdev_udev_tag_match evdev_udev_tag_matches[] = {
> { 0 },
> };
>
> +static inline double
> +deg2rad(double angle)
> +{
> + return angle * M_PI/180.0;
> +}
> +
> static void
> hw_set_key_down(struct evdev_device *device, int code, int pressed)
> {
> @@ -245,6 +255,80 @@ evdev_device_transform_y(struct evdev_device *device,
> return scale_axis(device->abs.absinfo_y, y, height);
> }
>
> +double
> +evdev_device_transform_touch_point_to_mm(struct evdev_device *device,
> + int32_t axis_value,
> + double axis_angle)
oh dear. I just spent about 15 min trying to figure out the conversion and
wondering why you pass x/y in here etc. "point" refers to a point on the
screen. You're passing in the size of the ellipsis so please don't name this
touch_point, it's very confusing. Name it ellipsis instead, that's how you
call it in the test suite too.
> +{
> + double x_res = device->abs.absinfo_x->resolution;
> + double y_res = device->abs.absinfo_y->resolution;
> +
> + if (x_res == y_res)
> + return axis_value / x_res;
> +
> + if (device->abs.absinfo_orientation == NULL)
> + return axis_value * 2.0 / (x_res + y_res);
I admit, I'm a bit confused in how this should work. if we don't have
orientation, the angle should always be 0, right?
so the returned value is either /xres or /yres, depending on what you pass
in. can you explain this in more detail for me please?
> +
> + return axis_value / (y_res*fabs(cos(deg2rad(axis_angle))) +
> + x_res*fabs(sin(deg2rad(axis_angle))));
can we please use a tmp scale variable for the fabs(cos(deg2rad())) nesting?
(besides, please dont mix spaces with no spaces)
also, this is doing my head in. we're using x for the touch major and y for
the touch minor, despite, in the default angle, the major being the y axis
and the minor being the x axis. Which leads to the odd case of
in the simplest case, we have an angle of 0 -> cos 1, sin 0. a touch major
is assigned to touch_point.x and the returned value is x/yres. that's
correct I think, but confusing as hell.
I think we should assign major to y and minor to x, that would make things a
lot less confusing.
> +}
> +
> +double
> +evdev_device_transform_touch_point(struct evdev_device *device,
> + int32_t axis_value,
> + double axis_angle,
> + uint32_t width,
> + uint32_t height)
> +{
> + double x_res = device->abs.absinfo_x->resolution;
> + double y_res = device->abs.absinfo_y->resolution;
> + double x_scale = width / (device->abs.dimensions.x + 1.0);
> + double y_scale = height / (device->abs.dimensions.y + 1.0);
> +
> + if (x_res == y_res)
> + return axis_value * x_scale;
> +
> + if (device->abs.absinfo_orientation == NULL)
> + return axis_value * (x_scale + y_scale) / 2.0;
> +
> + return axis_value * (y_scale*fabs(cos(deg2rad(axis_angle))) +
> + x_scale*fabs(sin(deg2rad(axis_angle))));
> +}
> +
> +double
> +evdev_device_transform_orientation(struct evdev_device *device,
> + int32_t orientation)
> +{
> + const struct input_absinfo *orientation_info =
> + device->abs.absinfo_orientation;
> +
> + /* ABS_MT_ORIENTATION is defined as a clockwise rotation - zero
> + * (instead of minimum) is mapped to the y-axis, and maximum is
> + * mapped to the x-axis. So minimum is likely to be negative but
> + * plays no role in scaling the value to degrees.*/
it does, because you're promising that the value is in degrees clockwise,
with the example explicitly stating 315 degrees. So you either need to
fix the example to say you're returning -45, or offset this properly:
double orientation;
orientation = (90.0 * orientation) / orientation_info->maximum;
return (360 + orientation) % 360;
I'd prefer the offset approach, we have everything else as 0-360 degrees
rather than a +/- angle.
> + if (orientation_info)
> + return (90.0 * orientation) / orientation_info->maximum;
> + else
> + return DEFAULT_TOUCH_ORIENTATION;
> +}
> +
> +double
> +evdev_device_transform_pressure(struct evdev_device *device,
> + int32_t pressure)
> +{
> + const struct input_absinfo *pressure_info = device->abs.absinfo_pressure;
> +
> + if (pressure_info) {
> + double max_pressure = pressure_info->maximum;
> + double min_pressure = pressure_info->minimum;
> + return (double)(pressure - min_pressure) /
> + (max_pressure - min_pressure);
> + }
> + else {
> + return DEFAULT_TOUCH_PRESSURE;
> + }
> +}
> +
> static inline void
> normalize_delta(struct evdev_device *device,
> const struct device_coords *delta,
> @@ -282,8 +366,14 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time)
> struct normalized_coords accel, unaccel;
> struct device_coords point;
> struct device_float_coords raw;
> + struct mt_slot *slot_data;
> + struct device_coords default_touch = {
> + .x = DEFAULT_TOUCH_MAJOR,
> + .y = DEFAULT_TOUCH_MINOR
> + };
>
> slot = device->mt.slot;
> + slot_data = &device->mt.slots[slot];
>
> switch (device->pending_event) {
> case EVDEV_NONE:
> @@ -314,7 +404,7 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time)
> if (!(device->seat_caps & EVDEV_DEVICE_TOUCH))
> break;
>
> - if (device->mt.slots[slot].seat_slot != -1) {
> + if (slot_data->seat_slot != -1) {
> log_bug_kernel(libinput,
> "%s: Driver sent multiple touch down for the "
> "same slot",
> @@ -323,38 +413,52 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time)
> }
>
> seat_slot = ffs(~seat->slot_map) - 1;
> - device->mt.slots[slot].seat_slot = seat_slot;
> + slot_data->seat_slot = seat_slot;
>
> if (seat_slot == -1)
> break;
>
> seat->slot_map |= 1 << seat_slot;
> - point = device->mt.slots[slot].point;
> + point = slot_data->point;
> transform_absolute(device, &point);
>
> - touch_notify_touch_down(base, time, slot, seat_slot,
> - &point);
> + touch_notify_touch_down(base,
> + time,
> + slot,
> + seat_slot,
> + &point,
> + &slot_data->touch_point,
> + slot_data->orientation,
> + slot_data->pressure);
> break;
> case EVDEV_ABSOLUTE_MT_MOTION:
> if (!(device->seat_caps & EVDEV_DEVICE_TOUCH))
> break;
>
> - seat_slot = device->mt.slots[slot].seat_slot;
> - point = device->mt.slots[slot].point;
> + seat_slot = slot_data->seat_slot;
> +
> + point = slot_data->point;
>
> if (seat_slot == -1)
> break;
>
> transform_absolute(device, &point);
> - touch_notify_touch_motion(base, time, slot, seat_slot,
> - &point);
> +
> + touch_notify_touch_motion(base,
> + time,
> + slot,
> + seat_slot,
> + &point,
> + &slot_data->touch_point,
> + slot_data->orientation,
> + slot_data->pressure);
> break;
> case EVDEV_ABSOLUTE_MT_UP:
> if (!(device->seat_caps & EVDEV_DEVICE_TOUCH))
> break;
>
> - seat_slot = device->mt.slots[slot].seat_slot;
> - device->mt.slots[slot].seat_slot = -1;
> + seat_slot = slot_data->seat_slot;
> + slot_data->seat_slot = -1;
>
> if (seat_slot == -1)
> break;
> @@ -386,7 +490,14 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time)
> point = device->abs.point;
> transform_absolute(device, &point);
>
> - touch_notify_touch_down(base, time, -1, seat_slot, &point);
> + touch_notify_touch_down(base,
> + time,
> + -1,
> + seat_slot,
> + &point,
> + &default_touch,
> + DEFAULT_TOUCH_ORIENTATION,
> + DEFAULT_TOUCH_PRESSURE);
> break;
> case EVDEV_ABSOLUTE_MOTION:
> point = device->abs.point;
> @@ -398,8 +509,14 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time)
> if (seat_slot == -1)
> break;
>
> - touch_notify_touch_motion(base, time, -1, seat_slot,
> - &point);
> + touch_notify_touch_motion(base,
> + time,
> + -1,
> + seat_slot,
> + &point,
> + &default_touch,
> + DEFAULT_TOUCH_ORIENTATION,
> + DEFAULT_TOUCH_PRESSURE);
> } else if (device->seat_caps & EVDEV_DEVICE_POINTER) {
> pointer_notify_motion_absolute(base, time, &point);
> }
> @@ -559,8 +676,9 @@ evdev_process_touch(struct evdev_device *device,
> struct input_event *e,
> uint64_t time)
> {
> - switch (e->code) {
> - case ABS_MT_SLOT:
> + struct mt_slot *current_slot = &device->mt.slots[device->mt.slot];
> +
> + if (e->code == ABS_MT_SLOT) {
> if ((size_t)e->value >= device->mt.slots_len) {
> log_bug_libinput(device->base.seat->libinput,
> "%s exceeds slots (%d of %d)\n",
> @@ -571,8 +689,7 @@ evdev_process_touch(struct evdev_device *device,
> }
> evdev_flush_pending_event(device, time);
> device->mt.slot = e->value;
> - break;
> - case ABS_MT_TRACKING_ID:
> + } else if(e->code == ABS_MT_TRACKING_ID) {
> if (device->pending_event != EVDEV_NONE &&
> device->pending_event != EVDEV_ABSOLUTE_MT_MOTION)
> evdev_flush_pending_event(device, time);
> @@ -580,17 +697,34 @@ evdev_process_touch(struct evdev_device *device,
> device->pending_event = EVDEV_ABSOLUTE_MT_DOWN;
> else
> device->pending_event = EVDEV_ABSOLUTE_MT_UP;
> - break;
> - case ABS_MT_POSITION_X:
> - device->mt.slots[device->mt.slot].point.x = e->value;
> - if (device->pending_event == EVDEV_NONE)
> - device->pending_event = EVDEV_ABSOLUTE_MT_MOTION;
> - break;
> - case ABS_MT_POSITION_Y:
> - device->mt.slots[device->mt.slot].point.y = e->value;
> - if (device->pending_event == EVDEV_NONE)
> + } else {
> + bool needs_wake = true;
> +
> + switch(e->code) {
> + case ABS_MT_POSITION_X:
> + current_slot->point.x = e->value;
> + break;
> + case ABS_MT_POSITION_Y:
> + current_slot->point.y = e->value;
> + break;
> + case ABS_MT_TOUCH_MAJOR:
> + current_slot->touch_point.x = e->value;
> + break;
> + case ABS_MT_TOUCH_MINOR:
> + current_slot->touch_point.y = e->value;
> + break;
> + case ABS_MT_ORIENTATION:
> + current_slot->orientation = e->value;
> + break;
> + case ABS_MT_PRESSURE:
> + current_slot->pressure = e->value;
> + break;
> + default:
> + needs_wake = false;
> + break;
> + }
> + if (needs_wake && device->pending_event == EVDEV_NONE)
> device->pending_event = EVDEV_ABSOLUTE_MT_MOTION;
> - break;
> }
> }
>
> @@ -1921,6 +2055,8 @@ evdev_configure_device(struct evdev_device *device)
> return -1;
> }
> }
> + device->abs.absinfo_orientation = libevdev_get_abs_info(evdev, ABS_MT_ORIENTATION);
> + device->abs.absinfo_pressure = libevdev_get_abs_info(evdev, ABS_MT_PRESSURE);
hmm, you know you're setting this after setting up the touchscreen?
(evdev_configure_mt_device) That's a bit odd, it means those two will be
null in that function. we don't need it right now, but still...
>
> 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 aa548d2..746a3df 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -109,6 +109,9 @@ enum evdev_device_model {
> struct mt_slot {
> int32_t seat_slot;
> struct device_coords point;
> + struct device_coords touch_point;
> + int32_t orientation;
> + int32_t pressure;
> };
>
> struct evdev_device {
> @@ -125,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;
> @@ -344,6 +348,26 @@ double
> evdev_device_transform_y(struct evdev_device *device,
> double y,
> uint32_t height);
> +double
> +evdev_device_transform_touch_point_to_mm(struct evdev_device *device,
> + int32_t axis_value,
> + double axis_angle);
> +
> +double
> +evdev_device_transform_touch_point(struct evdev_device *device,
> + int32_t axis_value,
> + double axis_angle,
> + uint32_t width,
> + uint32_t height);
> +
> +double
> +evdev_device_transform_orientation(struct evdev_device *device,
> + int32_t orientation);
> +
> +double
> +evdev_device_transform_pressure(struct evdev_device *device,
> + int32_t pressure);
> +
> int
> evdev_device_suspend(struct evdev_device *device);
>
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index d11f000..d960a82 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -350,14 +350,20 @@ touch_notify_touch_down(struct libinput_device *device,
> uint64_t time,
> int32_t slot,
> int32_t seat_slot,
> - const struct device_coords *point);
> + const struct device_coords *point,
> + const struct device_coords *touch_point,
> + int32_t orientation,
> + int32_t pressure);
>
> void
> touch_notify_touch_motion(struct libinput_device *device,
> uint64_t time,
> int32_t slot,
> int32_t seat_slot,
> - const struct device_coords *point);
> + const struct device_coords *point,
> + const struct device_coords *touch_point,
> + int32_t orientation,
> + int32_t pressure);
>
> void
> touch_notify_touch_up(struct libinput_device *device,
> diff --git a/src/libinput.c b/src/libinput.c
> index d164604..32ed817 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -112,6 +112,9 @@ struct libinput_event_touch {
> int32_t slot;
> int32_t seat_slot;
> struct device_coords point;
> + struct device_coords touch_point;
> + int32_t orientation;
> + int32_t pressure;
> };
>
> static void
> @@ -637,6 +640,124 @@ libinput_event_touch_get_y(struct libinput_event_touch *event)
> return evdev_convert_to_mm(device->abs.absinfo_y, event->point.y);
> }
>
> +LIBINPUT_EXPORT double
> +libinput_event_touch_get_major(struct libinput_event_touch *event)
> +{
> + struct evdev_device *device =
> + (struct evdev_device *) event->base.device;
> + double angle = evdev_device_transform_orientation(device,
> + event->orientation);
> +
> + require_event_type(libinput_event_get_context(&event->base),
> + event->base.type,
> + 0,
> + LIBINPUT_EVENT_TOUCH_DOWN,
> + LIBINPUT_EVENT_TOUCH_MOTION);
> +
> + return evdev_device_transform_touch_point_to_mm(device,
> + event->touch_point.x,
> + angle);
> +}
> +
> +LIBINPUT_EXPORT double
> +libinput_event_touch_get_major_transformed(struct libinput_event_touch *event,
> + uint32_t width,
> + uint32_t height)
> +{
> + struct evdev_device *device =
> + (struct evdev_device *) event->base.device;
> + double angle = evdev_device_transform_orientation(device,
> + event->orientation);
> +
> + require_event_type(libinput_event_get_context(&event->base),
> + event->base.type,
> + 0,
> + LIBINPUT_EVENT_TOUCH_DOWN,
> + LIBINPUT_EVENT_TOUCH_MOTION);
> +
> + return evdev_device_transform_touch_point(device,
> + event->touch_point.x,
> + angle,
> + width,
> + height);
> +}
> +
> +LIBINPUT_EXPORT double
> +libinput_event_touch_get_minor(struct libinput_event_touch *event)
> +{
> + struct evdev_device *device =
> + (struct evdev_device *) event->base.device;
> + double angle = evdev_device_transform_orientation(device,
> + event->orientation);
> +
> + require_event_type(libinput_event_get_context(&event->base),
> + event->base.type,
> + 0,
> + LIBINPUT_EVENT_TOUCH_DOWN,
> + LIBINPUT_EVENT_TOUCH_MOTION);
> +
> + return evdev_device_transform_touch_point_to_mm(device,
> + event->touch_point.y,
> + angle + 90.0);
> +
> +}
> +
> +LIBINPUT_EXPORT double
> +libinput_event_touch_get_minor_transformed(struct libinput_event_touch *event,
> + uint32_t width,
> + uint32_t height)
> +{
> + struct evdev_device *device =
> + (struct evdev_device *) event->base.device;
> + double angle =
> + evdev_device_transform_orientation(device,
> + event->orientation);
> +
> + require_event_type(libinput_event_get_context(&event->base),
> + event->base.type,
> + 0,
> + LIBINPUT_EVENT_TOUCH_DOWN,
> + LIBINPUT_EVENT_TOUCH_MOTION);
> +
> + return evdev_device_transform_touch_point(device,
> + event->touch_point.y,
> + angle + 90.0,
> + width,
> + height);
> +}
> +
> +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 evdev_device_transform_orientation(device,
> + event->orientation);
> +}
> +
> +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 evdev_device_transform_pressure(device,
> + event->pressure);
> +}
> +
> struct libinput_source *
> libinput_add_fd(struct libinput *libinput,
> int fd,
> @@ -1243,7 +1364,10 @@ touch_notify_touch_down(struct libinput_device *device,
> uint64_t time,
> int32_t slot,
> int32_t seat_slot,
> - const struct device_coords *point)
> + const struct device_coords *point,
> + const struct device_coords *touch_point,
> + int32_t orientation,
> + int32_t pressure)
> {
> struct libinput_event_touch *touch_event;
>
> @@ -1259,6 +1383,9 @@ touch_notify_touch_down(struct libinput_device *device,
> .slot = slot,
> .seat_slot = seat_slot,
> .point = *point,
> + .touch_point = *touch_point,
> + .orientation = orientation,
> + .pressure = pressure,
> };
>
> post_device_event(device, time,
> @@ -1271,7 +1398,10 @@ touch_notify_touch_motion(struct libinput_device *device,
> uint64_t time,
> int32_t slot,
> int32_t seat_slot,
> - const struct device_coords *point)
> + const struct device_coords *point,
> + const struct device_coords *touch_point,
> + int32_t orientation,
> + int32_t pressure)
> {
> struct libinput_event_touch *touch_event;
>
> @@ -1287,6 +1417,9 @@ touch_notify_touch_motion(struct libinput_device *device,
> .slot = slot,
> .seat_slot = seat_slot,
> .point = *point,
> + .touch_point = *touch_point,
> + .orientation = orientation,
> + .pressure = pressure,
> };
>
> post_device_event(device, time,
> diff --git a/src/libinput.h b/src/libinput.h
> index 5df7183..c705398 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -919,6 +919,200 @@ libinput_event_touch_get_y_transformed(struct libinput_event_touch *event,
> /**
> * @ingroup event_touch
> *
> + * @verbatim
> + * screen space
> + * major
> + * _______/\________
> + * / \
> + * +++++++
> + * +++ +++
> + * ++ ++
> + * + touch +
> + * + ellipse + ---> finger pointing direction
> + * ++ ++
> + * +++ +++
> + * +++++++
> + * @endverbatim
after looking at the touch_point and everything else, I think we should
rotate this graphic. The "finger pointing direction" helps of course, but
having it laid out like a finger is usually positioned on the screen (i.e.
vertical) will make comprehension a lot easier.
or you leave the ascci-art out completely, explain it in words and @ref to a
doxygen @page that has proper svg graphics.
> + *
> + * Return the diameter of the major axis of the touch ellipse in mm.
> + * This value might not be provided by the device, in that case the value
> + * 0.0 is returned
'.' missing
> + *
> + * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
> + * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> + *
> + * @note It is an application bug to call this function for events of type
> + * @ref LIBINPUT_EVENT_TOUCH_DOWN or @ref LIBINPUT_EVENT_TOUCH_MOTION.
> + *
> + * @param event The libinput touch event
> + * @return The current touch major value
> + */
> +double
> +libinput_event_touch_get_major(struct libinput_event_touch *event);
> +
> +/**
> + * @ingroup event_touch
> + *
> + * @verbatim
> + * screen space
> + * major
> + * _______/\________
> + * / \
> + * +++++++
> + * +++ +++
> + * ++ ++
> + * + touch +
> + * + ellipse + ---> finger pointing direction
> + * ++ ++
> + * +++ +++
> + * +++++++
> + * @endverbatim
> + *
> + * Return the diameter of the major axis of the touch ellipse in screen
> + * space. This value might not be provided by the device, in that case the
> + * value 0.0 is returned.
> + *
> + * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
> + * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> + *
> + * @note It is an application bug to call this function for events of type
> + * @ref LIBINPUT_EVENT_TOUCH_DOWN or @ref LIBINPUT_EVENT_TOUCH_MOTION.
> + *
> + * @param event The libinput touch event
> + * @param width The current output screen width
> + * @param height The current output screen height
> + * @return The current touch major value
> + */
> +double
> +libinput_event_touch_get_major_transformed(struct libinput_event_touch *event,
> + uint32_t width,
> + uint32_t height);
> +
> +/**
> + * @ingroup event_touch
> + *
> + * @verbatim
> + * screen space
> + *
> + * +++++++ -
> + * +++ +++ \
> + * ++ ++ |
> + * + touch + =- minor
> + * + ellipse + | ---> finger pointing direction
> + * ++ ++ |
> + * +++ +++ /
> + * +++++++ -
> + *
> + * @endverbatim
> + *
> + * Return the diameter of the minor axis of the touch ellipse in mm.
> + * This value might not be provided by the device, in this case the value
> + * 0.0 is returned.
> + *
> + * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
> + * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> + *
> + * @note It is an application bug to call this function for events of type
> + * @ref LIBINPUT_EVENT_TOUCH_DOWN or @ref LIBINPUT_EVENT_TOUCH_MOTION.
> + *
> + * @param event The libinput touch event
> + * @return The current touch minor value
> + */
> +double
> +libinput_event_touch_get_minor(struct libinput_event_touch *event);
> +
> +/**
> + * @ingroup event_touch
> + *
> + * @verbatim
> + * screen space
> + *
> + * +++++++ -
> + * +++ +++ \
> + * ++ ++ |
> + * + touch + =- minor
> + * + ellipse + | ---> finger pointing direction
> + * ++ ++ |
> + * +++ +++ /
> + * +++++++ -
> + *
> + * @endverbatim
> + *
> + * Return the diameter of the minor axis of the touch ellipse in screen
> + * space. This value might not be provided by the device, in this case
> + * the value 0.0 is returned.
> + *
> + * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
> + * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> + *
> + * @note It is an application bug to call this function for events of type
> + * @ref LIBINPUT_EVENT_TOUCH_DOWN or @ref LIBINPUT_EVENT_TOUCH_MOTION.
> + *
> + * @param event The libinput touch event
> + * @param width The current output screen width
> + * @param height The current output screen height
> + * @return The current touch minor value
> + */
> +double
> +libinput_event_touch_get_minor_transformed(struct libinput_event_touch *event,
> + uint32_t width,
> + uint32_t height);
> +
> +/**
> + * @ingroup event_touch
> + *
> + * Return the current pressure value touch point normalized to the range
> + * [0,1]. If this value is not provided by the device, it is always 0.0.
thinking about this - we will at some point have to add hovering to
libinput and that's where a pressure default of 0 is going to bite us.
I'd go with the opposite and say that if pressure doesn't exist, it's always
1 on touch.
> + *
> + * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
> + * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> + *
> + * @note It is an application bug to call this function for events of type
> + * @ref LIBINPUT_EVENT_TOUCH_DOWN or @ref LIBINPUT_EVENT_TOUCH_MOTION.
> + *
> + * @param event The libinput touch event
> + * @return The current pressure value
> + */
> +double
> +libinput_event_touch_get_pressure(struct libinput_event_touch *event);
> +
> +/**
> + * @ingroup event_touch
> + * @verbatim
> + *
> + * major ∧ y
> + * axis \ |
> + * \++ |
> + * + \ ++
> + * + \ |+
> + * + \| +
> + * + 0--+---------------> x
> + * + +
> + * ++ +
> + * +++
> + *
> + * @endverbatim
> + *
> + * Return the major axis rotation in degrees, clock wise from the logical north
> + * of the touch screen. In the example drawn above the value would be 315".
isn't " the unit for angle minutes? you'd need ° here, but since most of us
are handicapped on a US keyboard, just using "deg" is fine.
> + * This orientation might not be measured by the device, or only measured
> + * in coarse steps (e.g only indicating alignment with either of the axes).
> + *
> + * For events not of type @ref LIBINPUT_EVENT_TOUCH_DOWN, @ref
> + * LIBINPUT_EVENT_TOUCH_MOTION, this function returns 0.
> + *
> + * @note It is an application bug to call this function for events of type
> + * @ref LIBINPUT_EVENT_TOUCH_DOWN or @ref LIBINPUT_EVENT_TOUCH_MOTION.
> + *
> + * @param event The libinput touch event
> + * @return The current orientation value
> + */
> +double
> +libinput_event_touch_get_orientation(struct libinput_event_touch *event);
> +
> +/**
> + * @ingroup event_touch
> + *
> * @return The generic libinput_event of this event
> */
> struct libinput_event *
> diff --git a/src/libinput.sym b/src/libinput.sym
> index 773a7a4..60e4ebd 100644
> --- a/src/libinput.sym
> +++ b/src/libinput.sym
> @@ -147,3 +147,13 @@ global:
> libinput_device_config_tap_get_drag_lock_enabled;
> libinput_device_config_tap_get_default_drag_lock_enabled;
> } LIBINPUT_0.15.0;
> +
> +LIBINPUT_0.20.0 {
> +global:
> + libinput_event_touch_get_minor;
> + libinput_event_touch_get_major;
> + libinput_event_touch_get_minor_transformed;
> + libinput_event_touch_get_major_transformed;
> + libinput_event_touch_get_orientation;
> + libinput_event_touch_get_pressure;
> +} LIBINPUT_0.19.0;
> diff --git a/test/touch.c b/test/touch.c
> index b519613..1e47289 100644
> --- a/test/touch.c
> +++ b/test/touch.c
> @@ -33,6 +33,13 @@
> #include "libinput-util.h"
> #include "litest.h"
>
> +static inline void
> +fix_resolution(struct input_absinfo *info)
> +{
> + if (info->resolution == 0)
> + info->resolution = 1;
> +}
> +
> START_TEST(touch_frame_events)
> {
> struct litest_device *dev = litest_current_device();
> @@ -652,6 +659,157 @@ START_TEST(touch_initial_state)
> }
> END_TEST
>
> +START_TEST(touch_point_properties)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *ev;
> + struct libinput_event_touch *tev;
> + struct axis_replacement down_values[] = {
> + {ABS_MT_PRESSURE, 128},
> + {ABS_MT_ORIENTATION, 64},
> + {ABS_MT_TOUCH_MAJOR, 14},
> + {ABS_MT_TOUCH_MINOR, 8}};
> + struct axis_replacement move_values[] = {
> + {ABS_MT_ORIENTATION, 128},
> + {ABS_MT_TOUCH_MAJOR, 30}};
don't you need to terminate these with a -1 evcode?
> + struct input_absinfo const *orientation_info =
always use const <type>*, never <type> const*. they're the same in the end,
but <type> const* is not very common and easy to confuse with <type> * const
> + libevdev_get_abs_info(dev->evdev, ABS_MT_ORIENTATION);
please don't mix static initializes with function calls. there are some
execptions here (see litest_current_device()) so it's not as straightforward
to explain, most of this comes down to gut feeling.
> + struct input_absinfo const *pressure_info =
> + libevdev_get_abs_info(dev->evdev, ABS_MT_PRESSURE);
> + struct input_absinfo const *x_info =
> + libevdev_get_abs_info(dev->evdev, ABS_MT_POSITION_X);
> + struct input_absinfo const *y_info =
> + libevdev_get_abs_info(dev->evdev, ABS_MT_POSITION_Y);
> + double x_res, y_res, touch_minor_scale, touch_major_scale;
> + double expected_orientation;
> +
> + if (!orientation_info || !pressure_info || !x_info || !y_info ||
do we have touch devices without x/y? you should be able to filter those out
by passing LITEST_SINGLE_TOUCH into the last param of litest_add.
> + !libevdev_has_event_code(dev->evdev, EV_ABS, ABS_MT_TOUCH_MAJOR)||
> + !libevdev_has_event_code(dev->evdev, EV_ABS, ABS_MT_TOUCH_MINOR))
> + return;
> +
> + fix_resolution(x_info);
touch.c:692:17: warning: passing argument 1 of ‘fix_resolution’ discards
‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> + fix_resolution(y_info);
> +
> + expected_orientation = 90.0*64.0/orientation_info->maximum;
no spaces around math operators here but 5 lines down you use spaces?
> + x_res = x_info->resolution;
> + y_res = y_info->resolution;
> +
> + touch_major_scale = (x_res == y_res)
> + ? x_res
> + : (y_res*fabs(cos(M_PI * expected_orientation / 180.0)) +
> + x_res*fabs(sin(M_PI * expected_orientation / 180.0)));
> + touch_minor_scale = (x_res == y_res)
> + ? x_res
> + : (y_res*fabs(sin(M_PI * expected_orientation / 180.0)) +
> + x_res*fabs(cos(M_PI * expected_orientation / 180.0)));
move deg2rad to libinput-util.h and re-use it. And don't triple-nest
functions please, this makes my head spin just looking at it.
> +
> + litest_drain_events(li);
> +
> + litest_touch_down_extended(dev, 0, 5, 95, down_values);
> +
> + litest_wait_for_event_of_type(li, LIBINPUT_EVENT_TOUCH_DOWN, -1);
no, use the normal wait_for_event and then pass the type to is_touch_event,
otherwise you may accidentally discard events that shouldn't happen here.
> +
> + ev = libinput_get_event(li);
> + tev = litest_is_touch_event(ev, 0);
indentation
> +
> + ck_assert_double_eq(libinput_event_touch_get_pressure(tev), 128.0/
> + (pressure_info->maximum - pressure_info->minimum));
linebreak after the ',' - not halfway through an expression
> + ck_assert_double_eq(libinput_event_touch_get_orientation(tev),
> + expected_orientation);
> + ck_assert_double_eq(libinput_event_touch_get_major(tev), 14.0/touch_major_scale);
linebreak
> + ck_assert_double_eq(libinput_event_touch_get_minor(tev), 8.0/touch_minor_scale);
linebreak
> +
> + libinput_event_destroy(ev);
> +
> + litest_touch_move_extended(dev, 0, 5, 95, move_values);
> + litest_wait_for_event_of_type(li, LIBINPUT_EVENT_TOUCH_MOTION, -1);
> +
> +
> + expected_orientation = 90.0*128.0/orientation_info->maximum;
> + touch_major_scale = (x_res == y_res)
> + ? x_res
> + : (y_res*fabs(cos(M_PI * expected_orientation / 180.0)) +
> + x_res*fabs(sin(M_PI * expected_orientation / 180.0)));
tmp variables please
> + touch_minor_scale = (x_res == y_res)
> + ? x_res
> + : (y_res*fabs(sin(M_PI * expected_orientation / 180.0)) +
> + x_res*fabs(cos(M_PI * expected_orientation / 180.0)));
> +
> + ev = libinput_get_event(li);
> + tev = litest_is_touch_event(ev, 0);
indentation
> +
> + ck_assert_double_eq(libinput_event_touch_get_pressure(tev), 128.0/
> + (pressure_info->maximum - pressure_info->minimum));
> + ck_assert_double_eq(libinput_event_touch_get_orientation(tev),
> + expected_orientation);
> + ck_assert_double_eq(libinput_event_touch_get_major(tev), 30.0/touch_major_scale);
> + ck_assert_double_eq(libinput_event_touch_get_minor(tev), 8.0/touch_minor_scale);
see the comments above.
sigh, this is v5 of this patch and I'm still reviewing this for indentation
and coding style issues? not happy...
> +
> + libinput_event_destroy(ev);
> +}
> +END_TEST
> +
> +START_TEST(touch_point_no_minor_or_orienation)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *ev;
> + struct libinput_event_touch *tev;
> + struct axis_replacement down_values[] = {
> + {ABS_MT_PRESSURE, 43},
> + {ABS_MT_TOUCH_MAJOR, 23}};
> + struct axis_replacement move_values[] = {
> + {ABS_MT_TOUCH_MAJOR, 30}};
commenting here, because the other test already has enough comments: this is
made to look like a generic test, but really is very specific. if a touch
device doesn't support the given range this test will just fail with no hint
why. you'll need to put extra checks in to make sure that we fail with a
meaningful error message if we get a touch device that has e.g. a MAJOR max
ofo 20.
> +
> + struct input_absinfo const *orientation_info =
> + libevdev_get_abs_info(dev->evdev, ABS_MT_ORIENTATION);
> + struct input_absinfo const *pressure_info =
> + libevdev_get_abs_info(dev->evdev, ABS_MT_PRESSURE);
> + struct input_absinfo const *x_info =
> + libevdev_get_abs_info(dev->evdev, ABS_MT_POSITION_X);
> + double x_res;
> +
> + if (orientation_info || !pressure_info || !x_info ||
> + !libevdev_has_event_code(dev->evdev, EV_ABS, ABS_MT_TOUCH_MAJOR)||
> + libevdev_has_event_code(dev->evdev, EV_ABS, ABS_MT_TOUCH_MINOR))
> + return;
> +
> + fix_resolution(x_info);
> +
> + x_res = x_info->resolution;
> +
> + litest_drain_events(li);
> +
> + litest_touch_down_extended(dev, 0, 5, 95, down_values);
> +
> + litest_wait_for_event_of_type(li, LIBINPUT_EVENT_TOUCH_DOWN, -1);
> +
> + ev = libinput_get_event(li);
> + tev = litest_is_touch_event(ev, 0);
> +
> + ck_assert_double_eq(libinput_event_touch_get_orientation(tev), 0.0);
> + ck_assert_double_eq(libinput_event_touch_get_pressure(tev), 43.0 /
> + (pressure_info->maximum - pressure_info->minimum));
> + ck_assert_double_eq(libinput_event_touch_get_major(tev), 23.0/x_res);
move move those into an "expected_foo" temporary variable, that way it
becomes easier to read and easier to comment.
> + ck_assert_double_eq(libinput_event_touch_get_minor(tev), 0.0);
> +
> + libinput_event_destroy(ev);
> +
> + litest_touch_move_extended(dev, 0, 5, 95, move_values);
> + litest_wait_for_event_of_type(li, LIBINPUT_EVENT_TOUCH_MOTION, -1);
> +
> + ev = libinput_get_event(li);
> + tev = litest_is_touch_event(ev, 0);
> +
> + ck_assert_double_eq(libinput_event_touch_get_major(tev), 30.0/x_res);
> + ck_assert_double_eq(libinput_event_touch_get_minor(tev), 0.0);
> +
> + libinput_event_destroy(ev);
> +}
> +END_TEST
> +
> void
> litest_setup_tests(void)
> {
> @@ -676,6 +834,10 @@ litest_setup_tests(void)
> litest_add("touch:protocol a", touch_protocol_a_init, LITEST_PROTOCOL_A, LITEST_ANY);
> litest_add("touch:protocol a", touch_protocol_a_touch, LITEST_PROTOCOL_A, LITEST_ANY);
> litest_add("touch:protocol a", touch_protocol_a_2fg_touch, LITEST_PROTOCOL_A, LITEST_ANY);
> + litest_add("touch:properties", touch_point_properties, LITEST_TOUCH|LITEST_ELLIPSE,
> + LITEST_ANY);
> + litest_add("touch:propertiees", touch_point_no_minor_or_orienation,
typo
Cheers,
Peter
> + LITEST_TOUCH|LITEST_ELLIPSE, LITEST_ANY);
>
> litest_add_ranged("touch:state", touch_initial_state, LITEST_TOUCH, LITEST_PROTOCOL_A, &axes);
> }
> --
> 2.1.4
>
More information about the wayland-devel
mailing list