From e00003b14bd2a5bba046ce4fd5ee4f29d9c8a33d Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Thu, 22 Jul 2021 01:18:14 +0200 Subject: [PATCH] Playlist Selector cleanup code, fix selection * expand parent rows of selected playlist * do not subscribe multiple times to the same signal * scope variables, remove cruft * fix some many whitespace / indent issues --- gtk2_ardour/playlist_selector.cc | 387 +++++++++++++++---------------- gtk2_ardour/playlist_selector.h | 73 +++--- 2 files changed, 220 insertions(+), 240 deletions(-) diff --git a/gtk2_ardour/playlist_selector.cc b/gtk2_ardour/playlist_selector.cc index 8ef0ba59d9..a25d97288a 100644 --- a/gtk2_ardour/playlist_selector.cc +++ b/gtk2_ardour/playlist_selector.cc @@ -21,23 +21,23 @@ */ #include +#include + +#include "pbd/unwind.h" #include "ardour/audio_track.h" #include "ardour/audioplaylist.h" #include "ardour/midi_playlist.h" #include "ardour/playlist_factory.h" - #include "ardour/session_playlist.h" -#include - #include #include "ardour_ui.h" -#include "public_editor.h" -#include "playlist_selector.h" -#include "route_ui.h" #include "gui_thread.h" +#include "playlist_selector.h" +#include "public_editor.h" +#include "route_ui.h" #include "utils.h" #include "pbd/i18n.h" @@ -50,13 +50,13 @@ using namespace PBD; PlaylistSelector::PlaylistSelector () : ArdourDialog (_("Playlists")) + , _rui (0) + , _mode (plSelect) + , _ignore_selection (false) { - _rui = 0; - _mode = plSelect; - set_name ("PlaylistSelectorWindow"); - set_modal(false); - add_events (Gdk::KEY_PRESS_MASK|Gdk::KEY_RELEASE_MASK); + set_modal (false); + add_events (Gdk::KEY_PRESS_MASK | Gdk::KEY_RELEASE_MASK); set_size_request (500, 250); model = TreeStore::create (columns); @@ -66,46 +66,48 @@ PlaylistSelector::PlaylistSelector () tree.set_headers_visible (true); TreeViewColumn* name_column = tree.get_column (name_column_n); - name_column->set_sizing(TREE_VIEW_COLUMN_FIXED); - name_column->set_expand(true); - name_column->set_min_width(250); + name_column->set_sizing (TREE_VIEW_COLUMN_FIXED); + name_column->set_expand (true); + name_column->set_min_width (250); - scroller.add (tree); - scroller.set_policy (POLICY_AUTOMATIC, POLICY_AUTOMATIC); - Gtk::Frame *scroller_frame = manage(new Frame()); - scroller_frame->add(scroller); + Gtk::ScrolledWindow* scroller = manage (new Gtk::ScrolledWindow); + scroller->add (tree); + scroller->set_policy (POLICY_AUTOMATIC, POLICY_AUTOMATIC); - Gtk::Label *scope_label = manage(new Gtk::Label(_("Scope: "))); + Gtk::Frame* scroller_frame = manage (new Frame ()); + scroller_frame->add (*scroller); + + Gtk::Label* scope_label = manage (new Gtk::Label (_("Scope: "))); Gtk::RadioButtonGroup scope_group; _scope_grp_radio = manage (new RadioButton (scope_group, _("Only this track/group"))); _scope_rec_radio = manage (new RadioButton (scope_group, _("Rec-armed tracks"))); _scope_all_radio = manage (new RadioButton (scope_group, _("ALL tracks"))); - _scope_box = manage(new HBox()); - _scope_box->set_spacing(4); - _scope_box->set_border_width(2); - _scope_box->pack_start (*_scope_grp_radio, false, false); - _scope_box->pack_start (*_scope_rec_radio, false, false); - _scope_box->pack_start (*_scope_all_radio, false, false); - Gtk::Frame *scope_frame = manage(new Frame()); - scope_frame->add(*_scope_box); + _scope_box = manage (new HBox ()); + _scope_box->set_spacing (4); + _scope_box->set_border_width (2); + _scope_box->pack_start (*_scope_grp_radio, false, false); + _scope_box->pack_start (*_scope_rec_radio, false, false); + _scope_box->pack_start (*_scope_all_radio, false, false); + Gtk::Frame* scope_frame = manage (new Frame ()); + scope_frame->add (*_scope_box); - _button_box.set_spacing(6); - _button_box.pack_start(_btn_new_plist); - _button_box.pack_start(_btn_copy_plist); + _button_box.set_spacing (6); + _button_box.pack_start (_btn_new_plist); + _button_box.pack_start (_btn_copy_plist); - _scope_container.pack_start(*scope_label); - _scope_container.pack_start(*scope_frame); + _scope_container.pack_start (*scope_label); + _scope_container.pack_start (*scope_frame); - Gtk::Table *table = manage(new Gtk::Table()); + Gtk::Table* table = manage (new Gtk::Table ()); table->set_border_width (6); - table->attach (*scroller_frame, 0,2, 0,1, EXPAND|FILL, EXPAND|FILL, 0, 4); - table->attach (_scope_container, 0,2, 1,2, EXPAND|FILL, SHRINK, 0, 6); - table->attach (_button_box, 0,2, 2,3, EXPAND|FILL, SHRINK, 0, 0); + table->attach (*scroller_frame, 0, 2, 0, 1, EXPAND | FILL, EXPAND | FILL, 0, 4); + table->attach (_scope_container, 0, 2, 1, 2, EXPAND | FILL, SHRINK, 0, 6); + table->attach (_button_box, 0, 2, 2, 3, EXPAND | FILL, SHRINK, 0, 0); - get_vbox()->pack_start (*table, true, true); - get_vbox()->show_all(); + get_vbox ()->pack_start (*table, true, true); + get_vbox ()->show_all (); _btn_new_plist.set_name ("generic button"); _btn_new_plist.set_text ("New Playlist(s)"); @@ -115,120 +117,112 @@ PlaylistSelector::PlaylistSelector () _btn_copy_plist.set_text ("Copy Playlist(s)"); _btn_copy_plist.signal_clicked.connect (sigc::mem_fun (*this, &PlaylistSelector::copy_plist_button_clicked)); - select_connection = tree.get_selection()->signal_changed().connect (sigc::mem_fun(*this, &PlaylistSelector::selection_changed)); + tree.get_selection ()->signal_changed ().connect (sigc::mem_fun (*this, &PlaylistSelector::selection_changed)); - _scope_grp_radio->set_active(true); - - _ignore_selection = false; + _scope_grp_radio->set_active (true); } -void PlaylistSelector::new_plist_button_clicked() +void +PlaylistSelector::new_plist_button_clicked () { - if (_scope_all_radio->get_active()) { - PublicEditor::instance().new_playlists_for_all_tracks(false); - } else if (_scope_rec_radio->get_active()) { - PublicEditor::instance().new_playlists_for_armed_tracks(false); + if (_scope_all_radio->get_active ()) { + PublicEditor::instance ().new_playlists_for_all_tracks (false); + } else if (_scope_rec_radio->get_active ()) { + PublicEditor::instance ().new_playlists_for_armed_tracks (false); } else { - PublicEditor::instance().new_playlists_for_grouped_tracks(_rui, false); + PublicEditor::instance ().new_playlists_for_grouped_tracks (_rui, false); } } -void PlaylistSelector::copy_plist_button_clicked() +void +PlaylistSelector::copy_plist_button_clicked () { - if (_scope_all_radio->get_active()) { - PublicEditor::instance().new_playlists_for_all_tracks(true); - } else if (_scope_rec_radio->get_active()) { - PublicEditor::instance().new_playlists_for_armed_tracks(true); + if (_scope_all_radio->get_active ()) { + PublicEditor::instance ().new_playlists_for_all_tracks (true); + } else if (_scope_rec_radio->get_active ()) { + PublicEditor::instance ().new_playlists_for_armed_tracks (true); } else { - PublicEditor::instance().new_playlists_for_grouped_tracks(_rui, true); + PublicEditor::instance ().new_playlists_for_grouped_tracks (_rui, true); } } - -void PlaylistSelector::prepare(RouteUI* ruix, plMode mode) +void +PlaylistSelector::prepare (RouteUI* ruix, plMode mode) { _mode = mode; - + if (_rui != ruix) { _rui = ruix; + _track_connections.drop_connections (); - boost::shared_ptr this_track = _rui->track(); + boost::shared_ptr this_track = _rui->track (); if (this_track) { - this_track->PlaylistChanged.connect( - signal_connections, - invalidator(*this), - boost::bind(&PlaylistSelector::redisplay, this), - gui_context() - ); + this_track->PlaylistChanged.connect ( + _track_connections, + invalidator (*this), + boost::bind (&PlaylistSelector::redisplay, this), + gui_context ()); - this_track->PlaylistAdded.connect( - signal_connections, - invalidator(*this), - boost::bind(&PlaylistSelector::redisplay, this), - gui_context() - ); + this_track->PlaylistAdded.connect ( + _track_connections, + invalidator (*this), + boost::bind (&PlaylistSelector::redisplay, this), + gui_context ()); - this_track->DropReferences.connect( - signal_connections, - invalidator(*this), - boost::bind(&PlaylistSelector::ok_button_click, this), - gui_context() - ); + this_track->DropReferences.connect ( + _track_connections, + invalidator (*this), + boost::bind (&PlaylistSelector::ok_button_click, this), + gui_context ()); } } - - redisplay(); + + redisplay (); } PlaylistSelector::~PlaylistSelector () { - clear_map(); - - if (model) { - model->clear (); - } - if (current_playlist) { - current_playlist.reset(); - } - - signal_connections.drop_connections(); - select_connection.disconnect (); + clear_map (); } void PlaylistSelector::clear_map () { - for (TrackPlaylistMap::iterator x = trpl_map.begin(); x != trpl_map.end(); ++x) { + for (TrackPlaylistMap::iterator x = _trpl_map.begin (); x != _trpl_map.end (); ++x) { delete x->second; } - trpl_map.clear (); + _trpl_map.clear (); } void -PlaylistSelector::redisplay() +PlaylistSelector::redisplay () { - if (!_rui ) { + if (!_rui) { return; } - vector item; - string str; - if (_mode == plSelect) { - set_title (string_compose (_("Select a Playlist for: %1"), _rui->route()->name())); - _scope_container.show(); - _button_box.show(); + _scope_container.show (); + _button_box.show (); } else { - _scope_container.hide(); - _button_box.hide(); - if (_mode == plCopy) { - set_title (string_compose (_("Copy a Playlist for: %1"), _rui->route()->name())); - } else if (_mode == plShare) { - set_title (string_compose (_("Share a Playlist for: %1"), _rui->route()->name())); - } else if (_mode == plSteal) { - set_title (string_compose (_("Steal a Playlist for: %1"), _rui->route()->name())); - } + _scope_container.hide (); + _button_box.hide (); + } + + switch (_mode) { + case plSelect: + set_title (string_compose (_("Select a Playlist for: %1"), _rui->route ()->name ())); + break; + case plCopy: + set_title (string_compose (_("Copy a Playlist for: %1"), _rui->route ()->name ())); + break; + case plShare: + set_title (string_compose (_("Share a Playlist for: %1"), _rui->route ()->name ())); + break; + case plSteal: + set_title (string_compose (_("Steal a Playlist for: %1"), _rui->route ()->name ())); + break; } clear_map (); @@ -236,41 +230,34 @@ PlaylistSelector::redisplay() model->clear (); } - _session->playlists()->foreach (this, &PlaylistSelector::add_playlist_to_map); + _playlist_connections.drop_connections (); - boost::shared_ptr this_track = _rui->track(); + _session->playlists ()->foreach (this, &PlaylistSelector::add_playlist_to_map); - boost::shared_ptr proxy; + boost::shared_ptr this_track = _rui->track (); - if (this_track->playlist()) { - current_playlist = this_track->playlist(); - } - - for (TrackPlaylistMap::iterator x = trpl_map.begin(); x != trpl_map.end(); ++x) { + TreeModel::Row selected_row; + bool have_selected = false; + for (TrackPlaylistMap::iterator x = _trpl_map.begin (); x != _trpl_map.end (); ++x) { boost::shared_ptr tr = boost::dynamic_pointer_cast (_session->route_by_id (x->first)); /* add a node for the track */ string nodename; - if (!tr || tr->name().empty()) { + if (!tr || tr->name ().empty ()) { nodename = _("unassigned"); } else { - nodename = tr->name().c_str(); + nodename = tr->name ().c_str (); } TreeModel::Row row; - TreeModel::Row selected_row; - bool have_selected = false; - TreePath this_path; - //make a heading for each other track, if needed + /* make a heading for each other track, if needed */ if (_mode != plSelect) { - row = *(model->prepend()); + row = *(model->prepend ()); row[columns.text] = nodename; - boost::shared_ptr proxy = row[columns.playlist]; - proxy.reset (); } /* Now insert all the playlists for this diskstream/track in a subtree */ @@ -278,88 +265,81 @@ PlaylistSelector::redisplay() /* sort the playlists to match the order they appear in the track menu */ PlaylistSorterByID cmp; - sort (pls.begin(), pls.end(), cmp); + sort (pls.begin (), pls.end (), cmp); - for (vector >::iterator p = pls.begin(); p != pls.end(); ++p) { - - (*p)->PropertyChanged.connect (signal_connections, invalidator (*this), boost::bind (&PlaylistSelector::pl_property_changed, this, _1), gui_context()); + for (vector >::iterator p = pls.begin (); p != pls.end (); ++p) { + (*p)->PropertyChanged.connect (_playlist_connections, invalidator (*this), boost::bind (&PlaylistSelector::pl_property_changed, this, _1), gui_context ()); TreeModel::Row child_row; - if (tr == this_track && _mode==plSelect) { - child_row = *(model->append()); + if (tr == this_track && _mode == plSelect) { + child_row = *(model->append ()); } else if (_mode != plSelect) { - child_row = *(model->append (row.children())); + child_row = *(model->append (row.children ())); } - - if (child_row) { - child_row[columns.text] = (*p)->name(); - child_row[columns.pgrp] = (*p)->pgroup_id(); - child_row[columns.playlist] = *p; - if (*p == this_track->playlist()) { - selected_row = child_row; - have_selected = true; - } + if (!child_row) { + continue; + } + child_row[columns.text] = (*p)->name (); + child_row[columns.pgrp] = (*p)->pgroup_id (); + child_row[columns.playlist] = *p; + + if (*p == this_track->playlist ()) { + selected_row = child_row; + have_selected = true; } - } - if (have_selected) { - _ignore_selection = true; - tree.get_selection()->select (selected_row); - _ignore_selection = false; } } if (_mode != plSelect) { // Add unassigned (imported) playlists to the list list > unassigned; - _session->playlists()->unassigned (unassigned); + _session->playlists ()->unassigned (unassigned); - if ( unassigned.begin() != unassigned.end() ) { - - TreeModel::Row row; - TreeModel::Row selected_row; - bool have_selected = false; - TreePath this_path; + if (unassigned.begin () != unassigned.end ()) { + TreeModel::Row row = *(model->append ()); + row[columns.text] = _("Imported"); - row = *(model->append ()); - row[columns.text] = _("Imported"); - proxy = row[columns.playlist]; - proxy.reset (); - - for (list >::iterator p = unassigned.begin(); p != unassigned.end(); ++p) { + for (list >::iterator p = unassigned.begin (); p != unassigned.end (); ++p) { TreeModel::Row child_row; - child_row = *(model->append (row.children())); - child_row[columns.text] = (*p)->name(); - child_row[columns.pgrp] = (*p)->pgroup_id(); + child_row = *(model->append (row.children ())); + + child_row[columns.text] = (*p)->name (); + child_row[columns.pgrp] = (*p)->pgroup_id (); child_row[columns.playlist] = *p; - if (*p == this_track->playlist()) { - selected_row = child_row; + if (*p == this_track->playlist ()) { + selected_row = child_row; have_selected = true; } - - if (have_selected) { - _ignore_selection = true; - tree.get_selection()->select (selected_row); - _ignore_selection = false; - } } } - } //if !plSelect + } + + if (have_selected) { + PBD::Unwinder uw (_ignore_selection, true); + + TreeIter parent = selected_row.parent (); + while (parent) { + tree.expand_row (TreePath (parent), false); + parent = parent->parent (); + } + tree.get_selection ()->select (selected_row); + } } void -PlaylistSelector::pl_property_changed (PBD::PropertyChange const & what_changed) +PlaylistSelector::pl_property_changed (PBD::PropertyChange const& what_changed) { - redisplay(); + redisplay (); } void PlaylistSelector::add_playlist_to_map (boost::shared_ptr pl) { - if (pl->frozen()) { + if (pl->frozen ()) { return; } @@ -380,17 +360,17 @@ PlaylistSelector::add_playlist_to_map (boost::shared_ptr pl) TrackPlaylistMap::iterator x; - if ((x = trpl_map.find (pl->get_orig_track_id ())) == trpl_map.end()) { - x = trpl_map.insert (trpl_map.end(), make_pair (pl->get_orig_track_id(), new vector >)); + if ((x = _trpl_map.find (pl->get_orig_track_id ())) == _trpl_map.end ()) { + x = _trpl_map.insert (_trpl_map.end (), make_pair (pl->get_orig_track_id (), new vector >)); } x->second->push_back (pl); } void -PlaylistSelector::ok_button_click() +PlaylistSelector::ok_button_click () { - hide(); + hide (); } void @@ -401,17 +381,15 @@ PlaylistSelector::selection_changed () return; } - boost::shared_ptr pl; - - TreeModel::iterator iter = tree.get_selection()->get_selected(); + TreeModel::iterator iter = tree.get_selection ()->get_selected (); if (!iter || _rui == 0) { /* nothing selected */ return; } + boost::shared_ptr pl; if ((pl = ((*iter)[columns.playlist])) != 0) { - if (_rui->is_audio_track () && boost::dynamic_pointer_cast (pl) == 0) { return; } @@ -420,25 +398,31 @@ PlaylistSelector::selection_changed () } switch (_mode) { - /* @Robin: I dont see a way to undo these playlist actions */ - case plCopy: { - boost::shared_ptr playlist = PlaylistFactory::create (pl, string_compose ("%1.1", pl->name())); + /* @Robin: I dont see a way to undo these playlist actions + * @Ben: me neither :) + */ + case plCopy: + { + boost::shared_ptr playlist = PlaylistFactory::create (pl, string_compose ("%1.1", pl->name ())); /* playlist->reset_shares (); @Robin is this needed? */ _rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, playlist); - } break; + } + break; case plShare: - _rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl, false); /* share pl but do NOT set me as the owner */ + /* share pl but do NOT set me as the owner */ + _rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl, false); break; case plSteal: - _rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl); /* share the playlist and set ME as the owner */ + /* share the playlist and set ME as the owner */ + _rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl); break; case plSelect: - if (_scope_all_radio->get_active()) { - PublicEditor::instance().mapover_all_routes (sigc::bind (sigc::mem_fun (PublicEditor::instance(), &PublicEditor::mapped_select_playlist_matching), pl)); - } else if (_scope_rec_radio->get_active()) { - PublicEditor::instance().mapover_armed_routes (sigc::bind (sigc::mem_fun (PublicEditor::instance(), &PublicEditor::mapped_select_playlist_matching), pl)); + if (_scope_all_radio->get_active ()) { + PublicEditor::instance ().mapover_all_routes (sigc::bind (sigc::mem_fun (PublicEditor::instance (), &PublicEditor::mapped_select_playlist_matching), pl)); + } else if (_scope_rec_radio->get_active ()) { + PublicEditor::instance ().mapover_armed_routes (sigc::bind (sigc::mem_fun (PublicEditor::instance (), &PublicEditor::mapped_select_playlist_matching), pl)); } else { - PublicEditor::instance().mapover_grouped_routes (sigc::bind (sigc::mem_fun (PublicEditor::instance(), &PublicEditor::mapped_select_playlist_matching), pl), _rui, ARDOUR::Properties::group_select.property_id); + PublicEditor::instance ().mapover_grouped_routes (sigc::bind (sigc::mem_fun (PublicEditor::instance (), &PublicEditor::mapped_select_playlist_matching), pl), _rui, ARDOUR::Properties::group_select.property_id); } break; } @@ -448,17 +432,18 @@ PlaylistSelector::selection_changed () bool PlaylistSelector::on_key_press_event (GdkEventKey* ev) { - /* Allow these keys to have their in-dialog effect */ - switch (ev->keyval) { - case GDK_Up: - case GDK_Down: - return ArdourDialog::on_key_press_event (ev); + case GDK_Up: + case GDK_Down: + /* Allow these keys to have their in-dialog effect */ + return ArdourDialog::on_key_press_event (ev); + default: + break; } /* Don't just forward the key press ... make it act as if it occured in - whatever the main window currently is. - */ - Gtk::Window& main_window (ARDOUR_UI::instance()->main_window()); + * whatever the main window currently is. + */ + Gtk::Window& main_window (ARDOUR_UI::instance ()->main_window ()); return ARDOUR_UI_UTILS::relay_key_press (ev, &main_window); } diff --git a/gtk2_ardour/playlist_selector.h b/gtk2_ardour/playlist_selector.h index 0e7f068ad1..ace536dd3d 100644 --- a/gtk2_ardour/playlist_selector.h +++ b/gtk2_ardour/playlist_selector.h @@ -24,14 +24,14 @@ #include #include -#include #include -#include +#include #include +#include -#include "ardour_dialog.h" #include "ardour/playlist.h" #include "ardour/session_handle.h" +#include "ardour_dialog.h" namespace ARDOUR { class Session; @@ -67,63 +67,58 @@ public: plSteal }; - void redisplay(); - void prepare(RouteUI*, plMode in); + void redisplay (); + void prepare (RouteUI*, plMode in); protected: bool on_key_press_event (GdkEventKey*); private: - typedef std::map >*> TrackPlaylistMap; + typedef std::map >*> TrackPlaylistMap; - Gtk::ScrolledWindow scroller; - TrackPlaylistMap trpl_map; + void new_plist_button_clicked (); + void copy_plist_button_clicked (); - Gtk::HBox _scope_container; - Gtk::HBox *_scope_box; - Gtk::RadioButton *_scope_all_radio; - Gtk::RadioButton *_scope_rec_radio; - Gtk::RadioButton *_scope_grp_radio; - - Gtk::HBox _button_box; - - RouteUI* _rui; - - plMode _mode; - - ArdourWidgets::ArdourButton _btn_new_plist; - ArdourWidgets::ArdourButton _btn_copy_plist; - - void new_plist_button_clicked(); - void copy_plist_button_clicked(); - - sigc::connection select_connection; - PBD::ScopedConnectionList signal_connections; - void pl_property_changed (PBD::PropertyChange const & what_changed); + void pl_property_changed (PBD::PropertyChange const& what_changed); void add_playlist_to_map (boost::shared_ptr); - void playlist_added(); + void playlist_added (); void clear_map (); void ok_button_click (); void selection_changed (); - struct ModelColumns : public Gtk::TreeModel::ColumnRecord - { - ModelColumns () { + Gtk::HBox _scope_container; + Gtk::HBox* _scope_box; + Gtk::RadioButton* _scope_all_radio; + Gtk::RadioButton* _scope_rec_radio; + Gtk::RadioButton* _scope_grp_radio; + Gtk::HBox _button_box; + + ArdourWidgets::ArdourButton _btn_new_plist; + ArdourWidgets::ArdourButton _btn_copy_plist; + + TrackPlaylistMap _trpl_map; + RouteUI* _rui; + plMode _mode; + + struct ModelColumns : public Gtk::TreeModel::ColumnRecord { + ModelColumns () + { add (text); add (pgrp); add (playlist); } - Gtk::TreeModelColumn text; - Gtk::TreeModelColumn pgrp; - Gtk::TreeModelColumn > playlist; + Gtk::TreeModelColumn text; + Gtk::TreeModelColumn pgrp; + Gtk::TreeModelColumn > playlist; }; - ModelColumns columns; + ModelColumns columns; Glib::RefPtr model; - Gtk::TreeView tree; + Gtk::TreeView tree; - boost::shared_ptr current_playlist; + PBD::ScopedConnectionList _track_connections; + PBD::ScopedConnectionList _playlist_connections; bool _ignore_selection; };