From 6fc3426e6cbe7ca9144c16dc5ce8a7e16998767d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 12 Jan 2025 23:18:14 +0100 Subject: [PATCH 1/6] Add missing p prefix to pTexture --- .../renderers/allshader/waveformrendermark.cpp | 10 +++++----- .../renderers/allshader/waveformrendermark.h | 2 +- src/widget/wspinnyglsl.cpp | 10 +++++----- src/widget/wspinnyglsl.h | 2 +- src/widget/wvumeterglsl.cpp | 14 +++++++------- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/waveform/renderers/allshader/waveformrendermark.cpp b/src/waveform/renderers/allshader/waveformrendermark.cpp index a60d668ab97..397792e1802 100644 --- a/src/waveform/renderers/allshader/waveformrendermark.cpp +++ b/src/waveform/renderers/allshader/waveformrendermark.cpp @@ -97,7 +97,7 @@ void allshader::WaveformRenderMark::initializeGL() { } void allshader::WaveformRenderMark::drawTexture( - const QMatrix4x4& matrix, float x, float y, QOpenGLTexture* texture) { + const QMatrix4x4& matrix, float x, float y, QOpenGLTexture* pTexture) { const float devicePixelRatio = m_waveformRenderer->getDevicePixelRatio(); const float texx1 = 0.f; const float texy1 = 0.f; @@ -105,9 +105,9 @@ void allshader::WaveformRenderMark::drawTexture( const float texy2 = 1.f; const float posx1 = x; - const float posx2 = x + static_cast(texture->width() / devicePixelRatio); + const float posx2 = x + static_cast(pTexture->width() / devicePixelRatio); const float posy1 = y; - const float posy2 = y + static_cast(texture->height() / devicePixelRatio); + const float posy2 = y + static_cast(pTexture->height() / devicePixelRatio); const float posarray[] = {posx1, posy1, posx2, posy1, posx1, posy2, posx2, posy2}; const float texarray[] = {texx1, texy1, texx2, texy1, texx1, texy2, texx2, texy2}; @@ -130,11 +130,11 @@ void allshader::WaveformRenderMark::drawTexture( m_textureShader.setUniformValue(textureLocation, 0); - texture->bind(); + pTexture->bind(); glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); - texture->release(); + pTexture->release(); m_textureShader.disableAttributeArray(positionLocation); m_textureShader.disableAttributeArray(texcoordLocation); diff --git a/src/waveform/renderers/allshader/waveformrendermark.h b/src/waveform/renderers/allshader/waveformrendermark.h index 0937cfbbfa1..5f4b812388a 100644 --- a/src/waveform/renderers/allshader/waveformrendermark.h +++ b/src/waveform/renderers/allshader/waveformrendermark.h @@ -51,7 +51,7 @@ class allshader::WaveformRenderMark : public ::WaveformRenderMarkBase, QPointF p3); void drawMark(const QMatrix4x4& matrix, const QRectF& rect, QColor color); - void drawTexture(const QMatrix4x4& matrix, float x, float y, QOpenGLTexture* texture); + void drawTexture(const QMatrix4x4& matrix, float x, float y, QOpenGLTexture* pTexture); void updateUntilMark(double playPosition, double markerPosition); void drawUntilMark(const QMatrix4x4& matrix, float x); float getMaxHeightForText(float proportion) const; diff --git a/src/widget/wspinnyglsl.cpp b/src/widget/wspinnyglsl.cpp index 8ffed866e7b..bda968e4d3a 100644 --- a/src/widget/wspinnyglsl.cpp +++ b/src/widget/wspinnyglsl.cpp @@ -171,14 +171,14 @@ void WSpinnyGLSL::initializeGL() { m_vinylQualityShader.init(); } -void WSpinnyGLSL::drawTexture(QOpenGLTexture* texture) { +void WSpinnyGLSL::drawTexture(QOpenGLTexture* pTexture) { const float texx1 = 0.f; const float texy1 = 1.0; const float texx2 = 1.f; const float texy2 = 0.f; - const float tw = texture->width(); - const float th = texture->height(); + const float tw = pTexture->width(); + const float th = pTexture->height(); // fill centered const float posx2 = tw >= th ? 1.f : tw / th; @@ -197,11 +197,11 @@ void WSpinnyGLSL::drawTexture(QOpenGLTexture* texture) { m_textureShader.setAttributeArray( texcoordLocation, GL_FLOAT, texarray.data(), 2); - texture->bind(); + pTexture->bind(); glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); - texture->release(); + pTexture->release(); } void WSpinnyGLSL::drawVinylQuality() { diff --git a/src/widget/wspinnyglsl.h b/src/widget/wspinnyglsl.h index 2c418a7b867..d6441b52025 100644 --- a/src/widget/wspinnyglsl.h +++ b/src/widget/wspinnyglsl.h @@ -26,7 +26,7 @@ class WSpinnyGLSL : public WSpinnyBase, private QOpenGLFunctions { void initializeGL() override; void paintGL() override; void resizeGL(int w, int h) override; - void drawTexture(QOpenGLTexture* texture); + void drawTexture(QOpenGLTexture* pTexture); void cleanupGL(); void updateTextures(); diff --git a/src/widget/wvumeterglsl.cpp b/src/widget/wvumeterglsl.cpp index 39da264ecc9..e28da41334e 100644 --- a/src/widget/wvumeterglsl.cpp +++ b/src/widget/wvumeterglsl.cpp @@ -150,15 +150,15 @@ void WVuMeterGLSL::cleanupGL() { doneCurrent(); } -void WVuMeterGLSL::drawTexture(QOpenGLTexture* texture, +void WVuMeterGLSL::drawTexture(QOpenGLTexture* pTexture, const QRectF& targetRect, const QRectF& sourceRect) { - const float texx1 = static_cast(sourceRect.x() / texture->width()); - const float texy1 = static_cast(sourceRect.y() / texture->height()); + const float texx1 = static_cast(sourceRect.x() / pTexture->width()); + const float texy1 = static_cast(sourceRect.y() / pTexture->height()); const float texx2 = static_cast( - (sourceRect.x() + sourceRect.width()) / texture->width()); + (sourceRect.x() + sourceRect.width()) / pTexture->width()); const float texy2 = static_cast( - (sourceRect.y() + sourceRect.height()) / texture->height()); + (sourceRect.y() + sourceRect.height()) / pTexture->height()); const float posx1 = static_cast(targetRect.x()); const float posy1 = static_cast(targetRect.y()); @@ -176,9 +176,9 @@ void WVuMeterGLSL::drawTexture(QOpenGLTexture* texture, m_textureShader.setAttributeArray( texcoordLocation, GL_FLOAT, texarray.data(), 2); - texture->bind(); + pTexture->bind(); glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); - texture->release(); + pTexture->release(); } From ac9ac48029408f1fadbdfe678ef91ea610c16329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 14 Jan 2025 08:06:26 +0100 Subject: [PATCH 2/6] Make MarkerGeometry a class with const getter --- src/waveform/renderers/waveformmark.cpp | 64 ++++++++++++++++--------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/src/waveform/renderers/waveformmark.cpp b/src/waveform/renderers/waveformmark.cpp index 809e633e551..7c093d84b45 100644 --- a/src/waveform/renderers/waveformmark.cpp +++ b/src/waveform/renderers/waveformmark.cpp @@ -204,13 +204,8 @@ bool WaveformMark::contains(QPoint point, Qt::Orientation orientation) const { // Helper struct to calculate the geometry and fontsize needed by generateImage // to draw the label and text -struct MarkerGeometry { - bool m_isSymbol; // is the label normal text or a single symbol (e.g. open circle arrow) - QFont m_font; - QRectF m_contentRect; - QRectF m_labelRect; - QSizeF m_imageSize; - +class MarkerGeometry { + public: MarkerGeometry(const QString& label, bool useIcon, Qt::Alignment align, @@ -314,6 +309,29 @@ struct MarkerGeometry { return QSize{static_cast(m_imageSize.width() * devicePixelRatio), static_cast(m_imageSize.height() * devicePixelRatio)}; } + + const QFont font() const { + return m_font; + } + + const QRectF& contentRect() const { + return m_contentRect; + } + + const QRectF& labelRect() const { + return m_labelRect; + } + + const QSizeF& imageSize() const { + return m_imageSize; + } + + private: + bool m_isSymbol; // is the label normal text or a single symbol (e.g. open circle arrow) + QFont m_font; + QRectF m_contentRect; + QRectF m_labelRect; + QSizeF m_imageSize; }; QImage WaveformMark::generateImage(float devicePixelRatio) { @@ -355,7 +373,7 @@ QImage WaveformMark::generateImage(float devicePixelRatio) { // Determine drawing geometries const MarkerGeometry markerGeometry{label, useIcon, m_align, m_breadth, m_level}; - m_label.setAreaRect(markerGeometry.m_labelRect); + m_label.setAreaRect(markerGeometry.labelRect()); // Create the image QImage image{markerGeometry.getImageSize(devicePixelRatio), @@ -374,53 +392,53 @@ QImage WaveformMark::generateImage(float devicePixelRatio) { painter.setWorldMatrixEnabled(false); // Draw marker lines - const auto hcenter = markerGeometry.m_imageSize.width() / 2.f; + const auto hcenter = markerGeometry.imageSize().width() / 2.f; m_linePosition = static_cast(hcenter); // Draw the center line painter.setPen(fillColor()); - painter.drawLine(QLineF(hcenter, 0.f, hcenter, markerGeometry.m_imageSize.height())); + painter.drawLine(QLineF(hcenter, 0.f, hcenter, markerGeometry.imageSize().height())); painter.setPen(borderColor()); painter.drawLine(QLineF(hcenter - 1.f, 0.f, hcenter - 1.f, - markerGeometry.m_imageSize.height())); + markerGeometry.imageSize().height())); painter.drawLine(QLineF(hcenter + 1.f, 0.f, hcenter + 1.f, - markerGeometry.m_imageSize.height())); + markerGeometry.imageSize().height())); if (useIcon || label.length() != 0) { painter.setPen(borderColor()); // Draw the label rounded rect with border QPainterPath path; - path.addRoundedRect(markerGeometry.m_labelRect, 2.f, 2.f); + path.addRoundedRect(markerGeometry.labelRect(), 2.f, 2.f); painter.fillPath(path, fillColor()); painter.drawPath(path); // Center m_contentRect.width() and m_contentRect.height() inside m_labelRect // and apply the offset x,y so the text ends up in the centered width,height. - QPointF pos(markerGeometry.m_labelRect.x() + - (markerGeometry.m_labelRect.width() - - markerGeometry.m_contentRect.width()) / + QPointF pos(markerGeometry.labelRect().x() + + (markerGeometry.labelRect().width() - + markerGeometry.contentRect().width()) / 2.f - - markerGeometry.m_contentRect.x(), - markerGeometry.m_labelRect.y() + - (markerGeometry.m_labelRect.height() - - markerGeometry.m_contentRect.height()) / + markerGeometry.contentRect().x(), + markerGeometry.labelRect().y() + + (markerGeometry.labelRect().height() - + markerGeometry.contentRect().height()) / 2.f - - markerGeometry.m_contentRect.y()); + markerGeometry.contentRect().y()); if (useIcon) { QSvgRenderer svgRenderer(m_iconPath); - svgRenderer.render(&painter, QRectF(pos, markerGeometry.m_contentRect.size())); + svgRenderer.render(&painter, QRectF(pos, markerGeometry.contentRect().size())); } else { // Draw the text painter.setBrush(Qt::transparent); painter.setPen(labelColor()); - painter.setFont(markerGeometry.m_font); + painter.setFont(markerGeometry.font()); painter.drawText(pos, label); } From 5f693d0128fb4e92a31de4c0e31348db98540581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 14 Jan 2025 08:16:40 +0100 Subject: [PATCH 3/6] Don't use runtime asserts --- src/waveform/renderers/waveformmark.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/waveform/renderers/waveformmark.cpp b/src/waveform/renderers/waveformmark.cpp index 7c093d84b45..fe13783b25d 100644 --- a/src/waveform/renderers/waveformmark.cpp +++ b/src/waveform/renderers/waveformmark.cpp @@ -335,7 +335,7 @@ class MarkerGeometry { }; QImage WaveformMark::generateImage(float devicePixelRatio) { - assert(needsImageUpdate()); + DEBUG_ASSERT(needsImageUpdate()); // Load the pixmap from file. // If that succeeds loading the text and stroke is skipped. From 5f82622350b700607152087fe0c64b83e1496cc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 14 Jan 2025 08:21:39 +0100 Subject: [PATCH 4/6] Return early if there is nothing to paint. --- .../allshader/waveformrendermark.cpp | 27 ++++++++++++------- src/waveform/renderers/waveformmark.cpp | 7 +++++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/waveform/renderers/allshader/waveformrendermark.cpp b/src/waveform/renderers/allshader/waveformrendermark.cpp index 397792e1802..11d0c0d463d 100644 --- a/src/waveform/renderers/allshader/waveformrendermark.cpp +++ b/src/waveform/renderers/allshader/waveformrendermark.cpp @@ -231,6 +231,11 @@ void allshader::WaveformRenderMark::paintGL() { static_cast(pMark->m_pGraphics.get()) ->texture(); + if (!pTexture->isCreated()) { + // This happens if the height is zero + continue; + } + const float currentMarkPoint = std::round( static_cast( @@ -383,20 +388,22 @@ void allshader::WaveformRenderMark::drawUntilMark(const QMatrix4x4& matrix, floa // Note that in the legacy waveform widgets this is drawn directly // in the WaveformWidgetRenderer itself. Doing it here is cleaner. void allshader::WaveformRenderMark::updatePlayPosMarkTexture() { - float imgwidth; - float imgheight; + const float imgheight = m_waveformRenderer->getBreadth(); + const float imgwidth = 11.f; - const float height = m_waveformRenderer->getBreadth(); - const float devicePixelRatio = m_waveformRenderer->getDevicePixelRatio(); + if (imgheight == 0.0f) { + return; + } + const float devicePixelRatio = m_waveformRenderer->getDevicePixelRatio(); const float lineX = 5.5f; - imgwidth = 11.f; - imgheight = height; - QImage image(static_cast(imgwidth * devicePixelRatio), static_cast(imgheight * devicePixelRatio), QImage::Format_ARGB32_Premultiplied); + if (image.isNull()) { + return; + } image.setDevicePixelRatio(devicePixelRatio); image.fill(QColor(0, 0, 0, 0).rgba()); @@ -417,8 +424,8 @@ void allshader::WaveformRenderMark::updatePlayPosMarkTexture() { // lines next to playpos // Note: don't draw lines where they would overlap the triangles, // otherwise both translucent strokes add up to a darker tone. - painter.drawLine(QLineF(lineX + 1.f, 4.f, lineX + 1.f, height)); - painter.drawLine(QLineF(lineX - 1.f, 4.f, lineX - 1.f, height)); + painter.drawLine(QLineF(lineX + 1.f, 4.f, lineX + 1.f, imgheight)); + painter.drawLine(QLineF(lineX - 1.f, 4.f, lineX - 1.f, imgheight)); // triangle at top edge // Increase line/waveform contrast @@ -433,7 +440,7 @@ void allshader::WaveformRenderMark::updatePlayPosMarkTexture() { painter.setPen(fgColor); painter.setOpacity(1.0); // play position line - painter.drawLine(QLineF(lineX, 0.f, lineX, height)); + painter.drawLine(QLineF(lineX, 0.f, lineX, imgheight)); // triangle at top edge { QPointF baseL = QPointF(lineX - 4.f, 0.f); diff --git a/src/waveform/renderers/waveformmark.cpp b/src/waveform/renderers/waveformmark.cpp index fe13783b25d..8b7a754d842 100644 --- a/src/waveform/renderers/waveformmark.cpp +++ b/src/waveform/renderers/waveformmark.cpp @@ -337,6 +337,10 @@ class MarkerGeometry { QImage WaveformMark::generateImage(float devicePixelRatio) { DEBUG_ASSERT(needsImageUpdate()); + if (m_breadth == 0.0f) { + return {}; + } + // Load the pixmap from file. // If that succeeds loading the text and stroke is skipped. @@ -378,6 +382,9 @@ QImage WaveformMark::generateImage(float devicePixelRatio) { // Create the image QImage image{markerGeometry.getImageSize(devicePixelRatio), QImage::Format_ARGB32_Premultiplied}; + if (image.isNull()) { + return image; + } image.setDevicePixelRatio(devicePixelRatio); // Fill with transparent pixels From 8ae7d2f7206d38b08e63ab97d08ddd838ec2686a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 14 Jan 2025 19:26:03 +0100 Subject: [PATCH 5/6] Improve variable names --- .../renderers/allshader/waveformrendermark.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/waveform/renderers/allshader/waveformrendermark.cpp b/src/waveform/renderers/allshader/waveformrendermark.cpp index 11d0c0d463d..32b352a4fad 100644 --- a/src/waveform/renderers/allshader/waveformrendermark.cpp +++ b/src/waveform/renderers/allshader/waveformrendermark.cpp @@ -388,18 +388,18 @@ void allshader::WaveformRenderMark::drawUntilMark(const QMatrix4x4& matrix, floa // Note that in the legacy waveform widgets this is drawn directly // in the WaveformWidgetRenderer itself. Doing it here is cleaner. void allshader::WaveformRenderMark::updatePlayPosMarkTexture() { - const float imgheight = m_waveformRenderer->getBreadth(); - const float imgwidth = 11.f; + const float imgHeight = m_waveformRenderer->getBreadth(); + const float imgWidth = 11.f; - if (imgheight == 0.0f) { + if (imgHeight == 0.0f) { return; } const float devicePixelRatio = m_waveformRenderer->getDevicePixelRatio(); const float lineX = 5.5f; - QImage image(static_cast(imgwidth * devicePixelRatio), - static_cast(imgheight * devicePixelRatio), + QImage image(static_cast(imgWidth * devicePixelRatio), + static_cast(imgHeight * devicePixelRatio), QImage::Format_ARGB32_Premultiplied); if (image.isNull()) { return; @@ -424,8 +424,8 @@ void allshader::WaveformRenderMark::updatePlayPosMarkTexture() { // lines next to playpos // Note: don't draw lines where they would overlap the triangles, // otherwise both translucent strokes add up to a darker tone. - painter.drawLine(QLineF(lineX + 1.f, 4.f, lineX + 1.f, imgheight)); - painter.drawLine(QLineF(lineX - 1.f, 4.f, lineX - 1.f, imgheight)); + painter.drawLine(QLineF(lineX + 1.f, 4.f, lineX + 1.f, imgHeight)); + painter.drawLine(QLineF(lineX - 1.f, 4.f, lineX - 1.f, imgHeight)); // triangle at top edge // Increase line/waveform contrast @@ -440,7 +440,7 @@ void allshader::WaveformRenderMark::updatePlayPosMarkTexture() { painter.setPen(fgColor); painter.setOpacity(1.0); // play position line - painter.drawLine(QLineF(lineX, 0.f, lineX, imgheight)); + painter.drawLine(QLineF(lineX, 0.f, lineX, imgHeight)); // triangle at top edge { QPointF baseL = QPointF(lineX - 4.f, 0.f); From 4c368f40efcb0c753041e5f7f6f1993ef0cbd85d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 14 Jan 2025 19:30:27 +0100 Subject: [PATCH 6/6] Use VERIFY_OR_DEBUG_ASSERT() to check for valid images --- src/waveform/renderers/allshader/waveformrendermark.cpp | 2 +- src/waveform/renderers/waveformmark.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/waveform/renderers/allshader/waveformrendermark.cpp b/src/waveform/renderers/allshader/waveformrendermark.cpp index 32b352a4fad..31a467aafd8 100644 --- a/src/waveform/renderers/allshader/waveformrendermark.cpp +++ b/src/waveform/renderers/allshader/waveformrendermark.cpp @@ -401,7 +401,7 @@ void allshader::WaveformRenderMark::updatePlayPosMarkTexture() { QImage image(static_cast(imgWidth * devicePixelRatio), static_cast(imgHeight * devicePixelRatio), QImage::Format_ARGB32_Premultiplied); - if (image.isNull()) { + VERIFY_OR_DEBUG_ASSERT(!image.isNull()) { return; } image.setDevicePixelRatio(devicePixelRatio); diff --git a/src/waveform/renderers/waveformmark.cpp b/src/waveform/renderers/waveformmark.cpp index 8b7a754d842..68efa62cf4f 100644 --- a/src/waveform/renderers/waveformmark.cpp +++ b/src/waveform/renderers/waveformmark.cpp @@ -382,7 +382,7 @@ QImage WaveformMark::generateImage(float devicePixelRatio) { // Create the image QImage image{markerGeometry.getImageSize(devicePixelRatio), QImage::Format_ARGB32_Premultiplied}; - if (image.isNull()) { + VERIFY_OR_DEBUG_ASSERT(!image.isNull()) { return image; } image.setDevicePixelRatio(devicePixelRatio);