rework snap

snap now fills in a struct (MusicFrame) which contins a snapped frame
along with a music divisor.
this gives useful information wrt magnetic snap which may or may not
have rounded to an exact musical position.

region position may now be set musically (using quarter notes for now).

this patch fixes several problems in the current code:

	- dragging a list of music-locked regions now maintains correct
	  musical offsets within the list.

	- splitting regions using magnetic snap works correctly (#7192)

	- cut drag should now work correctly with magnetic snap.

	- musical length of split midi regions is no longer frame based.

16 files changed, 319 insertions, 119 deletions

diff --git a/libs/ardour/ardour/audioregion.h b/libs/ardour/ardour/audioregion.h
index d25cf0e421..a2c43460ed 100644
--- a/libs/ardour/ardour/audioregion.h
+++ b/libs/ardour/ardour/audioregion.h
@@ -189,7 +189,7 @@ class LIBARDOUR_API AudioRegion : public Region
 	AudioRegion (boost::shared_ptr<AudioSource>);
 	AudioRegion (const SourceList &);
 	AudioRegion (boost::shared_ptr<const AudioRegion>);
-	AudioRegion (boost::shared_ptr<const AudioRegion>, frameoffset_t offset, const int32_t sub_num);
+	AudioRegion (boost::shared_ptr<const AudioRegion>, ARDOUR::MusicFrame offset);
 	AudioRegion (boost::shared_ptr<const AudioRegion>, const SourceList&);
 	AudioRegion (SourceList &);
 
diff --git a/libs/ardour/ardour/midi_playlist.h b/libs/ardour/ardour/midi_playlist.h
index 3c031a994f..49eb892e42 100644
--- a/libs/ardour/ardour/midi_playlist.h
+++ b/libs/ardour/ardour/midi_playlist.h
@@ -87,6 +87,7 @@ public:
 	int set_state (const XMLNode&, int version);
 
 	bool destroy_region (boost::shared_ptr<Region>);
+	void _split_region (boost::shared_ptr<Region>, MusicFrame position);
 
 	void set_note_mode (NoteMode m) { _note_mode = m; }
 
diff --git a/libs/ardour/ardour/midi_region.h b/libs/ardour/ardour/midi_region.h
index 8f1edded21..b41f620e87 100644
--- a/libs/ardour/ardour/midi_region.h
+++ b/libs/ardour/ardour/midi_region.h
@@ -127,7 +127,7 @@ class LIBARDOUR_API MidiRegion : public Region
 
 	MidiRegion (const SourceList&);
 	MidiRegion (boost::shared_ptr<const MidiRegion>);
-	MidiRegion (boost::shared_ptr<const MidiRegion>, frameoffset_t offset, const int32_t sub_num = 0);
+	MidiRegion (boost::shared_ptr<const MidiRegion>, ARDOUR::MusicFrame offset);
 
 	framecnt_t _read_at (const SourceList&, Evoral::EventSink<framepos_t>& dst,
 	                     framepos_t position,
@@ -146,6 +146,7 @@ class LIBARDOUR_API MidiRegion : public Region
 	void recompute_at_end ();
 
 	void set_position_internal (framepos_t pos, bool allow_bbt_recompute, const int32_t sub_num);
+	void set_position_music_internal (double qn);
 	void set_length_internal (framecnt_t len, const int32_t sub_num);
 	void set_start_internal (framecnt_t, const int32_t sub_num);
 	void trim_to_internal (framepos_t position, framecnt_t length, const int32_t sub_num);
diff --git a/libs/ardour/ardour/playlist.h b/libs/ardour/ardour/playlist.h
index bdf17eabf0..580b245c47 100644
--- a/libs/ardour/ardour/playlist.h
+++ b/libs/ardour/ardour/playlist.h
@@ -135,14 +135,14 @@ public:
 
 	/* Editing operations */
 
-	void add_region (boost::shared_ptr<Region>, framepos_t position, float times = 1, bool auto_partition = false, const int32_t sub_num = 0);
+	void add_region (boost::shared_ptr<Region>, framepos_t position, float times = 1, bool auto_partition = false, int32_t sub_num = 0, double quarter_note = 0.0, bool for_music = false);
 	void remove_region (boost::shared_ptr<Region>);
 	void get_equivalent_regions (boost::shared_ptr<Region>, std::vector<boost::shared_ptr<Region> >&);
 	void get_region_list_equivalent_regions (boost::shared_ptr<Region>, std::vector<boost::shared_ptr<Region> >&);
 	void get_source_equivalent_regions (boost::shared_ptr<Region>, std::vector<boost::shared_ptr<Region> >&);
 	void replace_region (boost::shared_ptr<Region> old, boost::shared_ptr<Region> newr, framepos_t pos);
-	void split_region (boost::shared_ptr<Region>, framepos_t position, const int32_t sub_num);
-	void split (framepos_t at, const int32_t sub_num);
+	void split_region (boost::shared_ptr<Region>, MusicFrame position);
+	void split (MusicFrame at);
 	void shift (framepos_t at, frameoffset_t distance, bool move_intersected, bool ignore_music_glue);
 	void partition (framepos_t start, framepos_t end, bool cut = false);
 	void duplicate (boost::shared_ptr<Region>, framepos_t position, float times);
@@ -372,7 +372,7 @@ public:
 
 	virtual XMLNode& state (bool);
 
-	bool add_region_internal (boost::shared_ptr<Region>, framepos_t position, const int32_t sub_num = 0);
+	bool add_region_internal (boost::shared_ptr<Region>, framepos_t position, int32_t sub_num = 0, double quarter_note = 0.0, bool for_music = false);
 
 	int remove_region_internal (boost::shared_ptr<Region>);
 	void copy_regions (RegionList&) const;
@@ -390,7 +390,7 @@ public:
 	void begin_undo ();
 	void end_undo ();
 
-	void _split_region (boost::shared_ptr<Region>, framepos_t position, const int32_t sub_num);
+	virtual void _split_region (boost::shared_ptr<Region>, MusicFrame position);
 
 	typedef std::pair<boost::shared_ptr<Region>, boost::shared_ptr<Region> > TwoRegions;
 
diff --git a/libs/ardour/ardour/region.h b/libs/ardour/ardour/region.h
index a01ef703f3..85e0442a41 100644
--- a/libs/ardour/ardour/region.h
+++ b/libs/ardour/ardour/region.h
@@ -215,6 +215,7 @@ class LIBARDOUR_API Region
 	void set_length (framecnt_t, const int32_t sub_num);
 	void set_start (framepos_t);
 	void set_position (framepos_t, int32_t sub_num = 0);
+	void set_position_music (double qn);
 	void set_initial_position (framepos_t);
 	void special_set_position (framepos_t);
 	virtual void update_after_tempo_map_change (bool send_change = true);
@@ -346,7 +347,7 @@ class LIBARDOUR_API Region
 	Region (boost::shared_ptr<const Region>);
 
 	/** Construct a region from another region, at an offset within that region */
-	Region (boost::shared_ptr<const Region>, frameoffset_t start_offset, const int32_t sub_num);
+	Region (boost::shared_ptr<const Region>, ARDOUR::MusicFrame start_offset);
 
 	/** Construct a region as a copy of another region, but with different sources */
 	Region (boost::shared_ptr<const Region>, const SourceList&);
@@ -364,6 +365,7 @@ class LIBARDOUR_API Region
 	virtual int _set_state (const XMLNode&, int version, PBD::PropertyChange& what_changed, bool send_signal);
 	void post_set (const PBD::PropertyChange&);
 	virtual void set_position_internal (framepos_t pos, bool allow_bbt_recompute, const int32_t sub_num);
+	virtual void set_position_music_internal (double qn);
 	virtual void set_length_internal (framecnt_t, const int32_t sub_num);
 	virtual void set_start_internal (framecnt_t, const int32_t sub_num = 0);
 	bool verify_start_and_length (framepos_t, framecnt_t&);
diff --git a/libs/ardour/ardour/region_factory.h b/libs/ardour/ardour/region_factory.h
index 6971e77242..4b32da0512 100644
--- a/libs/ardour/ardour/region_factory.h
+++ b/libs/ardour/ardour/region_factory.h
@@ -59,7 +59,7 @@ public:
 	static PBD::Signal1<void,boost::shared_ptr<Region> >  CheckNewRegion;
 
 	/** create a "pure copy" of Region @param other */
-	static boost::shared_ptr<Region> create (boost::shared_ptr<const Region> other, bool announce = false, const int32_t sub_num = 0);
+	static boost::shared_ptr<Region> create (boost::shared_ptr<const Region> other, bool announce = false);
 
 	/** create a region from a single Source */
 	static boost::shared_ptr<Region> create (boost::shared_ptr<Source>,
@@ -72,8 +72,8 @@ public:
 	static boost::shared_ptr<Region> create (boost::shared_ptr<Region> other,
 	                                         const PBD::PropertyList&, bool announce = true);
 	/** create a copy of @param other starting at @param offset within @param other */
-	static boost::shared_ptr<Region> create (boost::shared_ptr<Region> other, frameoffset_t offset,
-	                                         const PBD::PropertyList&, bool announce = true, const int32_t sub_num = 0);
+	static boost::shared_ptr<Region> create (boost::shared_ptr<Region> other, ARDOUR::MusicFrame offset,
+	                                         const PBD::PropertyList&, bool announce = true);
 	/** create a "copy" of @param other but using a different set of sources @param srcs */
 	static boost::shared_ptr<Region> create (boost::shared_ptr<Region> other, const SourceList& srcs,
 	                                         const PBD::PropertyList&, bool announce = true);
diff --git a/libs/ardour/ardour/tempo.h b/libs/ardour/ardour/tempo.h
index a3ab8d68f6..1dce6347a0 100644
--- a/libs/ardour/ardour/tempo.h
+++ b/libs/ardour/ardour/tempo.h
@@ -386,10 +386,10 @@ class LIBARDOUR_API TempoMap : public PBD::StatefulDestructible
 
 	void replace_meter (const MeterSection&, const Meter&, const Timecode::BBT_Time& where, framepos_t frame, PositionLockStyle pls);
 
-	framepos_t round_to_bar  (framepos_t frame, RoundMode dir);
-	framepos_t round_to_beat (framepos_t frame, RoundMode dir);
+	MusicFrame round_to_bar  (framepos_t frame, RoundMode dir);
+	MusicFrame round_to_beat (framepos_t frame, RoundMode dir);
 	framepos_t round_to_beat_subdivision (framepos_t fr, int sub_num, RoundMode dir);
-	framepos_t round_to_quarter_note_subdivision (framepos_t fr, int sub_num, RoundMode dir);
+	MusicFrame round_to_quarter_note_subdivision (framepos_t fr, int sub_num, RoundMode dir);
 
 	void set_length (framepos_t frames);
 
@@ -564,7 +564,7 @@ private:
 	void recompute_meters (Metrics& metrics);
 	void recompute_map (Metrics& metrics, framepos_t end = -1);
 
-	framepos_t round_to_type (framepos_t fr, RoundMode dir, BBTPointType);
+	MusicFrame round_to_type (framepos_t fr, RoundMode dir, BBTPointType);
 
 	const MeterSection& first_meter() const;
 	MeterSection&       first_meter();
diff --git a/libs/ardour/ardour/types.h b/libs/ardour/ardour/types.h
index 46ef2bade9..0a2fd62f7b 100644
--- a/libs/ardour/ardour/types.h
+++ b/libs/ardour/ardour/types.h
@@ -319,6 +319,27 @@ namespace ARDOUR {
 		}
 	};
 
+	/* used for translating audio frames to an exact musical position using a note divisor.
+	   an exact musical position almost never falls exactly on an audio frame, but for sub-sample
+	   musical accuracy we need to derive exact musical locations from a frame position
+	   the division follows TempoMap::exact_beat_at_frame().
+	   division
+	   -1       musical location is the bar closest to frame
+	    0       musical location is the musical position of the frame
+	    1       musical location is the BBT beat closest to frame
+	    n       musical location is the quarter-note division n closest to frame
+	*/
+	struct MusicFrame {
+		framepos_t frame;
+		int32_t    division;
+
+		MusicFrame (framepos_t f, int32_t d) : frame (f), division (d) {}
+
+		void set (framepos_t f, int32_t d) {frame = f; division = d; }
+
+		MusicFrame operator- (MusicFrame other) { return MusicFrame (frame - other.frame, 0); }
+	};
+
 	/* XXX: slightly unfortunate that there is this and Evoral::Range<>,
 	   but this has a uint32_t id which Evoral::Range<> does not.
 	*/
diff --git a/libs/ardour/audioregion.cc b/libs/ardour/audioregion.cc
index bf4c5c6910..7d8c003791 100644
--- a/libs/ardour/audioregion.cc
+++ b/libs/ardour/audioregion.cc
@@ -279,13 +279,13 @@ AudioRegion::AudioRegion (boost::shared_ptr<const AudioRegion> other)
 	assert (_sources.size() == _master_sources.size());
 }
 
-AudioRegion::AudioRegion (boost::shared_ptr<const AudioRegion> other, framecnt_t offset, const int32_t sub_num)
-	: Region (other, offset, sub_num)
+AudioRegion::AudioRegion (boost::shared_ptr<const AudioRegion> other, MusicFrame offset)
+	: Region (other, offset)
 	, AUDIOREGION_COPY_STATE (other)
 	  /* As far as I can see, the _envelope's times are relative to region position, and have nothing
 	     to do with sources (and hence _start).  So when we copy the envelope, we just use the supplied offset.
 	  */
-	, _envelope (Properties::envelope, boost::shared_ptr<AutomationList> (new AutomationList (*other->_envelope.val(), offset, other->_length)))
+	, _envelope (Properties::envelope, boost::shared_ptr<AutomationList> (new AutomationList (*other->_envelope.val(), offset.frame, other->_length)))
 	, _automatable (other->session())
 	, _fade_in_suspended (0)
 	, _fade_out_suspended (0)
diff --git a/libs/ardour/midi_playlist.cc b/libs/ardour/midi_playlist.cc
index 67764d66ae..20c1c77111 100644
--- a/libs/ardour/midi_playlist.cc
+++ b/libs/ardour/midi_playlist.cc
@@ -33,7 +33,9 @@
 #include "ardour/midi_region.h"
 #include "ardour/midi_source.h"
 #include "ardour/midi_state_tracker.h"
+#include "ardour/region_factory.h"
 #include "ardour/session.h"
+#include "ardour/tempo.h"
 #include "ardour/types.h"
 
 #include "pbd/i18n.h"
@@ -381,6 +383,80 @@ MidiPlaylist::destroy_region (boost::shared_ptr<Region> region)
 
 	return changed;
 }
