Skip to content
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

replace qstylepainter with qpainter in vumeterlegacy, use by default #11722

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

m0dB
Copy link
Contributor

@m0dB m0dB commented Jul 8, 2023

The GL-based vumeters reportedly cause lag on Windows, but also on macOS I notice they take relatively a lot of time (maybe QOpenGLWindow doesn't play nice with multiple small windows?)

This PR reenables the legacy VuMeter by default, replacing the command line option --disable-vumetergl with --enable-vumetergl. It has one important change which improved it's behaviour on macOS drastically: using QPainter instead of QStylePainter. The background color is determined in the same way as the GL VuMeter.

Also on macOS this seem to behave better than when using the GL-based VuMeter. At least not worse. If we confirm this works well for everyone on all platforms, I propose to remove the GL VuMeter code altogether, as well as the command line option.

@JoergAtGithub
Copy link
Member

I tested this PR on Windows11 and first struggled to reproduce the lag as @NotYourAverageAl in the previous PR. I found out that it doesn't occur with every buildenv version. Luckily windows_buildenv.bat creates backup files of old CMakeSettings.json and I could restore the affected state.

The result of my testing is, that this PR with works smooth and flawless on my Windows11 system if I start mixxx.exe without further options. If I set --enable-vumetergl I see the lag again.

Furthermore I noticed, that using vumetergl always increase the CPU load and the GPU load significiantly.

Therefore my conclusion is, that this PR is the right thing for Windows 11. But how does it perform for Linux and especially macOS users?
@fwcd Could you test this on your Mac?

@daschuer
Copy link
Member

daschuer commented Jul 9, 2023

Does all this also apply to the 2.3 branch?
Maybe we should just add a one-liner that that disables GL VU-Meters on windows?

(I am currently building this on Ubuntu Focal)

@fwcd
Copy link
Member

fwcd commented Jul 9, 2023

macOS ARM seems to run pretty well too, though admittedly I couldn't notice a big difference in performance/responsiveness between default and --enable-vumetergl

@JoergAtGithub
Copy link
Member

Do you see a difference in CPU or GPU load?

@fwcd
Copy link
Member

fwcd commented Jul 9, 2023

Very slightly perhaps, but then only negligibly so.

Screenshot 2023-07-09 at 19 47 13

@daschuer
Copy link
Member

daschuer commented Jul 9, 2023

First of all, all configurations are running smooth. The noise in the value is at the same magnitude as the difference, so these values can only show a tendency.

On Ubuntu Focal I see with "intel_gpu_top" and "top" four decks playing
with this branch
52 % GPU and 50 % CPU

with --enable-vumetergl
57 % GPU and 56 % CPU

plain 2.4
52 % GPU 55 % CPU

2.4 with --disable-vumeter
47 % GPU 54 % CPU

plain 2.3
54 % GPU 55 % CPU

2.3 with --disable-vumeter
53 % GPU 57 % CPU

Are these measures reasonable?

Conclusion this one give the best performance over all! Thank you.

@JoergAtGithub
Copy link
Member

Than let's merge this! Proved on all three platforms equal or better than with VUMeterGL.
Thank you!

@JoergAtGithub JoergAtGithub merged commit cb30b1d into mixxxdj:2.4 Jul 9, 2023
@daschuer
Copy link
Member

daschuer commented Jul 9, 2023

With 2.3 with --disable-vumeter has the opposite effect on windows and macOS compared to what we see here.

How is the situation on Windows? Is there anything to do on the windows 2.3 branch?

@JoergAtGithub
Copy link
Member

On macOS and 2.3 VUMeterGL is reported to be much faster. But not on Windows, there some users see UI lag.

@m0dB
Copy link
Contributor Author

m0dB commented Jul 9, 2023

With 2.3 with --disable-vumeter has the opposite effect on windows and macOS compared to what we see here.

But that is, as far as I have seen with 2.4, because of the use of QStylePainter. It could be interesting to try 2.3 with the --disable-vumetergl and replacing QStylePainter with QPainter as done in this PR. But I don't know if that's worth the hassle. The real improvement is 2.4 anyway.

@m0dB
Copy link
Contributor Author

m0dB commented Jul 9, 2023

Than let's merge this! Proved on all three platforms equal or better than with VUMeterGL.
Thank you!

Great! It's been an interesting ride: the GL based VU meter being the first step in improving performance on macOS and now that we have the QOpenGLWindow/GLSL based waveforms and spinnies, being able to go back to the original VU meters (with a small tweak). Full circle :-)

@daschuer
Copy link
Member

daschuer commented Jul 9, 2023

But I don't know if that's worth the hassle.

It is definitely not worth to spend hours of refactoring. The only thing is IMHO worth to check if "--disable-vumeter" should be the default on Windows.

@foss-
Copy link
Contributor

foss- commented Jul 10, 2023

Tested on MacBookPro 14,3 Intel mac. Could not spot a big difference in performance comparing a recent main build and todays 2.4 beta build.

2.5 main:
2023-07-10 2 5 main build
2.4 beta:
2023-07-10 2 4 with this patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants