redesign API and internals of CoreSelection for more universality

We now have two basic methods for CoreSelection

* when selecting a stripable, use ::select_stripable_and_maybe_group() with
  appropriate arguments to cover the group selection aspects.

* when selecting an automation control that is part of a stripable, call
  ::select_stripable_with_control()

The old, more simply named methods (set/add/toggle etc.) have all been
made private, and their internal implementations changed somewhat.

This commit includes changes to control surfaces that use CoreSelection directly.

libs/ardour/ardour/selection.h

@@ -43,15 +43,11 @@ class LIBARDOUR_API CoreSelection : public PBD::Stateful {
 	CoreSelection (Session& s);
 	~CoreSelection ();
 
-	void toggle (std::shared_ptr<Stripable>, std::shared_ptr<AutomationControl>);
+	bool select_stripable_and_maybe_group (std::shared_ptr<Stripable> s, SelectionOperation op, bool with_group = true, bool routes_only = true, RouteGroup* = nullptr);
-	void add (std::shared_ptr<Stripable>, std::shared_ptr<AutomationControl>);
+	void select_stripable_with_control (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl>, SelectionOperation);
-	void remove (std::shared_ptr<Stripable>, std::shared_ptr<AutomationControl>);
-	void set (std::shared_ptr<Stripable>, std::shared_ptr<AutomationControl>);
-	void set (StripableList&);
 
 	void select_next_stripable (bool mixer_order, bool routes_only);
 	void select_prev_stripable (bool mixer_order, bool routes_only);
-	bool select_stripable_and_maybe_group (std::shared_ptr<Stripable> s, bool with_group, bool routes_only, RouteGroup*);
 
 	void clear_stripables();
 
@@ -124,6 +120,13 @@ class LIBARDOUR_API CoreSelection : public PBD::Stateful {
 		void select_adjacent_stripable (bool mixer_order, bool routes_only,
 		                                IterTypeCore (StripableList::*begin_method)(),
 		                                IterTypeCore (StripableList::*end_method)());
+
+	bool toggle (StripableList&, std::shared_ptr<AutomationControl>);
+	bool add (StripableList&, std::shared_ptr<AutomationControl>);
+	bool remove (StripableList&, std::shared_ptr<AutomationControl>);
+	bool set (StripableList&, std::shared_ptr<AutomationControl>, std::vector<std::shared_ptr<Stripable> > &);
+
+	bool do_select (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl> c, SelectionOperation op, bool with_group, bool routes_only, RouteGroup* not_allowed_in_group);
 };
 
 } // namespace ARDOUR

libs/ardour/ardour/types.h

@@ -979,7 +979,8 @@ enum SelectionOperation {
 	SelectionSet,
 	SelectionAdd,
 	SelectionToggle,
-	SelectionExtend
+	SelectionRemove,
+	SelectionExtend /* UI only operation, not core */
 };
 
 

libs/ardour/selection.cc

@@ -35,6 +35,116 @@
 using namespace ARDOUR;
 using namespace PBD;
 
