Review Request 115859: Force-allow background contrast while sliding desktops

[Martin Gräßlin]

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



kwin/effects/slide/slide.h
<https://git.reviewboard.kde.org/r/115859/#comment35245>

    please add override and you can remove the virtual in that case



kwin/effects/slide/slide.h
<https://git.reviewboard.kde.org/r/115859/#comment35249>

    bad naming: it's not blur, it's backgroundContrast.
    
    For new code we normaly use m_camelCase as variable naming.



kwin/effects/slide/slide.cpp
<https://git.reviewboard.kde.org/r/115859/#comment35246>

    This doesn't work as you want. If the data is not available it will be the same value as if it's set to false. You probably want to check whether the variant is set.



kwin/effects/slide/slide.cpp
<https://git.reviewboard.kde.org/r/115859/#comment35247>

    codingstyle: remove the white spaces around "w"



kwin/effects/slide/slide.cpp
<https://git.reviewboard.kde.org/r/115859/#comment35251>

    I think that's the wrong way to remove it. This would forbid it, but you want to remove the data, so just pass QVariant()



kwin/effects/slide/slide.cpp
<https://git.reviewboard.kde.org/r/115859/#comment35248>

    effects->postPaintWindow(w);



kwin/effects/slide/slide.cpp
<https://git.reviewboard.kde.org/r/115859/#comment35250>

    please only one empty line


- Martin Gräßlin


On Feb. 18, 2014, 2:58 p.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115859/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2014, 2:58 p.m.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> Force-allow background contrast while sliding
> 
> When the sliding effect is active, the background blur of the panel
> would be disabled unless forced with WindowForceBackgroundContrastRole,
> which is actually what we want: during sliding, the backgroundcontrast
> effect would otherwise be disabled, leading to the panel and popups
> flickering between contrast enabled and disabled.
> 
> With this patch, the panel keeps its coloring during desktop changes.
> 
> 
> Diffs
> -----
> 
>   kwin/effects/slide/slide.h c8e0a84 
>   kwin/effects/slide/slide.cpp 8ecb2a6 
> 
> Diff: https://git.reviewboard.kde.org/r/115859/diff/
> 
> 
> Testing
> -------
> 
> Switched desktops, panel keeps color, fullscreen video player doesn't seem to be affected negatively, everything still works as expected, except that the flickering in Plasma Dialogs is gone.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

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