[PATCH wayland-protocols v4] unstable: add xdg-toplevel-decoration protocol

Pekka Paalanen ppaalanen at gmail.com
Mon Apr 30 14:07:24 UTC 2018


On Sat, 24 Mar 2018 07:08:15 -0400
Simon Ser <contact at emersion.fr> 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 <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 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
-------------- 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/20180430/ebf4d536/attachment.sig>


More information about the wayland-devel mailing list