> On Dec. 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.
> 
> Luigi Toscano wrote:
>     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.
> 
> Martin Gräßlin wrote:
>     yeah frameworkintegration is bound to use private API. So not really a 
> problem in this special case.
>     
>     AFAIK some distros have adjusted the packaging to force a recompile for 
> new Qt releases.
> 
> Boudhayan Gupta wrote:
>     Ah, that makes sense. Okay, I'm dropping this issue.

Whew - Hacking is so much more fun if you can do it there where you're not 
supposed to go, rather than to avoid exactly that ;)
However, it might be possible to replace the direct call of the private 
function by a call through a function pointer to `createPlatformTheme` obtained 
through a dynamic resolver. Any idea if that is bound to fail through a Qt 
function because it'd detect a request for a private API?

In any case, you (Boudhayan) are right that this modified framework might end 
up being used also in *standalone* AppBundle builds rather than only in builds 
using shared resources (in which applications are also built as app bundles!). 
At least I hope it will, for the very reason I've been pushing the patch. But I 
think you're wrong to assume that using private APIs is a problem there. If 
anything it should be (much) less the case, because standalone app bundles form 
a much more protected environment. They're not meant to be upgraded internally, 
except maybe by some special mechanism controlled by the developer(s) in 
charge. Updates to Qt without an accompanying forced rebuild ("revbump" in 
MacPorts speak) to frameworkintegration are much more likely to occur in a 
shared environment.


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
-----------------------------------------------------------


On Dec. 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 Dec. 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
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to