graesslin added a comment.
Restricted Application edited projects, added KWin; removed Plasma.
Overall code looks good for me and the video looks really promising. I fear
it's too late for 5.12, which is a pity.
INLINE COMMENTS
> slidedesktops.cpp:144
> + QRegion clipRegion = buildClipRegion(currentPos, w, h);
> + QList<int> visibleDesktops;
> + for (int i = 1; i <= effects->numberOfDesktops(); i++) {
Please use QVector for new code. See
https://marcmutz.wordpress.com/effective-qt/containers/
> slidedesktops.cpp:160
> + m_paintCtx.fullscreenWindows.clear();
> + foreach (EffectWindow* w, effects->stackingOrder()) {
> + if (! w->isFullScreen()) {
Please don't use Q_FOREACH in new code. Prefer for (auto w :
qAsConst(effects->stackingOrder()))
> slidedesktops.cpp:169
> + const int lastDesktop = visibleDesktops.last();
> + foreach (int desktop, visibleDesktops) {
> + m_paintCtx.desktop = desktop;
same here
> slidedesktops.cpp:213
> + if (w->isDock()) {
> + foreach (EffectWindow* fw, m_paintCtx.fullscreenWindows) {
> + if (fw->isOnDesktop(m_paintCtx.desktop)
and here
> slidedesktops.cpp:339
> +
> + foreach (EffectWindow* w, effects->stackingOrder()) {
> + w->setData(WindowForceBlurRole, QVariant(true));
and here
> slidedesktops.cpp:360
> +{
> + foreach (EffectWindow* w, effects->stackingOrder()) {
> + w->setData(WindowForceBlurRole, QVariant(false));
and here as well
> slidedesktops.h:60
> +
> +private Q_SLOTS:
> + void desktopChanged(int old, int current, EffectWindow* with);
As you use modern connect syntax you don't need the Q_SLOTS keyword anymore.
REPOSITORY
R108 KWin
REVISION DETAIL
https://phabricator.kde.org/D9638
To: zzag, #vdg, #kwin, #plasma
Cc: graesslin, abetts, ngraham, plasma-devel, kwin, iodelay, bwowk, ZrenBot,
progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, sebas, apol, mart
