Redo hit test priorities in TrackPanel...

Zoom tool takes precedence;

Otherwise do special hits appropriate to the track subclass -- and only
WaveTrack here uses Tools toolbar state, and now disallows clicks on things
when they are not drawn because the view is spectrogram;

Finally, default to right button zooming in Multi tool, or to time shift in
that tool, or to selection adjustment in Multi or in Select tool.

src/LabelTrack.h

@@ -121,9 +121,10 @@ class AUDACITY_DLL_API LabelTrack final : public Track
 
    virtual ~ LabelTrack();
 
-   HitTestResult HitTest
+   HitTestResult DetailedHitTest
       (const TrackPanelMouseEvent &event,
-       const AudacityProject *pProject) override;
+       const AudacityProject *pProject, int currentTool, bool bMultiTool)
+      override;
 
    bool DoCaptureKey(wxKeyEvent &event);
    unsigned CaptureKey

src/NoteTrack.h

@@ -68,9 +68,10 @@ class AUDACITY_DLL_API NoteTrack final
    NoteTrack(const std::shared_ptr<DirManager> &projDirManager);
    virtual ~NoteTrack();
 
-   HitTestResult HitTest
+   HitTestResult DetailedHitTest
       (const TrackPanelMouseEvent &event,
-       const AudacityProject *pProject) override;
+       const AudacityProject *pProject, int currentTool, bool bMultiTool)
+      override;
 
    using Holder = std::unique_ptr<NoteTrack>;
    Track::Holder Duplicate() const override;

src/TimeTrack.h

@@ -49,9 +49,10 @@ class TimeTrack final : public Track {
    void Silence(double t0, double t1) override;
    void InsertSilence(double t, double len) override;
 
-   HitTestResult HitTest
+   HitTestResult DetailedHitTest
       (const TrackPanelMouseEvent &event,
-       const AudacityProject *pProject) override;
+       const AudacityProject *pProject, int currentTool, bool bMultiTool)
+      override;
 
    // Identifying the type of track
    int GetKind() const override { return Time; }

src/Track.h

@@ -108,7 +108,6 @@ class AUDACITY_DLL_API Track /* not final */
    bool           mMinimized;
 
  public:
-   mutable wxSize vrulerSize;
 
    // Given a bare pointer, find a shared_ptr.  But this is not possible for
    // a track not owned by any list, so the result can be null.
@@ -131,11 +130,22 @@ class AUDACITY_DLL_API Track /* not final */
       return {};
    }
 
-   // An implementation is defined for call-through from subclasses, but
-   // the inherited method is still marked pure virtual
+   // Cause certain overriding tool modes (Zoom; future ones?) to behave
+   // uniformly in all tracks, disregarding track contents.
+   // Do not further override this...
    HitTestResult HitTest
       (const TrackPanelMouseEvent &, const AudacityProject *pProject)
-      override = 0;
+      final;
+
+ public:
+
+   // Rather override this for subclasses:
+   virtual HitTestResult DetailedHitTest
+      (const TrackPanelMouseEvent &,
+       const AudacityProject *pProject, int currentTool, bool bMultiTool)
+      = 0;
+
+   mutable wxSize vrulerSize;
 
    // Return another, associated TrackPanelCell object that implements the
    // drop-down, close and minimize buttons, etc.

src/WaveTrack.h

@@ -103,9 +103,10 @@ class AUDACITY_DLL_API WaveTrack final : public PlayableTrack {
 
    virtual ~WaveTrack();
 
-   HitTestResult HitTest
+   HitTestResult DetailedHitTest
       (const TrackPanelMouseEvent &event,
-       const AudacityProject *pProject) override;
+       const AudacityProject *pProject, int currentTool, bool bMultiTool)
+      override;
 
    double GetOffset() const override;
    void SetOffset(double o) override;

src/tracks/labeltrack/ui/LabelTrackUI.cpp

@@ -20,11 +20,10 @@ Paul Licameli split from TrackPanel.cpp
 #include "../../../HitTestResult.h"
 #include "../../../Project.h"
 #include "../../../TrackPanelMouseEvent.h"
-#include "../../../toolbars/ToolsToolBar.h"
 
-HitTestResult LabelTrack::HitTest
+HitTestResult LabelTrack::DetailedHitTest
 (const TrackPanelMouseEvent &evt,
- const AudacityProject *pProject)
+ const AudacityProject *pProject, int, bool)
 {
    HitTestResult result;
    const wxMouseEvent &event = evt.event;
@@ -36,16 +35,6 @@ HitTestResult LabelTrack::HitTest
       // Missed glyph, try text box
       result = LabelTextHandle::HitTest(event, this);
 
-   if ( !result.handle )
-      result = Track::HitTest(evt, pProject);
-
-   if ( !result.handle ) {
-      // In case of multi tool, default to selection.
-      const ToolsToolBar *const pTtb = pProject->GetToolsToolBar();
-      if (pTtb->IsDown(multiTool))
-         result = SelectHandle::HitTest(evt, pProject, this);
-   }
-
    return result;
 }
 

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

