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

Fri Aug 18 07:31:31 UTC 2017

On Thu, 17 Aug 2017 17:03:22 +0300
Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:

> 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.  
> > 

...

> > > 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.

Hi Alf,

we have used the technical term "inert" to describe objects that are
essentially dead and should be destroyed by the client soon (but their
use must not cause protocol errors as clients would not be able to
guarantee they'll never use them, due to racing between the compositor
and the client).

> > 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.

Excellent.

Another thought - we could actually support both ways. We could have
two requests in wp_presentation both creating a wp_presentation_timing,
one with a wl_output and one with a wl_surface. In general there must
not be two factory methods to create objects of the same interface
because of ambiguity in object versioning, but if all factory methods
are in the same interface there is no problem.

But, we should be sure that both ways have their use cases that cannot
easily be covered with the other one, as it is more work to implement,
test and maintain. We can also always add another case later if the
need arises.

> > > +    <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.

Good.

> > > +
> > > +        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.

Or you can just refer to wp_presentation_feedback.presented rather
than copy the text, that would make it absolutely clear.

> > > +        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 :)

Good.

> > > +      </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?

As Michel noted, let's add a "msc" argument as well. The definition
should be equivalent to wp_presentation_feedback.presented seq_hi/lo.

> >   
> > > +    </event>
> > > +  </interface>
> > > +
> > >  </protocol>  

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/20170818/8b27fe8d/attachment-0001.sig>