[PATCH wayland-protocols] unstable: Add input-timestamps protocol

[Pekka Paalanen]

On Tue,  5 Dec 2017 18:07:02 +0200
Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:

> wl_pointer, wl_keyboard and wl_touch events currently use a 32-bit
> timestamp with millisecond resolution. In some cases, notably latency
> measurements, this resolution is too coarse to be useful.
> 
> This protocol provides additional high-resolution timestamps events,
> which are emitted before the corresponding input event. Each timestamp
> event contains a high-resolution, and ideally higher-accuracy, version
> of the 'time' argument of the first subsequent supported input event.
> 
> Clients that care about high-resolution timestamps just need to keep
> track of the last timestamp event they receive and associate it with the
> next supported input event that arrives.
> 
> Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> ---
>  Makefile.am                                        |   1 +
>  unstable/input-timestamps/README                   |   4 +
>  .../input-timestamps-unstable-v1.xml               | 138 +++++++++++++++++++++
>  3 files changed, 143 insertions(+)
>  create mode 100644 unstable/input-timestamps/README
>  create mode 100644 unstable/input-timestamps/input-timestamps-unstable-v1.xml

Hi Alf,

nice work, a comment below.

> 
> diff --git a/Makefile.am b/Makefile.am
> index cabc279..4b9a901 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -16,6 +16,7 @@ unstable_protocols =								\
>  	unstable/xwayland-keyboard-grab/xwayland-keyboard-grab-unstable-v1.xml	\
>  	unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml \
>  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
> +	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
>  	$(NULL)
>  
>  stable_protocols =								\
> diff --git a/unstable/input-timestamps/README b/unstable/input-timestamps/README
> new file mode 100644
> index 0000000..3e82890
> --- /dev/null
> +++ b/unstable/input-timestamps/README
> @@ -0,0 +1,4 @@
> +High-resolution timestamps for input events.
> +
> +Maintainers:
> +Alexandros Frantzis <alexandros.frantzis at collabora.com>
> diff --git a/unstable/input-timestamps/input-timestamps-unstable-v1.xml b/unstable/input-timestamps/input-timestamps-unstable-v1.xml
> new file mode 100644
> index 0000000..5a9d120
> --- /dev/null
> +++ b/unstable/input-timestamps/input-timestamps-unstable-v1.xml
> @@ -0,0 +1,138 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="input_timestamps_unstable_v1">
> +
> +  <copyright>
> +    Copyright © 2017 Collabora, Ltd.
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a
> +    copy of this software and associated documentation files (the "Software"),
> +    to deal in the Software without restriction, including without limitation
> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +    and/or sell copies of the Software, and to permit persons to whom the
> +    Software is furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice (including the next
> +    paragraph) shall be included in all copies or substantial portions of the
> +    Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +    DEALINGS IN THE SOFTWARE.
> +  </copyright>
> +
> +  <description summary="High-resolution timestamps for input events">
> +    This protocol specifies a way for a client to request and receive
> +    high-resolution timestamps for input events.
> +
> +    Warning! The protocol described in this file is experimental and
> +    backward incompatible changes may be made. Backward compatible changes
> +    may be added together with the corresponding interface version bump.
> +    Backward incompatible changes are done by bumping the version number in
> +    the protocol and interface names and resetting the interface version.
> +    Once the protocol is to be declared stable, the 'z' prefix and the
> +    version number in the protocol and interface names are removed and the
> +    interface version number is reset.
> +  </description>
> +
> +  <interface name="zwp_input_timestamps_manager_v1" version="1">
> +    <description summary="context object for high-resolution input timestamps">
> +      A global interface used for requesting high-resolution timestamps
> +      for input events.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the input timestamps manager object">
> +        Informs the server that the client will no longer be using this
> +        protocol object. Existing objects created by this object are not
> +        affected.
> +      </description>
> +    </request>
> +
> +    <request name="get_keyboard_timestamps">
> +      <description summary="subscribe to high-resolution keyboard timestamp events">
> +        Creates a new input timestamps object that represents a subscription
> +        to high-resolution timestamp events for all wl_keyboard events that
> +        carry a timestamp.
> +
> +        If the associated wl_keyboard object is invalidated, either through
> +        client action (e.g. release) or server-side changes, the input
> +        timestamps object becomes inert and the client should destroy it
> +        by calling zwp_input_timestamps_v1.destroy.
> +      </description>
> +      <arg name="id" type="new_id" interface="zwp_input_timestamps_v1"/>
> +      <arg name="keyboard" type="object" interface="wl_keyboard"
> +           summary="the wl_keyboard object for which to get timestamp events"/>
> +    </request>
> +
> +    <request name="get_pointer_timestamps">
> +      <description summary="subscribe to high-resolution pointer timestamp events">
> +        Creates a new input timestamps object that represents a subscription
> +        to high-resolution timestamp events for all wl_pointer events that
> +        carry a timestamp.
> +
> +        If the associated wl_pointer object is invalidated, either through
> +        client action (e.g. release) or server-side changes, the input
> +        timestamps object becomes inert and the client should destroy it
> +        by calling zwp_input_timestamps_v1.destroy.
> +      </description>
> +      <arg name="id" type="new_id" interface="zwp_input_timestamps_v1"/>
> +      <arg name="pointer" type="object" interface="wl_pointer"
> +           summary="the wl_pointer object for which to get timestamp events"/>
> +    </request>
> +
> +    <request name="get_touch_timestamps">
> +      <description summary="subscribe to high-resolution touch timestamp events">
> +        Creates a new input timestamps object that represents a subscription
> +        to high-resolution timestamp events for all wl_touch events that
> +        carry a timestamp.
> +
> +        If the associated wl_touch object becomes invalid, either through
> +        client action (e.g. release) or server-side changes, the input
> +        timestamps object becomes inert and the client should destroy it
> +        by calling zwp_input_timestamps_v1.destroy.
> +      </description>
> +      <arg name="id" type="new_id" interface="zwp_input_timestamps_v1"/>
> +      <arg name="touch" type="object" interface="wl_touch"
> +           summary="the wl_touch object for which to get timestamp events"/>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_input_timestamps_v1" version="1">
> +    <description summary="context object for input timestamps">
> +      Provides high-resolution timestamp events for a set of subscribed input
> +      events. The set of subscribed input events is determined by the
> +      zwp_input_timestamps_manager_v1 request used to create this object.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the input timestamps object">
> +        Informs the server that the client will no longer be using this
> +        protocol object. After the server processes the request, no more
> +        timestamp events will be emitted.

