Ensure that all Effects honour the grab roles correctly

Summary:
When windows get added some effects grab the window and want to be the
only one animating this window. For this the grab roles exists. An
effect being notified later on evaluates the grab state and does not
start the animation.

This process failed due to being dependent on the order the effects are
loaded. Window Added/Closed are signals emitted by EffectsHandler, thus
first come, first serve. The requested effect order does not play into
it.

Due to that it could happen that an Effect which should not animate,
started to animate as the grab was still there.

This change adds the possibility to be notified whenever the window data
changes. A new signal is added to EffectsHandler which is emitted
whenever the windowData changes. The interested effects connect to it
and cancel their (just started) animation for the window.

Adjusted effects are:
* ScaleIn
* Fade
* WobblyWindows

In case of WobblyWindows an additional logical error was fixed that the
animations were only run when an effect grabbed instead of the other way
around.

BUG: 336866
FIXED-IN: 5.8.4

Reviewers: #kwin, #plasma, broulik

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D3211
This commit is contained in:
Martin Gräßlin 2016-10-31 11:22:24 +01:00
parent ee7da425ce
commit fb69b791a1
8 changed files with 81 additions and 18 deletions

View file

@ -222,10 +222,6 @@ void SlidingPopupsTest::testWithOtherEffect()
// sliding popups should be active
QVERIFY(windowAddedSpy.wait());
QTRY_VERIFY(slidingPoupus->isActive());
QEXPECT_FAIL("scale, slide", "bug 336866", Continue);
QEXPECT_FAIL("fade, slide", "bug 336866", Continue);
QEXPECT_FAIL("wobblywindows, slide", "bug 336866", Continue);
QEXPECT_FAIL("slide, wobblywindows", "bug 336866", Continue);
QVERIFY(!otherEffect->isActive());
// wait till effect ends
@ -246,8 +242,6 @@ void SlidingPopupsTest::testWithOtherEffect()
// again we should have the sliding popups active
QVERIFY(slidingPoupus->isActive());
QEXPECT_FAIL("wobblywindows, slide", "bug 336866", Continue);
QEXPECT_FAIL("slide, wobblywindows", "bug 336866", Continue);
QVERIFY(!otherEffect->isActive());
QVERIFY(windowDeletedSpy.wait());
@ -347,10 +341,6 @@ void SlidingPopupsTest::testWithOtherEffectWayland()
// sliding popups should be active
QCOMPARE(windowAddedSpy.count(), 1);
QTRY_VERIFY(slidingPoupus->isActive());
QEXPECT_FAIL("scale, slide", "bug 336866", Continue);
QEXPECT_FAIL("fade, slide", "bug 336866", Continue);
QEXPECT_FAIL("wobblywindows, slide", "bug 336866", Continue);
QEXPECT_FAIL("slide, wobblywindows", "bug 336866", Continue);
QVERIFY(!otherEffect->isActive());
// wait till effect ends
@ -371,8 +361,6 @@ void SlidingPopupsTest::testWithOtherEffectWayland()
// again we should have the sliding popups active
QVERIFY(slidingPoupus->isActive());
QEXPECT_FAIL("wobblywindows, slide", "bug 336866", Continue);
QEXPECT_FAIL("slide, wobblywindows", "bug 336866", Continue);
QVERIFY(!otherEffect->isActive());
QVERIFY(windowDeletedSpy.wait());

View file

@ -1688,6 +1688,7 @@ void EffectWindowImpl::setData(int role, const QVariant &data)
dataMap[ role ] = data;
else
dataMap.remove(role);
emit effects->windowDataChanged(this, role);
}
QVariant EffectWindowImpl::data(int role) const

View file

