Unbreak region brush drag wrt undo, avoid some dangling commands in the gui

	- also allow moving of automation lines in internal mouse mode.
	- this is also a first pass at ensuring that if an operation does
	  nothing, avoid an undo entry.

4 files changed, 121 insertions, 62 deletions

diff --git a/gtk2_ardour/editor_drag.cc b/gtk2_ardour/editor_drag.cc
index 229b1ae1a0..d3ed92a97c 100644
--- a/gtk2_ardour/editor_drag.cc
+++ b/gtk2_ardour/editor_drag.cc
@@ -619,6 +619,11 @@ RegionMotionDrag::start_grab (GdkEvent* event, Gdk::Cursor* cursor)
 		assert(_last_pointer_time_axis_view >= 0);
 		_last_pointer_layer = tv.first->layer_display() == Overlaid ? 0 : tv.second;
 	}
+
+	if (_brushing) {
+		/* cross track dragging seems broken here. disabled for now. */
+		_y_constrained = true;
+	}
 }
 
 double
@@ -1223,13 +1228,13 @@ void
 RegionMoveDrag::motion (GdkEvent* event, bool first_move)
 {
 	if (_copy && first_move) {
-
-		if (_x_constrained) {
+		if (_x_constrained && !_brushing) {
 			_editor->begin_reversible_command (Operations::fixed_time_region_copy);
-		} else {
+		} else if (!_brushing) {
 			_editor->begin_reversible_command (Operations::region_copy);
+		} else if (_brushing) {
+			_editor->begin_reversible_command (Operations::drag_region_brush);
 		}
-
 		/* duplicate the regionview(s) and region(s) */
 
 		list<DraggingView> new_regionviews;
@@ -1287,14 +1292,14 @@ RegionMoveDrag::motion (GdkEvent* event, bool first_move)
 		}
 
 	} else if (!_copy && first_move) {
-
-		if (_x_constrained) {
+		if (_x_constrained && !_brushing) {
 			_editor->begin_reversible_command (_("fixed time region drag"));
-		} else {
+		} else if (!_brushing) {
 			_editor->begin_reversible_command (Operations::region_drag);
+		} else if (_brushing) {
+			_editor->begin_reversible_command (Operations::drag_region_brush);
 		}
 	}
-
 	RegionMotionDrag::motion (event, first_move);
 }
 
@@ -1502,12 +1507,6 @@ RegionMoveDrag::finished_no_copy (
 	set<RouteTimeAxisView*> views_to_update;
 	RouteTimeAxisView* new_time_axis_view = 0;
 
-	if (_brushing) {
-		/* all changes were made during motion event handlers */
-		_editor->commit_reversible_command ();
-		return;
-	}
-
 	typedef map<boost::shared_ptr<Playlist>, RouteTimeAxisView*> PlaylistMapping;
 	PlaylistMapping playlist_mapping;
 
@@ -1621,7 +1620,6 @@ RegionMoveDrag::finished_no_copy (
 			}
 
 			rv->region()->set_position (where);
-
 			_editor->session()->add_command (new StatefulDiffCommand (rv->region()));
 		}
 
@@ -1670,7 +1668,7 @@ RegionMoveDrag::finished_no_copy (
 
 	/* write commands for the accumulated diffs for all our modified playlists */
 	add_stateful_diff_commands_for_playlists (modified_playlists);
-
+	/* applies to _brushing */
 	_editor->commit_reversible_command ();
 
 	/* We have futzed with the layering of canvas items on our streamviews.
diff --git a/gtk2_ardour/editor_mouse.cc b/gtk2_ardour/editor_mouse.cc
index 5c106785e2..8c5e6711f9 100644
--- a/gtk2_ardour/editor_mouse.cc
+++ b/gtk2_ardour/editor_mouse.cc
@@ -766,6 +766,11 @@ Editor::button_press_handler_1 (ArdourCanvas::Item* item, GdkEvent* event, ItemT
 			return true;
 			break;
 
+		case AutomationLineItem:
+			_drags->set (new LineDrag (this, item), event);
+			return true;
+			break;
+
 		case StreamItem:
 			//in the past, we created a new midi region here, but perhaps that is best left to the Draw mode
 			break;
@@ -2200,7 +2205,7 @@ Editor::mouse_brush_insert_region (RegionView* rv, framepos_t pos)
 
 	// playlist is frozen, so we have to update manually XXX this is disgusting
 
-	playlist->RegionAdded (new_region); /* EMIT SIGNAL */
+	//playlist->RegionAdded (new_region); /* EMIT SIGNAL */
 }
 
 gint
@@ -2262,8 +2267,6 @@ Editor::add_region_brush_drag (ArdourCanvas::Item* item, GdkEvent*, RegionView*
 	}
 
 	_drags->add (new RegionMoveDrag (this, item, region_view, selection->regions.by_layer(), true, false));
-
-	begin_reversible_command (Operations::drag_region_brush);
 }
 
 /** Start a grab where a time range is selected, track(s) are selected, and the
@@ -2303,7 +2306,6 @@ Editor::start_selection_grab (ArdourCanvas::Item* /*item*/, GdkEvent* event)
 	/* A selection grab currently creates two undo/redo operations, one for
 	   creating the new region and another for moving it.
 	*/
