Merge pull request #11963 from daschuer/gh11929

Fix crash when no GL context is available
mixxxdj · Sep 12, 2023 · e8ae69a · e8ae69a
2 parents 750196e + 5544909
commit e8ae69a
Show file tree

Hide file tree

Showing 20 changed files with 83 additions and 127 deletions.
diff --git a/src/mixxxmainwindow.cpp b/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"
@@ -149,22 +152,33 @@ 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
+        QOpenGLContext context;
+        context.setFormat(WaveformWidgetFactory::getSurfaceFormat());
+        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();
+            return;
+        }
+        qDebug() << "QOpenGLContext::create() failed";
     }
+    qInfo() << "Initializing without OpenGL";
+    initialize();
 }
 #endif
 

diff --git a/src/waveform/sharedglcontext.cpp b/src/waveform/sharedglcontext.cpp
@@ -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) {

diff --git a/src/waveform/waveformwidgetfactory.cpp b/src/waveform/waveformwidgetfactory.cpp
@@ -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 {
@@ -185,6 +184,8 @@ WaveformWidgetFactory::WaveformWidgetFactory()
             if (!rendererString.isEmpty()) {
                 m_openGLVersion += " (" + rendererString + ")";
             }
+        } else {
+            qDebug() << "QOpenGLContext::currentContext() returns nullptr";
         }
         widget->doneCurrent();
         widget->hide();
@@ -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;
 }
@@ -1183,3 +1183,34 @@ QString WaveformWidgetFactory::buildWidgetDisplayName() const {
     }
     return QStringLiteral("%1 (%2)").arg(name, extras.join(QStringLiteral(", ")));
 }
+
+// static
+QSurfaceFormat WaveformWidgetFactory::getSurfaceFormat() {
+    QSurfaceFormat format;
+    format.setVersion(2, 1);
+    format.setProfile(QSurfaceFormat::CoreProfile);
+
+    // setSwapInterval sets the application preferred swap interval
+    // in minimum number of video frames that are displayed before a buffer swap occurs
+    // - 0 will turn the vertical refresh syncing off
+    // - 1 (default) means swapping after drawig a video frame to the buffer
+    // - n means swapping after drawing n video frames to the buffer
+    //
+    // The vertical sync setting requested by the OpenGL application, can be overwritten
+    // if a user changes the "Wait for vertical refresh" setting in AMD graphic drivers
+    // for Windows.
+
+#if defined(__APPLE__)
+    // On OS X, syncing to vsync has good performance FPS-wise and
+    // eliminates tearing. (This is an comment from pre QOpenGLWindow times)
+    format.setSwapInterval(1);
+#else
+    // It seems that on Windows (at least for some AMD drivers), the setting 1 is not
+    // not properly handled. We saw frame rates divided by exact integers, like it should
+    // be with values >1 (see https://github.com/mixxxdj/mixxx/issues/11617)
+    // Reported as https://bugreports.qt.io/browse/QTBUG-114882
+    // On Linux, horrible FPS were seen with "VSync off" before switching to QOpenGLWindow too
+    format.setSwapInterval(0);
+#endif
+    return format;
+}
diff --git a/src/waveform/waveformwidgetfactory.h b/src/waveform/waveformwidgetfactory.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <QObject>
+#include <QSurfaceFormat>
 #include <QVector>
 #include <vector>
 
@@ -98,6 +99,9 @@ class WaveformWidgetFactory : public QObject, public Singleton<WaveformWidgetFac
     }
     int findHandleIndexFromType(WaveformWidgetType::Type type);
 
+    /// Returns the desired surface format for the OpenGLWindow
+    static QSurfaceFormat getSurfaceFormat();
+
   protected:
     bool setWidgetType(
             WaveformWidgetType::Type type,

diff --git a/src/waveform/widgets/glrgbwaveformwidget.cpp b/src/waveform/widgets/glrgbwaveformwidget.cpp
@@ -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>();

diff --git a/src/waveform/widgets/glsimplewaveformwidget.cpp b/src/waveform/widgets/glsimplewaveformwidget.cpp
@@ -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>();

diff --git a/src/waveform/widgets/glslwaveformwidget.cpp b/src/waveform/widgets/glslwaveformwidget.cpp
@@ -38,9 +38,6 @@ GLSLWaveformWidget::GLSLWaveformWidget(
         QWidget* parent,
         GlslType type)
         : GLWaveformWidgetAbstract(group, parent) {
-    qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << isContextValid()
-             << "Sharing:" << isContextSharing();
 
     makeCurrentIfNeeded();
 
@@ -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();
-    }
-}
diff --git a/src/waveform/widgets/glslwaveformwidget.h b/src/waveform/widgets/glslwaveformwidget.h
@@ -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;
 };
 

