<div dir="ltr"><br><br><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:<br></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:<br>
> On Thu, Apr 28, 2016 at 10:36:14AM +0200, Martin Graesslin wrote:<br>
> > Hi Wayland and Plasma,<br>
> ><br>
> > I finished the implementation of xdg-shell in KWayland [1] and KWin [2].<br>
> > Overall it was more straight forward than I would have expected.<br>
> > Especially<br>
> > the changes to KWin were surprisingly minor (with one huge exception).<br>
> ><br>
> > Now some questions/remarks:<br>
> > * What's a server supposed to do in use_unstable_version?<br>
><br>
> If a client requests the wrong version, the compositor should send an<br>
> error, effectively disconnecting the client.<br>
><br>
> Note that this request is gone in v6 and is replaced by the same<br>
> unstable-protocol-version semantics used in wayland-protocols.<br>
<br>
Ok, that's what I though it is. I was just very unsure as killing the client<br>
is nothing I would consider "negotiate version" :-P<br>
<br>
><br>
> > * Why is the ping on the shell and not on the surface? I would have<br>
> > expected to ping the surface. At least that's how I would want to do it<br>
> > from KWin.<br>
> Because you don't ping a surface, you ping the client. It's the client<br>
> that may be inresponsive, and if the client is in responsive, it's safe<br>
> to assume all its surfaces are as well.<br>
<br>
Sorry, but I doubt this will work in practice. I have experienced many freezes<br>
with QtWayland applications and in all cases it would respond to the ping.<br>
Consider:<br>
* Thread 1: dead-locked waiting for a frame callback<br>
* Thread 2: Wayland connection getting ping, thread alive, will send pong<br>
<br>
I fear that the concept of pinging clients doesn't work in a world where<br>
connections are in a thread.<br>
<br>
Anyway if it's about whether the client is responsive, I suggest to make it a<br>
dedicated protocol. It's not really something which belongs to xdg-shell, it's<br>
a more general concept.<br></blockquote><div><br></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">
<br>
><br>
> > * Would it be possible to split the states and geometry? I find it weird<br>
> > that I have to send a configure request with a size of 0/0 when informing<br>
> > a surface that it's active.<br>
><br>
> Possible - yes, worth it? I don't know. It's clearly specified that<br>
> 0 means "let the client come up with it". If you think there is a clear<br>
> benefit, you're welcome to send a patch targeting v6.<br>
<br>
Yes, I think it might be worth it. I don't want that the client starts to<br>
think it can pick a different size when it becomes active.<br></blockquote><div><br></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">
<br>
><br>
> > The biggest problem for me is the request set_window_geometry. I think I<br>
> > mentioned it already before on the wayland list. Basically I have no idea<br>
> > how to implement that in KWin. We have only one geometry for a window and<br>
> > that's mapped to the size of the surface/x11 pixmap. This geometry is<br>
> > used all over the place, for positioning, for resizing and for rendering.<br>
> > I cannot add support for this without either breaking code or having to<br>
> > introduce a new internal API. That's lots of work with high potential for<br>
> > breakage :-(<br>
> ><br>
> > From the description it seems to be only relevant for shadows. Could we<br>
> > make shadows an optional part in xdg-shell? That the server can announce<br>
> > that it supports shadows in the surface?<br>
><br>
> It's not only about shadow. Let me explain a scenario where a window<br>
> geometry is needed, even when there is a mode where no shadow is drawn<br>
> by the client.<br>
><br>
> Consider we have the following window:<br>
><br>
><br>
> +-------------------------------------------+<br>
><br>
> | A window X |<br>
><br>
> +-------------------------------------------+<br>
><br>
> | /\ |<br>
> |<br>
> | / \ |<br>
> |<br>
> | / \ |<br>
> |<br>
> | / \ |<br>
> |<br>
> | /________\ |<br>
><br>
> +-------------------------------------------+<br>
><br>
> It can be split into two logical rectangles/areas: the window title, and<br>
> the main content. This window may be consisting of two separate<br>
> non-overlapping surfaces, for example one GLES surface, and one SHM<br>
> surface. Only one of these surfaces will be the "window", while the<br>
> other will be a subsurface.<br>
><br>
> xdg_surface.set_window_geometry would here be used to describe, in<br>
> relation to whatever surface was gets to act as "window", what the<br>
> window geometry would be.<br>
<br>
sorry, I didn't get the example at all.<br>
><br>
> > Also I'm not quite sure about that, but it looks to me like QtWayland<br>
> > thinks that the set_window_geometry is the geometry of the<br>
> > client-side-decoration, while on GTK it looked to me like being the size<br>
> > of the shadow. Either I did not understand that correctly, or our<br>
> > toolkits are not compatible.<br>
> Not sure what you mean here. The window geometry is the geometry of the<br>
> window (main content, window frame, window title etc) excluding the<br>
> shadow, in relation to the surface used to create the xdg_surface.<br>
<br>
Ah I mixed that around: On GTK it looks like being the geometry of the window<br>
without the shadow (but with decoration), on Qt it seems to be the geometry of<br>
the window without the decoration.<br>
<br>
><br>
> So if you have a surface which is 800x600, and shadow is 10 wide on all<br>
> edges, then you'd have a window geometry which is x: 10, y: 10, width:<br>
> 780, height: 580.<br>
><br>
> > At the moment I haven't implemented this request yet in KWayland as I<br>
> > would<br>
> > not be able to use it in KWin anyway.<br>
> ><br>
> > As a note: if it's just about shadow for us in Plasma that's a rather<br>
> > useless request. We already have a Wayland shadow implementation which<br>
> > allows us to have the shadow outside the surface.<br>
><br>
> So, what you are saying is that you want to change the xdg_shell to only<br>
> guarantee support for hybrid the CSD-SSD case. I don't agree that that<br>
> change would be for the best. There are a few reasons for this:<br>
<br>
I'm not saying anything about CSD-SSD here. I'm only talking about the shadow.<br>
<br>
For us the geoemtry is difficult to implement and if it's only about shadows<br>
it's difficult to justify that I spend weeks on implementing that or risk the<br>
stability of our overall product due to the heavy changes this will need.<br>
<br>
Cheers<br>
Martin_______________________________________________<br>
wayland-devel mailing list<br>
<a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a><br>
</blockquote></div></div>