[RFC wayland-protocols] presentation-time: Add request to subscribe to wl_output presentation timings

Thu Aug 17 14:03:22 UTC 2017

On Thu, Aug 17, 2017 at 11:10:12AM +0300, Pekka Paalanen wrote:
> On Mon,  7 Aug 2017 19:00:40 +0300
> Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:
> 
> > The presentation.timing request creates an object that informs the
> > client about the presentation timing of an wl_output in a precise and
> > low-overhead way, without the need to submit any surface content updates.
> 
> Hi Alf,
> 
> the last time I looked at a similar feature was in 2014, the
> refresh_stream interface mentioned in
> https://lists.freedesktop.org/archives/wayland-devel/2014-March/013598.html
> 
> However, I like your / the Chromium design much better, because it doesn't
> spam events on every refresh.
> 
> This is a good proposal.

Hi Pekka,

thank you very much for the feedback. I have replied to your
comments in-line.

> > This functionality can be useful, among others, for clients that want to know
> > the output timings before submitting any frames, and also for clients that
> > want to synchronize with server presentation timings using a simpler and lower
> > overhead way compared to the presentation.feedback objects.
> 
> Yes, I suppose this is justification enough, and particularly as CrOS
> is already using it.
> 
> > This functionality is inspired by the vsync-feedback protocol used in
> > Chromium OS ([1]), and, hopefully, with such an addition, Chromium OS may
> > be able to transition to the upstream protocol.
> > 
> > It may be useful to add a flags argument to the presentation_timing.updated
> > event, containing information that can be used to assess the reliability (and
> > other aspects) of the provided information, in a manner similar to the 'kind'
> > flags for the presentation_feedback.presented event.
> 
> I think adding flags would be nice indeed, even if we cannot specify
> any at the moment. Flags can be added in later revisions but only if
> the argument already exists, otherwise we would need to add a new event.

Ack. I will add a flags argument to the event.

> > A proof-of-concept implementation for Weston can be found at:
> > 
> > https://git.collabora.com/cgit/user/alf/weston.git/log/?h=presentation-time-v2
> > 
> > [1] https://chromium.googlesource.com/chromium/src/third_party/+/master/wayland-protocols/unstable/vsync-feedback/vsync-feedback-unstable-v1.xml
> > 
> > ---
> >  stable/presentation-time/presentation-time.xml | 68 ++++++++++++++++++++++++--
> >  1 file changed, 65 insertions(+), 3 deletions(-)
> > 
> 
> This is written as an evolution of wp_presentation global interface
> which has already been declared stable. There is the question whether
> that is fine or should the new feature be an independent extension.
> 
> To me this question is more about the ability to extend the
> interface(s) later and a feature being optional or mandatory. If the
> request is added in wp_presentation interface, then we intend it to be
> an integral part of wp_presentation, and that if a compositor wants to
> expose wp_presentation, then it will also want to expose the new
> feature. The rationale is that if a second new feature is added to
> wp_presentation, it is not possible to support it without also
> implementing the first new feature.
> 
> IOW, the question is, would anyone realistically need the option to not
> implement wp_presentation_timing while they do implement wp_presentation?
> 
> My initial feeling is "no", which would favour adding this in
> wp_presentation.
> 
> Also, adding to wp_presentation makes it easy to define things with the
> presentation clock provided by wp_presentation.
> 
> All in all, good.
> 
> > diff --git a/stable/presentation-time/presentation-time.xml b/stable/presentation-time/presentation-time.xml
> > index a46994c..56a9c9b 100644
> > --- a/stable/presentation-time/presentation-time.xml
> > +++ b/stable/presentation-time/presentation-time.xml
> > @@ -3,7 +3,7 @@
> >  <!-- wrap:70 -->
> >  
> >    <copyright>
> > -    Copyright © 2013-2014 Collabora, Ltd.
> > +    Copyright © 2013-2017 Collabora, Ltd.
> 
> Just checking, the Chromium extension's copyrights don't need to be
> added here, do they?

