-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix crash when no GL context is available #11963
Conversation
…when double click right one the waveforms.
I guess this should be targeted to 2.4? |
src/mixxxmainwindow.cpp
Outdated
// note: the format is set in the WGLWidget's OpenGLWindow constructor | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment no longer applies since the context has been created earlier in the function
src/mixxxmainwindow.cpp
Outdated
initialize(); | ||
#endif | ||
QSurfaceFormat format; | ||
format.setVersion(2, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this, we had a single place where QSurfaceFormat was set (in OpenGLWindow constructor). Also, I liked going through an actual QOpenGLWindow creation to do the check. But I am okay with this change, especially because I agree with the eventual removal of the initial widget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may consider to set the default format in a single place instead. But I was not sure about the implications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be easy enough to solve by just putting the QSurfaceFormat creation into a function that is called from both places, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. please fix pre-commit before merge
… have all GL configurations in one place.
Done. |
This fixes #11929
A next iteration would be to get rid of the initial GL widget. Maybe we can already use the context without the widget.
I am just afraid that this requires a lot of testing. Nothing suitable just before a release.