changeset 2093:6e3ef3aa341e spectrogramparam

Ensure that operations that need to iterate through action maps in order (most pointedly MainWindow::updateLayerShortcutsFor) get to do it in order of construction and not arbitrary pointer order. Fixes incorrect shortcut associations for add-layer/pane shortcuts in some cases

--- a/main/MainWindow.cpp	Tue Nov 13 14:09:25 2018 +0000
+++ b/main/MainWindow.cpp	Tue Nov 13 14:09:30 2018 +0000
@@ -845,6 +845,7 @@
     QMenu *numberingMenu = menu->addMenu(tr("Number New Instants with"));
     numberingMenu->setTearOffEnabled(true);
     QActionGroup *numberingGroup = new QActionGroup(this);
+    m_numberingActions.clear();
 
     Labeller::TypeNameMap types = m_labeller->getTypeNames();
     for (Labeller::TypeNameMap::iterator i = types.begin(); i != types.end(); ++i) {
@@ -858,7 +859,7 @@
         action->setChecked(m_labeller->getType() == i->first);
         numberingGroup->addAction(action);
         numberingMenu->addAction(action);
-        m_numberingActions[action] = (int)i->first;
+        m_numberingActions.push_back({ action, (int)i->first });
 
         if (i->first == Labeller::ValueFromTwoLevelCounter) {
 
@@ -1229,12 +1230,15 @@
 
     m_keyReference->setCategory(tr("Managing Panes and Layers"));
 
+    m_paneActions.clear();
+    m_layerActions.clear();
+    
     QAction *action = new QAction(il.load("pane"), tr("Add &New Pane"), this);
     action->setShortcut(tr("N"));
     action->setStatusTip(tr("Add a new pane containing only a time ruler"));
     connect(action, SIGNAL(triggered()), this, SLOT(addPane()));
     connect(this, SIGNAL(canAddPane(bool)), action, SLOT(setEnabled(bool)));
-    m_paneActions[action] = LayerConfiguration(LayerFactory::TimeRuler);
+    m_paneActions.push_back({ action, LayerConfiguration(LayerFactory::TimeRuler) });
     m_keyReference->registerShortcut(action);
     menu->addAction(action);
 
@@ -1268,7 +1272,7 @@
 
         connect(action, SIGNAL(triggered()), this, SLOT(addLayer()));
         connect(this, SIGNAL(canAddLayer(bool)), action, SLOT(setEnabled(bool)));
-        m_layerActions[action] = LayerConfiguration(type);
+        m_layerActions.push_back({ action, LayerConfiguration(type) });
         menu->addAction(action);
         m_rightButtonLayerMenu->addAction(action);
     }
@@ -1410,15 +1414,15 @@
                                     this, SLOT(addPane()));
                             connect(this, SIGNAL(canAddPane(bool)),
                                     action, SLOT(setEnabled(bool)));
-                            m_paneActions[action] =
-                                LayerConfiguration(type, model);
+                            m_paneActions.push_back
+                                ({ action, LayerConfiguration(type, model) });
                         } else {
                             connect(action, SIGNAL(triggered()),
                                     this, SLOT(addLayer()));
                             connect(this, SIGNAL(canAddLayer(bool)),
                                     action, SLOT(setEnabled(bool)));
-                            m_layerActions[action] =
-                                LayerConfiguration(type, model);
+                            m_layerActions.push_back
+                                ({ action, LayerConfiguration(type, model) });
                         }
                         if (shortcutText != "") {
                             m_keyReference->registerShortcut(action);
@@ -1454,6 +1458,12 @@
                         if (isDefault) {
                             action = new QAction(icon, actionText, this);
                             if (!model || model == getMainModel()) {
+                                // Default for the shortcut is to
+                                // attach to an action that uses the
+                                // main model as input. But this may
+                                // change when the user selects a
+                                // different pane - see
+                                // updateLayerShortcutsFor() below.
                                 action->setShortcut(shortcutText); 
                             }
                         } else {
@@ -1467,15 +1477,15 @@
                                     this, SLOT(addPane()));
                             connect(this, SIGNAL(canAddPane(bool)),
                                     action, SLOT(setEnabled(bool)));
-                            m_paneActions[action] =
-                                LayerConfiguration(type, model, c - 1);
+                            m_paneActions.push_back
+                                ({ action, LayerConfiguration(type, model, c - 1) });
                         } else {
                             connect(action, SIGNAL(triggered()),
                                     this, SLOT(addLayer()));
                             connect(this, SIGNAL(canAddLayer(bool)),
                                     action, SLOT(setEnabled(bool)));
-                            m_layerActions[action] =
-                                LayerConfiguration(type, model, c - 1);
+                            m_layerActions.push_back
+                                ({ action, LayerConfiguration(type, model, c - 1) });
                         }
 
                         submenu->addAction(action);
