<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/128567/">https://git.reviewboard.kde.org/r/128567/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 1st, 2016, 10:36 p.m. CEST, <b>Sune Vuorela</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/128567/diff/1/?file=472714#file472714line61" style="color: black; font-weight: bold; text-decoration: underline;">autotests/kwindowsystem_platform_wayland_test.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">61</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QVERIFY</span><span class="p">(</span><span class="n">socketSpy</span><span class="p">.</span><span class="n">wait</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;">Doesn't this stop when first wait fails?</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;">yes, that's fine. It would mean that in overall 5500 msec the socket did not appear. Which can be put as "Weston failed to start". The reason for the loop is rather to handle situation that other files are created before the socket file (e.g. the lock file of the socket) and we really wait for the socket to appear.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 1st, 2016, 10:36 p.m. CEST, <b>Sune Vuorela</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/128567/diff/1/?file=472716#file472716line587" style="color: black; font-weight: bold; text-decoration: underline;">src/kwindowsystem.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">587</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">enum</span> <span class="n">class</span> <span class="n">Platform</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;">Something about the word platform feels wrong here. But windowsystem also feels a bit wrong. But I guess Platform is the term used by Qt, so maybe it is okay.</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 deliberately decided for the name "Platform" as it's used by Qt.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 1st, 2016, 10:36 p.m. CEST, <b>Sune Vuorela</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/128567/diff/1/?file=472716#file472716line617" style="color: black; font-weight: bold; text-decoration: underline;">src/kwindowsystem.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">617</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">static</span> <span class="kt">bool</span> <span class="nf">isPlatformX11</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;">These convenience helpers just seems ... candidates for extreme growth. isPlatformSurfaceFlinger isPlatformWindows isPlatformWindowsRT isPlatformMacOsForWatch isPlatformMacOsForTV isPlatform.... Do we need them?</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;">Based on the usage in kde/workspace already I think we do need convenience helpers for the most common usages. Otherwise it might happen that people prefer if (QX11Info::isPlatformX11()) over if (KWindowSystem::platform() == KWindowSystem::Platform::X11).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also I don't expect that we will ever see support for these "exotic" platforms here in KWindowSystem. I'll only accept new platforms which also have a platform implementation. I don't expect that KWindowSystem will support Android or iOs.</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On August 1st, 2016, 5:27 p.m. CEST, Martin Gräßlin 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 KDE Frameworks.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Aug. 1, 2016, 5:27 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwindowsystem
</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;">A common pattern for our applications which have to do platform specific
tasks is to query the used platform with code like:
if (QX11Info::isPlatformX11()) {
// do X11 specific stuff
} else if (QGuiApplication::platformName().startsWith(QLatin1String("wayland"))) {
// do Wayland specific stuff
}
The problem with that is that it involves string comparisons and is easy
to get wrong. E.g. for X11 one should use the QX11Info helper, for
Wayland one has to compare with startsWith. There is lot of domain
specific knowledge going into it to make it work correctly and in an
efficient way.
This change introduces an enum based API which encapsulates the
knowledge: it does the proper comparisons and caches the result, so
that one does not need to do string comparisons again and again.
So far the implementation supports the platforms X11/XCB and Wayland.
In addition there are convenient methods to check for a specific
platform modelled after QX11Info::isPlatformX11.
Further platforms can be added by the maintainers of the respective
platforms.</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>autotests/CMakeLists.txt <span style="color: grey">(65ed8d4da50fd41dd3060576284456c83c3629ab)</span></li>
<li>autotests/helper/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/helper/wayland_platform.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/kwindowsystem_platform_wayland_test.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/kwindowsystemx11test.cpp <span style="color: grey">(cdf94e3f6573faea6dfd6038dded3ee1269e2552)</span></li>
<li>src/kwindowsystem.h <span style="color: grey">(551b01f5831e2fbdf0d0ad30f3debf0955da189e)</span></li>
<li>src/kwindowsystem.cpp <span style="color: grey">(4dfcce8bbfeed09f3f6ddec10a1eb5fd2d418313)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128567/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>