[PATCH 1/8] protocol: add presentation extension v4

[Bryce Harrington]

On Tue, Sep 16, 2014 at 10:34:29AM +0300, Pekka Paalanen wrote:
> On Mon, 15 Sep 2014 23:12:24 -0700
> Bryce Harrington <bryce at osg.samsung.com> wrote:
> 
> > Mostly just some minor grammar improvement suggestions...
> 
> Appreciated. :-)
> 
> > On Mon, Sep 15, 2014 at 04:16:40PM -0400, Louis-Francis Ratté-Boulianne wrote:
> > <snip>
> > > diff --git a/Makefile.am b/Makefile.am
> > > diff --git a/protocol/presentation_timing.xml b/protocol/presentation_timing.xml
> > > new file mode 100644
> > > index 0000000..8aadff0
> > > --- /dev/null
> > > +++ b/protocol/presentation_timing.xml
> > > @@ -0,0 +1,247 @@
> > > +<?xml version="1.0" encoding="UTF-8"?>
> > > +<protocol name="presentation_timing">
> > > +<!-- wrap:70 -->
> > > +
> > > +
> > > +    <request name="feedback">
> > > +      <description summary="request presentation feedback information">
> > > +        With this request, presentation feedback will be provided for
> > > +        the current content submission of the given surface. A new
> > > +        presentation_feedback object is created, and that object will
> > > +        deliver the information once. The object is tied to this
> > > +        content submission only. Multiple presentation_feedback
> > > +        objects may be created for the same submission, and they will
> > > +        all return the same information.
> > 
> > The language here is a bit clunky.  Here's my try:
> > 
> > "In response to this request, a new presentation_feedback object is
> > created for the given surface's current content submission.  The object
> > contains a static snapshot of the information.  Each
> > request against the same submission will create a new
> > presentation_feedback object, but they will all have the same
> > information."
> > 
> > Not sure that's any better!  But maybe gives some wording ideas.
> 
> Yeah, not sure...
> 
> How about:
> 
> "Request presentation feedback for the current content submission on
> the given surface. This creates a new presentation_feedback object,
> which will deliver the feedback information once. If multiple
> presentation_feedback objects are created for the same submission, they
> will all deliver the same information."

There we go, that sounds better.
 
> > Also, it sounds like the client will need to free this object itself?
> > Either way, it might be worth stating explicitly here.
> 
> Clients will always need to free all objects themselves, that's part of
> the Wayland C bindings. No need to add it here, as it applies to
> everything.

Alright, then in that case I noticed at least one other spot where the
management responsibility was documented in this patch, and so by this
design convention could be dropped.

> Also the destructor protocol existing is a clue, that getting an event
> won't destroy the protocol object.
> 
> That's actually something we could change. Remove the destructor
> protocol, and specify that 'presented' and 'discarded' events destroy
> the protocol object. Presentation_feedback being a pure feedback
> interface, I cannot imagine ever adding any requests to it. The only use
> case for destructor protocol would be to cancel the feedback, which
> probably is not common, and would still be possible anyway.
> 
<snip>
> > > +  <interface name="presentation_feedback" version="1">
> > > +    <description summary="presentation time feedback event">
> > > +      A presentation_feedback object returns the feedback information
> > > +      about a wl_surface content update becoming visible to the user.
> > 
> > "A presentation_feedback object returns an indication that a wl_surface
> > content update has become visible to the user."
> > 
> > Otherwise the sentence sounds a bit like, "An apple tastes like an
> > apple."  ;-)
> 
> Except it may not have become visible to the user: 'discarded'.
> 
> Presentation_feedback is the type of an object that delivers feedback
> about presentation, so I see no repetition there as the name is
> arbitrary and its meaning needs to be defined.
> 
> s/return/deliver/ would be better I think, if we use "deliver" above.
> 
> How about: (see further below...)
> 
> > > +      One object corresponds to one content update submission
> > > +      (wl_surface.commit). There are two possible outcomes: the
> > > +      content update may be presented to the user, in which case the
> > > +      presentation timestamp is delivered. Otherwise, the content
> > > +      update is discarded, and the user never had a chance to see it
> > > +      before it was superseded or the surface was destroyed.
> > 
> > "There are two possible outcomes:  The content update is presented to
> > the user, and a presentation timestamp delivered; or, the user did not
> > see the content update because it was superseded or its surface
> > destroyed, and the content update is discarded."
> 
> How about:
> 
> "A presentation_feedback object delivers the result of the
> associated wl_surface content update: whether it was presented to
> the user or discarded. Discarding may be due to the update being
> superseded by another update or the surface being destroyed."
> 
> Maybe that says it all?

Way better.  Yes I think this clarifies everything.

<snip to end>
> > Bryce
> 
> Agreed with every comment I didn't explicitly reply to above. :-)
> 
> 
> Thanks,
> pq

Thanks,
Bryce