Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Qt6. #16

Merged
merged 1 commit into from
Jul 7, 2023
Merged

Update to Qt6. #16

merged 1 commit into from
Jul 7, 2023

Conversation

DorianBDev
Copy link
Member

Description

Thanks to @agatti, a first step was done to update Degate to use Qt6.

Affected area(s)

  • Core
  • GUI
  • Tests

Changes type

  • Bug fix
  • Migration
  • New feature
  • Feature rework

Proposed changes

  • Qt5 -> Qt6

@DorianBDev
Copy link
Member Author

@agatti I took the liberty of creating this PR for you, I don't know if you want to continue the work, or if you know what remains to be done?

@agatti
Copy link
Contributor

agatti commented Aug 24, 2022

Well, there are a couple more things that need to be done but it should be pretty much enough for the time being. I've just checked things out and rebuilt it to make sure I didn't miss anything. Basically what's left is:

  • Figure out why Ninja builds fail with:
CMake Error:
  Running

   '/opt/homebrew/bin/ninja' '-C' '<$PATH>/Degate/build' '-t' 'recompact'

  failed with:

   ninja: error: build.ninja:1617: multiple rules generate languages/degate_en.ts

Makefile builds work. I have no idea if this fails on other platforms or how to fix this right now - at least there are fallbacks.

  • Update Catch2 submodule as on non-x86 Mac machines it fails with:
[ 92%] Building CXX object tests/CMakeFiles/DegateTests.dir/src/MainTests.cc.o
cd <$PATH>/Degate/build/tests && /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DBOOST_ATOMIC_NO_LIB -DBOOST_FILESYSTEM_NO_LIB -DBOOST_NO_CXX11_SCOPED_ENUMS -DBOOST_SYSTEM_NO_LIB -DBOOST_THREAD_NO_LIB -DQT_CONCURRENT_LIB -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_DEBUG -DQT_OPENGLWIDGETS_LIB -DQT_OPENGL_LIB -DQT_WIDGETS_LIB -DQT_XML_LIB -I<$PATH>/Degate/build/tests/DegateTests_autogen/include -I<$PATH>/Degate/src -I<$PATH>/Degate/tests/catch2 -I<$PATH>/Degate/tests/src -I<$PATH>/Degate/tests/../src -isystem /opt/homebrew/include -isystem /opt/homebrew/lib/QtWidgets.framework/Headers -iframework /opt/homebrew/lib -isystem /opt/homebrew/lib/QtCore.framework/Headers -isystem /opt/homebrew/share/qt/mkspecs/macx-clang -isystem /opt/homebrew/lib/QtGui.framework/Headers -isystem /opt/homebrew/lib/QtXml.framework/Headers -isystem /opt/homebrew/lib/QtOpenGL.framework/Headers -isystem /opt/homebrew/lib/QtOpenGLWidgets.framework/Headers -isystem /opt/homebrew/lib/QtConcurrent.framework/Headers -Wall -g -fno-inline -std=c++11 -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk -std=gnu++17 -MD -MT tests/CMakeFiles/DegateTests.dir/src/MainTests.cc.o -MF CMakeFiles/DegateTests.dir/src/MainTests.cc.o.d -o CMakeFiles/DegateTests.dir/src/MainTests.cc.o -c <$PATH>/Degate/tests/src/MainTests.cc
In file included from <$PATH>/Degate/tests/src/MainTests.cc:24:
<$PATH>/Degate/tests/catch2/catch.hpp:8237:13: error: unrecognized instruction mnemonic, did you mean: bit, cnt, hint, ins, not?
            CATCH_BREAK_INTO_DEBUGGER();
            ^
<$PATH>/Degate/tests/catch2/catch.hpp:7948:83: note: expanded from macro 'CATCH_BREAK_INTO_DEBUGGER'
        #define CATCH_BREAK_INTO_DEBUGGER() []{ if( Catch::isDebuggerActive() ) { CATCH_TRAP(); } }()
                                                                                  ^
<$PATH>/Degate/tests/catch2/catch.hpp:7913:34: note: expanded from macro 'CATCH_TRAP'
    #define CATCH_TRAP() __asm__("int $3\n" : : ) /* NOLINT */
                                 ^
<inline asm>:1:2: note: instantiated into assembly here
        int $3
        ^
1 error generated.
make[2]: *** [tests/CMakeFiles/DegateTests.dir/src/MainTests.cc.o] Error 1
make[1]: *** [tests/CMakeFiles/DegateTests.dir/all] Error 2
make: *** [all] Error 2/

I doubt an INT 3 x86 opcode would work on an Arm CPU... The main Degate binary still works though.

Now, I haven't checked things on Linux/BSD or Windows as I only have an M1 mac here. Can you please update the CI environment with the latest Qt6 and see if this works? I am willing to update the branch to get to a working set of builds on those platforms.

I don't remember if I also fixed Qt5 code that was deprecated in Qt6, but again, that's something that can be done at a later stage if I didn't get around to it. All I know is that macOS's clang does complain a fair bit on non-Qt code, so there's enough to be busy for a bit :)

@DorianBDev
Copy link
Member Author

Thanks for those precisions, it's already a great work! I'll try to update the CI, and if you are ok I'll ping you back when it's done 😉

@DorianBDev DorianBDev changed the base branch from develop to qt6 August 25, 2022 15:53
@DorianBDev
Copy link
Member Author

I updated the CI to use the Qt 6.2 version. It's on the qt6 branch (now the target of this PR), so I think you could update this PR to try those changes

@agatti
Copy link
Contributor

agatti commented Aug 25, 2022

Need a manual approval to start the first batch of CI tasks...

@agatti
Copy link
Contributor

agatti commented Aug 25, 2022

This is interesting.

I've updated Catch2 on my source tree and tests actually pass on my machine [1]. I'll spin up a linux virtual machine somewhere and see why tests fail. I also have a workaround for the translations build issue, although it's rather nasty (if win32 ... else ... endif).

Now, I'm using qt 6.3.1 - I'll see if I can downgrade to 6.2.4 on the mac to see if it fails on my machine.

[1]:

...

[$PATH/Degate/src/Core/Project/ProjectImporter.cc:102] Project loaded.
Node name                      : /
Bounding box                   : x = 0 .. 1000 / y = 0 .. 1000
Num elements in this node      : 1
Preferred max num of elements : 4

    + Element bounding box           : x = 500 .. 510 / y = 500 .. 510

	Node name                      : //NW
	Bounding box                   : x = 0 .. 500 / y = 0 .. 500
	Num elements in this node      : 3
	Preferred max num of elements : 4

	Node name                      : //NE
	Bounding box                   : x = 501 .. 1000 / y = 0 .. 500
	Num elements in this node      : 0
	Preferred max num of elements : 4

	Node name                      : //SW
	Bounding box                   : x = 0 .. 500 / y = 501 .. 1000
	Num elements in this node      : 0
	Preferred max num of elements : 4

	Node name                      : //SE
	Bounding box                   : x = 501 .. 1000 / y = 501 .. 1000
	Num elements in this node      : 4
	Preferred max num of elements : 4

[$PATH/Degate/src/Core/Image/ImageReader.h:160] Reading image with size: 112 x 46
[$PATH/Degate/src/Core/Image/ImageReader.h:172] Image read

x = 10 .. 90 / y = 10 ... 90
x = 10 .. 90 / y = 10 ... 90
===============================================================================
All tests passed (563 assertions in 41 test cases)

@agatti
Copy link
Contributor

agatti commented Aug 25, 2022

@DorianBDev the windows build fails due to a missing component of the installer framework ("/bin/windeployqt.exe: No such file or directory"). Can you please check whether that is present in the installation or if an additional PATH variable entry must be set to make windeployqt.exe available to the command line?

With this we are at least at build parity on all three platforms, now onto getting the image loading issue with Qt 6.2...

@agatti
Copy link
Contributor

agatti commented Aug 27, 2022

Ok, I created an aarch64 linux VM with Ubuntu 22.04 and Qt6 6.2.4. After changing the C++ standard from 11 to 17, it just built fine and ran tests too [1]. I'll try on x86_64 soon.

[1]

...

[$PATH/Degate/src/Core/Project/ProjectImporter.cc:86] Check if we have template images.
[$PATH/Degate/src/Core/Project/ProjectImporter.cc:89] Will grab template image for gate template ID: 1
[$PATH/Degate/src/Core/Project/ProjectImporter.cc:102] Project loaded.
Node name                      : /
Bounding box                   : x = 0 .. 1000 / y = 0 .. 1000
Num elements in this node      : 1
Preferred max num of elements : 4

    + Element bounding box           : x = 500 .. 510 / y = 500 .. 510

	Node name                      : //NW
	Bounding box                   : x = 0 .. 500 / y = 0 .. 500
	Num elements in this node      : 3
	Preferred max num of elements : 4

	Node name                      : //NE
	Bounding box                   : x = 501 .. 1000 / y = 0 .. 500
	Num elements in this node      : 0
	Preferred max num of elements : 4

	Node name                      : //SW
	Bounding box                   : x = 0 .. 500 / y = 501 .. 1000
	Num elements in this node      : 0
	Preferred max num of elements : 4

	Node name                      : //SE
	Bounding box                   : x = 501 .. 1000 / y = 501 .. 1000
	Num elements in this node      : 4
	Preferred max num of elements : 4

