Feedback on xdg-shell v5

Thu Apr 28 11:08:07 UTC 2016

On Thu, 28 Apr 2016 17:36:58 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> On Thu, Apr 28, 2016 at 10:36:14AM +0200, Martin Graesslin wrote:
> > Hi Wayland and Plasma,
> > 
> > I finished the implementation of xdg-shell in KWayland [1] and KWin [2]. 
> > Overall it was more straight forward than I would have expected. Especially 
> > the changes to KWin were surprisingly minor (with one huge exception).
> > 
> > Now some questions/remarks:

> > * Why is the ping on the shell and not on the surface? I would have expected 
> > to ping the surface. At least that's how I would want to do it from KWin.  
> 
> Because you don't ping a surface, you ping the client. It's the client
> that may be inresponsive, and if the client is in responsive, it's safe
> to assume all its surfaces are as well.

Hi,

I was going to plain agree and say, that all events to a client come
through the same connection (wl_display), and it does not make sense to
have a series of ping events on different objects when they could just
be collapsed into one equivalent event.

But then I thought about multiple client-side event queues. If a client
has multiple queues, and windows on different queues, it could be
possible that only some queues get stuck while others are serviced.
With per-surface pings, the compositor should then "shade out" only the
windows where the queue is actually stuck.

Would it be worth it?

Or, is there an underlying assumption that a client might not send pong
requests as a simple reaction to ping events directly, but delay or
delegate it to some mechanism responsible for other updates?

That is, is the ping-pong protocol a keep-alive for the Wayland
connection, for client's event queues, or something deeper in the
client?

> > 
> > The biggest problem for me is the request set_window_geometry. I think I 
> > mentioned it already before on the wayland list. Basically I have no idea how 
> > to implement that in KWin. We have only one geometry for a window and that's 
> > mapped to the size of the surface/x11 pixmap. This geometry is used all over 
> > the place, for positioning, for resizing and for rendering. I cannot add 
> > support for this without either breaking code or having to introduce a new 
> > internal API. That's lots of work with high potential for breakage :-(

Have you looked at what you need to do to support windows that are
built from non-overlapping sub-surfaces, like what Jonas describes below?

I suspect you might end up having to do that major internal API work
anyway by the sounds of it. A window may be a collection of surfaces,
not just one.

How do you do the window geometry with server-side decorations? Why is
using only a part of client surface so different from using a
combination of client and server-only surfaces?

> > 
> > From the description it seems to be only relevant for shadows. Could we make 
> > shadows an optional part in xdg-shell? That the server can announce that it 
> > supports shadows in the surface?  
> 
> It's not only about shadow. Let me explain a scenario where a window
> geometry is needed, even when there is a mode where no shadow is drawn
> by the client.
> 
> Consider we have the following window:
> 
> 
>     +-------------------------------------------+
>     |                   A window              X |
>     +-------------------------------------------+
>     |                                           |
>     |                     /\                    |
>     |                    /  \                   |
>     |                   /    \                  |
>     |                  /      \                 |
>     |                 /________\                |
>     |                                           |
>     |                                           |
>     +-------------------------------------------+
> 
> It can be split into two logical rectangles/areas: the window title, and
> the main content. This window may be consisting of two separate
> non-overlapping surfaces, for example one GLES surface, and one SHM
> surface. Only one of these surfaces will be the "window", while the
> other will be a subsurface.
> 
> xdg_surface.set_window_geometry would here be used to describe, in
> relation to whatever surface was gets to act as "window", what the
> window geometry would be.
> 
> > 
> > Also I'm not quite sure about that, but it looks to me like QtWayland thinks 
> > that the set_window_geometry is the geometry of the client-side-decoration, 
> > while on GTK it looked to me like being the size of the shadow. Either I did 
> > not understand that correctly, or our toolkits are not compatible.  
> 
> Not sure what you mean here. The window geometry is the geometry of the
> window (main content, window frame, window title etc) excluding the
> shadow, in relation to the surface used to create the xdg_surface.
> 
> So if you have a surface which is 800x600, and shadow is 10 wide on all
> edges, then you'd have a window geometry which is x: 10, y: 10, width:
> 780, height: 580.
> 
> > 
> > At the moment I haven't implemented this request yet in KWayland as I would 
> > not be able to use it in KWin anyway.
> > 
> > As a note: if it's just about shadow for us in Plasma that's a rather useless 
> > request. We already have a Wayland shadow implementation which allows us to 
> > have the shadow outside the surface.  
> 
> So, what you are saying is that you want to change the xdg_shell to only
> guarantee support for hybrid the CSD-SSD case. I don't agree that that
> change would be for the best. There are a few reasons for this:
> 
> Wayland has always been "what you see is rendered by clients, then
> composited by the compositor". Of course there are cases where this is
> not true, for example there may be effects that are not possible to have
> a client do, but it is the direction we should be working against.
> Making the default CSD-SSD goes against that.
> 
> Weston (and mutter for that matter) I assume will never implement the
> default "properly" (i.e. rendering server side shadows). We'd always
> rely on clients implementing optional features to function properly.
> 
> It'd effectively make the default case broken and partly unusable in
> any compositor not supporting full the CSD+SSD hybrid solution (i.e.
> server side shadows). In the case where the client doesn't draw any
> shadow, and the server doesn't draw any shadow, it'll also make it hard
> to support interactive resize, since the shadow region is usually used
> as a trigger area for resizing. For example, if you have a border-less
> window, without shadow you'll suddenly have no area where you can let
> your user click-drag to resize the window. The alternative is to have
> logical invisible regions outside of the window implemented by the
> compositor, but that make this even more a hybrid solution, but now also
> with completely invisible special reagions.

Huh, I always wondered what practical use shadows might have apart
from visual appearance. I can also see how using shadows for any user
interaction while they are outside of the defined window geometry is
going to fail.

> I'd much rather see a solid default which does things "the Wayland way"
> (as described above) that will work reasonably well in a minimalistic
> default weston reference shell, and doesn't imply that the compositor
> should go into effect drawing territory, and I don't think the hybrid
> solution is this.

I'm not sure there is a "the Wayland way" really for shadows. We did
start with clients just drawing their own shadow as part of the
surface. I assumed that was only because it was very easy to do for a
little bit of eye-candy. We'd have to ask Kristian if there was more
though behind it than that.

I would hope you can find a way to extract shadows as a separate part
of the protocol where the compositor and client can agree on who draws
it, if at all. I'm sure you'll have exactly the same issue with window
decorations in general, especially considering if shadows are actually
functional rather than just visual.

IMHO, the default minimalistic way is no shadows at all for anything.
That would be the Wayland way if any, keeping it simple by default and
everyone equally grumpy ;-). If the compositor then supports shadows on
either server or client side, or both sides by client decision, that
would be something more.

Thanks,
pq

> > 
> > [1] https://phabricator.kde.org/D1506
> > [2] https://phabricator.kde.org/D1507  
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160428/d1e6aa9e/attachment.sig>