@@ -21,44 +21,18 @@ Paul Licameli split from TrackPanel.cpp
 #include "../../../../TrackPanelMouseEvent.h"
 #include "../../../ui/SelectHandle.h"
 #include "StretchHandle.h"
-#include "../../../../toolbars/ToolsToolBar.h"
 
-HitTestResult NoteTrack::HitTest
+HitTestResult NoteTrack::DetailedHitTest
 (const TrackPanelMouseEvent &event,
- const AudacityProject *pProject)
+ const AudacityProject *pProject, int, bool )
 {
-   const ToolsToolBar *const pTtb = pProject->GetToolsToolBar();
-
-   // If the cursor is in the hot zone for stretching, that takes precedence
-   // over selection, but it didn't take precedence over other hits.
-   // That was the old logic, and maybe I tried too hard to preserve it just so.
-   // PRL.
-
    // Eligible for stretch?
-   HitTestResult result1;
+   HitTestResult result;
 #ifdef USE_MIDI
    StretchHandle::StretchState state;
-   result1 = StretchHandle::HitTest( event, pProject, this, state );
+   result = StretchHandle::HitTest( event, pProject, this, state );
 #endif
 
-   // But some other non-select tool like zoom may take priority.
-   HitTestResult result = Track::HitTest(event, pProject);
-   if (result.preview.cursor &&
-       !(result1.preview.cursor && pTtb->GetCurrentTool() == selectTool))
-      return result;
-
-   if (pTtb->IsDown(multiTool)) {
-      // Default to selection
-      if (!result1.preview.cursor &&
-          NULL != (result =
-         SelectHandle::HitTest(event, pProject, this)).preview.cursor)
-         return result;
-   }
-
-   // Do stretch!
-   if (result1.preview.cursor)
-      return result1;
-
    return result;
 }
 

src/tracks/playabletrack/wavetrack/ui/WaveTrackUI.cpp

@@ -23,47 +23,66 @@ Paul Licameli split from TrackPanel.cpp
 #include "SampleHandle.h"
 #include "../../../ui/TimeShiftHandle.h"
 
-HitTestResult WaveTrack::HitTest
+HitTestResult WaveTrack::DetailedHitTest
 (const TrackPanelMouseEvent &event,
- const AudacityProject *pProject)
+ const AudacityProject *pProject, int currentTool, bool bMultiTool)
 {
-   // FIXME: Should similar logic apply to NoteTrack (#if defined(USE_MIDI)) ?
-   // From here on the order in which we hit test determines
-   // which tool takes priority in the rare cases where it
-   // could be more than one.
+   // This is the only override of Track::DetailedHitTest that still
+   // depends on the state of the Tools toolbar.
+   // If that toolbar were eliminated, this could simplify to a sequence of
+   // hit test routines describable by a table.
 
-   // This hit was always tested first no matter which tool:
-   HitTestResult result = CutlineHandle::HitTest(event.event, event.rect, pProject, this);
-   if (result.preview.cursor)
-      return result;
+   WaveTrack *const wavetrack = static_cast<WaveTrack*>(event.pCell);
+   bool isWaveform = (wavetrack->GetDisplay() == WaveTrack::Waveform);
 
-   result = Track::HitTest(event, pProject);
-   if (result.preview.cursor)
-      return result;
+   if (bMultiTool && event.event.CmdDown())
+      // Ctrl modifier key in multi-tool overrides everything else
+      // (But this does not do the time shift constrained to the vertical only,
+      //  which is what happens when you hold Ctrl in the Time Shift tool mode)
+      return TimeShiftHandle::HitAnywhere(pProject);
 
-   const ToolsToolBar *const pTtb = pProject->GetToolsToolBar();
-   if (pTtb->IsDown(multiTool)) {
-      // Replicate some of the logic of TrackPanel::DetermineToolToUse
-      int currentTool = -1;
-      if (event.event.CmdDown())
-         result = TimeShiftHandle::HitAnywhere(pProject);
-      else if (NULL !=
-         (result = EnvelopeHandle::WaveTrackHitTest(event.event, event.rect, pProject, *this))
-         .preview.cursor)
-         ;
-      else if (NULL != (result =
-         TimeShiftHandle::HitTest(event.event, event.rect, pProject)).preview.cursor)
-         ;
-      else if (NULL != (result =
-         SampleHandle::HitTest(event.event, event.rect, pProject, this)).preview.cursor)
-         ;
-      else if (NULL != (result =
-         SelectHandle::HitTest(event, pProject, this)).preview.cursor)
-         // default of all other hit tests
-         ;
+   // Some special targets are not drawn in spectrogram,
+   // so don't hit them in such views.
+   else if (isWaveform) {
+      HitTestResult result;
+      if (NULL !=
+          (result = CutlineHandle::HitTest
+           (event.event, event.rect, pProject, this))
+           .preview.cursor)
+          // This overriding test applies in all tools
+          return result;
+      else if (bMultiTool) {
+         // Conditional hit tests
+         // If Tools toolbar were eliminated, we would keep these
+         // The priority of these, in case more than one might apply at one
+         // point, seems arbitrary
+         if (NULL != (result = EnvelopeHandle::WaveTrackHitTest
+              (event.event, event.rect, pProject, *this))
+             .preview.cursor)
+            ;
+         else if (NULL != (result = TimeShiftHandle::HitTest
+              (event.event, event.rect, pProject)).preview.cursor)
+            // This is the hit test on the "grips" drawn left and
+            // right in Multi only
+            ;
+         else if (NULL != (result = SampleHandle::HitTest
+              (event.event, event.rect, pProject, this)).preview.cursor)
+            ;
+         return result;
+      }
+      else switch ( currentTool ) {
+         // Unconditional hits appropriate to the tool
+         // If tools toolbar were eliminated, we would eliminate these
+         case envelopeTool:
+            return EnvelopeHandle::HitAnywhere(pProject);
+         case drawTool:
+            return SampleHandle::HitAnywhere(event.event, pProject);
+         default:
+            break;
+      }
    }
 
-   return result;
+   return {};
 }
 
 std::shared_ptr<TrackControls> WaveTrack::GetControls()

src/tracks/timetrack/ui/TimeTrackUI.cpp

@@ -15,18 +15,13 @@ Paul Licameli split from TrackPanel.cpp
 #include "../../../HitTestResult.h"
 #include "../../../TrackPanelMouseEvent.h"
 #include "../../../Project.h"
-#include "../../../toolbars/ToolsToolBar.h"
 
 #include "../../ui/EnvelopeHandle.h"
 
-HitTestResult TimeTrack::HitTest
+HitTestResult TimeTrack::DetailedHitTest
 (const TrackPanelMouseEvent &event,
- const AudacityProject *pProject)
+ const AudacityProject *pProject, int, bool)
 {
-   HitTestResult result = Track::HitTest(event, pProject);
-   if (result.preview.cursor)
-      return result;
-
    return EnvelopeHandle::TimeTrackHitTest
       ( event.event, event.rect, pProject, *this );
 }

