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

Opt to use OpenGL 2.0 instead of software rendered OpenGL 4.1 #478

Merged

Conversation

@fieldOfView
Copy link
Contributor

commented Apr 4, 2019

This PR checks if the best detected OpenGL version isn't using software rendering provided by the OS instead of a GPU renderer. With newer versions of Qt on OSX (and Linuxes ?), requesting a 4.1 context on a GPU that does not support hardware accelerated 4.1 rendering results in a software rendered context which is much slower than a hardware accelerated 2.0 context. This PR detects software rendering OpenGL 4.1 renderers, and falls back to OpenGL 2.0 instead.

Contributes to Ultimaker/Cura#4505 (CURA-6092)

This will need testing on many systems.

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

I had a choice to "contaminate" QtApplication with some OpenGL, or "contaminate" OpenGLContext with some Qt. I chose the former.

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

You're hive mind is strong here. I was just typing a message as to why you chose to do it like this.

I was looking at the same solution, but I was thinking about doing it in the openGL functions that we had. I will do a bit more poking around to see if we can get the best of both worlds here.

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Shit. It seems that this isn't a solution we can take :(

The line gl = QOpenGLContext.currentContext().versionFunctions() is causing a crash for me (ModuleNotFoundError: No module named 'PyQt5._QOpenGLFunctions_4_5_Core'). I really hope this is just an issue with my setup...

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

I have found a way to keep it in the 'detectBestOpenGLVersion' though, but I can't actually run it due to the crash :(

@fieldOfView fieldOfView force-pushed the fieldOfView:fix_software_rendered_41_context branch from ce65314 to b07be95 Apr 4, 2019
@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

