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

Simon Ser contact at emersion.fr
Sun Mar 11 20:49:19 UTC 2018


On March 3, 2018 12:14 PM, Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:
> 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.

Thanks for your 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. :-)

xdg-shell's wording includes drop-shadows as part of decorations:

> Client-side decorations often have invisible portions like drop-shadows which
> should be ignored for the purposes of aligning, placing and constraining
> windows.

So "decorations" here stand for both visible parts such as the titlebar and
invisible parts such as drop-shadows. Do you think the protocol needs to
disambiguate these concepts?

> > +      By advertizing this interface the server anounces support for server-side
> > +      window decorations.
> 
> Typos: advertising and announces.

Right.

> > +
> > +      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.

Will do.

> > +        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.)

One reason is consistency with xdg-shell.

But there are also real use-cases:
- A compositor might prefer SSDs or CSDs depending of the window container. For
  instance, a tiled compositor might prefer SSDs for tiled windows and CSDs for
  floating windows. Windows can be moved between a tiling and a floating
  container at run-time.
- Clients might expose a user setting that allows toggling SSDs. For instance,
  Chrome/Chromium already has such a feature. Requiring the user to restart the
  app or to quickly close and reopen the main window offers poor UX.

> 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

You're right wrt the destructor description, the change should happen at the
next commit.

> 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.)

The change hasn't been accepted yet, so I'll wait a little. As for other
comments, I believe I covered them in my answer above.

> > +
> > +    <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.

I've added this in the protocol description:

> Note that even if the server supports server-side window decorations, clients
> must still support client-side decorations.

> Thanks,
> 
> -- 
> 
> Quentin “Sardem FF7” Glidic
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list