On March 14, 2018 10:22 AM, Peter Hutterer <[email protected]> wrote: > sorry about the delay, but better late than too late ;)
No problem, thanks for your review! > On Sun, Mar 11, 2018 at 05:53:42PM -0400, Simon Ser wrote: > > This adds a new protocol to negotiate server- and client-side rendering of > > window decorations for xdg-toplevels. This allows compositors that want > > to draw decorations themselves to send their preference to clients, and > > clients that prefer server-side decorations to request them. > > > > This is inspired by a protocol from KDE [1] which has been implemented in > > KDE and Sway and was submitted for consideration in 2017 [2]. This patch > > provides an updated protocol with those concerns taken into account. > > > > Signed-off-by: Simon Ser <[email protected]> > > Reviewed-by: Drew DeVault <[email protected]> > > Reviewed-by: David Edmundson <[email protected]> > > Reviewed-by: Alan Griffiths <[email protected]> > > Reviewed-by: Tony Crisci <[email protected]> > > > > [1] > > https://github.com/KDE/kwayland/blob/master/src/client/protocols/server-decoration.xml > > [2] > > https://lists.freedesktop.org/archives/wayland-devel/2017-October/035564.html > > --- > > > > This was iterated on privately between representatives of Sway and wlroots > > (Simon Ser, Drew DeVault and Tony Crisci), KDE and Qt (David Edmundson), > > and Mir (Alan Griffiths). > > > > A proof-of-concept of a client and server implementation is available at > > [1]. > > > > Changes from v2 to v3: fix typos, clarify xdg_toplevel_decoration.destroy > > semantics, clarify that clients always need to support CSD and can receive > > configure events at any time. > > > > [1] https://github.com/swaywm/wlroots/pull/638 > > > > Makefile.am | 1 + > > unstable/xdg-toplevel-decoration/README | 4 + > > .../xdg-toplevel-decoration-unstable-v1.xml | 132 > > +++++++++++++++++++++ > > 3 files changed, 137 insertions(+) > > create mode 100644 unstable/xdg-toplevel-decoration/README > > create mode 100644 > > unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml > > > > diff --git a/Makefile.am b/Makefile.am > > index 4b9a901..07744e9 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -17,6 +17,7 @@ unstable_protocols = > > \ > > > > unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml > > \ > > unstable/xdg-output/xdg-output-unstable-v1.xml > > \ > > unstable/input-timestamps/input-timestamps-unstable-v1.xml \ > > + > > unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml > > \ > > $(NULL) > > > > stable_protocols = > > \ > > diff --git a/unstable/xdg-toplevel-decoration/README > > b/unstable/xdg-toplevel-decoration/README > > new file mode 100644 > > index 0000000..e927110 > > --- /dev/null > > +++ b/unstable/xdg-toplevel-decoration/README > > @@ -0,0 +1,4 @@ > > +xdg_toplevel_decoration protocol > > + > > +Maintainers: > > +Simon Ser <[email protected]> > > diff --git > > a/unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml > > b/unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml > > new file mode 100644 > > index 0000000..a34cb73 > > --- /dev/null > > +++ > > b/unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml > > @@ -0,0 +1,132 @@ > > +<?xml version="1.0" encoding="UTF-8"?> > > +<protocol name="xdg_toplevel_decoration_unstable_v1"> > > + <copyright> > > + Copyright © 2018 Simon Ser > > + > > + 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="zxdg_toplevel_decoration_manager_v1" version="1"> > > + <description summary="window decoration manager"> > > + This interface permits choosing between client-side and server-side > > + window decorations for a toplevel surface. > > + > > + A window decoration is a user interface component used to move, > > resize and > > + change a window's state. It can be managed either by the client > > (part of > > + the surface) or by the server. > > + > > + By advertising this interface the server announces support for > > server-side > > + window decorations. Note that even if the server supports server-side > > + window decorations, clients must still support client-side > > decorations. > > + > > + 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> > > + > > + <enum name="error"> > > + <entry name="unconfigured_buffer" value="1"/> > > + <entry name="already_constructed" value="2"/> > > + </enum> > > + > > + <request name="destroy" type="destructor"> > > + <description summary="destroy the decoration manager object"> > > + Destroy the decoration manager. > > + </description> > > + </request> > > + > > + <request name="get_decoration"> > > + <description summary="create a new decoration object"> > > + Create a new decoration object associated with the given toplevel. > > + > > + Creating an xdg_toplevel_decoration from an xdg_toplevel which has > > a > > + buffer attached or committed is a client error, and any attempts > > by a > > + client to attach or manipulate a buffer prior to the first > > + xdg_toplevel_decoration.configure call must also be treated as > > + errors. > > I'm missing a comment that describes what happens if the xdg_toplevel is > destroyed. There is some object dependency here that needs to be stated. Do > we need an event here? Or are we assuming that clients are smart enough to > keep track of destroy events. Either way - needs to be explicitly stated. What about requiring clients to destroy the zxdg_toplevel_decoration_v1 before the xdg_toplevel? > > + </description> > > + <arg name="id" type="new_id" > > interface="zxdg_toplevel_decoration_v1"/> > > + <arg name="toplevel" type="object" interface="xdg_toplevel"/> > > + </request> > > + </interface> > > + > > + <interface name="zxdg_toplevel_decoration_v1" version="1"> > > + <description summary="decoration object for a toplevel surface"> > > + The decoration object allows the client to switch between a > > client-side > > + and server-side window decoration for a toplevel surface. > > Needs a bit more expansion, because below it says: > "A configure event can be sent at any time, not necessarily in reply to a > set_mode request.". So it's not just the client that can switch, it's also > the compositor that can tell the client to do so. Yeah, you're right, that needs rephrasing. What about: The decoration object allows the compositor to switch between a client-side and server-side window decoration for a toplevel surface. The client can request to switch to another mode. > > + </description> > > + > > + <request name="destroy" type="destructor"> > > + <description summary="destroy the decoration object"> > > + Switch back to client-side-only window decoration mode at the next > > + commit. > > + </description> > > + </request> > > + > > + <enum name="mode"> > > + <description summary="window decoration modes"> > > + These values describe window decoration modes. > > + </description> > > + <entry name="client" value="1" summary="client-side window > > decoration"/> > > + <entry name="server" value="2" summary="server-side window > > decoration"/> > > chars are cheap: client_side and server_side doesn't hurt anyone and removes > the little bit of ambiguity, especially given how bad 'client' and 'server' > are already overloaded. Feel free to use CSD and SSD as well since that's a > fairly common acronym in this context, but maybe wait for all the colours of > the bikesheds to come in first :) Okay! > > + </enum> > > + > > + <request name="set_mode"> > > + <description summary="set the decoration mode"> > > + Set the toplevel surface decoration mode. > > + > > + After requesting a decoration mode, the compositor will respond by > > + emitting a xdg_surface.configure event. The client should then > > update > > + its content, drawing it with or without decorations depending on > > the > > + received mode. The client must also acknowledge the configure when > > + committing the new content (see xdg_surface.ack_configure). > > to clarify, this is xdg_surface.configure, not > zxdg_toplevel_decoration_v1.configure, right? Can we rename the latter to > avoid confusion here? Well, it's both. The client accumulates configure events from xdg_toplevel and zxdg_toplevel_decoration_v1, and xdg_surface.configure commits them (with a serial). Then the client must acknowledge all these configure events by sending xdg_surface.ack_configure. The zxdg_toplevel_decoration_v1.configure event is really just like xdg_toplevel.configure. > > + > > + The compositor can ignore this request. > > + </description> > > + <arg name="mode" type="uint" enum="mode" summary="the decoration > > mode"/> > > + </request> > > + > > + <event name="preferred_mode"> > > + <description summary="advertise the server's preferred mode"> > > + The preferred_mode event describes the server's preferred > > decoration > > s/server/compositor/ both times Noted. > > + mode for this toplevel surface. The event is sent when binding to > > the > > + decoration object and whenever the preferred mode changes. > > + </description> > > + <arg name="mode" type="uint" enum="mode" summary="the preferred > > mode"/> > > + </event> > > + > > + <event name="configure"> > > + <description summary="suggest a surface change"> > > + The configure event asks the client to change its decoration mode. > > The > > + configured state should not be applied immediately. See > > + xdg_surface.configure for details. > > + > > + A configure event can be sent at any time, not necessarily in > > reply to a > > + set_mode request. > > This implies that any client that talks that protocol can switch betwen CSD > and SSD and that the client *will* do so on request. That needs to be > explicitly stated. Otherwise you're looking at a race condition here where > a client that cannot/does not want to change cannot ack any future > xdg_surface.configure without confusing everyone. That's true. Will change to: A configure event can be sent at any time, not necessarily in reply to a set_mode request. The specified mode must be obeyed by the client. > Since we assume CSD by default, this implies that any client must be able to > do CSD, which should be explicitly stated here. It's already stated in the protocol description ("Note that even if the server supports server-side window decorations, clients must still support client-side decorations"). Is it necessary to write it one more time here? > Cheers, > Peter > > > + </description> > > + <arg name="mode" type="uint" enum="mode" summary="the decoration > > mode"/> > > + </event> > > + </interface> > > +</protocol> > > -- > > 2.16.2 > _______________________________________________ > wayland-devel mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
