<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/120471/">https://git.reviewboard.kde.org/r/120471/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 6th, 2014, 10:25 a.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120471/diff/1/?file=316079#file316079line407" style="color: black; font-weight: bold; text-decoration: underline;">src/client/registry.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Q_SIGNALS:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">407</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">sync</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="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;">I would recommend to move it to ConnectionThread as it's more connection related then registry related.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also please add a test case for it (both threaded and unthreaded) - should be fairly simple in fact.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">I've experimented with that, and ran into a few issues.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We need to call</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">wl_display_get_registry(display);
wl_display_sync(display);</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">in this order, that's what the Wayland API suggests.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've tried moving the whole callback mechanism into ConnectionThread, but ConnectionThread doesn't know enough about the Registry to issue the wl_display_sync right after wl_display_get_registry. Essentially, sync really is a global sync, and comes from the registry, rather than the ConnectionThread.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Semantically, it does make more sense in ConnectionThread, along with connected() and failed() etc.. So I've tried keeping the mechanism in Registry, but emitting ConnectionThread's signal, if a connectionthread has been set on the registry. This also doesn't work very well, as in the tests (as an example), one doesn't get a sync signal unless one sets up the Registry. ConnectionThread isn't what triggers the wl_display_sync callback, it's set up in the registry.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So what the wl_display_sync callback really does, is "I'm done announcing interfaces", it is in fact more related to Registry, and not to ConnectionThread. In that regard, the current patch may be the best option, perhaps giving sync() a clearer name, and reflecting it better in the API docs? Other solutions altogether?</p></pre>
<br />
<p>- Sebastian</p>
<br />
<p>On October 3rd, 2014, 3:15 a.m. UTC, Sebastian Kügler wrote:</p>
<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 kwin, Plasma and Martin Gräßlin.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated Oct. 3, 2014, 3:15 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwayland
</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;">Add Registry::sync() signal</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Emitted when the Wayland display is done flushing the initial interface
callbacks, announcing wl_display properties. This can be used to compress
events. Note that this signal is emitted only after announcing interfaces,
such as outputs, but not after receiving callbacks of interface properties,
such as the output's geometry, modes, etc..
This signal is emitted from the wl_display_sync callback.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For this, we add a wl_callback_listener to the registry's Private,
enqueue its events properly, if necessary, and trigger the signal
through a callback mechanism similar to the wl_registry callbacks.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This signal allows users of the API to find out when the signal
emissions, such as outputAnnounced, etc. for all currently existing
interfaces is complete.</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;">tests in libkscreen exercise this feature, it works as expected, meaning I can notify when all initial synchronization is done.</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/client/registry.h <span style="color: grey">(9e63a2b20c9734cc599f8c612441165b20d361bd)</span></li>
<li>src/client/registry.cpp <span style="color: grey">(22f948488b88f2a9fbf2fd78f0d223d05585fe17)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120471/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>