[PATCH libinput 1/2] Replace output screen size callback with transform helpers

Peter Hutterer peter.hutterer at who-t.net
Wed Jan 29 19:02:15 PST 2014


On Wed, Jan 29, 2014 at 09:33:11PM +0100, Jonas Ådahl wrote:
> Instead of automatically transforming absolute coordinates of touch and
> pointer events to screen coordinates, the user now uses the corresponding
> transform helper function. This means the coordinates returned by
> libinput_event_pointer_get_absolute_x(),
> libinput_event_pointer_get_absolute_y(), libinput_touch_get_x() and
> libinput_touch_get_y() has changed from being in output screen coordinate
> space to being in device specific coordinate space.
> 
> For example, where one before would call libinput_event_touch_get_x(event),
> one now calls libinput_event_touch_get_x_transformed(event, output_width).
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>  src/evdev.c    |  54 ++++++++++--------------
>  src/evdev.h    |  10 +++++
>  src/libinput.c |  44 ++++++++++++++++++++
>  src/libinput.h | 128 +++++++++++++++++++++++++++++++++++++++++++++++++--------
>  test/litest.c  |  11 -----
>  5 files changed, 186 insertions(+), 61 deletions(-)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 46bd35a..cb83a1f 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -86,6 +86,24 @@ transform_absolute(struct evdev_device *device, int32_t *x, int32_t *y)
>  	}
>  }
>  
> +li_fixed_t
> +evdev_device_transform_x(struct evdev_device *device,
> +			 li_fixed_t x,
> +			 uint32_t width)
> +{
> +	return (x - device->abs.min_x) * width /
> +		(device->abs.max_x - device->abs.min_x);
> +}
> +
> +li_fixed_t
> +evdev_device_transform_y(struct evdev_device *device,
> +			 li_fixed_t y,
> +			 uint32_t height)
> +{
> +	return (y - device->abs.min_y) * height /
> +		(device->abs.max_y - device->abs.min_y);

you're mixing coordinate systems here, x and y are in fixed_t but
abs.min/max is in normal integers. that breaks if you have a non-zero min.
You'll need to convert the rest to li_fixed_t too if you want to keep the
integer division.

also, should we add a non-zero min for width and height to scale to a screen
not the top/left-most? The compositor can just add it afterwards, but 
it would have to convert to fixed_t as well:

    x = libinput_event_touch_get_x_transformed(event, screen_width);
    x += li_fixed_from_int(screen_offset);

which is more error prone than something like:

    x = libinput_event_touch_get_x_transformed(event, screen_offset_x, screen_width);

also, is it likely that the caller always has the screen dimensions handy
when it comes to processing events? or would an config-style approach work
better:

   libinput_device_set_output_dimensions(device, xoff, yoff, width, height);
   ...
   x = libinput_event_touch_get_x_transformed(event);
   y = libinput_event_touch_get_y_transformed(event);

I also suspect that the device-specific dimensions will be rather useless if
we don't have a call to get the min/max from each device. which I should be
focusing on real soon :)

Cheers,
   Peter