[$PATH/Degate/src/Core/Image/ImageReader.h:160] Reading image with size: 112 x 46
[$PATH/Degate/src/Core/Image/ImageReader.h:172] Image read

x = 10 .. 90 / y = 10 ... 90
x = 10 .. 90 / y = 10 ... 90
===============================================================================
All tests passed (563 assertions in 41 test cases)

[$PATH/Degate/src/GUI/Preferences/PreferencesHandler.cc:290] Can't open recent projects file list.
agatti@lima-qt6:$PATH/Degate/build/tests/out/bin$ uname -a
Linux lima-qt6 5.15.0-46-generic #49-Ubuntu SMP Thu Aug 4 18:08:11 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux

I'm tempted to say that it's a matter on how and where the binaries are run. Both on the mac and on the arm linux vm I manually load DegateTests from the build/tests/out/bin directory - which seems like it's very same way CI does. I'm a bit lost now...

@DorianBDev
Copy link
Member Author

Hi! Thanks for these updates, next week will be a busy one for me, but I'll try to take a look at the CI as soon as possible. I'll also try to build on amd64 Linux and perhaps Windows

@DorianBDev
Copy link
Member Author

Ok I know why it fails on Linux (and not on all install): since Degate use many image format (and uses Qt underlying image loader), we need the special package called "qt6-image-formats-plugins" (or "qt6-imageformats" on Arch). Before installing the package, tests were failing, and after installing this specific package, all went fine. A simple update of the CI should fix the issue 😉

@DorianBDev
Copy link
Member Author

We can know it's the source of the problem when we get any "Degate/src/Core/Image/ImageReader.h:125] can't read size of **" error message

@agatti
Copy link
Contributor

agatti commented Aug 28, 2022

Excellent! So, just to avoid cluttering the changeset, I'll later open a few separate PRs with changes here that have nothing to do with Qt6 (Catch2 update, Arm changes). So, once they're in I'll rebase the whole lot on the new HEAD and squash the set into something cleaner.

@agatti
Copy link
Contributor

agatti commented Aug 30, 2022

Rebasing took care of Catch2 updates, so if the whole lot is ok with you I'll squash and force push for you to merge.

@XVilka
Copy link
Contributor

XVilka commented Jul 1, 2023

Had been there any progress with that?

@DorianBDev
Copy link
Member Author

Had been there any progress with that?

Unfortunately not, I just ran a new CI pass to see what remains to do. By rereading the PR and the commits, I don't think there is so much left to consider merging & releasing a Qt6 Degate's beta version.

Linux CI
Mac CI
Windows CI

@DorianBDev
Copy link
Member Author

The CI needs an update, it seems to be broken...

@agatti
Copy link
Contributor

agatti commented Jul 3, 2023

If this gets merged I can start sending PRs for removing Boost :) Qt provides pretty much everything Boost brings to the table and more, after all.

Speaking of Qt, there are also a few more UI/Geometry classes that can be removed in favour of using Qt's equivalents (Line, Point, Circle, file operation helpers, etc.)...

@DorianBDev
Copy link
Member Author

Yes I totally agree, Boost removal and core cleanup are in the roadmap since I moved from GTK to Qt. I'll fix the CI during the week and maybe finish the PR (quite a busy week here)

@DorianBDev DorianBDev merged commit 56c3e2f into DegateCommunity:qt6 Jul 7, 2023
@DorianBDev DorianBDev mentioned this pull request Jul 7, 2023
7 tasks
DorianBDev added a commit that referenced this pull request Aug 12, 2023
* Updated CI to Qt6.

* Fixed the CI for Qt6.

* Started switching to Qt6. (#16)

Co-authored-by: Alessandro Gatti <a.gatti@frob.it>

* Updated the CI for Qt6.

* Updated the Linux CI for Qt6.

* Updated the Windows & MacOS CI for Qt6.

* Possibly fixed a new bug introduced by Qt6 on OpenGL objects cleanup.

* Fixed more bugs with OpenGL (due to Qt6 port).

* Updated deploy CI.

* Updated deploy CI for mac and linux.

* Tried to fix linux deploy CI job.

---------

Co-authored-by: Alessandro Gatti <a.gatti@frob.it>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants