Prevent multiple tempo / meter changes being inserted at the same point

on the timeline; adding a new change at the same time as an existing one
will replace the existing one.  Should prevent #769 from happening.

Some cleanups and tweaks to tempo / meter dialogues.  Desensitize Remove
menu option for those changes that can't be removed.



git-svn-id: svn://localhost/ardour2/branches/3.0@7045 d708f5d6-7413-0410-9779-e7cbd77b26cf

9 files changed, 159 insertions, 71 deletions

diff --git a/gtk2_ardour/editor.cc b/gtk2_ardour/editor.cc
index 754b10f4da..98480743e0 100644
--- a/gtk2_ardour/editor.cc
+++ b/gtk2_ardour/editor.cc
@@ -346,7 +346,7 @@ Editor::Editor ()
 	session_range_marker_menu = 0;
 	range_marker_menu = 0;
 	marker_menu_item = 0;
-	tm_marker_menu = 0;
+	tempo_or_meter_marker_menu = 0;
 	transport_marker_menu = 0;
 	new_transport_marker_menu = 0;
 	editor_mixer_strip_width = Wide;
diff --git a/gtk2_ardour/editor.h b/gtk2_ardour/editor.h
index ca6ff920c8..7040bafa8a 100644
--- a/gtk2_ardour/editor.h
+++ b/gtk2_ardour/editor.h
@@ -1454,15 +1454,16 @@ public:
 	void update_punch_range_view (bool visibility=false);
         void new_transport_marker_menu_popdown ();
 	void marker_context_menu (GdkEventButton*, ArdourCanvas::Item*);
-	void tm_marker_context_menu (GdkEventButton*, ArdourCanvas::Item*);
+	void tempo_or_meter_marker_context_menu (GdkEventButton*, ArdourCanvas::Item*);
 	void transport_marker_context_menu (GdkEventButton*, ArdourCanvas::Item*);
 	void new_transport_marker_context_menu (GdkEventButton*, ArdourCanvas::Item*);
 	void build_range_marker_menu (bool loop_or_punch);
 	void build_marker_menu (bool);
-	void build_tm_marker_menu ();
+	void build_tempo_or_meter_marker_menu (bool);
 	void build_new_transport_marker_menu ();
+	void dynamic_cast_marker_object (void*, MeterMarker**, TempoMarker**) const;
 
-	Gtk::Menu* tm_marker_menu;
+	Gtk::Menu* tempo_or_meter_marker_menu;
 	Gtk::Menu* marker_menu;
 	Gtk::Menu* session_range_marker_menu;
 	Gtk::Menu* range_marker_menu;
diff --git a/gtk2_ardour/editor_markers.cc b/gtk2_ardour/editor_markers.cc
index 81ad022ce0..dad515671e 100644
--- a/gtk2_ardour/editor_markers.cc
+++ b/gtk2_ardour/editor_markers.cc
@@ -490,15 +490,25 @@ Editor::location_gone (Location *location)
 }
 
 void
-Editor::tm_marker_context_menu (GdkEventButton* ev, ArdourCanvas::Item* item)
+Editor::tempo_or_meter_marker_context_menu (GdkEventButton* ev, ArdourCanvas::Item* item)
 {
-	if (tm_marker_menu == 0) {
-		build_tm_marker_menu ();
-	}
-
 	marker_menu_item = item;
-	tm_marker_menu->popup (1, ev->time);
+	
+	MeterMarker* mm;
+	TempoMarker* tm;
+	dynamic_cast_marker_object (marker_menu_item->get_data ("marker"), &mm, &tm);
 
+	bool can_remove = false;
+
+	if (mm) {
+		can_remove = mm->meter().movable ();
+	} else if (tm) {
+		can_remove = tm->tempo().movable ();
+	} 
+	
+	delete tempo_or_meter_marker_menu;
+	build_tempo_or_meter_marker_menu (can_remove);
+	tempo_or_meter_marker_menu->popup (1, ev->time);
 }
 
 void
@@ -662,16 +672,18 @@ Editor::build_range_marker_menu (bool loop_or_punch)
 }
 
 void
