Agreed – my thought process on keeping it this way was a) to make extra sure I wasn’t breaking anyone’s Qt5 compilation as we start into this process, and b) to ease the transition for everyone running cMake from the command line (e.g. they don’t have to change anything in their cMake commands except QT5->QT6). I’m not sure either of those is a terribly strong reason, and the switch to an enumeration-style version selection, or even just purely numerical, is not a big undertaking. I’m in favor if wmayer thinks it’s reasonable.
Regarding some of the other “Qt5” things – Qt is helping us out here in Qt6, renaming things like “QT5_WRAP_UI” to “QT_WRAP_UI” as of Qt 5.15. For older builds of Qt5 we can probably define that ourselves, which will eliminate most of the BUILD_QT5 flag checks in the Mod subdirectories. Same goes for QT5_ADD_RESOURCES, and QT5_WRAP_CPP.
I don’t think it was ever planned to have two or three different variables but one suffices because it will never happen that the FreeCAD code will support more than two major versions of Qt.
Sure, if it helps to simplify things it’s fine to go this way.
VTK uses an IMO good process to handle both Qt5 and Qt6. In https://gitlab.kitware.com/vtk/vtk/-/blob/v9.2.2/CMake/vtkQt.cmake they define a VTK_QT_VERSION variable, which will subsequently be used throughout the cmake files in the form of Qt${VTK_QT_VERSION}_Found on all Qt variables which use a version. Maybe this is a way we could use as well to simplify the cmake code for using different versions of Qt?
That VTK setup is nice – with the minor change of defaulting to Qt5 (for now) I think we can probably use it almost exactly. I’ll have to look at how they do their variable substitution, the way I’m doing it right now is following what appears to be Qt’s overall strategy of dropping the version number from their macro names. So in the current PR I’ve got things like:
and the matching code for Qt6. Then all other code outside of the SetupQt.cmake file can just use the non-versioned variable, and not need any kind of switch on the Qt version (except in cases like Spacemouse or TechDraw, where there is some kind of Qt5-only dependency).
Update: I’ve updated PR 7647 to adopt VTK’s cMake Qt-selection strategy, to remove BUILD_QT5 and add FREECAD_QT_VERSION instead, and to consolidate almost all Qt5/Qt6 switches. I’ve also switched that PR to Draft since it now breaks compilation on Qt5 due to some missing header search location – obviously I have to get that fixed before merging! I think there’s probably still some refactoring and cleanup that could be done, but it’s getting pretty close to being usable enough to start working from.
I changed:
sansFont.setWeight(hGrp->GetInt("FontWeight", 87));
to:
sansFont.setWeight(QFont::Weight(hGrp->GetInt("FontWeight", 87)));
a) It feels wrong to use arbitary values instead of enum value names.
b) Reasonably, Qt had a reason for the change. Which would suggest FC does the same? Havn’t looked to see how far such a change would extend.
I once needed the Qt Webkit module because the nouveau driver has problems with Qt’s Webengine module. Luckily, the user vanuan gave me a hint to use the option of Software OpenGL and that indeed fixed the issue. It’s now quite a while that I use the Webengine module without bigger problems.
So, long story short: we can drop support of the Webkit module. Btw, does it even exist for Qt6?
That’s excellent – it doesn’t exist for Qt6, and removing it from the tests in SetupQt.cmake has that file under 50 lines now, which I feel pretty good about. Of course, now I have to change our CI setup! It turns out it was using WebKit when it compiled, so this PR is failing CI. I should be able to get to that later today.
“No matching member function for call to ‘connect’”.
AFAICT it is only where the types passed in functions to SLOT or SIGNAL are pointers.
Only found one maybe useful reference at qt centre
Is there a simpler solution?
I’ve merged PR 7647 (hopefully I haven’t broken anyone’s Qt5 build in the process…), so now we have a FREECAD_QT_VERSION variable in cMake that you can set to “6” to try to compile against Qt6.
A request for those working on this: it would be nice if we can keep the PRs small, so IMO once you’ve got a small problem resolved with getting Qt6 working, go ahead and submit a PR – this will get you a CI run to make sure you don’t break Qt5 builds, and we can merge the changes in bit by bit, rather than trying to do it all in one go. I think once we’ve done a few rounds and narrowed in on the appropriate solutions, we can go quite quickly.
I’d also suggest focusing on C++ first – we can’t even test the Python until the C++ compiles and runs.
I’ve merged the updated cmake stuff into my experimental build.
I have accumulated a bunch of changes to cmake files, which I am now reapplying to the new setup.
Most of it is cosmetic, bringing consistency, shortening lines so they don’t keep breaking, simple stuff like spaces on if’s etc, restating conditions etc in else and end.
There are many occurrences of “if(DEFINED ” that don’t consider might be “falsey” (empty, OFF, “”, etc). (probably been there for years)
In almost every case I think a simple “if(condition” is less error-prone.
I just realized an issue with removing QtWebkit. On MSYS2 a package Qt5WebEngineWidgets doesn’t exist because the code currently doesn’t compile and the only alternative is QtWebkit. For now I have to disable the Web module.
That’s unfortunate (especially considering Qt announced this deprecation in 2013!!). Do you think we should just put it back in our builds? It’s not that big of a deal while we still support Qt5. I have no idea what the state of Qt6 is on Mingw.
For me it’s not a big loss not to have the StartPage for a Mingw build. Sooner or later we anyway have to live without it once we drop Qt5 support – unless the problematic code will be ported to Mingw. From what I have seen it’s apparently the Chromium engine that currently doesn’t compile with Mingw.
So, I don’t mind if we leave it like this now. I only wanted to report the issue.
It is possible to replace the qtwebengine respectively chromium with newer versions. Here is an example:
1. Get QT
wget https://download.qt.io/official_releases/qt/5.15/5.15.6/single/qt-everywhere-opensource-src-5.15.6.tar.xz
tar -xf qt-everywhere-opensource-src-5.15.6.tar.xz
cd qt-everywhere-src-5.15.6/
2. Remove qtwebengine
rm -R qtwebengine
3. Get current version
git clone --recursive https://code.qt.io/qt/qtwebengine.git
cd qtwebengine
git checkout tags/v5.15.11-lts
4. Fix version conflict
cd ..
sed -i 's/5.15.11/5.15.6/g' qtwebengine/.qmake.conf
You can try the same with chromium:
5. Remove
rm -R qtwebengine/src/3rdparty/chromium
6. Get current Qt6.4 chromium version
Check the ChromiumVersions
cd qtwebengine/src/3rdparty
git clone --recursive https://github.com/chromium/chromium
cd chromium
…
but because Qt6Core_MOC_EXECUTABLE is not defined QtCore_MOC_EXECUTABLE will be empty and causes a build failure. I had to manually create the CMake variable Qt6Core_MOC_EXECUTABLE and set the path to moc
The compiler produces a couple of dozens of warnings. Nothing serious and everything is easy to fix.
TechDraw is the only module that is not yet ported. This will be the most difficult part.
When starting FreeCAD the first time it has crashed immediately after showing the splash screen. This is because some modules import PySide2 modules which pull in Qt5 binaries and thus causes the crash. So, we need a mechanism where PySide2 calls are redirected to PySide6. Therefore we can use the same trick as we did with PySide vs. PySide2
Closing FreeCAD causes another crash. That should be easy to fix.
