changeset 199:0ba33d6c0a71

Properly rationalise the show/hide/add/remove layer logic for pitch candidates: ctrl+return maps through to a show layer command (but the layers remain in the view) while the add/remove layer flow is used for layer creation after selection and for abandoning an edit or discarding a selection.

--- a/.hgsubstate	Wed Mar 05 11:39:28 2014 +0000
+++ b/.hgsubstate	Wed Mar 05 13:20:26 2014 +0000
@@ -2,4 +2,4 @@
 27d4e7152c954bf3c4387319db088fb3cd02436b sv-dependency-builds
 0c37405cf579c6cc87f5199044c2117baa977023 svapp
 d81c16e47e22b1c1a6600d3e9a8e0559fad8b539 svcore
-f831ca41d4a556cbc75aa5a180cf1e6a8d5598b5 svgui
+558c71a802d46d34e784bf184d4be661900f475c svgui

--- a/src/Analyser.cpp	Wed Mar 05 11:39:28 2014 +0000
+++ b/src/Analyser.cpp	Wed Mar 05 13:20:26 2014 +0000
@@ -32,6 +32,7 @@
 #include "layer/LayerFactory.h"
 #include "layer/SpectrogramLayer.h"
 #include "layer/Colour3DPlotLayer.h"
+#include "layer/ShowLayerCommand.h"
 
 #include <QSettings>
 
@@ -255,9 +256,8 @@
 {
     if (sel == m_reAnalysingSelection) return "";
 
-    showPitchCandidates(false);
-    m_reAnalysisCandidates.clear();
-    m_currentCandidate = -1;
+    discardPitchCandidates();
+
     m_reAnalysingSelection = sel;
 
     m_preAnalysis = Clipboard();
@@ -329,9 +329,13 @@
 
     foreach (Layer *layer, m_reAnalysisCandidates) {
         if (shown) {
-            m_document->addLayerToView(m_pane, layer);
+            CommandHistory::getInstance()->addCommand
+                (new ShowLayerCommand(m_pane, layer, true,
+                                      tr("Show Pitch Candidates")));
         } else {
-            m_document->removeLayerFromView(m_pane, layer);
+            CommandHistory::getInstance()->addCommand
+                (new ShowLayerCommand(m_pane, layer, false,
+                                      tr("Hide Pitch Candidates")));
         }
     }
 
@@ -345,6 +349,9 @@
     //!!! how do we know these came from the right selection? user
     //!!! might have made another one since this request was issued
 
+    CommandHistory::getInstance()->startCompoundOperation
+        (tr("Re-Analyse Selection"), true);
+
     vector<Layer *> all;
     for (int i = 0; i < (int)primary.size(); ++i) {
         all.push_back(primary[i]);
@@ -362,13 +369,19 @@
             }
             t->setBaseColour
                 (ColourDatabase::getInstance()->getColourIndex(tr("Bright Orange")));
-            if (m_candidatesVisible) {
-                m_document->addLayerToView(m_pane, t);
-            }
+            m_document->addLayerToView(m_pane, t);
             m_reAnalysisCandidates.push_back(t);
         }
     }
 
+    if (!all.empty()) {
+        bool show = m_candidatesVisible;
+        m_candidatesVisible = !show; // to ensure the following takes effect
+        showPitchCandidates(show);
+    }
+
+    CommandHistory::getInstance()->endCompoundOperation();
+
     emit layersChanged();
 }
 
@@ -459,13 +472,9 @@
 }
 
 void
-Analyser::clearReAnalysis(Selection sel)
+Analyser::abandonReAnalysis(Selection sel)
 {
-    showPitchCandidates(false);
-
-    m_reAnalysisCandidates.clear();
-    m_reAnalysingSelection = Selection();
-    m_currentCandidate = -1;
+    discardPitchCandidates();
 
     Layer *myLayer = m_layers[PitchTrack];
     if (!myLayer) return;
@@ -474,6 +483,21 @@
 }    
 
 void
