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