Re: Review Request 121390: make Qt5 theme build on Linux again
On 十二月 8, 2014, 9:07 a.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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
On 十二月 8, 2014, 9:07 a.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
Re: Review Request 121390: make Qt5 theme build on Linux again
On 十二月 8, 2014, 9:07 a.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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121390/ --- (Updated March 1, 2015, 10:02 p.m.) Status -- This change has been discarded. 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
Re: Review Request 121390: make Qt5 theme build on Linux again
--- 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. Summary (updated) - make Qt5 theme build on Linux again 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
Re: Review Request 121390: make Qt5 theme build on Linux again
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121390/#review71553 --- 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). - Martin Gräßlin 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
Re: Review Request 121390: make Qt5 theme build on Linux again
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). 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.) - 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
Re: Review Request 121390: make Qt5 theme build on Linux again
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.) 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. - Martin --- 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
Re: Review Request 121390: make Qt5 theme build on Linux again
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. ``` 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. - 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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
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. 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. - Martin --- 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:
Re: Review Request 121390: make Qt5 theme build on Linux again
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. @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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
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
Re: Review Request 121390: make Qt5 theme build on Linux again
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121390/ --- (Updated Dec. 8, 2014, 10:59 p.m.) Review request for KDE Frameworks, Qt KDE and Yichao Yu. Changes --- The compile-time check now uses QtCurve's own `QTC_ENABLE_X11` token that is defined when building for X11. My primary concern with this ticket is to restore something I broke; if Yichao would like me to add runtime-checks he is free to ask :) 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 (updated) - 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
