stripable color picking: stripable has handle to its color picker dialog

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.

gtk2_ardour/route_ui.cc

@@ -297,7 +297,6 @@ RouteUI::reset ()
 	mute_menu = 0;
 
 	delete_patch_change_dialog ();
-	_color_picker.reset ();
 
 	denormal_menu_item = 0;
 }
@@ -1714,7 +1713,11 @@ RouteUI::select_midi_patch ()
 void
 RouteUI::choose_color (Gtk::Window* parent)
 {
-	_color_picker.popup (_route, parent);
+	StripableColorDialog* scd = _route->active_color_picker();
+	if (!scd) {
+		scd = new StripableColorDialog (_route);
+	}
+	scd->popup (parent);
 }
 
 /** Set the route's own color.  This may not be used for display if

gtk2_ardour/route_ui.h

@@ -334,8 +334,6 @@ private:
 
 	std::vector<ArdourWidgets::ArdourButton*> _invert_buttons;
 
-	StripableColorDialog _color_picker;
-
 	sigc::connection send_blink_connection;
 	sigc::connection rec_blink_connection;
 

gtk2_ardour/stripable_colorpicker.cc

@@ -18,8 +18,12 @@
 
 #include "pbd/compose.h"
 
-#include "gtkmm2ext/colors.h"
+#include "ardour/stripable.h"
 
+#include "gtkmm2ext/colors.h"
+#include "gtkmm2ext/doi.h"
+
+#include "gui_thread.h"
 #include "public_editor.h"
 #include "stripable_colorpicker.h"
 #include "ui_config.h"
@@ -31,15 +35,26 @@ using namespace Gtk;
 bool StripableColorDialog::palette_initialized = false;
 Gtk::ColorSelection::SlotChangePaletteHook StripableColorDialog::gtk_palette_changed_hook;
 
-StripableColorDialog::StripableColorDialog ()
+StripableColorDialog::StripableColorDialog (std::shared_ptr<ARDOUR::Stripable> s)
 {
+	assert (s);
+
 	initialize_color_palette ();
 	signal_response().connect (sigc::mem_fun (*this, &StripableColorDialog::finish_color_edit));
+
+	_stripable = s;
+	_stripable->set_active_color_picker (this);
+	_stripable->DropReferences.connect (_connections, invalidator (*this), std::bind (&delete_when_idle<StripableColorDialog>, this), gui_context ());
+
 }
 
 StripableColorDialog::~StripableColorDialog ()
 {
-	reset ();
+	hide ();
+	_stripable->set_active_color_picker (nullptr);
+	_stripable.reset ();
+	_connections.drop_connections ();
+	_color_changed_connection.disconnect ();
 }
 
 void
@@ -57,8 +72,7 @@ StripableColorDialog::initialize_color_palette ()
 	if (palette_initialized) {
 		return;
 	}
-	gtk_palette_changed_hook =
-		get_color_selection()->set_change_palette_hook (&StripableColorDialog::palette_changed_hook);
+	gtk_palette_changed_hook = get_color_selection()->set_change_palette_hook (&StripableColorDialog::palette_changed_hook);
 
 	std::string cp = UIConfiguration::instance ().get_stripable_color_palette ();
 	if (!cp.empty()) {
@@ -69,65 +83,9 @@ StripableColorDialog::initialize_color_palette ()
 }
 
 void
-StripableColorDialog::reset ()
+StripableColorDialog::popup (Gtk::Window* parent)
 {
-	hide ();
-	if (_stripable && _stripable->active_color_picker() == this) {
-		_stripable->set_active_color_picker (0);
-	}
-	_stripable.reset ();
-	_color_changed_connection.disconnect ();
-}
-
-void
-StripableColorDialog::popup (const std::string& name, uint32_t color, Gtk::Window* parent)
-{
-	set_title (string_compose (_("Color Selection: %1"), name));
-	_initial_color = color;
-
-	get_color_selection()->set_has_opacity_control (false);
-	get_color_selection()->set_has_palette (true);
-
-	Gdk::Color c = Gtkmm2ext::gdk_color_from_rgba (_initial_color);
-
-	get_color_selection()->set_previous_color (c);
-	get_color_selection()->set_current_color (c);
-	_color_changed_connection.disconnect ();
-	_color_changed_connection = get_color_selection()->signal_color_changed().connect (sigc::mem_fun (*this, &StripableColorDialog::color_changed));
-
-	if (parent) {
-		set_transient_for (*parent);
-	}
-	set_position (UIConfiguration::instance().get_default_window_position());
-	present ();
-}
-
-void
-StripableColorDialog::popup (std::shared_ptr<ARDOUR::Stripable> s, Gtk::Window* parent)
-{
-	if (s && s->active_color_picker()) {
-		if (parent) {
-			s->active_color_picker()->set_transient_for (*parent);
-		}
-		s->active_color_picker()->set_position (Gtk::WIN_POS_CENTER_ALWAYS); // force update
-		s->active_color_picker()->set_position (UIConfiguration::instance().get_default_window_position());
-		s->active_color_picker()->present ();
-		return;
-	}
-	if (_stripable == s) {
-		/* keep modified color */
-		if (parent) {
-			set_transient_for (*parent);
-		}
-		set_position (Gtk::WIN_POS_CENTER_ALWAYS); // force update
-		set_position (UIConfiguration::instance().get_default_window_position());
-		present ();
-		return;
-	}
-
-	_stripable = s;
-	_stripable->set_active_color_picker (this);
-	popup (s->name(), _stripable->presentation_info().color (), parent);
+	popup (_stripable->name(), _stripable->presentation_info().color(), parent);
 }
 
 void
