Marker display and handling improvements

- prevent interaction with invisible markers on graph
- helper markers get their own (reduced) context menu
- only update required columns in table when marker data changes
- persistent helper markers for peak table
- New format for peak table markers

Software/PC_Application/LibreVNA-GUI/Traces/Marker/marker.cpp

@@ -130,6 +130,7 @@ QString Marker::formatToString(Marker::Format f)
     case Format::Inductance: return "Inductance";
     case Format::QualityFactor: return "Quality Factor";
     case Format::GroupDelay: return "Group Delay";
+    case Format::NumberOfPeaks: return "Number of peaks";
     case Format::TOI: return "Third order intercept";
     case Format::AvgTone: return "Average Tone Level";
     case Format::AvgModulationProduct: return "Average Modulation Product Level";
@@ -194,8 +195,6 @@ std::vector<Marker::Format> Marker::applicableFormats()
         case Type::Delta:
         case Type::Maximum:
         case Type::Minimum:
-        case Type::PeakTable:
-        case Type::NegativePeakTable:
             if(Trace::isSAParameter(parentTrace->liveParameter())) {
                 ret.push_back(Format::dBm);
                 ret.push_back(Format::dBuV);
@@ -218,6 +217,10 @@ std::vector<Marker::Format> Marker::applicableFormats()
                 }
             }
             break;
+        case Type::PeakTable:
+        case Type::NegativePeakTable:
+            ret.push_back(Format::NumberOfPeaks);
+            break;
         default:
             break;
         }
@@ -228,8 +231,6 @@ std::vector<Marker::Format> Marker::applicableFormats()
         case Type::Delta:
         case Type::Maximum:
         case Type::Minimum:
-        case Type::PeakTable:
-        case Type::NegativePeakTable:
             if(Trace::isSAParameter(parentTrace->liveParameter())) {
                 ret.push_back(Format::dBm);
                 ret.push_back(Format::dBuV);
@@ -253,6 +254,10 @@ std::vector<Marker::Format> Marker::applicableFormats()
                 }
             }
             break;
+        case Type::PeakTable:
+        case Type::NegativePeakTable:
+            ret.push_back(Format::NumberOfPeaks);
+            break;
         case Type::Bandpass:
             ret.push_back(Format::CenterBandwidth);
             ret.push_back(Format::InsertionLoss);
