On Fri, Apr 29, 2016 at 4:38 AM Martin Graesslin <mgraess...@kde.org> wrote:
> 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. > 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. > > > > > > * 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. > 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. > > > > > > 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 > wayland-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel