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

Spinnies black on Windows 10 with Angle and Gallium #11930

Closed
daschuer opened this issue Sep 7, 2023 · 64 comments
Closed

Spinnies black on Windows 10 with Angle and Gallium #11930

daschuer opened this issue Sep 7, 2023 · 64 comments

Comments

@daschuer
Copy link
Member

daschuer commented Sep 7, 2023

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 daschuer added the bug label Sep 7, 2023
@ronso0 ronso0 changed the title Qt6: Spinnies back on Windows 10 Qt6: Spinnies black on Windows 10 Sep 7, 2023
@Swiftb0y
Copy link
Member

Swiftb0y commented Sep 7, 2023

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

back or black?

@daschuer
Copy link
Member Author

daschuer commented Sep 7, 2023

Upsi, black. Edited.

@JoergAtGithub
Copy link
Member

This is not a general problem, on my Windows 11 system with AMD GPU the Qt6 spinnies look good and work smooth:
grafik

@Swiftb0y
Copy link
Member

Swiftb0y commented Sep 7, 2023

This is not a general problem, on my Windows 11 system with AMD GPU the Qt6 spinnies look good and work smooth:

Thanks for confirming. I suspected that the virtualization was at fault here. I guess the next step would to be try gallium on linux and see if the issue persists.

@daschuer
Copy link
Member Author

daschuer commented Sep 8, 2023

When I replace the GLSL Spinnies here by the non GL Types, it works:
daschuer@1c5f6f0

@Swiftb0y
Copy link
Member

Swiftb0y commented Sep 8, 2023

difficult to debug why it fails... especially when the glsl waveforms still work... nothing suspicious in the log?

@daschuer
Copy link
Member Author

daschuer commented Sep 10, 2023

I have updated to
4.5 (Compatibility Profile) Mesa 23.0.1 (llvmpipe (LLVM 15.0.7, 256 bits))
The latest opengl32sw.dll + liglapi.dll + libgallium_wgl.dll from VCPKG and it works everything with no regressions.

The only issue is that this extends the 2.5-rel package by 2 GB and exceeds the GitHub build time limits of 6 hours by far.

Since it is a plug-In DLL, we don't need anything else from the 2 GB than the three DLLs of together 44.1 MB (packed 16,7 MB)
Our windows installer is 83 MB

I can imagine to set up an extra VCPKG build, that produces these DLLs and just add them to the windows installer.
This will not be relevant for 2.5 with the QWindgets interface but is a prerequisite for QML on non GL machines.

What do you think?

@daschuer
Copy link
Member Author

daschuer commented Sep 10, 2023

By the way, with the same DLLs and the 2.4-beta the spinnies are still broken, not black, but wit artefacts:

grafik

So I guess there is something tricky going on with the GLSL spinnies.
The Mixxx log looks good:

Debug [Main] OpenGL driver version string "4.5 (Compatibility Profile) Mesa 23.0.1", vendor "Mesa", renderer "llvmpipe (LLVM 15.0.7, 256 bits)"
Debug [Main] Supported OpenGL version: 4.5

@Swiftb0y
Copy link
Member

This will not be relevant for 2.5 with the QWindgets interface but is a prerequisite for QML on non GL machines.

This will all be irrelevant for the QML UI anyways since that runs on QRhi and thus uses the native APIs. IIUC the problem here is only relevant for Qt6 without QML (so 2.5 and (hopefully not) later).

I can imagine to set up an extra VCPKG build, that produces these DLLs and just add them to the windows installer.

I don't know much about packaging so I can't give my opinion on that. But yes, the best way would probably to ship the up-to-date translation layer while avoiding the size and build-time overhead somehow, not sure how best to do that though.

@Swiftb0y
Copy link
Member

At least we now confirmed this to be a driver issue...

@daschuer
Copy link
Member Author

At least we now confirmed this to be a driver issue...

This conclusion is not 100 % confirmed. For me it is a combination of both.

  • GLSL Waveforms work with any drivers.
  • GLSL Spinny:
    • Work with QT6 and recent Mesa 23.0.1
    • Don't work with QT5 and recent Mesa 23.0.1
    • Don't work with QT6 and outdated Mesa 12.0.0-rc2

Conclusion: There is something fishy in the GLSL Spinnies that puts high demands on the Gallium Mesa Diver AND on the QT framework. Since we have many different drivers and GPUs out there, it would be good to remove that part from the GLSL and lower the requirements to the same level as the GLSL Waveforms.

@Swiftb0y
Copy link
Member

it would be good to remove that part from the GLSL and lower the requirements to the same level as the GLSL Waveforms.

I agree, however from what I've double checked already, they should have the same requirements pretty much.
IIRC Mesa has validation layers, have you tried to enable them to see whether it can tell us whats wrong?

@daschuer
Copy link
Member Author

