moved duplicated qglwidget calls to wglwidget methods (this prepares …

…for adding an alternative implementation using qopenglwindow) and use directconnection where appropriate
mixxxdj · Oct 15, 2022 · 31fa633 · 31fa633
1 parent 9177ed7
commit 31fa633
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();
+ //}
 }