Use KConfig XT to manage most fields of the KCM.
Simplify code.
Depends on D27955
Details
Details
Diff Detail
Diff Detail
- Repository
- R108 KWin
- Branch
- arcpatch-D27988_1
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 25500 Build 25518: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
| kcmkwin/kwincompositing/main.cpp | ||
|---|---|---|
| 91 | Is there a reason why the parent is not this? | |
| kcmkwin/kwincompositing/main.cpp | ||
|---|---|---|
| 95–96 | Yes, this is captured but I would say it's not an excuse for using a lambda. A method would be more appropriate in this case. | |
| kcmkwin/kwincompositing/kwincompositing_setting.kcfg | ||
|---|---|---|
| 48 | Perhaps we can add comment, kcfg can be used as documentation | |
| kcmkwin/kwincompositing/main.cpp | ||
| 258 | Using m_settings to track unmanaged state lead to settings containing old data and new data. Managed widget update settings only on save. | |
| kcmkwin/kwincompositing/kwincompositing_setting.kcfg | ||
|---|---|---|
| 48 | I have added the proper enum's name | |
| kcmkwin/kwincompositing/kwincompositing_setting.kcfg | ||
|---|---|---|
| 25 | This is asking for name + value (to avoid the dreaded "gL" on the C++ side) | |
| 45 | ditto | |
| 64 | ditto | |
| 72 | ditto | |
| kcmkwin/kwincompositing/main.cpp | ||
| 118 | { should be on its own line | |
| 265 | I'd split the clauses on several lines for readability | |
| 268 | ditto (especially for that one the line is getting quite long) | |
Comment Actions
Hmm, I cannot test this kcm because it crashes.
lldb output
* thread #1, name = 'kcmshell5', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
frame #0: 0x00007fffef767d07 kwincompositing.so`KWinCompositingKCM::load(this=0x00005555555ef6c0) at main.cpp:243:68
240 m_form.animationDurationFactor->setValue(index);
241
242 m_settings->findItem("Backend")->readConfig(m_settings->config());
-> 243 m_settings->findItem("GLCore")->readConfig(m_settings->config());
244 m_backend = m_settings->backend();
245 if (m_backend == KWinCompositingSetting::EnumBackend::OpenGL) {
246 m_glCore = m_settings->glCore();
(lldb) bt
* thread #1, name = 'kcmshell5', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
* frame #0: 0x00007fffef767d07 kwincompositing.so`KWinCompositingKCM::load(this=0x00005555555ef6c0) at main.cpp:243:68
frame #1: 0x00007fffef7682a2 kwincompositing.so`KWinCompositingKCM::qt_static_metacall(_o=0x00005555555ef6c0, _c=InvokeMetaMethod, _id=0, _a=0x000055555567a478) at main.moc:85:25
frame #2: 0x00007ffff605daab libQt5Core.so.5`QMetaCallEvent::placeMetaCall(this=0x000055555567a430, object=0x00005555555ef6c0) at qobject.cpp:617:24
frame #3: 0x00007ffff605e9a5 libQt5Core.so.5`QObject::event(this=0x00005555555ef6c0, e=0x000055555567a430) at qobject.cpp:1314:31
frame #4: 0x00007ffff6ee188d libQt5Widgets.so.5`QWidget::event(this=0x00005555555ef6c0, event=0x000055555567a430) at qwidget.cpp:9089:30
frame #5: 0x00007ffff6e92ea5 libQt5Widgets.so.5`QApplicationPrivate::notify_helper(this=0x000055555557ea20, receiver=0x00005555555ef6c0, e=0x000055555567a430) at qapplication.cpp:3671:31
frame #6: 0x00007ffff6e92c95 libQt5Widgets.so.5`QApplication::notify(this=0x00007fffffffdf70, receiver=0x00005555555ef6c0, e=0x000055555567a430) at qapplication.cpp:3621:31
frame #7: 0x00007ffff6016ddf libQt5Core.so.5`QCoreApplication::notifyInternal2(receiver=0x00005555555ef6c0, event=0x000055555567a430) at qcoreapplication.cpp:1061:24
frame #8: 0x00007ffff601783c libQt5Core.so.5`QCoreApplication::sendEvent(receiver=0x00005555555ef6c0, event=0x000055555567a430) at qcoreapplication.cpp:1456:27
frame #9: 0x00007ffff60185ab libQt5Core.so.5`QCoreApplicationPrivate::sendPostedEvents(receiver=0x0000000000000000, event_type=0, data=0x000055555556c400) at qcoreapplication.cpp:1815:36
frame #10: 0x00007ffff6017ee4 libQt5Core.so.5`QCoreApplication::sendPostedEvents(receiver=0x0000000000000000, event_type=0) at qcoreapplication.cpp:1674:46
frame #11: 0x00007ffff60b0164 libQt5Core.so.5`::postEventSourceDispatch(s=0x00005555555e4380, (null)=0x0000000000000000, (null)=0x0000000000000000) at qeventdispatcher_glib.cpp:277:39
frame #12: 0x00007ffff198c9be libglib-2.0.so.0`g_main_context_dispatch + 638
frame #13: 0x00007ffff198e831 libglib-2.0.so.0`___lldb_unnamed_symbol354$$libglib-2.0.so.0 + 545
frame #14: 0x00007ffff198e871 libglib-2.0.so.0`g_main_context_iteration + 49
frame #15: 0x00007ffff60b08bf libQt5Core.so.5`QEventDispatcherGlib::processEvents(this=0x00005555555e75d0, flags=(i = 36)) at qeventdispatcher_glib.cpp:423:43
frame #16: 0x00007fffef83242a libQt5XcbQpa.so.5`QXcbGlibEventDispatcher::processEvents(this=0x00005555555e75d0, flags=(i = 36)) at qxcbeventdispatcher.cpp:143:47
frame #17: 0x00007ffff601347b libQt5Core.so.5`QEventLoop::processEvents(this=0x00007fffffffde30, flags=(i = 36)) at qeventloop.cpp:139:68
frame #18: 0x00007ffff60137dd libQt5Core.so.5`QEventLoop::exec(this=0x00007fffffffde30, flags=(i = 0)) at qeventloop.cpp:232:22
frame #19: 0x00007ffff601767e libQt5Core.so.5`QCoreApplication::exec() at qcoreapplication.cpp:1369:36
frame #20: 0x00007ffff6560bd4 libQt5Gui.so.5`QGuiApplication::exec() at qguiapplication.cpp:1864:34
frame #21: 0x00007ffff6e8fa27 libQt5Widgets.so.5`QApplication::exec() at qapplication.cpp:2811:33
frame #22: 0x00007ffff7f7e4d3 libkdeinit5_kcmshell5.so`kdemain + 6435
frame #23: 0x00007ffff7dd6023 libc.so.6`__libc_start_main + 243
frame #24: 0x000055555555505e kcmshell5`_start + 46| kcmkwin/kwincompositing/kwincompositing_setting.kcfg | ||
|---|---|---|
| 71–72 | Please remove this entry because it's unused. In general, it would probably be nice to re-introduce the "OpenGL Platform Interface" option. It was originally present in the QML based version of this KCM, but then we switched back to qwidgets. :/ | |
| kcmkwin/kwincompositing/main.cpp | ||
| 67 | Make it static. | |
| 105 | stray whitespace between "kcfg_WindowsBlockCompositing" and "->". Please remove it. | |
| 245 | Maybe glCore? | |
| kcmkwin/kwincompositing/main.cpp | ||
|---|---|---|
| 245 | Indeed this caused the crash when I changed the name for this entry. | |
| kcmkwin/kwincompositing/main.cpp | ||
|---|---|---|
| 176–180 | It's translatable ? | |
| kcmkwin/kwincompositing/main.cpp | ||
|---|---|---|
| 176–180 | 1/ That's not much related to this diff : this wasn't changed. 2/ That's up to translators to say I would guess. | |
Comment Actions
"Scale method" and "Scale method" must be visible only when the rendering backend is set to "XRender".
Comment Actions
Avoid small glitch in form scale method field position when switching backend between opengl and Xrender
| kcmkwin/kwincompositing/main.cpp | ||
|---|---|---|
| 38 | Put the opening brace on newline. | |
Comment Actions
Most of kcm settings can have their field marked immutable by system administrators i.e https://userbase.kde.org/KDE_System_Administration/Kiosk/Introduction
And fields that are handled automatically by KConfig XT + KCM.
Those two fields have to be separately handled since they couldn't be handled this way.