[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-mac
Subject: Re: [KDE/Mac] Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)
From: "Luigi Toscano" <luigi.toscano () tiscali ! it>
Date: 2015-12-02 9:50:41
Message-ID: 20151202095041.29696.80794 () mimi ! kde ! org
[Download RAW message or body]
--===============1068891067365032453==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Dic. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > <https://git.reviewboard.kde.org/r/126198/diff/3/?file=420466#file420466line39>
> >
> > This makes me very nervous.
> >
> > Using private APIs is almost always a guarantee the application won't preserve \
> > binary compatibility even across point releases (eg Qt 5.5.0 - 5.5.1), let alone \
> > across major releases.
> > What makes it even more dangerous is that this file won't be built only on \
> > MacPorts/Fink but also by people making normal AppBundle releases - where you \
> > have no control over what versions of dependencies are being used.
> > Please try to find a public API alternative, even if it ends up being a giant \
> > hack. I once found a very elegant private API solution to making a QQuickWidget \
> > emit a release event on the mouse cursor, but just because of the whole binary \
> > compat problem (some Linux distros don't even ship apps depending on private \
> > headers) I had to create a dummy item inside the QtQuick code and interact with \
> > it to get the cursor released. Giant hack, but at least no private API usage.
> > Private API is **private** for a reason.
Afaik frameworkintegration is supposed to be an extension of the internal Qt world \
and it has already been the case (I asked the same question few Akademys ago). If you \
want to use QPA, you need to go down in the stack.
- Luigi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
-----------------------------------------------------------
On Dic. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> -----------------------------------------------------------
>
> (Updated Dic. 1, 2015, 9:29 p.m.)
>
>
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie Zimmerman.
>
>
> Repository: frameworkintegration
>
>
> Description
> -------
>
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; all \
> that is required is to include the `qgenericunixservices` and `qgenericunixthemes` \
> components in the build, and to append `"kde"` to the list returned by \
> `QCocoaIntegration::themeNames()` for instance under the condition that \
> `KDE_SESSION_VERSION` is set to a suitable value in the environment.
> This will allow KF5 and Qt5 applications to use the theme selected through KDE if \
> FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which seems like \
> a sufficiently specific set of conditions that is easy to avoid by users who prefer \
> to use the Mac native theme.
> While requestion the KDE theme is also possible through `-style kde` or `env \
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the \
> only way to get the full theme, including the font and colour selection. In my \
> opinion it is above all the font customisation which is a very welcome feature for \
> Qt/Mac; by default Qt applications use the default system font (Lucida Grande 13pt \
> or even 14pt) throughout. This is a good UI font, but not at that size (and most \
> "native" OS X applications indeed use a range of smaller sizes, depending on role.
> It does have introduce a number of regressions, which the current patch aims to \
> address. The most visible and problematic of these regressions is the loss of the \
> Mac-style menu bar and thus of all menu items (actions).
> The fix is straightforward : on OS X (and similarly affected platforms?), an \
> instance of the native Cocoa platform theme is created through the private API, and \
> used as a fallback rather than immediately falling back to the default \
> implementations from `QPlatformTheme`. In addition, methods missing from (not \
> overridden by) `KdePlatformTheme` are provided on OS X and call the corresponding \
> methods from the native theme. It is this change which restores the menubar and \
> even the Dock menu functionality. One minor regression remains but should be easy \
> to fix (elsewhere?): the Preferences menu loses its keyboard shortcut (Command-,).
> Given the fallback nature of the native platform instance I have preferred to print \
> a warning rather than using something like `qFatal`, above all because the message \
> printed by qFatal tends to get lost on OS X. I can replace my use of `qWarning` \
> with a dialog giving the choice between continuing or exiting the application - \
> code that would be called in the menu methods because only there is it certain that \
> the application actually needs a menubar.
> In line with experience and feedback from the KDE(4)-Mac community I have decided \
> to force the use of native dialogs rather than the ones from the KdePlatformPlugin. \
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in line \
> with platform guidelines, and ensure that the autotests are not built as app \
> bundles.
>
> Diffs
> -----
>
> autotests/CMakeLists.txt 7c2129c
> src/kstyle/CMakeLists.txt bc26667
> src/kstyle/kstyle.mm PRE-CREATION
> src/platformtheme/CMakeLists.txt 23f590e
> src/platformtheme/kdemactheme.h PRE-CREATION
> src/platformtheme/kdemactheme.mm PRE-CREATION
> src/platformtheme/khintssettingsmac.h PRE-CREATION
> src/platformtheme/khintssettingsmac.mm PRE-CREATION
> src/platformtheme/main_mac.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/126198/diff/
>
>
> Testing
> -------
>
> On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.
>
> I have not verified to what extent my use of a private `QGuiApplication` API links \
> builds to a specific Qt version (I consider that nothing shocking and a minor price \
> to pay).
> > > > Do I need to add some glue to the CMake file so that it will warn if the \
> > > > private headerfiles are not available? Apparently no changes were required to \
> > > > find them.
>
>
> File Attachments
> ----------------
>
> purely native OS X theme
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/650d0da7-52b3-40d1-a1f9-cb610494cf77__Screen_Shot_2015-11-30_at_15.42.31.png
> native theme but with `-style kde`
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/72e2a6aa-2a7c-465b-b404-fc1e52b6fc69__Screen_Shot_2015-11-30_at_15.43.02.png
> using the KDEPlatformTheme
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/309e5995-74fa-42fb-a6f3-936cedbf5246__Screen_Shot_2015-11-30_at_15.43.31.png
> on Linux, using a purely "native" theme
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/eaa1d907-bf05-4ca2-821b-83dc062aea04__QtCreatorNativeLNX.png
> KDEPlatformTheme with the "macintosh" native theme selected
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/01/de55a91f-3500-4db8-8a3b-d252fd7ea169__Screen_Shot_2015-12-01_at_13.52.35.png
>
>
> Thanks,
>
> René J.V. Bertin
>
>
--===============1068891067365032453==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<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/126198/">https://git.reviewboard.kde.org/r/126198/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On dicembre 2nd, 2015, 10:45 a.m. CET, \
<b>Boudhayan Gupta</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/126198/diff/3/?file=420466#file420466line39" \
style="color: black; font-weight: bold; text-decoration: \
underline;">src/platformtheme/kdemactheme.mm</a> <span style="font-weight: normal;">
(Diff revision 3)
</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">39</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="cp">#include <QtGui/private/qguiapplication_p.h></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;">This \
makes me very nervous.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Using private APIs is almost always a \
guarantee the application won't preserve binary compatibility even across point \
releases (eg Qt 5.5.0 - 5.5.1), let alone across major releases.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">What makes it even more dangerous is that this file won't be built only on \
MacPorts/Fink but also by people making normal AppBundle releases - where you have no \
control over what versions of dependencies are being used.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Please try to find a public API alternative, even if it ends up being a \
giant hack. I once found a very elegant private API solution to making a QQuickWidget \
emit a release event on the mouse cursor, but just because of the whole binary compat \
problem (some Linux distros don't even ship apps depending on private headers) I had \
to create a dummy item inside the QtQuick code and interact with it to get the cursor \
released. Giant hack, but at least no private API usage.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Private API is <strong style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;">private</strong> for a reason.</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;">Afaik frameworkintegration is supposed to be an extension of the internal \
Qt world and it has already been the case (I asked the same question few Akademys \
ago). If you want to use QPA, you need to go down in the stack.</p></pre> <br />
<p>- Luigi</p>
<br />
<p>On dicembre 1st, 2015, 9:29 p.m. CET, 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, KDE Frameworks and Valorie \
Zimmerman.</div> <div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Dic. 1, 2015, 9:29 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
frameworkintegration
</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;">The KdePlatformTheme plugin can be enabled on OS X \
with a minimal patch to Qt5; all that is required is to include the <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">qgenericunixservices</code> and <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">qgenericunixthemes</code> components in the build, and to append <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">"kde"</code> to the list returned by <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">QCocoaIntegration:: themeNames()</code> for instance under \
the condition that <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">KDE_SESSION_VERSION</code> is \
set to a suitable value in the environment.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">This will allow KF5 and \
Qt5 applications to use the theme selected through KDE if FrameworkIntegration is \
installed and KDE_SESSION_VERSION is set, which seems like a sufficiently specific \
set of conditions that is easy to avoid by users who prefer to use the Mac native \
theme.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">While requestion the KDE theme is also possible \
through <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: \
normal;margin: 0;line-height: inherit;">-style kde</code> or <code \
style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: \
0;line-height: inherit;">env QT_STYLE_OVERRIDE=kde</code> the use of the \
KdePlatformTheme plugin appears to be the only way to get the full theme, including \
the font and colour selection. In my opinion it is above all the font customisation \
which is a very welcome feature for Qt/Mac; by default Qt applications use the \
default system font (Lucida Grande 13pt or even 14pt) throughout. This is a good UI \
font, but not at that size (and most "native" OS X applications indeed use a range of \
smaller sizes, depending on role.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">It does have introduce \
a number of regressions, which the current patch aims to address. The most visible \
and problematic of these regressions is the loss of the Mac-style menu bar and thus \
of all menu items (actions).</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">The fix is straightforward : on OS X \
(and similarly affected platforms?), an instance of the native Cocoa platform theme \
is created through the private API, and used as a fallback rather than immediately \
falling back to the default implementations from <code style="text-rendering: \
inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">QPlatformTheme</code>. In addition, methods missing from (not overridden \
by) <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: \
normal;margin: 0;line-height: inherit;">KdePlatformTheme</code> are provided on OS X \
and call the corresponding methods from the native theme. It is this change which \
restores the menubar and even the Dock menu functionality. One minor regression \
remains but should be easy to fix (elsewhere?): the Preferences menu loses its \
keyboard shortcut (Command-,).</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Given the fallback \
nature of the native platform instance I have preferred to print a warning rather \
than using something like <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">qFatal</code>, above all because the message printed by qFatal tends to get \
lost on OS X. I can replace my use of <code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: \
inherit;">qWarning</code> with a dialog giving the choice between continuing or \
exiting the application - code that would be called in the menu methods because only \
there is it certain that the application actually needs a menubar.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">In line with experience and feedback from the KDE(4)-Mac community I have \
decided to force the use of native dialogs rather than the ones from the \
KdePlatformPlugin.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">In addition I set the fallback value \
for <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: \
normal;margin: 0;line-height: inherit;">ShowIconsOnPushButtons</code> to false in \
line with platform guidelines, and ensure that the autotests are not built as app \
bundles.</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 Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and \
QtCurve git/head.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">I have not verified to what extent my \
use of a private <code style="text-rendering: inherit;color: #4444cc;padding: \
0;white-space: normal;margin: 0;line-height: inherit;">QGuiApplication</code> API \
links builds to a specific Qt version (I consider that nothing shocking and a minor \
price to pay).</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;"> <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;"> \
<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;">Do I need to add some glue to the CMake file so that it will warn if the \
private headerfiles are not available? Apparently no changes were required to find \
them.</p> </blockquote>
</blockquote>
</blockquote></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">(7c2129c)</span></li>
<li>src/kstyle/CMakeLists.txt <span style="color: grey">(bc26667)</span></li>
<li>src/kstyle/kstyle.mm <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/platformtheme/CMakeLists.txt <span style="color: grey">(23f590e)</span></li>
<li>src/platformtheme/kdemactheme.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/platformtheme/kdemactheme.mm <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/platformtheme/khintssettingsmac.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/platformtheme/khintssettingsmac.mm <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/platformtheme/main_mac.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126198/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/2015/11/30/650d0da7-52b3-40d1-a1f9-cb610494cf77__Screen_Shot_2015-11-30_at_15.42.31.png">purely \
native OS X theme</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/72e2a6aa-2a7c-465b-b404-fc1e52b6fc69__Screen_Shot_2015-11-30_at_15.43.02.png">native \
theme but with `-style kde`</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/309e5995-74fa-42fb-a6f3-936cedbf5246__Screen_Shot_2015-11-30_at_15.43.31.png">using \
the KDEPlatformTheme</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/eaa1d907-bf05-4ca2-821b-83dc062aea04__QtCreatorNativeLNX.png">on \
Linux, using a purely "native" theme</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/12/01/de55a91f \
-3500-4db8-8a3b-d252fd7ea169__Screen_Shot_2015-12-01_at_13.52.35.png">KDEPlatformTheme \
with the "macintosh" native theme selected</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============1068891067365032453==--
[Attachment #3 (text/plain)]
_______________________________________________
kde-mac@kde.org
List Information: https://mail.kde.org/mailman/listinfo/kde-mac
KDE/Mac Information: http://community.kde.org/Mac
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic