[effects] Handle windowShown and windowHidden in fade effect

Summary:
For Wayland windows we can have a sequence of window unmapped
(windowHidden signal) followed by a windowClosed way later when the
application quits. This is for example the case with menus.

The result of this was that the fade out animation triggered when the
application quit showing all the already closed menus again.

This change implements a windowShown and windowHidden handler and
triggers the fadeIn/Out animation on it. If the window gets shown
again the existing fadeOut animation gets cancelled, so that it can run
again.

If a window gets closed for which a fade out animation has been run
already, it's not triggered again, thus ensuring that we don't see
zombie windows.

CCBUG: 372622

Reviewers: #kwin, #plasma_on_wayland, hein

Subscribers: plasma-devel, kwin

Tags: #plasma_on_wayland, #kwin

Differential Revision: https://phabricator.kde.org/D3419

autotests/integration/effects/CMakeLists.txt

@@ -2,3 +2,4 @@ if (XCB_ICCCM_FOUND)
     integrationTest(NAME testTranslucency SRCS translucency_test.cpp LIBS XCB::ICCCM)
     integrationTest(NAME testSlidingPopups SRCS slidingpopups_test.cpp LIBS XCB::ICCCM)
 endif()
+integrationTest(NAME testFade SRCS fade_test.cpp)

autotests/integration/effects/fade_test.cpp