src/tracks/ui/TrackUI.cpp

@@ -28,34 +28,34 @@ HitTestResult Track::HitTest
  const AudacityProject *pProject)
 {
    const ToolsToolBar * pTtb = pProject->GetToolsToolBar();
-   // Unless in Multimode keep using the current tool.
    const bool isMultiTool = pTtb->IsDown(multiTool);
-   if (!isMultiTool) {
-      switch (pTtb->GetCurrentTool()) {
-      case envelopeTool:
-         // Pass "false" for unsafe -- let the tool decide to cancel itself
-         return EnvelopeHandle::HitAnywhere(pProject);
-      case drawTool:
-         return SampleHandle::HitAnywhere(event.event, pProject);
-      case zoomTool:
-         return ZoomHandle::HitAnywhere(event.event, pProject);
-      case slideTool:
-         return TimeShiftHandle::HitAnywhere(pProject);
-      case selectTool:
-         return SelectHandle::HitTest(event, pProject, this);
+   const auto currentTool = pTtb->GetCurrentTool();
 
-      default:
-         // fallthru
-         ;
-      }
-   }
+   if ( !isMultiTool && currentTool == zoomTool )
+      // Zoom tool is a non-selecting tool that takes precedence in all tracks
+      // over all other tools, no matter what detail you point at.
+      return ZoomHandle::HitAnywhere(event.event, pProject);
 
-   // Replicate some of the logic of TrackPanel::DetermineToolToUse
-   HitTestResult result;
+   // In other tools, let subclasses determine detailed hits.
+   HitTestResult result =
+      DetailedHitTest( event, pProject, currentTool, isMultiTool );
 
-   if (isMultiTool)
+   // If there is no detailed hit for the subclass, there are still some
+   // general cases.
+
+   // Sliding applies in more than one track type.
+   if ( !result.handle && !isMultiTool && currentTool == slideTool )
+      result = TimeShiftHandle::HitAnywhere(pProject);
+
+   // Let the multi-tool right-click handler apply only in default of all
+   // other detailed hits.
+   if ( !result.handle && isMultiTool )
       result = ZoomHandle::HitTest(event.event, pProject);
 
+   // Finally, default of all is adjustment of the selection box.
+   if ( !result.handle && ( isMultiTool || currentTool == selectTool) )
+      result = SelectHandle::HitTest(event, pProject, this);
+
    return result;
 }