[PATCH wayland-protcols v3] unstable: add xdg-toplevel-decoration protocol
Peter Hutterer
peter.hutterer at who-t.net
Mon Mar 19 03:34:44 UTC 2018
On Wed, Mar 14, 2018 at 06:41:53AM -0400, Simon Ser wrote:
> On March 14, 2018 10:22 AM, Peter Hutterer <peter.hutterer at who-t.net> 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 <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 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 <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..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?
yeah, that makes sense and is the easiest one to implement and verify.
> > > + </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
typo: "between client-side" (without 'a')
> and server-side window decoration for a toplevel surface. The client can
> request to switch to another mode.
yep, works for me
> > > + </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.
ok, double-checked, there are multiple configure events already so *not*
renaming it configure would be more confusing. I withdraw my request.
> > > +
> > > + 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.
s/can/may/ because it sounds more formal :)
> > 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?
yes, because as you can see, by the time someone reads this event, they may
have already forgotten what was in the protocol description ;)
Cheers,
Peter
> > > + </description>
> > > + <arg name="mode" type="uint" enum="mode" summary="the decoration mode"/>
> > > + </event>
> > > + </interface>
> > > +</protocol>
> > > --
More information about the wayland-devel
mailing list