On Wed, 07 Nov 2018 20:22:38 +0000 Simon Ser <[email protected]> wrote:
> Thanks! With these fixes, this is now > > Reviewed-by: Simon Ser <[email protected]> > > 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 <[email protected]> > > --- > > > > 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 <[email protected]> > > +Daniel Stone <[email protected]> > > +Alexandros Frantzis <[email protected]> > 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
pgpWTlnf4hwFi.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
