This involves multiple interlinked changes:
1. Redesigning StripableColorDialog to:
a) require a std::shared_ptr<Stripable> at construction
b) simplify the rest of its API down to just ::popup (Gdk::Window* parent)
2. dropping ArdourColorButton which was only used in the RouteGroupDialog and
served no real purpose since no Stripable was involved (hence, deriving from
StripableColorDialog was pointless)
3. dropping StripableColorDialog members of both RouteUI and the VCA UI
objects.
4. relying on the already-existing Stripable active_color_picker() API to hold
a reference to the dialog, and using that before creating a new one.
THe dialogs will be deleted in a GUI idle callback when the DropReferences signal
is emitted for the Stripable.
The previous design is broken by the editor mixer strip. If used to open the
comment editor for multiple routes, it would assign its own RouteCommentEditor
as the comment editor for each route.
This new design follows the model used by plugin UIs, in which the libardour
object (here, a Route) itself has a handle on its editor, and we always
look that up before creating a new one.
The new design requires passing a std::shared_ptr<Route> to the comment editor;
the comment editor itself is deleted (in the GUI thread) when the
DropReferences signal is emitted for the Route.
This design continues to meet the original goal of having only a single comment
editor per Route, but in a more correct way than before, and a way that
parallels the handling of plugin UIs.
Properties::region_fx may not be emitted for add/remove/reorder
cases when no disk-reader overrride is required.
However we need to inform the GUI when such changes happen,
and various UI widgets listen to property changes.
We should not call CueEditor::rec_enable_change() from CueEditor::trigger_arm_change()
because (a) the rec-enable change is coming anyway (b) at the time a trigger is
disarmed, the triggerbox is still rec-enabled. This means that in the end, a MidiView
gets its ::begin_write() method called again before we call ::model_changed()
and that leads it to have non-null _unfinished_live_notes (i.e. we're actively
recording, so do thing).
This allows the user to not have to aim for such precise timing, since they can
hold the note down during the count-in.
At some point the question will arise why we don't do this for controllers
etc. too.
This is a bit ironic, since EventSink is an abstract base class for MidiBuffer, which is
already supported for a flush_notes() call. But we use MidiBuffer::push_back() for that,
mostly for efficiency purposes (write() can insert an event at any time).
There is some weird behavior here, where causing a refill of the listview (e.g. by changing
the status of a port flag) doesn't interact correctly with the scrollbar. I can't find
a solution at the present time, so just grow the listview vertical size to accomodate a lot
more (potential) MIDI ports in both lists (without altering the prefs dialog size)