Xen project Mailing List

Re: [Xen-devel] [RFC] kbdif: add multi-touch support

On Wed, 4 Jan 2017, Oleksandr Andrushchenko wrote: > First of all, thank you for comments You are welcome :-) > On 01/04/2017 03:03 AM, Stefano Stabellini wrote: > > On Tue, 3 Jan 2017, Oleksandr Andrushchenko wrote: > > > From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx> > > > > > > Signed-off-by: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx> > > > --- > > > xen/include/public/io/kbdif.h | 64 > > > +++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 64 insertions(+) > > > > > > diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h > > > index 2d2aebd..ad94b53 100644 > > > --- a/xen/include/public/io/kbdif.h > > > +++ b/xen/include/public/io/kbdif.h > > > @@ -45,6 +45,19 @@ > > > */ > > > #define XENKBD_TYPE_POS 4 > > > +/* > > > + * Multi-touch event > > > + * Capable backend sets feature-multi-touch in xenstore. > > > + * Frontend requests feature by setting request-multi-touch in xenstore. > > > + * Frontend supports up to XENKBD_MT_NUM_DEV virtual multi-touch input > > > devices, > > > + * configured by the backend in xenstore under mt-%d folder, %d being > > > + * a sequential number of the virtual input device: > > > + * o num-contacts - number of simultaneous touches supported > > > + * o width - width of the touch area in pixels > > > + * o height - height of the touch area in pixels > > Please write down the max width and height supported by the protocol, > > keeping in mind that motion events below use int32_t for coordinates. > I will put "width(height) of the... in pixels, 32-bit signed integer" I don't think I understand what you wrote here. To clarify, I meant that the doc should say what is the theoretical maximum for width and height, for example INT32_MAX. > > Are there any benefits of this compared to just setting up multiple kbd > > connections, one per multi-touch device? The only benefit I can think of > > is saving few pages. > Well, not only saving a few pages, but somewhat > simplifying handling of the protocol on both back and > front ends. But, you are probably right as current > protocol is capable of holding > (XENKBD_IN_RING_SIZE - XENKBD_IN_RING_OFFS) / XENKBD_IN_EVENT_SIZE == > (2048 - 1024) / 40 = 25 incoming events which may not be > enough for multiple mt devices delivering hundreds of > events per second. > Will set-up dedicated rings per mt device then. I think you'll find it is going to be simpler and faster for little extra memory costs. > Will also remove > uint8_t dev_idx; /* index of the multi-touch device */ > as every device will have its own ring. Right > Will extend XenStore configuration for mt devices with: > o page-ref (?) > o page-gref > o event-channel > as it is done by the Linux xen-kbdfront driver. > > BTW, is there any reason we need "page-ref"? My understanding is > that the pair of page-gref + event-channel is enough to establish > and uniquely identify the connection. It's page-gref that is superfluous. I don't know why the Linux frontend writes it, in fact the QEMU backend doesn't even read it. > > > +/* Sent when a new touch is made: touch is assigned a unique contact > > > + * ID, sent with this and consequent events related to this touch. > > > + * Contact ID will be reused after XENKBD_MT_EV_UP event. > > Will be reused or can be reused? > I would probably say "may be reused" as it depends on how > and if new touches/contacts are made > > Please provide an example of a Contact > > ID lifecycle. > Do you want it to be described in the protocol or just here? > If the latter then, for example, as Wayland documentation > describes it [1]: > "Touch interactions can consist of one or more contacts. > For each contact, a series of events is generated, starting > with a down event, followed by zero or more motion events, > and ending with an up event. Events relating to the same > contact point can be identified by the ID of the sequence." > So, if there is contact/touch a free Contact ID is assigned to > this contact(sequence) and it is "released" when contact is > done, e.g. after up event. From this point contact ID may be > reused. This is very useful info for people not familiar with Wayland. Please write this in the doc, and link to the Wayland documentation. > I was basing the mt additions to the protocol on Wayland [1], > Linux [2] and Windows [3] multi-touch support, so those may > better explain the idea > > What is the max Contact ID? > > > /* contact ID, [0; num-contacts - 1] */ > num-contacts - number of simultaneous touches supported > > > + */ > > > +#define XENKBD_MT_EV_DOWN 0 > > > +/* Touch point has been released */ > > > +#define XENKBD_MT_EV_UP 1 > > > +/* Touch point has changed its coordinate(s) */ > > > +#define XENKBD_MT_EV_MOTION 2 > > > +/* Input synchronization event: shows end of a set of events > > > + * which logically belong together. > > > + */ > > > +#define XENKBD_MT_EV_SYN 3 > > > +/* Touch point has changed its shape. Shape is approximated by an ellipse > > > + * through the major and minor axis lengths: major is the longer diameter > > > + * of the ellipse and minor is the shorter one. Center of the ellipse is > > > + * reported via XENKBD_MT_EV_DOWN/XENKBD_MT_EV_MOTION events. > > > + */ > > > +#define XENKBD_MT_EV_SHAPE 4 > > > +/* Touch point's shape has changed its orientation: calculated as a > > > clockwise > > > + * angle between the major axis of the ellipse and positive Y axis in > > > degrees, > > > + * [-180; +180]. > > > + */ > > > +#define XENKBD_MT_EV_ORIENT 5 > > > + > > > +struct xenkbd_mtouch { > > > + uint8_t type; /* XENKBD_TYPE_MTOUCH */ > > > + uint8_t dev_idx; /* index of the multi-touch device */ > > > + uint8_t event_type; /* XENKBD_MT_EV_??? */ > > > + uint8_t reserved; /* reserved for the future use */ > > > + int32_t contact_id; /* contact ID, [0; num-contacts - 1] */ > > > + union { > > > + /* XENKBD_MT_EV_DOWN/XENKBD_MT_EV_MOTION */ > > > + struct { > > > + int32_t abs_x; /* absolute X position, pixels */ > > > + int32_t abs_y; /* absolute Y position, pixels */ > > > + } pos; > > > + /* XENKBD_MT_EV_SHAPE */ > > > + struct { > > > + uint32_t major; /* length of the major axis, pixels */ > > > + uint32_t minor; /* length of the minor axis, pixels */ > > > + } shape; > > > + /* XENKBD_MT_EV_ORIENT */ > > > + uint16_t orientation; /* clockwise angle of the major axis */ > > > + } u; > > > +}; > > Need a binary representation. > > > Could you please elaborate more on this, this is not > clear to me. Sure, sorry. I meant something like: 0 1 2 3 4 +----+----+----+----+ |type|dev_|even|res | +----+----+----+----+ I know it is cumbersome, and I might not be a fun of it myself, but it is required for new Xen protocol changes. I wrote all of the binary representations manually but if you find a tool to do it, please let me know :-) > > > #define XENKBD_IN_EVENT_SIZE 40 > > > union xenkbd_in_event > > > @@ -76,6 +139,7 @@ union xenkbd_in_event > > > struct xenkbd_motion motion; > > > struct xenkbd_key key; > > > struct xenkbd_position pos; > > > + struct xenkbd_mtouch mtouch; > > > char pad[XENKBD_IN_EVENT_SIZE]; > > > }; > > > > > > -- > > > 2.7.4 > > Related question: what if I also extend existing > xenkbd_motion/key/position by adding uint32(8?)_t dev_idx > at the bottom of the structures, e.g. > > struct xenkbd_motion { > uint8_t type; /* XENKBD_TYPE_MOTION */ > int32_t rel_x; /* relative X motion */ > int32_t rel_y; /* relative Y motion */ > int32_t rel_z; /* relative Z motion (wheel) */ > uint32_t dev_idx; or uint8_t dev_idx; > }; > > This way: > 1. Existing fronts/backs will not be affected > 2. New fronts/backs may use this field to serve > multiple keyboards and/or pointer devices via single ring > > Should I make this change as a dedicated patch or do you think > it can be in the same one for mt? I thought we decided to use one ring per device? If so, why would you want to introduce dev_idx in xenkbd_motion? Also, where would the list of available devices be described? The current xenstore info assumes one ring per device. In any case, it should be separate from the mt discussion. > Thank you, > Oleksandr > [1] > https://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml#n2196 > [2] https://www.kernel.org/doc/Documentation/input/multi-touch-protocol.txt > [3] > https://msdn.microsoft.com/en-us/library/jj151564(v=vs.85).aspx#optional_hid_usages > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.