-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add Qt6 Support #7339
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
Add Qt6 Support #7339
Conversation
|
I successfully built LMMS with Qt 6.8.0 on Linux without the When compiling, there were a number of deprecation warnings unrelated to that change that we should probably address in this PR. |
I'm wondering if we should put our efforts into fixing merge conflicts with the |
|
@tresf I'd be fine with that. Maybe Windows MSVC could be our Qt 6 build in the CI since it wouldn't be hard to update the dependency to Qt 6, and the MinGW build is still available in case any issues arise with it. We could do it in this PR after fixing the merge conflicts. |
Merge conflicts fixed. Some touched lines don't appear to be related to this PR I've started a conversation thread on these files, please mark as resolved if they're warranted. The history is a bit weird (as most rebases are) and I made plenty of mistakes merging, but the branch should be in pretty good shape right now. I think it's ready for a CI job to run against Qt6 as well as a few set of eyes particularly from those that have helped with the code leading up to this point. Please push patches directly to this branch. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
Co-authored-by: michaelgregorius <[email protected]> Co-authored-by: Rossmaxx <[email protected]>
- Fix linking issues with Qt Framework files - Fix qmake detection
Fix implicit conversion from int when using QString.arg(...)
* Adds win32EventFilter a wrapper for nativeEventFilter on Windows * win32EventFilter is currently used to intercept top-level Window events (currently, to avoid VSTs setting transparency of the parent application)
QComboBox activated() replaced with textActivated() since Qt 5.14
* enabled VST support for Qt 6 builds * Note : Embedding on QT6 will be buggy on linux as a result of using qt embedding, which unfortunately is a qt bug which hasn't been resolved.
* Added lines in between bars * Changed bar lines to follow snap size * Changed default zoom and quantization value * Added constants for line widths * Added QSS configuration for new grid line colors * Tied line widths to QSS properties * Changed default quantization to 1/4 * Removed clear() from destructor model * Removed destructor in ComboBoxModel.h * Changed member set/get functions to pass by value * Updated signal connection with newer syntax
* ensured mouse event != nullptr before deref * separation of concerns: AFP WaveView updateCursor extract check to pointerCloseToStartEndOrLoop() * marked some function parameters as const
The Qt bug that used to be present appears to have been fixed, so the workaround can be removed
messmerd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just making note of two places where I'm not sure why a change was made.
The rest of the changes made in this PR I have reviewed and understand.
|
This PR is functionally complete and ready for review and testing. Testing instructionsTest the GUI on the Windows MSVC build to see if there are any regressions in how things appear or in interactions with GUI elements. If you find a bug that is also present in Windows MSVC on nightly, that's okay since it wasn't caused by this PR. For now, only Windows MSVC is using Qt6 and rest of the builds remain on Qt5. The Qt5 builds should behave the same as before this PR unless an egregious mistake was made in the deprecation helpers or elsewhere, so a quick check of the Qt5 builds might be a good idea. |
|
I managed to build lmms from the latest commit of qt6 branch, 48422da, with a small modification to OpulenZ to fix compilation error, in Guix system. Projects are not opening. The project browser window opens, but after I select the project and click on open, nothing happens, just the project browser window closes as if I did not open any project. (Edit): (Edit 2): |
Mind sharing the patch? PR is welcome as well! |
|
Conflicts: - src/gui/clips/ClipView.cpp - src/gui/editors/Editor.cpp - src/gui/editors/PianoRoll.cpp
Thanks for catching this. I've fixed it in the latest commit (fbee2e7) Another issue I noticed is a couple failed Qt assertions about sending events to objects owned by a different thread. This occurs in debug builds when I release my mouse when playing a note on the Piano Roll's piano. I'll have to look into this some more. EDIT: It still works fine outside of debug builds. I'm guessing it's unrelated to this PR and also present in Qt 5 builds built with the debug version of the Qt libraries, though I'd have to confirm. |
|
Tangentially related, @messmerd the last remaining task (besides adding this to our CI) was MSVC stuff... is that still the case or are we ok to merge into mainline now? If so, we can track any newly discovered bugs separately. |
I still need to look into the changes mentioned here, and spend a little more time using it to make sure there are no other serious issues like the one SameExpert reported. But other than that, this PR is ready to merge. |
If you're comfortable, let's resolve the above convo and merge. This will provide Qt6 support to the masses right now and we can sort any minor issues as we encounter them. One thing that we may want to consider is adding at least one Qt6 CI task to catch compilation errors, but that can be a follow-up. |
This PR does that already - Windows MSVC is building with Qt6. |
Ooooh, it ditches Qt5. Cool. Ok, well, who pulls the trigger? 🪙 / 🪙 |
Resolves #6614.