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

[Quentin Glidic]

(I wonder if Pekka or Daniel would like to be dropped off the CC list as 
it may be too desktop-ish for them?)


On 3/3/18 10:58 AM, Simon Ser wrote:

Hi,

Thanks for taking time reviewing the protocol and suggesting an alternative 
design.

On March 2, 2018 4:36 PM, Mike Blumenkrantz  
wrote:

Hi,

I agree with your point regarding a SSD-capable compositor not always wanting 
them, and certainly I can see the usefulness for cases such as what you've 
cited. However, in the example you provided, it's easy enough for an 
application to determine what desktop it's running in and then determine 
whether to use SSD--if the compositor implements and advertises this protocol 
then it's ultimately a client decision whether to use it.


I don't think "detecting the desktop environment" is a good idea. Some other 
DEs (elementary, Unity) might also prefer GTK+ CSDs even if they're not GNOME, and - 
supposing there's a good way to know which DE is currently running ­- hardcoding the 
names of DEs is a slippery slope.


Just a side note here: it is a slope clients slipped on already if they 
want to integrate with some DE settings. But I agree that it should be 
as limited as possible (and hopefully in some helper library/toolkit).




My issue is in the inconsistency of the protocol that has been proposed here. 
This is a protocol for enabling SSD; there should be no need for anything 
related to CSD, and I don't think there is a need for clients to have any form 
of protocol request to toggle between CSD and SSD.


This is not a protocol to enable SSDs, this is a protocol to negotiate client- 
or server-side decorations. This is stated in the protocol description.


Based on your assertion that borderless mode would be achieved in this protocol 
in the CSD mode, I would raise the counterpoint that in this case the client 
should just destroy the zxdg\_toplevel\_decoration_v1 object which, according 
to the destroy request, would revert to the surface using CSD anyway, 
furthering my assessment that there is indeed no need for any mention of CSD in 
the protocol requests.


This is racy - there will be frames with both client-side and server-side 
decorations.


(Will answer and elaborate on v2.)


This change allows for everything except the destroy request to be removed from 
zxdg\_toplevel\_decoration_v1, greatly simplifying the protocol as well as 
implementations of it. The case of the compositor "ignoring" the request to 
create SSD also becomes moot as well, since all that would mean is the compositor has 
chosen to use a borderless state at this time (e.g., tiling layout) which is irrelevant 
to the client anyway.


I don't understand your last point. A tiling layout would be exposed as SSD to 
the client.


Or not, tiling is orthogonal to CSD/SSD. We will have a tiling state in 
xdg-shell at some point, hopefully, that will enable nicer visuals for 
CSD too.




This aside, there's some other items of note here, such as the lack of 
precision related to when the change in decoration management state takes 
effect: most likely it should be applied on the next surface commit? I think 
this should be specified in the zxdg\_toplevel\_decoration_v1 interface 
description. Also, I would think that destroying the 
zxdg\_toplevel\_decoration\_manager\_v1 object while any 
zxdg\_toplevel\_decoration_v1 objects exist should be a client error, but this 
is similarly not specified anywhere.


The protocol integrates with the xdg-shell configuration mechanism, and I've been using 
the same wording as xdg-shell. xdg-toplevel-decoration just adds one property to the set 
of double-buffered xdg-surface properties (just like xdg-toplevel adds properties to 
xdg-surface). There are a bunch of "See xdg_surface.configure" in the 
description and as an extension of xdg-shell the protocol assumes the reader knows how 
xdg-shell works.


And it integrates rather well, IMO.
I have a few more comments that I will post on v2.

Thanks,

> [snip]

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

[Simon Ser]

Hi,

Thanks for taking time reviewing the protocol and suggesting an alternative 
design.

On March 2, 2018 4:36 PM, Mike Blumenkrantz  
wrote:
> Hi,
> 
> I agree with your point regarding a SSD-capable compositor not always wanting 
> them, and certainly I can see the usefulness for cases such as what you've 
> cited. However, in the example you provided, it's easy enough for an 
> application to determine what desktop it's running in and then determine 
> whether to use SSD--if the compositor implements and advertises this protocol 
> then it's ultimately a client decision whether to use it.

I don't think "detecting the desktop environment" is a good idea. Some other 
DEs (elementary, Unity) might also prefer GTK+ CSDs even if they're not GNOME, 
and - supposing there's a good way to know which DE is currently running ­- 
hardcoding the names of DEs is a slippery slope.

