> On Dec. 8, 2014, 3:07 p.m., 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 ;)
(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 ... - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121390/#review71553 ----------------------------------------------------------- On Dec. 8, 2014, 2:38 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121390/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2014, 2:38 p.m.) > > > 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 > >
