[PATCH wayland-protocols v4] Add zwp_linux_explicit_synchronization_v1

[Simon Ser]

> > > +      Explicit synchronization is guaranteed to be supported only for buffers
> > > +      created with any version of the zwp_linux_dmabuf buffer factory.
> >
> > I think we can drop the "z" prefix here.
>
> Hmm, not sure about this. We would be using a protocol prefix/name
> combination that doesn't exist (yet). Of course the intention is clear,
> but I think it would be better to update this to, e.g.,
> (z)wp_linux_dmabuf, when linux_dmabuf actually becomes stable.

Then add the v1 suffix zwp_linux_dmabuf_v1?

> > I wonder if failures to import a fence should really be protocol errors.
> > Protocol errors are meant to be used for protocol violations. I understand that
> > a client can send an invalid fence, but are there other reasons why a fence
> > cannot be imported? Maybe we could change this to "if the file descriptor isn't
> > a dma_fence"?
>
> Yes, the this is all about having a valid fence (and that's how it's
> implemented in the WIP branch). Requiring something more from this
> request would be asking too much, both from the server and the client. I
> will rephrase.
>
> It's unlikely that we won't be able to use valid fence at a later point.
> If this happens it's most likely a driver issue (or we have messed up in
> weston), in which case I think disconnecting the client is a valid
> option (compared to allowing bad data on screen), but that's a different
> case and error.

Makes sense.

> > > +    <request name="get_release">
> > > +      <description summary="release fence for last-attached buffer">
> > > +        Create a listener for the release of the buffer attached by the
> > > +        client with wl_buffer.attach. See zwp_buffer_release_v1
> > > +        documentation for more information.
> > > +
> > > +        The release object is double-buffered state, and will be applied
> > > +        on the next wl_surface.commit request for the associated surface.
> >
> > "will be applied" is a little bit strange for this request. Maybe change to
> > "will provide release information about the next wl_surface.commit request"?
>
> "will be applied" tries to convey that the release will be associated
> with the buffer that is attached at commit time (instead of any
> intermediate attachments).
>
> So, perhaps "will be associated with the buffer that is attached to the
> surface at wl_surface.commit time"?

+1

> > > +  <interface name="zwp_buffer_release_v1" version="1">
> > > +    <description summary="buffer release explicit synchronization">
> > > +      This object is instantiated in response to a
> > > +      zwp_surface_synchronization_v1.get_release request.
> > > +
> > > +      It provides an alternative to wl_buffer.release events, providing a unique
> > > +      release from a single wl_surface.commit request. The release event also
> > > +      supports explicit synchronization, providing a fence FD where relevant for
> > > +      the client to synchronize against before reusing the buffer.
> > > +
> > > +      Exactly one event, either a fenced_release or an immediate_release, will
> > > +      be emitted for each wl_surface.commit request.
> >
> > This makes it sound like this object can be used for multiple commits. Maybe we
> > can change the wording to "will be emitted after the wl_surface.commit request".
>
> Perhaps "will be emitted for the wl_surface.commit request", instead? My
> concern is that "after" may be misread as the commit immediately
> triggering an event (which, to be fair, doesn't make sense in this
> context).

Sounds good.

> > > +    <event name="fenced_release">
> > > +      <description summary="release buffer with fence">
> > > +        Sent when the compositor has finalised its usage of the associated
> > > +        buffer for the relevant commit, providing a dma_fence which will be
> > > +        signaled when all operations by the compositor on that buffer for that
> > > +        commit have finished.
> > > +
> > > +        Clients must not perform any destructive operations (e.g. clearing or
> > > +        rendering) on the buffer until that fence has signaled.
> >
> > We should probably add to this request and to immediate_release something among
> > the lines of:
> >
> > > Upon receiving this event, the client should destroy this object.
>
> In v5 I am changing zwp_buffer_release to use destroy-on-event, so this
> doesn't apply any more. Of course, the client should still destroy the
> proxy, but I don't think this is something we need to explicitly state.

Hmm. One should be careful when choosing destroy-on-event, it makes it
impossible to add requests to the interface later on.