<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/122279/">https://git.reviewboard.kde.org/r/122279/</a>
</td>
</tr>
</table>
<br />
<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've just pushed a slightly modified version. Can you check if it solves you problem.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">NOTE: one issue with include X11 header in Qt project is that it might interfere with some qt headers (in a way that I don't understand and don't want to figure out), which is what <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">fixx11h.h</code> is for. Also, please don't use any Qt macro in the lib folder unless it is only meant to be use for Qt (and there's no way around that), in which case, it should be wrapped qt detection macros.</p></pre>
<br />
<p>- Yichao Yu</p>
<br />
<p>On 一月 27th, 2015, 1:05 p.m. EST, René J.V. Bertin 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 Software on Mac OS X and Yichao Yu.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated 一月 27, 2015, 1:05 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
qtcurve
</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;">Two regressions were exposed/introduced by the recent efforts to make the code build even on systems without X11, notably OS X.</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When KDE4 is installed (for instance through MacPorts) and KDE4 support is activated, a KDE header is included (kapplication.h) that has its own "stub" typedef for the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Display</code> type. Only it defines the type to be a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">void*</code> rather than using the definition XOrg uses.
I have proposed to change that in kdelibs, but in the meantime it is safer to avoid typedef'ing DISPLAY on OS X, and use a different function prototype for the 2 function prototypes in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">x11base.h</code> that require it.
This does not appear to be an issue when building for GTk2/X11 on OS X.</p>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When building against a non-MacPorts (or Fink, or HomeBrew) Qt install, X headers will typically not be available. The toplevel CMake file catches that and includes a minimal stub from the QtCurve source tree, but <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> when the user has set <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QTC_ENABLE_X11=Off</code>.
This patch adds the forgotten <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">includes_directories</code> statement, and adds the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">SYSTEM</code> keyword in both cases because the xcb.h header is included as a system header using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">#include <xcb/xcb.h></code> (i.e. with sharp brackets and not double quotes).</p>
</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've also corrected a small typo in the CMake file.</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;">On OS X against Qt4+KDE 4.14.4, GTk2/X11 and Qt 5.4.0 ("Qt only") all from MacPorts.
On OS X against Qt 5.4.0 installed through Digia's installer.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">FWIW, my Qt 5.4.0/MacPorts install has both the regular "Cocoa" platform plugin and the xcb platform plugin. QtCurve works with both, and with the xcb plugin on local and remote X11 displays.</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>CMakeLists.txt <span style="color: grey">(9bc4c3d)</span></li>
<li>lib/utils/x11base.h <span style="color: grey">(7c41223)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122279/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>