Review Request 125736: Add Skip Window Close Animation to KWindowEffects

[Martin Gräßlin]

> On Oct. 21, 2015, 4:49 p.m., Boudhayan Gupta wrote:
> > src/platforms/xcb/kwindoweffects_x11.h, line 26
> > <https://git.reviewboard.kde.org/r/125736/diff/3/?file=412105#file412105line26>
> >
> >     This was my idea, which sitter also liked and is used in Phonon. The idea is that every individual added feature has its own class which is MI-ed with the core class, that way plugins can choose to implement only the extra features they want.
> >     
> >     For example if I add feature A in class 1, then feature B in class 2, but a plugin only wants to implement feature B without implementing feature A, they can't if you do what you're asking.
> 
> Martin Gräßlin wrote:
>     I see what you mean. But that could also be achieved with default implementation instead of pure virtuals. And I do see a problem with that approach: what if you want to add another method one day to e.g. enable close animation again? Then you need to extend KWindowEffectsSkipCloseAnimation in exactly the same way as I proposed originally.
> 
> Boudhayan Gupta wrote:
>     Then I add a KWindowEffectsEnableCloseAnimation extension.
>     
>     This is why I put the extensions in another header. kwindoweffects_p.h is going to get very ugly very fast if I keep adding micro-features - let's confine the ugliness to this one place.
>     
>     If you go the chain of inheritence way, it'll end up like this one day:
>     
>     https://quickgit.kde.org/?p=phonon.git&a=blob&h=6b5d878ec9e6e605557b5de12c179d6ac3d5daf8&f=phonon%2Faudiooutputinterface.h&o=plain

right, I see your point. Even if I consider it as unlikely that we ever get to a two digit number (based on history of the development of KWindowEffects) ;-)

Then please use a "_p.h" naming. And it also needs to be installed through CMake. Otherwise we cannot use it from kwayland-integration.


- Martin


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


On Oct. 21, 2015, 1:55 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, 1:55 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/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions.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/20151021/2b812a8b/attachment-0001.html>