avoid overlapping marks

CMakeLists.txt

@@ -1405,6 +1405,7 @@ else()
     src/waveform/renderers/waveformrendererrgb.cpp
     src/waveform/renderers/waveformrenderersignalbase.cpp
     src/waveform/renderers/waveformrendermark.cpp
+    src/waveform/renderers/waveformrendermarkbase.cpp
     src/waveform/renderers/waveformrendermarkrange.cpp
     src/waveform/renderers/waveformsignalcolors.cpp
     src/waveform/renderers/waveformwidgetrenderer.cpp

src/waveform/renderers/allshader/waveformrendermark.cpp

@@ -1,20 +1,14 @@
 #include "waveform/renderers/allshader/waveformrendermark.h"
 
-#include <QDomNode>
 #include <QOpenGLTexture>
 #include <QPainterPath>
 
-#include "track/track.h"
-#include "util/color/color.h"
 #include "util/colorcomponents.h"
 #include "util/texture.h"
 #include "waveform/renderers/allshader/matrixforwidgetgeometry.h"
-#include "waveform/renderers/allshader/moc_waveformrendermark.cpp"
 #include "waveform/renderers/allshader/rgbadata.h"
 #include "waveform/renderers/allshader/vertexdata.h"
-#include "waveform/renderers/waveformsignalcolors.h"
 #include "waveform/renderers/waveformwidgetrenderer.h"
-#include "widget/wimagestore.h"
 
 // On the use of QPainter:
 //
@@ -37,25 +31,30 @@ class TextureGraphics : public WaveformMark::Graphics {
     }
 };
 
-allshader::WaveformRenderMark::WaveformRenderMark(WaveformWidgetRenderer* waveformWidget)
-        : WaveformRenderer(waveformWidget),
-          m_bCuesUpdates(false) {
-}
+// Both allshader::WaveformRenderMark and the non-GL ::WaveformRenderMark derive
+// from WaveformRenderMarkBase. The base-class takes care of updating the marks
+// when needed and flagging them when their image needs to be updated (resizing,
+// cue changes, position changes)
+//
+// While in the case of ::WaveformRenderMark those images can be updated immediately,
+// in the case of allshader::WaveformRenderMark we need to do that when we have an
+// openGL context, as we create new textures.
+//
+// The boolean argument for the WaveformRenderMarkBase constructor indicates
+// that updateMarkImages should not be called immediately.
 