I am now using QOpenGLContext.currentContext().versionFunctions() the same way OpenGL.py does it. Could you try again?

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Nope, still nothing :( ModuleNotFoundError: No module named 'PyQt5._QOpenGLFunctions_4_5_Core'

I've also tried to use ctx.versionFunctions() and ctx.versionFunctions(gl_profile) , but neither work.

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

What version of PyQt are you using? I'm getting the feeling it has to do with that.

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

I'm using 5.10.

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Hmm. That must be it then, I'm on 5.11.3

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

Cura uses 5.10 though, right?
Still good to know this could cause issues on 5.11.

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Yeah, it does. But this was the only version I could get to run with qml debugging. I could downgrade back again, but that's just going to postpone the problem until we do...

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

But at least it would tell us if it is indeed the PyQt version.

Note that I don't understand how it fails. OpenGL.py does exactly the same here: https://github.com/Ultimaker/Uranium/blob/master/UM/View/GL/OpenGL.py#L54

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Yeah, I'm also at a loss here. I guess it has something to do with the moment when it happens. The openGL object is created on the first render call from the window (as the first time render is called, it creates a qtRenderer, which in turn creates that object).

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

In that case, could you try changing gl_widget.showMinimized() to gl_widget.show(), because then the window actually gets painted.

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Tried that one already, it didn't matter. I still get the same result

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Using PyOpenGL does seem to do the trick though. But that would mean that we'd have to add an extra dependency for just this....

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

I think I found the issue:
https://riverbankcomputing.com/pipermail/pyqt/2017-January/038640.html

PyQt currently only implements 2.0, 2.1 and 4.1Core

Looks like the detection finds the possibility to use 4.5Core on your system, but then PyQt cannot handle that context/version when we try to create it.

I've changed the code to always create the test-context with 4.1 even if something >4.1 is detected.

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

That did the trick, no more crashes.

@thopiekar

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

And what happens if I run this in a virtual machine (KVM)? There I want software rendering if I have no 3D accelerator. Or is Qt providing its own renderer?

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

The PR only rejects software 4.1 contexts. If there is only a software 4.1 renderer, the application continues to check if a 2.1 context is available. There is no check if the 2.1 context uses software rendering. So in your VM scenario, you end up with a software rendered gl 2.1 context.

In my testing, a software rendered 2.1 context performs better than a software 4.1 context (especially in Layer View, where the geometry shader does not have to draw in software mode).

@thopiekar

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

Great! Thank you for your feedback! 😃

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 5, 2019

I will see if I can create a 4.0 build with a backport of this fix this weekend, for affected OSX users to test (and use).

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 7, 2019

Looks like this needs a bit more work when running "frozen" on OSX.

@LipuFei

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

@fieldOfView I've created a branch from yours and I will create Cura builds with it. Branch: https://github.com/Ultimaker/Uranium/tree/CURA-6092_aldo_opengl_fix_mac

@LipuFei

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

@fieldOfView I got a crash on my Mac (Mojave 10.14.3) :(

Traceback (most recent call last):
  File "<frozen importlib._bootstrap>", line 969, in _find_and_load
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 664, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 634, in _load_backward_compatible
  File "/Users/ultimaker/build/env/master/inst/lib/python3.5/site-packages/cx_Freeze/initscripts/__startup__.py", line 12, in <module>
  File "<frozen importlib._bootstrap>", line 969, in _find_and_load
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 664, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 634, in _load_backward_compatible
  File "/Users/ultimaker/build/env/master/inst/lib/python3.5/site-packages/cx_Freeze/initscripts/Console.py", line 21, in <module>
  File "<frozen importlib._bootstrap>", line 969, in _find_and_load
  File "<frozen importlib._bootstrap>", line 958, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 664, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 634, in _load_backward_compatible
  File "/Users/ultimaker/build/master/build/inst/bin/cura_app.py", line 136, in <module>
  File "/Users/ultimaker/build/master/build/inst/lib/python3.5/site-packages/cura/CuraApplication.py", line 716, in run
  File "/Users/ultimaker/build/master/build/inst/lib/python3.5/site-packages/UM/Qt/QtApplication.py", line 291, in run
  File "/Users/ultimaker/build/master/build/inst/lib/python3.5/site-packages/UM/Application.py", line 209, in run
  File "/Users/ultimaker/build/master/build/inst/lib/python3.5/site-packages/cura/CuraApplication.py", line 316, in initialize
  File "/Users/ultimaker/build/master/build/inst/lib/python3.5/site-packages/UM/Qt/QtApplication.py", line 131, in initialize
  File "/Users/ultimaker/build/master/build/inst/lib/python3.5/site-packages/UM/View/GL/OpenGLContext.py", line 135, in detectBestOpenGLVersion
AttributeError: 'NoneType' object has no attribute 'initializeOpenGLFunctions'

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

I do have an idea for a band-aid solution to the issue; We can also add a message that if the 4.1 context is selected with a software renderer, it warns the user that this is the case and provides a button / action to force the compatibility mode.

It isn't as elegant as the automatic solution, but it's pretty much guaranteed to work (since it's just adding a message and doesn't need any "fundamental" changes to our detection)

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

@LipuFei, that's what I meant with that it needs some work. Apparently opening the hidden window does not let Qt initialise OpenGL on OSX (it does on Windows).

What GPU does your Mac have? Does it normally have hardware accelerated OpenGL 4.1?

@nallath

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

We only have macs that have a hardware accelerated 4.1 OpenGL (which is why we didnt find the issue in the first place).

Using a QOpenGLWidget did not work for a 4.1 context on OSX
@fieldOfView fieldOfView force-pushed the fieldOfView:fix_software_rendered_41_context branch from cbd14b9 to 5982274 Apr 11, 2019
@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

I have found a better way to initialize OpenGL to get the renderer, which also works on OSX.

This PR works properly on my OS X VM, which now correctly creates a (software) 2.0 context instead of a software 4.1 context. Would be nice to know if it still creates a 4.1 context on real hardware that supports it.

@LipuFei

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@fieldOfView 👍 This works on my Mac (Mojave 10.14.4).

@fieldOfView

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

🍾

Reports here are also favorable for macs that are affected by the issue (with 4.1-challenged gpus)
Ultimaker/Cura#4505 (comment)

I have not tested this latest iteration on Linux yet.

@diegopradogesto

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

I checked on macOS as well and it seems to work fine. @nallath checked on Linux and said it was OK.

Thanks for contributing on fixing this issue.

@diegopradogesto diegopradogesto merged commit 26791d6 into Ultimaker:master Apr 15, 2019
1 check passed
1 check passed
continuous-integration/jenkins/pr-merge This commit looks good
Details
@fieldOfView fieldOfView deleted the fieldOfView:fix_software_rendered_41_context branch Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.