D9638: [effects] add 'Slide Desktops' effect

[Martin Flöser]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180103/2109050e/attachment-0001.html>