Actually, I think we should add them, since the proposed additions are
heavily based on the Chromium protocol, I just forgot to do so for this RFC.

> >      Permission is hereby granted, free of charge, to any person obtaining a
> >      copy of this software and associated documentation files (the "Software"),
> > @@ -25,8 +25,8 @@
> >      DEALINGS IN THE SOFTWARE.
> >    </copyright>
> >  
> > -  <interface name="wp_presentation" version="1">
> > -    <description summary="timed presentation related wl_surface requests">
> > +  <interface name="wp_presentation" version="2">
> > +    <description summary="timed presentation related requests">
> >  
> >  <!-- Introduction -->
> >  
> > @@ -49,6 +49,16 @@
> >        presentation time can differ from the compositor's predicted
> >        display update time and the update's target time, especially
> >        when the compositor misses its target vertical blanking period.
> > +
> > +<!-- Presentation timing -->
> > +
> > +      In some cases it's also useful for clients to know about the
> > +      presentation timing of an output without having submitted a
> > +      surface content update. For this purpose the 'timing' request
> > +      creates an object that is used to inform the client about
> > +      the presentation timing of an wl_output in a precise and
> > +      low-overhead way.
> > +
> >      </description>
> >  
> >      <enum name="error">
> > @@ -122,6 +132,22 @@
> >        <arg name="clk_id" type="uint" summary="platform clock identifier"/>
> >      </event>
> >  
> > +    <!-- Version 2 additions -->
> > +
> > +    <request name="timing" since="2">
> > +      <description summary="request timing information">
> > +        Create a new timing object that represents a subscription to
> > +        presentation timing updates on the given wl_output object.
> > +
> > +        The newly created object will signal an update to notify the subscriber
> > +        of initial timing parameters as soon as these become available.
> 
> If the output was already or becomes destroyed in the compositor, there
> will be no event, right? More below.

Correct.

> > +      </description>
> > +      <arg name="output" type="object" interface="wl_output"
> > +           summary="the wl_output object to subscribe for timings of"/>
> > +      <arg name="id" type="new_id" interface="wp_presentation_timing"
> > +           summary="the new timing object"/>
> > +    </request>
> > +
> >    </interface>
> >  
> >    <interface name="wp_presentation_feedback" version="1">
> > @@ -263,4 +289,40 @@
> >      </event>
> >    </interface>
> >  
> > +  <interface name="wp_presentation_timing" version="1">
> 
> An interface can have an overhead doc as well. It might be a nice place
> to document how a wp_presentation_timing object reacts to:
> 
> a) the output being destroyed in the compositor
> 
> b) the wl_output object used in wp_presentation.timing request being
>    destroyed by the client

In both cases the timing object just becomes inactive (no events sent)
and should be eventually destroyed by the client. I will add a section
to clarify.

> Another thing is, how will a client know which output's timings it
> should be looking at for a specific wl_surface. The wl_surface enter
> and leave events do not tell that when a surface is on more than one
> output at the same time, the client won't know which one the compositor
> is using for the timings. The client needs to use
> wp_presentation_feedback to know which output it is at any time.
> 
> This brings a different idea to my mind: what if the timings were asked
> for a wl_surface instead of a wl_output? The issue with that is
> surfaces that are not currently shown, like ones that are not mapped
> yet, are off-screen, etc. OTOH, maybe the issue is not significant: in
> a multi-output setup, you cannot always know which output a surface
> will be synced to anyway until it actually becomes visible.
> 
> Which design would better serve the use cases?

I think associating the timing with the surface instead of the output is
both a good idea and it's also closer to the spirit of the current
presentation-timing v1 protocol. 'timing' would essentially become a
lighter, permanent and less spam-y version of 'feedback' (but, of
course, not as full-featured as 'feedback').

