From 56b24f959f0b2ba9a273e8be05e8323906957af8 Mon Sep 17 00:00:00 2001 From: Vlad Zagorodniy Date: Mon, 4 Feb 2019 16:20:33 +0200 Subject: [PATCH] [kcmkwin] Properly load effects Summary: EffectsModel has to communicate with KWin in order to receive the actual value of SupportedRole. So, in theory the model should notify about loaded effects after receiving response from KWin, but that's not the case. Test Plan: Desktop Effects KCM no longer flashes when resetting changes. Reviewers: #kwin, davidedmundson Reviewed By: #kwin, davidedmundson Subscribers: kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D18728 --- kcmkwin/common/effectsmodel.cpp | 134 ++++++++++++++---------- kcmkwin/common/effectsmodel.h | 10 ++ kcmkwin/kwindesktop/animationsmodel.cpp | 8 +- 3 files changed, 94 insertions(+), 58 deletions(-) diff --git a/kcmkwin/common/effectsmodel.cpp b/kcmkwin/common/effectsmodel.cpp index 1642a81c15..49d8e6ecbb 100644 --- a/kcmkwin/common/effectsmodel.cpp +++ b/kcmkwin/common/effectsmodel.cpp @@ -269,7 +269,7 @@ void EffectsModel::loadBuiltInEffects(const KConfigGroup &kwinConfig, const KPlu ); if (shouldStore(effect)) { - m_effects << effect; + m_pendingEffects << effect; } } } @@ -314,7 +314,7 @@ void EffectsModel::loadJavascriptEffects(const KConfigGroup &kwinConfig) } if (shouldStore(effect)) { - m_effects << effect; + m_pendingEffects << effect; } } } @@ -382,7 +382,7 @@ void EffectsModel::loadPluginEffects(const KConfigGroup &kwinConfig, const KPlug ); if (shouldStore(effect)) { - m_effects << effect; + m_pendingEffects << effect; } } } @@ -391,42 +391,50 @@ void EffectsModel::load(LoadOptions options) { KConfigGroup kwinConfig(KSharedConfig::openConfig("kwinrc"), "Plugins"); - const QVector oldEffects = m_effects; - - beginResetModel(); - m_effects.clear(); + m_pendingEffects.clear(); const KPluginInfo::List configs = KPluginTrader::self()->query(QStringLiteral("kwin/effects/configs/")); loadBuiltInEffects(kwinConfig, configs); loadJavascriptEffects(kwinConfig); loadPluginEffects(kwinConfig, configs); - if (options == LoadOptions::KeepDirty) { - for (const EffectData &oldEffect : oldEffects) { - if (!oldEffect.changed) { - continue; - } - auto effectIt = std::find_if(m_effects.begin(), m_effects.end(), - [serviceName = oldEffect.serviceName](const EffectData &data) { - return data.serviceName == serviceName; + qSort(m_pendingEffects.begin(), m_pendingEffects.end(), + [](const EffectData &a, const EffectData &b) { + if (a.category == b.category) { + if (a.exclusiveGroup == b.exclusiveGroup) { + return a.name < b.name; } - ); - if (effectIt == m_effects.end()) { - continue; + return a.exclusiveGroup < b.exclusiveGroup; } - effectIt->status = oldEffect.status; - effectIt->changed = effectIt->status != effectIt->originalStatus; + return a.category < b.category; } - } + ); - qSort(m_effects.begin(), m_effects.end(), [](const EffectData &a, const EffectData &b) { - if (a.category == b.category) { - if (a.exclusiveGroup == b.exclusiveGroup) { - return a.name < b.name; + auto commit = [this, options] { + if (options == LoadOptions::KeepDirty) { + for (const EffectData &oldEffect : m_effects) { + if (!oldEffect.changed) { + continue; + } + auto effectIt = std::find_if(m_pendingEffects.begin(), m_pendingEffects.end(), + [oldEffect](const EffectData &data) { + return data.serviceName == oldEffect.serviceName; + } + ); + if (effectIt == m_pendingEffects.end()) { + continue; + } + effectIt->status = oldEffect.status; + effectIt->changed = effectIt->status != effectIt->originalStatus; } - return a.exclusiveGroup < b.exclusiveGroup; } - return a.category < b.category; - }); + + beginResetModel(); + m_effects = m_pendingEffects; + m_pendingEffects.clear(); + endResetModel(); + + emit loaded(); + }; OrgKdeKwinEffectsInterface interface(QStringLiteral("org.kde.KWin"), QStringLiteral("/Effects"), @@ -434,43 +442,57 @@ void EffectsModel::load(LoadOptions options) if (interface.isValid()) { QStringList effectNames; - effectNames.reserve(m_effects.count()); - for (const EffectData &data : m_effects) { + effectNames.reserve(m_pendingEffects.count()); + for (const EffectData &data : m_pendingEffects) { effectNames.append(data.serviceName); } + const int serial = ++m_lastSerial; + QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(interface.areEffectsSupported(effectNames), this); - watcher->setProperty("effectNames", effectNames); - connect(watcher, &QDBusPendingCallWatcher::finished, [this](QDBusPendingCallWatcher *self) { - const QStringList effectNames = self->property("effectNames").toStringList(); - const QDBusPendingReply > reply = *self; - QList supportValues; - if (reply.isValid()) { - supportValues.append(reply.value()); - } - if (effectNames.size() == supportValues.size()) { + connect(watcher, &QDBusPendingCallWatcher::finished, this, + [=](QDBusPendingCallWatcher *self) { + self->deleteLater(); + + if (m_lastSerial != serial) { + return; + } + + const QDBusPendingReply > reply = *self; + if (reply.isError()) { + commit(); + return; + } + + const QList supportedValues = reply.value(); + if (supportedValues.count() != effectNames.count()) { + return; + } + for (int i = 0; i < effectNames.size(); ++i) { - const bool supportedValue = supportValues.at(i); - const QString &effectName = effectNames.at(i); - auto it = std::find_if(m_effects.begin(), m_effects.end(), [effectName](const EffectData &data) { - return data.serviceName == effectName; - }); - if (it != m_effects.end()) { - if ((*it).supported != supportedValue) { - (*it).supported = supportedValue; - QModelIndex i = findByPluginId(effectName); - if (i.isValid()) { - emit dataChanged(i, i, QVector() << SupportedRole); - } + const bool supported = supportedValues.at(i); + const QString effectName = effectNames.at(i); + + auto it = std::find_if(m_pendingEffects.begin(), m_pendingEffects.end(), + [effectName](const EffectData &data) { + return data.serviceName == effectName; } + ); + if (it == m_pendingEffects.end()) { + continue; + } + + if ((*it).supported != supported) { + (*it).supported = supported; } } - } - self->deleteLater(); - }); - } - endResetModel(); + commit(); + } + ); + } else { + commit(); + } } void EffectsModel::updateEffectStatus(const QModelIndex &rowIndex, Status effectState) diff --git a/kcmkwin/common/effectsmodel.h b/kcmkwin/common/effectsmodel.h index c7b43dc8b5..7ecc81c86b 100644 --- a/kcmkwin/common/effectsmodel.h +++ b/kcmkwin/common/effectsmodel.h @@ -207,6 +207,14 @@ public: **/ void requestConfigure(const QModelIndex &index, QWindow *transientParent); +Q_SIGNALS: + /** + * This signal is emitted when the model is loaded or reloaded. + * + * @see load + **/ + void loaded(); + protected: enum class Kind { BuiltIn, @@ -253,6 +261,8 @@ private: void loadPluginEffects(const KConfigGroup &kwinConfig, const KPluginInfo::List &configs); QVector m_effects; + QVector m_pendingEffects; + int m_lastSerial = -1; Q_DISABLE_COPY(EffectsModel) }; diff --git a/kcmkwin/kwindesktop/animationsmodel.cpp b/kcmkwin/kwindesktop/animationsmodel.cpp index 17e31dfdfd..2e1610a393 100644 --- a/kcmkwin/kwindesktop/animationsmodel.cpp +++ b/kcmkwin/kwindesktop/animationsmodel.cpp @@ -26,6 +26,12 @@ namespace KWin AnimationsModel::AnimationsModel(QObject *parent) : EffectsModel(parent) { + connect(this, &EffectsModel::loaded, this, + [this] { + setEnabled(modelCurrentEnabled()); + setCurrentIndex(modelCurrentIndex()); + } + ); connect(this, &AnimationsModel::currentIndexChanged, this, [this] { const QModelIndex index_ = index(m_currentIndex, 0); @@ -108,8 +114,6 @@ int AnimationsModel::modelCurrentIndex() const void AnimationsModel::load() { EffectsModel::load(); - setEnabled(modelCurrentEnabled()); - setCurrentIndex(modelCurrentIndex()); } void AnimationsModel::save()