Changeset View
Changeset View
Standalone View
Standalone View
kcmkwin/common/effectmodel.cpp
Show First 20 Lines • Show All 381 Lines • ▼ Show 20 Line(s) | 330 | for (const KPluginMetaData &pluginEffect : pluginEffects) { | |||
---|---|---|---|---|---|
382 | ); | 382 | ); | ||
383 | 383 | | |||
384 | if (shouldStore(effect)) { | 384 | if (shouldStore(effect)) { | ||
385 | m_effectsList << effect; | 385 | m_effectsList << effect; | ||
386 | } | 386 | } | ||
387 | } | 387 | } | ||
388 | } | 388 | } | ||
389 | 389 | | |||
390 | void EffectModel::load() | 390 | void EffectModel::load(LoadOptions options) | ||
391 | { | 391 | { | ||
392 | KConfigGroup kwinConfig(KSharedConfig::openConfig("kwinrc"), "Plugins"); | 392 | KConfigGroup kwinConfig(KSharedConfig::openConfig("kwinrc"), "Plugins"); | ||
393 | 393 | | |||
394 | const QVector<EffectData> oldEffects = m_effectsList; | ||||
395 | | ||||
394 | beginResetModel(); | 396 | beginResetModel(); | ||
395 | m_effectsChanged.clear(); | 397 | m_effectsChanged.clear(); | ||
396 | m_effectsList.clear(); | 398 | m_effectsList.clear(); | ||
397 | const KPluginInfo::List configs = KPluginTrader::self()->query(QStringLiteral("kwin/effects/configs/")); | 399 | const KPluginInfo::List configs = KPluginTrader::self()->query(QStringLiteral("kwin/effects/configs/")); | ||
398 | loadBuiltInEffects(kwinConfig, configs); | 400 | loadBuiltInEffects(kwinConfig, configs); | ||
399 | loadJavascriptEffects(kwinConfig); | 401 | loadJavascriptEffects(kwinConfig); | ||
400 | loadPluginEffects(kwinConfig, configs); | 402 | loadPluginEffects(kwinConfig, configs); | ||
401 | 403 | | |||
404 | if (options == LoadOptions::KeepDirty) { | ||||
405 | for (const EffectData &oldEffect : oldEffects) { | ||||
406 | if (!oldEffect.changed) { | ||||
407 | continue; | ||||
408 | } | ||||
409 | auto effectIt = std::find_if(m_effectsList.begin(), m_effectsList.end(), | ||||
410 | [serviceName = oldEffect.serviceName](const EffectData &data) { | ||||
411 | return data.serviceName == serviceName; | ||||
412 | } | ||||
413 | ); | ||||
414 | if (effectIt == m_effectsList.end()) { | ||||
415 | continue; | ||||
416 | } | ||||
417 | effectIt->effectStatus = oldEffect.effectStatus; | ||||
418 | effectIt->changed = effectIt->effectStatus != effectIt->originalStatus; | ||||
419 | } | ||||
420 | } | ||||
421 | | ||||
402 | qSort(m_effectsList.begin(), m_effectsList.end(), [](const EffectData &a, const EffectData &b) { | 422 | qSort(m_effectsList.begin(), m_effectsList.end(), [](const EffectData &a, const EffectData &b) { | ||
403 | if (a.category == b.category) { | 423 | if (a.category == b.category) { | ||
404 | if (a.exclusiveGroup == b.exclusiveGroup) { | 424 | if (a.exclusiveGroup == b.exclusiveGroup) { | ||
405 | return a.name < b.name; | 425 | return a.name < b.name; | ||
406 | } | 426 | } | ||
407 | return a.exclusiveGroup < b.exclusiveGroup; | 427 | return a.exclusiveGroup < b.exclusiveGroup; | ||
408 | } | 428 | } | ||
409 | return a.category < b.category; | 429 | return a.category < b.category; | ||
Show All 36 Lines | 459 | if (it != m_effectsList.end()) { | |||
446 | } | 466 | } | ||
447 | } | 467 | } | ||
448 | } | 468 | } | ||
449 | } | 469 | } | ||
450 | self->deleteLater(); | 470 | self->deleteLater(); | ||
451 | }); | 471 | }); | ||
452 | } | 472 | } | ||
453 | 473 | | |||
454 | m_effectsChanged = m_effectsList; | 474 | m_effectsChanged = m_effectsList; | ||
davidedmundson: I think there's a problem here.
Previously:
m_effectsList is the current state that we think… | |||||
455 | endResetModel(); | 475 | endResetModel(); | ||
456 | } | 476 | } | ||
457 | 477 | | |||
458 | void EffectModel::syncEffectsToKWin() | 478 | void EffectModel::syncEffectsToKWin() | ||
459 | { | 479 | { | ||
460 | OrgKdeKwinEffectsInterface interface(QStringLiteral("org.kde.KWin"), | 480 | OrgKdeKwinEffectsInterface interface(QStringLiteral("org.kde.KWin"), | ||
461 | QStringLiteral("/Effects"), | 481 | QStringLiteral("/Effects"), | ||
462 | QDBusConnection::sessionBus()); | 482 | QDBusConnection::sessionBus()); | ||
▲ Show 20 Lines • Show All 160 Lines • Show Last 20 Lines |
I think there's a problem here.
Previously:
m_effectsList is the current state that we think kwin has
m_effectsChanged is what the user has set
we're now losing what we think kwin has
In ::syncEffectsToKWin (line 484 here)
we compare the .effectsStatus of both
If a user makes a change, and then we call reload, and then the user saves:
m_effectsList[0]->effectStatus is the user set value
m_effectsList[0]->changed is true
so we'll write out the correct value in the config file all successfully
but m_effectsList.at(it).effectStatus == m_effectsChanged.at(it).effectStatus so we won't notify kwin.