lmms icon indicating copy to clipboard operation
lmms copied to clipboard

Add Qt 6 support

Open PhysSong opened this issue 3 years ago • 65 comments

Working branch: https://github.com/LMMS/lmms/tree/qt6

As 2023-05-26 is the EOL of Qt 5.15(without subscription licenses), it's worth starting working on Qt 6 support. Since we require C++17 and has helpers for deprecated Qt features, it might be not as hard as adding Qt 5 support.

See here for more information about how to port from Qt5 to Qt6: https://doc.qt.io/qt-6/portingguide.html

I made a checklist for Qt 6 support. Please feel free to suggest something to add.

  • [x] Fix build errors/deprecation warnings with Qt 6
    • [x] Fix all deprecation warnings with Qt 5.15 (#6615)
    • [x] Handle removal of Qt X11 extras (Only used in X11 embedding which is disabled er #7273)
    • [x] Replace QRegExp with QRegularExpression (#7133)
    • [x] ~MM_OPERATORS confilcts with getDefaultCtr/getCopyCtr/getMoveCtr of QMetaTypeInterface for SampleBuffer~: Removed in #7128
    • [x] Handle removal of QApplication::desktop
      • [x] in gui_templates.h: use QWidget's DPI instead(or entirely remove, if possible) (#7185)
      • [x] in ComboBox.cpp: use QWidget's DPI instead(or entirely remove, if possible) (#7179)
    • [x] Handle signature changes of QAbstractNativeEventFilter::nativeEventFilter in Qt 6 (#7254)
    • [x] Either fix qt5-x11embed to support Qt 6 or drop XEmbed support for Qt 6 (dropped in #7273)
    • [x] Remove usage of Qt5Compat module in hydrogen import plugin (#7562)
  • [x] Work around no official supports for Qt 6 on 32bit Windows (32 bit is unsupported per consensus)
  • [x] Complete Qt 6 TODOs/FIXMEs
    • [x] A QWheelEvent may have both x and y deltas, potentially breaking SongEditor::wheelEvent(see https://github.com/LMMS/lmms/pull/5619#issuecomment-673919832)
  • [x] Fix new bugs with Qt 6 builds, if any
    • [x] QComboBox::activated(const QString&) is renamed to textActivated in Qt 5.14, breaking signal-slot connections
  • [ ] Add CI builds for Qt 6
    • [x] Windows + MSVC
    • [ ] Windows + MinGW
    • [ ] macOS
    • [ ] Linux
  • [x] Check if packaging still works
    • [x] Windows
    • [x] macOS
    • [ ] Linux
  • [x] Update build instructions on Wiki

PhysSong avatar Jan 17 '23 13:01 PhysSong

Opened #7133 to replace QRegExp with QRegularExpression. Might wanna add to the list.

Rossmaxx avatar Mar 03 '24 07:03 Rossmaxx

MM_OPERATORS confilcts with getDefaultCtr/getCopyCtr/getMoveCtr of QMetaTypeInterface for SampleBuffer

does this still affect post the removal of MemoryManager and Sample refactor?

Rossmaxx avatar Mar 03 '24 07:03 Rossmaxx

I saw we could use Clazy to port much of the Code to Qt6, Here is the doc page: https://doc.qt.io/qt-6/porting-to-qt6-using-clazy.html, I already tried it out and it works good, This Page is also an great Resource for Porting over: https://doc.qt.io/qt-6/portingguide.html

Snowiiii avatar Mar 03 '24 17:03 Snowiiii

MM_OPERATORS confilcts with getDefaultCtr/getCopyCtr/getMoveCtr of QMetaTypeInterface for SampleBuffer

MemoryManager was removed in #7128. I think we can remove this one (or just check it off)?

sakertooth avatar Mar 17 '24 00:03 sakertooth

Handle removal of QApplication::desktop in gui_templates.h: use QWidget's DPI instead(or entirely remove, if possible)

done via #7159

Also, i didn't find any uses of QApplication::desktop in ComboBox.cpp

I tried to enable experimental qt6 support via a flag at #7182

Rossmaxx avatar Mar 28 '24 07:03 Rossmaxx

Pull request #7179 handles the removal of QApplication::desktop in ComboBox.cpp.

michaelgregorius avatar Mar 29 '24 20:03 michaelgregorius

The Linux and mingw builds fail for pull request #7179 because the packaged Qt versions are too old. QWidget::screen has been introduced with 5.14 and for example Ubuntu 18.04 only provides 5.9.5.

Edit: Fixed with commit c1ee2d8346bd9c725f167e4c2535b889cff997ef.

michaelgregorius avatar Mar 29 '24 21:03 michaelgregorius

#7182 is now in a mergeable state. Would help if anyone from here would review it.

Rossmaxx avatar Apr 10 '24 12:04 Rossmaxx

Hi @Rossmaxx,

I have checked out the branch and have run CMake as follows:

cmake .. -DCMAKE_INSTALL_PREFIX=../target-qt6 -DWANT_QT6=True

This leads to the following error with regards to ZynAddSubFx:

[...]
-- Configuring done (7.0s)
CMake Error at plugins/ZynAddSubFx/CMakeLists.txt:117 (target_link_libraries):
  The link interface of target "ZynAddSubFxCore" contains:

    Qt5::Widgets

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.



CMake Error at plugins/ZynAddSubFx/CMakeLists.txt:186 (TARGET_LINK_LIBRARIES):
  Target "RemoteZynAddSubFx" links to:

    Qt5::Core

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.



-- Generating done (0.4s)
CMake Generate step failed.  Build files cannot be regenerated correctly.

michaelgregorius avatar Apr 10 '24 18:04 michaelgregorius

I'd like to propose to create a "qt6-migration" branch in the official repository using the changes already done in #7182. That way the code could be worked on collaboratively without the need for intermediate pull requests until everything compiles/works. Once that state is reached there could then be one "official" pull request using that branch where some final "polishing" might be discussed.

michaelgregorius avatar Apr 10 '24 20:04 michaelgregorius

@michaelgregorius i forgot about that one. For now, cmake configuration only works if you set -DLMMS_MINIMAL=ON. A bit clunky and lacks features but that's the only way for now.

Rossmaxx avatar Apr 11 '24 03:04 Rossmaxx

Thanks for the info, @Rossmaxx!

michaelgregorius avatar Apr 11 '24 15:04 michaelgregorius

@Rossmaxx, I have made adjustments to your branch so that minimal mode now fully compiles. Is it okay if I push the changes into your branch?

michaelgregorius avatar Apr 11 '24 16:04 michaelgregorius

Is it okay if I push the changes into your branch?

Yes please.

Rossmaxx avatar Apr 11 '24 17:04 Rossmaxx

Is it okay if I push the changes into your branch?

Yes please.

Ok, great! Done with commit bb4d04b5e71.

michaelgregorius avatar Apr 11 '24 17:04 michaelgregorius

Here's a list of the build warnings that are currently given when building commit 684afdb7c9e from @Rossmaxx's repository: build_warnings.zip.

We will need to check how to deal with the different types of warnings. If we are lucky they can be fixed without some version specific ifdefs.

Edit: the file was created with the following command in the build directory:

make -j12 > build_out.txt 2> build_warnings.txt

michaelgregorius avatar Apr 20 '24 08:04 michaelgregorius

Handle signature changes of QAbstractNativeEventFilter::nativeEventFilter in Qt 6

Any ideas on what to do here?

Rossmaxx avatar Apr 20 '24 15:04 Rossmaxx

Handle signature changes of QAbstractNativeEventFilter::nativeEventFilter in Qt 6

Any ideas on what to do here?

The last parameter has type of long on Qt 5, but it is quintptr on Qt 6.

PhysSong avatar Apr 22 '24 01:04 PhysSong

Depending on the surrounding code this might also be solvable with a helper method that uses a type as the last parameter that's defined via a using depending on the Qt version used.

So similar to what was for example done for ControlLayout.h here: https://github.com/LMMS/lmms/pull/7182/files#diff-6e9cbb14ccab26c8ddbde9070cef9344193bb144261c3a89c42f001f327059f7

michaelgregorius avatar Apr 22 '24 14:04 michaelgregorius

Work around no official supports for Qt 6 on 32bit Windows

#7212 has gotten rid of the requirement for 32 bit qt for 64 bit windows. Check it off?

Rossmaxx avatar Apr 24 '24 07:04 Rossmaxx

I'd like to propose to create a "qt6-migration" branch in the official repository using the changes already done in #7182. That way the code could be worked on collaboratively without the need for intermediate pull requests until everything compiles/works. Once that state is reached there could then be one "official" pull request using that branch where some final "polishing" might be discussed.

For anyone interested: that branch is now available at https://github.com/LMMS/lmms/tree/qt6.

michaelgregorius avatar Apr 25 '24 16:04 michaelgregorius

The last parameter has type of long on Qt 5, but it is quintptr on Qt 6.

Added to #7086

Rossmaxx avatar Apr 28 '24 10:04 Rossmaxx

Current status of Qt6 builds: Linux - works Mac - works Windows MSVC - works Windows MinGW - not planned (pretty sure Qt 6 is not available there)

Rossmaxx avatar May 05 '24 14:05 Rossmaxx

Work around no official supports for Qt 6 on 32bit Windows

If Qt has dropped support for 32-bit OSs, I feel we should also drop support for 32-bit OSs.

tresf avatar May 05 '24 15:05 tresf

Mac - untested (@tresf can you help pls?)

Fantastic work! Mac tested, works. I tested with Unfa Spoken demo track and it sounds fine.

Commands (click to expand):
brew install git cmake pkgconfig fftw libogg libvorbis lame libsndfile libsamplerate jack \
fluid-synth sdl2 libgig libsoundio stk portaudio node fltk qt glib
#                                                          ^--- changed "qt5" to "qt"
rm -rf lmms
git clone --recurse-submodules -b master https://github.com/lmms/lmms
cd lmms
git checkout qt6
export CMAKE_PREFIX_PATH=$(brew --prefix qt)
#                                        ^--- changed "qt5" to "qt"
mkdir build
cd build
cmake .. -DCMAKE_INSTALL_PREFIX=../target -DWANT_QT6=true
#                                         ^--- added flag
make -j8
./lmms

AFAIR the DMG creation script on master is broken, so I didn't test that. Will sort that on master in a separate issue/PR.

tresf avatar May 05 '24 16:05 tresf

Will sort that on master in a separate issue/PR.

Feel free to work on that qt6 branch. As of now, the plan is to do everything there and open one final PR consisting of all changes.

Rossmaxx avatar May 05 '24 16:05 Rossmaxx

Windows MinGW - not planned (pretty sure Qt 6 is not available there)

If we decide to keep Mingw support around, we would require a PPA for this. There seems to be some disagreement on Discord as to whether or not this is worthwhile to keep around. Pinging @tobydox as an FYI in the event we decide to keep it. <3

Feel free to work [DMG generation issues] on that qt6 branch. As of now, the plan is to do everything there and open one final PR consisting of all changes.

Sorry, this probably wasn't clear but the DMG issues are unrelated to Qt6. Master's DMG is only 46 MB currently (should be ~100MB) , so something's been wrong on nightly for quite some time. I'll investigate this issue and fix it directly on master.

tresf avatar May 05 '24 16:05 tresf

Sorry, this probably wasn't clear but the DMG issues are unrelated to Qt6

I spoke too soon, some are. PR opened. #7240

~~Also, Zyn won't load on Apple. I've added a checklist item to the OP to track this.~~.

This bug is present on master too, removing.

tresf avatar May 06 '24 06:05 tresf

@DomClark I would appreciate if you could help me debug the qt6 build.

After applying the changes from #7086, it builds successfully for me (using Qt 6.7.1 and Visual Studio 2022). I'm happy to help debug your particular configuration, but it might be better to do that on Discord so as not to clutter this issue.

DomClark avatar May 06 '24 17:05 DomClark

After applying the changes from #7086, it builds successfully for me

Forgot that that PR had a fix. Will close that PR and open a new one against the qt6 branch.

Rossmaxx avatar May 07 '24 00:05 Rossmaxx