use RAII for class-wide PresentationInfo::Change signal, along with properties to describe what changed

5 files changed, 132 insertions, 71 deletions

diff --git a/libs/ardour/ardour/presentation_info.h b/libs/ardour/ardour/presentation_info.h
index 612187e376..aa94cfd7aa 100644
--- a/libs/ardour/ardour/presentation_info.h
+++ b/libs/ardour/ardour/presentation_info.h
@@ -232,16 +232,30 @@ class LIBARDOUR_API PresentationInfo : public PBD::Stateful
 	static Flag get_flags (XMLNode const& node);
 	static std::string state_node_name;
 
-	/* for things concerned about *any* PresentationInfo. This is emitted
-	 * only at the request of another object that has finished making some
-	 * changes (e.g. reordering things)
+	/* for things concerned about *any* PresentationInfo.
 	 */
 
-	static PBD::Signal0<void> Change;
+	static PBD::Signal1<void,PBD::PropertyChange const &> Change;
 
 	static void make_property_quarks ();
 
   protected:
+	friend class ChangeSuspender;
+	static void suspend_change_signal ();
+	static void unsuspend_change_signal ();
+
+  public:
+	class ChangeSuspender {
+          public:
+		ChangeSuspender() {
+			PresentationInfo::suspend_change_signal ();
+		}
+		~ChangeSuspender() {
+			PresentationInfo::unsuspend_change_signal ();
+		}
+	};
+
+  protected:
 	friend class Stripable;
 	void set_order (order_t order);
 
@@ -249,6 +263,10 @@ class LIBARDOUR_API PresentationInfo : public PBD::Stateful
 	order_t _order;
 	Flag    _flags;
 	color_t _color;
+
+	static PBD::PropertyChange _pending_static_changes;
+	static int _change_signal_suspended;
+	static void send_static_change (const PBD::PropertyChange&);
 };
 
 }
diff --git a/libs/ardour/ardour/stripable.h b/libs/ardour/ardour/stripable.h
index 942b48815b..92cf7bdced 100644
--- a/libs/ardour/ardour/stripable.h
+++ b/libs/ardour/ardour/stripable.h
@@ -77,7 +77,7 @@ class LIBARDOUR_API Stripable : public SessionObject {
 
 	/* set just the order */
 
-	void  set_presentation_order (PresentationInfo::order_t, bool notify_class_listeners = true);
+	void  set_presentation_order (PresentationInfo::order_t);
 
 	struct PresentationOrderSorter {
 		bool operator() (boost::shared_ptr<Stripable> a, boost::shared_ptr<Stripable> b) {
diff --git a/libs/ardour/presentation_info.cc b/libs/ardour/presentation_info.cc
index a44f5f4a8b..a9af018a9d 100644
--- a/libs/ardour/presentation_info.cc
+++ b/libs/ardour/presentation_info.cc
@@ -37,7 +37,10 @@ using namespace PBD;
 using std::string;
 
 string PresentationInfo::state_node_name = X_("PresentationInfo");
-PBD::Signal0<void> PresentationInfo::Change;
+
+PBD::Signal1<void,PropertyChange const &> PresentationInfo::Change;
+int PresentationInfo::_change_signal_suspended = 0;
+PBD::PropertyChange PresentationInfo::_pending_static_changes;
 
 namespace ARDOUR {
 	namespace Properties {
@@ -47,6 +50,42 @@ namespace ARDOUR {
 	}
 }
 
+void
+PresentationInfo::suspend_change_signal ()
+{
+	g_atomic_int_add (&_change_signal_suspended, 1);
+}
+
+void
+PresentationInfo::unsuspend_change_signal ()
+{
+	PropertyChange pc = _pending_static_changes;
+
+	/* XXX some possible race condition here; _pending_static_changes could
+	 * be reset by another thread before or after we decrement.
+	 */
+
+	if (g_atomic_int_dec_and_test (const_cast<gint*> (&_change_signal_suspended))) {
+		_pending_static_changes.clear ();
+		Change (pc); /* EMIT SIGNAL */
+	}
+}
+
+void
+PresentationInfo::send_static_change (const PropertyChange& what_changed)
+{
+	if (what_changed.empty()) {
+		return;
+	}
+
+	if (g_atomic_int_get (&_change_signal_suspended)) {
+		_pending_static_changes.add (what_changed);
+		return;
+	}
+
+	Change (what_changed);
+}
+
 const PresentationInfo::order_t PresentationInfo::max_order = UINT32_MAX;
 const PresentationInfo::Flag PresentationInfo::Bus = PresentationInfo::Flag (PresentationInfo::AudioBus|PresentationInfo::MidiBus);
 const PresentationInfo::Flag PresentationInfo::Track = PresentationInfo::Flag (PresentationInfo::AudioTrack|PresentationInfo::MidiTrack);
@@ -164,7 +203,7 @@ PresentationInfo::set_color (PresentationInfo::color_t c)
 	if (c != _color) {
 		_color = c;
 		send_change (PropertyChange (Properties::color));
-		Change (); /* EMIT SIGNAL */
+		send_static_change (PropertyChange (Properties::color));
 	}
 }
 
@@ -189,7 +228,7 @@ PresentationInfo::set_selected (bool yn)
 			_flags = Flag (_flags & ~Selected);
 		}
 		send_change (PropertyChange (Properties::selected));
-		Change (); /* EMIT SIGNAL */
+		send_static_change (PropertyChange (Properties::selected));
 	}
 }
 
