Fix Qt Deprecations

[sakertooth]

This PR fixes the following deprecations in src:

‘QMutex::QMutex(QMutex::RecursionMode)’ is deprecated: Use QRecursiveMutex instead of a recursive QMutex

‘QTextStream& QTextStreamFunctions::endl(QTextStream&)’ is deprecated: Use Qt::endl

‘Qt::MidButton’ is deprecated: MidButton is deprecated. Use MiddleButton instead

‘QPalette::Background’ is deprecated: Use QPalette::Window instead

‘QStringList QString::split(QChar, QString::SplitBehavior, Qt::CaseSensitivity) const’ is deprecated: Use 
Qt::SplitBehavior variant instead

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

• Linux (AppImage): lmms-1.3.0-alpha.1.196+gb3ccfeff2-linux-x86_64.AppImage (build link)

Windows

• Windows 32-bit: lmms-1.3.0-alpha.1.196+gb3ccfeff2-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha.1.196+gb3ccfeff2-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://output.circle-artifacts.com/output/job/8c4b39a6-87d3-4665-825c-7e43350abe15/artifacts/0/lmms-1.3.0-alpha.1.196+gb3ccfeff2-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/sakertooth/lmms/238?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/431110e6-f24d-4885-a7fb-455897ac07f6/artifacts/0/lmms-1.3.0-alpha.1.196+gb3ccfeff2-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/sakertooth/lmms/237?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/b862e0b4-4b01-4227-b3e0-ec5971a78f37/artifacts/0/lmms-1.3.0-alpha.1.196+gb3ccfeff2-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/sakertooth/lmms/239?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/v0pesmhd7rismgq9/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/43907999"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/xbpjtwugjm34q7vd/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/43907999"}]}, "commit_sha": "b3ccfeff2de3542f82b54a88e55676e144901926"}

[Veratil]

I don't see anything in the 5.15 Qt docs about QMutex::QMutex([QMutex::RecursionMode]) being deprecated. https://doc.qt.io/qt-5/qmutex.html#QMutex-1

Qt::endl is fine. I don't even see anything about QTextStreamFunctions anymore.

Qt::MiddleButton is fine.

QPalette::Background has been deprecated since 5.13, I don't think we should switch to QPalette::Window explicitly yet. I think we should use the normal #if (QT_VERSION >= QT_VERSION_CHECK(5,13,0)) guard instead for now.

It looks like Qt::SplitBehavior was copied from QString::SplitBehavior in 5.14. Another we'll probably want to use the version check live above for.

[sakertooth]

I don't see anything in the 5.15 Qt docs about QMutex::QMutex([QMutex::RecursionMode]) being deprecated. https://doc.qt.io/qt-5/qmutex.html#QMutex-1

Qt::endl is fine. I don't even see anything about QTextStreamFunctions anymore.

Qt::MiddleButton is fine.

QPalette::Background has been deprecated since 5.13, I don't think we should switch to QPalette::Window explicitly yet. I think we should use the normal #if (QT_VERSION >= QT_VERSION_CHECK(5,13,0)) guard instead for now.

It looks like Qt::SplitBehavior was copied from QString::SplitBehavior in 5.14. Another we'll probably want to use the version check live above for.

Seems like I had Qt 6 on my system, so it compiled fine on my end and gave a warning for QMutex::QMutex([QMutex::RecursionMode]). I'll be reverting some of the changes soon however.

[Veratil]

Seems like I had Qt 6 on my system, so it compiled fine on my end and gave a warning for QMutex::QMutex([QMutex::RecursionMode]). I'll be reverting some of the changes soon however.

You don't need to revert, just check out #5619 for examples of how to handle deprecation stuff. :)

[sakertooth]

Deprecation of QMutex::QMutex([QMutex::RecursionMode]) is not specified in the docs, but I found this in Qt's code:

#if QT_DEPRECATED_SINCE(5,15)
    enum RecursionMode { NonRecursive, Recursive };
    QT_DEPRECATED_VERSION_X(5, 15, "Use QRecursiveMutex instead of a recursive QMutex")
    explicit QMutex(RecursionMode mode);

    QT_DEPRECATED_VERSION_X(5, 15, "Use QRecursiveMutex instead of a recursive QMutex")
    bool isRecursive() const noexcept
    { return QBasicMutex::isRecursive(); }
#endif

[Veratil]

Deprecation of QMutex::QMutex([QMutex::RecursionMode]) is not specified in the docs, but I found this in Qt's code:

#if QT_DEPRECATED_SINCE(5,15)
    enum RecursionMode { NonRecursive, Recursive };
    QT_DEPRECATED_VERSION_X(5, 15, "Use QRecursiveMutex instead of a recursive QMutex")
    explicit QMutex(RecursionMode mode);

    QT_DEPRECATED_VERSION_X(5, 15, "Use QRecursiveMutex instead of a recursive QMutex")
    bool isRecursive() const noexcept
    { return QBasicMutex::isRecursive(); }
