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

[Pekka Paalanen]

On Tue, 16 Sep 2014 14:16:37 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

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

Did you intend to point it out?

We have two different cases here though. One is when the protocol
object gets destroyed; after that it is invalid to try to send events
or requests on/with it. The other is when the C object is freed. These
are distinct cases, and one does not exactly imply the other.

If there is a "destroy" request marked as destructor in the protocol,
then freeing the C object automatically destroys the protocol object so
that both sides will know about it. This is the normal case and does
not need explicit documentation, IMO.

If there is no destructor protocol for "destroy", freeing the C object
does not exactly destroy the protocol object; it will continue to exist
in the server, and the client will just ignore events coming through
it. In this case, the protocol designer must have guaranteed a way for
the protocol object to get destroyed in the server (e.g. the "done"
event for wl_callback).

If the protocol specifies, that an event destroys the protocol object,
the protocol must also guarantee that the client cannot race and send
requests on the protocol object after the server destroyed it. An
obvious way to do that is to not have any requests in the interface.

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


Thanks,
pq