moved duplicated qglwidget calls to wglwidget methods (this prepares …

…for adding an alternative implementation using qopenglwindow) and use directconnection where appropriate
mixxxdj · Oct 14, 2022 · 176b279 · 176b279
1 parent 3710eef
commit 176b279
Show file tree

Hide file tree

Showing 16 changed files with 93 additions and 142 deletions.
diff --git a/src/skin/legacy/legacyskinparser.cpp b/src/skin/legacy/legacyskinparser.cpp
@@ -1314,8 +1314,13 @@ QWidget* LegacySkinParser::parseSpinny(const QDomElement& node) {
     connect(waveformWidgetFactory,
             &WaveformWidgetFactory::renderSpinnies,
             pSpinny,
-            &WSpinny::render);
-    connect(waveformWidgetFactory, &WaveformWidgetFactory::swapSpinnies, pSpinny, &WSpinny::swap);
+            &WSpinny::render,
+            Qt::DirectConnection);
+    connect(waveformWidgetFactory,
+            &WaveformWidgetFactory::swapSpinnies,
+            pSpinny,
+            &WSpinny::swap,
+            Qt::DirectConnection);
     connect(pSpinny,
             &WSpinny::trackDropped,
             m_pPlayerManager,

diff --git a/src/waveform/waveformwidgetfactory.cpp b/src/waveform/waveformwidgetfactory.cpp
@@ -50,23 +50,12 @@ bool shouldRenderWaveform(WaveformWidgetAbstract* pWaveformWidget) {
 
     auto* glw = pWaveformWidget->getGLWidget();
     if (glw == nullptr) {
-        // Not a QGLWidget. We can simply use QWidget::isVisible.
+        // Not a WGLWidget. We can simply use QWidget::isVisible.
         auto* qwidget = qobject_cast<QWidget*>(pWaveformWidget->getWidget());
         return qwidget != nullptr && qwidget->isVisible();
     }
 
-    if (glw == nullptr || !glw->isValid() || !glw->isVisible()) {
-        return false;
-    }
-
-    // Strangely, a widget can have non-zero width/height, be valid and visible,
-    // yet still not show up on the screen. QWindow::isExposed tells us this.
-    const QWindow* window = glw->windowHandle();
-    if (window == nullptr || !window->isExposed()) {
-        return false;
-    }
-
-    return true;
+    return glw->shouldRender();
 }
 }  // anonymous namespace
 
