Lmms: Fix annoying Qt5 deprecated warnings

[JohannesLorenz]

Bug Summary

LMMS brings tons of Qt5 deprecated warnings (e.g. QFlag) which can make it hard to see relevant compiler issues. The warnings are justified, they should be fixed (suppressing them would not be the solution).

Steps to reproduce

Compile LMMS (master) with Qt5.

Expected behavior

No Qt5 warnings.

Actual behavior

Tons of Qt5 warnings (e.g. QFlag, see Logs; has the QString warning already been fixed?)

Affected LMMS versions

At least on master.

Logs

Click to expand

[  0%] Building CXX object src/CMakeFiles/lmmsobjs.dir/gui/Lv2ViewBase.cpp.o
In file included from lv2/include/MainWindow.h:34,
                 from lv2/src/gui/Lv2ViewBase.cpp:46:
lv2/include/SubWindow.h:58:67: warning: 'constexpr QFlags::QFlags(QFlags::Zero) [with Enum = Qt::WindowType; QFlags::Zero = int QFlags::Private::*]' is deprecated: Use default constructor instead [-Wdeprecated-declarations]
   58 |  SubWindow( QWidget *parent = NULL, Qt::WindowFlags windowFlags = 0 );
      |                                                                   ^
In file included from /usr/include/qt/QtCore/qglobal.h:1302,
                 from /usr/include/qt/QtCore/qchar.h:43,
                 from /usr/include/qt/QtCore/qstring.h:49,
                 from /usr/include/qt/QtCore/QString:1,
                 from lv2/include/Lv2ViewBase.h:33,
                 from lv2/src/gui/Lv2ViewBase.cpp:25:
/usr/include/qt/QtCore/qflags.h:123:80: note: declared here
  123 |     QT_DEPRECATED_X("Use default constructor instead") Q_DECL_CONSTEXPR inline QFlags(Zero) noexcept : i(0) {}
      |                                                                                ^~~~~~
In file included from lv2/src/gui/Lv2ViewBase.cpp:46:
lv2/include/MainWindow.h:64:83: warning: 'constexpr QFlags::QFlags(QFlags::Zero) [with Enum = Qt::WindowType; QFlags::Zero = int QFlags::Private::*]' is deprecated: Use default constructor instead [-Wdeprecated-declarations]
   64 |  LMMS_EXPORT SubWindow* addWindowedWidget(QWidget *w, Qt::WindowFlags windowFlags=0);
      |                                                                                   ^
In file included from /usr/include/qt/QtCore/qglobal.h:1302,
                 from /usr/include/qt/QtCore/qchar.h:43,
                 from /usr/include/qt/QtCore/qstring.h:49,
                 from /usr/include/qt/QtCore/QString:1,
                 from lv2/include/Lv2ViewBase.h:33,
                 from lv2/src/gui/Lv2ViewBase.cpp:25:
/usr/include/qt/QtCore/qflags.h:123:80: note: declared here
  123 |     QT_DEPRECATED_X("Use default constructor instead") Q_DECL_CONSTEXPR inline QFlags(Zero) noexcept : i(0) {}
      |                                                                                ^~~~~~
lv2/src/gui/Lv2ViewBase.cpp: In constructor 'Lv2ViewBase::Lv2ViewBase(QWidget*, Lv2ControlBase*)':
lv2/src/gui/Lv2ViewBase.cpp:176:64: warning: 'constexpr QFlags::QFlags(QFlags::Zero) [with Enum = Qt::WindowType; QFlags::Zero = int QFlags::Private::*]' is deprecated: Use default constructor instead [-Wdeprecated-declarations]
  176 |   m_helpWindow = gui->mainWindow()->addWindowedWidget(infoLabel);
      |                                                                ^
In file included from /usr/include/qt/QtCore/qglobal.h:1302,
                 from /usr/include/qt/QtCore/qchar.h:43,
                 from /usr/include/qt/QtCore/qstring.h:49,
                 from /usr/include/qt/QtCore/QString:1,
                 from lv2/include/Lv2ViewBase.h:33,
                 from lv2/src/gui/Lv2ViewBase.cpp:25:
/usr/include/qt/QtCore/qflags.h:123:80: note: declared here
  123 |     QT_DEPRECATED_X("Use default constructor instead") Q_DECL_CONSTEXPR inline QFlags(Zero) noexcept : i(0) {}
      | 

[Spekular]

Is your proposed fix to suppress these warnings, or to update the code to avoid deprecated code?

[JohannesLorenz]

I think they should be fixed properly. Suppressing deprecate warnings does not seem good to me.

[Spekular]

Ok great, that's what I was thinking as well. Just wanted clarification since it wasn't mentioned in the issue.

[uthidata]

I would like to work on this if nobody else is doing it.

[JohannesLorenz]

@uthidata Thanks for your reply. Currently no one volunteered, so you can do it.

[uthidata]

I encountered an issue while trying to fix the QWheelEvent warnings

In Automation Editor, when the user press Ctrl + Alt while scrolling, the quantize model should change.
In the old code, this was done by comparing wheelEvent->delta(), which is now deprecated.

The suggested fix is to turn it into wheelEvent->angleDelta(). The angleDelta() function returns a QPoint, so I had to add .y() to check the vertical scrolling distance. This works well for the other modifiers, and I have tested that the behavior stayed the same. However, on KDE at least, Alt + scroll results in a horizontal scroll, and the .y() will be 0, while the .x() is the expected values.

I want to work around this by adding the x and y values, with the assumption that people would not scroll on both axes at the same time. If you have other ideas, please tell me.

[JohannesLorenz]

Good found.

The sum can also be retrieved using QPoint::manhattanLength. Alternatively, you could try qMax(.x(), .y()). It's probably try and error what's best.

[uthidata]

Sadly manhattanLength returns the sum of absolute values whereas we need signed values. I will use .x and .y for now.

[uthidata]

The results of PR #5619 shows that my fixes are not appropriate for Qt 5.12, since some of the functions that show up as deprecated on my machine is not yet there until Qt 5.14, namely:

• QAtomicPointer::loadRelaxed()
• QSet(list.begin(), list.end())
• QWheelEvent::position()

The result being that it could not build for Ubuntu 16.04.
I can change the appveyor config to make it build on Windows, but I want to ask first. What should I do?

[Veratil]

The result being that it could not build for Ubuntu 16.04.
I can change the appveyor config to make it build on Windows, but I want to ask first. What should I do?

@PhysSong what do you think?

[uthidata]

The result being that it could not build for Ubuntu 16.04.

Well, scratch that, it cannot build for Ubuntu 20.04 either. Qt 5.14 is planned to land in Ubuntu 20.10.

I'm thinking of guarding these specific parts with preprocessors, and add a #pragma message telling about multiple versions instead. This way when the toolchain is eventually updated, maintainers would know which parts can be removed.

#pragma message("Remove deprecated version of this snippet when the toolchain is updated to 5.14")
#if (QT_VERSION >= QT_VERSION_CHECK(5,14,0))
    vcf.loadRelaxed()->playNote();
#else
    vcf.load()->playNote();
#endif

[JohannesLorenz]

Good idea.

If it happens more often you may want to wrap this into a function, like T* loadRelaxed(const QAtomicPointer<T>&).

[uthidata]

Ok, so this is what I'm going to do:

• update AppVeyor config to build with qt 5.14 on Windows
• add preprocessors around parts that will get deprecated soon
• keep fixes of parts that are already deprecated in 5.12

[PhysSong]

Some comments:

• I think we should keep supporting Qt 5.9, and probably 5.6.
• Fixing those warnings will help us supporting Qt 6 which will be released at the end of this year.

The result being that it could not build for Ubuntu 16.04.
I can change the appveyor config to make it build on Windows, but I want to ask first. What should I do?

@PhysSong what do you think?

I think we should first try not breaking builds with old Qt5 releases.

[uthidata]

Thanks for the feedback. I've made it so that it still runs on older versions of Qt.

I've also added preprocessor switches for some of these parts, along with a deprecation message printed with #pragma message for each. For deprecated or soon to be deprecated parts that are used in multiple files, I've added DeprecationHelper.h.

Compared to master branch, these warnings have dropped from 2800ish to 200ish lines on my machine (Qt 5.15).

The PR now builds on all configurations. Please check #5619.

[DomClark]

I'm not sure how useful the #pragma message is; the motivation here is partially to avoid annoying messages during compilation, so I'm not convinced that we want to substitute our own instead. Knowing which parts can be removed later is easy - just search the codebase for QT_VERSION. This is something we've already done before, to remove Qt 4 compatibility code.

[uthidata]

I'm not sure how useful the #pragma message is; the motivation here is partially to avoid annoying messages during compilation, so I'm not convinced that we want to substitute our own instead. Knowing which parts can be removed later is easy - just search the codebase for QT_VERSION. This is something we've already done before, to remove Qt 4 compatibility code.

Understandable, I will do a quick rebase along with the formatting.