Review Request 125015: Blur protocol in KWayland

[Martin Gräßlin]

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


Looks really good: didn't find anything in the server to complain about ;-)

On the autotest side you could also extend the test_wayland_registry.cpp by a testBindBlurManager method


src/client/blur.h (line 111)
<https://git.reviewboard.kde.org/r/125015/#comment58614>

    what is removeBlur doing? Shouldn't a delete of the Blur object be the same?



src/client/blur.h (line 125)
<https://git.reviewboard.kde.org/r/125015/#comment58612>

    copy paste error: Compositor -> BlurManger



src/client/blur.h (line 132)
<https://git.reviewboard.kde.org/r/125015/#comment58613>

    Compositor -> BlurManager



src/client/blur.h (line 142)
<https://git.reviewboard.kde.org/r/125015/#comment58611>

    copy paste error: Compositor::createSurface



src/client/blur.h (line 175)
<https://git.reviewboard.kde.org/r/125015/#comment58610>

    nitpick



src/client/blur.h (line 176)
<https://git.reviewboard.kde.org/r/125015/#comment58615>

    Please document how it's supposed to be used, e.g. whether null is allowed and what it means.
    
    Also please change to our wrapper Class Region instead of wl_region.



src/client/blur.cpp (line 26)
<https://git.reviewboard.kde.org/r/125015/#comment58609>

    I doubt QMarginsF is used in blur


- Martin Gräßlin


On Sept. 1, 2015, 2 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125015/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 2 p.m.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Repository: kwayland
> 
> 
> Description
> -------
> 
> a protocol to activate the blur behind windows and to optionally set a sub region of the window where to apply the blur to, in case the window is shaped
> 
> 
> Diffs
> -----
> 
>   autotests/client/CMakeLists.txt c2c1df2 
>   autotests/client/test_wayland_blur.cpp PRE-CREATION 
>   src/client/CMakeLists.txt 7d0d38a 
>   src/client/blur.h PRE-CREATION 
>   src/client/blur.cpp PRE-CREATION 
>   src/client/protocols/blur.xml PRE-CREATION 
>   src/client/registry.h 3e9d1f4 
>   src/client/registry.cpp 0c1c213 
>   src/client/surface.h fe744bc 
>   src/client/surface.cpp 98a3cec 
>   src/server/CMakeLists.txt 1cf09d3 
>   src/server/blur_interface.h PRE-CREATION 
>   src/server/blur_interface.cpp PRE-CREATION 
>   src/server/display.h 4c0e0c7 
>   src/server/display.cpp 884d7ea 
>   src/server/surface_interface.h 4935ff6 
>   src/server/surface_interface.cpp be885bd 
>   src/server/surface_interface_p.h 062c7e7 
> 
> Diff: https://git.reviewboard.kde.org/r/125015/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150901/5cdde732/attachment.html>