Playlist UI tweaks: refactor PlaylistSelector (life-cycle, code-cleanup)

* You can now show a selector for each track, rather than one global dialog
* Removed the 'cancel' button: it was too easy to unintentionally revert your selection
* Correctly sort and name new playlists when they are added or renamed from the P menu
* Fix problem where list contents disappeared when changing desktops (on_unmap_event)

gtk2_ardour/editor.cc

@@ -139,7 +139,6 @@
 #include "mixer_ui.h"
 #include "mouse_cursors.h"
 #include "note_base.h"
-#include "playlist_selector.h"
 #include "public_editor.h"
 #include "quantize_dialog.h"
 #include "region_peak_cursor.h"
@@ -254,7 +253,6 @@ Editor::Editor ()
 	: PublicEditor (global_hpacker)
 	, editor_mixer_strip_width (Wide)
 	, constructed (false)
-	, _playlist_selector (0)
 	, _time_info_box (0)
 	, no_save_visual (false)
 	, _leftmost_sample (0)
@@ -783,9 +781,6 @@ Editor::Editor ()
 
 	setup_toolbar ();
 
-	_playlist_selector = new PlaylistSelector();
-	_playlist_selector->signal_delete_event().connect (sigc::bind (sigc::ptr_fun (just_hide_it), static_cast<Window *> (_playlist_selector)));
-
 	RegionView::RegionViewGoingAway.connect (*this, invalidator (*this),  boost::bind (&Editor::catch_vanishing_regionview, this, _1), gui_context());
 
 	/* nudge stuff */
@@ -883,7 +878,6 @@ Editor::~Editor()
 	delete _regions;
 	delete _snapshots;
 	delete _locations;
-	delete _playlist_selector;
 	delete _time_info_box;
 	delete selection;
 	delete cut_buffer;
@@ -1306,7 +1300,6 @@ Editor::set_session (Session *t)
 	 * before the visible state has been loaded from instant.xml */
 	_leftmost_sample = session_gui_extents().first;
 
-	_playlist_selector->set_session (_session);
 	nudge_clock->set_session (_session);
 	_summary->set_session (_session);
 	_group_tabs->set_session (_session);
@@ -4058,12 +4051,6 @@ Editor::set_show_touched_automation (bool yn)
 	instant_save ();
 }
 
-PlaylistSelector&
-Editor::playlist_selector () const
-{
-	return *_playlist_selector;
-}
-
 samplecnt_t
 Editor::get_paste_offset (samplepos_t pos, unsigned paste_count, samplecnt_t duration)
 {

gtk2_ardour/editor.h

@@ -136,7 +136,6 @@ class MidiExportDialog;
 class MixerStrip;
 class MouseCursors;
 class NoteBase;
-class PlaylistSelector;
 class PluginSelector;
 class ProgressReporter;
 class QuantizeDialog;
@@ -331,7 +330,6 @@ public:
 
 	/* stuff that AudioTimeAxisView and related classes use */
 
-	PlaylistSelector& playlist_selector() const;
 	void clear_playlist (boost::shared_ptr<ARDOUR::Playlist>);
 
 	void clear_grouped_playlists (RouteUI* v);
@@ -607,8 +605,6 @@ private:
 	// to keep track of the playhead position for control_scroll
 	boost::optional<samplepos_t> _control_scroll_target;
 
-	PlaylistSelector* _playlist_selector;
-
 	TimeInfoBox*      _time_info_box;
 
 	typedef std::pair<TimeAxisView*,XMLNode*> TAVState;

gtk2_ardour/playlist_selector.cc

@@ -51,7 +51,7 @@ using namespace PBD;
 PlaylistSelector::PlaylistSelector ()
 	: ArdourDialog (_("Playlists"))
 {
-	_tav = 0;
+	_rui = 0;
 	_mode = plSelect;
 
 	set_name ("PlaylistSelectorWindow");
@@ -74,29 +74,34 @@ PlaylistSelector::PlaylistSelector ()
 
 	get_vbox()->show_all();
 
-	Button* close_btn = add_button (Gtk::Stock::CANCEL, RESPONSE_CANCEL);
 	Button* ok_btn = add_button (Gtk::Stock::OK, RESPONSE_OK);
-	close_btn->signal_clicked().connect (sigc::mem_fun(*this, &PlaylistSelector::close_button_click));
 	ok_btn->signal_clicked().connect (sigc::mem_fun(*this, &PlaylistSelector::ok_button_click));
 }
 
-void PlaylistSelector::set_tav(RouteTimeAxisView* tavx, plMode mode)
+void PlaylistSelector::prepare(RouteUI* ruix, plMode mode)
 {
 	_mode = mode;
 	
-	if (_tav == tavx) {
+	if (_rui == ruix) {
 		return;
 	}
 
-	_tav = tavx;
+	_rui = ruix;
 
-	boost::shared_ptr<Track> this_track = _tav->track();
+	boost::shared_ptr<Track> this_track = _rui->track();
 
 	if (this_track) {
+		this_track->PlaylistChanged.connect(
+			signal_connections,
+			invalidator(*this),
+			boost::bind(&PlaylistSelector::redisplay, this),
+			gui_context()
+		);
+
 		this_track->PlaylistAdded.connect(
 			signal_connections,
 			invalidator(*this),
-			boost::bind(&PlaylistSelector::playlist_added, this),
+			boost::bind(&PlaylistSelector::redisplay, this),
 			gui_context()
 		);
 
@@ -107,11 +112,23 @@ void PlaylistSelector::set_tav(RouteTimeAxisView* tavx, plMode mode)
 			gui_context()
 		);
 	}
+
+	redisplay();
 }
 
 PlaylistSelector::~PlaylistSelector ()
 {
-	clear_map ();
+	clear_map();
+
+	if (model) {
+		model->clear ();
+	}
+	if (current_playlist) {
+		current_playlist.reset();
+	}
+
+	signal_connections.drop_connections();
+	select_connection.disconnect ();
 }
 
 void
@@ -121,41 +138,28 @@ PlaylistSelector::clear_map ()
 		delete x->second;
 	}
 	trpl_map.clear ();
