Review Request 125120: support for the slide protocol

Martin Gräßlin mgraesslin at kde.org
Mon Sep 14 12:43:03 UTC 2015


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


The change for findWindow(Surface) looks good.


effects/slidingpopups/slidingpopups.cpp (line 274)
<https://git.reviewboard.kde.org/r/125120/#comment58974>

    two small suggestions:
    * reduce scope of variable by putting it into the if
    * use auto



effects/slidingpopups/slidingpopups.cpp (line 408)
<https://git.reviewboard.kde.org/r/125120/#comment58973>

    w might be null, given how effects->findWindow works. Please add a check for it.



effects/slidingpopups/slidingpopups.cpp (line 416)
<https://git.reviewboard.kde.org/r/125120/#comment58975>

    nitpick :-)



effects/slidingpopups/slidingpopups.cpp (line 436)
<https://git.reviewboard.kde.org/r/125120/#comment58976>

    are you going to do the un-duplications?


- Martin Gräßlin


On Sept. 14, 2015, 2:11 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125120/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2015, 2:11 p.m.)
> 
> 
> Review request for kwin, Plasma and Eike Hein.
> 
> 
> Repository: kwin
> 
> 
> Description
> -------
> 
> take and apply thhe informations from the wayland slide protocol in the sliding popups effect
> 
> 
> Diffs
> -----
> 
>   autotests/mock_effectshandler.h fba13cd 
>   effects.h 97c2f87 
>   effects.cpp f104410 
>   effects/slidingpopups/slidingpopups.h ac3cf10 
>   effects/slidingpopups/slidingpopups.cpp f6d9ec5 
>   libkwineffects/kwineffects.h 20a0768 
> 
> Diff: https://git.reviewboard.kde.org/r/125120/diff/
> 
> 
> Testing
> -------
> 
> slide to appear works, slide out to disappear doesn't.
> 
> the second time a popup is opened, kwin crashes and an assert is hit
> 
> kwin_wayland: /home/diau/git/kf5/kde/workspace/kwin/libkwineffects/kwineffects.cpp:908: KWin::WindowQuad KWin::WindowQuad::makeSubQuad(double, double, double, double) const: Assertion `x1 < x2 && y1 < y2 && x1 >= left() && x2 <= right() && y1 >= top() && y2 <= bottom()' failed.
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150914/79f75aeb/attachment.html>


More information about the Plasma-devel mailing list