GenericUI: only connect one PropertyChanged callback
authorJulien "_FrnchFrgg_" RIVAUD <frnchfrgg@free.fr>
Thu, 28 Jul 2016 17:43:43 +0000 (19:43 +0200)
committerJulien "_FrnchFrgg_" RIVAUD <frnchfrgg@free.fr>
Thu, 28 Jul 2016 21:19:08 +0000 (23:19 +0200)
The code connected the callback to the PropertyChanged signal from the
plugin once per filepath control created. Should the plugin have several
files to open, this would be at best wasteful and at worst racy.

Connect the callback a single time, since the same callback handles all
property updates that we're interested in. Also rename the methods,
members and typedefs so that it's clear what the code is trying to do.

gtk2_ardour/generic_pluginui.cc
gtk2_ardour/plugin_ui.h

index 316c7a1f0fad1c08a9b308661e051fce3ad68d2b..68bb429dbfb858246fb6c9466dc73e24889531ef 100644 (file)
@@ -150,6 +150,14 @@ GenericPluginUI::GenericPluginUI (boost::shared_ptr<PluginInsert> pi, bool scrol
 
        prefheight = 0;
        build ();
+
+       /* Listen for property changes that are not notified normally because
+        * AutomationControl has only support for numeric values currently.
+        * The only case is Variant::PATH for now */
+       plugin->PropertyChanged.connect(*this, invalidator(*this),
+                                       boost::bind(&GenericPluginUI::path_property_changed, this, _1, _2),
+                                       gui_context());
+
        main_contents.show ();
 }
 
@@ -719,16 +727,15 @@ GenericPluginUI::build_control_ui (const Evoral::Parameter&             param,
                                control_ui->pack_start (*control_ui->file_button, true, true);
                        }
 
-                       // Connect signals (TODO: do this via the Control)
+                       // Monitor changes from the user.
                        control_ui->file_button->signal_file_set().connect(
-                               sigc::bind(sigc::mem_fun(*this, &GenericPluginUI::set_property),
+                               sigc::bind(sigc::mem_fun(*this, &GenericPluginUI::set_path_property),
                                           desc, control_ui->file_button));
-                       plugin->PropertyChanged.connect(*this, invalidator(*this),
-                                                       boost::bind(&GenericPluginUI::property_changed, this, _1, _2),
-                                                       gui_context());
 
-                       _property_controls.insert(std::make_pair(desc.key, control_ui->file_button));
-                       control_ui->file_button = control_ui->file_button;
+                       /* Add the filebutton control to a map so that we can update it when
+                        * the corresponding property changes. This doesn't go through the usual
+                        * AutomationControls, because they don't support non-numeric values. */
+                       _filepath_controls.insert(std::make_pair(desc.key, control_ui->file_button));
 
                        return control_ui;
                }
@@ -1068,17 +1075,17 @@ GenericPluginUI::output_update ()
 }
 
 void
-GenericPluginUI::set_property (const ParameterDescriptor& desc,
-                               Gtk::FileChooserButton*    widget)
+GenericPluginUI::set_path_property (const ParameterDescriptor& desc,
+                                    Gtk::FileChooserButton*    widget)
 {
        plugin->set_property(desc.key, Variant(Variant::PATH, widget->get_filename()));
 }
 
 void
-GenericPluginUI::property_changed (uint32_t key, const Variant& value)
+GenericPluginUI::path_property_changed (uint32_t key, const Variant& value)
 {
-       PropertyControls::iterator c = _property_controls.find(key);
-       if (c != _property_controls.end()) {
+       FilePathControls::iterator c = _filepath_controls.find(key);
+       if (c != _filepath_controls.end()) {
                c->second->set_filename(value.get_path());
        } else {
                std::cerr << "warning: property change for property with no control" << std::endl;
index 721289ac0b3adc3c41c4ac12a1fb9db00dd0802f..001054bae9b8b7639e55f111a26c797042f1bd0e 100644 (file)
@@ -263,7 +263,9 @@ class GenericPluginUI : public PlugUIBase, public Gtk::VBox
        std::vector<ControlUI*>   input_controls;
        std::vector<ControlUI*>   input_controls_with_automation;
        std::vector<ControlUI*>   output_controls;
+
        sigc::connection screen_update_connection;
+
        void output_update();
 
        void build ();
@@ -291,12 +293,12 @@ class GenericPluginUI : public PlugUIBase, public Gtk::VBox
        bool integer_printer (char* buf, Gtk::Adjustment &, ControlUI *);
        bool midinote_printer(char* buf, Gtk::Adjustment &, ControlUI *);
 
-       void set_property (const ARDOUR::ParameterDescriptor& desc,
-                          Gtk::FileChooserButton*            widget);
-       void property_changed (uint32_t key, const ARDOUR::Variant& value);
+       typedef std::map<uint32_t, Gtk::FileChooserButton*> FilePathControls;
+       FilePathControls _filepath_controls;
+       void set_path_property (const ARDOUR::ParameterDescriptor& desc,
+                               Gtk::FileChooserButton*            widget);
+       void path_property_changed (uint32_t key, const ARDOUR::Variant& value);
 
-       typedef std::map<uint32_t, Gtk::FileChooserButton*> PropertyControls;
-       PropertyControls _property_controls;
 };
 
 class PluginUIWindow : public ArdourWindow