@ -43,12 +43,12 @@ effect.configChanged.connect(function() {
});
effects.windowAdded.connect(function(w) {
if (fadeWindows && isFadeWindow(w)) {
effect.animate(w, Effect.Opacity, fadeInTime, 1.0, 0.0);
w.fadeInWindowTypeAnimation = effect.animate(w, Effect.Opacity, fadeInTime, 1.0, 0.0);
}
});
effects.windowClosed.connect(function(w) {
if (fadeWindows && isFadeWindow(w)) {
animate({
w.fadeOutWindowTypeAnimation = animate({
window: w,
duration: fadeOutTime,
animations: [{
@ -59,3 +59,20 @@ effects.windowClosed.connect(function(w) {
});
}
});
effects.windowDataChanged.connect(function (window, role) {
if (role == Effect.WindowAddedGrabRole) {
if (effect.isGrabbed(window, Effect.WindowAddedGrabRole)) {
if (window.fadeInWindowTypeAnimation !== undefined) {
cancel(window.fadeInWindowTypeAnimation);
window.fadeInWindowTypeAnimation = undefined;
}
}
} else if (role == Effect.WindowClosedGrabRole) {
if (effect.isGrabbed(window, Effect.WindowClosedGrabRole)) {
if (window.fadeOutWindowTypeAnimation !== undefined) {
cancel(window.fadeOutWindowTypeAnimation);
window.fadeOutWindowTypeAnimation = undefined;
}
}
}
});

View file

@ -34,7 +34,7 @@ var scaleInEffect = {
},
scaleIn: function (window) {
"use strict";
animate({
window.scaleInWindowTypeAnimation = animate({
window: window,
duration: scaleInEffect.duration,
curve: QEasingCurve.InOutQuad,
@ -56,10 +56,22 @@ var scaleInEffect = {
}
scaleInEffect.scaleIn(window);
},
dataChanged: function (window, role) {
"use strict";
if (role == Effect.WindowAddedGrabRole) {
if (effect.isGrabbed(window, Effect.WindowAddedGrabRole)) {
if (window.scaleInWindowTypeAnimation !== undefined) {
cancel(window.scaleInWindowTypeAnimation);
window.scaleInWindowTypeAnimation = undefined;
}
}
}
},
init: function () {
"use strict";
effect.configChanged.connect(scaleInEffect.loadConfig);
effects.windowAdded.connect(scaleInEffect.added);
effects.windowDataChanged.connect(scaleInEffect.dataChanged);
}
};
scaleInEffect.init();

View file

@ -305,7 +305,6 @@ void SlidingPopupsEffect::startForShow(EffectWindow *w)
mAppearingWindows.insert(w, new QTimeLine(mWindowsData[ w ].fadeInDuration, this));
mAppearingWindows[ w ]->setCurveShape(QTimeLine::EaseInOutCurve);
// Tell other windowAdded() and windowClosed() effects to ignore this window
w->setData(WindowAddedGrabRole, QVariant::fromValue(static_cast<void*>(this)));
w->setData(WindowClosedGrabRole, QVariant::fromValue(static_cast<void*>(this)));
w->setData(WindowForceBlurRole, true);

View file

@ -153,6 +153,8 @@ WobblyWindowsEffect::WobblyWindowsEffect()
connect(effects, SIGNAL(windowStepUserMovedResized(KWin::EffectWindow*,QRect)), this, SLOT(slotWindowStepUserMovedResized(KWin::EffectWindow*,QRect)));
connect(effects, SIGNAL(windowFinishUserMovedResized(KWin::EffectWindow*)), this, SLOT(slotWindowFinishUserMovedResized(KWin::EffectWindow*)));
connect(effects, SIGNAL(windowMaximizedStateChanged(KWin::EffectWindow*,bool,bool)), this, SLOT(slotWindowMaximizeStateChanged(KWin::EffectWindow*,bool,bool)));
connect(effects, &EffectsHandler::windowDataChanged, this, &WobblyWindowsEffect::cancelWindowGrab);
}
WobblyWindowsEffect::~WobblyWindowsEffect()
@ -472,7 +474,7 @@ void WobblyWindowsEffect::stepMovedResized(EffectWindow* w)
void WobblyWindowsEffect::slotWindowAdded(EffectWindow* w)
{
if (m_openEffectEnabled && w->data(WindowAddedGrabRole).value<void*>() != this) {
if (m_openEffectEnabled && w->data(WindowAddedGrabRole).value<void*>() == nullptr) {
if (windows.contains(w)) {
// could this happen ??
WindowWobblyInfos& wwi = windows[w];
@ -499,7 +501,7 @@ void WobblyWindowsEffect::slotWindowClosed(EffectWindow* w)
if (windows.isEmpty())
effects->addRepaintFull();
}
} else if (m_closeEffectEnabled && w->data(WindowAddedGrabRole).value<void*>() != this) {
} else if (m_closeEffectEnabled && w->data(WindowClosedGrabRole).value<void*>() == nullptr) {
WindowWobblyInfos new_wwi;
initWobblyInfo(new_wwi, w->geometry());
wobblyCloseInit(new_wwi, w);
@ -1200,6 +1202,31 @@ void WobblyWindowsEffect::heightRingLinearMean(Pair** data_pointer, WindowWobbly
wwi.buffer = tmp;
}
void WobblyWindowsEffect::cancelWindowGrab(KWin::EffectWindow *w, int grabRole)
{
if (grabRole == WindowAddedGrabRole) {
if (w->data(WindowAddedGrabRole).value<void*>() != this) {
auto it = windows.find(w);
if (it != windows.end()) {
freeWobblyInfo(it.value());
windows.erase(it);
}
}
} else if (grabRole == WindowClosedGrabRole) {
if (w->data(WindowClosedGrabRole).value<void*>() != this) {
auto it = windows.find(w);
if (it != windows.end()) {
if (it.value().status == Closing) {
w->unrefWindow();
}
freeWobblyInfo(it.value());
windows.erase(it);
}
}
}
}
bool WobblyWindowsEffect::isActive() const
{
return !windows.isEmpty();

View file

@ -136,6 +136,7 @@ public Q_SLOTS:
private:
void cancelWindowGrab(KWin::EffectWindow *w, int grabRole);
void startMovedResized(EffectWindow* w);
void stepMovedResized(EffectWindow* w);
bool updateWindowWobblyDatas(EffectWindow* w, qreal time);

View file

@ -1537,6 +1537,21 @@ Q_SIGNALS:
**/
void windowHidden(KWin::EffectWindow *w);
/**
* This signal gets emitted when the data on EffectWindow @p w for @p role changed.
*
* An Effect can connect to this signal to read the new value and react on it.
* E.g. an Effect which does not operate on windows grabbed by another Effect wants
* to cancel the already scheduled animation if another Effect adds a grab.
*
* @param w The EffectWindow for which the data changed
* @param role The data role which changed
* @see EffectWindow::setData
* @see EffectWindow::data
* @since 5.8.4
**/
void windowDataChanged(KWin::EffectWindow *w, int role);
protected:
QVector< EffectPair > loaded_effects;
//QHash< QString, EffectFactory* > effect_factories;
@ -2017,6 +2032,9 @@ public:
/**
* Can be used to by effects to store arbitrary data in the EffectWindow.
*
* Invoking this method will emit the signal EffectsHandler::windowDataChanged.
* @see EffectsHandler::windowDataChanged
*/
Q_SCRIPTABLE virtual void setData(int role, const QVariant &data) = 0;
Q_SCRIPTABLE virtual QVariant data(int role) const = 0;