Qt deprecation fix #5619

uthidata · 2020-08-10T19:53:51Z

Fix Qt deprecation warnings

Replace QWheelEvent::delta with angleDelta
Replace QWheelEvent::orientation with comparison of angleDelta.x and y
Remove Qt::WindowFlags parameters when adding SubWindows
Remove QFileDialog::Options parameters when opening files
These parameters are currently not used anywhere.
Use QFlag(0) when initializing items in ladspaPortDialog instead of 0
Use explicit enum when specifying QStyle state in ComboBox's paintEvent
Use QElapsedTimer instead of QTime where appropriate
Use QAtomicPointer::loadRelaxed() instead of load()
Use QSet(list.begin(), list.end()) instead of list.toSet()
Use Qt::endl in QTextStream
Use QString::isEmpty() instead of comparing to QString::null
Use QFontMetrics::horizontalAdvance instead of width
Use QWheelEvent::position() instead of pos()

LmmsBot · 2020-08-10T20:15:27Z

🤖 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.2.2.693-linux-x86_64.AppImage (build link)

Windows

Windows 64-bit: lmms-1.2.2.693-mingw-win64.exe (build link)
Windows 32-bit: lmms-1.2.2.693-mingw-win32.exe (build link)
Windows 32-bit: lmms-1.2.2-msvc2017-win32.exe (build link)
Windows 64-bit: lmms-1.2.2-msvc2017-win64.exe (build link)

macOS

macOS : lmms-1.2.2.693-mac10.13.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://8532-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.693-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8532?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://8531-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.693-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8531?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://8535-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.693-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8535?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/kar6d2ci9dhiso4a/artifacts/build/lmms-1.2.2-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/34922746"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/22xj1oha6ced97y0/artifacts/build/lmms-1.2.2-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/34922746"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://8533-15778896-gh.circle-artifacts.com/0/lmms-1.2.2.693-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8533?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "be7fcaa185c32e15e981ee78835e5a8a99d1e712"}

PhysSong · 2020-08-11T02:25:31Z

I recommend you looking at the Qt documentation to check the version compatibility of those functions.

uthidata · 2020-08-11T02:39:12Z

I recommend you looking at the Qt documentation to check the version compatibility of those functions.

I am on it, got a bit sidetracked but I'm finishing up now.

plugins/sf2_player/patches_dialog.h

uthidata · 2020-08-11T23:15:42Z

@DomClark The pragma warnings and style violations are gone. I think it's ready.

plugins/lb302/lb302.cpp

DomClark

Looks good to me.

JohannesLorenz · 2020-08-13T00:08:23Z

@DomClark as you commented this as lgtm, what kind of review do you expect? At least, from testing, I'm satisfied, as this removed all the warnings mentioned in #5598 .

There are still open comments in this PR btw.

DomClark · 2020-08-13T00:12:23Z

What kind of review do you expect?

Just after some confirmation that this fixed the warnings you were seeing, since you opened the issue. Sounds like it does.

There are still open comments in this PR btw.

Resolved.

Will merge tomorrow if there are no objections.

PhysSong · 2020-08-13T09:50:22Z

Please wait, I have to check something about wheel events.

PhysSong · 2020-08-14T06:54:01Z

Replace QWheelEvent::orientation with comparison of angleDelta.x and y

It does work for Qt 5, but I think it's not a good idea.

If it's possible to handle both vertical and horizontal scroll events without that check, why not?
Qt 5 generates two events(vertical one and horizontal one) for arbitrary scrolling to be compatible with Qt 4. That's not true in Qt 6 anymore.

SongEditor::wheelEvent looks somewhat tricky to get rid of the trick, so let me check first. AutomationEditor and PianoRoll look fine.

PhysSong · 2020-08-14T07:20:15Z

To be clear, SongEditor handles horizontal scroll events and TrackContainerView::scrollArea handles vertical ones. This makes handling wheel events more complicated than other editors.

