On Sat, 24 Mar 2018 07:08:15 -0400 Simon Ser <[email protected]> wrote:
> This adds a new protocol to negotiate server-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 v3 to v4: > - Updated the definition of decorations to remove unnecessary constraints > (Eike) > - Fix ambiguity in zxdg_toplevel_decoration_v1 description (Peter) > - Specify that the decoration object must be destroyed before the toplevel > (Pekka, Peter) > - Changed decoration mode enum to "client_side" and "server_side" (Peter) > - Replaced "server" with "compositor" in preferred_mode event description > (Peter) > - State that the mode sent by the compositor with the configure event must be > obeyed (Peter) > - Reword client-side decorations description (Eike) > > [1] https://github.com/swaywm/wlroots/pull/638 > > Makefile.am | 1 + > unstable/xdg-toplevel-decoration/README | 4 + > .../xdg-toplevel-decoration-unstable-v1.xml | 139 > +++++++++++++++++++++ > 3 files changed, 144 insertions(+) > create mode 100644 unstable/xdg-toplevel-decoration/README > create mode 100644 > unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml Hi, the below is my review mostly on the mechanical side of destructors and error conditions. > 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..8fbc688 > --- /dev/null > +++ b/unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml > @@ -0,0 +1,139 @@ > +<?xml version="1.0" encoding="UTF-8"?> > +<protocol name="xdg_toplevel_decoration_unstable_v1"> > + <copyright> > + Copyright © 2018 Simon Ser > + </copyright> > + > + <interface name="zxdg_toplevel_decoration_manager_v1" version="1"> > + <description summary="window decoration manager"> > + This interface allows a compositor to announce support for server-side > + decorations and optionally express a preference for using them. > + > + A window decoration is a set of window controls as deemed appropriate > by > + the party managing them, such as user interface components used to > move, > + resize and change a window's state. > + > + A client can use this protocol to request being decorated by a > supporting > + compositor. > + > + If compositor and client do not negotiate the use of a server-side > + decoration using this protocol, clients continue to self-decorate as > they > + see fit. > + > + 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"/> These error values could have a summary attribute to explain them in a sentence if you wish. Since the error values are defined in this interface, it means they have to be sent on the zxdg_toplevel_decoration_manager_v1 object. I suppose that's fine? If they were defined in zxdg_toplevel_decoration_v1 interface they could be sent on the specific protocol object whose creation caused them, but since all errors are non-recoverable, the difference is not significant. A compositor can deliver a more accurate error message in the string argument for debugging anyway. > + </enum> > + > + <request name="destroy" type="destructor"> > + <description summary="destroy the decoration manager object"> > + Destroy the decoration manager. Presumably this has no effect to other existing objects? Particularly, existing zxdg_toplevel_decoration_v1 continue to function as is. > + </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. Please, be specific on which error code to send on which condition. Peter just recently lectured to me about specifying error conditions: "If <something> then protocol error <foo> is raised." ;-) Do you specifically require compositor to raise an error for mere wl_surface.attach(wl_buffer) even if it has not seen a wl_surface.commit with it? s/call/event/ ? What if a client creates multiple zxdg_toplevel_decoration_v1 objects for the same xdg_toplevel object? > + </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 compositor to toggle server-side > window > + decorations for a toplevel surface. The client can request to switch to > + another mode. > + > + The xdg_toplevel_decoration object must be destroyed before its > + xdg_toplevel. If the client does the wrong destruction, what error code is raised? > + </description> > + > + <request name="destroy" type="destructor"> > + <description summary="destroy the decoration object"> > + Switch back to a mode without any server-side decorations at the next > + commit. Ok, the destruction effect here is latched on wl_surface.commit. Good. > + </description> > + </request> > + > + <enum name="mode"> > + <description summary="window decoration modes"> > + These values describe window decoration modes. > + </description> > + <entry name="client_side" value="1" summary="no server-side > decoration"/> > + <entry name="server_side" value="2" summary="server-side window > decoration"/> > + </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 without decorations if the received mode is > + server-side decorations. The client must also acknowledge the > configure > + when committing the new content (see xdg_surface.ack_configure). > + > + The compositor can ignore this request. > + </description> > + <arg name="mode" type="uint" enum="mode" summary="the decoration > mode"/> What error code is raised if the 'mode' value is invalid? > + </request> > + > + <event name="preferred_mode"> > + <description summary="advertise the compositor's preferred mode"> > + The preferred_mode event describes the compositor's preferred > decoration > + 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. I think the references to xdg_surface.configure and .ack_configure are a little weak. Should they be more explicit? I wonder if there is any standard wording to invoke the configure/ack_configure sequence. > + > + 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. > + </description> > + <arg name="mode" type="uint" enum="mode" summary="the decoration > mode"/> > + </event> > + </interface> > +</protocol> After I read through all the events and requests a few times, I got the full picture on how the sequence works. Would it be appropriate to have the whole initial sequence (create, preferred_mode, set_mode, configure, attach, ack_configure, commit) summarized in one of the interface descriptions? Thanks, pq
pgpQNPne9BjGM.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