Not yet, all that is quite cumbersome because I am not at windows and need to do everything remote, and on a virtual machine. I hope a windows contributor can take over.

Now the Qt6 version has also the artefacts. :-/

@daschuer
Copy link
Member Author

@m0dB do you have an idea what we can do here? Something like minimal GLSL testing spinnie to check if GLSL is working at all?

For now I think we should just disable GLSL spinnies on windows by default and proceed to the other issues and finally release 2.4. The GL Spinnie was never a bottle neck on Windows.

What do you think?

@Swiftb0y
Copy link
Member

alternatively, asking the mesa team for help could also be an option. They usually don't seem to have problems investigating mesa-related GUI issues in applications...

@daschuer
Copy link
Member Author

Yes, this should do one with a windows build environment.

@JoergAtGithub
Copy link
Member

How should somebody on Windows debug a bug in the Mesa driver for Linux? For me on native Windows the spinnies work flawless.

@daschuer
Copy link
Member Author

daschuer commented Sep 11, 2023

This is the Mesa driver on Windows, uses as a opengl32sw.dll in case the windows driver is rejected

@m0dB
Copy link
Contributor

m0dB commented Sep 11, 2023

This is very strange. The shader used in WSpinnyGLSL, mixxx::TextureShader, is also used in all the GLSL-based Waveform types (in WaveformRenderMark). I will compare the code of both locations to see if I spot something that could explain this.

There is another shader used in WSpinnyGLSL, mixxx::VinylQualityShader, which is only used there. It shouldn't be used if vinyl quality isn't enabled, but it is always instantiated,so it might be good to comment m_vinylQualityShader out entirely to see if that makes a difference.

While looking at the code, it did spot two erroneous calls to m_textureShader.setAttributeArray instead of m_vinylQualityShader.setAttributeArray in WSpinnyGLSL::drawVinylQuality. I will create a PR with a fix.

@daschuer
Copy link
Member Author

On Ubuntu Focal, the spinnies are working with:
3.1 Mesa 21.2.6 (llvmpipe (LLVM 12.0.0, 256 bits))

The issue is that I see lost frames on 60 Hz. So my conclusion is that providing the Gallium (llvmpipe) driver on Windows has no benefit on for the waveforms, but the it allows spinnies. ,

@m0dB
Copy link
Contributor

m0dB commented Sep 13, 2023

Could you please rephrase that?

@daschuer
Copy link
Member Author

Situation on windows without sufficient GL support:

Angle (direct 3d):

  • gpu waveforms, No spinnies

Mesa Gallium:

  • Heavy to build and ship
  • Additional waveforms perform bad.
  • Spinnies broken

Idea to do:

  • Fix Angle GLSL ES waveforms
  • Fix Angle GLSL ES spinnies
  • Introduce a CPU software spinnie.

We have no benefit from gallium if we provide a CPU solution for all widgets.

ES support is also desired for Raspberry Pi and friends.

@daschuer
Copy link
Member Author

This issue happens also with Qt5 Mixxx 2.4-beta and ANGLE on the same VBox Windows 10.

@daschuer daschuer added this to the 2.4.0 milestone Sep 14, 2023
@daschuer daschuer changed the title Qt6: Spinnies black on Windows 10 Spinnies black on Windows 10 with Angle and Gallium Sep 14, 2023
@daschuer daschuer mentioned this issue Sep 14, 2023
@m0dB
Copy link
Contributor

m0dB commented Sep 14, 2023

I think the way to find the issue is to start with something minimal and gradually add features to the spinny and see when it fails. It must be something silly. I will prepare something this weekend but I'll need someone else to test it.

@m0dB
Copy link
Contributor

m0dB commented Sep 15, 2023

@daschuer can you try with my last changes? they add to command line options

--skipSpinnyTextures and --skipSpinnyVinylQuality

Please try with both, and with either one.

It also reduces the number of states to click through.

@JoergAtGithub
Copy link
Member

JoergAtGithub commented Sep 15, 2023

I tried this in a slightly modified setup, where I can run OpenGL ES under Angle - which is affected by this bug.
With angle the spinnies are always black, independent of the option:
grafik
with normal OpenGL, they are colored (red before I click on it):
grafik

@daschuer
Copy link
Member Author

With mixxx-2.4-alpha-1657-gd89e8995fb.msi the new flags have no effect.
All combinations have the visual moving artefacts.

How about using the GL Types on Windows as a workaround until we have a solution?
Or introduce CPU Spinnies? Is the later just a matter of changing the wiget type?

This way we decouple this Isse from the release date.

@m0dB
Copy link
Contributor

m0dB commented Sep 17, 2023

How about using the GL Types on Windows as a workaround until we have a solution?

Not sure what you mean by that. What does that have to do with the spinnies?

Or introduce CPU Spinnies? Is the later just a matter of changing the wiget type?