-	if (current_playlist) {
-		current_playlist.reset();
-	}
-}
-
-bool
-PlaylistSelector::on_unmap_event (GdkEventAny* ev)
-{
-	clear_map ();
-	if (model) {
-		model->clear ();
-	}
-	return Dialog::on_unmap_event (ev);
 }
 
 void
 PlaylistSelector::redisplay()
 {
-	if (!_tav ) {
+	if (!_rui ) {
 		return;
 	}
 
 	vector<const char*> item;
 	string str;
 
-	set_title (string_compose (_("Playlist for %1"), _tav->route()->name()));
+	set_title (string_compose (_("Playlist for %1"), _rui->route()->name()));
 
 	clear_map ();
-	select_connection.disconnect ();
+	if (model) {
-
 		model->clear ();
+	}
 
 	_session->playlists()->foreach (this, &PlaylistSelector::add_playlist_to_map);
 
-	boost::shared_ptr<Track> this_track = _tav->track();
+	boost::shared_ptr<Track> this_track = _rui->track();
 
 	boost::shared_ptr<Playlist> proxy;
 
@@ -199,6 +203,8 @@ PlaylistSelector::redisplay()
 
 		for (vector<boost::shared_ptr<Playlist> >::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());
+
 			TreeModel::Row child_row;
 
 			if (tr == this_track && _mode==plSelect) {
@@ -258,10 +264,15 @@ PlaylistSelector::redisplay()
 		}
 	} //if !plSelect
 	
-	show_all ();
 	select_connection = tree.get_selection()->signal_changed().connect (sigc::mem_fun(*this, &PlaylistSelector::selection_changed));
 }
 
+void
+PlaylistSelector::pl_property_changed (PBD::PropertyChange const & what_changed)
+{
+	redisplay();
+}
+
 void
 PlaylistSelector::add_playlist_to_map (boost::shared_ptr<Playlist> pl)
 {
@@ -269,16 +280,16 @@ PlaylistSelector::add_playlist_to_map (boost::shared_ptr<Playlist> pl)
 		return;
 	}
 