+void
+MidiPlaylist::_split_region (boost::shared_ptr<Region> region, MusicFrame playlist_position)
+{
+	if (!region->covers (playlist_position.frame)) {
+		return;
+	}
+
+	if (region->position() == playlist_position.frame ||
+	    region->last_frame() == playlist_position.frame) {
+		return;
+	}
+
+	boost::shared_ptr<const MidiRegion> mr = boost::dynamic_pointer_cast<MidiRegion>(region);
+
+	if (mr == 0) {
+		return;
+	}
+
+	boost::shared_ptr<Region> left;
+	boost::shared_ptr<Region> right;
+
+	string before_name;
+	string after_name;
+	const double before_qn = _session.tempo_map().exact_qn_at_frame (playlist_position.frame, playlist_position.division) - region->quarter_note();
+	const double after_qn = mr->length_beats() - before_qn;
+	MusicFrame before (playlist_position.frame - region->position(), playlist_position.division);
+	MusicFrame after (region->length() - before.frame, playlist_position.division);
+
+	/* split doesn't change anything about length, so don't try to splice */
+	bool old_sp = _splicing;
+	_splicing = true;
+
+	RegionFactory::region_name (before_name, region->name(), false);
+
+	{
+		PropertyList plist;
+
+		plist.add (Properties::length, before.frame);
+		plist.add (Properties::length_beats, before_qn);
+		plist.add (Properties::name, before_name);
+		plist.add (Properties::left_of_split, true);
+		plist.add (Properties::layering_index, region->layering_index ());
+		plist.add (Properties::layer, region->layer ());
+
+		/* note: we must use the version of ::create with an offset here,
+		   since it supplies that offset to the Region constructor, which
+		   is necessary to get audio region gain envelopes right.
+		*/
+		left = RegionFactory::create (region, MusicFrame (0, 0), plist, true);
+	}
+
+	RegionFactory::region_name (after_name, region->name(), false);
+
+	{
+		PropertyList plist;
+
+		plist.add (Properties::length, after.frame);
+		plist.add (Properties::length_beats, after_qn);
+		plist.add (Properties::name, after_name);
+		plist.add (Properties::right_of_split, true);
+		plist.add (Properties::layering_index, region->layering_index ());
+		plist.add (Properties::layer, region->layer ());
+
+		/* same note as above */
+		right = RegionFactory::create (region, before, plist, true);
+	}
+
+	add_region_internal (left, region->position(), 0, region->quarter_note(), true);
+	add_region_internal (right, region->position() + before.frame, before.division, region->quarter_note() + before_qn, true);
+
+	remove_region_internal (region);
+
+	_splicing = old_sp;
+}
 
 set<Evoral::Parameter>
 MidiPlaylist::contained_automation()
diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc
index 7511aadce8..3c885a8d99 100644
--- a/libs/ardour/midi_region.cc
+++ b/libs/ardour/midi_region.cc
@@ -102,20 +102,20 @@ MidiRegion::MidiRegion (boost::shared_ptr<const MidiRegion> other)
 }
 
 /** Create a new MidiRegion that is part of an existing one */
-MidiRegion::MidiRegion (boost::shared_ptr<const MidiRegion> other, frameoffset_t offset, const int32_t sub_num)
-	: Region (other, offset, sub_num)
+MidiRegion::MidiRegion (boost::shared_ptr<const MidiRegion> other, MusicFrame offset)
+	: Region (other, offset)
 	, _start_beats (Properties::start_beats, other->_start_beats)
 	, _length_beats (Properties::length_beats, other->_length_beats)
 {
-	if (offset != 0) {
-		_start_beats = (_session.tempo_map().exact_qn_at_frame (other->_position + offset, sub_num) - other->_quarter_note) + other->_start_beats;
-		update_length_beats (sub_num);
-		/* we've potentially shifted _start_beats, now reset _start frames to match */
-		_start = _session.tempo_map().frames_between_quarter_notes (_quarter_note - _start_beats, _quarter_note);
-	}
 
 	register_properties ();
 
+	const double offset_quarter_note = _session.tempo_map().exact_qn_at_frame (other->_position + offset.frame, offset.division) - other->_quarter_note;
+	if (offset.frame != 0) {
+		_start_beats = other->_start_beats + offset_quarter_note;
+		_length_beats = other->_length_beats - offset_quarter_note;
+	}
+
 	assert(_name.val().find("/") == string::npos);
 	midi_source(0)->ModelChanged.connect_same_thread (_source_connection, boost::bind (&MidiRegion::model_changed, this));
 	model_changed ();
@@ -345,6 +345,24 @@ MidiRegion::set_position_internal (framepos_t pos, bool allow_bbt_recompute, con
 	}
 }
 
