<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126101/">https://git.reviewboard.kde.org/r/126101/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma, Solid, Àlex Fiestas, Aleix Pol Gonzalez, Daniel Vrátil, and Martin Gräßlin.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated Nov. 20, 2015, 12:16 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks for reviewing again, latest changes address the remaining comments.
Most interesting change is that the plugin loading argument has moved from ConfigOperation into BackendManager, as suggested.
The code is now a bit more agressive towards switching backends, shutdownBackend is run whenever the mode or the backend changes. I've added there that in case we get a new backend requested, and the new backend would be loaded while the old isn't shut down, we run shutdownBackend() anyway. We know it's the right thing to do here, so instead of warning, or bailing out, we shutdown first, and then continue loading the new backend freshly.
make test passes entirely, though I get dbus timeouts in testconfigmonitor when running it directly from the commandline. It may (or may not) be related to the backendlauncher not shutting down correctly, but I haven't figured out if that's a new problem, or specific to a certain setup. I'll give this another good round of real world testing before pushing.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
libkscreen
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description (updated)</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patchset adds in-process operation to libkscreen</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">purpose:
- allow easier debugging
- for some backends (qscreen, upcoming kwayland) the out of process operation is not necessary since these backends are well-shielded</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This implementation is backwards compatible and opt-in for running setups.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If the user exports KSCREEN_BACKEND_INPROCESS=1 before running, all operations will be done in process. Otherwise, the out-of-process mode is used.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Changes revolve around the ConfigOperations, the BackendManager (which "factories" the backends, either in- or out-of-process), and the plugin loading logic</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Autotests should cover all the cases (and actually a few currently unsupported ones, such as using different backends in the same process).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Details on performance, etc.: http://vizzzion.org/blog/2015/11/wayland-and-libkscreen-benchmarks/</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Added a ton of autotests, made sure all existing ones pass.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">tried "KSCREEN_BACKEND_INPROCESS=1 kcmshell5 kscreen", all working as expected.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(86a0965)</span></li>
<li>autotests/CMakeLists.txt <span style="color: grey">(69af7f0)</span></li>
<li>autotests/testconfigmonitor.cpp <span style="color: grey">(a051226)</span></li>
<li>autotests/testinprocess.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/testqscreenbackend.cpp <span style="color: grey">(da4dbae)</span></li>
<li>autotests/testscreenconfig.cpp <span style="color: grey">(ecbcedf)</span></li>
<li>autotests/testxrandr.cpp <span style="color: grey">(b9b838a)</span></li>
<li>backends/fake/fake.cpp <span style="color: grey">(60264dd)</span></li>
<li>src/backendlauncher/backendloader.cpp <span style="color: grey">(52051df)</span></li>
<li>src/backendmanager.cpp <span style="color: grey">(ca9c746)</span></li>
<li>src/backendmanager_p.h <span style="color: grey">(c6418e2)</span></li>
<li>src/config.cpp <span style="color: grey">(75d947d)</span></li>
<li>src/configmonitor.h <span style="color: grey">(b6f1189)</span></li>
<li>src/configmonitor.cpp <span style="color: grey">(a14bc70)</span></li>
<li>src/configoperation.h <span style="color: grey">(2405d79)</span></li>
<li>src/configoperation.cpp <span style="color: grey">(87fe141)</span></li>
<li>src/configoperation_p.h <span style="color: grey">(afdc462)</span></li>
<li>src/getconfigoperation.h <span style="color: grey">(c85bfaa)</span></li>
<li>src/getconfigoperation.cpp <span style="color: grey">(22f92b4)</span></li>
<li>src/output.cpp <span style="color: grey">(bd381fa)</span></li>
<li>src/setconfigoperation.h <span style="color: grey">(020a990)</span></li>
<li>src/setconfigoperation.cpp <span style="color: grey">(6ea944f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126101/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>