-	if (!_tav) {
+	if (!_rui) {
 		return;
 	}
 
-	if (_tav->is_midi_track ()) {
+	if (_rui->is_midi_track ()) {
 		if (boost::dynamic_pointer_cast<MidiPlaylist> (pl) == 0) {
 			return;
 		}
 	} else {
-		assert (_tav->is_audio_track ());
+		assert (_rui->is_audio_track ());
 		if (boost::dynamic_pointer_cast<AudioPlaylist> (pl) == 0) {
 			return;
 		}
@@ -293,37 +304,12 @@ PlaylistSelector::add_playlist_to_map (boost::shared_ptr<Playlist> pl)
 	x->second->push_back (pl);
 }
 
-void
-PlaylistSelector::playlist_added()
-{
-	redisplay();
-}
-
-void
-PlaylistSelector::close_button_click ()
-{
-	if (_tav && current_playlist) {
-		_tav->track ()->use_playlist (_tav->is_audio_track () ? DataType::AUDIO : DataType::MIDI, current_playlist);
-	}
-	_tav = 0;
-	clear_map ();
-	hide ();
-}
-
 void
 PlaylistSelector::ok_button_click()
 {
-	_tav = 0;
-	clear_map ();
 	hide();
 }
 
-bool PlaylistSelector::on_delete_event (GdkEventAny*)
-{
-	close_button_click();
-	return false;
-}
-
 void
 PlaylistSelector::selection_changed ()
 {
@@ -331,17 +317,17 @@ PlaylistSelector::selection_changed ()
 
 	TreeModel::iterator iter = tree.get_selection()->get_selected();
 
-	if (!iter || _tav == 0) {
+	if (!iter || _rui == 0) {
 		/* nothing selected */
 		return;
 	}
 
 	if ((pl = ((*iter)[columns.playlist])) != 0) {
 
-		if (_tav->is_audio_track () && boost::dynamic_pointer_cast<AudioPlaylist> (pl) == 0) {
+		if (_rui->is_audio_track () && boost::dynamic_pointer_cast<AudioPlaylist> (pl) == 0) {
 			return;
 		}
-		if (_tav->is_midi_track () && boost::dynamic_pointer_cast<MidiPlaylist> (pl) == 0) {
+		if (_rui->is_midi_track () && boost::dynamic_pointer_cast<MidiPlaylist> (pl) == 0) {
 			return;
 		}
 
@@ -350,16 +336,16 @@ PlaylistSelector::selection_changed ()
 			case plCopy: {
 					boost::shared_ptr<Playlist> playlist = PlaylistFactory::create (pl, string_compose ("%1.1", pl->name()));
 					/* playlist->reset_shares ();  @Robin is this needed? */
-					_tav->track ()->use_playlist (_tav->is_audio_track () ? DataType::AUDIO : DataType::MIDI, playlist);
+					_rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, playlist);
 				} break;
 			case plShare:
-				_tav->track ()->use_playlist (_tav->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl, false);  /* share pl but do NOT set me as the owner */
+				_rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl, false);  /* share pl but do NOT set me as the owner */
 				break;
 			case plSteal:
-				_tav->track ()->use_playlist (_tav->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl); /* share the playlist and set ME as the owner */
+				_rui->track ()->use_playlist (_rui->is_audio_track () ? DataType::AUDIO : DataType::MIDI, pl); /* share the playlist and set ME as the owner */
 				break;
 			case plSelect:
-				_tav->use_playlist (NULL, pl);  //call route_ui::use_playlist because it is group-aware
+				_rui->use_playlist (NULL, pl);  //call route_ui::use_playlist because it is group-aware
 				break;
 		}
 	}

gtk2_ardour/playlist_selector.h

@@ -40,6 +40,7 @@ namespace ARDOUR {
 }
 
 class RouteUI;
+class RouteTimeAxisView;
 
 struct PlaylistSorterByID {
 	bool operator() (boost::shared_ptr<ARDOUR::Playlist> a, boost::shared_ptr<ARDOUR::Playlist> b) const {
@@ -67,10 +68,9 @@ public:
 	};
 
 	void redisplay();
-	void set_tav(RouteTimeAxisView*, plMode in);
+	void prepare(RouteUI*, plMode in);
 
 protected:
-	bool on_unmap_event (GdkEventAny*);
 	bool on_key_press_event (GdkEventKey*);
 
 private:
@@ -79,20 +79,19 @@ private:
 	Gtk::ScrolledWindow scroller;
 	TrackPlaylistMap trpl_map;
 
-	RouteTimeAxisView* _tav;
+	RouteUI* _rui;
 
 	plMode _mode;
 
 	sigc::connection select_connection;
 	PBD::ScopedConnectionList signal_connections;
+	void pl_property_changed (PBD::PropertyChange const & what_changed);
 
 	void add_playlist_to_map (boost::shared_ptr<ARDOUR::Playlist>);
 	void playlist_added();
 	void clear_map ();
-	void close_button_click ();
 	void ok_button_click ();
 	void selection_changed ();
-	bool on_delete_event (GdkEventAny*);
 
 	struct ModelColumns : public Gtk::TreeModel::ColumnRecord
 	{

gtk2_ardour/public_editor.h

@@ -89,7 +89,6 @@ class ArdourMarker;
 class MeterMarker;
 class MixerStrip;
 class MouseCursors;
-class PlaylistSelector;
 class RegionView;
 class RouteTimeAxisView;
 class Selection;
@@ -288,7 +287,6 @@ public:
 	virtual Editing::ZoomFocus get_zoom_focus () const = 0;
 	virtual samplecnt_t get_current_zoom () const = 0;
 	virtual void reset_zoom (samplecnt_t) = 0;
-	virtual PlaylistSelector& playlist_selector() const = 0;
 	virtual void clear_playlist (boost::shared_ptr<ARDOUR::Playlist>) = 0;
 	virtual void clear_grouped_playlists (RouteUI*) = 0;
 

gtk2_ardour/route_ui.cc

@@ -168,6 +168,7 @@ RouteUI::~RouteUI()
 	delete _record_menu;
 	delete _comment_window;
 	delete _invert_menu;
+	delete _playlist_selector;
 
 	send_blink_connection.disconnect ();
 	rec_blink_connection.disconnect ();
@@ -177,6 +178,7 @@ void
 RouteUI::init ()
 {
 	self_destruct = true;
+	_playlist_selector = 0;
 	mute_menu = 0;
 	solo_menu = 0;
 	sends_menu = 0;
@@ -2637,41 +2639,49 @@ RouteUI::use_playlist (RadioMenuItem *item, boost::weak_ptr<Playlist> wpl)
 void
 RouteUI::show_playlist_selector ()
 {
-	RouteTimeAxisView* rtv = dynamic_cast<RouteTimeAxisView*> (this);
+	if (!_playlist_selector) {
-	if (rtv) {
+		_playlist_selector = new PlaylistSelector();
-		ARDOUR_UI::instance()->the_editor().playlist_selector().set_tav(rtv, PlaylistSelector::plSelect);
+		_playlist_selector->set_session(_session);
-		ARDOUR_UI::instance()->the_editor().playlist_selector().redisplay ();
 	}
+
+	_playlist_selector->prepare(this, PlaylistSelector::plSelect);
+	_playlist_selector->show_all ();
 }
 
 void
 RouteUI::show_playlist_copy_selector ()
 {
-	RouteTimeAxisView* rtv = dynamic_cast<RouteTimeAxisView*> (this);
+	if (!_playlist_selector) {
-	if (rtv) {
+		_playlist_selector = new PlaylistSelector();
-		ARDOUR_UI::instance()->the_editor().playlist_selector().set_tav(rtv, PlaylistSelector::plCopy);
+		_playlist_selector->set_session(_session);
-		ARDOUR_UI::instance()->the_editor().playlist_selector().redisplay ();
 	}
+
+	_playlist_selector->prepare(this, PlaylistSelector::plCopy);
+	_playlist_selector->show_all ();
 }
 
 void
 RouteUI::show_playlist_share_selector ()
 {
-	RouteTimeAxisView* rtv = dynamic_cast<RouteTimeAxisView*> (this);
+	if (!_playlist_selector) {
-	if (rtv) {
+		_playlist_selector = new PlaylistSelector();
-		ARDOUR_UI::instance()->the_editor().playlist_selector().set_tav(rtv, PlaylistSelector::plShare);
+		_playlist_selector->set_session(_session);
-		ARDOUR_UI::instance()->the_editor().playlist_selector().redisplay ();
 	}
+
+	_playlist_selector->prepare(this, PlaylistSelector::plShare);
+	_playlist_selector->show_all ();
 }
 
 void
 RouteUI::show_playlist_steal_selector ()
 {
-	RouteTimeAxisView* rtv = dynamic_cast<RouteTimeAxisView*> (this);
+	if (!_playlist_selector) {
-	if (rtv) {
+		_playlist_selector = new PlaylistSelector();
-		ARDOUR_UI::instance()->the_editor().playlist_selector().set_tav(rtv, PlaylistSelector::plSteal);
+		_playlist_selector->set_session(_session);
-		ARDOUR_UI::instance()->the_editor().playlist_selector().redisplay ();
 	}
+
+	_playlist_selector->prepare(this, PlaylistSelector::plSteal);
+	_playlist_selector->show_all ();
 }
 
 void

gtk2_ardour/route_ui.h

@@ -69,6 +69,7 @@ namespace ArdourWidgets {
 class ArdourWindow;
 class IOSelectorWindow;
 class PatchChangeGridDialog;
+class PlaylistSelector;
 class SaveTemplateDialog;
 
 class RoutePinWindowProxy : public WM::ProxyBase
@@ -307,6 +308,8 @@ private:
 
 	std::string resolve_new_group_playlist_name (std::string const&, std::vector<boost::shared_ptr<ARDOUR::Playlist> > const&);
 
+	PlaylistSelector*  _playlist_selector;
+
 	Gtk::Menu*     _record_menu;
 	ArdourWindow*  _comment_window;
 	Gtk::TextView* _comment_area;

libs/ardour/ardour/playlist.h

@@ -111,7 +111,7 @@ public:
 	void set_region_ownership ();
 
 	std::string pgroup_id()                 { return _pgroup_id; }
-	void set_pgroup_id(std::string pgid)    { _pgroup_id = pgid; }
+	void set_pgroup_id(std::string pgid)    { _pgroup_id = pgid; PropertyChanged (Properties::name); }
 
 	virtual void clear (bool with_signals = true);
 	virtual void dump () const;