Differential D27955 Diff 77307 kcmkwin/kwincompositing/compositing.cpp

Changeset View

Standalone View

View Options

kcmkwin/kwincompositing/compositing.cpp

Show All 26 Lines
27#include <KLocalizedString> 27​#include <KLocalizedString>
28#include <KSharedConfig> 28​#include <KSharedConfig>
29 29​
30#include <QDBusInterface> 30​#include <QDBusInterface>
31#include <QDBusReply> 31​#include <QDBusReply>
32#include <QHash> 32​#include <QHash>
33#include <QDebug> 33​#include <QDebug>
34 34​
35​#include "kwincompositing_setting.h"
36​
35namespace KWin { 37​namespace KWin {
36namespace Compositing { 38​namespace Compositing {
37 39​
38Compositing::Compositing(QObject *parent) 40​Compositing::Compositing(QObject *parent)
39 : QObject(parent) 41​ : QObject(parent)
40 , m_animationSpeed(1.0) 42​ , m_animationSpeed(1.0)
41 , m_windowThumbnail(0) 43​ , m_windowThumbnail(0)
42 , m_glScaleFilter(0) 44​ , m_glScaleFilter(0)
43 , m_xrScaleFilter(false) 45​ , m_xrScaleFilter(false)
44 , m_glSwapStrategy(0) 46​ , m_glSwapStrategy(0)
45 , m_compositingType(0) 47​ , m_compositingType(0)
46 , m_compositingEnabled(true) 48​ , m_compositingEnabled(true)
47 , m_changed(false) 49​ , m_changed(false)
48 , m_openGLPlatformInterfaceModel(new OpenGLPlatformInterfaceModel(this)) 50​ , m_openGLPlatformInterfaceModel(new OpenGLPlatformInterfaceModel(this))
49 , m_openGLPlatformInterface(0) 51​ , m_openGLPlatformInterface(0)
50 , m_windowsBlockCompositing(true) 52​ , m_windowsBlockCompositing(true)
51 , m_compositingInterface(new OrgKdeKwinCompositingInterface(QStringLiteral("org.kde.KWin"), QStringLiteral("/Compositor"), QDBusConnection::sessionBus(), this)) 53​ , m_compositingInterface(new OrgKdeKwinCompositingInterface(QStringLiteral("org.kde.KWin"), QStringLiteral("/Compositor"), QDBusConnection::sessionBus(), this))
52 , m_config(KSharedConfig::openConfig("kwinrc")) 54​ , m_settings(new KWinCompositingSetting(parent))
53{ 55​{
54 reset(); 56​ load();
55 connect(this, &Compositing::animationSpeedChanged, this, &Compositing::changed); 57​ connect(this, &Compositing::animationSpeedChanged, this, &Compositing::onChanged);
zzagUnsubmitted
Done

Naming nitpick: onChanged isn't really a great name for a signal. Usually, "on" is used with slots.

What about connecting Compositing::animationSpeedChanged to Compositing::updateSettings? e.g.

connect(this, &Compositing::animationSpeedChanged,       this, &Compositing::updateSettings);
zzag: Naming nitpick: onChanged isn't really a great name for a signal. Usually, "on" is used with…
56 connect(this, &Compositing::windowThumbnailChanged, this, &Compositing::changed); 58​ connect(this, &Compositing::windowThumbnailChanged, this, &Compositing::onChanged);
57 connect(this, &Compositing::glScaleFilterChanged, this, &Compositing::changed); 59​ connect(this, &Compositing::glScaleFilterChanged, this, &Compositing::onChanged);
58 connect(this, &Compositing::xrScaleFilterChanged, this, &Compositing::changed); 60​ connect(this, &Compositing::xrScaleFilterChanged, this, &Compositing::onChanged);
59 connect(this, &Compositing::glSwapStrategyChanged, this, &Compositing::changed); 61​ connect(this, &Compositing::glSwapStrategyChanged, this, &Compositing::onChanged);
60 connect(this, &Compositing::compositingTypeChanged, this, &Compositing::changed); 62​ connect(this, &Compositing::compositingTypeChanged, this, &Compositing::onChanged);
61 connect(this, &Compositing::compositingEnabledChanged, this, &Compositing::changed); 63​ connect(this, &Compositing::compositingEnabledChanged, this, &Compositing::onChanged);
62 connect(this, &Compositing::openGLPlatformInterfaceChanged, this, &Compositing::changed); 64​ connect(this, &Compositing::openGLPlatformInterfaceChanged, this, &Compositing::onChanged);
63 connect(this, &Compositing::windowsBlockCompositingChanged, this, &Compositing::changed); 65​ connect(this, &Compositing::windowsBlockCompositingChanged, this, &Compositing::onChanged);
64 66​
65 connect(this, &Compositing::changed, [this]{ 67​ connect(this, &Compositing::onChanged, [this]{
66 m_changed = true; 68​ updateSettings();
67 }); 69​ });
zzagUnsubmitted
Done

Shouldn't the parent be this?

zzag: Shouldn't the parent be `this`?
68} 70​}
69 71​
70void Compositing::reset() 72​void Compositing::load()
71{ 73​{
72 KConfigGroup globalConfig(m_config, QStringLiteral("KDE")); 74​ m_settings->load();
73 setAnimationSpeed(globalConfig.readEntry("AnimationDurationFactor", 1.0)); ​
74 ​
75 KConfigGroup kwinConfig(m_config, QStringLiteral("Compositing")); ​
76 setWindowThumbnail(kwinConfig.readEntry("HiddenPreviews", 5) - 4); ​
77 setGlScaleFilter(kwinConfig.readEntry("GLTextureFilter", 2)); ​
78 setXrScaleFilter(kwinConfig.readEntry("XRenderSmoothScale", false)); ​
79 setCompositingEnabled(kwinConfig.readEntry("Enabled", true)); ​
80 75​
81 auto swapStrategy = [&kwinConfig]() { 76​ applyValues();
82 const QString glSwapStrategyValue = kwinConfig.readEntry("GLPreferBufferSwap", "a"); ​
83 77​
84 if (glSwapStrategyValue == "n") { 78​ m_changed = false;
85 return 0; 79​ emit defaulted(m_settings->isDefaults());
86 } else if (glSwapStrategyValue == "a") { ​
87 return 1; ​
88 } else if (glSwapStrategyValue == "e") { ​
89 return 2; ​
90 } else if (glSwapStrategyValue == "p") { ​
91 return 3; ​
92 } else if (glSwapStrategyValue == "c") { ​
93 return 4; ​
94 } 80​}
95 return 0; ​
96 }; ​
97 setGlSwapStrategy(swapStrategy()); ​
98 81​
99 auto type = [&kwinConfig]{ 82​void Compositing::applyValues()
100 const QString backend = kwinConfig.readEntry("Backend", "OpenGL"); 83​{
101 const bool glCore = kwinConfig.readEntry("GLCore", false); 84​ setAnimationSpeed(m_settings->animationDurationFactor());
85​ setWindowThumbnail(m_settings->hiddenPreviews() - 4);
ervinUnsubmitted
Done

That - 4 is very obscure here

ervin: That `- 4` is very obscure here
zzagUnsubmitted
Done

Heh, - 4 and + 4 were obscure before so this patch doesn't make things better or worse. Just for context, we add and subtract 4 because long time ago HiddenPreviews had values with different meaning than what we have now. In order to break backwards compatibility, the new values were shifted by 4.

zzag: Heh, `- 4` and `+ 4` were obscure before so this patch doesn't make things better or worse.
ervinUnsubmitted
Done

And we didn't wrap that in conversion functions? :-)

ervin: And we didn't wrap that in conversion functions? :-)
86​ setGlScaleFilter(m_settings->gLTextureFilter());
ervinUnsubmitted
Done

Would be nice to use key + name in the kcfg to have proper capitalization for "gl"

ervin: Would be nice to use key + name in the kcfg to have proper capitalization for "gl"
87​ setXrScaleFilter(m_settings->xRenderSmoothScale());
88​ setCompositingEnabled(m_settings->enabled());
89​ setGlSwapStrategy(m_settings->gLPreferBufferSwap());
ervinUnsubmitted
Done

ditto

ervin: ditto
90​
91​ const auto type = [this]{
92​ const int backend = m_settings->backend();
93​ const bool glCore = m_settings->gLCore();
102 94​
103 if (backend == QStringLiteral("OpenGL")) { 95​ if (backend == KWinCompositingSetting::EnumBackend::OpenGL) {
104 if (glCore) { 96​ if (glCore) {
105 return CompositingType::OPENGL31_INDEX; 97​ return CompositingType::OPENGL31_INDEX;
106 } else { 98​ } else {
107 return CompositingType::OPENGL20_INDEX; 99​ return CompositingType::OPENGL20_INDEX;
108 } 100​ }
109 } else { 101​ } else {
110 return CompositingType::XRENDER_INDEX; 102​ return CompositingType::XRENDER_INDEX;
111 } 103​ }
112 }; 104​ };
113 setCompositingType(type()); 105​ setCompositingType(type());
114 106​
115 const QModelIndex index = m_openGLPlatformInterfaceModel->indexForKey(kwinConfig.readEntry("GLPlatformInterface", "glx")); 107​ const QModelIndex index = m_openGLPlatformInterfaceModel->indexForKey(m_settings->gLPlatformInterface());
ervinUnsubmitted
Done

ditto

ervin: ditto
116 setOpenGLPlatformInterface(index.isValid() ? index.row() : 0); 108​ setOpenGLPlatformInterface(index.isValid() ? index.row() : 0);
117 109​
118 setWindowsBlockCompositing(kwinConfig.readEntry("WindowsBlockCompositing", true)); 110​ setWindowsBlockCompositing(m_settings->windowsBlockCompositing());
119 ​
120 m_changed = false; ​
121} 111​}
122 112​
123void Compositing::defaults() 113​void Compositing::defaults()
124{ 114​{
125 setAnimationSpeed(1.0); 115​ m_settings->setDefaults();
126 setWindowThumbnail(1); 116​
127 setGlScaleFilter(2); 117​ applyValues();
128 setXrScaleFilter(false); 118​
129 setGlSwapStrategy(1); 119​ m_changed = m_settings->isSaveNeeded();
130 setCompositingType(CompositingType::OPENGL20_INDEX); ​
131 const QModelIndex index = m_openGLPlatformInterfaceModel->indexForKey(QStringLiteral("glx")); ​
132 setOpenGLPlatformInterface(index.isValid() ? index.row() : 0); ​
133 setWindowsBlockCompositing(true); ​
134 m_changed = true; ​
135} 120​}
136 121​
137bool Compositing::OpenGLIsUnsafe() const 122​bool Compositing::OpenGLIsUnsafe() const
138{ 123​{
139 KConfigGroup kwinConfig(m_config, "Compositing"); 124​ return m_settings->openGLIsUnsafe();
140 return kwinConfig.readEntry("OpenGLIsUnsafe", true); ​
141} 125​}
142 126​
143bool Compositing::OpenGLIsBroken() 127​bool Compositing::OpenGLIsBroken()
144{ 128​{
145 KConfigGroup kwinConfig(m_config, "Compositing"); 129​ const int oldBackend = m_settings->backend();
146 130​ m_settings->setBackend(KWinCompositingSetting::EnumBackend::OpenGL);
147 QString oldBackend = kwinConfig.readEntry("Backend", "OpenGL"); 131​ m_settings->save();
148 kwinConfig.writeEntry("Backend", "OpenGL"); ​
149 kwinConfig.sync(); ​
150 132​
151 if (m_compositingInterface->openGLIsBroken()) { 133​ if (m_compositingInterface->openGLIsBroken()) {
152 kwinConfig.writeEntry("Backend", oldBackend); 134​ m_settings->setBackend(oldBackend);
153 kwinConfig.sync(); 135​ m_settings->save();
154 return true; 136​ return true;
155 } 137​ }
156 138​
157 kwinConfig.writeEntry("OpenGLIsUnsafe", false); 139​ m_settings->setOpenGLIsUnsafe(false);
158 kwinConfig.sync(); 140​ m_settings->save();
159 return false; 141​ return false;
160} 142​}
161 143​
162void Compositing::reenableOpenGLDetection() 144​void Compositing::reenableOpenGLDetection()
163{ 145​{
164 KConfigGroup kwinConfig(m_config, "Compositing"); 146​ m_settings->setOpenGLIsUnsafe(false);
165 kwinConfig.writeEntry("OpenGLIsUnsafe", false); 147​ m_settings->save();
166 kwinConfig.sync(); ​
167} 148​}
168 149​
169qreal Compositing::animationSpeed() const 150​qreal Compositing::animationSpeed() const
170{ 151​{
171 return m_animationSpeed; 152​ return m_animationSpeed;
172} 153​}
173 154​
174int Compositing::windowThumbnail() const 155​int Compositing::windowThumbnail() const
▲ Show 20 Lines • Show All 88 Lines • ▼ Show 20 Line(s)240{
263 if (enabled == m_compositingEnabled) { 244​ if (enabled == m_compositingEnabled) {
264 return; 245​ return;
265 } 246​ }
266 247​
267 m_compositingEnabled = enabled; 248​ m_compositingEnabled = enabled;
268 emit compositingEnabledChanged(enabled); 249​ emit compositingEnabledChanged(enabled);
269} 250​}
270 251​
271void Compositing::save() 252​void Compositing::updateSettings()
272{ 253​{
273 // this writes to the KDE group of the kwinrc, when loading we rely on kconfig cascading to 254​ // this writes to the KDE group of the kwinrc, when loading we rely on kconfig cascading to
274 // load a global value, or allow a kwin override 255​ // load a global value, or allow a kwin override
275 KConfigGroup generalConfig(m_config, "KDE"); ​
276 if (!isRunningPlasma()) { 256​ if (!isRunningPlasma()) {
277 generalConfig.writeEntry("AnimationDurationFactor", animationSpeed()); 257​ m_settings->setAnimationDurationFactor(animationSpeed());
278 } 258​ }
279 KConfigGroup kwinConfig(m_config, "Compositing"); ​
280 259​
281 kwinConfig.writeEntry("HiddenPreviews", windowThumbnail() + 4); 260​ m_settings->setHiddenPreviews(windowThumbnail() + 4);
282 kwinConfig.writeEntry("GLTextureFilter", glScaleFilter()); 261​ m_settings->setGLTextureFilter(glScaleFilter());
283 kwinConfig.writeEntry("XRenderSmoothScale", xrScaleFilter()); 262​ m_settings->setXRenderSmoothScale(xrScaleFilter());
284 if (!compositingRequired()) { 263​ if (!compositingRequired()) {
285 kwinConfig.writeEntry("Enabled", compositingEnabled()); 264​ m_settings->setEnabled(compositingEnabled());
286 } 265​ }
287 auto swapStrategy = [this] { 266​ m_settings->setGLPreferBufferSwap(glSwapStrategy());
288 switch (glSwapStrategy()) { 267​ int backend;
ervinUnsubmitted
Done

would be nice to initialize it

ervin: would be nice to initialize it
289 case 0: ​
290 return QStringLiteral("n"); ​
291 case 2: ​
292 return QStringLiteral("e"); ​
293 case 3: ​
294 return QStringLiteral("p"); ​
295 case 4: ​
296 return QStringLiteral("c"); ​
297 case 1: ​
298 default: ​
299 return QStringLiteral("a"); ​
300 } ​
301 }; ​
302 kwinConfig.writeEntry("GLPreferBufferSwap", swapStrategy()); ​
303 QString backend; ​
304 bool glCore = false; 268​ bool glCore = false;
305 switch (compositingType()) { 269​ switch (compositingType()) {
306 case CompositingType::OPENGL31_INDEX: 270​ case CompositingType::OPENGL31_INDEX:
307 backend = "OpenGL"; 271​ backend = KWinCompositingSetting::EnumBackend::OpenGL;
308 glCore = true; 272​ glCore = true;
309 break; 273​ break;
310 case CompositingType::OPENGL20_INDEX: 274​ case CompositingType::OPENGL20_INDEX:
311 backend = "OpenGL"; 275​ backend = KWinCompositingSetting::EnumBackend::OpenGL;
312 glCore = false; 276​ glCore = false;
313 break; 277​ break;
314 case CompositingType::XRENDER_INDEX: 278​ case CompositingType::XRENDER_INDEX:
315 backend = "XRender"; 279​ backend = KWinCompositingSetting::EnumBackend::XRender;
316 glCore = false; 280​ glCore = false;
317 break; 281​ break;
318 } 282​ }
319 kwinConfig.writeEntry("Backend", backend); 283​ m_settings->setBackend(backend);
320 kwinConfig.writeEntry("GLCore", glCore); 284​ m_settings->setGLCore(glCore);
321 if (!compositingRequired()) { 285​ if (!compositingRequired()) {
322 kwinConfig.writeEntry("WindowsBlockCompositing", windowsBlockCompositing()); 286​ m_settings->setWindowsBlockCompositing(windowsBlockCompositing());
323 } 287​ }
324 kwinConfig.sync(); 288​
289​ emit changed(m_settings->isSaveNeeded());
290​ emit defaulted(m_settings->isDefaults());
291​}
292​
293​void Compositing::save()
294​{
295​ updateSettings();
296​ m_settings->save();
325 297​
326 if (m_changed) { 298​ if (m_changed) {
zzagUnsubmitted
Done

Given that m_changed is not set to true when anything has been changed, there is a chance that we won't send a reinit request.

We probably need to save the return value of m_settings->isSaveNeeded() before calling m_settings->save() and check it here.

zzag: Given that m_changed is not set to true when anything has been changed, there is a chance that…
327 // Send signal to all kwin instances 299​ // Send signal to all kwin instances
328 QDBusMessage message = QDBusMessage::createSignal(QStringLiteral("/Compositor"), 300​ QDBusMessage message = QDBusMessage::createSignal(QStringLiteral("/Compositor"),
329 QStringLiteral("org.kde.kwin.Compositing"), 301​ QStringLiteral("org.kde.kwin.Compositing"),
330 QStringLiteral("reinit")); 302​ QStringLiteral("reinit"));
331 QDBusConnection::sessionBus().send(message); 303​ QDBusConnection::sessionBus().send(message);
332 m_changed = false; 304​ m_changed = false;
333 } 305​ }
334} 306​}
▲ Show 20 Lines • Show All 79 Lines • ▼ Show 20 Line(s)385{
414 QHash<int, QByteArray> roleNames; 386​ QHash<int, QByteArray> roleNames;
415 roleNames[NameRole] = "NameRole"; 387​ roleNames[NameRole] = "NameRole";
416 roleNames[TypeRole] = QByteArrayLiteral("type"); 388​ roleNames[TypeRole] = QByteArrayLiteral("type");
417 return roleNames; 389​ return roleNames;
418} 390​}
419 391​
420QModelIndex CompositingType::index(int row, int column, const QModelIndex &parent) const 392​QModelIndex CompositingType::index(int row, int column, const QModelIndex &parent) const
421{ 393​{
422 ​
423if (parent.isValid() || column > 0 || column < 0 || row < 0 || row >= m_compositingList.count()) { 394​ if (parent.isValid() || column > 0 || column < 0 || row < 0 || row >= m_compositingList.count()) {
424 return QModelIndex(); 395​ return QModelIndex();
425 } 396​ }
426 397​
427 return createIndex(row, column); 398​ return createIndex(row, column);
428} 399​}
429 400​
430QModelIndex CompositingType::parent(const QModelIndex &child) const 401​QModelIndex CompositingType::parent(const QModelIndex &child) const
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines