Rework note tracks to store top and bottom notes instead of pitch height

This simplifies a bunch of other work -- in particular zooming so that specific notes are visible, and keeping the same notes on screen when resizing the track.

Also included is a fix to YToIPitch to make it use mPitchHeight directly -- this solves some roundoff errors, which previously caused inaccurate results on some zoom levels.

src/NoteTrack.cpp

@@ -123,8 +123,8 @@ NoteTrack::NoteTrack(const std::shared_ptr<DirManager> &projDirManager)
 #ifdef EXPERIMENTAL_MIDI_OUT
    mVelocity = 0;
 #endif
-   mBottomNote = 24;
-   mPitchHeight = 5.0f;
+   mBottomNote = MinPitch;
+   mTopNote = MaxPitch;
 
    mVisibleChannels = ALL_CHANNELS;
 }
@@ -186,7 +186,7 @@ Track::Holder NoteTrack::Duplicate() const
    }
    // copy some other fields here
    duplicate->SetBottomNote(mBottomNote);
-   duplicate->mPitchHeight = mPitchHeight;
+   duplicate->SetTopNote(mTopNote);
    duplicate->mVisibleChannels = mVisibleChannels;
    duplicate->SetOffset(GetOffset());
 #ifdef EXPERIMENTAL_MIDI_OUT
@@ -211,24 +211,6 @@ double NoteTrack::GetEndTime() const
    return GetStartTime() + GetSeq().get_real_dur();
 }
 
-void NoteTrack::DoSetHeight(int h)
-{
-   auto oldHeight = GetHeight();
-   NoteTrackDisplayData oldData = NoteTrackDisplayData(this, wxRect(0, 0, 1, oldHeight));
-   auto oldMargin = oldData.GetNoteMargin();
-   PlayableTrack::DoSetHeight(h);
-   NoteTrackDisplayData newData = NoteTrackDisplayData(this, wxRect(0, 0, 1, h));
-   auto margin = newData.GetNoteMargin();
-   Zoom(
-      wxRect{ 0, 0, 1, h }, // only height matters
-      h - margin - 1, // preserve bottom note
-      (float)(h - 2 * margin) /
-           std::max(1, oldHeight - 2 * oldMargin),
-      false
-   );
-}
-
-
 void NoteTrack::WarpAndTransposeNotes(double t0, double t1,
                                       const TimeWarper &warper,
                                       double semitones)
@@ -962,6 +944,56 @@ void NoteTrack::WriteXML(XMLWriter &xmlFile) const
    xmlFile.EndTag(wxT("notetrack"));
 }
 