Veratil · 2020-08-14T16:14:59Z

To be clear, SongEditor handles horizontal scroll events and TrackContainerView::scrollArea handles vertical ones. This makes handling wheel events more complicated than other editors.

It doesn't handle horizontal well, but I made this #5174 which still needs to handle Qt version differences.

uthidata · 2020-08-14T16:41:36Z

If it's possible to handle both vertical and horizontal scroll events without that check, why not?

The check was there before. I merely translated it into non-deprecated form.

Qt 5 generates two events(vertical one and horizontal one) for arbitrary scrolling to be compatible with Qt 4. That's not true in Qt 6 anymore.

I'm not sure what you mean by this. Qt 6 will remove the orientation function, and the angleDelta with x and y will be there. Comparing their absolute values is the fastest way to determine the orientation. Unless you're saying the orientation is not important, but there should be a design document on how you expect it to work. Should the modifiers work regardless of orientation? Which orientation would be prefered?... I think it is not in the scope of this PR. I would gladly change them if you post an issue detailing the problem.

Now that I think of it, maybe we can put the desired behaviors in a unit test?

It doesn't handle horizontal well, but I made this #5174 which still needs to handle Qt version differences.

Unit tests could theoretically solve this across the board. No more manual testings.

PhysSong · 2020-08-14T23:51:39Z

I mean, wheel events can have arbitrary orientation in Qt6(not restricted to horizontal or vertical).

uthidata · 2020-08-15T00:08:14Z

What's the desired behavior of wheel scroll on those windows? So I can write tests.

PhysSong · 2020-08-15T02:41:44Z

A note on QWheelEvent::position(): There's QWheelEvent::posF() with slightly different signature(the former returns a value whereas the latter returns a constant reference).

PhysSong · 2020-08-15T02:43:12Z

Ideally, A wheel event with (dx, dy) should behave the same as the combination of (dx, 0) and (0, dy). If that's tricky for any reason, you should add a TODO comment for Qt 6.

uthidata · 2020-08-15T11:38:55Z

Ideally, A wheel event with (dx, dy) should behave the same as the combination of (dx, 0) and (0, dy). If that's tricky for any reason, you should add a TODO comment for Qt 6.

The comparison is not there to differentiate between (dx, 0) and (0, dy), but to determine the right direction to scroll based on a single (dx, dy).

A note on QWheelEvent::position(): There's QWheelEvent::posF() with slightly different signature(the former returns a value whereas the latter returns a constant reference).

QWheelEvent::posF is deprecated.

PhysSong · 2020-08-17T04:17:04Z

Ideally, A wheel event with (dx, dy) should behave the same as the combination of (dx, 0) and (0, dy). If that's tricky for any reason, you should add a TODO comment for Qt 6.

The comparison is not there to differentiate between (dx, 0) and (0, dy), but to determine the right direction to scroll based on a single (dx, dy).

IMHO, (dx, dy) should scroll (dx, dy), neither (dx, 0) nor (0, dy).

A note on QWheelEvent::position(): There's QWheelEvent::posF() with slightly different signature(the former returns a value whereas the latter returns a constant reference).

QWheelEvent::posF is deprecated.

I know, but I mentioned it for position().toPoint() vs. pos() vs. posF.toPoint().

uthidata · 2020-08-18T05:47:06Z

IMHO, (dx, dy) should scroll (dx, dy), neither (dx, 0) nor (0, dy).

How can one scroll (dx, dy) while holding Ctrl + Shift? There is only one scrolling dimension, but two inputs. They will have to combine, or selected. My code currently selects dy.

uthidata · 2020-08-26T16:42:03Z

Hi, what can I change to get this merged?

PhysSong · 2020-08-27T05:48:01Z

I think the best way for now is reverting QWheelEvent::orientation() changes or adding a FIXME comment(for Qt 6).

