Review Request 125017: make blur effect work in wayland

[Martin Gräßlin]

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



effects/blur/blur.cpp (line 42)
<https://git.reviewboard.kde.org/r/125017/#comment58600>

    what do you mean with "can this be shared?"



effects/blur/blur.cpp (lines 44 - 45)
<https://git.reviewboard.kde.org/r/125017/#comment58601>

    as you don't use the blurManager you can simplify as:
    display->createBlurManager(this)->create();
    
    I suggest to pass this as parent for blurManager, so that it gets deleted when the BlurEffect gets deleted. Also there's no need to set display as parent: if the display gets destroyed all interfaces get destroyed.



effects/blur/blur.cpp (lines 159 - 161)
<https://git.reviewboard.kde.org/r/125017/#comment58603>

    double contains check: move them together



libkwineffects/kwineffects.h (line 1098)
<https://git.reviewboard.kde.org/r/125017/#comment58607>

    as that adds a new virtual we need to increase so-version of library (that's fine)



libkwineffects/kwineffects.h (line 1657)
<https://git.reviewboard.kde.org/r/125017/#comment58604>

    same as in Toplevel: surface instead of surfaceInterface?



libkwineffects/kwineffects.h (lines 1890 - 1892)
<https://git.reviewboard.kde.org/r/125017/#comment58606>

    maybe add some documentation: e.g. pointing out that this only matters on Wayland and is null on X11.



libkwineffects/kwineffects.h (line 1893)
<https://git.reviewboard.kde.org/r/125017/#comment58605>

    and same



libkwineffects/kwineffects.cpp (line 703)
<https://git.reviewboard.kde.org/r/125017/#comment58608>

    I suggest to move that into waylandServer() - the property is also used on TopLevel, thus it needs to be registered in kwin core.



toplevel.h (lines 205 - 208)
<https://git.reviewboard.kde.org/r/125017/#comment58599>

    nitpick :-)



toplevel.h (line 209)
<https://git.reviewboard.kde.org/r/125017/#comment58602>

    maybe property name "surface" instead of "surfaceInterface"


- 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/125017/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2015, 2 p.m.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Repository: kwin
> 
> 
> Description
> -------
> 
> use the new kwayland protocol to enable blur behind windows
> 
> 
> Diffs
> -----
> 
>   autotests/mock_effectshandler.h beed94a 
>   effects.h 59f4abd 
>   effects.cpp d7c6462 
>   effects/blur/blur.h fe70b6d 
>   effects/blur/blur.cpp e32b17f 
>   libkwineffects/CMakeLists.txt 0a369c6 
>   libkwineffects/kwineffects.h 151cc48 
>   libkwineffects/kwineffects.cpp 41f091f 
>   toplevel.h c71a905 
>   wayland_server.cpp 3ad2d93 
> 
> Diff: https://git.reviewboard.kde.org/r/125017/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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