> My issue is in the inconsistency of the protocol that has been proposed here. 
> This is a protocol for enabling SSD; there should be no need for anything 
> related to CSD, and I don't think there is a need for clients to have any 
> form of protocol request to toggle between CSD and SSD.

This is not a protocol to enable SSDs, this is a protocol to negotiate client- 
or server-side decorations. This is stated in the protocol description.

> Based on your assertion that borderless mode would be achieved in this 
> protocol in the CSD mode, I would raise the counterpoint that in this case 
> the client should just destroy the zxdg\_toplevel\_decoration_v1 object 
> which, according to the destroy request, would revert to the surface using 
> CSD anyway, furthering my assessment that there is indeed no need for any 
> mention of CSD in the protocol requests.

This is racy - there will be frames with both client-side and server-side 
decorations.

> This change allows for everything except the destroy request to be removed 
> from zxdg\_toplevel\_decoration_v1, greatly simplifying the protocol as well 
> as implementations of it. The case of the compositor "ignoring" the request 
> to create SSD also becomes moot as well, since all that would mean is the 
> compositor has chosen to use a borderless state at this time (e.g., tiling 
> layout) which is irrelevant to the client anyway.

I don't understand your last point. A tiling layout would be exposed as SSD to 
the client.

> This aside, there's some other items of note here, such as the lack of 
> precision related to when the change in decoration management state takes 
> effect: most likely it should be applied on the next surface commit? I think 
> this should be specified in the zxdg\_toplevel\_decoration_v1 interface 
> description. Also, I would think that destroying the 
> zxdg\_toplevel\_decoration\_manager\_v1 object while any 
> zxdg\_toplevel\_decoration_v1 objects exist should be a client error, but 
> this is similarly not specified anywhere.

The protocol integrates with the xdg-shell configuration mechanism, and I've 
been using the same wording as xdg-shell. xdg-toplevel-decoration just adds one 
property to the set of double-buffered xdg-surface properties (just like 
xdg-toplevel adds properties to xdg-surface). There are a bunch of "See 
xdg_surface.configure" in the description and as an extension of xdg-shell the 
protocol assumes the reader knows how xdg-shell works.

> Regards,
> 
> Mike
> 
> On Thu, Mar 1, 2018 at 1:18 PM Simon Ser  wrote:
> 
> > Hi Mike,
> > 
> > I don't think a compositor supporting the protocol means that it always 
> > wants SSDs. For instance, I can imagine a DE like GNOME supporting it:
> > 
> > - Allowing clients that prefer SSDs to use SSDs, so that toolkits like GLFW 
> > or winit and apps like mpv don't have to draw decorations that'll look 
> > alien and don't respect the user's preferences
> > - But still preferring CSDs, so that GTK+ apps can use them
> > 
> > Also, the CSD mode allows the client to do not draw decorations at all.
> > 
> > Regards,
> > 
> > On March 1, 2018 7:01 PM, Mike Blumenkrantz 
> >  wrote:
> > > Hi,
> > >
> > > This patch is certainly an improvement upon the previous protocol draft 
> > > which I reviewed, but it still does not address the most significant 
> > > issue that I pointed out, which is that it both is too complex and lacks 
> > > features. Why is there any "mode" when this is a protocol for enabling 
> > > server-side decorations? By using the protocol in a server or client, you 
> > > are enabling server-side decoration; why would there be any mention of 
> > > client-side at all? If the compositor, for whatever reason, decides to do 
> > > a runtime disable of SSD then it can simply destroy the global.
> > >
> > > Please see again my mail on this topic from the previous thread: 
> > > https://lists.freedesktop.org/archives/wayland-devel/2018-January/036495.html
> > >
> > > Regards,
> > >
> > > Mike

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

[Mike Blumenkrantz]

Hi,

I agree with your point regarding a SSD-capable compositor not always
wanting them, and certainly I can see the usefulness for cases such as what
you've cited. However, in the example you provided, it's easy enough for an
application to determine what desktop it's running in and then determine
whether to use SSD--if the compositor implements and advertises this
protocol then it's ultimately a client decision whether to use it.

My issue is in the inconsistency of the protocol that has been proposed
here. This is a protocol for enabling SSD; there should be no need for
anything related to CSD, and I don't think there is a need for clients to
have any form of protocol request to toggle between CSD and SSD.