@@ -1492,7 +1502,8 @@
                                 this, SLOT(addLayer()));
                         connect(this, SIGNAL(canAddLayer(bool)),
                                 action, SLOT(setEnabled(bool)));
-                        m_layerActions[action] = LayerConfiguration(type, 0, 0);
+                        m_layerActions.push_back
+                            ({ action, LayerConfiguration(type, 0, 0) });
                         m_rightButtonLayerMenu->addAction(action);
                     }
                 }
@@ -1537,7 +1548,7 @@
     action->setStatusTip(tr("Add a new layer showing a time ruler"));
     connect(action, SIGNAL(triggered()), this, SLOT(addLayer()));
     connect(this, SIGNAL(canAddLayer(bool)), action, SLOT(setEnabled(bool)));
-    m_layerActions[action] = LayerConfiguration(LayerFactory::TimeRuler);
+    m_layerActions.push_back({ action, LayerConfiguration(LayerFactory::TimeRuler) });
     menu->addAction(action);
 
     menu->addSeparator();
@@ -1607,6 +1618,10 @@
 void
 MainWindow::updateLayerShortcutsFor(Model *model)
 {
+    // Called when e.g. the current pane has changed, to ensure the
+    // various layer shortcuts select an action whose input model is
+    // the active one in this pane
+    
     set<LayerFactory::LayerType> seen;
     
     for (auto &a : m_paneActions) {
@@ -1774,6 +1789,9 @@
         }
     }
 
+    m_transformActions.clear();
+    m_transformActionsReverse.clear();
+    
     for (int i = 0; in_range_for(transforms, i); ++i) {
         
         QString name = transforms[i].name;
@@ -1806,7 +1824,7 @@
 
         QAction *action = new QAction(tr("%1...").arg(name), this);
         connect(action, SIGNAL(triggered()), this, SLOT(addLayer()));
-        m_transformActions[action] = transforms[i].identifier;
+        m_transformActions.push_back({ action, transforms[i].identifier });
         m_transformActionsReverse[transforms[i].identifier] = action;
         connect(this, SIGNAL(canAddLayer(bool)), action, SLOT(setEnabled(bool)));
 
@@ -1832,7 +1850,7 @@
 
         action = new QAction(tr("%1...").arg(output == "" ? pluginName : output), this);
         connect(action, SIGNAL(triggered()), this, SLOT(addLayer()));
-        m_transformActions[action] = transforms[i].identifier;
+        m_transformActions.push_back({ action, transforms[i].identifier });
         connect(this, SIGNAL(canAddLayer(bool)), action, SLOT(setEnabled(bool)));
         action->setStatusTip(transforms[i].longDescription);
 
@@ -2082,7 +2100,7 @@
         QAction *action = new QAction(icon, name, this);
         connect(action, SIGNAL(triggered()), this, SLOT(addLayer()));
         connect(this, SIGNAL(canAddLayer(bool)), action, SLOT(setEnabled(bool)));
-        m_existingLayerActions[action] = layer;
+        m_existingLayerActions.push_back({ action, layer });
 
         m_existingLayersMenu->addAction(action);
 
@@ -2090,7 +2108,7 @@
             action = new QAction(icon, name, this);
             connect(action, SIGNAL(triggered()), this, SLOT(addLayer()));
             connect(this, SIGNAL(canAddLayer(bool)), action, SLOT(setEnabled(bool)));
-            m_sliceActions[action] = layer;
+            m_sliceActions.push_back({ action, layer });
             m_sliceMenu->addAction(action);
         }
     }
@@ -2308,10 +2326,10 @@
 
     toolbar = addToolBar(tr("Tools Toolbar"));
     QActionGroup *group = new QActionGroup(this);
-
+    m_toolActions.clear();
+    
     m_keyReference->setCategory(tr("Tool Selection"));
-    QAction *action = toolbar->addAction(il.load("navigate"),
-                                         tr("Navigate"));
+    QAction *action = toolbar->addAction(il.load("navigate"), tr("Navigate"));
     action->setCheckable(true);
     action->setChecked(true);
     action->setShortcut(tr("1"));
@@ -2320,7 +2338,7 @@
     connect(this, SIGNAL(replacedDocument()), action, SLOT(trigger()));
     group->addAction(action);
     m_keyReference->registerShortcut(action);
-    m_toolActions[ViewManager::NavigateMode] = action;
+    m_toolActions.push_back({ ViewManager::NavigateMode, action });
     
     m_keyReference->setCategory
         (tr("Navigate Tool Mouse Actions"));
@@ -2338,15 +2356,14 @@
          tr("Double-click left button on an item to edit it"));
 
     m_keyReference->setCategory(tr("Tool Selection"));