@@ -451,7 +456,11 @@ QString Marker::readableData(Format f)
         switch(type) {
         case Type::PeakTable:
         case Type::NegativePeakTable:
-            return "Found " + QString::number(helperMarkers.size()) + " peaks";
+            switch(f) {
+            case Format::NumberOfPeaks: return "Found " + QString::number(helperMarkers.size()) + " peak" + (helperMarkers.size() == 1 ? "" : "s");
+            default: return "Invalid";
+            }
+            break;
         case Type::Delta: {
             if(!delta) {
                 return "Invalid delta marker";
@@ -588,6 +597,7 @@ QString Marker::readableData(Format f)
             case Format::maxDeltaPos:
                 return "max. Δ+:"+Unit::ToString(maxDeltaPos, "dB", " ", 4);
                 break;
+            case Format::NumberOfPeaks:
             case Format::Last:
                 return "Invalid";
             }
@@ -892,10 +902,6 @@ void Marker::deltaDeleted()
 
 void Marker::updateContextmenu()
 {
-    if(parent) {
-        // do nothing, using contextmenu from parent anyway
-        return;
-    }
     // check if the contextmenu or one of its submenus is currently open
     auto *activeWidget = QApplication::activePopupWidget();
     while (activeWidget) {
@@ -910,19 +916,22 @@ void Marker::updateContextmenu()
     contextmenu.clear();
     contextmenu.addSection("Marker");
 
-    auto typemenu = contextmenu.addMenu("Type");
+    if(!parent) {
-    auto typegroup = new QActionGroup(&contextmenu);
+        // type can only be changed for top level markers
-    for(auto t : getSupportedTypes()) {
+        auto typemenu = contextmenu.addMenu("Type");
-        auto setTypeAction = new QAction(typeToString(t), typemenu);
+        auto typegroup = new QActionGroup(&contextmenu);
-        setTypeAction->setCheckable(true);
+        for(auto t : getSupportedTypes()) {
-        if(t == type) {
+            auto setTypeAction = new QAction(typeToString(t), typemenu);
-            setTypeAction->setChecked(true);
+            setTypeAction->setCheckable(true);
+            if(t == type) {
+                setTypeAction->setChecked(true);
+            }
+            connect(setTypeAction, &QAction::triggered, [=](){
+                setType(t);
+            });
+            typegroup->addAction(setTypeAction);
+            typemenu->addAction(setTypeAction);
         }
-        connect(setTypeAction, &QAction::triggered, [=](){
-            setType(t);
-        });
-        typegroup->addAction(setTypeAction);
-        typemenu->addAction(setTypeAction);
     }
 
     auto table = contextmenu.addMenu("Data Format in Table");
@@ -965,48 +974,51 @@ void Marker::updateContextmenu()
         }
     }
 
-    contextmenu.addSeparator();
+    if(!parent) {
-
+        // grouping and deleting is only possible for top level markers
-    bool needsSeparator = false;
-    if((applicableGroups.size() > 0 && group == nullptr) || applicableGroups.size() > 1) {
-        // there are other groups available than the one the marker might already be assigned to
-        auto addGroupMenu = new QMenu("Add to linked group");
-        auto groupGroup = new QActionGroup(addGroupMenu);
-        for(auto g : model->getGroups()) {
-            auto addGroupAction = new QAction(QString::number(g->getNumber()));
-            groupGroup->addAction(addGroupAction);
-            addGroupAction->setCheckable(true);
-            if(g == group) {
-                // already assigned to this group
-                addGroupAction->setChecked(true);
-            }
-            connect(addGroupAction, &QAction::triggered, [=](bool checked){
-                if(checked) {
-                    g->add(this);
-                }
-            });
-            addGroupMenu->addAction(addGroupAction);
-        }
-        contextmenu.addMenu(addGroupMenu);
-        needsSeparator = true;
-    }
-    if(group != nullptr) {
-        // "remove from group" available
-        auto removeGroup = new QAction("Remove from linked group", &contextmenu);
-        connect(removeGroup, &QAction::triggered, [=](){
-            group->remove(this);
-        });
-        contextmenu.addAction(removeGroup);
-        needsSeparator = true;
-    }
-    if(needsSeparator) {
         contextmenu.addSeparator();
+
+        bool needsSeparator = false;
+        if((applicableGroups.size() > 0 && group == nullptr) || applicableGroups.size() > 1) {
+            // there are other groups available than the one the marker might already be assigned to
+            auto addGroupMenu = new QMenu("Add to linked group");
+            auto groupGroup = new QActionGroup(addGroupMenu);
+            for(auto g : model->getGroups()) {
+                auto addGroupAction = new QAction(QString::number(g->getNumber()));
+                groupGroup->addAction(addGroupAction);
+                addGroupAction->setCheckable(true);
+                if(g == group) {
+                    // already assigned to this group
+                    addGroupAction->setChecked(true);
+                }
+                connect(addGroupAction, &QAction::triggered, [=](bool checked){
+                    if(checked) {
+                        g->add(this);
+                    }
+                });
+                addGroupMenu->addAction(addGroupAction);
+            }
+            contextmenu.addMenu(addGroupMenu);
+            needsSeparator = true;
+        }
+        if(group != nullptr) {
+            // "remove from group" available
+            auto removeGroup = new QAction("Remove from linked group", &contextmenu);
+            connect(removeGroup, &QAction::triggered, [=](){
+                group->remove(this);
+            });
+            contextmenu.addAction(removeGroup);
+            needsSeparator = true;
+        }
+        if(needsSeparator) {
+            contextmenu.addSeparator();
+        }
+
+
+        auto deleteAction = new QAction("Delete", &contextmenu);
+        connect(deleteAction, &QAction::triggered, this, &Marker::deleteLater);
+        contextmenu.addAction(deleteAction);
     }
-
-
-    auto deleteAction = new QAction("Delete", &contextmenu);
-    connect(deleteAction, &QAction::triggered, this, &Marker::deleteLater);
-    contextmenu.addAction(deleteAction);
 }
 
 void Marker::traceTypeChanged()
@@ -1231,6 +1243,7 @@ void Marker::setType(Marker::Type t)
         helper->suffix = h.suffix;
         helper->assignTrace(parentTrace);
         helper->setType(h.type);
+        helper->setVisible(visible);
         helperMarkers.push_back(helper);
     }
     if(type == Type::Flatness) {
@@ -1771,11 +1784,7 @@ void Marker::setVisible(bool visible)
 }
 
 QMenu *Marker::getContextMenu() {
-    if(parent) {
+    return &contextmenu;
-        return parent->getContextMenu();
-    } else {
-        return &contextmenu;
-    }
 }
 
 void Marker::update()
@@ -1800,19 +1809,28 @@ void Marker::update()
         break;
     case Type::PeakTable:
     case Type::NegativePeakTable: {
-        deleteHelperMarkers();
         auto peaks = parentTrace->findPeakFrequencies(100, peakThreshold, 3.0, xmin, xmax, type == Type::NegativePeakTable);
-        char suffix = 'a';
+        for(unsigned int i=0;i<peaks.size();i++) {
-        for(auto p : peaks) {
+            if(helperMarkers.size() <= i) {
-            auto helper = new Marker(model, number, this);
+                // needs to create a new helper marker
-            helper->suffix = suffix;
+                auto helper = new Marker(model, number, this);
-            helper->assignTrace(parentTrace);
+                helper->suffix = QChar('a' + i);
-            helper->setPosition(p);
+                helper->assignTrace(parentTrace);
-            helper->formatTable = formatTable;
+                helper->updateContextmenu();
-            helper->formatGraph = formatGraph;
+                helper->setVisible(visible);
-            helper->updateContextmenu();
+                helperMarkers.push_back(helper);
-            suffix++;
+            }
-            helperMarkers.push_back(helper);
+            // update the position of the helper marker
+            helperMarkers[i]->setPosition(peaks[i]);
+        }
+        if(helperMarkers.size() > peaks.size()) {
+            // need to remove some helper markers
+            emit beginRemoveHelperMarkers(this);
+            for(unsigned int i = peaks.size(); i< helperMarkers.size();i++) {
+                delete helperMarkers[i];
+            }
+            helperMarkers.resize(peaks.size());
+            emit endRemoveHelperMarkers(this);
         }
     }
         break;

Software/PC_Application/LibreVNA-GUI/Traces/Marker/marker.h

@@ -35,6 +35,8 @@ public:
         Inductance,
         QualityFactor,
         GroupDelay,
+        // Peak table
+        NumberOfPeaks,
         // Noise marker parameters
         Noise,
         PhaseNoise,

Software/PC_Application/LibreVNA-GUI/Traces/Marker/markermodel.cpp

@@ -132,11 +132,11 @@ void MarkerModel::markerDataChanged(Marker *m)
         // only update the other columns, do not override editor data
         emit dataChanged(index(row, ColIndexData), index(row, ColIndexData));
     } else {
-        emit dataChanged(index(row, ColIndexNumber), index(row, ColIndexData));
+        emit dataChanged(index(row, ColIndexSettings), index(row, ColIndexData));
         // also update any potential helper markers
         for(unsigned int i=0;i<m->getHelperMarkers().size();i++) {
             auto modelIndex = createIndex(i, 0, m);
-            emit dataChanged(index(i, ColIndexNumber, modelIndex), index(i, ColIndexData, modelIndex));
+            emit dataChanged(index(i, ColIndexSettings, modelIndex), index(i, ColIndexData, modelIndex));
         }
     }
 }

Software/PC_Application/LibreVNA-GUI/Traces/traceplot.cpp

@@ -551,6 +551,10 @@ Marker *TracePlot::markerAtPosition(QPoint p, bool onlyMovable)
         }
         auto markers = t.first->getMarkers();
         for(Marker* m : markers) {
+            if(!m->isVisible()) {
+                // can not interact with invisible markers, pretend that there is nothing here
+                continue;
+            }
             if(!m->isMovable() && onlyMovable) {
                 continue;
             }
@@ -563,14 +567,7 @@ Marker *TracePlot::markerAtPosition(QPoint p, bool onlyMovable)
             unsigned int distance = diff.x() * diff.x() + diff.y() * diff.y();
             if(distance < closestDistance) {
                 closestDistance = distance;
-                if(m->getParent()) {
+                closestMarker = m;
-                    closestMarker = m->getParent();
-                    if(closestMarker->getType() == Marker::Type::Flatness) {
-                        closestMarker = m;
-                    }
-                } else {
-                    closestMarker = m;
-                }
             }
         }
     }