+void NoteTrack::SetBottomNote(int note)
+{
+   if (note < MinPitch)
+      note = MinPitch;
+   else if (note > 96)
+      note = 96;
+
+   wxCHECK(note <= mTopNote, );
+
+   mBottomNote = note;
+}
+
+void NoteTrack::SetTopNote(int note)
+{
+   if (note > MaxPitch)
+      note = MaxPitch;
+
+   wxCHECK(note >= mBottomNote, );
+
+   mTopNote = note;
+}
+
+void NoteTrack::SetNoteRange(int note1, int note2)
+{
+   // Bounds check
+   if (note1 > MaxPitch)
+      note1 = MaxPitch;
+   else if (note1 < MinPitch)
+      note1 = MinPitch;
+   if (note2 > MaxPitch)
+      note2 = MaxPitch;
+   else if (note2 < MinPitch)
+      note2 = MinPitch;
+   // Swap to ensure ordering
+   if (note2 < note1) { auto tmp = note1; note1 = note2; note2 = tmp; }
+
+   mBottomNote = note1;
+   mTopNote = note2;
+}
+
+void NoteTrack::ShiftNoteRange(int offset)
+{
+   // Ensure everything stays in bounds
+   if (mBottomNote + offset < MinPitch || mTopNote + offset > MaxPitch)
+       return;
+
+   mBottomNote += offset;
+   mTopNote += offset;
+}
+
 #if 0
 void NoteTrack::StartVScroll()
 {
@@ -972,29 +1004,27 @@ void NoteTrack::VScroll(int start, int end)
 {
     int ph = GetPitchHeight();
     int delta = ((end - start) + ph / 2) / ph;
-    SetBottomNote(mStartBottomNote + delta);
+    ShiftNoteRange(delta);
 }
 #endif
 
 void NoteTrack::Zoom(const wxRect &rect, int y, float multiplier, bool center)
 {
-   // Construct track rectangle to map pitch to screen coordinates
-   // Only y and height are needed:
-   wxRect trackRect(0, rect.GetY(), 1, rect.GetHeight());
-   NoteTrackDisplayData data = NoteTrackDisplayData(this, trackRect);
+   NoteTrackDisplayData data = NoteTrackDisplayData(this, rect);
    int clickedPitch = data.YToIPitch(y);
-   // zoom by changing the pitch height
-   SetPitchHeight(rect.height, mPitchHeight * multiplier);
-   NoteTrackDisplayData newData = NoteTrackDisplayData(this, trackRect); // update because mPitchHeight changed
+   int extent = mTopNote - mBottomNote + 1;
+   int newExtent = (int) (extent / multiplier);
+   float position;
    if (center) {
-      int newCenterPitch = newData.YToIPitch(rect.GetY() + rect.GetHeight() / 2);
       // center the pitch that the user clicked on
-      SetBottomNote(mBottomNote + (clickedPitch - newCenterPitch));
+      position = .5;
    } else {
-      int newClickedPitch = newData.YToIPitch(y);
       // align to keep the pitch that the user clicked on in the same place
-      SetBottomNote(mBottomNote + (clickedPitch - newClickedPitch));
+      position = extent / (clickedPitch - mBottomNote);
    }
+   int newBottomNote = clickedPitch - (newExtent * position);
+   int newTopNote = clickedPitch + (newExtent * (1 - position));
+   SetNoteRange(newBottomNote, newTopNote);
 }
 
 
@@ -1002,27 +1032,53 @@ void NoteTrack::ZoomTo(const wxRect &rect, int start, int end)
 {
    wxRect trackRect(0, rect.GetY(), 1, rect.GetHeight());
    NoteTrackDisplayData data = NoteTrackDisplayData(this, trackRect);
-   int topPitch = data.YToIPitch(start);
-   int botPitch = data.YToIPitch(end);
-   if (topPitch < botPitch) { // swap
-      int temp = topPitch; topPitch = botPitch; botPitch = temp;
-   }
-   if (topPitch == botPitch) { // can't divide by zero, do something else
+   int pitch1 = data.YToIPitch(start);
+   int pitch2 = data.YToIPitch(end);
+   if (pitch1 == pitch2) {
+      // Just zoom in instead of zooming to show only one note
       Zoom(rect, start, 1, true);
       return;
    }
-   auto trialPitchHeight = (float)trackRect.height / (topPitch - botPitch);
-   Zoom(rect, (start + end) / 2, trialPitchHeight / mPitchHeight, true);
+   // It's fine for this to be in either order
+   SetNoteRange(pitch1, pitch2);
 }
 
 NoteTrackDisplayData::NoteTrackDisplayData(const NoteTrack* track, const wxRect &r)
 {
-   mPitchHeight = track->mPitchHeight;
-   mMargin = std::min(r.height / 4, (GetPitchHeight(1) + 1) / 2);
+   auto span = track->GetTopNote() - track->GetBottomNote() + 1; // + 1 to make sure it includes both
+
+   mMargin = std::min((int) (r.height / (float)(span)) / 2, r.height / 4);
+
+   // Count the number of dividers between B/C and E/F
+   int numC = 0, numF = 0;
+   auto botOctave = track->GetBottomNote() / 12, botNote = track->GetBottomNote() % 12;
+   auto topOctave = track->GetTopNote() / 12, topNote = track->GetTopNote() % 12;
+   if (topOctave == botOctave)
+   {
+      if (botNote == 0) numC = 1;
+      if (topNote <= 5) numF = 1;
+   }
+   else
+   {
+      numC = topOctave - botOctave;
+      numF = topOctave - botOctave - 1;
+      if (botNote == 0) numC++;
+      if (botNote <= 5) numF++;
+      if (topOctave <= 5) numF++;
+   }
+   // Effective space, excluding the margins and the lines between some notes
+   auto effectiveHeight = r.height - (2 * (mMargin + 1)) - numC - numF;
+   // Guarenteed that both the bottom and top notes will be visible
+   // (assuming that the clamping below does not happen)
+   mPitchHeight = effectiveHeight / ((float) span);
+
+   if (mPitchHeight < MinPitchHeight)
+      mPitchHeight = MinPitchHeight;
+   if (mPitchHeight > MaxPitchHeight)
+      mPitchHeight = MaxPitchHeight;
 
    mBottom = r.y + r.height - GetNoteMargin() - 1 - GetPitchHeight(1) +
-            (track->GetBottomNote() / 12) * GetOctaveHeight() +
-               GetNotePos(track->GetBottomNote() % 12);
+            botOctave * GetOctaveHeight() + GetNotePos(botNote);
 }
 
 int NoteTrackDisplayData::IPitchToY(int p) const
@@ -1035,7 +1091,9 @@ int NoteTrackDisplayData::YToIPitch(int y) const
    y -= octave * GetOctaveHeight();
    // result is approximate because C and G are one pixel taller than
    // mPitchHeight.
-   return (y / GetPitchHeight(1)) + octave * 12;
+   // Poke 1-13-18: However in practice this seems not to be an issue,
+   // as long as we use mPitchHeight and not the rounded version
+   return (y / mPitchHeight) + octave * 12;
 }
 
 const float NoteTrack::ZoomStep = powf( 2.0f, 0.25f );

src/NoteTrack.h

@@ -64,12 +64,9 @@ using QuantizedTimeAndBeat = std::pair< double, double >;
 
 class StretchHandle;
 
-class NoteTrackDisplayData;
-
 class AUDACITY_DLL_API NoteTrack final
    : public NoteTrackBase
 {
-   friend class NoteTrackDisplayData;
  public:
    NoteTrack(const std::shared_ptr<DirManager> &projDirManager);
    virtual ~NoteTrack();
@@ -86,8 +83,6 @@ class AUDACITY_DLL_API NoteTrack final
    double GetStartTime() const override;
    double GetEndTime() const override;
 
-   void DoSetHeight(int h) override;
-
    Alg_seq &GetSeq() const;
 
    void WarpAndTransposeNotes(double t0, double t1,
@@ -125,21 +120,22 @@ class AUDACITY_DLL_API NoteTrack final
    bool StretchRegion
       ( QuantizedTimeAndBeat t0, QuantizedTimeAndBeat t1, double newDur );
 
+   /// Gets the current bottom note (a pitch)
    int GetBottomNote() const { return mBottomNote; }
-   int GetPitchHeight(int factor) const
-   { return std::max(1, (int)(factor * mPitchHeight)); }
-   void SetPitchHeight(int rectHeight, float h)
-   {
-      // Impose certain zoom limits
-      auto octavePadding = 2 * 10; // 10 octaves times 2 single-pixel seperations per pixel
-      auto availableHeight = rectHeight - octavePadding;
-      auto numNotes = 128.f;
-      auto minSpacePerNote =
-         std::max((float)MinPitchHeight, availableHeight / numNotes);
-      mPitchHeight =
-         std::max(minSpacePerNote,
-                  std::min((float)MaxPitchHeight, h));
-   }
+   /// Gets the current top note (a pitch)
+   int GetTopNote() const { return mTopNote; }
+   /// Sets the bottom note (a pitch), making sure that it is never greater than the top note.
+   void SetBottomNote(int note);
+   /// Sets the top note (a pitch), making sure that it is never less than the bottom note.
+   void SetTopNote(int note);
+   /// Sets the top and bottom note (both pitches) automatically, swapping them if needed.
+   void SetNoteRange(int note1, int note2);
+
+   /// Zooms so that the entire track is visible
+   void ZoomMaxExtent() { SetNoteRange(MinPitch, MaxPitch); }
+   /// Shifts all notes vertically by the given pitch
+   void ShiftNoteRange(int offset);
+
    /// Zooms out a constant factor (subject to zoom limits)
    void ZoomOut(const wxRect &rect, int y) { Zoom(rect, y, 1.0f / ZoomStep, true); }
    /// Zooms in a contant factor (subject to zoom limits)
@@ -148,15 +144,6 @@ class AUDACITY_DLL_API NoteTrack final
    /// If center is true, the result will be centered at y.
    void Zoom(const wxRect &rect, int y, float multiplier, bool center);
    void ZoomTo(const wxRect &rect, int start, int end);
-   void SetBottomNote(int note)
-   {
-      if (note < 0)
-         note = 0;
-      else if (note > 96)
-         note = 96;
-
-      mBottomNote = note;
-   }
 
 #if 0
    // Vertical scrolling is performed by dragging the keyboard at
@@ -219,7 +206,7 @@ class AUDACITY_DLL_API NoteTrack final
    float mVelocity; // velocity offset
 #endif
 
-   int mBottomNote;
+   int mBottomNote, mTopNote;
 #if 0
    // Also unused from vertical scrolling
    int mStartBottomNote;
@@ -229,7 +216,7 @@ class AUDACITY_DLL_API NoteTrack final
    // but it is rounded off whenever drawing:
    float mPitchHeight;
 
-   enum { MinPitchHeight = 1, MaxPitchHeight = 25 };
+   enum { MinPitch = 0, MaxPitch = 127 };
    static const float ZoomStep;
 
    int mVisibleChannels; // bit set of visible channels
@@ -241,6 +228,7 @@ protected:
    std::shared_ptr<TrackVRulerControls> DoGetVRulerControls() override;
 };
 
+/// Data used to display a note track
 class NoteTrackDisplayData {
 private:
    float mPitchHeight;
@@ -249,6 +237,8 @@ private:
    // mY + mHeight - (GetNoteMargin() + 1 + GetPitchHeight())
    int mBottom;
    int mMargin;
+
+   enum { MinPitchHeight = 1, MaxPitchHeight = 25 };
 public:
    NoteTrackDisplayData(const NoteTrack* track, const wxRect &r);
 
@@ -281,7 +271,6 @@ public:
    // of the line separating B and C
    int GetWhitePos(int i) const { return 1 + (i * GetOctaveHeight()) / 7; }
 };
-
 #endif // USE_MIDI
 
 #ifndef SONIFY

src/import/ImportMIDI.cpp

@@ -89,6 +89,7 @@ bool ImportMIDI(const FilePath &fName, NoteTrack * dest)
    // then middle pitch class is D. Round mean_pitch to the nearest D:
    int mid_pitch = ((mean_pitch - 2 + 6) / 12) * 12 + 2;
    dest->SetBottomNote(mid_pitch - 14);
+   dest->SetTopNote(mid_pitch + 13);
    return true;
 }
 

src/tracks/playabletrack/notetrack/ui/NoteTrackControls.cpp

@@ -104,8 +104,7 @@ enum {
    OnDownOctaveID,
 };
 
-/// This only applies to MIDI tracks.  Presumably, it shifts the
-/// whole sequence by an octave.
+/// Scrolls the note track up or down by an octave
 void NoteTrackMenuTable::OnChangeOctave(wxCommandEvent &event)
 {
    NoteTrack *const pTrack = static_cast<NoteTrack*>(mpData->pTrack);
@@ -114,8 +113,7 @@ void NoteTrackMenuTable::OnChangeOctave(wxCommandEvent &event)
       || event.GetId() == OnDownOctaveID);
 
    const bool bDown = (OnDownOctaveID == event.GetId());
-   pTrack->SetBottomNote
-      (pTrack->GetBottomNote() + ((bDown) ? -12 : 12));
+   pTrack->ShiftNoteRange((bDown) ? -12 : 12);
 
    AudacityProject *const project = ::GetActiveProject();
    project->ModifyState(true);

src/tracks/playabletrack/notetrack/ui/NoteTrackVRulerControls.cpp

@@ -76,7 +76,7 @@ unsigned NoteTrackVRulerControls::HandleWheelRotation
    } else if (!event.CmdDown() && event.ShiftDown()) {
       // Scroll some fixed number of notes, independent of zoom level or track height:
       static const int movement = 6; // 6 semitones is half an octave
-      nt->SetBottomNote(nt->GetBottomNote() + (int) (steps * movement));
+      nt->ShiftNoteRange((int) (steps * movement));
    } else {
       return RefreshNone;
    }

