refactoring of various clip GUI code

This better defines when a region/trigger/track is being set and
signals connected/disconnected to/from, along with some more
code sharing between the MIDI (Pianoroll) and audio (AudioClipEditor)
versions of things

gtk2_ardour/audio_clip_editor.cc

@@ -52,8 +52,9 @@
 #include "editor.h"
 #include "keyboard.h"
 #include "region_view.h"
-#include "verbose_cursor.h"
+#include "timers.h"
 #include "ui_config.h"
+#include "verbose_cursor.h"
 
 #include "pbd/i18n.h"
 
@@ -468,24 +469,6 @@ AudioClipEditor::drop_waves ()
 	waves.clear ();
 }
 
-void
-AudioClipEditor::set_trigger (TriggerReference& tr)
-{
-	EC_LOCAL_TEMPO_SCOPE;
-
-//	if (tr == ref) {
-//		return;
-//	}
-
-	CueEditor::set_trigger (tr);
-	rec_box.show ();
-
-	main_ruler->show ();
-	main_ruler->set_range (0, pixel_to_sample (_visible_canvas_width - 2.));
-
-	set_region (tr.trigger()->the_region());
-}
-
 void
 AudioClipEditor::set_region (std::shared_ptr<Region> region)
 {
@@ -791,16 +774,20 @@ AudioClipEditor::maybe_update ()
 
 		if (!playing_trigger) {
 
-			if (_drags->active() || !_region || !_track || !_track->triggerbox()) {
+			if (_drags->active() || !_track || !_track->triggerbox()) {
 				return;
 			}
 
 			if (_track->triggerbox()->record_enabled() == Recording) {
-
 				_playhead_cursor->set_position (data_capture_duration);
 			}
 
+			if (!_region) {
+				return;
+			}
+
 		} else {
+
 			if (playing_trigger->active ()) {
 				if (playing_trigger->the_region()) {
 					_playhead_cursor->set_position (playing_trigger->current_pos().samples() + playing_trigger->the_region()->start().samples());
@@ -809,6 +796,7 @@ AudioClipEditor::maybe_update ()
 				_playhead_cursor->set_position (0);
 			}
 		}
+
 #if 0
 	} else if (view->midi_region()) {
 
@@ -836,12 +824,20 @@ AudioClipEditor::maybe_update ()
 }
 
 void
-AudioClipEditor::unset (bool trigger_too)
+AudioClipEditor::unset_region ()
 {
 	EC_LOCAL_TEMPO_SCOPE;
 
 	drop_waves ();
-	CueEditor::unset (trigger_too);
+
+	CueEditor::unset_region ();
+}
+
+void
+AudioClipEditor::unset_trigger ()
+{
+	EC_LOCAL_TEMPO_SCOPE;
+	CueEditor::unset_trigger ();
 }
 
 Gdk::Cursor*
@@ -897,4 +893,3 @@ AudioClipEditor::grid_type_chosen (Editing::GridType gt)
 		grid_actions[Editing::GridTypeMinSec]->set_active (true);
 	}
 }
-

gtk2_ardour/audio_clip_editor.h

@@ -78,7 +78,6 @@ public:
 
 	Gtk::Widget& contents ();
 
-	void set_trigger (ARDOUR::TriggerReference&);
 	void set_region (std::shared_ptr<ARDOUR::Region> r);
 	void region_changed (const PBD::PropertyChange& what_changed);
 
@@ -131,7 +130,11 @@ public:
 
 	void maybe_update ();
 
-	bool idle_data_captured () { return false; }
+	bool idle_data_captured ();
+
+	void set_overlay_text (std::string const &);
+	void hide_overlay_text ();
+	void show_overlay_text ();
 
  private:
 	ArdourCanvas::Container*         line_container;
@@ -191,7 +194,9 @@ public:
 	void show_count_in (std::string const &);
 	void hide_count_in ();
 
-	void unset (bool trigger_too);
+	void unset_region ();
+	void unset_trigger ();
+
 	void load_shared_bindings ();
 
 	void compute_fixed_ruler_scale ();

gtk2_ardour/cue_editor.cc

@@ -664,8 +664,6 @@ CueEditor::rec_enable_change ()
 	rec_blink_connection.disconnect ();
 	count_in_connection.disconnect ();
 
