Review Request 127352: [kstyle] Implement window shadows on Wayland

[Hugo Pereira Da Costa]

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


Fix it, then Ship it!




couple of nitpicking (which I cand easily fix myself in case you are busy elsewhere.
asside from that, I could not test because have no working wayland setup here (too old distro/drivers), so I'll just trust you. 

other than that, good to go imho.
(and don't hesitate to also push to oxygen, if you have time, and without a review. Note that I can do it myself too -without testing-, otherwise)


kstyle/breezeshadowhelper.h (line 33)
<https://git.reviewboard.kde.org/r/127352/#comment63727>

    I thought the policy in case of "external" dependencies, was to include the header rather than using forward declarations. (in case for instance a class is turned into a struct, a namespace into a class, or whatever).
    At least have been doing so with Qt since forever.
    
    If not, please add some indentation here after the braces)



kstyle/breezeshadowhelper.h (line 120)
<https://git.reviewboard.kde.org/r/127352/#comment63728>

    some comments before the new methods ?



kstyle/breezeshadowhelper.h (line 134)
<https://git.reviewboard.kde.org/r/127352/#comment63729>

    idem



kstyle/breezeshadowhelper.h (line 172)
<https://git.reviewboard.kde.org/r/127352/#comment63730>

    other class members start with "_" rather than "m_"
    please keep it internally consistent (even if not with the rest of e.g. kwin)



kstyle/breezeshadowhelper.cpp (line 82)
<https://git.reviewboard.kde.org/r/127352/#comment63731>

    please keep "//_________________________" (the number of "_" being unimportant) before the beginning of each function, for consistency with the rest of the class.



kstyle/breezeshadowhelper.cpp (line 381)
<https://git.reviewboard.kde.org/r/127352/#comment63732>

    //______________________________



kstyle/breezeshadowhelper.cpp (line 435)
<https://git.reviewboard.kde.org/r/127352/#comment63733>

    //________________________________
    (and elsewhere in case I missed some)


- Hugo Pereira Da Costa


On March 12, 2016, 8:28 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127352/
> -----------------------------------------------------------
> 
> (Updated March 12, 2016, 8:28 a.m.)
> 
> 
> Review request for Plasma and Hugo Pereira Da Costa.
> 
> 
> Repository: breeze
> 
> 
> Description
> -------
> 
> Integrate with Wayland and create a ShadowManager if available.
> With the ShadowManager it's possible to create a Shadow for a Surface.
> 
> The Wayland shadow is very similar to the X11 based one, so a lot of
> code can be shared. The code is slightly refactored to share the common
> code paths to create and destroy the shadow.
> 
> 
> Diffs
> -----
> 
>   kstyle/CMakeLists.txt b2c6dc847fef1c7876a0ac1af9e28f47422b620b 
>   kstyle/breezeshadowhelper.h 0c868d3686eec67e630ce08c36df6b4cf34d7f1c 
>   kstyle/breezeshadowhelper.cpp 1eb1ddc6a154d8865b39936718f6a5d5ef8f7fdd 
>   kstyle/config-breeze.h.cmake ba2b5f64c1ca855c541fd7eb0e919b8f897f481f 
> 
> Diff: https://git.reviewboard.kde.org/r/127352/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160313/5072409b/attachment-0001.html>