diff --git a/src/waveform/widgets/glvsynctestwidget.cpp b/src/waveform/widgets/glvsynctestwidget.cpp
@@ -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)

diff --git a/src/waveform/widgets/glwaveformwidget.cpp b/src/waveform/widgets/glwaveformwidget.cpp
@@ -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>();

diff --git a/src/waveform/widgets/qtrgbwaveformwidget.cpp b/src/waveform/widgets/qtrgbwaveformwidget.cpp
@@ -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>();

diff --git a/src/waveform/widgets/qtsimplewaveformwidget.cpp b/src/waveform/widgets/qtsimplewaveformwidget.cpp
@@ -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>();

diff --git a/src/waveform/widgets/qtvsynctestwidget.cpp b/src/waveform/widgets/qtvsynctestwidget.cpp
@@ -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();
 }
 

diff --git a/src/waveform/widgets/qtwaveformwidget.cpp b/src/waveform/widgets/qtwaveformwidget.cpp
@@ -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>();

diff --git a/src/widget/openglwindow.cpp b/src/widget/openglwindow.cpp
@@ -4,40 +4,15 @@
 #include <QResizeEvent>
 
 #include "moc_openglwindow.cpp"
+#include "waveform/waveformwidgetfactory.h"
 #include "widget/tooltipqopengl.h"
 #include "widget/trackdroptarget.h"
 #include "widget/wglwidget.h"
 
 OpenGLWindow::OpenGLWindow(WGLWidget* pWidget)
         : m_pWidget(pWidget),
           m_dirty(false) {
-    QSurfaceFormat format;
-    format.setVersion(2, 1);
-    format.setProfile(QSurfaceFormat::CoreProfile);
-
-    // setSwapInterval sets the application preferred swap interval
-    // in minimum number of video frames that are displayed before a buffer swap occurs
-    // - 0 will turn the vertical refresh syncing off
-    // - 1 (default) means swapping after drawig a video frame to the buffer
-    // - n means swapping after drawing n video frames to the buffer
-    //
-    // The vertical sync setting requested by the OpenGL application, can be overwritten
-    // if a user changes the "Wait for vertical refresh" setting in AMD graphic drivers
-    // for Windows.
-
-#if defined(__APPLE__)
-    // On OS X, syncing to vsync has good performance FPS-wise and
-    // eliminates tearing. (This is an comment from pre QOpenGLWindow times)
-    format.setSwapInterval(1);
-#else
-    // It seems that on Windows (at least for some AMD drivers), the setting 1 is not
-    // not properly handled. We saw frame rates divided by exact integers, like it should
-    // be with values >1 (see https://github.com/mixxxdj/mixxx/issues/11617)
-    // Reported as https://bugreports.qt.io/browse/QTBUG-114882
-    // On Linux, horrible FPS were seen with "VSync off" before switching to QOpenGLWindow too
-    format.setSwapInterval(0);
-#endif
-    setFormat(format);
+    setFormat(WaveformWidgetFactory::getSurfaceFormat());
 }
 
 OpenGLWindow::~OpenGLWindow() {

diff --git a/src/widget/wglwidgetqglwidget.cpp b/src/widget/wglwidgetqglwidget.cpp
@@ -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();
 }

diff --git a/src/widget/wglwidgetqglwidget.h b/src/widget/wglwidgetqglwidget.h
@@ -11,10 +11,7 @@ class WGLWidget : public QGLWidget {
     WGLWidget(QWidget* pParent);
 
     bool isContextValid() const;
-    bool isContextSharing() const;
-
     bool shouldRender() const;
-
     void makeCurrentIfNeeded();
 
   protected:

diff --git a/src/widget/wglwidgetqopengl.cpp b/src/widget/wglwidgetqopengl.cpp
@@ -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();

diff --git a/src/widget/wglwidgetqopengl.h b/src/widget/wglwidgetqopengl.h
@@ -20,7 +20,6 @@ class WGLWidget : public QWidget {
     ~WGLWidget();
 
     bool isContextValid() const;
-    bool isContextSharing() const;
 
     bool shouldRender() const;