-	std::cerr << "REC, state " << ref.box()->record_enabled() << std::endl;
-
 	switch (ref.box()->record_enabled()) {
 	case Recording:
 		rec_enable_button.set_active_state (Gtkmm2ext::ExplicitActive);
@@ -1214,33 +1212,125 @@ CueEditor::set_track (std::shared_ptr<Track> t)
 {
 	EC_LOCAL_TEMPO_SCOPE;
 
+	track_connections.drop_connections ();
+
 	_track = t;
-	_track->solo_control()->Changed.connect (object_connections, invalidator (*this), std::bind (&CueEditor::update_solo_display, this), gui_context());
+	_track->solo_control()->Changed.connect (track_connections, invalidator (*this), std::bind (&CueEditor::update_solo_display, this), gui_context());
 	solo_button.set_sensitive (true);
 	update_solo_display ();
 }
 
+void
+CueEditor::set_trigger (TriggerReference& tref)
+{
+	EC_LOCAL_TEMPO_SCOPE;
+
+	unset_trigger ();
+
+	ref = tref;
+
+	TriggerPtr trigger (ref.trigger());
+
+	if (!trigger) {
+		return;
+	}
+
+	rec_box.show ();
+	rec_enable_button.set_sensitive (true);
+
+	idle_update_queued.store (0);
+
+	ref.box()->Captured.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::data_captured, this, _1), gui_context());
+	ref.box()->RecEnableChanged.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::rec_enable_change, this), gui_context());
+	maybe_set_count_in ();
+
+	Stripable* st = dynamic_cast<Stripable*> (ref.box()->owner());
+	assert (st);
+
+	set_track (std::dynamic_pointer_cast<Track> (st->shared_from_this()));
+
+	if (_track) {
+		_track->DropReferences.connect (track_connections, invalidator (*this), std::bind (&CueEditor::unset_trigger, this), gui_context());
+	}
+
+	trigger->PropertyChanged.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::trigger_prop_change, this, _1), gui_context());
+	trigger->ArmChanged.connect (trigger_connections, invalidator (*this), std::bind (&CueEditor::trigger_arm_change, this), gui_context());
+
+	if (trigger) {
+		set_region (trigger->the_region());
+	} else {
+		set_region (nullptr);
+	}
+
+	_update_connection = Timers::super_rapid_connect (sigc::mem_fun (*this, &CueEditor::maybe_update));
+}
+
 void
 CueEditor::set_region (std::shared_ptr<Region> r)
 {
-	unset (false);
+	unset_region ();
 
 	_region = r;
 
-	if (_region) {
+	if (!_region) {
-		std::shared_ptr<TempoMap> tmap = _region->tempo_map();
-		if (tmap) {
-			start_local_tempo_map (tmap);
-		}
-
-		if (!get_canvas()->is_visible()) {
-			_visible_pending_region = r;
-		} else {
-			_visible_pending_region.reset ();
-		}
-	} else {
 		_visible_pending_region.reset ();
+		return;
 	}
+
+
+	std::shared_ptr<TempoMap> tmap = _region->tempo_map();
+
+	if (tmap) {
+		start_local_tempo_map (tmap);
+	}
+
+	if (!get_canvas()->is_visible()) {
+		/* We can't really handle a region until we have a size, so
+		   defer the rest of this until we do.
+
+		   XXX visibility is not a very good proxy for size (though
+		   it's not terrible either.
+		*/
+		_visible_pending_region = r;
+		return;
+	}
+
+	_visible_pending_region.reset ();
+
+	r->DropReferences.connect (region_connections, invalidator (*this), std::bind (&CueEditor::unset_region, this), gui_context());
+	r->PropertyChanged.connect (region_connections, invalidator (*this), std::bind (&CueEditor::region_prop_change, this, _1), gui_context());
+}
+
+void
+CueEditor::unset_region ()
+{
+	if (_local_tempo_map) {
+		end_local_tempo_map ();
+	}
+
+	_history.clear ();
+	_update_connection.disconnect();
+	region_connections.drop_connections ();
+	rec_blink_connection.disconnect ();
+	capture_connections.drop_connections ();
+	_region.reset ();
+}
+
+void
+CueEditor::unset_trigger ()
+{
+	ref = TriggerReference ();
+	solo_button.set_active_state (Gtkmm2ext::Off);
+	solo_button.set_sensitive (false);
+	trigger_connections.drop_connections ();
+	track_connections.drop_connections ();
+	_track.reset ();
+
+	/* Since set_trigger() calls set_region(), we need the symmetry here
+	 * that calling unset_trigger() also calls unset_region().
+	 */
+
+	unset_region ();
 }
 
 void
@@ -1284,29 +1374,6 @@ CueEditor::set_from_rsu (RegionUISettings& rsu)
 	}
 }
 
