diff --git a/autotests/integration/effects/scripted_effects_test.cpp b/autotests/integration/effects/scripted_effects_test.cpp --- a/autotests/integration/effects/scripted_effects_test.cpp +++ b/autotests/integration/effects/scripted_effects_test.cpp @@ -23,7 +23,7 @@ #include "composite.h" #include "cursor.h" -#include "cursor.h" +#include "deleted.h" #include "effect_builtins.h" #include "effectloader.h" #include "effects.h" @@ -68,6 +68,9 @@ void testScreenEdgeTouch(); void testFullScreenEffect_data(); void testFullScreenEffect(); + void testKeepAlive_data(); + void testKeepAlive(); + private: ScriptedEffect *loadEffect(const QString &name); }; @@ -132,6 +135,7 @@ { qRegisterMetaType(); qRegisterMetaType(); + qRegisterMetaType(); qRegisterMetaType(); QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); QVERIFY(workspaceCreatedSpy.isValid()); @@ -421,6 +425,62 @@ QCOMPARE(effects->activeFullScreenEffect(), nullptr); } +void ScriptedEffectsTest::testKeepAlive_data() +{ + QTest::addColumn("file"); + QTest::addColumn("keepAlive"); + + QTest::newRow("keep") << "keepAliveTest" << true; + QTest::newRow("don't keep") << "keepAliveTestDontKeep" << false; +} + +void ScriptedEffectsTest::testKeepAlive() +{ + // this test checks whether closed windows are kept alive + // when keepAlive property is set to true(false) + + QFETCH(QString, file); + QFETCH(bool, keepAlive); + + auto *effect = new ScriptedEffectWithDebugSpy; + QSignalSpy effectOutputSpy(effect, &ScriptedEffectWithDebugSpy::testOutput); + QVERIFY(effectOutputSpy.isValid()); + QVERIFY(effect->load(file)); + + // create a window + using namespace KWayland::Client; + auto *surface = Test::createSurface(Test::waylandCompositor()); + QVERIFY(surface); + auto *shellSurface = Test::createXdgShellV6Surface(surface, surface); + QVERIFY(shellSurface); + auto *c = Test::renderAndWaitForShown(surface, QSize(100, 50), Qt::blue); + QVERIFY(c); + QCOMPARE(workspace()->activeClient(), c); + + // no active animations at the beginning + QCOMPARE(effect->state().count(), 0); + + // trigger windowClosed signal + surface->deleteLater(); + QVERIFY(effectOutputSpy.count() == 1 || effectOutputSpy.wait()); + + if (keepAlive) { + QCOMPARE(effect->state().count(), 1); + + QTest::qWait(500); + QCOMPARE(effect->state().count(), 1); + + QTest::qWait(500 + 100); // 100ms is extra safety margin + QCOMPARE(effect->state().count(), 0); + } else { + // the test effect doesn't keep the window alive, so it should be + // removed immediately + QSignalSpy deletedRemovedSpy(workspace(), &Workspace::deletedRemoved); + QVERIFY(deletedRemovedSpy.isValid()); + QVERIFY(deletedRemovedSpy.count() == 1 || deletedRemovedSpy.wait(100)); // 100ms is less than duration of the animation + QCOMPARE(effect->state().count(), 0); + } +} WAYLANDTEST_MAIN(ScriptedEffectsTest) #include "scripted_effects_test.moc" diff --git a/autotests/integration/effects/scripts/keepAliveTest.js b/autotests/integration/effects/scripts/keepAliveTest.js new file mode 100644 --- /dev/null +++ b/autotests/integration/effects/scripts/keepAliveTest.js @@ -0,0 +1,13 @@ +"use strict"; + +effects.windowClosed.connect(function (window) { + animate({ + window: window, + type: Effect.Scale, + duration: 1000, + from: 1.0, + to: 0.0 + // by default, keepAlive is set to true + }); + sendTestResponse("triggered"); +}); diff --git a/autotests/integration/effects/scripts/keepAliveTestDontKeep.js b/autotests/integration/effects/scripts/keepAliveTestDontKeep.js new file mode 100644 --- /dev/null +++ b/autotests/integration/effects/scripts/keepAliveTestDontKeep.js @@ -0,0 +1,13 @@ +"use strict"; + +effects.windowClosed.connect(function (window) { + animate({ + window: window, + type: Effect.Scale, + duration: 1000, + from: 1.0, + to: 0.0, + keepAlive: false + }); + sendTestResponse("triggered"); +}); diff --git a/effects/dialogparent/package/contents/code/main.js b/effects/dialogparent/package/contents/code/main.js --- a/effects/dialogparent/package/contents/code/main.js +++ b/effects/dialogparent/package/contents/code/main.js @@ -77,6 +77,7 @@ animate({ window: w, duration: dialogParentEffect.duration, + keepAlive: false, animations: [{ type: Effect.Saturation, from: 0.4, diff --git a/libkwineffects/anidata.cpp b/libkwineffects/anidata.cpp --- a/libkwineffects/anidata.cpp +++ b/libkwineffects/anidata.cpp @@ -3,6 +3,7 @@ This file is part of the KDE project. Copyright (C) 2011 Thomas Lübking +Copyright (C) 2018 Vlad Zagorodniy 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 @@ -41,6 +42,17 @@ effects->setActiveFullScreenEffect(nullptr); } +KeepAliveLock::KeepAliveLock(EffectWindow *w) + : m_window(w) +{ + m_window->refWindow(); +} + +KeepAliveLock::~KeepAliveLock() +{ + m_window->unrefWindow(); +} + AniData::AniData() : attribute(AnimationEffect::Opacity) , customCurve(0) // Linear @@ -51,12 +63,13 @@ , windowType((NET::WindowTypeMask)0) , waitAtSource(false) , keepAtTarget(false) + , keepAlive(true) { } AniData::AniData(AnimationEffect::Attribute a, int meta_, int ms, const FPx2 &to_, QEasingCurve curve_, int delay, const FPx2 &from_, bool waitAtSource_, bool keepAtTarget_, - FullScreenEffectLockPtr fullScreenEffectLock_) + FullScreenEffectLockPtr fullScreenEffectLock_, bool keepAlive) : attribute(a) , curve(curve_) , from(from_) @@ -69,6 +82,7 @@ , fullScreenEffectLock(fullScreenEffectLock_) , waitAtSource(waitAtSource_) , keepAtTarget(keepAtTarget_) + , keepAlive(keepAlive) { } diff --git a/libkwineffects/anidata_p.h b/libkwineffects/anidata_p.h --- a/libkwineffects/anidata_p.h +++ b/libkwineffects/anidata_p.h @@ -3,6 +3,7 @@ This file is part of the KDE project. Copyright (C) 2011 Thomas Lübking +Copyright (C) 2018 Vlad Zagorodniy 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 @@ -40,11 +41,28 @@ }; typedef QSharedPointer FullScreenEffectLockPtr; +/** + * Keeps windows alive during animation after they got closed + **/ +class KeepAliveLock +{ +public: + KeepAliveLock(EffectWindow *w); + ~KeepAliveLock(); + +private: + EffectWindow *m_window; + Q_DISABLE_COPY(KeepAliveLock) +}; +typedef QSharedPointer KeepAliveLockPtr; + class KWINEFFECTS_EXPORT AniData { public: AniData(); AniData(AnimationEffect::Attribute a, int meta, int ms, const FPx2 &to, - QEasingCurve curve, int delay, const FPx2 &from, bool waitAtSource, bool keepAtTarget = false, FullScreenEffectLockPtr=FullScreenEffectLockPtr()); + QEasingCurve curve, int delay, const FPx2 &from, bool waitAtSource, + bool keepAtTarget = false, FullScreenEffectLockPtr=FullScreenEffectLockPtr(), + bool keepAlive = true); inline void addTime(int t) { time += t; } inline bool isOneDimensional() const { return from[0] == from[1] && to[0] == to[1]; @@ -62,6 +80,8 @@ NET::WindowTypeMask windowType; QSharedPointer fullScreenEffectLock; bool waitAtSource, keepAtTarget; + bool keepAlive; + KeepAliveLockPtr keepAliveLock; }; } // namespace diff --git a/libkwineffects/kwinanimationeffect.h b/libkwineffects/kwinanimationeffect.h --- a/libkwineffects/kwinanimationeffect.h +++ b/libkwineffects/kwinanimationeffect.h @@ -162,18 +162,19 @@ * @param delay - When the animation will start compared to "now" (the window will remain at the "from" position until then) * @param from - The starting value, the default is invalid, ie. the attribute for the window is not transformed in the beginning * @param fullScreen - Sets this effect as the active full screen effect for the duration of the animation + * @param keepAlive - Whether closed windows should be kept alive during animation * @return an ID that you can use to cancel a running animation */ - quint64 animate( EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve = QEasingCurve(), int delay = 0, FPx2 from = FPx2(), bool fullScreen = false) - { return p_animate(w, a, meta, ms, to, curve, delay, from, false, fullScreen); } + quint64 animate( EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve = QEasingCurve(), int delay = 0, FPx2 from = FPx2(), bool fullScreen = false, bool keepAlive = true) + { return p_animate(w, a, meta, ms, to, curve, delay, from, false, fullScreen, keepAlive); } /** * Equal to ::animate() with one important difference: * The target value for the attribute is kept until you ::cancel() this animation * @return an ID that you need to use to cancel this manipulation */ - quint64 set( EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve = QEasingCurve(), int delay = 0, FPx2 from = FPx2(), bool fullScreen = false) - { return p_animate(w, a, meta, ms, to, curve, delay, from, true, fullScreen); } + quint64 set( EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve = QEasingCurve(), int delay = 0, FPx2 from = FPx2(), bool fullScreen = false, bool keepAlive = true) + { return p_animate(w, a, meta, ms, to, curve, delay, from, true, fullScreen, keepAlive); } /** * this allows to alter the target (but not type or curve) of a running animation @@ -212,7 +213,7 @@ AniMap state() const; private: - quint64 p_animate(EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve, int delay, FPx2 from, bool keepAtTarget, bool fullScreenEffect); + quint64 p_animate(EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve, int delay, FPx2 from, bool keepAtTarget, bool fullScreenEffect, bool keepAlive); QRect clipRect(const QRect &windowRect, const AniData&) const; void clipWindow(const EffectWindow *, const AniData &, WindowQuadList &) const; float interpolated( const AniData&, int i = 0 ) const; diff --git a/libkwineffects/kwinanimationeffect.cpp b/libkwineffects/kwinanimationeffect.cpp --- a/libkwineffects/kwinanimationeffect.cpp +++ b/libkwineffects/kwinanimationeffect.cpp @@ -44,7 +44,6 @@ m_justEndedAnimation = 0; } AnimationEffect::AniMap m_animations; - EffectWindowList m_zombies; static quint64 m_animCounter; quint64 m_justEndedAnimation; // protect against cancel QWeakPointer m_fullScreenEffectLock; @@ -217,7 +216,7 @@ } } -quint64 AnimationEffect::p_animate( EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve, int delay, FPx2 from, bool keepAtTarget, bool fullScreenEffect) +quint64 AnimationEffect::p_animate( EffectWindow *w, Attribute a, uint meta, int ms, FPx2 to, QEasingCurve curve, int delay, FPx2 from, bool keepAtTarget, bool fullScreenEffect, bool keepAlive) { const bool waitAtSource = from.isValid(); validate(a, meta, &from, &to, w); @@ -248,7 +247,7 @@ fullscreen = d->m_fullScreenEffectLock.toStrongRef(); } } - it->first.append(AniData(a, meta, ms, to, curve, delay, from, waitAtSource, keepAtTarget, fullscreen)); + it->first.append(AniData(a, meta, ms, to, curve, delay, from, waitAtSource, keepAtTarget, fullscreen, keepAlive)); quint64 ret_id = ++d->m_animCounter; it->first.last().id = ret_id; it->second = QRect(); @@ -298,11 +297,6 @@ if (anim->id == animationId) { entry->first.erase(anim); // remove the animation if (entry->first.isEmpty()) { // no other animations on the window, release it. - const int i = d->m_zombies.indexOf(entry.key()); - if ( i > -1 ) { - d->m_zombies.removeAt( i ); - entry.key()->unrefWindow(); - } d->m_animations.erase(entry); } if (d->m_animations.isEmpty()) @@ -378,11 +372,6 @@ } } if (entry->first.isEmpty()) { - const int i = d->m_zombies.indexOf(entry.key()); - if ( i > -1 ) { - d->m_zombies.removeAt( i ); - entry.key()->unrefWindow(); - } data.paint |= entry->second; // d->m_damageDirty = true; // TODO likely no longer required entry = d->m_animations.erase(entry); @@ -397,11 +386,6 @@ // janitorial... if (d->m_animations.isEmpty()) { disconnectGeometryChanges(); - if (!d->m_zombies.isEmpty()) { // this is actually not supposed to happen - foreach (EffectWindow *w, d->m_zombies) - w->unrefWindow(); - d->m_zombies.clear(); - } } effects->prePaintScreen(data, time); @@ -924,16 +908,33 @@ void AnimationEffect::_windowClosed( EffectWindow* w ) { Q_D(AnimationEffect); - if (d->m_animations.contains(w) && !d->m_zombies.contains(w)) { - w->refWindow(); - d->m_zombies << w; + + auto it = d->m_animations.find(w); + if (it == d->m_animations.end()) { + return; + } + + KeepAliveLockPtr keepAliveLock; + + QList &animations = (*it).first; + for (auto animationIt = animations.begin(); + animationIt != animations.end(); + ++animationIt) { + if (!(*animationIt).keepAlive) { + continue; + } + + if (keepAliveLock.isNull()) { + keepAliveLock = KeepAliveLockPtr::create(w); + } + + (*animationIt).keepAliveLock = keepAliveLock; } } void AnimationEffect::_windowDeleted( EffectWindow* w ) { Q_D(AnimationEffect); - d->m_zombies.removeAll( w ); // TODO this line is a workaround for a bug in KWin 4.8.0 & 4.8.1 d->m_animations.remove( w ); } diff --git a/scripting/scriptedeffect.h b/scripting/scriptedeffect.h --- a/scripting/scriptedeffect.h +++ b/scripting/scriptedeffect.h @@ -102,8 +102,8 @@ public Q_SLOTS: //curve should be of type QEasingCurve::type or ScriptedEffect::EasingCurve - quint64 animate(KWin::EffectWindow *w, Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from = KWin::FPx2(), uint metaData = 0, int curve = QEasingCurve::Linear, int delay = 0, bool fullScreen = false); - quint64 set(KWin::EffectWindow *w, Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from = KWin::FPx2(), uint metaData = 0, int curve = QEasingCurve::Linear, int delay = 0, bool fullScreen = false); + quint64 animate(KWin::EffectWindow *w, Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from = KWin::FPx2(), uint metaData = 0, int curve = QEasingCurve::Linear, int delay = 0, bool fullScreen = false, bool keepAlive = true); + quint64 set(KWin::EffectWindow *w, Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from = KWin::FPx2(), uint metaData = 0, int curve = QEasingCurve::Linear, int delay = 0, bool fullScreen = false, bool keepAlive = true); bool retarget(quint64 animationId, KWin::FPx2 newTarget, int newRemainingTime = -1); bool cancel(quint64 animationId) { return AnimationEffect::cancel(animationId); } virtual bool borderActivated(ElectricBorder border); diff --git a/scripting/scriptedeffect.cpp b/scripting/scriptedeffect.cpp --- a/scripting/scriptedeffect.cpp +++ b/scripting/scriptedeffect.cpp @@ -103,7 +103,14 @@ } struct AnimationSettings { - enum { Type = 1<<0, Curve = 1<<1, Delay = 1<<2, Duration = 1<<3, FullScreen = 1<<4 }; + enum { + Type = 1<<0, + Curve = 1<<1, + Delay = 1<<2, + Duration = 1<<3, + FullScreen = 1<<4, + KeepAlive = 1<<5 + }; AnimationEffect::Attribute type; QEasingCurve::Type curve; FPx2 from; @@ -113,6 +120,7 @@ uint set; uint metaData; bool fullScreenEffect; + bool keepAlive; }; AnimationSettings animationSettingsFromObject(QScriptValue &object) @@ -164,6 +172,14 @@ settings.fullScreenEffect = false; } + QScriptValue keepAlive = object.property(QStringLiteral("keepAlive")); + if (keepAlive.isValid() && keepAlive.isBool()) { + settings.keepAlive = keepAlive.toBool(); + settings.set |= AnimationSettings::KeepAlive; + } else { + settings.keepAlive = true; + } + return settings; } @@ -230,6 +246,9 @@ if (!(s.set & AnimationSettings::FullScreen)) { s.fullScreenEffect = settings.at(0).fullScreenEffect; } + if (!(s.set & AnimationSettings::KeepAlive)) { + s.keepAlive = settings.at(0).keepAlive; + } s.metaData = 0; typedef QMap MetaTypeMap; @@ -298,7 +317,8 @@ setting.metaData, setting.curve, setting.delay, - setting.fullScreenEffect)); + setting.fullScreenEffect, + setting.keepAlive)); ++i; } return array; @@ -328,7 +348,9 @@ setting.from, setting.metaData, setting.curve, - setting.delay)); + setting.delay, + setting.fullScreenEffect, + setting.keepAlive)); } return engine->newVariant(animIds); @@ -610,24 +632,24 @@ } } -quint64 ScriptedEffect::animate(KWin::EffectWindow* w, KWin::AnimationEffect::Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from, uint metaData, int curve, int delay, bool fullScreen) +quint64 ScriptedEffect::animate(KWin::EffectWindow* w, KWin::AnimationEffect::Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from, uint metaData, int curve, int delay, bool fullScreen, bool keepAlive) { QEasingCurve qec; if (curve < QEasingCurve::Custom) qec.setType(static_cast(curve)); else if (curve == GaussianCurve) qec.setCustomType(qecGaussian); - return AnimationEffect::animate(w, a, metaData, ms, to, qec, delay, from, fullScreen); + return AnimationEffect::animate(w, a, metaData, ms, to, qec, delay, from, fullScreen, keepAlive); } -quint64 ScriptedEffect::set(KWin::EffectWindow* w, KWin::AnimationEffect::Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from, uint metaData, int curve, int delay, bool fullScreen) +quint64 ScriptedEffect::set(KWin::EffectWindow* w, KWin::AnimationEffect::Attribute a, int ms, KWin::FPx2 to, KWin::FPx2 from, uint metaData, int curve, int delay, bool fullScreen, bool keepAlive) { QEasingCurve qec; if (curve < QEasingCurve::Custom) qec.setType(static_cast(curve)); else if (curve == GaussianCurve) qec.setCustomType(qecGaussian); - return AnimationEffect::set(w, a, metaData, ms, to, qec, delay, from, fullScreen); + return AnimationEffect::set(w, a, metaData, ms, to, qec, delay, from, fullScreen, keepAlive); } bool ScriptedEffect::retarget(quint64 animationId, KWin::FPx2 newTarget, int newRemainingTime)