Review Request 125120: [wip] support for the slide protocol

[Thomas Lübking]

> On Sept. 10, 2015, 6:33 vorm., Martin Gräßlin wrote:
> > effects/slidingpopups/slidingpopups.cpp, line 277
> > <https://git.reviewboard.kde.org/r/125120/diff/1/?file=402386#file402386line277>
> >
> >     this connect is dangerous: it doesn't have a context. It will fire after slide is unloaded -> crash. And it might fire also when the EffectWindow for the Surface would get destroyed.
> >     
> >     I suggest to change it to something like:
> >     connect(surf, &slideOnShowHideChanged, this, [this, surf] { slotThingy(effects->findWindow(surf); });
> 
> Marco Martin wrote:
>     I don't have an effectWindow findWindow(surface)
>     the only one existing takes a window id
> 
> Martin Gräßlin wrote:
>     didn't you have a draft for that during the Blur change? Seems in the end we did not add it?
> 
> Marco Martin wrote:
>     hmm, nope, i added effectwindow to surface, not surface to effectwindow.
>     onblurchanged directly takes the effectwindow from the captured context as well
> 
> Martin Gräßlin wrote:
>     allright, so if we compare with blur we only need this as context

That's like begging for stack corruptions.

The connection could be stored as dynamic property on the window or alongside the other "Data" and disconnected when the window is deleted.


- Thomas


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


On Sept. 10, 2015, 11:03 vorm., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125120/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2015, 11:03 vorm.)
> 
> 
> 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
> -----
> 
>   effects/slidingpopups/slidingpopups.h ac3cf10 
>   effects/slidingpopups/slidingpopups.cpp f6d9ec5 
> 
> 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/20150910/934b806f/attachment.html>