+void
+MidiRegion::set_position_music_internal (double qn)
+{
+	Region::set_position_music_internal (qn);
+	/* set _start to new position in tempo map */
+	_start = _session.tempo_map().frames_between_quarter_notes (quarter_note() - start_beats(), quarter_note());
+
+	if (position_lock_style() == AudioTime) {
+		_length_beats = _session.tempo_map().quarter_note_at_frame (_position + _length) - quarter_note();
+
+	} else {
+		/* 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).
+		*/
+		_length = _session.tempo_map().frames_between_quarter_notes (quarter_note(), quarter_note() + length_beats());
+	}
+}
+
 framecnt_t
 MidiRegion::read_at (Evoral::EventSink<framepos_t>& out,
                      framepos_t                     position,
diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc
index 7cea903d5d..99e65ab3e2 100644
--- a/libs/ardour/playlist.cc
+++ b/libs/ardour/playlist.cc
@@ -669,7 +669,7 @@ Playlist::clear_pending ()
 
 /** Note: this calls set_layer (..., DBL_MAX) so it will reset the layering index of region */
 void
-Playlist::add_region (boost::shared_ptr<Region> region, framepos_t position, float times, bool auto_partition, const int32_t sub_num)
+Playlist::add_region (boost::shared_ptr<Region> region, framepos_t position, float times, bool auto_partition, int32_t sub_num, double quarter_note, bool for_music)
 {
 	RegionWriteLock rlock (this);
 	times = fabs (times);
@@ -688,19 +688,18 @@ Playlist::add_region (boost::shared_ptr<Region> region, framepos_t position, flo
 	}
 
 	if (itimes >= 1) {
-		add_region_internal (region, pos, sub_num);
+		add_region_internal (region, pos, sub_num, quarter_note, for_music);
 		set_layer (region, DBL_MAX);
 		pos += region->length();
 		--itimes;
 	}
 
-
 	/* note that itimes can be zero if we being asked to just
 	   insert a single fraction of the region.
 	*/
 
 	for (int i = 0; i < itimes; ++i) {
-		boost::shared_ptr<Region> copy = RegionFactory::create (region, true, sub_num);
+		boost::shared_ptr<Region> copy = RegionFactory::create (region, true);
 		add_region_internal (copy, pos, sub_num);
 		set_layer (copy, DBL_MAX);
 		pos += region->length();
@@ -743,7 +742,7 @@ Playlist::set_region_ownership ()
 }
 
 bool
-Playlist::add_region_internal (boost::shared_ptr<Region> region, framepos_t position, const int32_t sub_num)
+Playlist::add_region_internal (boost::shared_ptr<Region> region, framepos_t position, int32_t sub_num, double quarter_note, bool for_music)
 {
 	if (region->data_type() != _type) {
 		return false;
@@ -755,8 +754,11 @@ Playlist::add_region_internal (boost::shared_ptr<Region> region, framepos_t posi
 		boost::shared_ptr<Playlist> foo (shared_from_this());
 		region->set_playlist (boost::weak_ptr<Playlist>(foo));
 	}
-
-	region->set_position (position, sub_num);
+	if (for_music) {
+		region->set_position_music (quarter_note);
+	} else {
+		region->set_position (position, sub_num);
+	}
 
 	regions.insert (upper_bound (regions.begin(), regions.end(), region, cmp), region);
 	all_regions.insert (region);
@@ -1411,7 +1413,7 @@ Playlist::duplicate_ranges (std::list<AudioRange>& ranges, float times)
  }
 
  void
- Playlist::split (framepos_t at, const int32_t sub_num)
+ Playlist::split (MusicFrame at)
  {
 	 RegionWriteLock rlock (this);
 	 RegionList copy (regions.rlist());
@@ -1420,33 +1422,34 @@ Playlist::duplicate_ranges (std::list<AudioRange>& ranges, float times)
 	  */
 
 	 for (RegionList::iterator r = copy.begin(); r != copy.end(); ++r) {
-		 _split_region (*r, at, sub_num);
+		 _split_region (*r, at);
 	 }
  }
 
  void
- Playlist::split_region (boost::shared_ptr<Region> region, framepos_t playlist_position, const int32_t sub_num)
+ Playlist::split_region (boost::shared_ptr<Region> region, MusicFrame playlist_position)
  {
 	 RegionWriteLock rl (this);
-	 _split_region (region, playlist_position, sub_num);
+	 _split_region (region, playlist_position);
  }
 
  void
- Playlist::_split_region (boost::shared_ptr<Region> region, framepos_t playlist_position, const int32_t sub_num)
+ Playlist::_split_region (boost::shared_ptr<Region> region, MusicFrame playlist_position)
  {
-	 if (!region->covers (playlist_position)) {
+	 if (!region->covers (playlist_position.frame)) {
 		 return;
 	 }
 
-	 if (region->position() == playlist_position ||
-	     region->last_frame() == playlist_position) {
+	 if (region->position() == playlist_position.frame ||
+	     region->last_frame() == playlist_position.frame) {
 		 return;
 	 }
 
 	 boost::shared_ptr<Region> left;
 	 boost::shared_ptr<Region> right;
-	 frameoffset_t before;
-	 frameoffset_t after;
+
+	 MusicFrame before (playlist_position.frame - region->position(), playlist_position.division);
+	 MusicFrame after (region->length() - before.frame, 0);
 	 string before_name;
 	 string after_name;
 
@@ -1455,15 +1458,12 @@ Playlist::duplicate_ranges (std::list<AudioRange>& ranges, float times)
 	 bool old_sp = _splicing;
 	 _splicing = true;
 
-	 before = playlist_position - region->position();
-	 after = region->length() - before;
-
 	 RegionFactory::region_name (before_name, region->name(), false);
 
 	 {
 		 PropertyList plist;
 
-		 plist.add (Properties::length, before);
+		 plist.add (Properties::length, before.frame);
 		 plist.add (Properties::name, before_name);
 		 plist.add (Properties::left_of_split, true);
 		 plist.add (Properties::layering_index, region->layering_index ());
@@ -1473,7 +1473,7 @@ Playlist::duplicate_ranges (std::list<AudioRange>& ranges, float times)
 		    since it supplies that offset to the Region constructor, which
 		    is necessary to get audio region gain envelopes right.
 		 */
-		 left = RegionFactory::create (region, 0, plist, true, sub_num);
+		 left = RegionFactory::create (region, MusicFrame (0, 0), plist, true);
 	 }
 
 	 RegionFactory::region_name (after_name, region->name(), false);
@@ -1481,18 +1481,19 @@ Playlist::duplicate_ranges (std::list<AudioRange>& ranges, float times)
 	 {
 		 PropertyList plist;
 
-		 plist.add (Properties::length, after);
+		 plist.add (Properties::length, after.frame);
 		 plist.add (Properties::name, after_name);
 		 plist.add (Properties::right_of_split, true);
 		 plist.add (Properties::layering_index, region->layering_index ());
 		 plist.add (Properties::layer, region->layer ());
 
 		 /* same note as above */
-		 right = RegionFactory::create (region, before, plist, true, sub_num);
+		 right = RegionFactory::create (region, before, plist, true);
 	 }
 
-	 add_region_internal (left, region->position());
-	 add_region_internal (right, region->position() + before);
+	 add_region_internal (left, region->position(), 0);
+	 add_region_internal (right, region->position() + before.frame, before.division);
+
 	 remove_region_internal (region);
 
 	 _splicing = old_sp;
diff --git a/libs/ardour/region.cc b/libs/ardour/region.cc
index 0d0a40d17a..4eda001708 100644
--- a/libs/ardour/region.cc
+++ b/libs/ardour/region.cc
@@ -329,7 +329,7 @@ Region::Region (boost::shared_ptr<const Region> other)
     the start within \a other is given by \a offset
     (i.e. relative to the start of \a other's sources, the start is \a offset + \a other.start()
 */
-Region::Region (boost::shared_ptr<const Region> other, frameoffset_t offset, const int32_t sub_num)
+Region::Region (boost::shared_ptr<const Region> other, MusicFrame offset)
 	: SessionObject(other->session(), other->name())
 	, _type (other->data_type())
 	, REGION_COPY_STATE (other)
@@ -343,7 +343,6 @@ Region::Region (boost::shared_ptr<const Region> other, frameoffset_t offset, con
 	/* override state that may have been incorrectly inherited from the other region
 	 */
 
-	_position = other->_position + offset;
 	_locked = false;
 	_whole_file = false;
 	_hidden = false;
@@ -351,9 +350,17 @@ Region::Region (boost::shared_ptr<const Region> other, frameoffset_t offset, con
 	use_sources (other->_sources);
 	set_master_sources (other->_master_sources);
 
-	_start = other->_start + offset;
-	_beat = _session.tempo_map().exact_beat_at_frame (_position, sub_num);
-	_quarter_note = _session.tempo_map().exact_qn_at_frame (_position, sub_num);
+	_position = other->_position + offset.frame;
+	_start = other->_start + offset.frame;
+
+	/* prevent offset of 0 from altering musical position */
+	if (offset.frame != 0) {
+		const double offset_qn = _session.tempo_map().exact_qn_at_frame (other->_position + offset.frame, offset.division)
+			- other->_quarter_note;
+
+		_quarter_note = other->_quarter_note + offset_qn;
+		_beat = _session.tempo_map().beat_at_quarter_note (_quarter_note);
+	}
 
 	/* if the other region had a distinct sync point
 	   set, then continue to use it as best we can.
@@ -589,6 +596,13 @@ Region::set_position (framepos_t pos, int32_t sub_num)
 		return;
 	}
 
+	/* do this even if the position is the same. this helps out
+	   a GUI that has moved its representation already.
+	*/
+	PropertyChange p_and_l;
+
+	p_and_l.add (Properties::position);
+
 	if (position_lock_style() == AudioTime) {
 		set_position_internal (pos, true, sub_num);
 	} else {
@@ -596,67 +610,79 @@ Region::set_position (framepos_t pos, int32_t sub_num)
 			_beat = _session.tempo_map().exact_beat_at_frame (pos, sub_num);
 		}
 
-		/* will set pulse accordingly */
+		/* will set quarter note accordingly */
 		set_position_internal (pos, false, sub_num);
 	}
 
-	/* do this even if the position is the same. this helps out
-	   a GUI that has moved its representation already.
-	*/
-	PropertyChange p_and_l;
-
-	p_and_l.add (Properties::position);
-	/* Currently length change due to position change is only implemented
-	   for MidiRegion (Region has no length in beats).
-	   Notify a length change regardless (its more efficient for MidiRegions),
-	   and when Region has a _length_beats we will need it here anyway).
-	*/
-	p_and_l.add (Properties::length);
+	if (position_lock_style() == MusicTime) {
+		p_and_l.add (Properties::length);
+	}
 
 	send_change (p_and_l);
 
 }
 
-/** A gui may need to create a region, then place it in an initial
- *  position determined by the user.
- *  When this takes place within one gui operation, we have to reset
- *  _last_position to prevent an implied move.
- */
 void
-Region::set_initial_position (framepos_t pos)
+Region::set_position_internal (framepos_t pos, bool allow_bbt_recompute, const int32_t sub_num)
 {
-	if (!can_move()) {
-		return;
-	}
+	/* We emit a change of Properties::position even if the position hasn't changed
+	   (see Region::set_position), so we must always set this up so that
+	   e.g. Playlist::notify_region_moved doesn't use an out-of-date last_position.
+	*/
+	_last_position = _position;
 
 	if (_position != pos) {
 		_position = pos;
 
+		if (allow_bbt_recompute) {
+			recompute_position_from_lock_style (sub_num);
+		} else {
+			/* MusicTime dictates that we glue to ardour beats. the pulse may have changed.*/
+			_quarter_note = _session.tempo_map().quarter_note_at_beat (_beat);
+		}
+
 		/* check that the new _position wouldn't make the current
 		   length impossible - if so, change the length.
 
 		   XXX is this the right thing to do?
 		*/
-
 		if (max_framepos - _length < _position) {
 			_last_length = _length;
 			_length = max_framepos - _position;
 		}
-
-		recompute_position_from_lock_style (0);
-		/* ensure that this move doesn't cause a range move */
-		_last_position = _position;
 	}
+}
 
+void
+Region::set_position_music (double qn)
+{
+	if (!can_move()) {
+		return;
+	}
 
 	/* do this even if the position is the same. this helps out
 	   a GUI that has moved its representation already.
 	*/
-	send_change (Properties::position);
+	PropertyChange p_and_l;
+
+	p_and_l.add (Properties::position);
+
+	if (!_session.loading()) {
+		_beat = _session.tempo_map().beat_at_quarter_note (qn);
+	}
+
+	/* will set frame accordingly */
+	set_position_music_internal (qn);
+
+	if (position_lock_style() == MusicTime) {
+		p_and_l.add (Properties::length);
+	}
+
+	send_change (p_and_l);
 }
 
 void
-Region::set_position_internal (framepos_t pos, bool allow_bbt_recompute, const int32_t sub_num)
+Region::set_position_music_internal (double qn)
 {
 	/* We emit a change of Properties::position even if the position hasn't changed
 	   (see Region::set_position), so we must always set this up so that
@@ -664,26 +690,58 @@ Region::set_position_internal (framepos_t pos, bool allow_bbt_recompute, const i
 	*/
 	_last_position = _position;
 
-	if (_position != pos) {
-		_position = pos;
+	if (_quarter_note != qn) {
+		_position = _session.tempo_map().frame_at_quarter_note (qn);
+		_quarter_note = qn;
 
-		if (allow_bbt_recompute) {
-			recompute_position_from_lock_style (sub_num);
-		} else {
-			/* MusicTime dictates that we glue to ardour beats. the pulse may have changed.*/
-			_quarter_note = _session.tempo_map().quarter_note_at_beat (_beat);
+		/* check that the new _position wouldn't make the current
+		   length impossible - if so, change the length.
+
+		   XXX is this the right thing to do?
+		*/
+		if (max_framepos - _length < _position) {
+			_last_length = _length;
+			_length = max_framepos - _position;
 		}
+	}
+}
+
+/** A gui may need to create a region, then place it in an initial
+ *  position determined by the user.
+ *  When this takes place within one gui operation, we have to reset
+ *  _last_position to prevent an implied move.
+ */
+void
+Region::set_initial_position (framepos_t pos)
+{
+	if (!can_move()) {
+		return;
+	}
+
+	if (_position != pos) {
+		_position = pos;
 
 		/* check that the new _position wouldn't make the current
 		   length impossible - if so, change the length.
 
 		   XXX is this the right thing to do?
 		*/
+
 		if (max_framepos - _length < _position) {
 			_last_length = _length;
 			_length = max_framepos - _position;
 		}
+
+		recompute_position_from_lock_style (0);
+		/* ensure that this move doesn't cause a range move */
+		_last_position = _position;
 	}
+
+
+	/* do this even if the position is the same. this helps out
+	   a GUI that has moved its representation already.
+	*/
+	send_change (Properties::position);
 }
 
 void
diff --git a/libs/ardour/region_factory.cc b/libs/ardour/region_factory.cc
index 16a9e02e37..78425db88c 100644
--- a/libs/ardour/region_factory.cc
+++ b/libs/ardour/region_factory.cc
@@ -46,7 +46,7 @@ std::map<std::string, PBD::ID>                RegionFactory::region_name_map;
 RegionFactory::CompoundAssociations           RegionFactory::_compound_associations;
 
 boost::shared_ptr<Region>
-RegionFactory::create (boost::shared_ptr<const Region> region, bool announce, const int32_t sub_num)
+RegionFactory::create (boost::shared_ptr<const Region> region, bool announce)
 {
 	boost::shared_ptr<Region> ret;
 	boost::shared_ptr<const AudioRegion> ar;
@@ -54,7 +54,7 @@ RegionFactory::create (boost::shared_ptr<const Region> region, bool announce, co
 
 	if ((ar = boost::dynamic_pointer_cast<const AudioRegion>(region)) != 0) {
 
-		ret = boost::shared_ptr<Region> (new AudioRegion (ar, 0, sub_num));
+		ret = boost::shared_ptr<Region> (new AudioRegion (ar, MusicFrame (0, 0)));
 
 	} else if ((mr = boost::dynamic_pointer_cast<const MidiRegion>(region)) != 0) {
 
@@ -71,7 +71,7 @@ RegionFactory::create (boost::shared_ptr<const Region> region, bool announce, co
 			source->set_ancestor_name(mr->sources().front()->name());
 			ret = mr->clone(source);
 		} else {
-			ret = boost::shared_ptr<Region> (new MidiRegion (mr, 0, sub_num));
+			ret = boost::shared_ptr<Region> (new MidiRegion (mr, MusicFrame (0, 0)));
 		}
 
 	} else {
@@ -87,8 +87,6 @@ RegionFactory::create (boost::shared_ptr<const Region> region, bool announce, co
 			ret->set_position_lock_style (MusicTime);
 		}
 
-		ret->set_position (region->position(), sub_num);
-
 		/* pure copy constructor - no property list */
 		if (announce) {
 			map_add (ret);
@@ -144,7 +142,7 @@ RegionFactory::create (boost::shared_ptr<Region> region, const PropertyList& pli
 }
 
 boost::shared_ptr<Region>
-RegionFactory::create (boost::shared_ptr<Region> region, frameoffset_t offset, const PropertyList& plist, bool announce, const int32_t sub_num)
+RegionFactory::create (boost::shared_ptr<Region> region, MusicFrame offset, const PropertyList& plist, bool announce)
 {
 	boost::shared_ptr<Region> ret;
 	boost::shared_ptr<const AudioRegion> other_a;
@@ -152,11 +150,11 @@ RegionFactory::create (boost::shared_ptr<Region> region, frameoffset_t offset, c
 
 	if ((other_a = boost::dynamic_pointer_cast<AudioRegion>(region)) != 0) {
 
-		ret = boost::shared_ptr<Region> (new AudioRegion (other_a, offset, sub_num));
+		ret = boost::shared_ptr<Region> (new AudioRegion (other_a, offset));
 
 	} else if ((other_m = boost::dynamic_pointer_cast<MidiRegion>(region)) != 0) {
 
-		ret = boost::shared_ptr<Region> (new MidiRegion (other_m, offset, sub_num));
+		ret = boost::shared_ptr<Region> (new MidiRegion (other_m, offset));
 
 	} else {
 		fatal << _("programming error: RegionFactory::create() called with unknown Region type")
diff --git a/libs/ardour/strip_silence.cc b/libs/ardour/strip_silence.cc
index 3141f422a8..1cbb81a3ed 100644
--- a/libs/ardour/strip_silence.cc
+++ b/libs/ardour/strip_silence.cc
@@ -116,7 +116,7 @@ StripSilence::run (boost::shared_ptr<Region> r, Progress* progress)
 		plist.add (Properties::position, r->position() + (i->first - r->start()));
 
 		copy = boost::dynamic_pointer_cast<AudioRegion> (
-			RegionFactory::create (region, (i->first - r->start()), plist)
+			RegionFactory::create (region, MusicFrame (i->first - r->start(), 0), plist)
 			);
 
 		copy->set_name (RegionFactory::new_region_name (region->name ()));
diff --git a/libs/ardour/tempo.cc b/libs/ardour/tempo.cc
index 469bbac4de..d690896c15 100644
--- a/libs/ardour/tempo.cc
+++ b/libs/ardour/tempo.cc
@@ -3676,13 +3676,13 @@ TempoMap::bbt_duration_at (framepos_t pos, const BBT_Time& bbt, int dir)
 	return 0;
 }
 
-framepos_t
+MusicFrame
 TempoMap::round_to_bar (framepos_t fr, RoundMode dir)
 {
 	return round_to_type (fr, dir, Bar);
 }
 
-framepos_t
+MusicFrame
 TempoMap::round_to_beat (framepos_t fr, RoundMode dir)
 {
 	return round_to_type (fr, dir, Beat);
@@ -3784,7 +3784,7 @@ TempoMap::round_to_beat_subdivision (framepos_t fr, int sub_num, RoundMode dir)
 	return ret_frame;
 }
 
-framepos_t
+MusicFrame
 TempoMap::round_to_quarter_note_subdivision (framepos_t fr, int sub_num, RoundMode dir)
 {
 	Glib::Threads::RWLock::ReaderLock lm (lock);
@@ -3865,7 +3865,7 @@ TempoMap::round_to_quarter_note_subdivision (framepos_t fr, int sub_num, RoundMo
 			if (rem > ticks) {
 				if (beats == 0) {
 					/* can't go backwards past zero, so ... */
-					return 0;
+					return MusicFrame (0, 0);
 				}
 				/* step back to previous beat */
 				--beats;
@@ -3880,35 +3880,46 @@ TempoMap::round_to_quarter_note_subdivision (framepos_t fr, int sub_num, RoundMo
 		}
 	}
 
-	const framepos_t ret_frame = frame_at_minute (minute_at_pulse_locked (_metrics, (beats + (ticks / BBT_Time::ticks_per_beat)) / 4.0));
+	MusicFrame ret (0, 0);
+	ret.frame = frame_at_minute (minute_at_pulse_locked (_metrics, (beats + (ticks / BBT_Time::ticks_per_beat)) / 4.0));
+	ret.division = sub_num;
 
-	return ret_frame;
+	return ret;
 }
 
-framepos_t
+MusicFrame
 TempoMap::round_to_type (framepos_t frame, RoundMode dir, BBTPointType type)
 {
 	Glib::Threads::RWLock::ReaderLock lm (lock);
-
-	const double beat_at_framepos = max (0.0, beat_at_minute_locked (_metrics, minute_at_frame (frame)));
+	const double minute = minute_at_frame (frame);
+	const double beat_at_framepos = max (0.0, beat_at_minute_locked (_metrics, minute));
 	BBT_Time bbt (bbt_at_beat_locked (_metrics, beat_at_framepos));
+	MusicFrame ret (0, 0);
 
 	switch (type) {
 	case Bar:
+		ret.division = -1;
+
 		if (dir < 0) {
 			/* find bar previous to 'frame' */
 			if (bbt.bars > 0)
 				--bbt.bars;
 			bbt.beats = 1;
 			bbt.ticks = 0;
-			return frame_at_minute (minute_at_bbt_locked (_metrics, bbt));
+
+			ret.frame = frame_at_minute (minute_at_bbt_locked (_metrics, bbt));
+
+			return ret;
 
 		} else if (dir > 0) {
 			/* find bar following 'frame' */
 			++bbt.bars;
 			bbt.beats = 1;
 			bbt.ticks = 0;
-			return frame_at_minute (minute_at_bbt_locked (_metrics, bbt));
+
+			ret.frame = frame_at_minute (minute_at_bbt_locked (_metrics, bbt));
+
+			return ret;
 		} else {
 			/* true rounding: find nearest bar */
 			framepos_t raw_ft = frame_at_minute (minute_at_bbt_locked (_metrics, bbt));
@@ -3919,26 +3930,39 @@ TempoMap::round_to_type (framepos_t frame, RoundMode dir, BBTPointType type)
 			framepos_t next_ft = frame_at_minute (minute_at_bbt_locked (_metrics, bbt));
 
 			if ((raw_ft - prev_ft) > (next_ft - prev_ft) / 2) {
-				return next_ft;
+				ret.frame = next_ft;
+
+				return ret;
 			} else {
-				return prev_ft;
+				--bbt.bars;
+				ret.frame = prev_ft;
+
+				return ret;
 			}
 		}
 
 		break;
 
 	case Beat:
+		ret.division = 1;
+
 		if (dir < 0) {
-			return frame_at_minute (minute_at_beat_locked (_metrics, floor (beat_at_framepos)));
+			ret.frame = frame_at_minute (minute_at_beat_locked (_metrics, floor (beat_at_framepos)));
+
+			return ret;
 		} else if (dir > 0) {
-			return frame_at_minute (minute_at_beat_locked (_metrics, ceil (beat_at_framepos)));
+			ret.frame = frame_at_minute (minute_at_beat_locked (_metrics, ceil (beat_at_framepos)));
+
+			return ret;
 		} else {
-			return frame_at_minute (minute_at_beat_locked (_metrics, floor (beat_at_framepos + 0.5)));
+			ret.frame = frame_at_minute (minute_at_beat_locked (_metrics, floor (beat_at_framepos + 0.5)));
+
+			return ret;
 		}
 		break;
 	}
 
-	return 0;
+	return MusicFrame (0, 0);
 }
 
 void