@@ -205,7 +244,7 @@ PresentationInfo::set_hidden (bool yn)
 		}
 
 		send_change (PropertyChange (Properties::hidden));
-		Change (); /* EMIT SIGNAL */
+		send_static_change (PropertyChange (Properties::hidden));
 	}
 }
 
@@ -217,7 +256,7 @@ PresentationInfo::set_order (order_t order)
 	if (order != _order) {
 		_order = order;
 		send_change (PropertyChange (Properties::order));
-		Change (); /* EMIT SIGNAL */
+		send_static_change (PropertyChange (Properties::order));
 	}
 }
 
diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 7941a66e54..f1b9c9341b 100644
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -418,6 +418,8 @@ Session::Session (AudioEngine &eng,
 
 	_state_of_the_state = StateOfTheState (_state_of_the_state & ~Dirty);
 
+	PresentationInfo::Change.connect_same_thread (*this, boost::bind (&Session::notify_presentation_info_change, this));
+
 	Config->ParameterChanged.connect_same_thread (*this, boost::bind (&Session::config_changed, this, _1, false));
 	config.ParameterChanged.connect_same_thread (*this, boost::bind (&Session::config_changed, this, _1, true));
 
@@ -3484,81 +3486,85 @@ Session::add_routes_inner (RouteList& new_routes, bool input_auto_connect, bool
 	DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("ensure order gap starting at %1 for %2\n", order, new_routes.size()));
 	ensure_route_presentation_info_gap (order, new_routes.size());
 
-	for (RouteList::iterator x = new_routes.begin(); x != new_routes.end(); ++x, ++added) {
+	{
+		PresentationInfo::ChangeSuspender cs;
 
-		boost::weak_ptr<Route> wpr (*x);
-		boost::shared_ptr<Route> r (*x);
+		for (RouteList::iterator x = new_routes.begin(); x != new_routes.end(); ++x, ++added) {
 
-		r->solo_control()->Changed.connect_same_thread (*this, boost::bind (&Session::route_solo_changed, this, _1, _2,wpr));
-		r->solo_isolate_control()->Changed.connect_same_thread (*this, boost::bind (&Session::route_solo_isolated_changed, this, wpr));
-		r->mute_control()->Changed.connect_same_thread (*this, boost::bind (&Session::route_mute_changed, this));
+			boost::weak_ptr<Route> wpr (*x);
+			boost::shared_ptr<Route> r (*x);
 
-		r->output()->changed.connect_same_thread (*this, boost::bind (&Session::set_worst_io_latencies_x, this, _1, _2));
-		r->processors_changed.connect_same_thread (*this, boost::bind (&Session::route_processors_changed, this, _1));
-		r->processor_latency_changed.connect_same_thread (*this, boost::bind (&Session::queue_latency_recompute, this));
+			r->solo_control()->Changed.connect_same_thread (*this, boost::bind (&Session::route_solo_changed, this, _1, _2,wpr));
+			r->solo_isolate_control()->Changed.connect_same_thread (*this, boost::bind (&Session::route_solo_isolated_changed, this, wpr));
+			r->mute_control()->Changed.connect_same_thread (*this, boost::bind (&Session::route_mute_changed, this));
 
-		if (r->is_master()) {
-			_master_out = r;
-		}
+			r->output()->changed.connect_same_thread (*this, boost::bind (&Session::set_worst_io_latencies_x, this, _1, _2));
+			r->processors_changed.connect_same_thread (*this, boost::bind (&Session::route_processors_changed, this, _1));
+			r->processor_latency_changed.connect_same_thread (*this, boost::bind (&Session::queue_latency_recompute, this));
 
-		if (r->is_monitor()) {
-			_monitor_out = r;
-		}
+			if (r->is_master()) {
+				_master_out = r;
+			}
 
-		boost::shared_ptr<Track> tr = boost::dynamic_pointer_cast<Track> (r);
-		if (tr) {
-			tr->PlaylistChanged.connect_same_thread (*this, boost::bind (&Session::track_playlist_changed, this, boost::weak_ptr<Track> (tr)));
-			track_playlist_changed (boost::weak_ptr<Track> (tr));
-			tr->rec_enable_control()->Changed.connect_same_thread (*this, boost::bind (&Session::update_route_record_state, this));
-
-			boost::shared_ptr<MidiTrack> mt = boost::dynamic_pointer_cast<MidiTrack> (tr);
-			if (mt) {
-				mt->StepEditStatusChange.connect_same_thread (*this, boost::bind (&Session::step_edit_status_change, this, _1));
-				mt->output()->changed.connect_same_thread (*this, boost::bind (&Session::midi_output_change_handler, this, _1, _2, boost::weak_ptr<Route>(mt)));
-				mt->presentation_info().PropertyChanged.connect_same_thread (*this, boost::bind (&Session::midi_track_presentation_info_changed, this, _1, boost::weak_ptr<MidiTrack>(mt)));
+			if (r->is_monitor()) {
+				_monitor_out = r;
+			}
+
+			boost::shared_ptr<Track> tr = boost::dynamic_pointer_cast<Track> (r);
+			if (tr) {
+				tr->PlaylistChanged.connect_same_thread (*this, boost::bind (&Session::track_playlist_changed, this, boost::weak_ptr<Track> (tr)));
+				track_playlist_changed (boost::weak_ptr<Track> (tr));
+				tr->rec_enable_control()->Changed.connect_same_thread (*this, boost::bind (&Session::update_route_record_state, this));
+
+				boost::shared_ptr<MidiTrack> mt = boost::dynamic_pointer_cast<MidiTrack> (tr);
+				if (mt) {
+					mt->StepEditStatusChange.connect_same_thread (*this, boost::bind (&Session::step_edit_status_change, this, _1));
+					mt->output()->changed.connect_same_thread (*this, boost::bind (&Session::midi_output_change_handler, this, _1, _2, boost::weak_ptr<Route>(mt)));
+					mt->presentation_info().PropertyChanged.connect_same_thread (*this, boost::bind (&Session::midi_track_presentation_info_changed, this, _1, boost::weak_ptr<MidiTrack>(mt)));
+				}
 			}
-		}
 
-		if (!r->presentation_info().special()) {
+			if (!r->presentation_info().special()) {
 
-			DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("checking PI state for %1\n", r->name()));
+				DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("checking PI state for %1\n", r->name()));
 
-			/* presentation info order may already have been set from XML */
+				/* presentation info order may already have been set from XML */
 
-			if (!r->presentation_info().order_set()) {
+				if (!r->presentation_info().order_set()) {
 
-				if (order == PresentationInfo::max_order) {
-					/* just add to the end */
-					r->set_presentation_order (n_routes + added, false);
-					DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("group order not set, set to NR %1 + %2 = %3\n", n_routes, added, n_routes + added));
+					if (order == PresentationInfo::max_order) {
+						/* just add to the end */
+						r->set_presentation_order (n_routes + added);
+						DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("group order not set, set to NR %1 + %2 = %3\n", n_routes, added, n_routes + added));
+					} else {
+						r->set_presentation_order (order + added);
+						DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("group order not set, set to %1 + %2 = %3\n", order, added, order + added));
+					}
 				} else {
-					r->set_presentation_order (order + added);
-					DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("group order not set, set to %1 + %2 = %3\n", order, added, order + added));
+					DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("group order already set to %1\n", r->presentation_info().order()));
 				}
-			} else {
-				DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("group order already set to %1\n", r->presentation_info().order()));
 			}