@@ -370,11 +359,13 @@ void WaveformWidgetFactory::addVuMeter(WVuMeterGL* pVuMeter) {
     connect(this,
             &WaveformWidgetFactory::renderVuMeters,
             pVuMeter,
-            &WVuMeterGL::render);
+            &WVuMeterGL::render,
+            Qt::DirectConnection);
     connect(this,
             &WaveformWidgetFactory::swapVuMeters,
             pVuMeter,
-            &WVuMeterGL::swap);
+            &WVuMeterGL::swap,
+            Qt::DirectConnection);
 }
 
 void WaveformWidgetFactory::slotSkinLoaded() {
@@ -734,9 +725,7 @@ void WaveformWidgetFactory::swap() {
                 }
                 WGLWidget* glw = pWaveformWidget->getGLWidget();
                 if (glw != nullptr) {
-                    if (glw->context() != QGLContext::currentContext()) {
-                        glw->makeCurrent();
-                    }
+                    glw->makeCurrentIfNeeded();
                     glw->swapBuffers();
                 }
                 //qDebug() << "swap x" << m_vsyncThread->elapsed();

diff --git a/src/waveform/widgets/glrgbwaveformwidget.cpp b/src/waveform/widgets/glrgbwaveformwidget.cpp
@@ -15,8 +15,8 @@
 GLRGBWaveformWidget::GLRGBWaveformWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
@@ -28,11 +28,6 @@ GLRGBWaveformWidget::GLRGBWaveformWidget(const QString& group, QWidget* parent)
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 

diff --git a/src/waveform/widgets/glsimplewaveformwidget.cpp b/src/waveform/widgets/glsimplewaveformwidget.cpp
@@ -18,8 +18,8 @@
 GLSimpleWaveformWidget::GLSimpleWaveformWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
@@ -31,11 +31,6 @@ GLSimpleWaveformWidget::GLSimpleWaveformWidget(const QString& group, QWidget* pa
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 

diff --git a/src/waveform/widgets/glslwaveformwidget.cpp b/src/waveform/widgets/glslwaveformwidget.cpp
@@ -39,11 +39,10 @@ GLSLWaveformWidget::GLSLWaveformWidget(
         GlslType type)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
-    if (QGLContext::currentContext() != context()) {
-        makeCurrent();
-    }
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
+
+    makeCurrentIfNeeded();
 
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
@@ -63,11 +62,6 @@ GLSLWaveformWidget::GLSLWaveformWidget(
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 
@@ -97,13 +91,13 @@ mixxx::Duration GLSLWaveformWidget::render() {
 void GLSLWaveformWidget::resize(int width, int height) {
     // NOTE: (vrince) this is needed since we allocation buffer on resize
     // and the Gl Context should be properly set
-    makeCurrent();
+    makeCurrentIfNeeded();
     WaveformWidgetAbstract::resize(width, height);
 }
 
 void GLSLWaveformWidget::mouseDoubleClickEvent(QMouseEvent *event) {
     if (event->button() == Qt::RightButton) {
-        makeCurrent();
+        makeCurrentIfNeeded();
 #if !defined(QT_NO_OPENGL) && !defined(QT_OPENGL_ES_2)
         if (m_signalRenderer) {
             m_signalRenderer->debugClick();

diff --git a/src/waveform/widgets/glvsynctestwidget.cpp b/src/waveform/widgets/glvsynctestwidget.cpp
@@ -18,8 +18,8 @@
 GLVSyncTestWidget::GLVSyncTestWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<WaveformRenderBackground>(); // 172 µs
 //  addRenderer<WaveformRendererEndOfTrack>(); // 677 µs 1145 µs (active)
@@ -33,13 +33,7 @@ GLVSyncTestWidget::GLVSyncTestWidget(const QString& group, QWidget* parent)
     // addRenderer<WaveformRenderMark>(); // 711 µs
     // addRenderer<WaveformRenderBeat>(); // 1183 µs
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
-    qDebug() << "GLVSyncTestWidget.isSharing() =" << isSharing();
 }
 
 GLVSyncTestWidget::~GLVSyncTestWidget() {

diff --git a/src/waveform/widgets/glwaveformwidget.cpp b/src/waveform/widgets/glwaveformwidget.cpp
@@ -20,8 +20,8 @@
 GLWaveformWidget::GLWaveformWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
@@ -33,11 +33,6 @@ GLWaveformWidget::GLWaveformWidget(const QString& group, QWidget* parent)
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 

diff --git a/src/waveform/widgets/qthsvwaveformwidget.cpp b/src/waveform/widgets/qthsvwaveformwidget.cpp
@@ -16,9 +16,7 @@
 
 QtHSVWaveformWidget::QtHSVWaveformWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
-    if (QGLContext::currentContext() != context()) {
-        makeCurrent();
-    }
+    makeCurrentIfNeeded();
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
     addRenderer<WaveformRendererPreroll>();
@@ -27,9 +25,6 @@ QtHSVWaveformWidget::QtHSVWaveformWidget(const QString& group, QWidget* parent)
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
     m_initSuccess = init();
 }
 

diff --git a/src/waveform/widgets/qtrgbwaveformwidget.cpp b/src/waveform/widgets/qtrgbwaveformwidget.cpp
@@ -17,8 +17,8 @@
 QtRGBWaveformWidget::QtRGBWaveformWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
@@ -28,11 +28,6 @@ QtRGBWaveformWidget::QtRGBWaveformWidget(const QString& group, QWidget* parent)
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 

diff --git a/src/waveform/widgets/qtsimplewaveformwidget.cpp b/src/waveform/widgets/qtsimplewaveformwidget.cpp
@@ -19,8 +19,8 @@ QtSimpleWaveformWidget::QtSimpleWaveformWidget(
         QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
@@ -30,11 +30,6 @@ QtSimpleWaveformWidget::QtSimpleWaveformWidget(
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 

diff --git a/src/waveform/widgets/qtvsynctestwidget.cpp b/src/waveform/widgets/qtvsynctestwidget.cpp
@@ -18,16 +18,11 @@
 QtVSyncTestWidget::QtVSyncTestWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<QtVSyncTestRenderer>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 

diff --git a/src/waveform/widgets/qtwaveformwidget.cpp b/src/waveform/widgets/qtwaveformwidget.cpp
@@ -18,8 +18,8 @@
 QtWaveformWidget::QtWaveformWidget(const QString& group, QWidget* parent)
         : GLWaveformWidgetAbstract(group, parent) {
     qDebug() << "Created WGLWidget. Context"
-             << "Valid:" << context()->isValid()
-             << "Sharing:" << context()->isSharing();
+             << "Valid:" << isContextValid()
+             << "Sharing:" << isContextSharing();
 
     addRenderer<WaveformRenderBackground>();
     addRenderer<WaveformRendererEndOfTrack>();
@@ -29,11 +29,6 @@ QtWaveformWidget::QtWaveformWidget(const QString& group, QWidget* parent)
     addRenderer<WaveformRenderBeat>();
     addRenderer<WaveformRenderMark>();
 
-    setAttribute(Qt::WA_NoSystemBackground);
-    setAttribute(Qt::WA_OpaquePaintEvent);
-
-    setAutoBufferSwap(false);
-
     m_initSuccess = init();
 }
 

diff --git a/src/widget/wglwidget.h b/src/widget/wglwidget.h
@@ -5,4 +5,19 @@
 class WGLWidget : public QGLWidget {
   public:
     WGLWidget(QWidget* parent);
+
+    bool isContextValid() const;
+    bool isContextSharing() const;
+
+    bool shouldRender() const;
+
+    void makeCurrentIfNeeded();
 };
+
+// Other QGLWidget / QWidget methods called:
+// - void swapBuffers() {
+// - To avoid xcb crash, in legacyskinparser.cpp
+//   - void setParent(QWidget* parent) {
+// - Used by WSpinny:
+//   - void installEventFilter(QObject *);
+// - connect
diff --git a/src/widget/wglwidgetqglwidget.cpp b/src/widget/wglwidgetqglwidget.cpp
@@ -1,6 +1,34 @@
+#include <QWindow>
+
 #include "waveform/sharedglcontext.h"
 #include "widget/wglwidget.h"
 
 WGLWidget::WGLWidget(QWidget* parent)
         : QGLWidget(parent, SharedGLContext::getWidget()) {
+    setAttribute(Qt::WA_NoSystemBackground);
+    setAttribute(Qt::WA_OpaquePaintEvent);
+    setAutoFillBackground(false);
+    setAutoBufferSwap(false);
+    // Not interested in repaint or update calls, as we draw from the vsync thread
+    setUpdatesEnabled(false);
+}
+
+bool WGLWidget::isContextValid() const {
+    return context()->isValid();
+}
+
+bool WGLWidget::isContextSharing() const {
+    return context()->isSharing();
+}
+
+bool WGLWidget::shouldRender() const {
+    return true;
+    return isValid() && isVisible() && windowHandle() && windowHandle()->isExposed();
+}
+
+void WGLWidget::makeCurrentIfNeeded() {
+    // TODO m0dB is this really needed? is calling makeCurrent without this 'if' really a problem?
+    //if (context() != QGLContext::currentContext()) {
+    makeCurrent();
+    //}
 }