-void
-CueEditor::set_trigger (TriggerReference& tref)
-{
-	EC_LOCAL_TEMPO_SCOPE;
-
-	if (tref == ref) {
-		return;
-	}
-
-	_update_connection.disconnect ();
-	object_connections.drop_connections ();
-
-	ref = tref;
-
-	Stripable* st = dynamic_cast<Stripable*> (ref.box()->owner());
-	assert (st);
-
-	set_track (std::dynamic_pointer_cast<Track> (st->shared_from_this()));
-	set_region (ref.trigger()->the_region());
-
-	_update_connection = Timers::rapid_connect (sigc::mem_fun (*this, &CueEditor::maybe_update));
-}
-
 void
 CueEditor::ruler_locate (GdkEventButton* ev)
 {
@@ -1336,7 +1403,6 @@ CueEditor::maybe_set_count_in ()
 	EC_LOCAL_TEMPO_SCOPE;
 
 	if (!ref.box()) {
-		std::cerr << "msci no box\n";
 		return;
 	}
 
@@ -1351,7 +1417,6 @@ CueEditor::maybe_set_count_in ()
 	count_in_to = ref.box()->start_time (valid);
 
 	if (!valid) {
-		std::cerr << "no start time\n";
 		return;
 	}
 
@@ -1359,12 +1424,10 @@ CueEditor::maybe_set_count_in ()
 	Temporal::Beats const & a_q (tmap->quarters_at_sample (audible));
 
 	if ((count_in_to - a_q).get_beats() == 0) {
-		std::cerr << "not enough time\n";
 		return;
 	}
 
 	count_in_connection = ARDOUR_UI::Clock.connect (sigc::bind (sigc::mem_fun (*this, &CueEditor::count_in),  ARDOUR_UI::clock_signal_interval()));
-	std::cerr << "count in started" << std::endl;
 }
 
 void
@@ -1470,37 +1533,12 @@ CueEditor::idle_data_captured ()
 	return false;
 }
 
-void
-CueEditor::unset (bool trigger_too)
-{
-	if (_local_tempo_map) {
-		end_local_tempo_map ();
-	}
-	_history.clear ();
-	_update_connection.disconnect();
-	object_connections.drop_connections ();
-	rec_blink_connection.disconnect ();
-	count_in_connection.disconnect ();
-	capture_connections.drop_connections ();
-
-	_region.reset ();
-
-	if (trigger_too) {
-		ref = TriggerReference ();
-		solo_button.set_active_state (Gtkmm2ext::Off);
-		solo_button.set_sensitive (false);
-		_track.reset ();
-	} else if (_track) {
-		/* re-subscribe to object_connections */
-		set_track (_track);
-	}
-}
-
 void
 CueEditor::session_going_away ()
 {
 	EditingContext::session_going_away ();
-	unset (true);
+	unset_region ();
+	unset_trigger ();
 }
 
 void

gtk2_ardour/cue_editor.h

@@ -214,12 +214,14 @@ class CueEditor : public EditingContext, public PBD::HistoryOwner
 	void set_recording_length (Temporal::BBT_Offset bars);
 
 	bool rec_button_press (GdkEventButton*);
-	void rec_enable_change ();
+	virtual void rec_enable_change ();
 	void blink_rec_enable (bool);
 	sigc::connection rec_blink_connection;
 
 	sigc::connection _update_connection;
-	PBD::ScopedConnectionList object_connections;
+	PBD::ScopedConnectionList region_connections;
+	PBD::ScopedConnectionList trigger_connections;
+	PBD::ScopedConnectionList track_connections;
 
 	void trigger_arm_change ();
 
@@ -257,7 +259,8 @@ class CueEditor : public EditingContext, public PBD::HistoryOwner
 	PBD::ScopedConnectionList capture_connections;
 	samplecnt_t data_capture_duration;
 
-	virtual void unset (bool trigger_too);
+	virtual void unset_region ();
+	virtual void unset_trigger ();
 
 	RegionUISettings region_ui_settings;
 	void maybe_set_from_rsu ();
@@ -268,4 +271,7 @@ class CueEditor : public EditingContext, public PBD::HistoryOwner
 	bool _scroll_drag;
 	bool hscroll_press (GdkEventButton*);
 	bool hscroll_release (GdkEventButton*);
+
+	virtual void region_prop_change (PBD::PropertyChange const & what_changed) {}
+	virtual void trigger_prop_change (PBD::PropertyChange const & what_changed) {}
 };