+bool
+CoreSelection::do_select (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl> c, SelectionOperation op, bool with_group, bool routes_only, RouteGroup* not_allowed_in_group)
+{
+	std::shared_ptr<Route> r;
+	StripableList sl;
+	bool changed = false;
+	std::vector<std::shared_ptr<Stripable> > removed;
+
+	/* no selection of hidden stripables (though they can be selected and
+	 * then hidden
+	 */
+
+	if (s->is_hidden()) {
+		return false;
+	}
+
+	/* monitor is never selectable */
+
+	if (s->is_monitor() || s->is_surround_master ()) {
+		return false;
+	}
+
+	if (!(r = std::dynamic_pointer_cast<Route> (s)) && routes_only) {
+		return false;
+	}
+
+	if (r) {
+
+		/* no selection of inactive routes, though they can be selected
+		 * and made inactive.
+		 */
+
+		if (!r->active()) {
+			return false;
+		}
+
+		if (!c && with_group) {
+
+
+			if (!not_allowed_in_group || !r->route_group() || r->route_group() != not_allowed_in_group) {
+
+				if (r->route_group() && r->route_group()->is_select() && r->route_group()->is_active()) {
+					for (auto & ri : *(r->route_group()->route_list())) {
+						if (ri != r) {
+							sl.push_back (ri);
+						}
+					}
+				}
+			}
+		}
+	}
+
+	/* it is important to make the "primary" stripable being selected the last in this
+	 * list
+	 */
+
+	sl.push_back (s);
+
+	switch (op) {
+	case SelectionAdd:
+		changed = add (sl, c);
+		break;
+	case SelectionToggle:
+		changed = toggle (sl, c);
+		break;
+	case SelectionSet:
+		changed = set (sl, c, removed);
+		break;
+	case SelectionRemove:
+		changed = remove (sl, c);
+		break;
+	default:
+		return false;
+	}
+
+	if (changed || !removed.empty()) {
+
+		send_selection_change ();
+
+		/* send per-object signal to notify interested parties
+		   the selection status has changed
+		*/
+
+		PropertyChange pc (Properties::selected);
+
+		for (auto & s : removed) {
+			s->presentation_info().PropertyChanged (pc);
+		}
+
+		for (auto & s: sl) {
+			s->presentation_info().PropertyChanged (pc);
+		}
+
+	}
+
+	return changed;
+}
+
+bool
+CoreSelection::select_stripable_and_maybe_group (std::shared_ptr<Stripable> s, SelectionOperation op, bool with_group, bool routes_only, RouteGroup* not_allowed_in_group)
+{
+	return do_select (s, nullptr, op, with_group, routes_only, not_allowed_in_group);
+}
+
+void
+CoreSelection::select_stripable_with_control (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl> c, SelectionOperation op)
+{
+	do_select (s, c, op, c ? false : true, false, nullptr);
+}
+
 void
 CoreSelection::send_selection_change ()
 {
@@ -68,7 +178,7 @@ CoreSelection::select_adjacent_stripable (bool mixer_order, bool routes_only,
 		stripables.sort (ARDOUR::Stripable::Sorter (mixer_order));
 
 		for (StripableList::iterator s = stripables.begin(); s != stripables.end(); ++s) {
-			if (select_stripable_and_maybe_group (*s, true, routes_only, 0)) {
+			if (select_stripable_and_maybe_group (*s, SelectionSet, true, routes_only, nullptr)) {
 				break;
 			}
 		}
@@ -105,7 +215,7 @@ CoreSelection::select_adjacent_stripable (bool mixer_order, bool routes_only,
 		if (select_me) {
 
 			if (!this->selected (*i)) { /* not currently selected */
-				if (select_stripable_and_maybe_group (*i, true, routes_only, group)) {
+				if (select_stripable_and_maybe_group (*i, SelectionSet, true, routes_only, group)) {
 					return;
 				}
 			}
@@ -127,7 +237,7 @@ CoreSelection::select_adjacent_stripable (bool mixer_order, bool routes_only,
 		/* monitor is never selectable anywhere. for now, anyway */
 
 		if (!routes_only || r) {
-			if (select_stripable_and_maybe_group (*s, true, routes_only, 0)) {
+			if (select_stripable_and_maybe_group (*s, SelectionSet, true, routes_only, 0)) {
 				return;
 			}
 		}
@@ -148,89 +258,39 @@ CoreSelection::select_prev_stripable (bool mixer_order, bool routes_only)
 
 
 bool
-CoreSelection::select_stripable_and_maybe_group (std::shared_ptr<Stripable> s, bool with_group, bool routes_only, RouteGroup* not_allowed_in_group)
+CoreSelection::toggle (StripableList& sl, std::shared_ptr<AutomationControl> c)
 {
-	std::shared_ptr<Route> r;
+	assert (sl.size() == 1 || !c);
-	StripableList sl;
+	bool changed = false;
+	StripableList sl2;
 
-	/* no selection of hidden stripables (though they can be selected and
+	for (auto & s : sl) {
-	 * then hidden
-	 */
-
-	if (s->is_hidden()) {
-		return false;
-	}
-
-	/* monitor is never selectable */
-
-	if (s->is_monitor() || s->is_surround_master ()) {
-		return false;
-	}
-
-	if ((r = std::dynamic_pointer_cast<Route> (s))) {
-
-		/* no selection of inactive routes, though they can be selected
-		 * and made inactive.
-		 */
-
-		if (!r->active()) {
-			return false;
-		}
-
-		if (with_group) {
-
-			if (!not_allowed_in_group || !r->route_group() || r->route_group() != not_allowed_in_group) {
-
-				if (r->route_group() && r->route_group()->is_select() && r->route_group()->is_active()) {
-					std::shared_ptr<RouteList> rl = r->route_group()->route_list ();
-					for (RouteList::iterator ri = rl->begin(); ri != rl->end(); ++ri) {
-						if (*ri != r) {
-							sl.push_back (*ri);
-						}
-					}
-				}
-
-				/* it is important to make the "primary" stripable being selected the last in this
-				 * list
-				 */
-
-				sl.push_back (s);
-				set (sl);
-				return true;
-			}
-
-		} else {
-			set (s, std::shared_ptr<AutomationControl>());
-			return true;
-		}
-
-	} else if (!routes_only) {
-		set (s, std::shared_ptr<AutomationControl>());
-		return true;
-	}
-
-	return false;
-}
-
-void
-CoreSelection::toggle (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl> c)
-{
 		DEBUG_TRACE (DEBUG::Selection, string_compose ("toggle: s %1 selected %2 c %3 selected %4\n",
 		                                               s, selected (s), c, selected (c)));
+
+		sl2.clear ();
+		sl2.push_back (s);
+
 		if ((c && selected (c)) || selected (s)) {
-		remove (s, c);
+			if (remove (sl2, c)) {
+				changed = true;
+			}
 		} else {
-		add (s, c);
+			if (add (sl2, c)) {
+				changed = true;
+			}
 		}
 	}
 
-void
+	return changed;
-CoreSelection::set (StripableList& sl)
+}
+
+bool
+CoreSelection::set (StripableList& sl, std::shared_ptr<AutomationControl> c, std::vector<std::shared_ptr<Stripable> > & removed)
 {
-	bool send = false;
+	assert (sl.size() == 1 || !c);
-	std::shared_ptr<AutomationControl> no_control;
 
-	std::vector<std::shared_ptr<Stripable> > removed;
+	bool changed = false;
 
 	{
 		Glib::Threads::RWLock::WriterLock lm (_lock);
@@ -248,83 +308,67 @@ CoreSelection::set (StripableList& sl)
 
 		for (StripableList::iterator s = sl.begin(); s != sl.end(); ++s) {
 
-			SelectedStripable ss (*s, no_control, _selection_order.fetch_add (1));
+			SelectedStripable ss (*s, c, _selection_order.fetch_add (1));
 
 			if (_stripables.insert (ss).second) {
 				DEBUG_TRACE (DEBUG::Selection, string_compose ("set:added %1 to s/c selection\n", (*s)->name()));
-				send = true;
+				changed = true;
 			} else {
 				DEBUG_TRACE (DEBUG::Selection, string_compose ("%1 already in s/c selection\n", (*s)->name()));
 			}
 		}
 
-		if (sl.size () > 0) {
+		if (!sl.empty()) {
 			_first_selected_stripable = sl.back ();
 		} else {
 			_first_selected_stripable.reset ();
 		}
 	}
 
-	if (send || !removed.empty()) {
+	return changed;
-
-		send_selection_change ();
-
-		/* send per-object signal to notify interested parties
-		   the selection status has changed
-		*/
-
-		PropertyChange pc (Properties::selected);
-
-		for (std::vector<std::shared_ptr<Stripable> >::iterator s = removed.begin(); s != removed.end(); ++s) {
-			(*s)->presentation_info().PropertyChanged (pc);
 }
 
-		for (StripableList::iterator s = sl.begin(); s != sl.end(); ++s) {
+bool
-			(*s)->presentation_info().PropertyChanged (pc);
+CoreSelection::add (StripableList& sl, std::shared_ptr<AutomationControl> c)
-		}
-
-	}
-
-}
-
-void
-CoreSelection::add (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl> c)
 {
-	bool send = false;
+	assert (sl.size() == 1 || !c);
+
+	bool changed = false;
 
 	{
 		Glib::Threads::RWLock::WriterLock lm (_lock);
 
+		for (auto & s : sl) {
 			SelectedStripable ss (s, c, _selection_order.fetch_add (1));
 
 			if (_stripables.insert (ss).second) {
 				DEBUG_TRACE (DEBUG::Selection, string_compose ("added %1/%2 to s/c selection\n", s->name(), c));
-			send = true;
+				changed  = true;
 			} else {
 				DEBUG_TRACE (DEBUG::Selection, string_compose ("%1/%2 already in s/c selection\n", s->name(), c));
 			}
-		_first_selected_stripable = s;
 		}
 
-	if (send) {
+		if (!sl.empty()) {
-		send_selection_change ();
+			_first_selected_stripable = sl.back();
-		/* send per-object signal to notify interested parties
+		} else {
-		   the selection status has changed
+			_first_selected_stripable.reset ();
-		*/
-		if (s) {
-			PropertyChange pc (Properties::selected);
-			s->presentation_info().PropertyChanged (pc);
-		}
 		}
 	}
 
-void
+	return changed;
-CoreSelection::remove (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl> c)
+}
+
+bool
+CoreSelection::remove (StripableList & sl, std::shared_ptr<AutomationControl> c)
 {
-	bool send = false;
+	assert (sl.size() == 1 || !c);
+	bool changed = false;
+
 	{
 		Glib::Threads::RWLock::WriterLock lm (_lock);
 
+		for (auto & s : sl) {
 			SelectedStripable ss (s, c, 0);
 
 			SelectedStripables::iterator i = _stripables.find (ss);
@@ -332,52 +376,16 @@ CoreSelection::remove (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationC
 			if (i != _stripables.end()) {
 				_stripables.erase (i);
 				DEBUG_TRACE (DEBUG::Selection, string_compose ("removed %1/%2 from s/c selection\n", s, c));
-			send = true;
+				changed = true;
 			}
+
 			if (s == _first_selected_stripable.lock ()) {
 				_first_selected_stripable.reset ();
 			}
 		}
-
-	if (send) {
-		send_selection_change ();
-		/* send per-object signal to notify interested parties
-		   the selection status has changed
-		*/
-		if (s) {
-			PropertyChange pc (Properties::selected);
-			s->presentation_info().PropertyChanged (pc);
-		}
-	}
 	}
 
-void
+	return changed;
-CoreSelection::set (std::shared_ptr<Stripable> s, std::shared_ptr<AutomationControl> c)
-{
-	{
-		Glib::Threads::RWLock::WriterLock lm (_lock);
-
-		SelectedStripable ss (s, c, _selection_order.fetch_add (1));
-
-		if (_stripables.size() == 1 && _stripables.find (ss) != _stripables.end()) {
-			return;
-		}
-
-		_stripables.clear ();
-		_stripables.insert (ss);
-		_first_selected_stripable = s;
-		DEBUG_TRACE (DEBUG::Selection, string_compose ("set s/c selection to %1/%2\n", s->name(), c));
-	}
-
-	send_selection_change ();
-
-	/* send per-object signal to notify interested parties
-	   the selection status has changed
-	*/
-	if (s) {
-		PropertyChange pc (Properties::selected);
-		s->presentation_info().PropertyChanged (pc);
-	}
 }
 
 void

libs/ardour/vst3_plugin.cc

@@ -3180,11 +3180,11 @@ VST3PI::setContextInfoValue (FIDString id, int32 value)
 		std::shared_ptr<Stripable> stripable = s->session ().stripable_by_id (s->id ());
 		assert (stripable);
 		if (value == 0) {
-			s->session ().selection ().remove (stripable, std::shared_ptr<AutomationControl> ());
+			s->session ().selection ().select_stripable_and_maybe_group (stripable, SelectionRemove);
 		} else if (_add_to_selection) {
-			s->session ().selection ().add (stripable, std::shared_ptr<AutomationControl> ());
+			s->session ().selection ().select_stripable_and_maybe_group (stripable, SelectionAdd);
 		} else {
-			s->session ().selection ().set (stripable, std::shared_ptr<AutomationControl> ());
+			s->session ().selection ().select_stripable_and_maybe_group (stripable, SelectionSet);
 		}
 	} else if (0 == strcmp (id, ContextInfo::kMultiSelect)) {
 		_add_to_selection = value != 0;

libs/ctrl-interface/control_protocol/control_protocol.cc

@@ -363,25 +363,25 @@ ControlProtocol::first_selected_stripable () const
 void
 ControlProtocol::add_stripable_to_selection (std::shared_ptr<ARDOUR::Stripable> s)
 {
-	session->selection().add (s, std::shared_ptr<AutomationControl>());
+	session->selection().select_stripable_and_maybe_group (s, SelectionAdd);
 }
 
 void
 ControlProtocol::set_stripable_selection (std::shared_ptr<ARDOUR::Stripable> s)
 {
-	session->selection().select_stripable_and_maybe_group (s, true, true, 0);
+	session->selection().select_stripable_and_maybe_group (s, SelectionSet);
 }
 
 void
 ControlProtocol::toggle_stripable_selection (std::shared_ptr<ARDOUR::Stripable> s)
 {
-	session->selection().toggle (s, std::shared_ptr<AutomationControl>());
+	session->selection().select_stripable_and_maybe_group (s, SelectionToggle);
 }
 
 void
 ControlProtocol::remove_stripable_from_selection (std::shared_ptr<ARDOUR::Stripable> s)
 {
-	session->selection().remove (s, std::shared_ptr<AutomationControl>());
+	session->selection().select_stripable_and_maybe_group (s, SelectionRemove);
 }
 
 void
@@ -389,7 +389,7 @@ ControlProtocol::add_rid_to_selection (int rid)
 {
 	std::shared_ptr<Stripable> s = session->get_remote_nth_stripable (rid, PresentationInfo::MixerStripables);
 	if (s) {
-		session->selection().add (s, std::shared_ptr<AutomationControl>());
+		session->selection().select_stripable_and_maybe_group (s, SelectionAdd);
 	}
 }
 
@@ -398,7 +398,7 @@ ControlProtocol::set_rid_selection (int rid)
 {
 	std::shared_ptr<Stripable> s = session->get_remote_nth_stripable (rid, PresentationInfo::MixerStripables);
 	if (s) {
-		session->selection().select_stripable_and_maybe_group (s, true, true, 0);
+		session->selection().select_stripable_and_maybe_group (s, SelectionSet, true, true, 0);
 	}
 }
 
@@ -407,7 +407,7 @@ ControlProtocol::toggle_rid_selection (int rid)
 {
 	std::shared_ptr<Stripable> s = session->get_remote_nth_stripable (rid, PresentationInfo::MixerStripables);
 	if (s) {
-		session->selection().toggle (s, std::shared_ptr<AutomationControl>());
+		session->selection().select_stripable_and_maybe_group (s, SelectionToggle);
 	}
 }
 
@@ -416,7 +416,7 @@ ControlProtocol::remove_rid_from_selection (int rid)
 {
 	std::shared_ptr<Stripable> s = session->get_remote_nth_stripable (rid, PresentationInfo::MixerStripables);
 	if (s) {
-		session->selection().remove (s, std::shared_ptr<AutomationControl>());
+		session->selection().select_stripable_and_maybe_group (s, SelectionRemove);
 	}
 }
 

libs/surfaces/console1/console1.cc

@@ -1195,7 +1195,7 @@ Console1::select_rid_by_index (uint32_t index)
 	}
 	std::shared_ptr<Stripable> s = session->get_remote_nth_stripable (rid, PresentationInfo::MixerStripables);
 	if (s) {
-		session->selection ().select_stripable_and_maybe_group (s, true, false, 0);
+		session->selection ().select_stripable_and_maybe_group (s, SelectionSet, true, false, 0);
 	} else {
 		success = false;
 	}

libs/surfaces/launchpad_pro/lppro.cc

@@ -1473,7 +1473,7 @@ LaunchPadPro::select_stripable (int n)
 
 	std::shared_ptr<Route> r = session->get_remote_nth_route (scroll_x_offset + n);
 	if (r) {
-		session->selection().set (r, std::shared_ptr<AutomationControl>());
+		session->selection().select_stripable_and_maybe_group (r, SelectionSet);
 	}
 }
 

libs/surfaces/push2/cues.cc

@@ -289,7 +289,7 @@ CueLayout::button_lower (uint32_t n)
 		tb->stop_all_quantized ();
 	} else {
 		/* select track */
-		_session.selection().set (_route[n], std::shared_ptr<AutomationControl>());
+		_session.selection().select_stripable_and_maybe_group (_route[n], SelectionSet);
 	}
 }
 

libs/surfaces/push2/mix.cc

@@ -414,7 +414,7 @@ MixLayout::button_lower (uint32_t n)
 		return;
 	}
 
-	_session.selection().set (_stripable[n], std::shared_ptr<AutomationControl>());
+	_session.selection().select_stripable_and_maybe_group (_stripable[n], SelectionSet);
 }
 
 void
@@ -690,7 +690,7 @@ MixLayout::button_select_release ()
 		/* no visible track selected, select first (if any) */
 
 		if (_stripable[0]) {
-			_session.selection().set (_stripable[0], std::shared_ptr<AutomationControl>());
+			_session.selection().select_stripable_and_maybe_group (_stripable[0], SelectionSet);
 		}
 
 	} else {
@@ -706,7 +706,7 @@ MixLayout::button_select_release ()
 					_session.selection().clear_stripables ();
 					switch_bank (_bank_start - 1);
 					if (_stripable[0]) {
-						_session.selection().set (_stripable[0], std::shared_ptr<AutomationControl>());
+						_session.selection().select_stripable_and_maybe_group (_stripable[0], SelectionSet);
 					}
 				}
 			} else {
@@ -716,7 +716,7 @@ MixLayout::button_select_release ()
 					--n;
 				}
 				if (n >= 0) {
-					_session.selection().set (_stripable[n], std::shared_ptr<AutomationControl>());
+					_session.selection().select_stripable_and_maybe_group (_stripable[n], SelectionSet);
 				}
 			}
 
@@ -728,10 +728,10 @@ MixLayout::button_select_release ()
 				/* current selected is rightmost ... cancel selection,
 				   switch banks by one, and select righmost
 				*/
-				_session.selection().toggle (_stripable[selected], std::shared_ptr<AutomationControl>());
+				_session.selection().select_stripable_and_maybe_group (_stripable[selected], SelectionToggle);
 				switch_bank (_bank_start + 1);
 				if (_stripable[7]) {
-					_session.selection().set (_stripable[7], std::shared_ptr<AutomationControl>());
+					_session.selection().select_stripable_and_maybe_group (_stripable[7], SelectionSet);
 				}
 			} else {
 				/* select next, if any */
@@ -741,7 +741,7 @@ MixLayout::button_select_release ()
 				}
 
 				if (n != 8) {
-					_session.selection().set (_stripable[n], std::shared_ptr<AutomationControl>());
+					_session.selection().select_stripable_and_maybe_group (_stripable[n], SelectionSet);
 				}
 			}
 		}