On Mon, 9 Apr 2018 12:12:49 +1000
Peter Hutterer <peter.hutte...@who-t.net> wrote:

> 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(-)
> > 

> > -/**
> > - * 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?")

Ok.

> >  {
> >     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.

That's a good idea indeed. I think I'll re-spin with that.


> > +            const struct timespec *time, int touch_id,
> > +            double x, double y, double norm_x, double norm_y,
> > +            int touch_type)
> > +{

> > -   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

Yup.


Thanks,
pq

Attachment: pgp9ey8uT4XQN.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to