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