mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.48k stars 1.27k forks source link

Spinnies black on Windows 10 with Angle and Gallium #11930

Closed daschuer closed 1 year ago

daschuer commented 1 year ago

Bug Description

The spinnies are black. All Waveform Types are working. Legacy GL types are loosing frames.

OpenGL-Version: 3.0 Mesa 12.0.0-rc2 (Gallium 0.4 on llvmpipe (LLVM 3.6, 256 bits))

With --enable-vumetergl, the VU-Meters are black as well.

In Virtual Box 7.0

Version

2.5.0-alpha

OS

Windows 10

daschuer commented 1 year ago

Maybe @m0dB can confirm, but for my understanding it is just not using the Shafers but based on the QOpenGlWidget like the legacy GL waveforms.

I consider that something is broken with the GSLS types, so it would be the best to solve that root cause, but since it will probably thakr some time, we need a band aid.

An alternative would be to merge the flag as it is and tell all users that have a spinny issue to set it. Some of them will never complain and use Mixxx without spinnies.

We have also the situation (like in 2.3) that users without GL have No spinnies at all. Is that something we may consider here as well, introducing a software spinnie, that will likely outperform the MESA Gallium glsl type? This is a Qt 6 topic anyway where we have no ANGLE driver without patching it back in.

m0dB commented 1 year ago

It is interesting, to say the least, that this works while the most basic GL (without GLSL) of calling glClear doesn't while the waveforms do. I will look into this once more to see if there is a significant difference with how the waveforms code is called. Maybe some missing initialisation.

I am not near my laptop though so this will have to wait a bit.

In any case, as I understand, this is quite corner case and for most users it will work, right? I'd go for simply adding the command line option, also for the reasons @JoergAtGithub mentions.

daschuer commented 1 year ago

OK, that is at least a workaround that can be used instantly. The useres will tell us if this was a good decision or not and we can change it any time later. Can you prepare a PR for that?

Maybe the issue is cause by the widget stack? We may create a skin with only a spinnie and check if the issue persists. @JoergAtGithub do you have time to test that?

m0dB commented 1 year ago

Hold your horses!

I seems that, contrary to the waveform renderers, initializeOpenGLFunctions() is not called for WSpinnyGLSL! This certainly would explain what we are seeing.

Note that I found this browsing the source code on my phone so I might have overlooked something. I will add the call to this branch later on tonight so you can try if it makes a difference.

m0dB commented 1 year ago

Sorry, no time. I will do this tomorrow. Or if you want to try, add the call to initializeOpenGLFunctions in WSpinnyGLSL::initializeGL

m0dB commented 1 year ago

@daschuer @JoergAtGithub changes commited to https://github.com/m0dB/mixxx/tree/wspinnyoption

daschuer commented 1 year ago

Juchhu :tada: it works, on both also with --enable-vumetergl.

Some thoughts:

Does it make sense to replace the No Shader GL spinnie with a software version? Shall we default to GL VU_meters?

JoergAtGithub commented 1 year ago

Juchhu 🎉 it works, on both also with --enable-vumetergl.

Can confirm this with ANGLE on Windows11

* Does this probably help with the VU-Meter issue where we default to the software version?

It seems, that GL VU Meter are now also fast on Windows - no more GUI lag. But this needs confirmation by other users too.

m0dB commented 1 year ago

On macOS as well it is smooth. I propose the following: I turn this branch into a PR that makes the GLSL based spinny and vu meters the default and adds command line options to revert to the legacy spinny and the legacy vu meter. What do you think?

daschuer commented 1 year ago

That works for me.

The alternative with a little more flexibility would be to replace the GL Spinnie that has probably no use case anymore with a software based type. It will probably behave better than the Gallium fallback once we switch to QT6 where we have no ANGLE anymore (unless we use the angle patches).

If it is really only a matter of placing the inherited base widget, this should be not much of a deal. If the efforts are bigger we may rely on Gallium and spend our time elsewhere.

m0dB commented 1 year ago

I am not sure what you mean with "the GL Spinnie".

We have WSpinny and WSpinnyGLSL both derived from WSpinnyBase, which in turn is derived from the WGLWidget. WSpinny does all its drawing with QPainter, but on the QOpenGLWindow as paint device, because it uses swapBuffer.

We could change WSpinny so it doesn't use a QOpenGLWindow, but that's not trivial, because of the baseclass that it shares with WSpinnyGLSL and because we would need to replace the swapBuffers call with a repaint or update.

We could make the use of QOpenGLWindow in WGLWidget optional. That way WSpinny and WSpinnyGLSL could still use the same base class. Or we could derive WSpinny from QWidget and WGLWidget from WSpinnyGLSL respectively, and use the common code from WSPinnyBase in both either through multiple inheritence or encapsulation.

I am not sure though if any of this is worth the hassle and the risk so short before release.

JoergAtGithub commented 1 year ago

Let's not to major changes before the release of 2.4 !!! Just merge this at it is and change the Windows default for GL-VU-Meters.

m0dB commented 1 year ago

Yes, I agree! But I wanted to expose the situation to clarify what I think @daschuer is suggesting.

m0dB commented 1 year ago

PR created