@@ -138,43 +96,47 @@ StripableColorDialog::finish_color_edit (int response)
 	if (response == RESPONSE_OK) {
 		ColorChanged (Gtkmm2ext::gdk_color_to_rgba (get_color_selection()->get_current_color())); /* EMIT SIGNAL */
 	}
-	if (_stripable && response == RESPONSE_OK) {
+
+	if (response == RESPONSE_OK) {
 		for (ARDOUR::RouteList::iterator i = rl.begin(); i != rl.end(); ++i) {
 			(*i)->presentation_info().set_color (Gtkmm2ext::gdk_color_to_rgba (get_color_selection()->get_current_color()));
 		}
 		_stripable->presentation_info().set_color (Gtkmm2ext::gdk_color_to_rgba (get_color_selection()->get_current_color()));
-	} else if (_stripable) {
+	} else {
 		_stripable->presentation_info().set_color (_initial_color);
 	}
-	reset ();
+
+	hide ();
 }
 
 void
 StripableColorDialog::color_changed ()
 {
-	if (_stripable) {
-		_stripable->presentation_info().set_color (Gtkmm2ext::gdk_color_to_rgba (get_color_selection()->get_current_color()));
-	}
-}
-
-
-ArdourColorButton::ArdourColorButton ()
-{
-	_color_picker.ColorChanged.connect (sigc::mem_fun(*this, &ArdourColorButton::color_selected));
+	_stripable->presentation_info().set_color (Gtkmm2ext::gdk_color_to_rgba (get_color_selection()->get_current_color()));
 }
 
 void
-ArdourColorButton::on_clicked ()
+StripableColorDialog::popup (const std::string& name, uint32_t color, Gtk::Window* parent)
 {
-	_color_picker.popup ("", Gtkmm2ext::gdk_color_to_rgba (get_color ()), dynamic_cast<Gtk::Window*> (get_toplevel()));
-	_color_picker.get_window ()->set_transient_for (get_window ());
-}
+	set_title (string_compose (_("Color Selection: %1"), name));
+	_initial_color = color;
 
-void
-ArdourColorButton::color_selected (uint32_t color)
-{
-	Gdk::Color c;
-	Gtkmm2ext::set_color_from_rgba (c, color);
-	set_color (c);
-	g_signal_emit_by_name (GTK_WIDGET(gobj()), "color-set", 0);
+	Gtk::ColorSelection* color_selection (get_color_selection());
+
+	color_selection->set_has_opacity_control (false);
+	color_selection->set_has_palette (true);
+
+	Gdk::Color c = Gtkmm2ext::gdk_color_from_rgba (_initial_color);
+
+	color_selection->set_previous_color (c);
+	color_selection->set_current_color (c);
+
+	_color_changed_connection.disconnect ();
+
+ 	if (parent) {
+ 		set_transient_for (*parent);
+ 	}
+
+ 	set_position (UIConfiguration::instance().get_default_window_position());
+ 	present ();
 }

gtk2_ardour/stripable_colorpicker.h

@@ -23,44 +23,33 @@
 #include <ytkmm/colorbutton.h>
 #include <ytkmm/colorselection.h>
 
-#include "ardour/stripable.h"
+#include "ardour/presentation_info.h"
+
+namespace ARDOUR {
+class Stripable;
+}
 
 class StripableColorDialog : public Gtk::ColorSelectionDialog
 {
 public:
-	StripableColorDialog ();
+	StripableColorDialog (std::shared_ptr<ARDOUR::Stripable>);
 	~StripableColorDialog ();
-	void reset ();
-	void popup (std::shared_ptr<ARDOUR::Stripable> s, Gtk::Window*);
-	void popup (const std::string&, uint32_t, Gtk::Window*);
+	void popup (Gtk::Window*);
 	sigc::signal<void, uint32_t> ColorChanged;
 
 private:
 	void initialize_color_palette ();
 	void finish_color_edit (int response);
 	void color_changed ();
+	void popup (const std::string& name, uint32_t color, Gtk::Window* parent);
 
 	std::shared_ptr<ARDOUR::Stripable> _stripable;
 	ARDOUR::PresentationInfo::color_t _initial_color;
 
 	sigc::connection _color_changed_connection;
-
+	PBD::ScopedConnectionList _connections;
 
 	static bool palette_initialized;
 	static void palette_changed_hook (const Glib::RefPtr<Gdk::Screen>&, const Gdk::ArrayHandle_Color&);
 	static Gtk::ColorSelection::SlotChangePaletteHook gtk_palette_changed_hook;
 };
-
-class ArdourColorButton : public Gtk::ColorButton
-{
-public:
-	ArdourColorButton ();
-
-protected:
-	void on_clicked();
-	void color_selected (uint32_t color);
-
-private:
-	StripableColorDialog _color_picker;
-};
-

gtk2_ardour/vca_master_strip.cc

@@ -593,7 +593,13 @@ VCAMasterStrip::state_id () const
 void
 VCAMasterStrip::start_color_edit ()
 {
-	_color_picker.popup (_vca, dynamic_cast<Gtk::Window*> (get_toplevel()));
+	StripableColorDialog* scd = _vca->active_color_picker();
+
+	if (!scd) {
+		scd = new StripableColorDialog (_vca);
+	}
+
+	scd->popup (dynamic_cast<Gtk::Window*> (get_toplevel()));
 }
 
 bool

gtk2_ardour/vca_master_strip.h

@@ -114,7 +114,6 @@ private:
 	void update_bottom_padding ();
 
 	void start_color_edit ();
-	StripableColorDialog _color_picker;
 };
 
 

gtk2_ardour/vca_time_axis.cc

@@ -560,6 +560,13 @@ VCATimeAxisView::drop_all_slaves ()
 }
 
 void
-VCATimeAxisView::choose_color () {
-	_color_picker.popup (_vca, PublicEditor::instance ().current_toplevel());
+VCATimeAxisView::choose_color ()
+{
+	StripableColorDialog* scd = _vca->active_color_picker();
+
+	if (!scd) {
+		scd = new StripableColorDialog (_vca);
+	}
+
+	scd->popup (dynamic_cast<Gtk::Window*> (PublicEditor::instance ().current_toplevel()));
 }

gtk2_ardour/vca_time_axis.h

@@ -21,7 +21,6 @@
 
 #include "widgets/ardour_button.h"
 
-#include "stripable_colorpicker.h"
 #include "stripable_time_axis.h"
 #include "gain_meter.h"
 
@@ -95,8 +94,5 @@ protected:
 
 	void drop_all_slaves ();
 	void choose_color ();
-
-private:
-	StripableColorDialog _color_picker;
 };