-
 	begin_reversible_command (Operations::selection_grab);
 
 	boost::shared_ptr<Playlist> playlist = clicked_axisview->playlist();
@@ -2316,6 +2318,7 @@ Editor::start_selection_grab (ArdourCanvas::Item* /*item*/, GdkEvent* event)
 
 	if (latest_regionviews.empty()) {
 		/* something went wrong */
+		abort_reversible_command ();
 		return;
 	}
 
diff --git a/gtk2_ardour/editor_ops.cc b/gtk2_ardour/editor_ops.cc
index 5c45951e04..41b7bcc8eb 100644
--- a/gtk2_ardour/editor_ops.cc
+++ b/gtk2_ardour/editor_ops.cc
@@ -336,9 +336,9 @@ Editor::move_range_selection_start_or_end_to_region_boundary (bool move_end, boo
 		return;
 	}
 
-	begin_reversible_command (_("alter selection"));
+	begin_reversible_selection_op (_("alter selection"));
 	selection->set_preserving_all_ranges (start, end);
-	commit_reversible_command ();
+	commit_reversible_selection_op ();
 }
 
 bool
@@ -2172,10 +2172,7 @@ void
 Editor::remove_location_at_playhead_cursor ()
 {
 	if (_session) {
-
 		//set up for undo
-		begin_reversible_command (_("remove marker"));
-		
 		XMLNode &before = _session->locations()->get_state();
 		bool removed = false;
 
@@ -2191,6 +2188,7 @@ Editor::remove_location_at_playhead_cursor ()
 		
 		//store undo
 		if (removed) {
+			begin_reversible_command (_("remove marker"));
 			XMLNode &after = _session->locations()->get_state();
 			_session->add_command(new MementoCommand<Locations>(*(_session->locations()), &before, &after));
 			
@@ -2238,8 +2236,6 @@ Editor::add_location_from_region ()
 		return;
 	}
 
-	begin_reversible_command (_("add marker"));
-	
 	XMLNode &before = _session->locations()->get_state();
 
 	string markername;
@@ -2256,6 +2252,7 @@ Editor::add_location_from_region ()
 		return;
 	}
 
+	begin_reversible_command (_("add marker"));
 	// single range spanning all selected
 	Location *location = new Location (*_session, selection->regions.start(), selection->regions.end_frame(), markername, Location::IsRangeMarker);
 	_session->locations()->add (location, true);
@@ -3302,6 +3299,7 @@ void
 Editor::region_fill_track ()
 {
 	RegionSelection rs = get_regions_from_selection_and_entered ();
+	bool commit = false;
 
 	if (!_session || rs.empty()) {
 		return;
@@ -3318,21 +3316,27 @@ Editor::region_fill_track ()
 		boost::shared_ptr<Playlist> pl = region->playlist();
 
 		if (end <= region->last_frame()) {
-			return;
+			goto out;
 		}
 
 		double times = (double) (end - region->last_frame()) / (double) region->length();
 
 		if (times == 0) {
-			return;
+			goto out;
 		}
 
 		pl->clear_changes ();
 		pl->add_region (RegionFactory::create (region, true), region->last_frame(), times);
 		_session->add_command (new StatefulDiffCommand (pl));
+		commit = true;
 	}
 
-	commit_reversible_command ();
+out:
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -3362,6 +3366,7 @@ Editor::region_fill_selection ()
 
 	framepos_t selection_length = end - start;
 	float times = (float)selection_length / region->length();
+	bool commit = false;
 
 	begin_reversible_command (Operations::fill_selection);
 
@@ -3376,9 +3381,14 @@ Editor::region_fill_selection ()
 		playlist->clear_changes ();
 		playlist->add_region (RegionFactory::create (region, true), start, times);
 		_session->add_command (new StatefulDiffCommand (playlist));
+		commit = true;
 	}
 
-	commit_reversible_command ();
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -3730,7 +3740,7 @@ void
 Editor::trim_to_region(bool forward)
 {
 	RegionSelection rs = get_regions_from_selection_and_entered ();
-
+	bool commit = false;
 	begin_reversible_command (_("trim to region"));
 
 	boost::shared_ptr<Region> next_region;
@@ -3746,7 +3756,7 @@ Editor::trim_to_region(bool forward)
 		AudioTimeAxisView* atav = dynamic_cast<AudioTimeAxisView*> (&arv->get_time_axis_view());
 
 		if (!atav) {
-			return;
+			continue;
 		}
 
 		float speed = 1.0;
@@ -3786,9 +3796,14 @@ Editor::trim_to_region(bool forward)
 		}
 
 		_session->add_command(new StatefulDiffCommand (region));
+		commit = true;
 	}
 