This could be said simply:
"Cancel the subscription to the high-resolution timestamps."

It's fine either way.

> +      </description>
> +    </request>
> +
> +    <event name="timestamp">
> +      <description summary="high-resolution timestamp event">
> +        The timestamp event is associated with the first subsequent input event

+ carrying a timestamp

> +        which belongs to the set of input events this object is subscribed to.

This is a bit hard to write down precisely. How about:

"The timestamp event is associated with the first subsequent input event
group (as defined in the respective input interface, e.g.
wl_pointer.frame) or single input event that already carries a
timestamp with less precision than this timestamp event."

I inferred from Peter's wording that we want to require a new event for
a new input event/group even if the timestamp would be the same.
Therefore "the first" and "single".

> +
> +        The timestamp provided by this event is a high-resolution version of
> +        the timestamp argument of the associated input event. The provided

"..of the associated input event(s)." would perhaps cover the above
change in wording?

> +        timestamp is in the same clock domain and is at least as accurate as
> +        the associated input event timestamp.
> +      </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>
> +
> +</protocol>

That wording in one paragraph is the only thing I could hook onto,
otherwise this looks excellent. You have:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

regardless of whether the changes I proposed are done or not. I wonder
what Peter would think of my wording.


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/20171211/fa9fdade/attachment-0001.sig>