<div dir="ltr"> <div class="gmail_quote"><div dir="ltr">On Fri, Apr 29, 2016 at 4:38 AM Martin Graesslin <<a href="mailto:mgraesslin@kde.org">mgraesslin@kde.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thursday, April 28, 2016 5:36:58 PM CEST Jonas Ådahl 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: > > * What's a server supposed to do in use_unstable_version? > > If a client requests the wrong version, the compositor should send an > error, effectively disconnecting the client. > > Note that this request is gone in v6 and is replaced by the same > unstable-protocol-version semantics used in wayland-protocols. Ok, that's what I though it is. I was just very unsure as killing the client is nothing I would consider "negotiate version" :-P > > > * 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. Sorry, but I doubt this will work in practice. I have experienced many freezes with QtWayland applications and in all cases it would respond to the ping. Consider: * Thread 1: dead-locked waiting for a frame callback * Thread 2: Wayland connection getting ping, thread alive, will send pong I fear that the concept of pinging clients doesn't work in a world where connections are in a thread. Anyway if it's about whether the client is responsive, I suggest to make it a dedicated protocol. It's not really something which belongs to xdg-shell, it's a more general concept. </blockquote><div> </div><div>I disagree that ping should be outside xdg-shell. The spec for this is perfectly functional and has been successfully implemented by a number of compositors/toolkits; claiming that you doubt its practicality based on your choice to add complexity by using threads (and the bugs that you've described in your implementation) is not a strong argument towards proving that ping is not able to be successfully implemented.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > > > * Would it be possible to split the states and geometry? I find it weird > > that I have to send a configure request with a size of 0/0 when informing > > a surface that it's active. > > Possible - yes, worth it? I don't know. It's clearly specified that > 0 means "let the client come up with it". If you think there is a clear > benefit, you're welcome to send a patch targeting v6. Yes, I think it might be worth it. I don't want that the client starts to think it can pick a different size when it becomes active. </blockquote><div> </div><div>Compositors can continue to send sizes for all configure events. There is nothing in the spec which prohibits this behavior, it only says that sizes may be omitted.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > > > 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 :-( > > > > 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. sorry, I didn't get the example at all. > > > 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. Ah I mixed that around: On GTK it looks like being the geometry of the window without the shadow (but with decoration), on Qt it seems to be the geometry of the window without the decoration. > > 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: I'm not saying anything about CSD-SSD here. I'm only talking about the shadow. For us the geoemtry is difficult to implement and if it's only about shadows it's difficult to justify that I spend weeks on implementing that or risk the stability of our overall product due to the heavy changes this will need. Cheers Martin_______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div></div>