-	commit_reversible_command ();
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -3927,7 +3942,7 @@ Editor::bounce_range_selection (bool replace, bool enable_processing)
 		boost::shared_ptr<Playlist> playlist;
 
 		if ((playlist = rtv->playlist()) == 0) {
-			return;
+			continue;
 		}
 
 		InterThreadInfo itt;
@@ -5281,6 +5296,7 @@ void
 Editor::apply_filter (Filter& filter, string command, ProgressReporter* progress)
 {
 	RegionSelection rs = get_regions_from_selection_and_entered ();
+	bool commit = false;
 
 	if (rs.empty()) {
 		return;
@@ -5331,7 +5347,6 @@ Editor::apply_filter (Filter& filter, string command, ProgressReporter* progress
 					}
 
 				}
-
 				/* We might have removed regions, which alters other regions' layering_index,
 				   so we need to do a recursive diff here.
 				*/
@@ -5340,8 +5355,9 @@ Editor::apply_filter (Filter& filter, string command, ProgressReporter* progress
 				_session->add_commands (cmds);
 				
 				_session->add_command(new StatefulDiffCommand (playlist));
+				commit = true;
 			} else {
-				return;
+				goto out;
 			}
 
 			if (progress) {
@@ -5352,8 +5368,12 @@ Editor::apply_filter (Filter& filter, string command, ProgressReporter* progress
 		r = tmp;
 		++n;
 	}
-
-	commit_reversible_command ();
+out:
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -5643,6 +5663,7 @@ Editor::set_fade_length (bool in)
 	framepos_t pos = get_preferred_edit_position();
 	framepos_t len;
 	char const * cmd;
+	bool commit = false;
 
 	if (pos > rv->region()->last_frame() || pos < rv->region()->first_frame()) {
 		/* edit point is outside the relevant region */
@@ -5671,7 +5692,7 @@ Editor::set_fade_length (bool in)
 		AudioRegionView* tmp = dynamic_cast<AudioRegionView*> (*x);
 
 		if (!tmp) {
-			return;
+			continue;
 		}
 
 		boost::shared_ptr<AutomationList> alist;
@@ -5690,12 +5711,16 @@ Editor::set_fade_length (bool in)
 			tmp->audio_region()->set_fade_out_length (len);
 			tmp->audio_region()->set_fade_out_active (true);
 		}
-
 		XMLNode &after = alist->get_state();
 		_session->add_command(new MementoCommand<AutomationList>(*alist, &before, &after));
+		commit = true;
 	}
 
-	commit_reversible_command ();
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -5706,14 +5731,14 @@ Editor::set_fade_in_shape (FadeShape shape)
 	if (rs.empty()) {
 		return;
 	}
-
+	bool commit = false;
 	begin_reversible_command (_("set fade in shape"));
 
 	for (RegionSelection::iterator x = rs.begin(); x != rs.end(); ++x) {
 		AudioRegionView* tmp = dynamic_cast<AudioRegionView*> (*x);
 
 		if (!tmp) {
-			return;
+			continue;
 		}
 
 		boost::shared_ptr<AutomationList> alist = tmp->audio_region()->fade_in();
@@ -5723,10 +5748,14 @@ Editor::set_fade_in_shape (FadeShape shape)
 
 		XMLNode &after = alist->get_state();
 		_session->add_command(new MementoCommand<AutomationList>(*alist.get(), &before, &after));
+		commit = true;
 	}
 
-	commit_reversible_command ();
-
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -5737,14 +5766,14 @@ Editor::set_fade_out_shape (FadeShape shape)
 	if (rs.empty()) {
 		return;
 	}
-
+	bool commit = false;
 	begin_reversible_command (_("set fade out shape"));
 
 	for (RegionSelection::iterator x = rs.begin(); x != rs.end(); ++x) {
 		AudioRegionView* tmp = dynamic_cast<AudioRegionView*> (*x);
 
 		if (!tmp) {
-			return;
+			continue;
 		}
 
 		boost::shared_ptr<AutomationList> alist = tmp->audio_region()->fade_out();
@@ -5754,9 +5783,14 @@ Editor::set_fade_out_shape (FadeShape shape)
 
 		XMLNode &after = alist->get_state();
 		_session->add_command(new MementoCommand<AutomationList>(*alist.get(), &before, &after));
+		commit = true;
 	}
 
-	commit_reversible_command ();
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -5767,14 +5801,14 @@ Editor::set_fade_in_active (bool yn)
 	if (rs.empty()) {
 		return;
 	}
-
+	bool commit = false;
 	begin_reversible_command (_("set fade in active"));
 
 	for (RegionSelection::iterator x = rs.begin(); x != rs.end(); ++x) {
 		AudioRegionView* tmp = dynamic_cast<AudioRegionView*> (*x);
 
 		if (!tmp) {
-			return;
+			continue;
 		}
 
 
@@ -5783,9 +5817,14 @@ Editor::set_fade_in_active (bool yn)
 		ar->clear_changes ();
 		ar->set_fade_in_active (yn);
 		_session->add_command (new StatefulDiffCommand (ar));
+		commit = true;
 	}
 
-	commit_reversible_command ();
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -5796,14 +5835,14 @@ Editor::set_fade_out_active (bool yn)
 	if (rs.empty()) {
 		return;
 	}
-
+	bool commit = false;
 	begin_reversible_command (_("set fade out active"));
 
 	for (RegionSelection::iterator x = rs.begin(); x != rs.end(); ++x) {
 		AudioRegionView* tmp = dynamic_cast<AudioRegionView*> (*x);
 
 		if (!tmp) {
-			return;
+			continue;
 		}
 
 		boost::shared_ptr<AudioRegion> ar (tmp->audio_region());
@@ -5811,9 +5850,14 @@ Editor::set_fade_out_active (bool yn)
 		ar->clear_changes ();
 		ar->set_fade_out_active (yn);
 		_session->add_command(new StatefulDiffCommand (ar));
+		commit = true;
 	}
 
-	commit_reversible_command ();
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 }
 
 void
