Review Request 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

> On Oct. 23, 2015, 12:11 p.m., Martin Gräßlin wrote:
> > src/kwindoweffects.h, lines 162-168
> > <https://git.reviewboard.kde.org/r/125736/diff/4/?file=412108#file412108line162>
> >
> >     Maybe we should extend the documentation a little bit? E.g. that it's only a hint to the WM, that it's free to ignore it, that not all WMs might support it.
> >     
> >     Maybe also give an example for a use case? The screen shot case?

Yep, that sounds like a good idea. I'll do that.


> On Oct. 23, 2015, 12:11 p.m., Martin Gräßlin wrote:
> > src/platforms/xcb/kwindoweffects.cpp, lines 73-75
> > <https://git.reviewboard.kde.org/r/125736/diff/4/?file=412114#file412114line73>
> >
> >     Just as a note: KWin doesn't set the atom name on the root window, so it will always return false in this case.
> >     
> >     At the moment I don't have an idea for how to do it. Normal way would be to add it to the NET_SUPPORTED in NETWM.

So should I always return true for this for now? This does mean that the API will be lying, at least for now.


> On Oct. 23, 2015, 12:11 p.m., Martin Gräßlin wrote:
> > src/platforms/xcb/kwindoweffects.cpp, line 333
> > <https://git.reviewboard.kde.org/r/125736/diff/4/?file=412114#file412114line333>
> >
> >     nitpick: unrelated whitespace change.

That's my text-editor (Atom) being a smart-ass. If you really don't want this in the patch, I'll use a second text-editor that doesn't automatically trim empty lines (nano?) to re-add the space.


- Boudhayan


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


On Oct. 21, 2015, 8:53 p.m., Boudhayan Gupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125736/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2015, 8:53 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> Adds a Skip Window Close Animation effect to KWindowEffects, with an implementation for X11.
> 
> Binary compatibility with older plugins is **not** preserved. I welcome comments on how exactly to do this - creating a new base class (KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins off that sounds kind of complicated). Maybe we can just drop internal ABI compatibility and update kwayland-integration to implement the new method too?
> 
> 
> Diffs
> -----
> 
>   autotests/kwindoweffectstest.cpp 0e83bdc 
>   src/CMakeLists.txt 1598b4f 
>   src/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions_p.h PRE-CREATION 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/kwindoweffects.cpp c8da6d2 
>   src/platforms/xcb/kwindoweffects_x11.h c240ddf 
> 
> Diff: https://git.reviewboard.kde.org/r/125736/diff/
> 
> 
> Testing
> -------
> 
> * make test succeeds with the new plugin installed
> * make test succeeds on all tests in the kwindoweffects test except the skipCloseAnimation unit with the old plugin installed. skipCloseAnimation segfaults.
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20151023/a37bdbc4/attachment.html>