<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/120437/">https://git.reviewboard.kde.org/r/120437/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 30th, 2014, 7:28 p.m. CEST, <b>Yichao Yu</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/120437/diff/2/?file=315817#file315817line27" style="color: black; font-weight: bold; text-decoration: underline;">CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">27</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">option</span><span class="p">(</span><span class="s">QTC_ENABLE_X11</span> <span class="s2">"Enable X11"</span> <span class="s">On</span><span class="p">)</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">27</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">option</span><span class="p">(</span><span class="s">QTC_ENABLE_X11</span> <span class="s2">"Enable X11"</span> <span class="s">On</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 see your point. It's fine on Qt4, where you know what the backend is at compile time. However, it is probably not possible on Qt5, and this behavior is introduced precisely to take that into account (all the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">qtcX11*</code> calls are no-op unless it is enabled at compile time and is initialized at runtime).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also these are not really Qt/X11 code.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
They are not Qt since they are provided by <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">libqtcurve-utils</code> which doesn't link to Qt.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
They are not X11 on OSX since <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">libqtcurve-utils</code> is not linked to X11 on OSX either...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The Qt/X11 calls are indeed filtered out. The Qt::X11 part is filtered out at initialization time (see <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">qt4/style/qtcurve_plugin.cpp</code>) and the X11(xcb) part is filtered out in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">libqtcurve-utils</code> (see <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">lib/utils/x11utils.c</code>) which makes all x11 calls no-op when xcb is not found or disabled.</p></pre>
</blockquote>
<p>On September 30th, 2014, 7:49 p.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Heh, whatever backend Qt5 uses on OS X (raster or OpenGL), it is certainly not X11, on that platform we can be quite sure about that!</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">They are not X11 on OSX since libqtcurve-utils is not linked to X11 on OSX either...</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You think so? ;)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">> otool -L /opt/local/lib/libqtcurve-utils.dylib
/opt/local/lib/libqtcurve-utils.1.0.dylib:
/opt/local/lib/libqtcurve-utils.1.dylib <span style="color: #666666">(</span>compatibility version 1.0.0, current version 1.0.0<span style="color: #666666">)</span>
/opt/local/MacPorts/lib/libX11-xcb.1.dylib <span style="color: #666666">(</span>compatibility version 2.0.0, current version 2.0.0<span style="color: #666666">)</span>
/opt/local/MacPorts/lib/libX11.6.dylib <span style="color: #666666">(</span>compatibility version 10.0.0, current version 10.0.0<span style="color: #666666">)</span>
/opt/local/lib/libxcb.1.dylib <span style="color: #666666">(</span>compatibility version 3.0.0, current version 3.0.0<span style="color: #666666">)</span>
/usr/lib/libSystem.B.dylib <span style="color: #666666">(</span>compatibility version 1.0.0, current version 125.2.11<span style="color: #666666">)</span>
/opt/local/lib/libgcc/libgcc_s.1.dylib <span style="color: #666666">(</span>compatibility version 1.0.0, current version 1.0.0<span style="color: #666666">)</span>
> otool -L /opt/local/lib/libxcb.1.dylib
/opt/local/lib/libxcb.1.dylib:
/opt/local/lib/libxcb.1.dylib <span style="color: #666666">(</span>compatibility version 3.0.0, current version 3.0.0<span style="color: #666666">)</span>
/opt/local/lib/libXau.6.dylib <span style="color: #666666">(</span>compatibility version 7.0.0, current version 7.0.0<span style="color: #666666">)</span>
/opt/local/lib/libXdmcp.6.dylib <span style="color: #666666">(</span>compatibility version 7.0.0, current version 7.0.0<span style="color: #666666">)</span>
/usr/lib/libSystem.B.dylib <span style="color: #666666">(</span>compatibility version 1.0.0, current version 125.2.11<span style="color: #666666">)</span>
</pre></div>
</p></pre>
</blockquote>
<p>On September 30th, 2014, 7:58 p.m. CEST, <b>Yichao Yu</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Actually by backend I mean the window system qpa plugin, which is probably quartz on OSX?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">= = .... I didn't know that libxcb is actually presented on OSX.... anyway ... it doesn't hurt to do this on Qt4....</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For Qt5, the issue is that there isn't a way at all to figure out at compile time whether it will run on X11 (even on Linux). But unless someone failed to compile qtcurve-qt5 on OSX, I won't worry about it ......</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;">Ah, in my own testing I set <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">ENABLE_X11=OFF</code> when building for Qt5. For now it's not possible to build for both Qt4 and Qt5 at the same time, at least not using MacPorts. I noticed it is on Linux; I presume the build system relies on qtchooser to get version-specific behaviour out of the same qmake programme?</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On September 30th, 2014, 7:07 p.m. CEST, 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, André Marcelo Alvarenga, Yuri Chornoivan, and Yichao Yu.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Sept. 30, 2014, 7:07 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;">KDE is built on a cross-platform framework, and as such most applications (not directly linked to the Plasma desktop or X11) function or can be made to function fine.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Qt will use a native style by default, but supports the same style plugins as it does on other platforms, which bring the advantage of better (more precise) and more compact layout, without by definition reducing integration with the OS X desktop.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As such, QtCurve 1.8.14 worked "out of the box" (http://kde-look.org/content/show.php?content=40492, source from http://craigd.wikispaces.com/file/view) and I created a MacPorts port for it (https://trac.macports.org/ticket/44527). With the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">kde4-workspace</code> port installed, one has almost the full customisation experience, minus everything window-manager related of course, nor full control of window backgrounds (not allowed by Qt and/or OS X).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The current version, which has seen many changes that require X11, was less straightforward to get to work, requiring a considerable collection of small changes.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The brunt of the present patchset consists of making the code in question conditional on the presence of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Q_WS_X11</code>, <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Q_OS_MAC</code>, <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">__MACH__</code> or <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">__APPLE__</code> in non Qt code and <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">APPLE</code> in the cmake files. A few patches introduce functions not available on OS X (getline) or replace them with OS X specifics (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">clock_gettime</code> -> <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">mach_absolute_timer</code>, nicely initialised with a dylib constructor function :) ) while others simply ensure that header files are found (or inexistent ones ignored).<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
The only functional guess/change I made is in <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">setOpacityProp()</code>, where I added a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">w->setWindowOpacity( 0.01 * opacity )</code> for non Q_WS_X11 code, presuming that <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">prop</code> is simply on the 0-100 percentage as exposed in the preferences dialog.</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 10.6.8 against kdelibs 4.14.1 and Qt 4.8.6 . This OS requires to build v1.8.18 with a gcc version from MacPorts in order to get the necessary C++11 support; newer OS versions will use a recent clang version (system compiler).</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;">Qt4/KDE4 support: OK (see screenshot)</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">GTk2 support: OK</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">X11 support: builds but I have no idea what it's supposed to do</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Qt5 support: OK (against Qt 5.3.1 obtained with Digia's installer)</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I tested building on Linux (Kubuntu 14.04, KDE SC 4.13.3, using clang 3.4) after the 2nd update to the patchset and that worked fine.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
One issue that could use attention is the Qt5 detection: Qt5.3 is apparently required and <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">ENABLE_QT5</code> is not unset when an earlier Qt5 version is found instead.</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">(f76fd1b)</span></li>
<li>gtk2/common/config_file.c <span style="color: grey">(d732ca9)</span></li>
<li>gtk2/style/CMakeLists.txt <span style="color: grey">(01e8891)</span></li>
<li>gtk2/style/qt_settings.c <span style="color: grey">(f5a5c98)</span></li>
<li>lib/cairo/CMakeLists.txt <span style="color: grey">(c66c63c)</span></li>
<li>lib/utils/CMakeLists.txt <span style="color: grey">(15757ed)</span></li>
<li>lib/utils/color.h <span style="color: grey">(2c7081f)</span></li>
<li>lib/utils/map.c <span style="color: grey">(a829e9e)</span></li>
<li>lib/utils/process.c <span style="color: grey">(f2490ef)</span></li>
<li>lib/utils/timer.c <span style="color: grey">(879451e)</span></li>
<li>qt4/config/CMakeLists.txt <span style="color: grey">(15454e6)</span></li>
<li>qt4/config/exportthemedialog.h <span style="color: grey">(42590ec)</span></li>
<li>qt4/config/exportthemedialog.cpp <span style="color: grey">(f39b86d)</span></li>
<li>qt4/kwin/CMakeLists.txt <span style="color: grey">(654604b)</span></li>
<li>qt4/kwinconfig/CMakeLists.txt <span style="color: grey">(cbd8b62)</span></li>
<li>qt4/style/CMakeLists.txt <span style="color: grey">(f38d029)</span></li>
<li>qt4/style/qtcurve.cpp <span style="color: grey">(7346c2f)</span></li>
<li>qt4/style/qtcurve_plugin.cpp <span style="color: grey">(f390da4)</span></li>
<li>qt5/CMakeLists.txt <span style="color: grey">(1d0359e)</span></li>
<li>qt5/style/CMakeLists.txt <span style="color: grey">(b6cb222)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120437/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/02545bef-04d2-4a45-8955-e13bf7d063a0__Screen_shot_2014-09-30_at_14.05.54.png">sample showing a native file dialog for comparison</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/fe5564f2-5e4e-485a-96d7-d192ff104261__QtCurve-1818.png">KDE4 systemsettings and the QtCurve configuration dialog</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/6bd8dd5a-9b94-43c3-88d9-c9be32a56b72__QtCurve-1814.png">KDE4 systemsettings and QtCurve config dialog with QtCuve 1.8.14, for comparison</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/09/30/39207a85-4092-4ea1-a181-051d0db5bb96__.reviewboardrc">a little convenience file for uploading to RB from KDevelop</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>