[RFC wayland] protocol: Add high-resolution wl_touch timestamp event

Fri Nov 24 11:59:10 UTC 2017

On Tue, 21 Nov 2017 15:45:48 +0200
Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:

> wl_touch events currently use a 32-bit timestamp with millisecond
> resolution. In some cases, notably latency measurements, this resolution
> is too coarse-grained to be useful.
> 
> This protocol update adds a wl_touch.timestamp event, which is emitted
> just before an up, motion or down touch event. The timestamp event
> contains a high-resolution, and ideally higher-accuracy, version of the
> 'time' argument of the up/motion/down event that follows.
> 
> From a client implementation perspective clients can just ignore this event if
> they don't care about the improved resolution. Clients that care just need to
> keep track of the last timestamp event they receive and associate it with the
> next up/motion/down event that arrives.
> 
> Some points for discussion:
> 
> 1. Should there be a request to explicitly enable/disable the timestamp event
>    (to reduce extra event overhead when improved timestamps are not needed)?

Hi,

I would tend to believe it makes no difference. Motion events could be
fairly high rate in theory, so would be good to hear if anyone thinks
otherwise, but I think it's harmless.

> 2. Should we introduce similar timestamp events for keyboard and pointer?

No, unless someone actually has a use for them. That brings the
question, why are you only interested in touch timestamps and not
keyboard and pointer?

Why are keyboard and pointer latency not interesting to you?

> 3. Should this be a separate protocol instead of a core update (being separate
>    would also take care of point (1))?

That's a good question. I don't think I have obvious technical reasons
to pick one approach over the other. I can think of benefits for it
being a separate extension, though:

- The overhead you mentioned, making this optional.

- If a compositor actually cannot get any more precise timestamps than
  the core protocol already delivers, then it could avoid sending
  useless events. Vice versa, having the extension advertised would
  give clients the promise that there could be more precise timestamps
  available.

- Extensibility. Say, maybe in the future we want input timestamps in a
  specific clock domain. That could be nicely added if this is a
  separate extension, and less so if this was a core augmentation.

However, if precise input timestamps were a separate extension, we
would need to think if different wl_seats could have a different clock
domain. That could be the case with remote input, but OTOH remote input
should be converted into a local clock if possible.

I would slightly lean on a separate extension that is worded such that
the timestamp events are (subscribed) per wl_seat. It depends a little
bit on whether we want keyboard and pointer timestamps as well.

IOW, a client would maintain a timestamp field for each wl_seat, so
that different seats could, if necessary, have different clock domains.
This is currently somewhat undefined in the core protocol, but I think
it would be reasonable.

> A proof of concept implementation for weston can be found at:
> 
> https://git.collabora.com/cgit/user/alf/weston.git/log/?h=wl-touch-v7

I took a quick peek and the implementation looks exactly like I would expect.

> 
> Note that the weston implementation depends on a patchset that changes weston
> to use higher resolution timestamps internally. The patchset been submitted
> for review at:
> 
> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035851.html
> 
> Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> ---
>  protocol/wayland.xml | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index aabc7ae..b01811e 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -1657,7 +1657,7 @@
>      </request>
>     </interface>
>  
> -  <interface name="wl_seat" version="6">
> +  <interface name="wl_seat" version="7">
>      <description summary="group of input devices">
>        A seat is a group of keyboards, pointer and touch devices. This
>        object is published as a global during start up, or when such a
> @@ -1766,7 +1766,7 @@
>  
>    </interface>
>  
> -  <interface name="wl_pointer" version="6">
> +  <interface name="wl_pointer" version="7">
>      <description summary="pointer input device">
>        The wl_pointer interface represents one or more input devices,
>        such as mice, which control the pointer location and pointer_focus
> @@ -2089,7 +2089,7 @@
>      </event>
>    </interface>
>  
> -  <interface name="wl_keyboard" version="6">
> +  <interface name="wl_keyboard" version="7">
>      <description summary="keyboard input device">
>        The wl_keyboard interface represents one or more keyboards
>        associated with a seat.
> @@ -2200,7 +2200,7 @@
>      </event>
>    </interface>
>  
> -  <interface name="wl_touch" version="6">
> +  <interface name="wl_touch" version="7">
>      <description summary="touchscreen input device">
>        The wl_touch interface represents a touchscreen
>        associated with a seat.
> @@ -2342,6 +2342,25 @@
>        <arg name="id" type="int" summary="the unique ID of this touch point"/>
>        <arg name="orientation" type="fixed" summary="angle between major axis and positive surface y-axis in degrees"/>
>      </event>
> +
> +    <!-- Version 7 additions -->
> +
> +    <event name="timestamp" since="7">
> +      <description summary="high-resolution timestamp event">
> +        The timestamp event is sent just before a wl_touch.up, wl_touch.motion
> +        or wl_touch.down event and provides a high-resolution version of the
> +        time argument of the event that follows.
> +
> +        The timestamp provided by this event is at least as accurate
> +        as the timestamp provided by the wl_touch event that follows.
> +      </description>
> +      <arg name="tv_sec_hi" type="uint"
> +           summary="high 32 bits of the seconds part of the timestamp"/>
> +      <arg name="tv_sec_lo" type="uint"
> +           summary="low 32 bits of the seconds part of the timestamp"/>
> +      <arg name="tv_nsec" type="uint"
> +           summary="nanoseconds part of the timestamp"/>
> +    </event>
>    </interface>
>  
>    <interface name="wl_output" version="3">

If we went with this approach, this patch is completely correct.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20171124/6f293757/attachment.sig>