Fix inverted logic of SnapOff with snap modifiers pressed.

	- also clean up, rename and comment some previous hanges.

9 files changed, 102 insertions, 53 deletions

diff --git a/gtk2_ardour/editor.cc b/gtk2_ardour/editor.cc
index 45c7ca94b0..474fb4c6f9 100644
--- a/gtk2_ardour/editor.cc
+++ b/gtk2_ardour/editor.cc
@@ -2604,18 +2604,21 @@ Editor::snap_to_with_modifier (framepos_t& start, GdkEvent const * event, RoundM
 	} else {
 		if (_snap_mode != SnapOff) {
 			snap_to_internal (start, direction, for_mark);
+		} else if (ArdourKeyboard::indicates_snap_delta (event->button.state)) {
+			/* SnapOff, but we pressed the snap_delta modifier */
+			snap_to_internal (start, direction, for_mark);
 		}
 	}
 }
 
 void
-Editor::snap_to (framepos_t& start, RoundMode direction, bool for_mark, bool explicitly)
+Editor::snap_to (framepos_t& start, RoundMode direction, bool for_mark, bool ensure_snap)
 {
-	if (!_session || _snap_mode == SnapOff) {
+	if (!_session || (_snap_mode == SnapOff && !ensure_snap)) {
 		return;
 	}
 
-	snap_to_internal (start, direction, for_mark, explicitly);
+	snap_to_internal (start, direction, for_mark, ensure_snap);
 }
 
 void
@@ -2685,7 +2688,7 @@ Editor::timecode_snap_to_internal (framepos_t& start, RoundMode direction, bool
 }
 
 void
-Editor::snap_to_internal (framepos_t& start, RoundMode direction, bool for_mark, bool explicitly)
+Editor::snap_to_internal (framepos_t& start, RoundMode direction, bool for_mark, bool ensure_snap)
 {
 	const framepos_t one_second = _session->frame_rate();
 	const framepos_t one_minute = _session->frame_rate() * 60;
@@ -2855,7 +2858,7 @@ Editor::snap_to_internal (framepos_t& start, RoundMode direction, bool for_mark,
 
 	case SnapMagnetic:
 
-		if (explicitly) {
+		if (ensure_snap) {
 			return;
 		}
 
diff --git a/gtk2_ardour/editor.h b/gtk2_ardour/editor.h
index 663a97a56d..603103532f 100644
--- a/gtk2_ardour/editor.h
+++ b/gtk2_ardour/editor.h
@@ -437,7 +437,7 @@ class Editor : public PublicEditor, public PBD::ScopedConnectionList, public ARD
 	void snap_to (framepos_t&       first,
 	              ARDOUR::RoundMode direction = ARDOUR::RoundNearest,
 	              bool              for_mark  = false,
-		      bool              explicitly = false);
+		      bool              ensure_snap = false);
 
 	void snap_to_with_modifier (framepos_t&       first,
 	                            GdkEvent const *  ev,
@@ -2133,7 +2133,7 @@ class Editor : public PublicEditor, public PBD::ScopedConnectionList, public ARD
 	void snap_to_internal (framepos_t&       first,
 	                       ARDOUR::RoundMode direction = ARDOUR::RoundNearest,
 	                       bool              for_mark  = false,
-			       bool              explicitly = false);
+			       bool              ensure_snap = false);
 
 	void timecode_snap_to_internal (framepos_t&       first,
 	                                ARDOUR::RoundMode direction = ARDOUR::RoundNearest,
diff --git a/gtk2_ardour/editor_drag.cc b/gtk2_ardour/editor_drag.cc
index c390533522..014b4679e8 100644
--- a/gtk2_ardour/editor_drag.cc
+++ b/gtk2_ardour/editor_drag.cc
@@ -2394,10 +2394,28 @@ NoteResizeDrag::motion (GdkEvent* event, bool /*first_move*/)
 		MidiRegionView* mrv = dynamic_cast<MidiRegionView*>(*r);
 		if (mrv) {
 			double sd = 0.0;
-			if (!ArdourKeyboard::indicates_snap_delta (event->button.state)) {
+			bool snap = true;
+			bool apply_snap_delta = !ArdourKeyboard::indicates_snap_delta (event->button.state);
+
+			if (ArdourKeyboard::indicates_snap (event->button.state)) {
+				if (_editor->snap_mode () != SnapOff) {
+					snap = false;
+				}
+			} else {
+				if (_editor->snap_mode () == SnapOff) {
+					snap = false;
+					/* inverted logic here - we;re in snapoff but we've pressed the snap delta modifier */
+					if (!apply_snap_delta) {
+						snap = true;
+					}
+				}
+			}
+
+			if (apply_snap_delta) {
 				sd = _snap_delta;
 			}
-			mrv->update_resizing (nb, at_front, _drags->current_pointer_x() - grab_x(), relative, sd, event->button.state);
+
+			mrv->update_resizing (nb, at_front, _drags->current_pointer_x() - grab_x(), relative, sd, snap);
 		}
 	}
 }
@@ -2411,11 +2429,26 @@ NoteResizeDrag::finished (GdkEvent* event, bool /*movement_occurred*/)
 		assert (nb);
 		MidiRegionView* mrv = dynamic_cast<MidiRegionView*>(*r);
 		double sd = 0.0;
-		if (!ArdourKeyboard::indicates_snap_delta (event->button.state)) {
-			sd = _snap_delta;
-		}
+		bool snap = true;
+		bool apply_snap_delta = !ArdourKeyboard::indicates_snap_delta (event->button.state);
 		if (mrv) {
-			mrv->commit_resizing (nb, at_front, _drags->current_pointer_x() - grab_x(), relative, sd, event->button.state);
+			if (ArdourKeyboard::indicates_snap (event->button.state)) {
+				if (_editor->snap_mode () != SnapOff) {
+					snap = false;
+				}
+			} else {
+				if (_editor->snap_mode () == SnapOff) {
+					snap = false;
+					/* inverted logic here - we;re in snapoff but we've pressed the snap delta modifier */
+					if (!apply_snap_delta) {
+						snap = true;
+					}
+				}
+			}
+			if (apply_snap_delta) {
+				sd = _snap_delta;
+			}
+			mrv->commit_resizing (nb, at_front, _drags->current_pointer_x() - grab_x(), relative, sd, snap);
 		}
 	}
 
@@ -5196,7 +5229,7 @@ NoteDrag::start_grab (GdkEvent* event, Gdk::Cursor *)
 
 /** @return Current total drag x change in frames */
 frameoffset_t
-NoteDrag::total_dx (guint const state) const
+NoteDrag::total_dx (const guint state) const
 {
 	/* dx in frames */
 	frameoffset_t const dx = _editor->pixel_to_sample (_drags->current_pointer_x() - grab_x());
@@ -5212,13 +5245,25 @@ NoteDrag::total_dx (guint const state) const
 	/* prevent the note being dragged earlier than the region's position */
 	st = max (st, rp);
 
-	/* snap and return corresponding delta */
+	/* possibly snap and return corresponding delta */
+
+	bool snap = true;
 
 	if (ArdourKeyboard::indicates_snap (state)) {
-		return (st - rp) + rp - n - snap_delta (state);
+		if (_editor->snap_mode () != SnapOff) {
+			snap = false;
+		}
+	} else {
+		if (_editor->snap_mode () == SnapOff) {
+			snap = false;
+			/* inverted logic here - we;re in snapoff but we've pressed the snap delta modifier */
+			if (ArdourKeyboard::indicates_snap_delta (state)) {
+				snap = true;
+			}
+		}
 	}
 
-	return _region->snap_frame_to_frame (st - rp) + rp - n - snap_delta (state);
+	return _region->snap_frame_to_frame (st - rp, snap) + rp - n - snap_delta (state);
 }
 
 /** @return Current total drag y change in note number */
diff --git a/gtk2_ardour/editor_drag.h b/gtk2_ardour/editor_drag.h
index 7d02a23e52..bf08fc31a1 100644
--- a/gtk2_ardour/editor_drag.h
+++ b/gtk2_ardour/editor_drag.h
@@ -527,7 +527,7 @@ class NoteDrag : public Drag
 
   private:
 
-	ARDOUR::frameoffset_t total_dx (guint const) const;
+	ARDOUR::frameoffset_t total_dx (const guint) const;
 	int8_t total_dy () const;
 
 	MidiRegionView* _region;
diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index fda5aec725..0650068d0a 100644
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -2624,22 +2624,25 @@ MidiRegionView::note_dropped(NoteBase *, frameoffset_t dt, int8_t dnote)
 }
 
 /** @param x Pixel relative to the region position.
+ *  @param ensure_snap defaults to false. true = snap always, ignoring snap mode and magnetic snap.
+ *  Used for inverting the snap logic with key modifiers and snap delta calculation.
  *  @return Snapped frame relative to the region position.
  */
 framepos_t
-MidiRegionView::snap_pixel_to_sample(double x, bool explicitly)
+MidiRegionView::snap_pixel_to_sample(double x, bool ensure_snap)
 {
 	PublicEditor& editor (trackview.editor());
-	return snap_frame_to_frame (editor.pixel_to_sample (x), explicitly);
+	return snap_frame_to_frame (editor.pixel_to_sample (x), ensure_snap);
 }
 
 /** @param x Pixel relative to the region position.
+ *  @param ensure_snap defaults to false. true = ignore magnetic snap and snap mode (used for snap delta calculation).
  *  @return Snapped pixel relative to the region position.
  */
 double
-MidiRegionView::snap_to_pixel(double x, bool explicitly)
+MidiRegionView::snap_to_pixel(double x, bool ensure_snap)
 {
-	return (double) trackview.editor().sample_to_pixel(snap_pixel_to_sample(x, explicitly));
+	return (double) trackview.editor().sample_to_pixel(snap_pixel_to_sample(x, ensure_snap));
 }
 
 double
@@ -2747,7 +2750,7 @@ MidiRegionView::begin_resizing (bool /*at_front*/)
  * @param snap_delta snap offset of the primary note in pixels. used in SnapRelative SnapDelta mode.
  */
 void
-MidiRegionView::update_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, guint key_state)
+MidiRegionView::update_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, bool with_snap)
 {
 	bool cursor_set = false;
 
@@ -2780,17 +2783,17 @@ MidiRegionView::update_resizing (NoteBase* primary, bool at_front, double delta_
 		}
 
 		if (at_front) {
-			if (ArdourKeyboard::indicates_snap (key_state)) {
-				resize_rect->set_x0 (current_x - snap_delta);
+			if (with_snap) {
+				resize_rect->set_x0 (snap_to_pixel(current_x, true) - snap_delta);
 			} else {
-				resize_rect->set_x0 (snap_to_pixel(current_x) - snap_delta);
+				resize_rect->set_x0 (current_x - snap_delta);
 			}
 			resize_rect->set_x1 (canvas_note->x1());
 		} else {
-			if (ArdourKeyboard::indicates_snap (key_state)) {
-				resize_rect->set_x1 (current_x - snap_delta);
+			if (with_snap) {
+				resize_rect->set_x1 (snap_to_pixel(current_x, true) - snap_delta);
 			} else {
-				resize_rect->set_x1 (snap_to_pixel(current_x) - snap_delta);
+				resize_rect->set_x1 (current_x - snap_delta);
 			}
 			resize_rect->set_x0 (canvas_note->x0());
 		}
@@ -2808,7 +2811,7 @@ MidiRegionView::update_resizing (NoteBase* primary, bool at_front, double delta_
 				sign = -1;
 			}
 
-			const double  snapped_x = snap_pixel_to_sample (current_x);
+			const double  snapped_x = (with_snap ? snap_pixel_to_sample (current_x, true) : trackview.editor ().pixel_to_sample (current_x));
 			Evoral::Beats beats     = region_frames_to_region_beats (snapped_x);
 			Evoral::Beats len       = Evoral::Beats();
 
@@ -2840,10 +2843,8 @@ MidiRegionView::update_resizing (NoteBase* primary, bool at_front, double delta_
  *  Parameters the same as for \a update_resizing().
  */
 void
-MidiRegionView::commit_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, guint key_state)
+MidiRegionView::commit_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, bool with_snap)
 {
-	bool snap_keys = ArdourKeyboard::indicates_snap (key_state);
-
 	_note_diff_command = _model->new_note_diff_command (_("resize notes"));
 
 	for (std::vector<NoteResizeData *>::iterator i = _resize_data.begin(); i != _resize_data.end(); ++i) {
@@ -2887,13 +2888,7 @@ MidiRegionView::commit_resizing (NoteBase* primary, bool at_front, double delta_
 		}
 
 		/* Convert that to a frame within the source */
-		framepos_t current_fr;
-
-		if (snap_keys) {
-			current_fr = trackview.editor().pixel_to_sample (current_x) + _region->start ();
-		} else {
-			current_fr = snap_pixel_to_sample (current_x) + _region->start ();
-		}
+		const framepos_t current_fr = snap_pixel_to_sample (current_x, with_snap) + _region->start ();
 
 		/* and then to beats */
 		const Evoral::Beats x_beats = region_frames_to_region_beats (current_fr);
diff --git a/gtk2_ardour/midi_region_view.h b/gtk2_ardour/midi_region_view.h
index 9c39fa1576..4cab6b16bf 100644
--- a/gtk2_ardour/midi_region_view.h
+++ b/gtk2_ardour/midi_region_view.h
@@ -223,8 +223,8 @@ public:
 	 */
 	void begin_resizing(bool at_front);
 
-	void update_resizing (NoteBase*, bool, double, bool, double, guint);
-	void commit_resizing (NoteBase*, bool, double, bool, double, guint);
+	void update_resizing (NoteBase* primary, bool at_front, double delta_x, bool relative, double snap_delta, bool with_snap);
+	void commit_resizing (NoteBase* primary, bool at_front, double delat_x, bool relative, double snap_delta, bool with_snap);
 	void abort_resizing ();
 
 	/** Change the channel of the selection.
@@ -250,17 +250,18 @@ public:
 
 	/** Snap a region relative pixel coordinate to pixel units.
 	 * @param x a pixel coordinate relative to region start
-	 * @param explicitly do not use magnetic snap (required for snap delta calculation)
+	 * @param ensure_snap do not use magnetic snap (required for snap delta calculation)
 	 * @return the snapped pixel coordinate relative to region start
 	 */
-	double snap_to_pixel(double x, bool explicitly = false);
+	double snap_to_pixel(double x, bool ensure_snap = false);
 
 	/** Snap a region relative pixel coordinate to frame units.
 	 * @param x a pixel coordinate relative to region start
-	 * @param explicitly do not use magnetic snap (required for snap delta calculation)
+	 * @param ensure_snap ignore SnapOff and magnetic snap.
+	 * Required for inverting snap logic with modifier keys and snap delta calculation.
 	 * @return the snapped framepos_t coordinate relative to region start
 	 */
-	framepos_t snap_pixel_to_sample(double x, bool explicitly = false);
+	framepos_t snap_pixel_to_sample(double x, bool ensure_snap = false);
 
 	/** Convert a timestamp in beats into frames (both relative to region position) */
 	framepos_t region_beats_to_region_frames(Evoral::Beats beats) const;
diff --git a/gtk2_ardour/public_editor.h b/gtk2_ardour/public_editor.h
index 463d7614d2..951c3e9b0e 100644
--- a/gtk2_ardour/public_editor.h
+++ b/gtk2_ardour/public_editor.h
@@ -141,11 +141,14 @@ class PublicEditor : public Gtk::Window, public PBD::StatefulDestructible, publi
 	 */
 	virtual void set_snap_threshold (double t) = 0;
 
-	/** Snap a value according to the current snap setting. */
+	/**
+	 * Snap a value according to the current snap setting.
+	 * ensure_snap overrides SnapOff and magnetic snap
+	 */
 	virtual void snap_to (framepos_t&       first,
 	                      ARDOUR::RoundMode direction = ARDOUR::RoundNearest,
 	                      bool              for_mark  = false,
-			      bool              explicitly = false) = 0;
+			      bool              ensure_snap = false) = 0;
 
 	/** Undo some transactions.
 	 * @param n Number of transactions to undo.
diff --git a/gtk2_ardour/region_view.cc b/gtk2_ardour/region_view.cc
index 5a8e3f235b..75e091f2d0 100644
--- a/gtk2_ardour/region_view.cc
+++ b/gtk2_ardour/region_view.cc
@@ -939,10 +939,12 @@ RegionView::move_contents (frameoffset_t distance)
 
 /** Snap a frame offset within our region using the current snap settings.
  *  @param x Frame offset from this region's position.
+ *  @param ensure_snap whether to ignore snap_mode (in the case of SnapOff) and magnetic snap.
+ *  Used when inverting snap mode logic with key modifiers, or snap distance calculation.
  *  @return Snapped frame offset from this region's position.
  */
 frameoffset_t
-RegionView::snap_frame_to_frame (frameoffset_t x, bool explicitly) const
+RegionView::snap_frame_to_frame (frameoffset_t x, bool ensure_snap) const
 {
 	PublicEditor& editor = trackview.editor();
 
@@ -951,12 +953,12 @@ RegionView::snap_frame_to_frame (frameoffset_t x, bool explicitly) const
 
 	/* try a snap in either direction */
 	framepos_t frame = session_frame;
-	editor.snap_to (frame, RoundNearest, false, explicitly);
+	editor.snap_to (frame, RoundNearest, false, ensure_snap);
 
 	/* if we went off the beginning of the region, snap forwards */
 	if (frame < _region->position ()) {
 		frame = session_frame;
-		editor.snap_to (frame, RoundUpAlways, false, explicitly);
+		editor.snap_to (frame, RoundUpAlways, false, ensure_snap);
 	}
 
 	/* back to region relative */
diff --git a/gtk2_ardour/region_view.h b/gtk2_ardour/region_view.h
index 23b08f4b12..f17e37a72d 100644
--- a/gtk2_ardour/region_view.h
+++ b/gtk2_ardour/region_view.h
@@ -121,8 +121,8 @@ class RegionView : public TimeAxisViewItem
 		}
 	};
 
-	ARDOUR::frameoffset_t snap_frame_to_frame (ARDOUR::frameoffset_t, bool explicitly = false) const;
-	
+	ARDOUR::frameoffset_t snap_frame_to_frame (ARDOUR::frameoffset_t, bool ensure_snap = false) const;
+
   protected:
 
 	/** Allows derived types to specify their visibility requirements