src/tracks/playabletrack/notetrack/ui/NoteTrackVZoomHandle.cpp

@@ -195,8 +195,6 @@ private:
    }
 
    virtual void InitMenu(Menu *pMenu, void *pUserData) override;
-
-   void OnWaveformScaleType(wxCommandEvent &evt);
 };
 
 NoteTrackVRulerMenuTable &NoteTrackVRulerMenuTable::Instance()
@@ -213,8 +211,7 @@ void NoteTrackVRulerMenuTable::InitMenu(Menu *WXUNUSED(pMenu), void *pUserData)
 void NoteTrackVRulerMenuTable::OnZoom( int iZoomCode ){
    switch( iZoomCode ){
    case kZoomReset:
-      mpData->pTrack->SetBottomNote(0);
-      mpData->pTrack->SetPitchHeight(mpData->rect.height, 1);
+      mpData->pTrack->ZoomMaxExtent();
       break;
    case kZoomIn:
       mpData->pTrack->ZoomIn(mpData->rect, mpData->yy);
@@ -299,8 +296,7 @@ UIHandle::Result NoteTrackVZoomHandle::Release
    else if (event.ShiftDown() || event.RightUp()) {
       if (event.ShiftDown() && event.RightUp()) {
          // Zoom out completely
-         pTrack->SetBottomNote(0);
-         pTrack->SetPitchHeight(evt.rect.height, 1);
+         pTrack->ZoomMaxExtent();
       } else {
          // Zoom out
          pTrack->ZoomOut(evt.rect, mZoomEnd);