[PATCH wayland-protcols v2] unstable: add xdg-toplevel-decoration protocol

Quentin Glidic sardemff7+wayland at sardemff7.net
Sat Mar 3 11:14:18 UTC 2018


On 3/2/18 4:33 PM, 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 <contact at emersion.fr>
> Reviewed-by: Drew DeVault <sir at cmpwn.com>
> Reviewed-by: David Edmundson <david at davidedmundson.co.uk>
> Reviewed-by: Alan Griffiths <alan.griffiths at canonical.com>
> Reviewed-by: Tony Crisci <tony at dubstepdish.com>
> 
> [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 is a nice protocol, and I would implement it (in clients) in its 
current form, but I still have a few comments.


> ---
> 
> 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 v1 to v2: moved the enum above in the protocol, added
> Signed-off-by and Reviewed-by tags, updated the commit message.
> 
> [1] https://github.com/swaywm/wlroots/pull/638
> 
>   Makefile.am                                        |   1 +
>   unstable/xdg-toplevel-decoration/README            |   4 +
>   .../xdg-toplevel-decoration-unstable-v1.xml        | 127 +++++++++++++++++++++
>   3 files changed, 132 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 <contact at emersion.fr>
> 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..acc1ed2
> --- /dev/null
> +++ b/unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml
> @@ -0,0 +1,127 @@
> +<?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.

You mention decorations only, but this (or some other text below maybe) 
should mention shadows as well. You may consider them as part of the 
decoration, but xdg_surface.set_window_geometry() is designed to include 
decoration and exclude shadows. Yet, e.g. GTK+ allows to resize using 
shadows IIRC.

Nit: I think GTK+ allows to move from any dead zone in the surface too. :-)


> +      By advertizing this interface the server anounces support for server-side
> +      window decorations.

Typos: advertising and announces.


> +
> +      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 a zxdg_toplevel_decoration_v1 from a xdg_toplevel which has a

Nit: always use the final names in the descriptions, that it is still 
clear enough and it avoids noise in future patches.


> +        buffer attached or committed is a client error, and any attempts by a
> +        client to attach or manipulate a buffer prior to the first
> +        zxdg_toplevel_decoration_v1.configure call must also be treated as
> +        errors.
> +      </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.
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the decoration object">
> +        Switch back to client-side-only window decoration mode.
> +      </description>
> +    </request>
> +
> +    <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).
> +
> +        The compositor can ignore this request.
> +      </description>
> +      <arg name="mode" type="uint" enum="mode" summary="the decoration mode"/>
> +    </request>

This request I’m still not sure about it.
Why would an SSD-capable client would want to switch from CSD 
back-and-forth? What is the use case here? (Preferably a user use case.)

If there is a serious use case, then it is probably the nicest way. 
Though I can think of another one:
1a. destroying the object gets back to CSD *at the next commit*. (Your 
destructor description is not clear on when it happens, and yet it says 
it will happen ;-))
1b. destroying the xdg_surface altogether gets back to CSD.
2. recreating an xdg_surface gets back to SSD

If there is none, destroying the object can just be an error.

IMO, the configure event is enough to carry the compositor decision, and 
a client wanting SSD has little chance to want CSD back.

If we drop the request, it can always be added back as an addition to 
unstable-v1 (since adding is not a breaking change).

If we keep the request we should change the wording, as I commented on 
IRC, to “the compositor may respond”. (There is a pending patch to 
xdg-shell for that too.)


> +
> +    <event name="preferred_mode">
> +      <description summary="advertise the server's preferred mode">
> +        The preferred_mode event describes the server'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.
> +      </description>
> +      <arg name="mode" type="uint" enum="mode" summary="the decoration mode"/>
> +    </event>
> +
> +    <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"/>
> +    </enum>
> +  </interface>
> +</protocol>
> 

Last but not least: it should be much much clearer that the compositor 
is in charge here. This is not about magic SSD, clients must support CSD 
in all cases and should not error out if this global is not here. And 
even then, it may want CSD in some cases.

Thanks,

-- 

Quentin “Sardem FF7” Glidic


More information about the wayland-devel mailing list