On Fri, Mar 23, 2018 at 02:00:56PM +0200, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> 
> notify_touch_cal() is an extended form of notify_touch(), adding
> normalized touch coordinates which are necessary for calibrating a
> touchscreen.
> 
> It would be possible to invert the transformation and convert from
> global coordinates to normalized device coordinates in input.c without
> adding this API, but this way it is more robust against code changes.
> 
> Recovering normalized device coordinates is necessary because libinput
> calibration matrix must be given in normalized units, and it would be
> difficult to compute otherwise. Libinput API does not offer normalized
> coordinates directly either, but those can be fetched by pretending the
> output resolution is 1x1.
> 
> Anticipating touch calibration mode, the old notify_touch() is renamed
> into a private process_touch_normal(), and the new notify_touch_cal()
> delegates to it.
> 
> Co-developed by Louis-Francis and Pekka.
> 
> Cc: Louis-Francis Ratté-Boulianne <l...@collabora.com>
> Signed-off-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
> ---
>  libweston/compositor.h      | 21 +++++++++++++++-
>  libweston/input.c           | 60 
> ++++++++++++++++++++++++++++++++++++---------
>  libweston/libinput-device.c | 11 ++++++++-
>  3 files changed, 79 insertions(+), 13 deletions(-)
> 
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index b992b7eb..a1264e5a 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -1532,9 +1532,28 @@ notify_keyboard_focus_in(struct weston_seat *seat, 
> struct wl_array *keys,
>  void
>  notify_keyboard_focus_out(struct weston_seat *seat);
>  
> +/* an arbitrary unlikely value */
> +#define WESTON_INVALID_TOUCH_COORDINATE ((double)777e+7)
> +
>  void
> +notify_touch_cal(struct weston_touch_device *device,
> +              const struct timespec *time, int touch_id,
> +              double x, double y,
> +              double norm_x, double norm_y, int touch_type);
> +
> +/** Feed in touch down, motion, and up events, non-calibratable device.
> + *
> + * @sa notify_touch_cal
> + */
> +static inline void
>  notify_touch(struct weston_touch_device *device, const struct timespec *time,
> -          int touch_id, double x, double y, int touch_type);
> +          int touch_id, double x, double y, int touch_type)
> +{
> +     notify_touch_cal(device, time, touch_id, x, y,
> +                      WESTON_INVALID_TOUCH_COORDINATE,
> +                      WESTON_INVALID_TOUCH_COORDINATE, touch_type);
> +}
> +
>  void
>  notify_touch_frame(struct weston_touch_device *device);
>  
> diff --git a/libweston/input.c b/libweston/input.c
> index bd7a9167..1658422c 100644
> --- a/libweston/input.c
> +++ b/libweston/input.c
> @@ -2336,17 +2336,10 @@ weston_touch_set_focus(struct weston_touch *touch, 
> struct weston_view *view)
>       touch->focus = view;
>  }
>  
> -/**
> - * notify_touch - emulates button touches and notifies surfaces accordingly.
> - *
> - * It assumes always the correct cycle sequence until it gets here: 
> touch_down
> - * → touch_update → ... → touch_update → touch_end. The driver is responsible
> - * for sending along such order.
> - *
> - */
> -WL_EXPORT void
> -notify_touch(struct weston_touch_device *device, const struct timespec *time,
> -          int touch_id, double double_x, double double_y, int touch_type)
> +static void
> +process_touch_normal(struct weston_touch_device *device,
> +                  const struct timespec *time, int touch_id,
> +                  double double_x, double double_y, int touch_type)

IMO just process_touch() is better, otherwise it's more confusing ("wait?
why is this normal? and what's not normal? or is this an abbreviation of
normalized?")

