> 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