-		}
 
 #if !defined(__APPLE__) && !defined(__FreeBSD__)
-		/* clang complains: 'operator<<' should be declared prior to the call site or in an associated namespace of one of its
-		 * arguments std::ostream& operator<<(std::ostream& o, ARDOUR::PresentationInfo const& rid)"
-		 */
-		DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("added route %1, group order %2 type %3 (summary: %4)\n",
-		                                               r->name(),
-		                                               r->presentation_info().order(),
-		                                               enum_2_string (r->presentation_info().flags()),
-		                                               r->presentation_info()));
+			/* clang complains: 'operator<<' should be declared prior to the call site or in an associated namespace of one of its
+			 * arguments std::ostream& operator<<(std::ostream& o, ARDOUR::PresentationInfo const& rid)"
+			 */
+			DEBUG_TRACE (DEBUG::OrderKeys, string_compose ("added route %1, group order %2 type %3 (summary: %4)\n",
+			                                               r->name(),
+			                                               r->presentation_info().order(),
+			                                               enum_2_string (r->presentation_info().flags()),
+			                                               r->presentation_info()));
 #endif
 
 
-		if (input_auto_connect || output_auto_connect) {
-			auto_connect_route (r, input_auto_connect, ChanCount (), ChanCount (), existing_inputs, existing_outputs);
-			existing_inputs += r->n_inputs();
-			existing_outputs += r->n_outputs();
-		}
+			if (input_auto_connect || output_auto_connect) {
+				auto_connect_route (r, input_auto_connect, ChanCount (), ChanCount (), existing_inputs, existing_outputs);
+				existing_inputs += r->n_inputs();
+				existing_outputs += r->n_outputs();
+			}
 