+Analyser::discardPitchCandidates()
+{
+    foreach (Layer *layer, m_reAnalysisCandidates) {
+        // This will cause the layer to be deleted later (ownership is
+        // transferred to the remove command)
+        m_document->removeLayerFromView(m_pane, layer);
+    }
+
+    m_reAnalysisCandidates.clear();
+    m_currentCandidate = -1;
+    m_reAnalysingSelection = Selection();
+    m_candidatesVisible = false;
+}
+
+void
 Analyser::takePitchTrackFrom(Layer *otherLayer)
 {
     Layer *myLayer = m_layers[PitchTrack];

--- a/src/Analyser.h	Wed Mar 05 11:39:28 2014 +0000
+++ b/src/Analyser.h	Wed Mar 05 13:20:26 2014 +0000
@@ -104,29 +104,25 @@
 
     /**
      * Return true if the analysed pitch candidates are currently
-     * visible (by default they are hidden after construction until
-     * the user requests them). Note that the shown/hidden state is
-     * independent of whether any pitch candidates actually exist --
-     * it's possible they might be shown but not have been created yet
-     * because creation (through reAnalyseSelection) is asynchronous.
-     *
-     *!!! this interface is not right
+     * visible (they are hidden from the call to reAnalyseSelection
+     * until they are requested through showPitchCandidates()). Note
+     * that this may return true even when no pitch candidate layers
+     * actually exist yet, because they are constructed
+     * asynchronously. If that is the case, then the layers will
+     * appear when they are created (otherwise they will remain hidden
+     * after creation).
      */
     bool arePitchCandidatesShown() const;
 
     /**
-     * Show or hide the analysed pitch candidate layers. As in
-     * arePitchCandidatesShown, this is independent of whether the
-     * candidate layers actually exist. Call reAnalyseSelection to
-     * schedule creation of those layers.
-     *
-     *!!! this interface is not right
+     * Show or hide the analysed pitch candidate layers. This is reset
+     * (to "hide") with each new call to reAnalyseSelection. Because
+     * the layers are created asynchronously, setting this to true
+     * does not guarantee that they appear immediately, only that they
+     * will appear once they have been created.
      */
     void showPitchCandidates(bool shown);
 
-    bool haveHigherPitchCandidate() const;
-    bool haveLowerPitchCandidate() const;
-
     /**
      * If a re-analysis has been activated, switch the selected area
      * of the main pitch track to a different candidate from the
@@ -135,6 +131,24 @@
     void switchPitchCandidate(Selection sel, bool up);
 
     /**
+     * Return true if it is possible to switch up to another pitch
+     * candidate. This may mean that the currently selected pitch
+     * candidate is not the highest, or it may mean that no alternate
+     * pitch candidate has been selected at all yet (but some are
+     * available).
+     */
+    bool haveHigherPitchCandidate() const;
+
+    /**
+     * Return true if it is possible to switch down to another pitch
+     * candidate. This may mean that the currently selected pitch
+     * candidate is not the lowest, or it may mean that no alternate
+     * pitch candidate has been selected at all yet (but some are
+     * available).
+     */
+    bool haveLowerPitchCandidate() const;
+
+    /**
      * Delete the pitch estimates from the selected area of the main
      * pitch track.
      */
@@ -147,12 +161,13 @@
     void shiftOctave(Selection sel, bool up);
 
     /**
-     * Remove any re-analysis layers (equivalent to
-     * showPitchCandidates(false)) and also reset the pitch track in
+     * Remove any re-analysis layers and also reset the pitch track in
      * the given selection to its state prior to the last re-analysis,
-     * abandoning any changes made since then.
+     * abandoning any changes made since then. No re-analysis layers
+     * will be available until after the next call to
+     * reAnalyseSelection.
      */
-    void clearReAnalysis(Selection sel);
+    void abandonReAnalysis(Selection sel);
 
     /**
      * Import the pitch track from the given layer into our
@@ -189,6 +204,8 @@
     QString addWaveform();
     QString addAnalyses();
 
+    void discardPitchCandidates();
+    
     // Document::LayerCreationHandler method
     void layersCreated(std::vector<Layer *>, std::vector<Layer *>);
 

--- a/src/MainWindow.cpp	Wed Mar 05 11:39:28 2014 +0000
+++ b/src/MainWindow.cpp	Wed Mar 05 13:20:26 2014 +0000
@@ -1788,12 +1788,12 @@
 
     cerr << "MainWindow::abandonSelection()" << endl;
 
-    CommandHistory::getInstance()->startCompoundOperation(tr("Clear Selection"), true);
+    CommandHistory::getInstance()->startCompoundOperation(tr("Abandon Selection"), true);
 
     MultiSelection::SelectionList selections = m_viewManager->getSelections();
     if (!selections.empty()) {
         Selection sel = *selections.begin();
-        m_analyser->clearReAnalysis(sel);
+        m_analyser->abandonReAnalysis(sel);
     }
 
     MainWindowBase::clearSelection();
@@ -1909,7 +1909,12 @@
 void
 MainWindow::togglePitchCandidates()
 {
+    CommandHistory::getInstance()->startCompoundOperation(tr("Toggle Pitch Candidates"), true);
+
     m_analyser->showPitchCandidates(!m_analyser->arePitchCandidatesShown());
+
+    CommandHistory::getInstance()->endCompoundOperation();
+
     updateMenuStates();
 }