As you mention, a problem with this approach is that, in theory, we lose
the ability to know the timings before the first frame, since in general
a surface needs to be first committed to with a buffer to figure out on
which output it will land. However, I also agree this is fine in
practice, since for single-display systems, full-screen surfaces, or for
systems with simple, size-independent multi-display window-management
policies, the server can potentially send the timing information before
the first frame.  For systems with more complex multi-display WM
policies, the client wouldn't be able to tell which output will be
driving rendering anyway, so we don't lose any functionality.

I will check with Chromium if such a change still be fine for them, and,
if so, I will come back with an updated proposal.

> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy timing object">
> > +        Destroy this timing object.
> > +      </description>
> > +    </request>
> > +
> > +    <event name="updated">
> > +      <description summary="the timing has been updated">
> > +        Notifies the client that the timing of the associated wl_output
> > +        has changed.
> 
> Maybe there should be some wording that the timing does not necessarily
> need to change for the compositor to send this event. A compositor
> could send this event at arbitrary times to keep the predictions from
> drifting too far in clients. E.g. I could imagine Weston sending this
> event again when the prediction error grows beyond a certain threshold
> to reset the timebase.

The word 'timing' refers to both timebase and interval (so it covers the
case you mention), but it would probably be better to reword to remove
any ambiguity.

> > +
> > +        Timing information consists of two data, timebase and interval.
> > +        Timebase is an absolute timestamp of the event that caused the
> > +        timing to change. Interval is a period of time between subsequent
> > +        presentation events.
> 
> "Event that caused the timing to change" is not unambiguously a vblank,
> timings might have changed slightly before the first vblank produced
> with the new timings. Should this be written so that the timebase is
> always the same kind of timestamp as wp_presentation_feedback.presented
> provides?

Yes, it makes sense to use the same wording for both.

> > +        For the interpretation of the timestamp, see presentation.clock_id
> > +        event.
> > +
> > +        If the server cannot accurately predict the interval between
> > +        subsequent presentation events for the output, or the output does
> > +        not have a constant presentation rate, then the interval argument
> > +        must be zero.
> 
> Yes. Should we note that such occasion could also be temporary, or is
> it obvious?

I will clarify that the inability to predict may be temporary (which
makes even more sense if we move to a per-surface timing object).
Ideally, nothing should be left to the reader's imagination when writing
specifications :)

> > +      </description>
> > +      <arg name="timebase_tv_sec_hi" type="uint"
> > +           summary="high 32 bits of the seconds part of the new presentation timebase"/>
> > +      <arg name="timebase_tv_sec_low" type="uint"
> > +           summary="low 32 bits of the seconds part of the new presentation timebase"/>
> > +      <arg name="timebase_tv_nsec" type="uint"
> > +           summary="nanoseconds part of the new presentation timebase"/>
> > +      <arg name="interval" type="uint"
> > +           summary="new presentation interval in nanoseconds"/>
> 
> I wonder if people would find use for MSC counter in this event? Roman
> Gilg for Xwayland/Present perhaps?
> 
> > +    </event>
> > +  </interface>
> > +
> >  </protocol>
> 
> Ideas to think about in the future, probably not worth including right
> now:
> 
> This extension gives out the vblank timings, but is that enough? Should
> we have provision for compositor deadlines as well, or are those too
> implementation-specific to be exposed in a generic extension?
> 
> By deadline I mean the point at which the compositor starts a repaint
> in the output repaint cycle, which is an arbitrary time point between
> the previous and next vblank. E.g. Weston has a crude configurable
> delay for starting a new repaint after a vblank, and clients that miss
> the deadline get postponed vblank+2 instead of hitting vblank+1.
> 
> Details of Weston's current behaviour are here:
> http://ppaalanen.blogspot.fi/2015/02/weston-repaint-scheduling.html
> But that is now slightly outdated since we now have a single repaint
> timer instead of per-output timers, and it causes some rounding
> (jitter?) on the deadlines.
> 
> I have no idea what other compositors do, or if the concept of a
> deadline like this is universal. If it's not universal, then nevermind.
> 
> Thanks,
> pq

Thanks,
Alexandros