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

Fix crash when no GL context is available #11963

Merged
merged 7 commits into from Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 39 additions & 22 deletions src/mixxxmainwindow.cpp
@@ -1,22 +1,25 @@
#include "mixxxmainwindow.h"

#include <QDebug>
#include <QDesktopServices>
#include <QFileDialog>
#include <QOpenGLContext>
#include <QUrl>

#include "widget/wglwidget.h"

#ifdef MIXXX_USE_QOPENGL
#include "widget/tooltipqopengl.h"
#include "widget/winitialglwidget.h"
#else
#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
#include <QGLFormat>
#endif

#include <QUrl>
#ifdef __LINUX__
#include <QtDBus>
#endif
#include <QtDebug>

#include "widget/wglwidget.h"

#ifdef MIXXX_USE_QOPENGL
#include "widget/tooltipqopengl.h"
#include "widget/winitialglwidget.h"
#endif

#include "dialog/dlgabout.h"
#include "dialog/dlgdevelopertools.h"
Expand Down Expand Up @@ -149,22 +152,36 @@ MixxxMainWindow::MixxxMainWindow(std::shared_ptr<mixxx::CoreServices> pCoreServi

#ifdef MIXXX_USE_QOPENGL
void MixxxMainWindow::initializeQOpenGL() {
#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
// Qt 6 will nno longer crash if no GL is available and
// QGLFormat::hasOpenGL() has been removed.
if (!CmdlineArgs::Instance().getSafeMode() && QGLFormat::hasOpenGL()) {
#else
if (!CmdlineArgs::Instance().getSafeMode()) {
// This widget and its QOpenGLWindow will be used to query QOpenGL
// information (version, driver, etc) in WaveformWidgetFactory.
// The "SharedGLContext" terminology here doesn't really apply,
// but allows us to take advantage of the existing classes.
WInitialGLWidget* widget = new WInitialGLWidget(this);
widget->setGeometry(QRect(0, 0, 3, 3));
SharedGLContext::setWidget(widget);
// When the widget's QOpenGLWindow has been initialized, we continue
// with the actual initialization
connect(widget, &WInitialGLWidget::onInitialized, this, &MixxxMainWindow::initialize);
widget->show();
// note: the format is set in the WGLWidget's OpenGLWindow constructor
} else {
initialize();
#endif
QSurfaceFormat format;
format.setVersion(2, 1);
Copy link
Contributor

@m0dB m0dB Sep 11, 2023

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.

Copy link
Member Author

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.

Copy link
Member

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?

format.setProfile(QSurfaceFormat::CoreProfile);
QOpenGLContext context;
context.setFormat(format);
if (context.create()) {
// This widget and its QOpenGLWindow will be used to query QOpenGL
// information (version, driver, etc) in WaveformWidgetFactory.
// The "SharedGLContext" terminology here doesn't really apply,
// but allows us to take advantage of the existing classes.
WInitialGLWidget* widget = new WInitialGLWidget(this);
widget->setGeometry(QRect(0, 0, 3, 3));
SharedGLContext::setWidget(widget);
// When the widget's QOpenGLWindow has been initialized, we continue
// with the actual initialization
connect(widget, &WInitialGLWidget::onInitialized, this, &MixxxMainWindow::initialize);
widget->show();
// note: the format is set in the WGLWidget's OpenGLWindow constructor
return;
Copy link
Member

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

}
}
// Initialize without OpenGL.
initialize();
}
#endif

Expand Down
5 changes: 1 addition & 4 deletions src/waveform/sharedglcontext.cpp
Expand Up @@ -14,10 +14,7 @@ WGLWidget* SharedGLContext::s_pSharedGLWidget = nullptr;
// static
void SharedGLContext::setWidget(WGLWidget* pWidget) {
s_pSharedGLWidget = pWidget;
#ifdef MIXXX_USE_QOPENGL
qDebug() << "Set root GL Context widget valid:"
<< pWidget << (pWidget && pWidget->isContextValid());
#else
#ifndef MIXXX_USE_QOPENGL
qDebug() << "Set root GL Context widget valid:"
<< pWidget << (pWidget && pWidget->isValid());
if (pWidget) {
Expand Down
16 changes: 8 additions & 8 deletions src/waveform/waveformwidgetfactory.cpp
Expand Up @@ -174,7 +174,6 @@ WaveformWidgetFactory::WaveformWidgetFactory()
m_openGLVersion += majorVersion == 0 ? QString("None") : versionString;

if (majorVersion * 100 + minorVersion >= 201) {
m_openGlAvailable = true;
if (pContext->isOpenGLES()) {
m_openGlesAvailable = true;
} else {
Expand All @@ -185,6 +184,8 @@ WaveformWidgetFactory::WaveformWidgetFactory()
if (!rendererString.isEmpty()) {
m_openGLVersion += " (" + rendererString + ")";
}
} else {
qDebug() << "QOpenGLContext::currentContext() returns nullptr";
}
widget->doneCurrent();
widget->hide();
Expand Down Expand Up @@ -818,16 +819,15 @@ void WaveformWidgetFactory::swap() {
}

WaveformWidgetType::Type WaveformWidgetFactory::autoChooseWidgetType() const {
if (m_openGlAvailable) {
if (m_openGLShaderAvailable) {
if (isOpenGlShaderAvailable()) {
#ifndef MIXXX_USE_QOPENGL
return WaveformWidgetType::GLSLRGBWaveform;
return WaveformWidgetType::GLSLRGBWaveform;
#else
return WaveformWidgetType::AllShaderRGBWaveform;
return WaveformWidgetType::AllShaderRGBWaveform;
#endif
} else {
return WaveformWidgetType::GLRGBWaveform;
}
}
if (isOpenGlAvailable() || isOpenGlesAvailable()) {
return WaveformWidgetType::GLRGBWaveform;
}
return WaveformWidgetType::RGBWaveform;
}
Expand Down
4 changes: 0 additions & 4 deletions src/waveform/widgets/glrgbwaveformwidget.cpp
Expand Up @@ -14,10 +14,6 @@

GLRGBWaveformWidget::GLRGBWaveformWidget(const QString& group, QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<GLWaveformRenderBackground>();
addRenderer<WaveformRendererEndOfTrack>();
addRenderer<WaveformRendererPreroll>();
Expand Down
4 changes: 0 additions & 4 deletions src/waveform/widgets/glsimplewaveformwidget.cpp
Expand Up @@ -17,10 +17,6 @@

GLSimpleWaveformWidget::GLSimpleWaveformWidget(const QString& group, QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<GLWaveformRenderBackground>();
addRenderer<WaveformRendererEndOfTrack>();
addRenderer<WaveformRendererPreroll>();
Expand Down
15 changes: 0 additions & 15 deletions src/waveform/widgets/glslwaveformwidget.cpp
Expand Up @@ -38,9 +38,6 @@ GLSLWaveformWidget::GLSLWaveformWidget(
QWidget* parent,
GlslType type)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

makeCurrentIfNeeded();

Expand Down Expand Up @@ -97,15 +94,3 @@ void GLSLWaveformWidget::resize(int width, int height) {
WaveformWidgetAbstract::resize(width, height);
doneCurrent();
}

void GLSLWaveformWidget::mouseDoubleClickEvent(QMouseEvent *event) {
if (event->button() == Qt::RightButton) {
makeCurrentIfNeeded();
#if !defined(QT_NO_OPENGL) && !defined(QT_OPENGL_ES_2)
if (m_signalRenderer) {
m_signalRenderer->debugClick();
}
#endif // !defined(QT_NO_OPENGL) && !defined(QT_OPENGL_ES_2)
doneCurrent();
}
}
3 changes: 0 additions & 3 deletions src/waveform/widgets/glslwaveformwidget.h
Expand Up @@ -23,12 +23,9 @@ class GLSLWaveformWidget : public GLWaveformWidgetAbstract {
protected:
void castToQWidget() override;
void paintEvent(QPaintEvent* event) override;
void mouseDoubleClickEvent(QMouseEvent *) override;
mixxx::Duration render() override;

private:
GLSLWaveformRendererSignal* m_signalRenderer;

friend class WaveformWidgetFactory;
};

Expand Down
4 changes: 0 additions & 4 deletions src/waveform/widgets/glvsynctestwidget.cpp
Expand Up @@ -17,10 +17,6 @@

GLVSyncTestWidget::GLVSyncTestWidget(const QString& group, QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<GLWaveformRenderBackground>(); // 172 µs
// addRenderer<WaveformRendererEndOfTrack>(); // 677 µs 1145 µs (active)
// addRenderer<WaveformRendererPreroll>(); // 652 µs 2034 µs (active)
Expand Down
4 changes: 0 additions & 4 deletions src/waveform/widgets/glwaveformwidget.cpp
Expand Up @@ -19,10 +19,6 @@

GLWaveformWidget::GLWaveformWidget(const QString& group, QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<GLWaveformRenderBackground>();
addRenderer<WaveformRendererEndOfTrack>();
addRenderer<WaveformRendererPreroll>();
Expand Down
4 changes: 0 additions & 4 deletions src/waveform/widgets/qtrgbwaveformwidget.cpp
Expand Up @@ -16,10 +16,6 @@

QtRGBWaveformWidget::QtRGBWaveformWidget(const QString& group, QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<GLWaveformRenderBackground>();
addRenderer<WaveformRendererEndOfTrack>();
addRenderer<WaveformRendererPreroll>();
Expand Down
4 changes: 0 additions & 4 deletions src/waveform/widgets/qtsimplewaveformwidget.cpp
Expand Up @@ -18,10 +18,6 @@ QtSimpleWaveformWidget::QtSimpleWaveformWidget(
const QString& group,
QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<GLWaveformRenderBackground>();
addRenderer<WaveformRendererEndOfTrack>();
addRenderer<WaveformRendererPreroll>();
Expand Down
5 changes: 0 additions & 5 deletions src/waveform/widgets/qtvsynctestwidget.cpp
Expand Up @@ -17,12 +17,7 @@

QtVSyncTestWidget::QtVSyncTestWidget(const QString& group, QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<QtVSyncTestRenderer>();

m_initSuccess = init();
}

Expand Down
4 changes: 0 additions & 4 deletions src/waveform/widgets/qtwaveformwidget.cpp
Expand Up @@ -17,10 +17,6 @@

QtWaveformWidget::QtWaveformWidget(const QString& group, QWidget* parent)
: GLWaveformWidgetAbstract(group, parent) {
qDebug() << "Created WGLWidget. Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

addRenderer<GLWaveformRenderBackground>();
addRenderer<WaveformRendererEndOfTrack>();
addRenderer<WaveformRendererPreroll>();
Expand Down
4 changes: 0 additions & 4 deletions src/widget/wglwidgetqglwidget.cpp
Expand Up @@ -20,10 +20,6 @@ bool WGLWidget::isContextValid() const {
return context()->isValid();
}

bool WGLWidget::isContextSharing() const {
return context()->isSharing();
}

bool WGLWidget::shouldRender() const {
return isValid() && isVisible() && windowHandle() && windowHandle()->isExposed();
}
Expand Down
3 changes: 0 additions & 3 deletions src/widget/wglwidgetqglwidget.h
Expand Up @@ -11,10 +11,7 @@ class WGLWidget : public QGLWidget {
WGLWidget(QWidget* pParent);

bool isContextValid() const;
bool isContextSharing() const;

bool shouldRender() const;

void makeCurrentIfNeeded();

protected:
Expand Down
4 changes: 0 additions & 4 deletions src/widget/wglwidgetqopengl.cpp
Expand Up @@ -58,10 +58,6 @@ bool WGLWidget::isContextValid() const {
return m_pOpenGLWindow && m_pOpenGLWindow->context() && m_pOpenGLWindow->context()->isValid();
}

bool WGLWidget::isContextSharing() const {
return true;
}

void WGLWidget::makeCurrentIfNeeded() {
if (m_pOpenGLWindow && m_pOpenGLWindow->context() != QOpenGLContext::currentContext()) {
m_pOpenGLWindow->makeCurrent();
Expand Down
1 change: 0 additions & 1 deletion src/widget/wglwidgetqopengl.h
Expand Up @@ -20,7 +20,6 @@ class WGLWidget : public QWidget {
~WGLWidget();

bool isContextValid() const;
bool isContextSharing() const;

bool shouldRender() const;

Expand Down
4 changes: 1 addition & 3 deletions src/widget/wspinnybase.cpp
Expand Up @@ -43,6 +43,7 @@ WSpinnyBase::WSpinnyBase(
m_pVinylControlEnabled(nullptr),
m_pSignalEnabled(nullptr),
m_pSlipEnabled(nullptr),
m_pShowCoverProxy(nullptr),
m_bShowCover(true),
m_dInitialPos(0.),
m_iVinylInput(-1),
Expand Down Expand Up @@ -73,9 +74,6 @@ WSpinnyBase::WSpinnyBase(
#endif // __VINYLCONTROL__
// Drag and drop
setAcceptDrops(true);
qDebug() << "WSpinnyBase(): Created WGLWidget, Context"
<< "Valid:" << isContextValid()
<< "Sharing:" << isContextSharing();

CoverArtCache* pCache = CoverArtCache::instance();
if (pCache) {
Expand Down