Replace use of QGLWidget with QOpenGLWidget

[sgielen]

The QGLWidget class is deprecated (http://doc.qt.io/qt-5/qglwidget.html) and is causing problems with macOS rendering (https://bugreports.qt.io/browse/QTBUG-71292).

This pull request isn't ready to be merged as-is, because it removes some code that may be important, but it is a start. With this patch, Mixxx renders fine on my Mac and it also seems to solve the UI lag issue from before (although I still need to confirm that).

Feel free to use this patch as a starting point for your own work, or to give me pointers on how to improve it further for eventual merging.

[uklotzde]

Fedora/XWayland/Intel: Waveform display seems to be more choppy, movement is not as smooth as before. Maybe the missing vsync?

I didn't notice any other unwanted side effects.

[Be-ing]

I confirm that waveform movement is choppier than before, also with XWayland and Intel integrated graphics on Fedora.

[Be-ing]

Just some minor code style comments. I'm not familiar enough with this area of the code or OpenGL to comment further on the code, but I can test.

[Be-ing]

We conventionally put Qt includes above Mixxx includes.

[Be-ing]

Is the waveform scrolling smooth on macOS, or is it also jerky for you?

[ywwg]

the Shared Context code was needed so that all of the GL widgets would share a single OpenGL context (and therefore a single rendering pass I think) rather than having individual contexts for each widget (which multiples the rendering load I think). Having more contexts makes rendering much slower. Is there a reason this was removed?

[sgielen]

@Be-ing It is jerky for me too, but I've never seen it not being jerky; I always had lots of frame drops, so I can't really compare.

@ywwg QGLWidget has a constructor parameter to receive a context, but QOpenGLWidget does not; I don't know how to give QOpenGLWidgets shared contexts (to be fair, I didn't spend a lot of time to find out). So that would be the next step for this PR: figuring out how to do that (whether it's even necessary with QOpenGLWidget, or perhaps that's done automatically via the window widget?), and confirming that it fixes the jerkiness too.

[uklotzde]

"When multiple QOpenGLWidgets are added as children to the same top-level widget, their contexts will share with each other."

[Be-ing]

Replacing VSyncThread::swapGl(glw, i;) below with glw->context()->swapBuffers(window); does what was done before by VSyncThread::swapGl, but it does not solve the jerky waveform scrolling. (GitHub won't let me comment directly on that line because you have not changed it.)

[ywwg]

"When multiple QOpenGLWidgets are added as children to the same top-level widget, their contexts will share with each other."

cool, so it should be automatic!

[Be-ing]

I always had lots of frame drops, so I can't really compare.

Can you compare to the official 2.1.4 build?

[sgielen]

With the build you linked, it is not 100% smooth but pretty good: https://sjorsgielen.nl/Schermopname1.mov - if I set framerate limit to 60 it seems to stick just above 50. It says it's using OpenGL 2.1.

With the version I built, it uses OpenGL 4.1 and this is what it looks like with the framerate limit set to the same as the video above: https://sjorsgielen.nl/Schermopname2.mov - I'd say this is a similar smoothness, perhaps even a bit smoother. If I limit to 60 fps, it stays above 59 most of the time.

[Be-ing]

Interesting that 2.1.4 is so choppy for you. We have not had similar reports about that. What GPU are you using?

The second video is the same jerkiness I see on Linux with Intel integrated graphics.

[Be-ing]

Spinnies are broken:

[sgielen]

Also, I took another look at the UI lag issue, and I don't think this commit solves it.

When I run this branch with --safeMode, as I do in 2.2 because GL is broken for me there (the issue this PR works around, https://bugreports.qt.io/browse/QTBUG-71292), it is just as slow as 2.2 with --safeMode.

Only when I remove --safeMode, as I can do in this branch, is it actually fast again.

Here's a video of 2.2 with --safeMode, you can see how slowly Mixxx responds to mouse commands: https://sjorsgielen.nl/Recording-2.2-slow.mov

And here's a video of Mixxx with the one commit in this branch, first with --safeMode and then without: https://sjorsgielen.nl/Recording-branch.mov

[Be-ing]

We should probably move to using QOpenGLContext::versionFunctions instead of the old QGLFormat in the WaveformWidgetFactory constructor, but I don't know if we need to take care of that now before 2.2.0.

When I hardcode m_openGLShaderAvailable to true in the WaveformWidgetFactory constructor, that fixes my issue with the RGB (GLSL) renderer not appearing as an option in the preferences, and that renderer works. So perhaps we should update that old QGLFormat code.

[banad60]

hello developers,

i report https://bugs.launchpad.net/mixxx/+bug/1799637, because in bionic the spinnys do not work proper since qt5. May be this behavior belongs to this PR.

[Be-ing]

This is required for waveforms to work running with -platform wayland on Linux.

[banad60]

This is required for waveforms to work running with -platform wayland on Linux.
OK, but on ubuntu-18.04/16.04-platforms is still Xorg in use.

[Be-ing]

@sgielen do you think you'll have time to continue working on this soon?

[rryan]

@sgielen, sent you a PR: sgielen#1

These changes make GLSL renderers and spinnies work. I haven't tested on Linux or Windows yet.

There's still more to do: QOpenGLWidget makes the VsyncThread unnecessary, since QOpenGLWidgets are always off-screen surfaces that are composited at vsync intervals whenever a re-render of the backing store is requested.

We should stop rendering widgets when the VSyncThread thinks we should, and instead listen to QOpenGLWidget::aboutToCompose signals to know when to render. We need a thread like VsyncThread to drive rendering at a desired FPS though.

/cc: @daschuer

[daschuer]

Do we have a pending Mixxx bug for the issue? It is unclear for me how critical this one is. @sgielen would you mind to file two Bugs, one for de depricayed issue and one for the real issue you see with the old widgets. Thank you.

[daschuer]

The purpose of the vsync part is not mainly to vsync the waveforms. It is the ability to predict the swap moment. This is required to render the right audio sample that is passed to the soundcard DAC.

The old situation is somehow a working hack. Since QtWidgets are not designed for that. I think that a video rendered or a gaming engine has this audio sync integrated. Maybe we should consider to revamp waveforms on top of such technology.

If we continue this route we need to hack a new solution that predicts the swap.
Without it we will experience the waveform jerking.

[rryan]

Do we have a pending Mixxx bug for the issue? It is unclear for me how critical this one is. @sgielen would you mind to file two Bugs, one for de depricayed issue and one for the real issue you see with the old widgets. Thank you.

Here it is: https://bugs.launchpad.net/mixxx/+bug/1687937

[rryan]

one for de depricayed issue and one for the real issue you see with the old widgets. Thank you.

I agree deprecations are not critical. QGLWidget will surely be around until Qt 6, so we can still use it for a while with no problem.

I think there are real performance issues with the way we use QGLWidget and the way we swap them one by one. QOpenGLWidget introduces a much better approach to multiple GL widgets, by rendering them offscreen and compositing them into one surface at vsync intervals.

[rryan]

I filed https://bugs.launchpad.net/mixxx/+bug/1810099 that describes the critical issue.

[rryan]

The purpose of the vsync part is not mainly to vsync the waveforms. It is the ability to predict the swap moment. This is required to render the right audio sample that is passed to the soundcard DAC.

The old situation is somehow a working hack. Since QtWidgets are not designed for that. I think that a video rendered or a gaming engine has this audio sync integrated. Maybe we should consider to revamp waveforms on top of such technology.

If we continue this route we need to hack a new solution that predicts the swap.
Without it we will experience the waveform jerking.

Hm, good point. I think we can estimate the swap interval because QOpenGLWidget provides us signals when compositing is about to begin and when a swap happens (http://doc.qt.io/qt-5/qopenglwidget.html#frameSwapped).

Although, swapping doesn't truly correspond to "displayed on the screen", because on modern OSes, Mixxx itself is being rendered to an offscreen surface and composited with other applications, right?

QOpenGLWidget also introduces the ability to do thread-safe background rendering of a QOpenGLWidget, which was not possible with QGLWidget. This means we could do rendering in a background thread to free up the main thread.

[daschuer]

Although, swapping doesn't truly correspond to "displayed on the screen", because on modern OSes, Mixxx itself is being rendered to an offscreen surface and composited with other applications, right?

So you think there is a kind of fifo buffer for off screen renderings? I don't know about that. But anyway, the final swap of the screen buffer is the interesting moment for us. All multimedia applications need to be aware of that so I am sure we can get the info somehow.

[Be-ing]

QOpenGLWidget also introduces the ability to do thread-safe background rendering of a QOpenGLWidget, which was not possible with QGLWidget. This means we could do rendering in a background thread to free up the main thread.

Hmm, what if we had a separate thread for each waveform?

[rryan]

Although, swapping doesn't truly correspond to "displayed on the screen", because on modern OSes, Mixxx itself is being rendered to an offscreen surface and composited with other applications, right?

So you think there is a kind of fifo buffer for off screen renderings? I don't know about that. But anyway, the final swap of the screen buffer is the interesting moment for us. All multimedia applications need to be aware of that so I am sure we can get the info somehow.

I believe this is how Wayland works, and how macOS works. Not sure about Windows or X11. But the Compiz craze from the 2000s was all about compositing window managers.

[rryan]

the final swap of the screen buffer is the interesting moment for us. All multimedia applications need to be aware of that so I am sure we can get the info somehow.

I have a prototype I'll push soon that estimates the time to the next swap on a per-widget basis using a moving average of past renders -- it's working pretty well on macOS with 4 decks / 4 spinnies full screen.

[rryan]

Prototype is here: sgielen@116fb61
Only implemented for the GLSL widget for the moment.

[rryan]

Prototype is here: sgielen@116fb61
Only implemented for the GLSL widget for the moment.

With 4 decks, 4 spinnies, 60 FPS waveforms and 1.4ms latency, fullscreen on a 15" retina (2880 x 1800) display:

• my branch: 65% CPU usage, a tiny amount of interface lag
• 2.2.0: 90% CPU usage, really laggy and hard to use
• 2.1.6: 97% CPU usage, strangely variable lag (comes and goes), not as bad as 2.2.0 though

Setting latency to 90ms, the waveforms are still smooth, which suggests that the VisualPlayPosition interpolation based on the swap interval is working.

[Be-ing]

Closing in favor of #1974. Thanks for starting this @sgielen!