Only for Windows, right?

As I don't understand why this is not working, I am not sure if changing the widget type is sufficient. But I can add a command line option to try this.

@JoergAtGithub
Copy link
Member

Only for Windows, right?

As I don't understand why this is not working, I am not sure if changing the widget type is sufficient. But I can add a command line option to try this.

The problem only occurs on old Windows systems with broken OpenGL drivers. In that case Qt5 falls back to ANGLE OpenGL emulation. ANGLE supports OpenGL ES only, and OpenGL ES shows the broken(black) spinnies.
I needed to enforce this fallback on my system by setting $env:QT_OPENGL = 'angle'.

For Windows we could discontinue ANGLE support easily - because it's not supported by Qt6 anyway. But AFAIK OpenGL ES is also needed for real Embedded Systems like a Raspberry Pi. This is not needed to be supported in the official Mixxx packages of cause, but some users compile it for these platforms themself.

@m0dB
Copy link
Contributor

m0dB commented Sep 17, 2023

But I don't understand why the waveforms do render correctly, and why the Spinnies, with even the most basic OpenGL - without any shaders, just calling glClear with a glClearColor, do not work. That makes me think that the maybe the problem is that the QOpenGLWindow based widget does not work with small windows with ANGLE? In any case, adding some command line options to force the use of the old spinnies might give more insight. But this would still be QOpenGLWindow based.

@daschuer
Copy link
Member Author

How about using the GL Types on Windows as a workaround until we have a solution?

Not sure what you mean by that. What does that have to do with the spinnies?

The 2.3 non Shader Spinies work. Can we just use them on windows and carry on?

Or introduce CPU Spinnies? Is the later just a matter of changing the wiget type?

Only for Windows, right?

Yes, I am not sure about the side effects so ...

As I don't understand why this is not working, I am not sure if changing the widget type is sufficient. But I can add a command line option to try this.

Of cause understanding the root cause would be the best foundation. But If this releases some pressure form us it is welcome. Just as an idea.

@daschuer
Copy link
Member Author

That makes me think that the maybe the problem is that the QOpenGLWindow based widget does not work with small windows with ANGLE?

The issue also happens with the OpenGL Gallium (non ES) software rendering driver that should be entirely hardware independent.
Interestingly it fails however only on windows and not on Linux. So Windows or MSVC is also part of the game.
The Galium HUD is by the way rendered correctly in the spinnie areas on top of the garbage.

@JoergAtGithub
Copy link
Member

I tried to start with --enable-vumetergl and than the VUMeter are also black:
grafik

@m0dB
Copy link
Contributor

m0dB commented Sep 17, 2023

@daschuer can you try https://github.com/m0dB/mixxx/tree/wspinnyoption and run with
--enable-legacy-spinny ?

@daschuer
Copy link
Member Author

Works on Windows 10 Virtual Box 7 with Angle and MESA GALLIUM. Without the flag both is broken.

How to continue? It looks like the legacy version is good enough on windows and I propose to always use the legacy spinny but keep a flag that allows to switch to the Shader version. This should offer a good experience by default.
What do you think?

@JoergAtGithub
Copy link
Member

I don't think we should change this short before release. The current version is confirmed to be working by every Windows test user I know.
Only with Virtual Box and ANGLE the problem occurs, therefore we should change it only for these special setups.

@daschuer
Copy link
Member Author

But you could confirm the black spinny on a native install using ANGLE as well, right?
And we had never spinny issues with 2.3 on windows using the GL No-Shader version.
From that point of view I consider it better tested than the Shader version and a good band aid to lower the risk.

Do you experience any notable performance gain with the new spinnys ?

@JoergAtGithub
Copy link
Member

No, performance is the same. But the combination of old and new is completly new and untested. I consider this as too risky to solve such a rare case.

@daschuer
Copy link
Member Author

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
Copy link
Contributor

m0dB commented Sep 18, 2023

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
Copy link
Member Author

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
Copy link
Contributor

m0dB commented Sep 18, 2023

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
Copy link
Contributor

m0dB commented Sep 18, 2023

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

@m0dB
Copy link
Contributor

m0dB commented Sep 19, 2023

@daschuer
Copy link
Member Author

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

Some thoughts:

  • Does this probably help with the VU-Meter issue where we default to the software version?
  • The mesa gallium GL shader works less good than the pure software implementation

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

@JoergAtGithub
Copy link
Member

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
Copy link
Contributor

m0dB commented Sep 21, 2023

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
Copy link
Member Author

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
Copy link
Contributor

m0dB commented Sep 22, 2023

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
Copy link
Member

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
Copy link
Contributor

m0dB commented Sep 22, 2023

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

@m0dB
Copy link
Contributor

m0dB commented Sep 23, 2023

PR created

@m0dB m0dB closed this as completed Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants