From 31fa6332c9e21874e3501fa41ccfeaefe4a2c4f6 Mon Sep 17 00:00:00 2001 From: m0dB <79429057+m0dB@users.noreply.github.com> Date: Fri, 14 Oct 2022 12:50:02 +0200 Subject: [PATCH] moved duplicated qglwidget calls to wglwidget methods (this prepares for adding an alternative implementation using qopenglwindow) and use directconnection where appropriate --- src/skin/legacy/legacyskinparser.cpp | 9 +++-- src/waveform/waveformwidgetfactory.cpp | 25 ++++---------- src/waveform/widgets/glrgbwaveformwidget.cpp | 9 ++--- .../widgets/glsimplewaveformwidget.cpp | 9 ++--- src/waveform/widgets/glslwaveformwidget.cpp | 18 ++++------ src/waveform/widgets/glvsynctestwidget.cpp | 10 ++---- src/waveform/widgets/glwaveformwidget.cpp | 9 ++--- src/waveform/widgets/qthsvwaveformwidget.cpp | 7 +--- src/waveform/widgets/qtrgbwaveformwidget.cpp | 9 ++--- .../widgets/qtsimplewaveformwidget.cpp | 9 ++--- src/waveform/widgets/qtvsynctestwidget.cpp | 9 ++--- src/waveform/widgets/qtwaveformwidget.cpp | 9 ++--- src/widget/wglwidget.h | 15 +++++++++ src/widget/wglwidgetqglwidget.cpp | 28 ++++++++++++++++ src/widget/wspinny.cpp | 33 +++++-------------- src/widget/wvumetergl.cpp | 27 +++------------ 16 files changed, 93 insertions(+), 142 deletions(-) diff --git a/src/skin/legacy/legacyskinparser.cpp b/src/skin/legacy/legacyskinparser.cpp index b47211952db1..9eb1b26f4272 100644 --- 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 index 4de62e6b015d..52ddb22f2a93 100644 --- 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(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 index 0f695676eca2..99f607fe0574 100644 --- 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(); addRenderer(); @@ -28,11 +28,6 @@ GLRGBWaveformWidget::GLRGBWaveformWidget(const QString& group, QWidget* parent) addRenderer(); addRenderer(); - 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 index ecf8b05ae8dc..64a57e3eabe5 100644 --- 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(); addRenderer(); @@ -31,11 +31,6 @@ GLSimpleWaveformWidget::GLSimpleWaveformWidget(const QString& group, QWidget* pa addRenderer(); addRenderer(); - 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 index 0f3f7b6af260..6a7ac8bc831d 100644 --- 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(); addRenderer(); @@ -63,11 +62,6 @@ GLSLWaveformWidget::GLSLWaveformWidget( addRenderer(); addRenderer(); - 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 index 91fef79fe2bc..938bba5967d4 100644 --- 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(); // 172 µs // addRenderer(); // 677 µs 1145 µs (active) @@ -33,13 +33,7 @@ GLVSyncTestWidget::GLVSyncTestWidget(const QString& group, QWidget* parent) // addRenderer(); // 711 µs // addRenderer(); // 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 index c4578c9624ab..6279dc9decec 100644 --- 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(); addRenderer(); @@ -33,11 +33,6 @@ GLWaveformWidget::GLWaveformWidget(const QString& group, QWidget* parent) addRenderer(); addRenderer(); - 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 index 3292f5184447..c1cc090fae2f 100644 --- 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(); addRenderer(); addRenderer(); @@ -27,9 +25,6 @@ QtHSVWaveformWidget::QtHSVWaveformWidget(const QString& group, QWidget* parent) addRenderer(); addRenderer(); - 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 index c1630f745187..aa3fc3d0efc8 100644 --- 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(); addRenderer(); @@ -28,11 +28,6 @@ QtRGBWaveformWidget::QtRGBWaveformWidget(const QString& group, QWidget* parent) addRenderer(); addRenderer(); - 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 index f3aa293d3f6c..fcaf7bfab982 100644 --- 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(); addRenderer(); @@ -30,11 +30,6 @@ QtSimpleWaveformWidget::QtSimpleWaveformWidget( addRenderer(); addRenderer(); - 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 index 42b25c6af3c9..3e20ecbfb449 100644 --- 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(); - 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 index 8ea69788bd17..9b2c75dc46e9 100644 --- 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(); addRenderer(); @@ -29,11 +29,6 @@ QtWaveformWidget::QtWaveformWidget(const QString& group, QWidget* parent) addRenderer(); addRenderer(); - 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 index 161135dca3c9..9a2f5fed166f 100644 --- 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 index eaf3d3cabe47..40013e5c7b85 100644 --- a/src/widget/wglwidgetqglwidget.cpp +++ b/src/widget/wglwidgetqglwidget.cpp @@ -1,6 +1,34 @@ +#include + #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(); + //} } diff --git a/src/widget/wspinny.cpp b/src/widget/wspinny.cpp index 8f0dca297044..b58b1a109e44 100644 --- a/src/widget/wspinny.cpp +++ b/src/widget/wspinny.cpp @@ -72,11 +72,9 @@ WSpinny::WSpinny( //Drag and drop setAcceptDrops(true); qDebug() << "WSpinny(): Created WGLWidget, Context" - << "Valid:" << context()->isValid() - << "Sharing:" << context()->isSharing(); - if (QGLContext::currentContext() != context()) { - makeCurrent(); - } + << "Valid:" << isContextValid() + << "Sharing:" << isContextSharing(); + makeCurrentIfNeeded(); CoverArtCache* pCache = CoverArtCache::instance(); if (pCache) { @@ -95,12 +93,6 @@ WSpinny::WSpinny( connect(m_pCoverMenu, &WCoverArtMenu::coverInfoSelected, this, &WSpinny::slotCoverInfoSelected); connect(m_pCoverMenu, &WCoverArtMenu::reloadCoverArt, this, &WSpinny::slotReloadCoverArt); - - setAttribute(Qt::WA_NoSystemBackground); - setAttribute(Qt::WA_OpaquePaintEvent); - - setAutoFillBackground(false); - setAutoBufferSwap(false); } WSpinny::~WSpinny() { @@ -327,12 +319,8 @@ void WSpinny::paintEvent(QPaintEvent *e) { } void WSpinny::render(VSyncThread* vSyncThread) { - if (!isValid() || !isVisible()) { - return; - } - - auto* window = windowHandle(); - if (window == nullptr || !window->isExposed()) { + // TODO @m0dB move outside? + if (!shouldRender()) { return; } @@ -414,16 +402,11 @@ void WSpinny::render(VSyncThread* vSyncThread) { } void WSpinny::swap() { - if (!isValid() || !isVisible()) { - return; - } - auto* window = windowHandle(); - if (window == nullptr || !window->isExposed()) { + // TODO @m0dB move outside? + if (!shouldRender()) { return; } - if (context() != QGLContext::currentContext()) { - makeCurrent(); - } + makeCurrentIfNeeded(); swapBuffers(); } diff --git a/src/widget/wvumetergl.cpp b/src/widget/wvumetergl.cpp index b3467cbaa49d..9c17d57eebce 100644 --- a/src/widget/wvumetergl.cpp +++ b/src/widget/wvumetergl.cpp @@ -28,14 +28,6 @@ WVuMeterGL::WVuMeterGL(QWidget* parent) m_iPeakHoldTime(0), m_iPeakFallTime(0), m_dPeakHoldCountdownMs(0) { - 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); } void WVuMeterGL::setup(const QDomNode& node, const SkinContext& context) { @@ -177,12 +169,8 @@ void WVuMeterGL::render(VSyncThread* vSyncThread) { return; } - if (!isValid() || !isVisible()) { - return; - } - - auto* window = windowHandle(); - if (window == nullptr || !window->isExposed()) { + // TODO @m0dB move outside? + if (!shouldRender()) { return; } @@ -298,16 +286,11 @@ void WVuMeterGL::render(VSyncThread* vSyncThread) { } void WVuMeterGL::swap() { - if (!isValid() || !isVisible() || !m_bSwapNeeded) { - return; - } - auto* window = windowHandle(); - if (window == nullptr || !window->isExposed()) { + // TODO @m0dB move shouldRender outside? + if (!m_bSwapNeeded || !shouldRender()) { return; } - if (context() != QGLContext::currentContext()) { - makeCurrent(); - } + makeCurrentIfNeeded(); swapBuffers(); m_bSwapNeeded = false; }