> On Dez. 8, 2014, 2:07 nachm., Martin Gräßlin wrote:
> > this is wrong - please have a look at various frameworks on how to do it
> > properly. In the end it should be:
> > #if HAVE_X11
> > // x11 specific stuff
> > #endif
> >
> > obviously it also needs a runtime check:
> > if (QX11Info::isPlatformX11())
> >
> > as we no longer should assume that X11 is the only platform on Unix(non
> > OSX).
>
> René J.V. Bertin wrote:
> I found a reference to HAVE_X11 online, but that token is not defined.
> Note also that the Qt5 theme is supposed to build without KF5 too, for pure
> Qt5 applications, so this kind of token should rather be provided by the Qt
> cmake files rather than KDE's.
>
> I'll leave the runtime checks to the QtCurve devs, as they obviously
> should be made in lots of locations and it's their call. I myself don't see
> the point in doing a runtime check whether a platform "is" X11, though. It's
> known at build time if a platform "is" X11. Doing a runtime check before each
> theming action if `Q11Info::isX11Active()` (or some such call) seems to be an
> expensive concession to a rather improbable set-up. If distros really decide
> to give the user a choice between X11 and Wayland at login ... let them
> figure out how to streamline that first, and then add runtime checks for the
> active graphics backend where really needed...
> (In fact, I myself would avoid anything tied to the display layer as much
> as possible; the fact I had to go back in a few months after the previous
> porting effort goes to show how easy it is to break builds on other platforms
> with that kind of functionality - as if my own error wasn't enough already.)
>
> Martin Gräßlin wrote:
> HAVE_X11 is neither defined by Qt5 nor by KF5. It needs to be set
> manually depending on whether the source is built with or without X11 support.
>
> Concerning the runtime check:
> kwrite -platform wayland
>
> and your app is going to crash like hell if there is no runtime checks.
>
> René J.V. Bertin wrote:
> ```> neon5-exec /opt/project-neon5/bin/kwrite -platform wayland
> This application failed to start because it could not find or load the Qt
> platform plugin "wayland".
>
> Available platform plugins are: linuxfb, minimal, offscreen, xcb.
>
> Reinstalling the application may fix this problem.
> Abort (core dumped)
> ```
>
> Right, so with runtime checks it doesn't crash, it just self-destructs.
> Very useful difference :)
> Of course an application will crash if it tries to use an unavailable
> displaying method, or if the linker tries to load shared libraries that
> aren't present. In fact, with X11 it might actually exit nicely with a
> message about a display rather than crash.
>
> This just underlines my point: the only use for runtime checks in this
> context if is the same binaries are supposed to work with multiple displaying
> backends (or platform plugins). Whether QtCurve ought to support that is a
> call for its developers to make, like I said above. The only way to do that
> properly without (too much) overhead is to do the check at initialisation
> time rather than preceding each backend-specific call, i.e. use
> functionpointers or some OO/C++ alternative. I don't know how preferable
> Wayland is to X11; without that I see only an interest for people working on
> Wayland development under X11 for this kind of runtime switch support.
> To put this into context: I've often thought how it'd be nice if Qt-mac
> would be able to use X11, but I've always dismissed the possibility that that
> might be a runtime switch, exactly because it would introduce too much
> overhead and/or complexity for a feature that'd be used only rarely.
>
> Thomas Lübking wrote:
> > Right, so with runtime checks it doesn't crash, it just self-destructs.
>
> You're missing the point entirely. The compile time checks have no
> implication on the runtime environment.
> Of course you cannot use the wayland platform plugin if it's not
> available, but you *can* compile Qt/KDE w/ X11 and wayland present - but
> making X11 calls when running on the wayland PP will crash the application ->
> thus you must check whether you're operating on X11/xdg at runtime.
>
> Also testing for "UNIX but not OSX" to make X11 calls is plain wrong.
> Could be framebuffer console or wayland and no X11 installed at all.
>
> Martin Gräßlin wrote:
> for more information please see my blog post:
> http://blog.martin-graesslin.com/blog/2014/02/running-frameworks-powered-applications-on-wayland/
>
> Btw. the QtWayland PPA will be available starting with Qt 5.4 - a version
> I'm already using.
>
> René J.V. Bertin wrote:
> @Thomas: we're not talking about compile time checks. Those evidently
> don't have any implication on the runtime environment (if done correctly :)).
> I guess my point is simply that the fact that you can (= it's possible
> to) compile Qt/KDE with every conceivable display/rendering engine present
> doesn't mean that indidual KDE applications or plugins can no longer decide
> to support only a subset to be set at build time. *)
>
> No issue either with "Unix but not OS X" - that's what I came up with for
> lack of something better. Turns out Yichao has his own alternative to
> HAVE_X11, I'll see if I can make do with that.
>
> *) or else I'll start making a ruckus to have kwin and more Plasma
> goodies on OS X!! ;)
>
> Martin Gräßlin wrote:
> Yes, it's not about compile time checks, it's about introducing runtime
> checks as Thomas and I wrote ;-)
>
> René J.V. Bertin wrote:
> Actually, Thomas wrote "The compile time checks have no implication on
> the runtime". Surely a typo, but those can have devastating consequences
> around code ;)
>
> René J.V. Bertin wrote:
> (published too fast again)
>
> Actually, that blog post of yours also starts out by talking exclusively
> about compile-time checks for about 2/3 of its length. It's only after the
> screenshot that it becomes clear you actually use the compile-time check to
> include a runtime-check or not. A casual reader might be tempted to stop
> reading early, thinking he got the message ...
>
> And I can't stop thinking something that has been stamped into me: "ifs
> are expensive". Guess that shows my age ...
That's not a typo. Meaning distorting partial quote.
I wrote:
"The compile time checks have no implication on the runtime *environment*."
"Ifs are expensive" might be stamped into your mind and/or true, but they're
completely inavoidable in this context.
Just that X11 was available at runtime does NOT ("no more w/ Qt5") mean that
it's available at runtime.
=> Keep the branching out of hot loops (as always) ;-)
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121390/#review71553
-----------------------------------------------------------
On Dez. 8, 2014, 1:38 nachm., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121390/
> -----------------------------------------------------------
>
> (Updated Dez. 8, 2014, 1:38 nachm.)
>
>
> Review request for KDE Frameworks, Qt KDE and Yichao Yu.
>
>
> Repository: qtcurve
>
>
> Description
> -------
>
> Yesterday's patches for OS X building broke the build of the Qt5 parts on
> Linux (and other Unix/X11 platforms). I had presumed that Q_WS_X11 would be
> defined in that context as it is when building Qt4 code, but apparently it
> isn't.
>
> This patch restores building on Unix/X11 by replacing
>
> `#ifdef Q_WS_X11`
>
> with
>
> `#if defined(Q_OS_UNIX) && !defined(Q_OS_OSX)`
>
> please verify if that catches all possible contexts where X11 is to be used?!
> (Qt/Cygwin might use X11?)
>
>
> Diffs
> -----
>
> qt5/style/blurhelper.cpp 5dcc95c
> qt5/style/qtcurve.cpp 7b0d7e6
> qt5/style/qtcurve_plugin.cpp febc977
> qt5/style/qtcurve_utils.cpp 728c26a
> qt5/style/shadowhelper.cpp a239cf1
> qt5/style/utils.cpp 0680442
> qt5/style/windowmanager.cpp 3c2bc1c
>
> Diff: https://git.reviewboard.kde.org/r/121390/diff/
>
>
> Testing
> -------
>
> On KUbuntu 14.04 with Qt 5.3.2 and KF5 in the (sadly discontinued) Project
> Neon5 environment.
>
>
> Thanks,
>
> René J.V. Bertin
>
>