-    action = toolbar->addAction(il.load("select"),
-                                tr("Select"));
+    action = toolbar->addAction(il.load("select"), tr("Select"));
     action->setCheckable(true);
     action->setShortcut(tr("2"));
     action->setStatusTip(tr("Select ranges"));
     connect(action, SIGNAL(triggered()), this, SLOT(toolSelectSelected()));
     group->addAction(action);
     m_keyReference->registerShortcut(action);
-    m_toolActions[ViewManager::SelectMode] = action;
+    m_toolActions.push_back({ ViewManager::SelectMode, action });
         
     m_keyReference->setCategory
         (tr("Select Tool Mouse Actions"));
@@ -2367,8 +2384,7 @@
         tr("Shift-click left button and drag to select without snapping to items or grid"));
 
     m_keyReference->setCategory(tr("Tool Selection"));
-    action = toolbar->addAction(il.load("move"),
-                                tr("Edit"));
+    action = toolbar->addAction(il.load("move"), tr("Edit"));
     action->setCheckable(true);
     action->setShortcut(tr("3"));
     action->setStatusTip(tr("Edit items in layer"));
@@ -2376,7 +2392,7 @@
     connect(this, SIGNAL(canEditLayer(bool)), action, SLOT(setEnabled(bool)));
     group->addAction(action);
     m_keyReference->registerShortcut(action);
-    m_toolActions[ViewManager::EditMode] = action;
+    m_toolActions.push_back({ ViewManager::EditMode, action });
     
     m_keyReference->setCategory
         (tr("Edit Tool Mouse Actions"));
@@ -2388,8 +2404,7 @@
         tr("Double-click left button on an item to edit it"));
 
     m_keyReference->setCategory(tr("Tool Selection"));
-    action = toolbar->addAction(il.load("draw"),
-                                tr("Draw"));
+    action = toolbar->addAction(il.load("draw"), tr("Draw"));
     action->setCheckable(true);
     action->setShortcut(tr("4"));
     action->setStatusTip(tr("Draw new items in layer"));
@@ -2397,7 +2412,7 @@
     connect(this, SIGNAL(canEditLayer(bool)), action, SLOT(setEnabled(bool)));
     group->addAction(action);
     m_keyReference->registerShortcut(action);
-    m_toolActions[ViewManager::DrawMode] = action;
+    m_toolActions.push_back({ ViewManager::DrawMode, action });
 
     m_keyReference->setCategory
         (tr("Draw Tool Mouse Actions"));
@@ -2406,8 +2421,7 @@
         tr("Click left button and drag to create new item"));
 
     m_keyReference->setCategory(tr("Tool Selection"));
-    action = toolbar->addAction(il.load("erase"),
-                                tr("Erase"));
+    action = toolbar->addAction(il.load("erase"), tr("Erase"));
     action->setCheckable(true);
     action->setShortcut(tr("5"));
     action->setStatusTip(tr("Erase items from layer"));
@@ -2415,7 +2429,7 @@
     connect(this, SIGNAL(canEditLayer(bool)), action, SLOT(setEnabled(bool)));
     group->addAction(action);
     m_keyReference->registerShortcut(action);
-    m_toolActions[ViewManager::EraseMode] = action;
+    m_toolActions.push_back({ ViewManager::EraseMode, action });
 
     m_keyReference->setCategory
         (tr("Erase Tool Mouse Actions"));
@@ -2432,7 +2446,7 @@
     connect(this, SIGNAL(canMeasureLayer(bool)), action, SLOT(setEnabled(bool)));
     group->addAction(action);
     m_keyReference->registerShortcut(action);
-    m_toolActions[ViewManager::MeasureMode] = action;
+    m_toolActions.push_back({ ViewManager::MeasureMode, action });
 
     m_keyReference->setCategory
         (tr("Measure Tool Mouse Actions"));
@@ -3834,13 +3848,17 @@
         return;
     }
 