@@ -5849,11 +5893,15 @@ Editor::toggle_region_fades (int dir)
 	}
 
 	/* XXX should this undo-able? */
+	bool commit = false;
+	begin_reversible_command (_("toggle fade active"));
 
 	for (RegionSelection::iterator i = rs.begin(); i != rs.end(); ++i) {
 		if ((ar = boost::dynamic_pointer_cast<AudioRegion>((*i)->region())) == 0) {
 			continue;
 		}
+		ar->clear_changes ();
+
 		if (dir == 1 || dir == 0) {
 			ar->set_fade_in_active (!yn);
 		}
@@ -5861,6 +5909,14 @@ Editor::toggle_region_fades (int dir)
 		if (dir == -1 || dir == 0) {
 			ar->set_fade_out_active (!yn);
 		}
+		_session->add_command(new StatefulDiffCommand (ar));
+		commit = true;
+	}
+
+	if (commit) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
 	}
 }
 
@@ -6958,12 +7014,11 @@ Editor::insert_time (
 	bool all_playlists, bool ignore_music_glue, bool markers_too, bool glued_markers_too, bool locked_markers_too, bool tempo_too
 	)
 {
-	bool commit = false;
 
 	if (Config->get_edit_mode() == Lock) {
 		return;
 	}
-
+	bool commit = false;
 	begin_reversible_command (_("insert time"));
 
 	TrackViewList ts = selection->tracks.filter_to_unique_playlists ();
@@ -7016,7 +7071,6 @@ Editor::insert_time (
 		RouteTimeAxisView* rtav = dynamic_cast<RouteTimeAxisView*> (*x);
 		if (rtav) {
 			rtav->route ()->shift (pos, frames);
-			commit = true;
 		}
 	}
 
@@ -7055,6 +7109,7 @@ Editor::insert_time (
 		if (moved) {
 			XMLNode& after (_session->locations()->get_state());
 			_session->add_command (new MementoCommand<Locations>(*_session->locations(), &before, &after));
+			commit = true;
 		}
 	}
 
@@ -7064,6 +7119,8 @@ Editor::insert_time (
 
 	if (commit) {
 		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
 	}
 }
 void
@@ -7131,13 +7188,11 @@ void
 Editor::cut_time (framepos_t pos, framecnt_t frames, InsertTimeOption opt, 
 		     bool ignore_music_glue, bool markers_too, bool tempo_too)
 {
-	bool commit = false;
-	
 	if (Config->get_edit_mode() == Lock) {
 		error << (_("Cannot insert or delete time when in Lock edit.")) << endmsg;
 		return;
 	}
-
+	bool commit = false;
 	begin_reversible_command (_("cut time"));
 
 	for (TrackSelection::iterator x = selection->tracks.begin(); x != selection->tracks.end(); ++x) {
@@ -7164,7 +7219,6 @@ Editor::cut_time (framepos_t pos, framecnt_t frames, InsertTimeOption opt,
 		RouteTimeAxisView* rtav = dynamic_cast<RouteTimeAxisView*> (*x);
 		if (rtav) {
 			rtav->route ()->shift (pos, -frames);
-			commit = true;
 		}
 	}
 
@@ -7233,6 +7287,8 @@ Editor::cut_time (framepos_t pos, framecnt_t frames, InsertTimeOption opt,
 	
 	if (commit) {
 		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
 	}
 }
 
diff --git a/gtk2_ardour/editor_timefx.cc b/gtk2_ardour/editor_timefx.cc
index 2025ec70c3..5908e50249 100644
--- a/gtk2_ardour/editor_timefx.cc
+++ b/gtk2_ardour/editor_timefx.cc
@@ -133,6 +133,8 @@ Editor::pitch_shift (RegionSelection& regions, float fraction)
 
 	if (ret == 0) {
 		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
 	}
 
 	return ret;