Fixes to get Qt5 working on Windows. #872
Conversation
| bool | ||
| DocumentWindow::winEvent(MSG *message, | ||
| long *result) | ||
| DocumentWindow::nativeEvent(const QByteArray& eventType, void* message, long* result) | ||
| { |
There was a problem hiding this comment.
avoid double curly braces, which may confuse indenteds. I would prefer having two separate function definitions (will make the change)
There was a problem hiding this comment.
What would you think about something like this instead
#if QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)
bool
DocumentWindow::nativeEvent(const QByteArray& eventType, void* message, long* result)
{
auto t = handleWindowEvent(static_cast<MSG*>(message), result);
return t.first ? t.second : QMainWindow::nativeEvent(eventType, message, result);
}
#else
bool
DocumentWindow::winEvent(MSG* msg, long *result)
{
auto t = handleWindowEvent(msg, result);
return t.first ? t.second : QMainWindow::winEvent(msg, result);
}
#endif
std::pair<bool, bool>
DocumentWindow::handleWindowEvent(MSG *msg, long *result)
{
switch (msg->message) {
case WM_DDE_INITIATE:
return std::make_pair(true, ddeInitiate(msg, result));
break;
case WM_DDE_EXECUTE:
return std::make_pair(true, ddeExecute(msg, result));
break;
case WM_DDE_TERMINATE:
return std::make_pair(true, ddeTerminate(msg, result));
break;
}
return std::make_pair(false, false);
}
My thinking here is that it keeps the main logic that is common in a single place and keeps the Qt version differences minimal and in their appropriate #if sections. I don't have a strong feeling here, just trying to address your concern and avoid duplicate code. It's too bad I can't use std::optional. It would be a little cleaner looking.
| pyside: INCLUDEPATH += $$PYTHON_SITE_PACKAGES/PySide2/include/PySide2/QtCore | ||
| pyside: INCLUDEPATH += $$PYTHON_SITE_PACKAGES/PySide2/include/PySide2/QtGui | ||
| pyside: INCLUDEPATH += $$PYTHON_SITE_PACKAGES/PySide2/include/PySide2/QtWidgets | ||
| shiboken: INCLUDEPATH += $$system(pkg-config --variable=includedir shiboken2) |
There was a problem hiding this comment.
that makes pkg-config a build requirement. Will that still be possible if we switch to - say - vcpkg?
There was a problem hiding this comment.
There already is a dependency on pkg-config. I'm basically doing the same thing that the Qt4 block is doing below.
I'm not really familiar with vcpkg but I suspect it would be trivial to get similar info from whatever it uses to describe where the package include directory is. FWIW, as a side project, I've been trying to get various Natron components to build with the Conan package manager (https://conan.io/). I have prototypes of openfx-misc and openfx-io building on Windows, Linux, & Mac all from the same source. Part of why I was trying to get Qt5 working is because I didn't want to port Qt4 to conan and I knew you folks were trying to migrate to Qt5 anyways. For conan, at least, it does support pkg-config and will allow you to actually build it from source if needed. Conan will automatically generate the .pc files for all your dependencies if needed. This is usually only required for automake/make builds though because it uses more direct means with cmake.
|
@acolwell @rodlie are you both able to make a Qt5 win build? Can we attach one to https://github.com/NatronGitHub/Natron/releases/tag/v2.6.0-alpha1 for testing? |
Currently I only have a server that can produce RB-2.4 and RB-2.5 win builds for Qt4. I can probably setup a new MSYS env and start testing, but I don't know how much free time I have this week. |
I'm happy to setup a fresh MSYS env to do an official RC build. I was doing fresh installs quite often when I was trying to get my build environment setup. I haven't gotten breakpad working yet in my dev builds so I guess I'll have to figure that out. I was wondering if you all are interested in some changes that I have which rename some of the MINGW packages with a natron_ prefix so that they are easier to differentiate from the official packages. For me at least, it makes it way easier to keep track of what Natron specific stuff I had to install in my MSYS environment. |
That would be great :)
For some reason it must be built on Windows 8 or lower, if I build breakpad (included with Natron) on Windows 10(+) all crashes are ignored. Same with building it in docker for Linux, will not work. If I build for Linux in an VM or on actual hardware it works. I modified Natron to produce local crash reports some time ago ...
but never finished it due to this issue.
I have no problem with that. |
Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:
PR Description
What type of PR is this? (Check one of the boxes below)
What does this pull request do?
This change fixes various Qt5 related build busters on Windows.
Have you tested your changes (if applicable)? If so, how?
Yes. I built Qt4 and Qt5 installers on Windows in a MSYS2 environment with tools/jenkins/launchBuildMain.sh. I was able to unzip the generated zip files and launch Natron. I verified I was able to load a simple project in each of these builds, but I did not do exhaustive testing to verify all features work with Qt5.