-    PaneActionMap::iterator i = m_paneActions.find(action);
+    PaneActions::iterator i = m_paneActions.begin();
+    while (i != m_paneActions.end()) {
+        if (i->first == action) break;
+        ++i;
+    }
 
     if (i == m_paneActions.end()) {
         cerr << "WARNING: MainWindow::addPane: unknown action "
-                  << action->objectName() << endl;
+             << action->objectName() << endl;
         cerr << "known actions are:" << endl;
-        for (PaneActionMap::const_iterator i = m_paneActions.begin();
+        for (PaneActions::const_iterator i = m_paneActions.begin();
              i != m_paneActions.end(); ++i) {
             cerr << i->first << ", name " << i->first->text() << endl;
         }
@@ -3943,7 +3961,11 @@
         return;
     }
 
-    ExistingLayerActionMap::iterator ei = m_existingLayerActions.find(action);
+    ExistingLayerActions::iterator ei = m_existingLayerActions.begin();
+    while (ei != m_existingLayerActions.end()) {
+        if (ei->first == action) break;
+        ++ei;
+    }
 
     if (ei != m_existingLayerActions.end()) {
         Layer *newLayer = ei->second;
@@ -3952,7 +3974,11 @@
         return;
     }
 
-    ei = m_sliceActions.find(action);
+    ei = m_sliceActions.begin();
+    while (ei != m_sliceActions.end()) {
+        if (ei->first == action) break;
+        ++ei;
+    }
 
     if (ei != m_sliceActions.end()) {
         Layer *newLayer = m_document->createLayer(LayerFactory::Slice);
@@ -3972,11 +3998,19 @@
         return;
     }
 
-    TransformActionMap::iterator i = m_transformActions.find(action);
+    TransformActions::iterator i = m_transformActions.begin();
+    while (i != m_transformActions.end()) {
+        if (i->first == action) break;
+        ++i;
+    }
 
     if (i == m_transformActions.end()) {
 
-        LayerActionMap::iterator i = m_layerActions.find(action);
+        LayerActions::iterator i = m_layerActions.begin();
+        while (i != m_layerActions.end()) {
+            if (i->first == action) break;
+            ++i;
+        }
         
         if (i == m_layerActions.end()) {
             cerr << "WARNING: MainWindow::addLayer: unknown action "
@@ -3995,7 +4029,12 @@
 
             newLayer = m_document->createEmptyLayer(type);
             if (newLayer) {
-                m_toolActions[ViewManager::DrawMode]->trigger();
+                for (auto &a : m_toolActions) {
+                    if (a.first == ViewManager::DrawMode) {
+                        a.second->trigger();
+                        break;
+                    }
+                }
             }
 
         } else {
@@ -4729,7 +4768,10 @@
     QAction *a = dynamic_cast<QAction *>(sender());
     if (!a) return;
 
-    int type = m_numberingActions[a];
+    int type = 0;
+    for (auto &ai : m_numberingActions) {
+        if (ai.first == a) type = ai.second;
+    }
     
     if (m_labeller) m_labeller->setType(Labeller::ValueType(type));
 

--- a/main/MainWindow.h	Tue Nov 13 14:09:25 2018 +0000
+++ b/main/MainWindow.h	Tue Nov 13 14:09:30 2018 +0000
@@ -249,29 +249,39 @@
 
     QString shortcutFor(LayerFactory::LayerType, bool isPaneMenu);
     void updateLayerShortcutsFor(Model *);
+
+    // Map from menu action to the resulting layer configurations
+    // etc. These all used to be std::maps, but we sometimes want to
+    // iterate through actions in order of creation, not in order of
+    // arbitrary QAction pointer. And speed of random lookup is not
+    // important.
+    //
+    // Some of these would still be fine as maps, but we might as well
+    // consistently use the same arrangement throughout.
     
-    typedef std::map<QAction *, LayerConfiguration> PaneActionMap;
-    PaneActionMap m_paneActions;
+    typedef std::vector<std::pair<QAction *, LayerConfiguration>> PaneActions;
+    PaneActions m_paneActions;
 
-    typedef std::map<QAction *, LayerConfiguration> LayerActionMap;
-    LayerActionMap m_layerActions;
+    typedef std::vector<std::pair<QAction *, LayerConfiguration>> LayerActions;
+    LayerActions m_layerActions;
 
-    typedef std::map<QAction *, TransformId> TransformActionMap;
-    TransformActionMap m_transformActions;
+    typedef std::vector<std::pair<QAction *, Layer *>> ExistingLayerActions;
+    ExistingLayerActions m_existingLayerActions;
+    ExistingLayerActions m_sliceActions;
 
+    typedef std::vector<std::pair<ViewManager::ToolMode, QAction *>> ToolActions;
+    ToolActions m_toolActions;
+
+    typedef std::vector<std::pair<QAction *, int>> NumberingActions;
+    NumberingActions m_numberingActions;
+
+    typedef std::vector<std::pair<QAction *, TransformId>> TransformActions;
+    TransformActions m_transformActions;
+
+    // This one only makes sense as a map though
     typedef std::map<TransformId, QAction *> TransformActionReverseMap;
     TransformActionReverseMap m_transformActionsReverse;
 
-    typedef std::map<QAction *, Layer *> ExistingLayerActionMap;
-    ExistingLayerActionMap m_existingLayerActions;
-    ExistingLayerActionMap m_sliceActions;
-
-    typedef std::map<ViewManager::ToolMode, QAction *> ToolActionMap;
-    ToolActionMap m_toolActions;
-
-    typedef std::map<QAction *, int> NumberingActionMap;
-    NumberingActionMap m_numberingActions;
-
     QString getReleaseText() const;
     
     virtual void setupMenus();