[PATCH wayland-protocols] unstable: Add input-timestamps protocol
Alexandros Frantzis
alexandros.frantzis at collabora.com
Wed Dec 13 14:48:28 UTC 2017
On Mon, Dec 11, 2017 at 03:25:28PM +0200, Pekka Paalanen wrote:
> 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.
Hi Pekka,
thanks for the review, some comments 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".
In this version I opted not to provide timestamps for frame/group events
(see discussion notes email). One reason is that it's not clear what a
timestamp on a frame event (that doesn't already have a timestamp)
refers to, and I don't think this protocol can or should try to provide
that meaning. That's why I decided to emit high-res timestamps only for
events that already have one.
Also, in terms of phrasing, my idea was that zwp_input_timestamps_v1
wouldn't describe the set of input events it emits timestamps for, so it
wouldn't impose assumptions that may not be true generally. The
subscribed input event sets are described in the respective
zw_input_timestamps_manager_v1.get_*_timestamps requests. In the spirit
of this, if we wanted to support the frame event we would add it in the
respective get_*_timestamp description.
> > +
> > + 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>
I wanted to add here that one thing you brought up in the weston event
tests patchset is the need to more accurately define the representation
of the high-res timestamps in terms of normalization. I will do so in an
upcoming v2 patchset.
> > + </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,
Alexandros
More information about the wayland-devel
mailing list