Based on your assertion that borderless mode would be achieved in this
protocol in the CSD mode, I would raise the counterpoint that in this case
the client should just destroy the zxdg_toplevel_decoration_v1 object
which, according to the destroy request, would revert to the surface using
CSD anyway, furthering my assessment that there is indeed no need for any
mention of CSD in the protocol requests.

This change allows for everything except the destroy request to be removed
from zxdg_toplevel_decoration_v1, greatly simplifying the protocol as well
as implementations of it. The case of the compositor "ignoring" the request
to create SSD also becomes moot as well, since all that would mean is the
compositor has chosen to use a borderless state at this time (e.g., tiling
layout) which is irrelevant to the client anyway.

This aside, there's some other items of note here, such as the lack of
precision related to when the change in decoration management state takes
effect: most likely it should be applied on the next surface commit? I
think this should be specified in the zxdg_toplevel_decoration_v1 interface
description. Also, I would think that destroying the
zxdg_toplevel_decoration_manager_v1
object while any zxdg_toplevel_decoration_v1 objects exist should be a
client error, but this is similarly not specified anywhere.

Regards,
Mike

On Thu, Mar 1, 2018 at 1:18 PM Simon Ser  wrote:

> Hi Mike,
>
> I don't think a compositor supporting the protocol means that it always
> wants SSDs. For instance, I can imagine a DE like GNOME supporting it:
> - Allowing clients that prefer SSDs to use SSDs, so that toolkits like
> GLFW or winit and apps like mpv don't have to draw decorations that'll look
> alien and don't respect the user's preferences
> - But still preferring CSDs, so that GTK+ apps can use them
>
> Also, the CSD mode allows the client to do not draw decorations at all.
>
> Regards,
>
> On March 1, 2018 7:01 PM, Mike Blumenkrantz <
> michael.blumenkra...@gmail.com> wrote:
> > Hi,
> >
> > This patch is certainly an improvement upon the previous protocol draft
> which I reviewed, but it still does not address the most significant issue
> that I pointed out, which is that it both is too complex and lacks
> features. Why is there any "mode" when this is a protocol for enabling
> server-side decorations? By using the protocol in a server or client, you
> are enabling server-side decoration; why would there be any mention of
> client-side at all? If the compositor, for whatever reason, decides to do a
> runtime disable of SSD then it can simply destroy the global.
> >
> > Please see again my mail on this topic from the previous thread:
> https://lists.freedesktop.org/archives/wayland-devel/2018-January/036495.html
> >
> > Regards,
> >
> > Mike
> >
> > On Sun, Feb 18, 2018 at 3:01 PM Simon Ser  wrote:
> >
> > > This adds a new protocol to negotiate server- and client-side
> rendering of
> > >
> > > window decorations for xdg-toplevels.
> > >
> > > \-\-\-
> > >
> > > This is inspired by a protocol from KDE\[0\] which has been
> implemented in KDE
> > >
> > > and Sway and was submitted for consideration in 2017\[1\]. This patch
> provides an
> > >
> > > updated protocol with those concerns taken into account.
> > >
> > > This was iterated on privately between representatives of Sway and
> wlroots
> > >
> > > (Simon Ser and Drew DeVault), KDE (David Edmundson), and Mir (Alan
> Griffiths).
> > >
> > > A proof-of-concept of a client and server implementation is available
> at \[2\].
> > >
> > > \[0\]
> https://github.com/KDE/kwayland/blob/master/src/client/protocols/server-decoration.xml
> > >
> > > \[1\]
> https://lists.freedesktop.org/archives/wayland-devel/2017-October/035564.html
> > >
> > > \[2\] 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
> 

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

[Simon Ser]

Hi Mike,

I don't think a compositor supporting the protocol means that it always wants 
SSDs. For instance, I can imagine a DE like GNOME supporting it:
- Allowing clients that prefer SSDs to use SSDs, so that toolkits like GLFW or 
winit and apps like mpv don't have to draw decorations that'll look alien and 
don't respect the user's preferences
- But still preferring CSDs, so that GTK+ apps can use them

Also, the CSD mode allows the client to do not draw decorations at all.

Regards,

