Fix many offset errors when drawing notes in midi regions that have been edited.

	- hopefully this can close bugs such as 6270 & 6793

5 files changed, 93 insertions, 24 deletions

diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index 76d46b86be..9e271cc398 100644
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -928,6 +928,11 @@ MidiRegionView::create_note_at (framepos_t t, double y, Evoral::Beats length, bo
 
 	MidiTimeAxisView* const mtv  = dynamic_cast<MidiTimeAxisView*>(&trackview);
 	MidiStreamView* const   view = mtv->midi_view();
+	boost::shared_ptr<MidiRegion> mr  = boost::dynamic_pointer_cast<MidiRegion> (_region);
+
+	if (!mr) {
+		return;
+	}
 
 	// Start of note in frames relative to region start
 	if (snap_t) {
@@ -935,9 +940,8 @@ MidiRegionView::create_note_at (framepos_t t, double y, Evoral::Beats length, bo
 		t = snap_frame_to_grid_underneath (t, grid_frames);
 	}
 
-	const MidiModel::TimeType beat_time = region_frames_to_region_beats(
-		t + _region->start());
-
+	const MidiModel::TimeType beat_time = Evoral::Beats (trackview.session()->tempo_map().beat_at_frame (_region->position() + t)
+							     - (mr->beat() - mr->start_beats().to_double()));
 	const double  note     = view->y_to_note(y);
 	const uint8_t chan     = mtv->get_channel_for_add();
 	const uint8_t velocity = get_velocity_for_add(beat_time);
diff --git a/libs/ardour/ardour/midi_region.h b/libs/ardour/ardour/midi_region.h
index b952c90f82..b4557ed1dd 100644
--- a/libs/ardour/ardour/midi_region.h
+++ b/libs/ardour/ardour/midi_region.h
@@ -101,7 +101,7 @@ class LIBARDOUR_API MidiRegion : public Region
 	boost::shared_ptr<const MidiModel> model() const;
 
 	void fix_negative_start ();
-
+	Evoral::Beats start_beats () {return _start_beats.val(); }
   protected:
 
 	virtual bool can_trim_start_before_source_start () const {
@@ -134,6 +134,7 @@ class LIBARDOUR_API MidiRegion : public Region
 	void set_position_internal (framepos_t pos, bool allow_bbt_recompute);
 	void set_length_internal (framecnt_t len);
 	void set_start_internal (framecnt_t);
+	void trim_to_internal (framepos_t position, framecnt_t length);
 	void update_length_beats ();
 
 	void model_changed ();
diff --git a/libs/ardour/ardour/region.h b/libs/ardour/ardour/region.h
index 29f4244dff..3a8a694c49 100644
--- a/libs/ardour/ardour/region.h
+++ b/libs/ardour/ardour/region.h
@@ -171,7 +171,7 @@ class LIBARDOUR_API Region
 	Trimmable::CanTrim can_trim () const;
 
 	PositionLockStyle position_lock_style () const { return _position_lock_style; }
-
+	double beat () { return _beat; }
 	void set_position_lock_style (PositionLockStyle ps);
 	void recompute_position_from_lock_style ();
 
@@ -358,6 +358,8 @@ class LIBARDOUR_API Region
 	virtual void set_position_internal (framepos_t pos, bool allow_bbt_recompute);
 	virtual void set_length_internal (framecnt_t);
 	virtual void set_start_internal (framecnt_t);
+	bool verify_start_and_length (framepos_t, framecnt_t&);
+	void first_edit ();
 
 	DataType _type;
 
@@ -393,15 +395,13 @@ class LIBARDOUR_API Region
   private:
 	void mid_thaw (const PBD::PropertyChange&);
 
-	void trim_to_internal (framepos_t position, framecnt_t length);
+	virtual void trim_to_internal (framepos_t position, framecnt_t length);
 	void modify_front (framepos_t new_position, bool reset_fade);
 	void modify_end (framepos_t new_position, bool reset_fade);
 
 	void maybe_uncopy ();
-	void first_edit ();
 
 	bool verify_start (framepos_t);
-	bool verify_start_and_length (framepos_t, framecnt_t&);
 	bool verify_start_mutable (framepos_t&_start);
 	bool verify_length (framecnt_t&);
 
diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc
index 5d952824ea..e0297b71f0 100644
--- a/libs/ardour/midi_region.cc
+++ b/libs/ardour/midi_region.cc
@@ -182,8 +182,7 @@ MidiRegion::post_set (const PropertyChange& pc)
 void
 MidiRegion::set_start_beats_from_start_frames ()
 {
-	BeatsFramesConverter c (_session.tempo_map(), _position - _start);
-	_start_beats = c.from (_start);
+	_start_beats = Evoral::Beats (beat() - _session.tempo_map().beat_at_frame (_position - _start));
 }
 
 void
@@ -198,8 +197,7 @@ MidiRegion::update_after_tempo_map_change (bool /* send */)
 {
 	Region::update_after_tempo_map_change (false);
 
-	/* _position has now been updated for the new tempo map */
-	_start = _position - _session.tempo_map().framepos_minus_beats (_position, _start_beats);
+	/* _start has now been updated. */
 	_length = _session.tempo_map().framepos_plus_beats (_position, _length_beats) - _position;
 
 	PropertyChange s_and_l;
@@ -213,8 +211,7 @@ MidiRegion::update_after_tempo_map_change (bool /* send */)
 void
 MidiRegion::update_length_beats ()
 {
-	BeatsFramesConverter converter (_session.tempo_map(), _position);
-	_length_beats = converter.from (_length);
+	_length_beats = Evoral::Beats (_session.tempo_map().beat_at_frame (_position + _length) - beat());
 }
 
 void
@@ -223,13 +220,12 @@ MidiRegion::set_position_internal (framepos_t pos, bool allow_bbt_recompute)
 	Region::set_position_internal (pos, allow_bbt_recompute);
 
 	/* set _start to new position in tempo map */
-	_start = _position - _session.tempo_map().framepos_minus_beats (_position, _start_beats);
+	_start = _position - _session.tempo_map().frame_at_beat (beat() - _start_beats.val().to_double());
 
 	/* leave _length_beats alone, and change _length to reflect the state of things
 	   at the new position (tempo map may dictate a different number of frames).
 	*/
-	BeatsFramesConverter converter (_session.tempo_map(), _position);
-	Region::set_length_internal (converter.to (_length_beats));
+	Region::set_length_internal (_session.tempo_map().frame_at_beat (beat() + _length_beats.val().to_double()) - _position);
 }
 
 framecnt_t
@@ -467,3 +463,71 @@ MidiRegion::set_start_internal (framecnt_t s)
 	Region::set_start_internal (s);
 	set_start_beats_from_start_frames ();
 }
+
+void
+MidiRegion::trim_to_internal (framepos_t position, framecnt_t length)
+{
+	framepos_t new_start;
+
+	if (locked()) {
+		return;
+	}
+
+	PropertyChange what_changed;
+
+	/* beat has not been set by set_position_internal */
+	const double beat_delta = _session.tempo_map().beat_at_frame (position) - beat();
+	const double old_beat = beat();
+
+	/* Set position before length, otherwise for MIDI regions this bad thing happens:
+	 * 1. we call set_length_internal; length in beats is computed using the region's current
+	 *    (soon-to-be old) position
+	 * 2. we call set_position_internal; position is set and length in frames re-computed using
+	 *    length in beats from (1) but at the new position, which is wrong if the region
+	 *    straddles a tempo/meter change.
+	 */
+
+	if (_position != position) {
+		set_position_internal (position, true);
+		what_changed.add (Properties::position);
+	}
+
+	const double new_beat = _session.tempo_map().beat_at_frame (position);
+	const double new_start_beat = _start_beats.val().to_double() + beat_delta;
+
+	new_start = _position - _session.tempo_map().frame_at_beat (new_beat - new_start_beat);
+
+	if (!verify_start_and_length (new_start, length)) {
+		return;
+	}
+
+	if (_start != new_start) {
+		_start_beats = Evoral::Beats (new_start_beat);
+		what_changed.add (Properties::start_beats);
+
+		set_start_internal (new_start);
+		what_changed.add (Properties::start);
+	}
+
+
+
+	if (_length != length) {
+		set_length_internal (length);
+		what_changed.add (Properties::length);
+	}
+
+	set_whole_file (false);
+
+	PropertyChange start_and_length;
+
+	start_and_length.add (Properties::start);
+	start_and_length.add (Properties::length);
+
+	if (what_changed.contains (start_and_length)) {
+		first_edit ();
+	}
+
+	if (!what_changed.empty()) {
+		send_change (what_changed);
+	}
+}
diff --git a/libs/ardour/region.cc b/libs/ardour/region.cc
index 15a3da8b8b..9119659870 100644
--- a/libs/ardour/region.cc
+++ b/libs/ardour/region.cc
@@ -441,7 +441,6 @@ Region::set_length (framecnt_t len)
 		}
 
 
-		_last_length = _length;
 		set_length_internal (len);
 		_whole_file = false;
 		first_edit ();
@@ -459,6 +458,7 @@ Region::set_length (framecnt_t len)
 void
 Region::set_length_internal (framecnt_t len)
 {
+	_last_length = _length;
 	_length = len;
 }
 
@@ -925,6 +925,12 @@ Region::trim_to_internal (framepos_t position, framecnt_t length)
 
 	PropertyChange what_changed;
 
+	if (_start != new_start) {
+		set_start_internal (new_start);
+		what_changed.add (Properties::start);
+	}
+
+
 	/* Set position before length, otherwise for MIDI regions this bad thing happens:
 	 * 1. we call set_length_internal; length in beats is computed using the region's current
 	 *    (soon-to-be old) position
@@ -941,11 +947,6 @@ Region::trim_to_internal (framepos_t position, framecnt_t length)
 		what_changed.add (Properties::position);
 	}
 
-	if (_start != new_start) {
-		set_start_internal (new_start);
-		what_changed.add (Properties::start);
-	}
-
 	if (_length != length) {
 		if (!property_changes_suspended()) {
 			_last_length = _length;
@@ -1834,4 +1835,3 @@ Region::latest_possible_frame () const
 
 	return _position + (minlen - _start) - 1;
 }
-