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

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

Reply via email to