Review Request 126301: Add protocol for server side decoration

[Martin Gräßlin]

> On Dec. 11, 2015, 2: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.

none means that neither client nor server should do CSD.


> On Dec. 11, 2015, 2:54 p.m., Sebastian Kügler wrote:
> > src/client/server_decoration.h, line 187
> > <https://git.reviewboard.kde.org/r/126301/diff/1/?file=421560#file421560line187>
> >
> >     I still don't like this "Mode", but it's probably proximity-bias of the output sort. Decorations have nothing to do with mode settings. Using the same name makes grepping harder.
> >     
> >     decorationMode or decoratorMode would IMO be better.

I disagree. It's namespaced, it's not Mode, it's ServerSideDecoration::Mode. Similar it's OutputDevice::Mode. If we change to DecorationMode it becomes ServerSideDecoration::DecorationMode. We lose the advantage of namespacing.


> On Dec. 11, 2015, 2: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. :)

I have to think about it.


> On Dec. 11, 2015, 2: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. :)

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


- Martin


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


On Dec. 10, 2015, 3:03 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126301/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2015, 3:03 p.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/test_wayland_registry.cpp 772da821d634efa1c72d13a7c081994bd78ab7fd 
>   autotests/client/CMakeLists.txt 014b5e0798618394ecf11c8b8cfa796dcf9c37f3 
>   autotests/client/test_server_side_decoration.cpp PRE-CREATION 
>   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 154e0f0b4c00375ca7d7f0a980392877fe743f50 
>   src/server/display.cpp 81ea7316e3b77f6db95748339d3dfaa303333d33 
>   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/20151216/e52d3cfd/attachment-0001.html>