gtk2_ardour/pianoroll.cc

@@ -1350,54 +1350,6 @@ Pianoroll::trigger_prop_change (PBD::PropertyChange const & what_changed)
 	}
 }
 
-void
-Pianoroll::region_prop_change (PBD::PropertyChange const & what_changed)
-{
-	EC_LOCAL_TEMPO_SCOPE;
-
-	if (what_changed.contains (Properties::length)) {
-		/* XXX what, if anything, should we do here ? */
-	}
-}
-
-void
-Pianoroll::set_trigger (TriggerReference & tref)
-{
-	EC_LOCAL_TEMPO_SCOPE;
-
-	if (ref == tref) {
-		return;
-	}
-
-	TriggerPtr trigger (tref.trigger());
-
-	CueEditor::set_trigger (tref);
-
-	rec_box.show ();
-	rec_enable_button.set_sensitive (true);
-
-	idle_update_queued.store (0);
-
-	ref.box()->Captured.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::data_captured, this, _1), gui_context());
-	ref.box()->RecEnableChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::rec_enable_change, this), gui_context());
-	maybe_set_count_in ();
-
-	if (_track) {
-		_track->DropReferences.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::unset, this, true), gui_context());
-	}
-
-	trigger->PropertyChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::trigger_prop_change, this, _1), gui_context());
-	trigger->ArmChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::trigger_arm_change, this), gui_context());
-	_update_connection.disconnect ();
-	_update_connection = Timers::super_rapid_connect (sigc::mem_fun (*this, &CueEditor::maybe_update));
-
-	if (trigger) {
-		set_region (trigger->the_region());
-	} else {
-		set_region (nullptr);
-	}
-}
-
 void
 Pianoroll::make_a_region ()
 {
@@ -1426,12 +1378,18 @@ Pianoroll::make_a_region ()
 }
 
 void
-Pianoroll::unset (bool trigger_too)
+Pianoroll::unset_region ()
 {
-	CueEditor::unset (trigger_too);
+	CueEditor::unset_region ();
 	view->set_region (nullptr);
 }
 
+void
+Pianoroll::unset_trigger ()
+{
+	CueEditor::unset_trigger ();
+}
+
 void
 Pianoroll::build_cc_menu (ArdourWidgets::MetaButton* ccbtn)
 {
@@ -1490,7 +1448,7 @@ Pianoroll::set_region (std::shared_ptr<ARDOUR::Region> region)
 
 	std::shared_ptr<MidiRegion> r (std::dynamic_pointer_cast<ARDOUR::MidiRegion> (region));
 
-	if (!r || !region) {
+	if (!r) {
 		_update_connection.disconnect ();
 		return;
 	}
@@ -1501,18 +1459,12 @@ Pianoroll::set_region (std::shared_ptr<ARDOUR::Region> region)
 
 	set_visible_channel (view->pick_visible_channel());
 
-	r->DropReferences.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::unset, this, false), gui_context());
-	r->PropertyChanged.connect (object_connections, invalidator (*this), std::bind (&Pianoroll::region_prop_change, this, _1), gui_context());
-
 	/* Compute zoom level to show entire source plus some margin if possible */
 
 	zoom_to_show (timecnt_t (timepos_t (max_extents_scale() * max_zoom_extent ().second.samples())));
 
 	bg->display_region (*view);
 
-	_update_connection.disconnect ();
-	_update_connection = Timers::rapid_connect (sigc::mem_fun (*this, &Pianoroll::maybe_update));
-
 	maybe_set_from_rsu ();
 }
 

gtk2_ardour/pianoroll.h

@@ -78,7 +78,6 @@ class Pianoroll : public CueEditor
 	int32_t get_grid_beat_divisions (Editing::GridType gt) const { return 1; }
 	int32_t get_grid_music_divisions (Editing::GridType gt, uint32_t event_state) const { return 1; }
 
-	void set_trigger (ARDOUR::TriggerReference&);
 	void set_region (std::shared_ptr<ARDOUR::Region>);
 	void set_track (std::shared_ptr<ARDOUR::Track>);
 
@@ -198,9 +197,9 @@ class Pianoroll : public CueEditor
 	PBD::ScopedConnectionList view_connections;
 	void maybe_update ();
 	void trigger_prop_change (PBD::PropertyChange const &);
-	void region_prop_change (PBD::PropertyChange const &);
 
-	void unset (bool trigger_too);
+	void unset_region ();
+	void unset_trigger ();
 
 	void bindings_changed ();