Bugs 1043, 1044 -- Be careful with new display modes in preferences

Write a new key so version 2.1.0 isn't confused by the reordered values

Add some future proofing against similar to 2.1.1 code

src/TrackPanel.cpp

@@ -8992,12 +8992,12 @@ void TrackPanel::OnSetDisplay(wxCommandEvent & event)
    id -= OnWaveformID;
    WaveTrack *wt = (WaveTrack *) mPopupMenuTarget;
    if (wt->GetDisplay() != id) {
-      wt->SetDisplay(id);
+      wt->SetDisplay(WaveTrack::WaveTrackDisplay(id));
       mTrackArtist->InvalidateSpectrumCache(wt);
 
       WaveTrack *l = (WaveTrack *) wt->GetLink();
       if (l) {
-         l->SetDisplay(id);
+         l->SetDisplay(WaveTrack::WaveTrackDisplay(id));
          mTrackArtist->InvalidateSpectrumCache(l);
       }
 #ifdef EXPERIMENTAL_OUTPUT_DISPLAY

src/WaveTrack.cpp

@@ -84,7 +84,7 @@ WaveTrack::WaveTrack(DirManager *projDirManager, sampleFormat format, double rat
       rate = GetActiveProject()->GetRate();
    }
 
-   gPrefs->Read(wxT("/GUI/DefaultViewMode"), &mDisplay, 0);
+   mDisplay = FindDefaultViewMode();
 
    mLegacyProjectFileOffset = 0;
 
@@ -106,8 +106,8 @@ WaveTrack::WaveTrack(DirManager *projDirManager, sampleFormat format, double rat
 WaveTrack::WaveTrack(WaveTrack &orig):
    Track(orig)
 {
-   gPrefs->Read(wxT("/GUI/DefaultViewMode"), &mDisplay, 0);
+   mDisplay = FindDefaultViewMode();
-   mLastDisplay=-1;
+   mLastDisplay = -1;
 
    mLegacyProjectFileOffset = 0;
 
@@ -179,6 +179,73 @@ void WaveTrack::SetOffset(double o)
    mOffset = o;
 }
 
+//static
+WaveTrack::WaveTrackDisplay WaveTrack::FindDefaultViewMode()
+{
+   // PRL:  Bugs 1043, 1044
+   // 2.1.1 writes a new key for this preference, which got new values,
+   // to avoid confusing version 2.1.0 if it reads the preference file afterwards.
+   // Prefer the new preference key if it is present
+
+   WaveTrack::WaveTrackDisplay viewMode;
+   gPrefs->Read(wxT("/GUI/DefaultViewModeNew"), (int*)&viewMode, -1);
+
+   // Default to the old key only if not, default the value if it's not there either
+   wxASSERT(WaveTrack::MinDisplay >= 0);
+   if (viewMode < 0) {
+      int oldMode;
+      gPrefs->Read(wxT("/GUI/DefaultViewMode"), &oldMode,
+         int(WaveTrack::WaveformDisplay));
+      viewMode = WaveTrack::ConvertLegacyDisplayValue(oldMode);
+   }
+
+   // Now future-proof 2.1.1 against a recurrence of this sort of bug!
+   viewMode = WaveTrack::ValidateWaveTrackDisplay(viewMode);
+
+   return viewMode;
+}
+
+// static
+WaveTrack::WaveTrackDisplay
+WaveTrack::ConvertLegacyDisplayValue(int oldValue)
+{
+   // Remap old values.
+   enum OldValues {
+      Waveform,
+      WaveformDB,
+      Spectrogram,
+      SpectrogramLogF,
+      Pitch,
+   };
+
+   WaveTrackDisplay newValue;
+   switch (oldValue) {
+   default:
+   case Waveform:
+      newValue = WaveTrack::WaveformDisplay; break;
+   case WaveformDB:
+      newValue = WaveTrack::WaveformDBDisplay; break;
+   case Spectrogram:
+      newValue = WaveTrack::SpectrumDisplay; break;
+   case SpectrogramLogF:
+      newValue = WaveTrack::SpectrumLogDisplay; break;
+   case Pitch:
+      newValue = WaveTrack::PitchDisplay; break;
+   }
+   return newValue;
+}
+
+// static
+WaveTrack::WaveTrackDisplay
+WaveTrack::ValidateWaveTrackDisplay(WaveTrackDisplay display)
+{
+   // To do, in future:  detect obsolete values between min and max
+   if (display >= int(MinDisplay) && display <= int(MaxDisplay))
+      return display;
+   else
+      return MinDisplay;
+}
+
 void WaveTrack::GetDisplayBounds(float *min, float *max)
 {
    *min = mDisplayMin;

src/WaveTrack.h

@@ -404,20 +404,40 @@ class AUDACITY_DLL_API WaveTrack: public Track {
    // and will be taken out of the WaveTrack class:
    //
 
-   enum {
+   enum WaveTrackDisplay {
-      WaveformDisplay,
+
+      // DO NOT REORDER OLD VALUES!  Replace obsoletes with placeholders.
+
+      WaveformDisplay = 0,
+      MinDisplay = WaveformDisplay,
+
       WaveformDBDisplay,
       SpectrumDisplay,
       SpectrumLogDisplay,
       SpectralSelectionDisplay,
       SpectralSelectionLogDisplay,
       PitchDisplay,
-      NoDisplay            // Preview track has no display
-   } WaveTrackDisplay;
 
-   void SetDisplay(int display) {
+      // Add values here, and update MaxDisplay.
-      if(mDisplay<2)
+
-         mLastDisplay=mDisplay;    // remember last display mode for wave and wavedb so they can remap
+      MaxDisplay = PitchDisplay,
+
+      NoDisplay,            // Preview track has no display
+   };
+
+   // Read appropriate value from preferences
+   static WaveTrackDisplay FindDefaultViewMode();
+
+   // Handle remapping of enum values from 2.1.0 and earlier
+   static WaveTrackDisplay ConvertLegacyDisplayValue(int oldValue);
+
+   // Handle restriction of range of values of the enum from future versions
+   static WaveTrackDisplay ValidateWaveTrackDisplay(WaveTrackDisplay display);
+
+   void SetDisplay(WaveTrackDisplay display) {
+      if(mDisplay < 2)
+         // remember last display mode for wave and wavedb so they can remap the vertical ruler
+         mLastDisplay = mDisplay;
       mDisplay = display;
       if( mDisplay == SpectralSelectionDisplay ){
       }
@@ -450,7 +470,7 @@ class AUDACITY_DLL_API WaveTrack: public Track {
    //
    float         mDisplayMin;
    float         mDisplayMax;
-   int           mDisplay; // type of display, from WaveTrackDisplay enum
+   WaveTrackDisplay mDisplay;
    int           mLastDisplay; // last display mode
    int           mDisplayNumLocations;
    int           mDisplayNumLocationsAllocated;

src/prefs/TracksPrefs.cpp

@@ -20,16 +20,24 @@
 #include "../Audacity.h"
 #include "TracksPrefs.h"
 
+#include <algorithm>
 #include <wx/defs.h>
 
 #include "../Experimental.h"
+#include "../Prefs.h"
 #include "../ShuttleGui.h"
+#include "../WaveTrack.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 
 TracksPrefs::TracksPrefs(wxWindow * parent)
 :  PrefsPanel(parent, _("Tracks"))
 {
+   // Bugs 1043, 1044
+   // First rewrite legacy preferences
+   gPrefs->Write(wxT("/GUI/DefaultViewModeNew"),
+      WaveTrack::FindDefaultViewMode());
+
    Populate();
 }
 
@@ -92,8 +100,9 @@ void TracksPrefs::PopulateOrExchange(ShuttleGui & S)
 
       S.StartMultiColumn(2);
       {
+
          S.TieChoice(_("Default &View Mode:"),
-                     wxT("/GUI/DefaultViewMode"),
+                     wxT("/GUI/DefaultViewModeNew"),
                      0,
                      mViewChoices,
                      mViewCodes);