From 57ea5acfa0be98dad652c54aeb4715c736900b61 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Mon, 19 Jan 2026 19:47:05 -0700 Subject: [PATCH] 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 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 | 7 +- gtk2_ardour/route_ui.h | 2 - gtk2_ardour/stripable_colorpicker.cc | 134 ++++++++++----------------- gtk2_ardour/stripable_colorpicker.h | 29 ++---- gtk2_ardour/vca_master_strip.cc | 8 +- gtk2_ardour/vca_master_strip.h | 1 - gtk2_ardour/vca_time_axis.cc | 11 ++- gtk2_ardour/vca_time_axis.h | 4 - 8 files changed, 78 insertions(+), 118 deletions(-) diff --git a/gtk2_ardour/route_ui.cc b/gtk2_ardour/route_ui.cc index 50e0e97099..d6ce89176a 100644 --- a/gtk2_ardour/route_ui.cc +++ b/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 diff --git a/gtk2_ardour/route_ui.h b/gtk2_ardour/route_ui.h index 80f186f94a..e2dc9b90bf 100644 --- a/gtk2_ardour/route_ui.h +++ b/gtk2_ardour/route_ui.h @@ -334,8 +334,6 @@ private: std::vector _invert_buttons; - StripableColorDialog _color_picker; - sigc::connection send_blink_connection; sigc::connection rec_blink_connection; diff --git a/gtk2_ardour/stripable_colorpicker.cc b/gtk2_ardour/stripable_colorpicker.cc index 897a9e12ab..ec2268257c 100644 --- a/gtk2_ardour/stripable_colorpicker.cc +++ b/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 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, 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 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 (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 (); } diff --git a/gtk2_ardour/stripable_colorpicker.h b/gtk2_ardour/stripable_colorpicker.h index 20e829f06f..dbf68698d1 100644 --- a/gtk2_ardour/stripable_colorpicker.h +++ b/gtk2_ardour/stripable_colorpicker.h @@ -23,44 +23,33 @@ #include #include -#include "ardour/stripable.h" +#include "ardour/presentation_info.h" + +namespace ARDOUR { +class Stripable; +} class StripableColorDialog : public Gtk::ColorSelectionDialog { public: - StripableColorDialog (); + StripableColorDialog (std::shared_ptr); ~StripableColorDialog (); - void reset (); - void popup (std::shared_ptr s, Gtk::Window*); - void popup (const std::string&, uint32_t, Gtk::Window*); + void popup (Gtk::Window*); sigc::signal 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 _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&, 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; -}; - diff --git a/gtk2_ardour/vca_master_strip.cc b/gtk2_ardour/vca_master_strip.cc index 72d448aacb..67b598782f 100644 --- a/gtk2_ardour/vca_master_strip.cc +++ b/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 (get_toplevel())); + StripableColorDialog* scd = _vca->active_color_picker(); + + if (!scd) { + scd = new StripableColorDialog (_vca); + } + + scd->popup (dynamic_cast (get_toplevel())); } bool diff --git a/gtk2_ardour/vca_master_strip.h b/gtk2_ardour/vca_master_strip.h index f09d4118dd..d3e75a6f35 100644 --- a/gtk2_ardour/vca_master_strip.h +++ b/gtk2_ardour/vca_master_strip.h @@ -114,7 +114,6 @@ private: void update_bottom_padding (); void start_color_edit (); - StripableColorDialog _color_picker; }; diff --git a/gtk2_ardour/vca_time_axis.cc b/gtk2_ardour/vca_time_axis.cc index f425d9848f..986722f2cd 100644 --- a/gtk2_ardour/vca_time_axis.cc +++ b/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 (PublicEditor::instance ().current_toplevel())); } diff --git a/gtk2_ardour/vca_time_axis.h b/gtk2_ardour/vca_time_axis.h index 0f2ede507f..b6f6146cef 100644 --- a/gtk2_ardour/vca_time_axis.h +++ b/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; };