-Editor::build_tm_marker_menu ()
+Editor::build_tempo_or_meter_marker_menu (bool can_remove)
 {
 	using namespace Menu_Helpers;
 
-	tm_marker_menu = new Menu;
-	MenuList& items = tm_marker_menu->items();
-	tm_marker_menu->set_name ("ArdourContextMenu");
+	tempo_or_meter_marker_menu = new Menu;
+	MenuList& items = tempo_or_meter_marker_menu->items();
+	tempo_or_meter_marker_menu->set_name ("ArdourContextMenu");
 
 	items.push_back (MenuElem (_("Edit"), sigc::mem_fun(*this, &Editor::marker_menu_edit)));
 	items.push_back (MenuElem (_("Remove"), sigc::mem_fun(*this, &Editor::marker_menu_remove)));
+
+	items.back().set_sensitive (can_remove);
 }
 
 void
@@ -972,43 +984,48 @@ Editor::marker_menu_loop_range ()
 }
 
 void
-Editor::marker_menu_edit ()
+Editor::dynamic_cast_marker_object (void* p, MeterMarker** m, TempoMarker** t) const
 {
-	MeterMarker* mm;
-	TempoMarker* tm;
-	Marker* marker;
-
-	if ((marker = reinterpret_cast<Marker *> (marker_menu_item->get_data ("marker"))) == 0) {
+	Marker* marker = reinterpret_cast<Marker*> (p);
+	if (!marker) {
 		fatal << _("programming error: marker canvas item has no marker object pointer!") << endmsg;
 		/*NOTREACHED*/
 	}
 
-	if ((mm = dynamic_cast<MeterMarker*> (marker)) != 0) {
-		edit_meter_section (&mm->meter());
-	} else if ((tm = dynamic_cast<TempoMarker*> (marker)) != 0) {
-		edit_tempo_section (&tm->tempo());
-	} else {
-		fatal << X_("programming erorr: unhandled marker type in Editor::marker_menu_edit")
+	*m = dynamic_cast<MeterMarker*> (marker);
+	*t = dynamic_cast<TempoMarker*> (marker);
+
+	if (*m == 0 && *t == 0) {
+		fatal << X_("programming erorr: unhandled marker type in Editor::dynamic_cast_marker_object")
 		      << endmsg;
 		/*NOTREACHED*/
 	}
 }
 
 void
-Editor::marker_menu_remove ()
+Editor::marker_menu_edit ()
 {
 	MeterMarker* mm;
 	TempoMarker* tm;
-	Marker* marker;
+	dynamic_cast_marker_object (marker_menu_item->get_data ("marker"), &mm, &tm);
 
-	if ((marker = reinterpret_cast<Marker *> (marker_menu_item->get_data ("marker"))) == 0) {
-		fatal << _("programming error: marker canvas item has no marker object pointer!") << endmsg;
-		/*NOTREACHED*/
+	if (mm) {
+		edit_meter_section (&mm->meter());
+	} else if (tm) {
+		edit_tempo_section (&tm->tempo());
 	}
+}
+
+void
+Editor::marker_menu_remove ()
+{
+	MeterMarker* mm;
+	TempoMarker* tm;
+	dynamic_cast_marker_object (marker_menu_item->get_data ("marker"), &mm, &tm);
 
-	if ((mm = dynamic_cast<MeterMarker*> (marker)) != 0) {
+	if (mm) {
 		remove_meter_marker (marker_menu_item);
-	} else if ((tm = dynamic_cast<TempoMarker*> (marker)) != 0) {
+	} else if (tm) {
 		remove_tempo_marker (marker_menu_item);
 	} else {
 		remove_marker (*marker_menu_item, (GdkEvent*) 0);
diff --git a/gtk2_ardour/editor_mouse.cc b/gtk2_ardour/editor_mouse.cc
index 197121eb06..bd55c04b5e 100644
--- a/gtk2_ardour/editor_mouse.cc
+++ b/gtk2_ardour/editor_mouse.cc
@@ -1152,11 +1152,11 @@ Editor::button_release_handler (ArdourCanvas::Item* item, GdkEvent* event, ItemT
 				break;
 
 			case TempoMarkerItem:
-				tm_marker_context_menu (&event->button, item);
+				tempo_or_meter_marker_context_menu (&event->button, item);
 				break;
 
 			case MeterMarkerItem:
-				tm_marker_context_menu (&event->button, item);
+				tempo_or_meter_marker_context_menu (&event->button, item);
 				break;
 
 			case CrossfadeViewItem:
diff --git a/gtk2_ardour/editor_tempodisplay.cc b/gtk2_ardour/editor_tempodisplay.cc
index fd598b8c81..edfaee3694 100644
--- a/gtk2_ardour/editor_tempodisplay.cc
+++ b/gtk2_ardour/editor_tempodisplay.cc
@@ -296,7 +296,7 @@ Editor::remove_tempo_marker (ArdourCanvas::Item* item)
 	}
 
 	if (tempo_marker->tempo().movable()) {
-	  Glib::signal_idle().connect (sigc::bind (sigc::mem_fun(*this, &Editor::real_remove_tempo_marker), &tempo_marker->tempo()));
+		Glib::signal_idle().connect (sigc::bind (sigc::mem_fun(*this, &Editor::real_remove_tempo_marker), &tempo_marker->tempo()));
 	}
 }
 
diff --git a/gtk2_ardour/tempo_dialog.cc b/gtk2_ardour/tempo_dialog.cc
index 93cb5fcf01..71fd5b40b4 100644
--- a/gtk2_ardour/tempo_dialog.cc
+++ b/gtk2_ardour/tempo_dialog.cc
@@ -34,8 +34,8 @@ using namespace ARDOUR;
 using namespace PBD;
 
 TempoDialog::TempoDialog (TempoMap& map, nframes_t frame, const string & action)
-	: ArdourDialog (_("edit tempo")),
-	  bpm_adjustment (60.0, 1.0, 999.9, 0.1, 1.0, 1.0),
+	: ArdourDialog (_("New Tempo")),
+	  bpm_adjustment (60.0, 1.0, 999.9, 0.1, 1.0),
 	  bpm_spinner (bpm_adjustment),
 	  ok_button (action),
 	  cancel_button (_("Cancel")),
@@ -50,8 +50,8 @@ TempoDialog::TempoDialog (TempoMap& map, nframes_t frame, const string & action)
 }
 
 TempoDialog::TempoDialog (TempoSection& section, const string & action)
-	: ArdourDialog ("tempo dialog"),
-	  bpm_adjustment (60.0, 1.0, 999.9, 0.1, 1.0, 1.0),
+	: ArdourDialog ("Edit Tempo"),
+	  bpm_adjustment (60.0, 1.0, 999.9, 0.1, 1.0),
 	  bpm_spinner (bpm_adjustment),
 	  ok_button (action),
 	  cancel_button (_("Cancel")),
@@ -142,6 +142,7 @@ TempoDialog::init (const BBT_Time& when, double bpm, double note_type, bool mova
 	bpm_spinner.signal_activate().connect (sigc::bind (sigc::mem_fun (*this, &TempoDialog::response), RESPONSE_ACCEPT));
 	bpm_spinner.signal_button_press_event().connect (sigc::mem_fun (*this, &TempoDialog::bpm_button_press), false);
 	bpm_spinner.signal_button_release_event().connect (sigc::mem_fun (*this, &TempoDialog::bpm_button_release), false);
+	bpm_spinner.signal_changed().connect (sigc::mem_fun (*this, &TempoDialog::bpm_changed));
 	when_bar_entry.signal_activate().connect (sigc::bind (sigc::mem_fun (*this, &TempoDialog::response), RESPONSE_ACCEPT));
 	when_bar_entry.signal_key_release_event().connect (sigc::mem_fun (*this, &TempoDialog::entry_key_release), false);
 	when_beat_entry.signal_activate().connect (sigc::bind (sigc::mem_fun (*this, &TempoDialog::response), RESPONSE_ACCEPT));
@@ -149,6 +150,12 @@ TempoDialog::init (const BBT_Time& when, double bpm, double note_type, bool mova
 	note_types.signal_changed().connect (sigc::mem_fun (*this, &TempoDialog::note_types_change));
 }
 
+void
+TempoDialog::bpm_changed ()
+{
+	set_response_sensitive (RESPONSE_ACCEPT, true);
+}
+
 bool
 TempoDialog::bpm_button_press (GdkEventButton*)
 {
@@ -233,7 +240,7 @@ TempoDialog::note_types_change ()
 
 
 MeterDialog::MeterDialog (TempoMap& map, nframes_t frame, const string & action)
-	: ArdourDialog ("meter dialog"),
+	: ArdourDialog ("New Meter"),
 	  ok_button (action),
 	  cancel_button (_("Cancel"))
 {
@@ -246,7 +253,7 @@ MeterDialog::MeterDialog (TempoMap& map, nframes_t frame, const string & action)
 }
 
 MeterDialog::MeterDialog (MeterSection& section, const string & action)
-	: ArdourDialog ("meter dialog"),
+	: ArdourDialog ("Edit Meter"),
 	  ok_button (action),
 	  cancel_button (_("Cancel"))
 {
diff --git a/gtk2_ardour/tempo_dialog.h b/gtk2_ardour/tempo_dialog.h
index c75f60ac23..c1a88367ca 100644
--- a/gtk2_ardour/tempo_dialog.h
+++ b/gtk2_ardour/tempo_dialog.h
@@ -35,20 +35,9 @@
 
 #include "ardour_dialog.h"
 
-struct TempoDialog : public ArdourDialog
+class TempoDialog : public ArdourDialog
 {
-	Gtk::ComboBoxText note_types;
-	std::vector<std::string> strings;
-	Gtk::Adjustment   bpm_adjustment;
-	Gtk::SpinButton   bpm_spinner;
-	Gtk::Button  ok_button;
-	Gtk::Button  cancel_button;
-	Gtk::Entry   when_bar_entry;
-	Gtk::Entry   when_beat_entry;
-	Gtk::Label   when_bar_label;
-	Gtk::Label   when_beat_label;
-	char buf[64];
-
+public:
 	TempoDialog (ARDOUR::TempoMap&, nframes_t, const std::string & action);
 	TempoDialog (ARDOUR::TempoSection&, const std::string & action);
 
@@ -63,18 +52,24 @@ private:
 	bool bpm_button_release (GdkEventButton* );
 	bool entry_key_release (GdkEventKey* );
 	void note_types_change ();
-};
 
-struct MeterDialog : public ArdourDialog
-{
-	Gtk::Entry   bpb_entry;
 	Gtk::ComboBoxText note_types;
 	std::vector<std::string> strings;
+	Gtk::Adjustment   bpm_adjustment;
+	Gtk::SpinButton   bpm_spinner;
 	Gtk::Button  ok_button;
 	Gtk::Button  cancel_button;
 	Gtk::Entry   when_bar_entry;
+	Gtk::Entry   when_beat_entry;
+	Gtk::Label   when_bar_label;
+	Gtk::Label   when_beat_label;
 	char buf[64];
+};
 
+class MeterDialog : public ArdourDialog
+{
+public:
+	
 	MeterDialog (ARDOUR::TempoMap&, nframes_t, const std::string & action);
 	MeterDialog (ARDOUR::MeterSection&, const std::string & action);
 
@@ -87,6 +82,14 @@ private:
 	bool entry_key_press (GdkEventKey* );
 	bool entry_key_release (GdkEventKey* );
 	void note_types_change ();
+
+	Gtk::Entry   bpb_entry;
+	Gtk::ComboBoxText note_types;
+	std::vector<std::string> strings;
+	Gtk::Button  ok_button;
+	Gtk::Button  cancel_button;
+	Gtk::Entry   when_bar_entry;
+	char buf[64];
 };
 
 #endif /* __ardour_gtk_tempo_dialog_h__ */
diff --git a/libs/ardour/ardour/tempo.h b/libs/ardour/ardour/tempo.h
index 596444173f..21a1185725 100644
--- a/libs/ardour/ardour/tempo.h
+++ b/libs/ardour/ardour/tempo.h
@@ -86,7 +86,7 @@ class MetricSection {
 	virtual ~MetricSection() {}
 
 	const BBT_Time& start() const { return _start; }
-	nframes64_t       frame() const { return _frame; }
+	nframes64_t     frame() const { return _frame; }
 
 	void set_movable (bool yn) { _movable = yn; }
 	bool movable() const { return _movable; }
@@ -105,9 +105,11 @@ class MetricSection {
 	*/
 	virtual XMLNode& get_state() const = 0;
 
+	int compare (MetricSection *, bool) const;
+
   private:
 	BBT_Time       _start;
-	nframes64_t      _frame;
+	nframes64_t    _frame;
 	bool           _movable;
 };
 
diff --git a/libs/ardour/tempo.cc b/libs/ardour/tempo.cc
index 127c0a10e4..f2a5c5be40 100644
--- a/libs/ardour/tempo.cc
+++ b/libs/ardour/tempo.cc
@@ -370,16 +370,46 @@ TempoMap::do_insert (MetricSection* section, bool with_bbt)
 {
 	Metrics::iterator i;
 
+	/* Look for any existing MetricSection that is of the same type and
+	   at the same time as the new one, and remove it before adding
+	   the new one.
+	*/
+
+	Metrics::iterator to_remove = metrics->end ();
+
 	for (i = metrics->begin(); i != metrics->end(); ++i) {
 
-		if (with_bbt) {
-			if ((*i)->start() < section->start()) {
-				continue;
-			}
-		} else {
-			if ((*i)->frame() < section->frame()) {
-				continue;
-			}
+		int const c = (*i)->compare (section, with_bbt);
+
+		if (c < 0) {
+			/* this section is before the one to be added; go back round */
+			continue;
+		} else if (c > 0) {
+			/* this section is after the one to be added; there can't be any at the same time */
+			break;
+		}
+
+		/* hacky comparison of type */
+		bool const a = dynamic_cast<TempoSection*> (*i) != 0;
+		bool const b = dynamic_cast<TempoSection*> (section) != 0;
+
+		if (a == b) {
+			to_remove = i;
+			break;
+		}
+	}
+
+	if (to_remove != metrics->end()) {
+		/* remove the MetricSection at the same time as the one we are about to add */
+		metrics->erase (to_remove);
+	}
+
+	/* Add the given MetricSection */
+
+	for (i = metrics->begin(); i != metrics->end(); ++i) {
+
+		if ((*i)->compare (section, with_bbt) < 0) {
+			continue;
 		}
 
 		metrics->insert (i, section);
@@ -400,7 +430,6 @@ TempoMap::add_tempo (const Tempo& tempo, BBT_Time where)
 		Glib::RWLock::WriterLock lm (lock);
 
 		/* new tempos always start on a beat */
-
 		where.ticks = 0;
 
 		do_insert (new TempoSection (where, tempo.beats_per_minute(), tempo.note_type()), true);
@@ -468,7 +497,6 @@ TempoMap::add_meter (const Meter& meter, BBT_Time where)
 		}
 
 		/* new meters *always* start on a beat. */
-
 		where.ticks = 0;
 
 		do_insert (new MeterSection (where, meter.beats_per_bar(), meter.note_divisor()), true);
@@ -1892,3 +1920,33 @@ TempoMap::bbt_subtract (const BBT_Time& start, const BBT_Time& decrement) const
 	result.bars -= op.bars;
 	return result;
 }
+
+/** Compare the time of this with that of another MetricSection.
+ *  @param with_bbt True to compare using ::start(), false to use ::frame().
+ *  @return -1 for less than, 0 for equal, 1 for greater than.
+ */
+
+int
+MetricSection::compare (MetricSection* other, bool with_bbt) const
+{
+	if (with_bbt) {
+		if (start() == other->start()) {
+			return 0;
+		} else if (start() < other->start()) {
+			return -1;
+		} else {
+			return 1;
+		}
+	} else {
+		if (frame() == other->frame()) {
+			return 0;
+		} else if (frame() < other->frame()) {
+			return -1;
+		} else {
+			return 1;
+		}
+	}
+
+	/* NOTREACHED */
+	return 0;
+}