[PATCH wayland-protocols v6] Add zwp_linux_explicit_synchronization_v1

Thu Nov 8 13:00:58 UTC 2018

On Wed, 07 Nov 2018 20:22:38 +0000
Simon Ser <contact at emersion.fr> wrote:

> Thanks! With these fixes, this is now
> 
> Reviewed-by: Simon Ser <contact at emersion.fr>
> 
> Below is a small comment, I don't know if it's relevant or not, I just wanted to
> point it out.
> 
> > Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> > ---
> >
> > Changes in patch v6:
> >   - Fixed wl_buffer.attach -> wl_surface.attach typos.
> >   - Added NO_BUFFER error and updated request descriptions.
> >   - Consistently used "error is raised" phrasing.
> >   - Added comment about buffer_release object destruction in event
> >     descriptions.
> >
> > Changes in patch v5:
> >   - Further clarified the per-commit nature of buffer_release.
> >   - Used the wp_linux_dmabuf name to refer to all versions of that protocol.
> >   - Fixed wl_buffer.attach typo.
> >   - Clarified when an INVALID_FENCE error is raised.
> >   - Improved wording explaining the double-buffer state of buffer_release.
> >   - Allowed get_release requests on all non-null buffers.
> >   - Clarified that the compositor is free to select buffer_release events
> >     on a release by release basis.
> >   - Changed buffer_release to be self-destroyed after it emits an event.
> >
> > Changes in patch v4:
> >   - Guaranteed protocol compatibility only with zwp_linux_dmabuf buffers.
> >   - Added the UNSUPPORTED_BUFFER error.
> >
> > Changes in patch v3:
> >   - Reworded implicit/explicit synchronization intro in
> >     zwp_surface_synchronization_v1 description.
> >   - Removed confusing mention of wl_buffer.release in
> >     zwp_surface_synchronization_v1 description.
> >   - Clarified which fences are affected on sync object destruction.
> >   - Removed unclear mention about wl_buffer destruction
> >     in fenced_release description.
> >   - Clarified that the release events and their guarantees apply to
> >     the relevant commit only.
> >   - Reformatted text.
> >
> > Changes in patch v2:
> >   - Added NO_SURFACE error for zwp_surface_synchronization_v1 requests.
> >   - Removed restriction to destroy a zwp_surface_synchronization_v1 object
> >     after the associated wl_surface is destroyed.
> >   - Clarified which buffer the acquire fence is associated with.
> >   - Clarified that exactly one event, either a fenced_release or a
> >     immediate_release, will be emitted for each commit.
> >
> >  Makefile.am                                   |   1 +
> >  .../linux-explicit-synchronization/README     |   6 +
> >  ...x-explicit-synchronization-unstable-v1.xml | 234 ++++++++++++++++++
> >  3 files changed, 241 insertions(+)
> >  create mode 100644 unstable/linux-explicit-synchronization/README
> >  create mode 100644 unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 6394e26..7dfbb9e 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -21,6 +21,7 @@ unstable_protocols =								\
> >  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
> >  	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
> >  	unstable/xdg-decoration/xdg-decoration-unstable-v1.xml	\
> > +	unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
> >  	$(NULL)
> >
> >  stable_protocols =								\
> > diff --git a/unstable/linux-explicit-synchronization/README b/unstable/linux-explicit-synchronization/README
> > new file mode 100644
> > index 0000000..f13b404
> > --- /dev/null
> > +++ b/unstable/linux-explicit-synchronization/README
> > @@ -0,0 +1,6 @@
> > +Linux explicit synchronization (dma-fence) protocol
> > +
> > +Maintainers:
> > +David Reveman <reveman at chromium.org>
> > +Daniel Stone <daniels at collabora.com>
> > +Alexandros Frantzis <alexandros.frantzis at collabora.com>  
> diff --git a/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml > b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> > new file mode 100644
> > index 0000000..b87663f
> > --- /dev/null
> > +++ b/unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml
> > @@ -0,0 +1,234 @@
> > +<?xml version="1.0" encoding="UTF-8"?>
> > +<protocol name="zwp_linux_explicit_synchronization_unstable_v1">
> > +
> > +  <copyright>
> > +    Copyright 2016 The Chromium Authors.
> > +    Copyright 2017 Intel Corporation
> > +    Copyright 2018 Collabora, Ltd
> > +
> > +    Permission is hereby granted, free of charge, to any person obtaining a
> > +    copy of this software and associated documentation files (the "Software"),
> > +    to deal in the Software without restriction, including without limitation
> > +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > +    and/or sell copies of the Software, and to permit persons to whom the
> > +    Software is furnished to do so, subject to the following conditions:
> > +
> > +    The above copyright notice and this permission notice (including the next
> > +    paragraph) shall be included in all copies or substantial portions of the
> > +    Software.
> > +
> > +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > +    DEALINGS IN THE SOFTWARE.
> > +  </copyright>
> > +
> > +  <interface name="zwp_linux_explicit_synchronization_v1" version="1">
> > +    <description summary="protocol for providing explicit synchronization">
> > +      This global is a factory interface, allowing clients to request
> > +      explicit synchronization for buffers on a per-surface basis.
> > +
> > +      See zwp_surface_synchronization_v1 for more information.
> > +
> > +      This interface is derived from Chromium's
> > +      zcr_linux_explicit_synchronization_v1.
> > +
> > +      Warning! The protocol described in this file is experimental and
> > +      backward incompatible changes may be made. Backward compatible changes
> > +      may be added together with the corresponding interface version bump.
> > +      Backward incompatible changes are done by bumping the version number in
> > +      the protocol and interface names and resetting the interface version.
> > +      Once the protocol is to be declared stable, the 'z' prefix and the
> > +      version number in the protocol and interface names are removed and the
> > +      interface version number is reset.
> > +    </description>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy explicit synchronization factory object">
> > +        Destroy this explicit synchronization factory object. Other objects,
> > +        including zwp_surface_synchronization_v1 objects created by this
> > +        factory, shall not be affected by this request.
> > +      </description>
> > +    </request>
> > +
> > +    <enum name="error">
> > +      <entry name="synchronization_exists" value="0"
> > +             summary="the surface already has a synchronization object associated"/>
> > +    </enum>
> > +
> > +    <request name="get_synchronization">
> > +      <description summary="extend surface interface for explicit synchronization">
> > +        Instantiate an interface extension for the given wl_surface to provide
> > +        explicit synchronization.
> > +
> > +        If the given wl_surface already has an explicit synchronization object
> > +        associated, the synchronization_exists protocol error is raised.
> > +      </description>
> > +
> > +      <arg name="id" type="new_id" interface="zwp_surface_synchronization_v1"
> > +           summary="the new synchronization interface id"/>
> > +      <arg name="surface" type="object" interface="wl_surface"
> > +           summary="the surface"/>
> > +    </request>
> > +  </interface>
> > +
> > +  <interface name="zwp_surface_synchronization_v1" version="1">  
> 
> I missed this before, but: is there a reason why the "linux" prefix has been
> dropped here? Maybe it should be renamed to
> zwp_linux_surface_synchronization_v1? What about zwp_buffer_release_v1?

That's a good question, because these names are kind of global. Not
really global, but it could cause some name conflicts if the same
program or a compilation unit needed to use two different but
same-named interfaces. They are less global than the same of the global
interface though, which needs to be unique per platform for real.

The stable names would be wp_surface_synchronization and
wp_buffer_release, with the root interface being
wp_linux_explicit_synchronization.

The dmabuf extension relies of the Linux definition of a dmabuf. This
extension relies on the Linux definition of a fence, AFAIU.

So yes, I think repeating "linux" in the interface names would be
appropriate.

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

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/20181108/268b0028/attachment.sig>