Feedback on xdg-shell v5

[Jonas Ådahl]

On Fri, Apr 29, 2016 at 12:37:22PM +0300, Pekka Paalanen wrote:
> On Fri, 29 Apr 2016 15:31:28 +0800
> Jonas Ådahl <jadahl at gmail.com> wrote:
> 
> > On Thu, Apr 28, 2016 at 02:08:07PM +0300, Pekka Paalanen wrote:
> > > 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?  
> > 
> > I doubt it. What would you do? It's not like you can disconnect half of
> > a client.
> 
> Wasn't it more about marking unresponsive surfaces and providing UI
> aids for dealing with them, e.g. moving the window out of the way?
> 
> If you hover over the "kill this app" option, the compositor could e.g.
> color all the surfaces that would go, not just the ones unresponsive.
> 
> > > 
> > > 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?  
> > 
> > My understanding of the ping-pong protocol is to make it possible for
> > the compositor to know when a client has frozen so that it can get rid
> > of it (ala "Force Quit").
> 
> Yeah, that's the final action, but there could be some less drastic
> options too.
> 
> > > > > 
> > > > > 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.  
> > 
> > Why would it fail? It has worked perfectly fine so far. With SSD, this
> > is how things are done as well on X11 - part of the shadow around the
> > window will be available for triggering window resizing. Think of the
> > shadow as part of the window border, instead of just shadow.
> 
> Is the shadow a part of window decorations but outside of the
> window geometry?

Yes.

> 
> When you then snap another window beside the first one, you'd snap
> based on the window geometry, right? Then you lose the shadow region of
> the window that happens to be below. The shadow of the window on top
> will extend into and clobber the window below. Or would snap leave a
> shadowy region between the windows? Or do you use window geometry to
> cull the shadow of both windows, in which case you lose it from both on
> the touching egdes?

Well, the protocol has no say in how snapping would be implemented or
how the compositor draws, but normally snapping would be against the
window geometry. Normally the shadow would clobber the window below,
yes, that is part of the purpose, i.e. to make it clear which window is
above and which is below.

> 
> But yeah, I have no idea how that works. I've never had nor wished for
> shadows, so I can't really understand the big deal about them. I'm
> happy having just nice decorations which are accounted into the window
> geometry.
> 
> > >   
> > > > 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.  
> > 
> > With "the Wayland way" I really only meant that the compositor, when
> > possible, doesn't spend time rendering eye candy and/or user interfaces.
> 
> Sure. This brings us to my suggestion of the default by the Wayland
> way: no shadows at all. No implementation is simpler implementation.
> 
> > > 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.  
> > 
> > We do indeed need a way to do these negotiations. But we also need to
> > have a sane, workable fallback as well.
> > 
> > > 
> > > 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.  
> 
> Right, so, first sorry for butting in with the above comment. I'm not a
> desktop designer and I don't know how desktop works.
> 
> I just couldn't stand by and watch how this discussion seemed to
> deadlock at the start where both sides just defend their own
> needs and cannot propose a middle-ground. So I'm here to make you both
> sad. ;-)
> 
> > 
> > It doesn't seem very minimalistic, because it really just means that
> > compositors will need to do server side shadow rendering. Clients,
> > especially the ones not using Qt, GTK+ or EFL, will have much less
> > incentive to add support for it client side, and we'd effectively end up
> > with either having to add all the complexity that server side shadow
> > involves in the compositor, or have an unusable default.
> 
> No, I do literally mean: no shadows at all by server nor clients,
> unless it is advertised.
> 
> > If we would to default to no client side shadows, we need to solve the
> > issues such a mode introduces which doesn't rely on server side shadows.
> > I don't know of any solutions.
> 
> Make the decorations thicker. That's how it worked before shadows were
> invented. Because, really, the shadows *are* just thickening of the
> decorations, especially if you intend to handle input on those regions
> which causes input to not go through to the window below. Whether you
> color it by window background gray or translucent black makes no
> difference.
> 
> 
> In IRC you asked what do I propose then. Ok, here's hand-waving. There
> are four different cases:
> 
> 1) No-one draws any shadows. This is the default. Clients know to not
> rely on shadows, so they will e.g. draw thicker decorations if they
> intend them to be used as handles etc.
> 
> 2) The server is capable of drawing shadows. The server advertises a
> global "server-shadows". This informs clients that if they don't do
> anything, they will get shadows by the server. The global interface
> could include a request to please don't make me shadows.
> 
> 3) The server supports client-side shadows. The server advertises a
> global "client-shadows" to note that it supports it, and clients can
> then draw their own shadows. If shadows were the only thing needing the
> window geometry request, it could be in that interface instead of the
> xdg-shell core set.
> 
> 4) Server supports both ways, so advertises both globals. A client can
> pick which one it wants.
> 
> With this setup, if you have a client that does not use any of those
> global shadow interfaces, then it will get only server-side shadows if
> the server supports it. Such a client would not draw its own shadows
> and not rely on shadows being there to be usable.
> 
> The only client implementation not supported by this scheme is when the
> client has no way to *not* draw shadows. How likely is that? Why would
> you need considerably more code to not do something?
> 
> Here the default behaviour would be accomplished by no-one implementing
> any additional code. If the server does not care about shadows, just
> don't implement anything. If the client does not care about shadows,
> just don't implement anything. If either does care about shadows, then
> you may as well be nice and respect the other party.
> 
> I phrased that with globals, but I suppose you could also replicate an
> equivalent advertise/support/interface mechanism under xdg-shell too.

I don't think this is a reasonable solution. It will put the burden
on application/toolkit designers to design for a use case which will
never be used since, by design, it's the crappy middle ground no one
will want to use. This wouldn't result in "no implementation" it would
effectively result in an extra implementation that'll probably never be
exercised.

I think we must at least support drawing a simple window as it has been
done for years now, and with client side decorations that requires a
window geometry which may be smaller than the surface geometry.

FWIW, popup menus are relevant here as well, as they also have drop
shadow. In the proposed xdg_shell v6 version, xdg_popup's may have
window geometry as well, and this is needed for the positioning system
so that the compositor can place popups within the working area.

> 
> 
> Unfortunately, the issue with CSD vs. SSD is not this simple, as
> someone *has to* draw decorations. I claim that we can live without
> shadows, but not without decorations.
> 
> Also note to everyone else, that I never mentioned anything about the
> user's "freedom" to say anything about the matter. The user wanting one
> or the other is irrelevant here. This is just about making compositors
> and client toolkits agree.

Indeed. I believe we should support freedom here, and this thread all
about the mode which is supposed to be the "base" mode.

For the records, the way I imagine this would work is somewhat different
from Pekka's suggestion, as I think the compositor should be the one who
decides the way a client should draw itself, as long as the client
supports it. Described in pseudo protocol, right now we do:

<- get xdg_surface
<- (xdg_surface.set_maximized/set_fullscreen/set_title, etc)
<- wl_surface.commit
-> xdg_surface.configure(maximized, WxH)
<- wl_surface.attach(buffer)
<- wl_surface.commit()

With negotiated drawing mode (be it CSD/SSD hybrid (no shadows) or SSD
(no shadow, no window title), it'd just be one extra pair of
configuration state:

<- get xdg_surface
<- (xdg_surface.set_maximized/set_fullscreen/set_title, etc)
<- xdg_surface.supports_modes(ssd|ssd+csd-hybrid)
<- wl_surface.commit
-> xdg_surface.configure(maximized, WxH, ssd)
<- wl_surface.attach(buffer)
<- wl_surface.commit()

Whether this eventually would become part of xdg_shell or implemented as
extensions is not practically relevant.


Jonas