-void allshader::WaveformRenderMark::setup(const QDomNode& node, const SkinContext& context) {
-    WaveformSignalColors signalColors = *m_waveformRenderer->getWaveformSignalColors();
-    m_marks.setup(m_waveformRenderer->getGroup(), node, context, signalColors);
+allshader::WaveformRenderMark::WaveformRenderMark(WaveformWidgetRenderer* waveformWidget)
+        : ::WaveformRenderMarkBase(waveformWidget, false) {
 }
 
 void allshader::WaveformRenderMark::initializeGL() {
-    WaveformRenderer::initializeGL();
+    allshader::WaveformRendererAbstract::initializeGL();
     m_rgbaShader.init();
     m_textureShader.init();
 
-    for (const auto& pMark : std::as_const(m_marks)) {
-        generateMarkImage(pMark);
-    }
-    generatePlayPosMarkTexture();
+    // Will create textures so requires OpenGL context
+    updateMarkImages();
+    updatePlayPosMarkTexture();
 }
 
 void allshader::WaveformRenderMark::drawTexture(float x, float y, QOpenGLTexture* texture) {
@@ -157,16 +156,13 @@ void allshader::WaveformRenderMark::paintGL() {
     const float devicePixelRatio = m_waveformRenderer->getDevicePixelRatio();
     QList<WaveformWidgetRenderer::WaveformMarkOnScreen> marksOnScreen;
 
-    checkCuesUpdated();
-
     glEnable(GL_BLEND);
     glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
 
-    for (const auto& pMark : std::as_const(m_marks)) {
-        if (!pMark->m_pGraphics || pMark->m_pGraphics->m_obsolete) {
-            generateMarkImage(pMark);
-        }
+    // Will create textures so requires OpenGL context
+    updateMarkImages();
 
+    for (const auto& pMark : std::as_const(m_marks)) {
         QOpenGLTexture* pTexture =
                 static_cast<TextureGraphics*>(pMark->m_pGraphics.get())
                         ->texture();
@@ -238,7 +234,7 @@ void allshader::WaveformRenderMark::paintGL() {
 // Generate the texture used to draw the play position marker.
 // Note that in the legacy waveform widgets this is drawn directly
 // in the WaveformWidgetRenderer itself. Doing it here is cleaner.
-void allshader::WaveformRenderMark::generatePlayPosMarkTexture() {
+void allshader::WaveformRenderMark::updatePlayPosMarkTexture() {
     float imgwidth;
     float imgheight;
 
@@ -317,71 +313,12 @@ void allshader::WaveformRenderMark::drawTriangle(QPainter* painter,
 }
 
 void allshader::WaveformRenderMark::resizeGL(int, int) {
-    for (const auto& pMark : std::as_const(m_marks)) {
-        generateMarkImage(pMark);
-    }
-    generatePlayPosMarkTexture();
-}
-
-void allshader::WaveformRenderMark::onSetTrack() {
-    slotCuesUpdated();
-
-    TrackPointer trackInfo = m_waveformRenderer->getTrackInfo();
-    if (!trackInfo) {
-        return;
-    }
-    connect(trackInfo.get(),
-            &Track::cuesUpdated,
-            this,
-            &allshader::WaveformRenderMark::slotCuesUpdated);
-}
-
-void allshader::WaveformRenderMark::slotCuesUpdated() {
-    m_bCuesUpdates = true;
-}
-
-void allshader::WaveformRenderMark::checkCuesUpdated() {
-    if (!m_bCuesUpdates) {
-        return;
-    }
-    m_bCuesUpdates = false;
-
-    TrackPointer trackInfo = m_waveformRenderer->getTrackInfo();
-    if (!trackInfo) {
-        return;
-    }
-
-    QList<CuePointer> loadedCues = trackInfo->getCuePoints();
-    for (const CuePointer& pCue : loadedCues) {
-        const int hotCue = pCue->getHotCue();
-        if (hotCue == Cue::kNoHotCue) {
-            continue;
-        }
-
-        // Here we assume no two cues can have the same hotcue assigned,
-        // because WaveformMarkSet stores one mark for each hotcue.
-        WaveformMarkPointer pMark = m_marks.getHotCueMark(hotCue);
-        if (pMark.isNull()) {
-            continue;
-        }
-
-        QString newLabel = pCue->getLabel();
-        QColor newColor = mixxx::RgbColor::toQColor(pCue->getColor());
-        if (pMark->m_text.isNull() || newLabel != pMark->m_text ||
-                !pMark->fillColor().isValid() ||
-                newColor != pMark->fillColor()) {
-            pMark->m_text = newLabel;
-            const int dimBrightThreshold = m_waveformRenderer->getDimBrightThreshold();
-            pMark->setBaseColor(newColor, dimBrightThreshold);
-            generateMarkImage(pMark);
-        }
-    }
-
-    m_marks.update();
+    // Will create textures so requires OpenGL context
+    updateMarkImages();
+    updatePlayPosMarkTexture();
 }
 
-void allshader::WaveformRenderMark::generateMarkImage(WaveformMarkPointer pMark) {
+void allshader::WaveformRenderMark::updateMarkImage(WaveformMarkPointer pMark) {
     pMark->m_pGraphics = std::make_unique<TextureGraphics>(
-            createTexture(pMark->generateImage(m_waveformRenderer->getBreadth(),
-                    m_waveformRenderer->getDevicePixelRatio())));
+            createTexture(pMark->generateImage(m_waveformRenderer->getDevicePixelRatio())));
 }

src/waveform/renderers/allshader/waveformrendermark.h

@@ -1,13 +1,11 @@
 #pragma once
 
 #include <QColor>
-#include <QObject>
 
 #include "shaders/rgbashader.h"
 #include "shaders/textureshader.h"
-#include "util/class.h"
-#include "waveform/renderers/allshader/waveformrenderer.h"
-#include "waveform/renderers/waveformmarkset.h"
+#include "waveform/renderers/allshader/waveformrendererabstract.h"
+#include "waveform/renderers/waveformrendermarkbase.h"
 
 class QDomNode;
 class SkinContext;
@@ -17,44 +15,38 @@ namespace allshader {
 class WaveformRenderMark;
 }
 
-class allshader::WaveformRenderMark final : public QObject, public allshader::WaveformRenderer {
-    Q_OBJECT
+class allshader::WaveformRenderMark : public ::WaveformRenderMarkBase,
+                                      public allshader::WaveformRendererAbstract {
   public:
     explicit WaveformRenderMark(WaveformWidgetRenderer* waveformWidget);
 
-    void setup(const QDomNode& node, const SkinContext& context) override;
+    void draw(QPainter* painter, QPaintEvent* event) override {
+        Q_UNUSED(painter);
+        Q_UNUSED(event);
+    }
+
+    allshader::WaveformRendererAbstract* allshaderWaveformRenderer() override {
+        return this;
+    }
 
     void initializeGL() override;
     void paintGL() override;
     void resizeGL(int w, int h) override;
 
-    // Called when a new track is loaded.
-    void onSetTrack() override;
-
-  public slots:
-    // Called when the loaded track's cues are added, deleted or modified and
-    // when a new track is loaded.
-    // It updates the marks' names and regenerates their image if needed.
-    // This method is used for hotcues.
-    void slotCuesUpdated();
-
   private:
-    void checkCuesUpdated();
+    void updateMarkImage(WaveformMarkPointer pMark) override;
 
-    void generateMarkImage(WaveformMarkPointer pMark);
-    void generatePlayPosMarkTexture();
+    void updatePlayPosMarkTexture();
 
     void drawTriangle(QPainter* painter,
             const QBrush& fillColor,
             QPointF p1,
             QPointF p2,
             QPointF p3);
 
-    WaveformMarkSet m_marks;
     mixxx::RGBAShader m_rgbaShader;
     mixxx::TextureShader m_textureShader;
     std::unique_ptr<QOpenGLTexture> m_pPlayPosMarkTexture;
-    bool m_bCuesUpdates;
 
     void drawMark(const QRectF& rect, QColor color);
     void drawTexture(float x, float y, QOpenGLTexture* texture);

src/waveform/renderers/waveformmark.cpp

@@ -10,6 +10,11 @@
 #include "widget/wskincolor.h"
 
 namespace {
+
+// Without some padding, the user would only have a single pixel width that
+// would count as hovering over the WaveformMark.
+constexpr float lineHoverPadding = 5.0;
+
 Qt::Alignment decodeAlignmentFlags(const QString& alignString, Qt::Alignment defaultFlags) {
     QStringList stringFlags = alignString.toLower()
                                       .split('|',
@@ -56,9 +61,10 @@ Qt::Alignment decodeAlignmentFlags(const QString& alignString, Qt::Alignment def
 WaveformMark::WaveformMark(const QString& group,
         const QDomNode& node,
         const SkinContext& context,
+        int priority,
         const WaveformSignalColors& signalColors,
         int hotCue)
-        : m_linePosition{}, m_breadth{}, m_iHotCue{hotCue} {
+        : m_linePosition{}, m_breadth{}, m_level{}, m_iPriority(priority), m_iHotCue(hotCue) {
     QString positionControl;
     QString endPositionControl;
     if (hotCue != Cue::kNoHotCue) {
@@ -121,40 +127,50 @@ WaveformMark::WaveformMark(const QString& group,
 WaveformMark::~WaveformMark() = default;
 
 void WaveformMark::setBaseColor(QColor baseColor, int dimBrightThreshold) {
-    if (m_pGraphics) {
-        m_pGraphics->m_obsolete = true;
+    if (m_fillColor == baseColor) {
+        return;
     }
+
     m_fillColor = baseColor;
     m_borderColor = Color::chooseContrastColor(baseColor, dimBrightThreshold);
     m_labelColor = Color::chooseColorByBrightness(baseColor,
             QColor(255, 255, 255, 255),
             QColor(0, 0, 0, 255),
             dimBrightThreshold);
-};
 
-bool WaveformMark::contains(QPoint point, Qt::Orientation orientation) const {
-    // Without some padding, the user would only have a single pixel width that
-    // would count as hovering over the WaveformMark.
-    float lineHoverPadding = 5.0;
+    setNeedsImageUpdate();
+}
+
+bool WaveformMark::lineHovered(QPoint point, Qt::Orientation orientation) const {
     if (orientation == Qt::Vertical) {
-        point = QPoint(point.y(), m_breadth - point.x());
+        // Note that for vertical orientation, breadth is set to the width.
+        point = QPoint(point.y(), static_cast<int>(m_breadth) - point.x());
     }
-    bool lineHovered = m_linePosition >= point.x() - lineHoverPadding &&
+    return m_linePosition >= point.x() - lineHoverPadding &&
             m_linePosition <= point.x() + lineHoverPadding;
+}
 
-    return m_label.area().contains(point) || lineHovered;
+bool WaveformMark::contains(QPoint point, Qt::Orientation orientation) const {
+    if (orientation == Qt::Vertical) {
+        point = QPoint(point.y(), static_cast<int>(m_breadth) - point.x());
+    }
+    return m_label.area().contains(point);
 }
 
 // Helper struct to calculate the geometry and fontsize needed by generateImage
 // to draw the label and text
 struct MarkerGeometry {
-    bool m_isSymbol; // it the label normal text or a single symbol (e.g. open circle arrow)
+    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;
 
-    MarkerGeometry(const QString& label, bool useIcon, Qt::Alignment align, float breadth) {
+    MarkerGeometry(const QString& label,
+            bool useIcon,
+            Qt::Alignment align,
+            float breadth,
+            int level) {
         // If the label is 1 character long, and this character isn't a letter or a number,
         // we can assume it's a special symbol
         m_isSymbol = !useIcon && label.length() == 1 && !label[0].isLetterOrNumber();
@@ -240,9 +256,10 @@ struct MarkerGeometry {
         if (alignV == Qt::AlignVCenter) {
             m_labelRect.moveTop((m_imageSize.height() - m_labelRect.height()) / 2.f);
         } else if (alignV == Qt::AlignBottom) {
-            m_labelRect.moveBottom(m_imageSize.height() - 0.5f);
+            m_labelRect.moveBottom(m_imageSize.height() - 0.5f -
+                    level * (m_labelRect.height() + 2.f));
         } else {
-            m_labelRect.moveTop(0.5f);
+            m_labelRect.moveTop(0.5f + level * (m_labelRect.height() + 2.f));
         }
     }
     QSize getImageSize(float devicePixelRatio) const {
@@ -251,12 +268,12 @@ struct MarkerGeometry {
     }
 };
 
-QImage WaveformMark::generateImage(float breadth, float devicePixelRatio) {
+QImage WaveformMark::generateImage(float devicePixelRatio) {
+    assert(needsImageUpdate());
+
     // Load the pixmap from file.
     // If that succeeds loading the text and stroke is skipped.
 
-    m_breadth = static_cast<int>(breadth);
-
     if (!m_pixmapPath.isEmpty()) {
         QString path = m_pixmapPath;
         // Use devicePixelRatio to properly scale the image
@@ -292,7 +309,7 @@ QImage WaveformMark::generateImage(float breadth, float devicePixelRatio) {
     const bool useIcon = m_iconPath != "";
 
     // Determine drawing geometries
-    const MarkerGeometry markerGeometry(label, useIcon, m_align, breadth);
+    const MarkerGeometry markerGeometry{label, useIcon, m_align, m_breadth, m_level};
 
     m_label.setAreaRect(markerGeometry.m_labelRect);