<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/123896/">https://git.reviewboard.kde.org/r/123896/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been marked as submitted.</h1>
</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 Marble.</div>
<div>By Dennis Nienhüser.</div>
<p style="color: grey;"><i>Updated June 1, 2015, 4:40 p.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;">Submitted with commit ac2a7714a45b49db86cea1a1526da6981d4d9c07 by Dennis Nienhüser to branch master.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
marble
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </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;">Our current DBus interface exposes all signals, slots and properties of both MarbleWidget and MarbleMap to the DBus session bus. There are a couple of problems:
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> 3rd party developers who include MarbleWidget have their application exposed to DBus and the Marble part of it can be controlled from there.
</em> not all method signatures are compatible with DBus. In particular QRegion and Marble specific types cannot be send over DBus (without us implementing support for it). In Qt5 there seems to be a change that warns against that in the shell. This means that in Qt5 you get lots of debug spam all the time in the shell, e.g. a simple map drag results in hundreds of warnings a la
QDBusAbstractAdaptor: Cannot relay signal Marble::MarbleMap::renderStateChanged(RenderState): Unregistered input type in parameter list: RenderState
QDBusAbstractAdaptor: Cannot relay signal Marble::MarbleWidget::mouseClickGeoPosition(double,double,GeoDataCoordinates::Unit): Unregistered input type in parameter list: GeoDataCoordinates::Unit
QDBusAbstractAdaptor: Cannot relay signal Marble::MarbleMap::repaintNeeded(QRegion): Type not registered with QtDBus in parameter list: QRegion
* it's an utter mess. Currently we expose more than 120 (!) Marble specific things to DBus</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch tries to improve that by
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> Not exposing anything to DBus in the library, but only from the Qt and KDE applications (i.e. move session bus registration to ControlView.cpp)
</em> Expose only a limited subset of methods and properties (implemented in the new class MarbleDBusInterface)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The drawbacks are that we're changing the interface (hence break any external scripts etc. that rely on it), and limit it at the same time -- some information that people might be using is not available anymore. I'm not aware of anyone really using the DBus interface though, so I'd risk changing it. For limited functionality I'm happy to bring in more things if there's a sane use case behind it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The two screenshots show a sketch of the old and the refactored DBus interface.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Refactored DBus Interface:
<img alt="Refactored DBus interface (in qdbusviewer)" src="http://nienhueser.de/marble/marble-dbus-interface-1.png" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /></p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Current DBus Interface:
<img alt="Current DBus interface (in qdbusviewer)" src="http://nienhueser.de/marble/marble-dbus-interface-old.png" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /></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;">Tested retrieval and changing of all properties/methods using qdbusviewer-qt5. Tested signal emission using dbus-monitor.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/apps/marble-ui/ControlView.cpp <span style="color: grey">(432c11a)</span></li>
<li>src/lib/marble/CMakeLists.txt <span style="color: grey">(b52fb1f)</span></li>
<li>src/lib/marble/MarbleDBusInterface.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/marble/MarbleDBusInterface.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/marble/MarbleMap.cpp <span style="color: grey">(13becfc)</span></li>
<li>src/lib/marble/MarbleWidget.cpp <span style="color: grey">(f83fcb8)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123896/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>