- replace QWheelEvent::delta with angleDelta - replace QWheelEvent::orientation with comparison of angleDelta.x and y

- use QFlag(0) instead of 0 when appropriate or prompted - use explicit enum when specifying QStyle state in ComboBox's paintEvent

- Use QElapsedTimer instead of QTime where appropriate - Use QAtomicPointer::loadRelaxed() instead of load() - Use QSet<T>(list.begin(), list.end()) instead of list.toSet() - Use Qt::endl in QTextStream - Use QString::isEmpty() instead of comparing to QString::null - Use QFontMetrics::horizontalAdvance instead of width - Use QWheelEvent::position() instead of pos()

uthidata · 2020-08-29T19:58:00Z

I just added FIXME comments where orientation was replaced.

JohannesLorenz · 2020-09-05T12:33:05Z

Can anyone please summarize which points are still open in this PR?

uthidata · 2020-09-05T12:52:46Z

All deprecation warnings are fixed. The point about horizontal/vertical scrolling distinction is dealt with as suggested. I don't think there's anything left to do.

JohannesLorenz · 2020-09-05T13:01:58Z

@PhysSong Can you please check whether your suggestion was coded correctly, and if yes merge this PR?

PhysSong · 2020-09-05T13:06:10Z

I'll check this again tomorrow and merge if looks okay.

JohannesLorenz

approved from my side - no compiler warnings anymore

PhysSong · 2020-09-11T04:06:41Z

I'll merge it in a day unless there are any objections.

Resolves the conflict on src/tracks/Pattern.cpp caused by LMMS#5619. On this PR the only change to the file was removing an unnecessary include of the header "StringPairDrag.h", which the PR LMMS#5619 also did. So the file was kept the same way as in the master branch.

PhysSong · 2020-11-30T04:57:49Z

src/core/audio/AudioFileOgg.cpp

+	QRandomGenerator::global()->seed(time(0));
+	m_serialNo = 0xD0000000 + QRandomGenerator::global()->generate() % 0x0FFFFFFF;


Reported on Discord: an user got the following error on OGG export.

Attempted to overwrite a QRandomGenerator to system() or global().

The documentation says QRandomGenerator::global() is already seeded and can't be overwritten by seed().

Open a new issue with this information so it gets attention and a PR can reference it?

Qt6 TODO: Orientation check by comparing angleDelta().x() and y() won't make sense because the direction is arbitrary in Qt 6.

uthidata mentioned this pull request Aug 10, 2020

Fix annoying Qt5 deprecated warnings #5598

Closed

DomClark linked an issue Aug 11, 2020 that may be closed by this pull request

Fix annoying Qt5 deprecated warnings #5598

Closed

DomClark reviewed Aug 11, 2020

View reviewed changes

plugins/sf2_player/patches_dialog.h Outdated Show resolved Hide resolved

DomClark reviewed Aug 12, 2020

View reviewed changes

plugins/lb302/lb302.cpp Outdated Show resolved Hide resolved

DomClark approved these changes Aug 12, 2020

View reviewed changes

DomClark requested a review from JohannesLorenz August 12, 2020 14:21

uthidata added 3 commits August 29, 2020 21:00

Fix deprecation warnings from QWheelEvent

ff362cf

- replace QWheelEvent::delta with angleDelta - replace QWheelEvent::orientation with comparison of angleDelta.x and y

Fix deprecation warnings caused by QFlags

d40e394

- use QFlag(0) instead of 0 when appropriate or prompted - use explicit enum when specifying QStyle state in ComboBox's paintEvent

JohannesLorenz mentioned this pull request Sep 5, 2020