@@ -0,0 +1,182 @@
+/********************************************************************
+KWin - the KDE window manager
+This file is part of the KDE project.
+
+Copyright (C) 2016 Martin Gräßlin <mgraesslin@kde.org>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#include "kwin_wayland_test.h"
+#include "composite.h"
+#include "effects.h"
+#include "effectloader.h"
+#include "cursor.h"
+#include "platform.h"
+#include "scene_qpainter.h"
+#include "shell_client.h"
+#include "wayland_server.h"
+#include "workspace.h"
+#include "effect_builtins.h"
+
+#include <KConfigGroup>
+
+#include <KWayland/Client/buffer.h>
+#include <KWayland/Client/shell.h>
+#include <KWayland/Client/surface.h>
+
+using namespace KWin;
+using namespace KWayland::Client;
+static const QString s_socketName = QStringLiteral("wayland_test_effects_translucency-0");
+
+class FadeTest : public QObject
+{
+Q_OBJECT
+private Q_SLOTS:
+    void initTestCase();
+    void init();
+    void cleanup();
+
+    void testWindowCloseAfterWindowHidden_data();
+    void testWindowCloseAfterWindowHidden();
+
+private:
+    Effect *m_fadeEffect = nullptr;
+};
+
+void FadeTest::initTestCase()
+{
+    qRegisterMetaType<KWin::ShellClient*>();
+    qRegisterMetaType<KWin::AbstractClient*>();
+    qRegisterMetaType<KWin::Effect*>();
+    QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated);
+    QVERIFY(workspaceCreatedSpy.isValid());
+    kwinApp()->platform()->setInitialWindowSize(QSize(1280, 1024));
+    QVERIFY(waylandServer()->init(s_socketName.toLocal8Bit()));
+
+    // disable all effects - we don't want to have it interact with the rendering
+    auto config = KSharedConfig::openConfig(QString(), KConfig::SimpleConfig);
+    KConfigGroup plugins(config, QStringLiteral("Plugins"));
+    ScriptedEffectLoader loader;
+    const auto builtinNames = BuiltInEffects::availableEffectNames() << loader.listOfKnownEffects();
+    for (QString name : builtinNames) {
+        plugins.writeEntry(name + QStringLiteral("Enabled"), false);
+    }
+
+    config->sync();
+    kwinApp()->setConfig(config);
+
+    qputenv("KWIN_EFFECTS_FORCE_ANIMATIONS", "1");
+    kwinApp()->start();
+    QVERIFY(workspaceCreatedSpy.wait());
+    QVERIFY(KWin::Compositor::self());
+}
+
+void FadeTest::init()
+{
+    QVERIFY(Test::setupWaylandConnection(s_socketName));
+
+    // load the translucency effect
+    EffectsHandlerImpl *e = static_cast<EffectsHandlerImpl*>(effects);
+    // find the effectsloader
+    auto effectloader = e->findChild<AbstractEffectLoader*>();
+    QVERIFY(effectloader);
+    QSignalSpy effectLoadedSpy(effectloader, &AbstractEffectLoader::effectLoaded);
+    QVERIFY(effectLoadedSpy.isValid());
+
+    QVERIFY(!e->isEffectLoaded(QStringLiteral("kwin4_effect_fade")));
+    QVERIFY(e->loadEffect(QStringLiteral("kwin4_effect_fade")));
+    QVERIFY(e->isEffectLoaded(QStringLiteral("kwin4_effect_fade")));
+
+    QCOMPARE(effectLoadedSpy.count(), 1);
+    m_fadeEffect = effectLoadedSpy.first().first().value<Effect*>();
+    QVERIFY(m_fadeEffect);
+}
+
+void FadeTest::cleanup()
+{
+    Test::destroyWaylandConnection();
+    EffectsHandlerImpl *e = static_cast<EffectsHandlerImpl*>(effects);
+    if (e->isEffectLoaded(QStringLiteral("kwin4_effect_fade"))) {
+        e->unloadEffect(QStringLiteral("kwin4_effect_fade"));
+    }
+    QVERIFY(!e->isEffectLoaded(QStringLiteral("kwin4_effect_fade")));
+    m_fadeEffect = nullptr;
+}
+
+void FadeTest::testWindowCloseAfterWindowHidden_data()
+{
+    QTest::addColumn<Test::ShellSurfaceType>("type");
+
+    QTest::newRow("wlShell") << Test::ShellSurfaceType::WlShell;
+    QTest::newRow("xdgShellV5") << Test::ShellSurfaceType::XdgShellV5;
+}
+
+void FadeTest::testWindowCloseAfterWindowHidden()
+{
+    // this test simulates the showing/hiding/closing of a Wayland window
+    // especially the situation that a window got unmapped and destroyed way later
+    QVERIFY(!m_fadeEffect->isActive());
+
+    QSignalSpy windowAddedSpy(effects, &EffectsHandler::windowAdded);
+    QVERIFY(windowAddedSpy.isValid());
+    QSignalSpy windowHiddenSpy(effects, &EffectsHandler::windowHidden);
+    QVERIFY(windowHiddenSpy.isValid());
+    QSignalSpy windowShownSpy(effects, &EffectsHandler::windowShown);
+    QVERIFY(windowShownSpy.isValid());
+    QSignalSpy windowClosedSpy(effects, &EffectsHandler::windowClosed);
+    QVERIFY(windowClosedSpy.isValid());
+
+    QScopedPointer<Surface> surface(Test::createSurface());
+    QFETCH(Test::ShellSurfaceType, type);
+    QScopedPointer<QObject> shellSurface(Test::createShellSurface(type, surface.data()));
+    auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(c);
+    QTRY_COMPARE(windowAddedSpy.count(), 1);
+    QTRY_COMPARE(m_fadeEffect->isActive(), true);
+
+    QTest::qWait(500);
+    QTRY_COMPARE(m_fadeEffect->isActive(), false);
+
+    // now unmap the surface
+    surface->attachBuffer(Buffer::Ptr());
+    surface->commit(Surface::CommitFlag::None);
+    QVERIFY(windowHiddenSpy.wait());
+    QCOMPARE(m_fadeEffect->isActive(), true);
+    QTest::qWait(500);
+    QTRY_COMPARE(m_fadeEffect->isActive(), false);
+
+    // and map again
+    Test::render(surface.data(), QSize(100, 50), Qt::red);
+    QVERIFY(windowShownSpy.wait());
+    QTRY_COMPARE(m_fadeEffect->isActive(), true);
+    QTest::qWait(500);
+    QTRY_COMPARE(m_fadeEffect->isActive(), false);
+
+    // and unmap once more
+    surface->attachBuffer(Buffer::Ptr());
+    surface->commit(Surface::CommitFlag::None);
+    QVERIFY(windowHiddenSpy.wait());
+    QCOMPARE(m_fadeEffect->isActive(), true);
+    QTest::qWait(500);
+    QTRY_COMPARE(m_fadeEffect->isActive(), false);
+
+    // and now destroy
+    shellSurface.reset();
+    surface.reset();
+    QVERIFY(windowClosedSpy.wait());
+    QCOMPARE(m_fadeEffect->isActive(), false);
+}
+
+WAYLANDTEST_MAIN(FadeTest)
+#include "fade_test.moc"

effects/fade/package/contents/code/main.js

@@ -41,13 +41,21 @@ loadConfig();
 effect.configChanged.connect(function() {
     loadConfig();
 });
-effects.windowAdded.connect(function(w) {
+function fadeInHandler(w) {
     if (fadeWindows && isFadeWindow(w)) {
+        if (w.fadeOutWindowTypeAnimation !== undefined) {
+            cancel(w.fadeOutWindowTypeAnimation);
+            w.fadeOutWindowTypeAnimation = undefined;
+        }
         w.fadeInWindowTypeAnimation = effect.animate(w, Effect.Opacity, fadeInTime, 1.0, 0.0);
     }
-});
-effects.windowClosed.connect(function(w) {
+}
+function fadeOutHandler(w) {
     if (fadeWindows && isFadeWindow(w)) {
+        if (w.fadeOutWindowTypeAnimation !== undefined) {
+            // don't animate again as it was already animated through window hidden
+            return;
+        }
         w.fadeOutWindowTypeAnimation = animate({
             window: w,
             duration: fadeOutTime,
@@ -58,7 +66,11 @@ effects.windowClosed.connect(function(w) {
             }]
         });
     }
-});
+}
+effects.windowAdded.connect(fadeInHandler);
+effects.windowShown.connect(fadeInHandler);
+effects.windowClosed.connect(fadeOutHandler);
+effects.windowHidden.connect(fadeOutHandler);
 effects.windowDataChanged.connect(function (window, role) {
     if (role == Effect.WindowAddedGrabRole) {
         if (effect.isGrabbed(window, Effect.WindowAddedGrabRole)) {