-		ARDOUR::GUIIdle ();
+			ARDOUR::GUIIdle ();
+		}
 	}
 
 	if (_monitor_out && IO::connecting_legal) {
@@ -3773,7 +3779,10 @@ Session::remove_routes (boost::shared_ptr<RouteList> routes_to_remove)
 		return;
 	}
 
-	PresentationInfo::Change(); /* EMIT SIGNAL */
+	PropertyChange so;
+	so.add (Properties::selected);
+	so.add (Properties::order);
+	PresentationInfo::Change (PropertyChange (so));
 
 	/* save the new state of the world */
 
@@ -6830,7 +6839,6 @@ Session::notify_presentation_info_change ()
 		return;
 	}
 
-	PresentationInfo::Change (); /* EMIT SIGNAL */
 	reassign_track_numbers();
 
 #ifdef USE_TRACKS_CODE_FEATURES
diff --git a/libs/ardour/stripable.cc b/libs/ardour/stripable.cc
index 84d2bafccf..135526f2a3 100644
--- a/libs/ardour/stripable.cc
+++ b/libs/ardour/stripable.cc
@@ -39,13 +39,9 @@ Stripable::Stripable (Session& s, string const & name, PresentationInfo const &
 }
 
 void
-Stripable::set_presentation_order (PresentationInfo::order_t order, bool notify_class_listeners)
+Stripable::set_presentation_order (PresentationInfo::order_t order)
 {
 	_presentation_info.set_order (order);
-
-	if (notify_class_listeners) {
-		PresentationInfo::Change ();
-	}
 }
 
 int