> +}
> +
>  static void
>  evdev_flush_pending_event(struct evdev_device *device, uint32_t time)
>  {
> @@ -242,16 +260,6 @@ evdev_process_touch(struct evdev_device *device,
>  		    struct input_event *e,
>  		    uint32_t time)
>  {
> -	struct libinput *libinput = device->base.seat->libinput;
> -	int screen_width;
> -	int screen_height;
> -
> -	libinput->interface->get_current_screen_dimensions(
> -		&device->base,
> -		&screen_width,
> -		&screen_height,
> -		libinput->user_data);
> -
>  	switch (e->code) {
>  	case ABS_MT_SLOT:
>  		evdev_flush_pending_event(device, time);
> @@ -267,16 +275,12 @@ evdev_process_touch(struct evdev_device *device,
>  			device->pending_event = EVDEV_ABSOLUTE_MT_UP;
>  		break;
>  	case ABS_MT_POSITION_X:
> -		device->mt.slots[device->mt.slot].x =
> -			(e->value - device->abs.min_x) * screen_width /
> -			(device->abs.max_x - device->abs.min_x);
> +		device->mt.slots[device->mt.slot].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].y =
> -			(e->value - device->abs.min_y) * screen_height /
> -			(device->abs.max_y - device->abs.min_y);
> +		device->mt.slots[device->mt.slot].y = e->value;
>  		if (device->pending_event == EVDEV_NONE)
>  			device->pending_event = EVDEV_ABSOLUTE_MT_MOTION;
>  		break;
> @@ -287,28 +291,14 @@ static inline void
>  evdev_process_absolute_motion(struct evdev_device *device,
>  			      struct input_event *e)
>  {
> -	struct libinput *libinput = device->base.seat->libinput;
> -	int screen_width;
> -	int screen_height;
> -
> -	libinput->interface->get_current_screen_dimensions(
> -		&device->base,
> -		&screen_width,
> -		&screen_height,
> -		libinput->user_data);
> -
>  	switch (e->code) {
>  	case ABS_X:
> -		device->abs.x =
> -			(e->value - device->abs.min_x) * screen_width /
> -			(device->abs.max_x - device->abs.min_x);
> +		device->abs.x = e->value;
>  		if (device->pending_event == EVDEV_NONE)
>  			device->pending_event = EVDEV_ABSOLUTE_MOTION;
>  		break;
>  	case ABS_Y:
> -		device->abs.y =
> -			(e->value - device->abs.min_y) * screen_height /
> -			(device->abs.max_y - device->abs.min_y);
> +		device->abs.y = e->value;
>  		if (device->pending_event == EVDEV_NONE)
>  			device->pending_event = EVDEV_ABSOLUTE_MOTION;
>  		break;
> diff --git a/src/evdev.h b/src/evdev.h
> index 58ae552..37c32e5 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -146,6 +146,16 @@ int
>  evdev_device_has_capability(struct evdev_device *device,
>  			    enum libinput_device_capability capability);
>  
> +li_fixed_t
> +evdev_device_transform_x(struct evdev_device *device,
> +			 li_fixed_t x,
> +			 uint32_t width);
> +
> +li_fixed_t
> +evdev_device_transform_y(struct evdev_device *device,
> +			 li_fixed_t y,
> +			 uint32_t height);
> +
>  void
>  evdev_device_remove(struct evdev_device *device);
>  
> diff --git a/src/libinput.c b/src/libinput.c
> index a77f165..cfce2c5 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -245,6 +245,28 @@ libinput_event_pointer_get_absolute_y(
>  	return event->y;
>  }
>  
> +LIBINPUT_EXPORT li_fixed_t
> +libinput_event_pointer_get_absolute_x_transformed(
> +	struct libinput_event_pointer *event,
> +	uint32_t width)
> +{
> +	struct evdev_device *device =
> +		(struct evdev_device *) event->base.device;
> +
> +	return evdev_device_transform_x(device, event->x, width);
> +}
> +
> +LIBINPUT_EXPORT li_fixed_t
> +libinput_event_pointer_get_absolute_y_transformed(
> +	struct libinput_event_pointer *event,
> +	uint32_t height)
> +{
> +	struct evdev_device *device =
> +		(struct evdev_device *) event->base.device;
> +
> +	return evdev_device_transform_y(device, event->y, height);
> +}
> +
>  LIBINPUT_EXPORT uint32_t
>  libinput_event_pointer_get_button(
>  	struct libinput_event_pointer *event)
> @@ -295,6 +317,28 @@ libinput_event_touch_get_x(
>  }
>  
>  LIBINPUT_EXPORT li_fixed_t
> +libinput_event_touch_get_x_transformed(
> +	struct libinput_event_touch *event,
> +	uint32_t width)
> +{
> +	struct evdev_device *device =
> +		(struct evdev_device *) event->base.device;
> +
> +	return evdev_device_transform_x(device, event->x, width);
> +}
> +
> +LIBINPUT_EXPORT li_fixed_t
> +libinput_event_touch_get_y_transformed(
> +	struct libinput_event_touch *event,
> +	uint32_t height)
> +{
> +	struct evdev_device *device =
> +		(struct evdev_device *) event->base.device;
> +
> +	return evdev_device_transform_y(device, event->y, height);
> +}
> +
> +LIBINPUT_EXPORT li_fixed_t
>  libinput_event_touch_get_y(
>  	struct libinput_event_touch *event)
>  {
> diff --git a/src/libinput.h b/src/libinput.h
> index ddaeb73..8d347b9 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -395,16 +395,19 @@ libinput_event_pointer_get_dy(
>  /**
>   * @ingroup event_pointer
>   *
> - * Return the absolute x coordinate of the device, scaled to screen
> - * coordinates.
> - * The axes' positive direction is device-specific. For pointer events that
> - * are not of type LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE, this function
> - * returns 0.
> + * Return the current absolute x coordinate of the pointer event.
> + *
> + * The coordinate is in a device specific coordinate space; to get the
> + * corresponding output screen coordinate, use
> + * libinput_event_pointer_get_x_transformed().
> + *
> + * For pointer events that are not of type
> + * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE, this function returns 0.
>   *
>   * @note It is an application bug to call this function for events other than
>   * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE.
>   *
> - * @return the current absolute x coordinate scaled to screen coordinates.
> + * @return the current absolute x coordinate
>   */
>  li_fixed_t
>  libinput_event_pointer_get_absolute_x(
> @@ -413,15 +416,19 @@ libinput_event_pointer_get_absolute_x(
>  /**
>   * @ingroup event_pointer
>   *
> - * Return the absolute y coordinate of the device, scaled to screen coordinates.
> - * The axes' positive direction is device-specific. For pointer events that
> - * are not of type LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE, this function
> - * returns 0.
> + * Return the current absolute y coordinate of the pointer event.
> + *
> + * The coordinate is in a device specific coordinate space; to get the
> + * corresponding output screen coordinate, use
> + * libinput_event_pointer_get_y_transformed().
> + *
> + * For pointer events that are not of type
> + * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE, this function returns 0.
>   *
>   * @note It is an application bug to call this function for events other than
>   * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE.
>   *
> - * @return the current absolute y coordinate scaled to screen coordinates.
> + * @return the current absolute y coordinate
>   */
>  li_fixed_t
>  libinput_event_pointer_get_absolute_y(
> @@ -430,6 +437,50 @@ libinput_event_pointer_get_absolute_y(
>  /**
>   * @ingroup event_pointer
>   *
> + * Return the current absolute x coordinate of the pointer event, transformed to
> + * screen coordinates.
> + *
> + * For pointer events that are not of type
> + * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE, the return value of this function is
> + * undefined.
> + *
> + * @note It is an application bug to call this function for events other than
> + * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE.
> + *
> + * @param event The libinput pointer event
> + * @param width The current output screen width
> + * @return the current absolute x coordinate transformed to a screen coordinate
> + */
> +li_fixed_t
> +libinput_event_pointer_get_absolute_x_transformed(
> +	struct libinput_event_pointer *event,
> +	uint32_t width);
> +
> +/**
> + * @ingroup event_pointer
> + *
> + * Return the current absolute y coordinate of the pointer event, transformed to
> + * screen coordinates.
> + *
> + * For pointer events that are not of type
> + * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE, the return value of this function is
> + * undefined.
> + *
> + * @note It is an application bug to call this function for events other than
> + * LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE.
> + *
> + * @param event The libinput pointer event
> + * @param height The current output screen height
> + * @return the current absolute y coordinate transformed to a screen coordinate
> + */
> +li_fixed_t
> +libinput_event_pointer_get_absolute_y_transformed(
> +	struct libinput_event_pointer *event,
> +	uint32_t height);
> +
> +/**
> + * @ingroup event_pointer
> + *
>   * Return the button that triggered this event.
>   * For pointer events that are not of type LIBINPUT_EVENT_POINTER_BUTTON,
>   * this function returns 0.
> @@ -531,9 +582,16 @@ libinput_event_touch_get_slot(
>  /**
>   * @ingroup event_touch
>   *
> + * Return the current absolute x coordinate of the touch event.
> + *
> + * The coordinate is in a device specific coordinate space; to get the
> + * corresponding output screen coordinate, use
> + * libinput_event_touch_get_x_transformed().
> + *
>   * @note this function should not be called for LIBINPUT_EVENT_TOUCH_FRAME.
>   *
> - * @return the absolute X coordinate on this touch device, scaled to screen coordinates.
> + * @param event The libinput touch event
> + * @return the current absolute x coordinate
>   */
>  li_fixed_t
>  libinput_event_touch_get_x(
> @@ -542,9 +600,16 @@ libinput_event_touch_get_x(
>  /**
>   * @ingroup event_touch
>   *
> + * Return the current absolute y coordinate of the touch event.
> + *
> + * The coordinate is in a device specific coordinate space; to get the
> + * corresponding output screen coordinate, use
> + * libinput_event_touch_get_y_transformed().
> + *
>   * @note this function should not be called for LIBINPUT_EVENT_TOUCH_FRAME.
>   *
> - * @return the absolute X coordinate on this touch device, scaled to screen coordinates.
> + * @param event The libinput touch event
> + * @return the current absolute y coordinate
>   */
>  li_fixed_t
>  libinput_event_touch_get_y(
> @@ -553,6 +618,38 @@ libinput_event_touch_get_y(
>  /**
>   * @ingroup event_touch
>   *
> + * Return the current absolute x coordinate of the touch event, transformed to
> + * screen coordinates.
> + *
> + * @note this function should not be called for LIBINPUT_EVENT_TOUCH_FRAME.
> + *
> + * @param event The libinput touch event
> + * @param width The current output screen width
> + * @return the current absolute x coordinate transformed to a screen coordinate
> + */
> +li_fixed_t
> +libinput_event_touch_get_x_transformed(struct libinput_event_touch *event,
> +				       uint32_t width);
> +
> +/**
> + * @ingroup event_touch
> + *
> + * Return the current absolute y coordinate of the touch event, transformed to
> + * screen coordinates.
> + *
> + * @note this function should not be called for LIBINPUT_EVENT_TOUCH_FRAME.
> + *
> + * @param event The libinput touch event
> + * @param height The current output screen height
> + * @return the current absolute y coordinate transformed to a screen coordinate
> + */
> +li_fixed_t
> +libinput_event_touch_get_y_transformed(struct libinput_event_touch *event,
> +				       uint32_t height);
> +
> +/**
> + * @ingroup event_touch
> + *
>   * @note this function should not be called for LIBINPUT_EVENT_TOUCH_FRAME.
>   *
>   * @return the type of touch that occured on the device
> @@ -586,11 +683,6 @@ struct libinput_interface {
>  	 * libinput_create_from_udev()
>  	 */
>  	void (*close_restricted)(int fd, void *user_data);
> -
> -	void (*get_current_screen_dimensions)(struct libinput_device *device,
> -					      int *width,
> -					      int *height,
> -					      void *user_data);
>  };
>  
>  /**
> diff --git a/test/litest.c b/test/litest.c
> index 5235e3c..216e1a0 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -318,20 +318,9 @@ close_restricted(int fd, void *userdata)
>  	close(fd);
>  }
>  
> -static void
> -get_current_screen_dimensions(struct libinput_device *device,
> -			      int *width,
> -			      int *height,
> -			      void *user_data)
> -{
> -	*width = 1024;
> -	*height = 768;
> -}
> -
>  const struct libinput_interface interface = {
>  	.open_restricted = open_restricted,
>  	.close_restricted = close_restricted,
> -	.get_current_screen_dimensions = get_current_screen_dimensions,
>  };
>  
>  struct litest_device *
> -- 
> 1.8.3.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 


More information about the wayland-devel mailing list