Review Request 126301: Add protocol for server side decoration

[Sebastian Kügler]

> On Dec. 11, 2015, 1:54 p.m., Sebastian Kügler wrote:
> > src/server/display.cpp, line 330
> > <https://git.reviewboard.kde.org/r/126301/diff/1/?file=421564#file421564line330>
> >
> >     Also, I'm confused.
> >     
> >     you connect to d, so d ends up being [this], which you then don't use, but you use d, and delete that?
> >     
> >     Please revisit so some muggle like me understands this. :)
> 
> Martin Gräßlin wrote:
>     I don't connect to d, d is the context. As I just read a review request by you where you never specified the context: please read up in the documentation about the meaning of the context in lambda connects. This prevents against "surprises"
>     
>     The actual mistake is the capture of this which is not needed. That's a consistent copy and paste error in Display

Well, I did, and that was why you confused me. Fixing the capturing makes it logical to me. :)


> On Dec. 11, 2015, 1:54 p.m., Sebastian Kügler wrote:
> > src/client/server_decoration.h, line 179
> > <https://git.reviewboard.kde.org/r/126301/diff/1/?file=421560#file421560line179>
> >
> >     apidocs missing
> >     
> >     It would be useful to explain the modes here, and possibly mention than None means that the server expects the client to do CSD.
> 
> Martin Gräßlin wrote:
>     none means that neither client nor server should do CSD.

Put that in the apidocs, then. :-) 

(*I* know what it's doing, the user of this API may not.)


> On Dec. 11, 2015, 1:54 p.m., Sebastian Kügler wrote:
> > src/server/CMakeLists.txt, line 100
> > <https://git.reviewboard.kde.org/r/126301/diff/1/?file=421562#file421562line100>
> >
> >     I don't get the name here, maybe surface_decoration would be better? (It's not about the server, it's about decoration a surface.)
> >     
> >     Naming of course bleeds all over the whole protocol, so consider this a generic comment. :)
> 
> Martin Gräßlin wrote:
>     I have to think about it.
> 
> Martin Gräßlin wrote:
>     I just read through the protocol again and given the documentation for the manager it's all about server-side decorations: "By announcing support for this interface the server indicates that it supports server-side decorations."
>     
>     So this means it's more about the server-side decorations than about surface decorations. After all even without the protocol windows are decorated.
>     
>     So I tend to keep the name as it is, surface_decoration is not more precise. I'm open for other suggestions of course.

Alright, let's go for it, then.


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126301/#review89348
-----------------------------------------------------------


On Dec. 16, 2015, 11:07 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126301/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2015, 11:07 a.m.)
> 
> 
> Review request for Plasma and Sebastian Kügler.
> 
> 
> Repository: kwayland
> 
> 
> Description
> -------
> 
> [client] Add implementation for ServerSideDecoration
> 
> 
> [server] Add implementation for server side decoration protocol
> 
> 
> [autotest] Add tests for ServerSideDecoration protocol
> 
> 
> Diffs
> -----
> 
>   autotests/client/CMakeLists.txt 014b5e0798618394ecf11c8b8cfa796dcf9c37f3 
>   autotests/client/test_server_side_decoration.cpp PRE-CREATION 
>   autotests/client/test_wayland_registry.cpp 772da821d634efa1c72d13a7c081994bd78ab7fd 
>   src/client/CMakeLists.txt 1d2b5492954e07fc77b2209a123ef8e43e340e8a 
>   src/client/protocols/server-decoration.xml PRE-CREATION 
>   src/client/registry.h c079852a4d96471face2a06795a531abf2e4d8c0 
>   src/client/registry.cpp 71640e184da4699bdc27a993b710b1f761c919d2 
>   src/client/server_decoration.h PRE-CREATION 
>   src/client/server_decoration.cpp PRE-CREATION 
>   src/server/CMakeLists.txt cd5c7bb1a93a470fd58cb9c6d86068da961addeb 
>   src/server/display.h 23b1972ba1081922a0b89a3b04b87f23f67d5086 
>   src/server/display.cpp b236900ff9bab0f153269fd4d328ade7d44505de 
>   src/server/server_decoration_interface.h PRE-CREATION 
>   src/server/server_decoration_interface.cpp PRE-CREATION 
>   src/tools/mapping.txt 9dc8204d65092aa86f6ced8ae5a131a2f89018d0 
> 
> Diff: https://git.reviewboard.kde.org/r/126301/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20151217/3dc87342/attachment.html>