#endif

Alright, I think this should be fine then. 👍

[Veratil]

Mostly style changes to move the preprocessor directives to the beginning of the line.
One move request to put a doc comment back in place.
One question about removing a previously added deprecation section.

[JohannesLorenz]

Just for information, clazy 1.11 also warns that you should replace QMutex by QRecursiveMutex, so I'm very positive about this PR.

[JohannesLorenz]

I left only 1 minor comment. Code looks mostly OK.

[JohannesLorenz]

@Veratil are you OK with the last rework commit? If yes, then there is only testing review left.

[JohannesLorenz]

Testing review failed!

I rebased this to master on my local machine and loaded a large demo song ("Greippi - Krem Kaakkuja (Second Flight Remix).mmpz"). In master, the song plays normal, and the CPU is low if nothing is being played. In this branch, the song plays only with interruptions, and the CPU is high both when playing and when not playing. I assume that it's from the QMutex change (you could try to find out what causes it by removing parts of your PR).

[Veratil]

Testing review failed!

I rebased this to master on my local machine and loaded a large demo song ("Greippi - Krem Kaakkuja (Second Flight Remix).mmpz"). In master, the song plays normal, and the CPU is low if nothing is being played. In this branch, the song plays only with interruptions, and the CPU is high both when playing and when not playing. I assume that it's from the QMutex change (you could try to find out what causes it by removing parts of your PR).

Does it do the same if you didn't rebase?

[JohannesLorenz]

Does it do the same if you didn't rebase?

Yes. I just did the rebase to exclude that a master commit would have caused the difference. Can you reproduce this issue?

[Veratil]

Does it do the same if you didn't rebase?

Yes. I just did the rebase to exclude that a master commit would have caused the difference. Can you reproduce this issue?

Unfortunately I can't even play it on any branch in my VM because it takes up too much CPU. It's constantly in the red.

[sakertooth]

Testing review failed!

I rebased this to master on my local machine and loaded a large demo song ("Greippi - Krem Kaakkuja (Second Flight Remix).mmpz"). In master, the song plays normal, and the CPU is low if nothing is being played. In this branch, the song plays only with interruptions, and the CPU is high both when playing and when not playing. I assume that it's from the QMutex change (you could try to find out what causes it by removing parts of your PR).

For me, it just segfaults for the same demo. I have no idea what is going on 😆, and the different demos I have tried segfault for different reasons it seems. Was doing a partial build.

[JohannesLorenz]

For me, it just segfaults for the same demo. I have no idea what is going on laughing, and the different demos I have tried segfault for different reasons it seems.

Can you please run it in gdb, and print us a backtrace (bt)?

[sakertooth]

For me, it just segfaults for the same demo. I have no idea what is going on laughing, and the different demos I have tried segfault for different reasons it seems.

Can you please run it in gdb, and print us a backtrace (bt)?

Sorry, disregard that. It was because I was doing a partial build for lmms. I usually do this to speed up build times, especially when I don't think I need to build the full suite of plugins.

Pertaining to the issue you described above, my CPU usage is fairly high when playing the project and then stays on some middle ground constantly. Not sure what could be the issue. Going to do some testing.

[sakertooth]

Tested this on master, and I am seeing the same issue. I do not think this issue originates from this PR.

[JohannesLorenz]

Tested this on master, and I am seeing the same issue

Dumb question, did you recompile and reinstall? Do you have a good computer?

[sakertooth]

Tested this on master, and I am seeing the same issue

Dumb question, did you recompile and reinstall? Do you have a good computer?

My computer is fine, truly (although having 8 GB of RAM can sometimes be a bottleneck under heavy load, it should be more than enough). I reconfigured my CMake configuration and rebuilt everything on the master branch, but the CPU usage issue remains. It stays high on idle.

I forgot to mention that the playback for me has no interruptions of any kind even with the CPU being fairly high.

[JohannesLorenz]

My bad. I tested with pulseaudio (which is not RT safe), and thus got very strange results.

With ALSA, I now get much better results. It plays well there. Testing passed.

Also, the last commit looks good. @Veratil shall we merge this?

[Veratil]

I'm okay with it. 👍

[JohannesLorenz]

@sakertooth The PR is good, but can you please fix this merge conflict so we can merge it?

[sakertooth]

@sakertooth The PR is good, but can you please fix this merge conflict so we can merge it?

Ah, just saw that the namespace PR was merged. Resolving merge conflicts soon.

[JohannesLorenz]

I think there are 1 or 2 merge errors, but only whitespace.

[sakertooth]

Merge?