>  {
>       struct weston_touch *touch = device->aggregate;
>       struct weston_touch_grab *grab = device->aggregate->grab;
> @@ -2423,6 +2416,51 @@ notify_touch(struct weston_touch_device *device, const 
> struct timespec *time,
>       }
>  }
>  
> +/** Feed in touch down, motion, and up events, calibratable device.
> + *
> + * It assumes always the correct cycle sequence until it gets here: 
> touch_down
> + * → touch_update → ... → touch_update → touch_end. The driver is responsible
> + * for sending along such order.
> + *
> + * \param device The physical device that generated the event.
> + * \param time The event timestamp.
> + * \param touch_id ID for the touch point of this event (multi-touch).
> + * \param double_x X coordinate in compositor global space.
> + * \param double_y Y coordinate in compositor global space.
> + * \param norm_x Normalized device X coordinate in calibration space.
> + * \param norm_y Normalized device Y coordinate in calibration space.
> + * \param touch_type Either WL_TOUCH_DOWN, WL_TOUCH_UP, or WL_TOUCH_MOTION.
> + *
> + * Coordinates double_x and double_y are used for normal operation.
> + *
> + * Coordinates norm_x and norm_y are only used for touch device calibration.
> + * If and only if the weston_touch_device does not support calibrating,
> + * norm_x and norm_y must be WESTON_INVALID_TOUCH_COORDINATE.
> + *
> + * The calibration space is the normalized coordinate space
> + * [0.0, 1.0]×[0.0, 1.0] of the weston_touch_device. This is assumed to
> + * map to the similar normalized coordinate space of the associated
> + * weston_output.
> + */
> +WL_EXPORT void
> +notify_touch_cal(struct weston_touch_device *device,

this should be notify_touch_normalized() because that's what the extra
values are. That calibration affects those is just an implementation detail.

Also, at this point I can only say creating structs for each coordinate type
in libinput has helped greatly in understanding what exactly you're dealing
with at any point in time. see libinput-private.h:

/* A coordinate pair in device coordinates */
struct device_coords {
        int x, y;
};

/* A dpi-normalized coordinate pair */
struct normalized_coords {
        double x, y;
};

/* A pair of coordinates normalized to a [0,1] or [-1, 1] range */
struct normalized_range_coords {
        double x, y;
};


etc. These are passed through the various functions, so the compiler will
tell you when you're passing a device coordinate into something that should
take a [0, 1] normalized range. I cannot recommend this enough when you're
dealing with more than one coordinate system.

> +              const struct timespec *time, int touch_id,
> +              double x, double y, double norm_x, double norm_y,
> +              int touch_type)
> +{
> +     if (touch_type != WL_TOUCH_UP) {
> +             if (weston_touch_device_can_calibrate(device)) {
> +                     assert(norm_x != WESTON_INVALID_TOUCH_COORDINATE);
> +                     assert(norm_y != WESTON_INVALID_TOUCH_COORDINATE);
> +             } else {
> +                     assert(norm_x == WESTON_INVALID_TOUCH_COORDINATE);
> +                     assert(norm_y == WESTON_INVALID_TOUCH_COORDINATE);
> +             }
> +     }
> +
> +     process_touch_normal(device, time, touch_id, x, y, touch_type);
> +}
> +
>  WL_EXPORT void
>  notify_touch_frame(struct weston_touch_device *device)
>  {
> diff --git a/libweston/libinput-device.c b/libweston/libinput-device.c
> index 9f5793aa..6fd49572 100644
> --- a/libweston/libinput-device.c
> +++ b/libweston/libinput-device.c
> @@ -417,6 +417,8 @@ handle_touch_with_coords(struct libinput_device 
> *libinput_device,
>               libinput_device_get_user_data(libinput_device);
>       double x;
>       double y;
> +     double norm_x;
> +     double norm_y;
>       uint32_t width, height;
>       struct timespec time;
>       int32_t slot;
> @@ -436,7 +438,14 @@ handle_touch_with_coords(struct libinput_device 
> *libinput_device,
>       weston_output_transform_coordinate(device->output,
>                                          x, y, &x, &y);
>  
> -     notify_touch(device->touch_device, &time, slot, x, y, touch_type);
> +     if (weston_touch_device_can_calibrate(device->touch_device)) {
> +             norm_x = libinput_event_touch_get_x_transformed(touch_event, 1);
> +             norm_y =  libinput_event_touch_get_y_transformed(touch_event, 
> 1);

one space to many

Cheers,
   Peter

> +             notify_touch_cal(device->touch_device, &time, slot, x, y,
> +                              norm_x, norm_y, touch_type);
> +     } else {
> +             notify_touch(device->touch_device, &time, slot, x, y, 
> touch_type);
> +     }
>  }
>  
>  static void
> -- 
> 2.16.1
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to