Reorganising LMMS, renaming editors and classes and everything in between (cont. #120) #5592

Closed

56 tasks

JohannesLorenz approved these changes Sep 5, 2020

View reviewed changes

PhysSong merged commit 3d8b310 into LMMS:master Sep 13, 2020

PhysSong mentioned this pull request Sep 13, 2020

Refactor Clipboard methods #5627

Merged

PhysSong reviewed Nov 30, 2020

View reviewed changes

PhysSong mentioned this pull request Nov 30, 2020

Fix crash on OGG export with Qt >= 5.10 #5813

Merged

Veratil mentioned this pull request Apr 23, 2022

Fix Qt Deprecations #6386

Merged

sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022

Qt deprecation fix (LMMS#5619)

b2d04ef

Qt6 TODO: Orientation check by comparing angleDelta().x() and y() won't make sense because the direction is arbitrary in Qt 6.

PhysSong mentioned this pull request Jan 17, 2023

Add Qt 6 support #6614

Open

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Qt deprecation fix #5619

Qt deprecation fix #5619

uthidata commented Aug 10, 2020

LmmsBot commented Aug 10, 2020 •

edited

Loading

Linux

Windows

macOS

PhysSong commented Aug 11, 2020

uthidata commented Aug 11, 2020

uthidata commented Aug 11, 2020

DomClark left a comment

JohannesLorenz commented Aug 13, 2020

DomClark commented Aug 13, 2020

PhysSong commented Aug 13, 2020

PhysSong commented Aug 14, 2020

PhysSong commented Aug 14, 2020

Veratil commented Aug 14, 2020

uthidata commented Aug 14, 2020

PhysSong commented Aug 14, 2020

uthidata commented Aug 15, 2020

PhysSong commented Aug 15, 2020

PhysSong commented Aug 15, 2020

uthidata commented Aug 15, 2020

PhysSong commented Aug 17, 2020

uthidata commented Aug 18, 2020

uthidata commented Aug 26, 2020

PhysSong commented Aug 27, 2020

uthidata commented Aug 29, 2020

JohannesLorenz commented Sep 5, 2020

uthidata commented Sep 5, 2020

JohannesLorenz commented Sep 5, 2020

PhysSong commented Sep 5, 2020

JohannesLorenz left a comment

PhysSong commented Sep 11, 2020

PhysSong Nov 30, 2020

JohannesLorenz Nov 30, 2020

		QRandomGenerator::global()->seed(time(0));
		m_serialNo = 0xD0000000 + QRandomGenerator::global()->generate() % 0x0FFFFFFF;

Qt deprecation fix #5619

Qt deprecation fix #5619

Conversation

uthidata commented Aug 10, 2020

LmmsBot commented Aug 10, 2020 • edited Loading

Linux

Windows

macOS

PhysSong commented Aug 11, 2020

uthidata commented Aug 11, 2020

uthidata commented Aug 11, 2020

DomClark left a comment

Choose a reason for hiding this comment

JohannesLorenz commented Aug 13, 2020

DomClark commented Aug 13, 2020

PhysSong commented Aug 13, 2020

PhysSong commented Aug 14, 2020

PhysSong commented Aug 14, 2020

Veratil commented Aug 14, 2020

uthidata commented Aug 14, 2020

PhysSong commented Aug 14, 2020

uthidata commented Aug 15, 2020

PhysSong commented Aug 15, 2020

PhysSong commented Aug 15, 2020

uthidata commented Aug 15, 2020

PhysSong commented Aug 17, 2020

uthidata commented Aug 18, 2020

uthidata commented Aug 26, 2020

PhysSong commented Aug 27, 2020

uthidata commented Aug 29, 2020

JohannesLorenz commented Sep 5, 2020

uthidata commented Sep 5, 2020

JohannesLorenz commented Sep 5, 2020

PhysSong commented Sep 5, 2020

JohannesLorenz left a comment

Choose a reason for hiding this comment

PhysSong commented Sep 11, 2020

PhysSong Nov 30, 2020

Choose a reason for hiding this comment

JohannesLorenz Nov 30, 2020

Choose a reason for hiding this comment

LmmsBot commented Aug 10, 2020 •

edited

Loading