On March 1, 2018 7:01 PM, Mike Blumenkrantz  
wrote:
> Hi,
> 
> This patch is certainly an improvement upon the previous protocol draft which 
> I reviewed, but it still does not address the most significant issue that I 
> pointed out, which is that it both is too complex and lacks features. Why is 
> there any "mode" when this is a protocol for enabling server-side 
> decorations? By using the protocol in a server or client, you are enabling 
> server-side decoration; why would there be any mention of client-side at all? 
> If the compositor, for whatever reason, decides to do a runtime disable of 
> SSD then it can simply destroy the global.
> 
> Please see again my mail on this topic from the previous thread: 
> https://lists.freedesktop.org/archives/wayland-devel/2018-January/036495.html
> 
> Regards,
> 
> Mike
> 
> On Sun, Feb 18, 2018 at 3:01 PM Simon Ser  wrote:
> 
> > This adds a new protocol to negotiate server- and client-side rendering of
> > 
> > window decorations for xdg-toplevels.
> > 
> > \-\-\-
> > 
> > This is inspired by a protocol from KDE\[0\] which has been implemented in 
> > KDE
> > 
> > and Sway and was submitted for consideration in 2017\[1\]. This patch 
> > provides an
> > 
> > updated protocol with those concerns taken into account.
> > 
> > This was iterated on privately between representatives of Sway and wlroots
> > 
> > (Simon Ser and Drew DeVault), KDE (David Edmundson), and Mir (Alan 
> > Griffiths).
> > 
> > A proof-of-concept of a client and server implementation is available at 
> > \[2\].
> > 
> > \[0\] 
> > https://github.com/KDE/kwayland/blob/master/src/client/protocols/server-decoration.xml
> > 
> > \[1\] 
> > https://lists.freedesktop.org/archives/wayland-devel/2017-October/035564.html
> > 
> > \[2\] 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 000..e927110
> > 
> > \-\-\- /dev/null
> > 
> > \+\+\+ b/unstable/xdg-toplevel-decoration/README
> > 
> > @@ -0,0 +1,4 @@
> > 
> > +xdg\_toplevel\_decoration protocol
> > 
> > +
> > 
> > +Maintainers:
> > 
> > +Simon Ser 
> > 
> > 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 000..acc1ed2
> > 
> > \-\-\- /dev/null
> > 
> > \+\+\+ 
> > b/unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml
> > 
> > @@ -0,0 +1,127 @@
> > 
> > +
> > 
> > +
> > 
> > +  
> > 
> > +    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 

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

[Mike Blumenkrantz]

Hi,

This patch is certainly an improvement upon the previous protocol draft
which I reviewed, but it still does not address the most significant issue
that I pointed out, which is that it both is too complex and lacks
features. Why is there any "mode" when this is a protocol for enabling
server-side decorations? By using the protocol in a server or client, you
are enabling server-side decoration; why would there be any mention of
client-side at all? If the compositor, for whatever reason, decides to do a
runtime disable of SSD then it can simply destroy the global.

Please see again my mail on this topic from the previous thread:
https://lists.freedesktop.org/archives/wayland-devel/2018-January/036495.html

Regards,
Mike

On Sun, Feb 18, 2018 at 3:01 PM Simon Ser  wrote:

> This adds a new protocol to negotiate server- and client-side rendering of
> window decorations for xdg-toplevels.
> ---
> This is inspired by a protocol from KDE[0] which has been implemented in
> KDE
> and Sway and was submitted for consideration in 2017[1]. This patch
> provides an
> updated protocol with those concerns taken into account.
>
> This was iterated on privately between representatives of Sway and wlroots
> (Simon Ser and Drew DeVault), KDE (David Edmundson), and Mir (Alan
> Griffiths).
>
> A proof-of-concept of a client and server implementation is available at
> [2].
>
> [0]
> https://github.com/KDE/kwayland/blob/master/src/client/protocols/server-decoration.xml
> [1]
> https://lists.freedesktop.org/archives/wayland-devel/2017-October/035564.html
> [2] 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 000..e927110
> --- /dev/null
> +++ b/unstable/xdg-toplevel-decoration/README
> @@ -0,0 +1,4 @@
> +xdg_toplevel_decoration protocol
> +
> +Maintainers:
> +Simon Ser 
> 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 000..acc1ed2
> --- /dev/null
> +++
> b/unstable/xdg-toplevel-decoration/xdg-toplevel-decoration-unstable-v1.xml
> @@ -0,0 +1,127 @@
> +
> +
> +  
> +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.
> +  
> +
> +  
> +
> +  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 advertizing this interface the server anounces support for
> server-side
> +  window decorations.
> +
> +  Warning!