fix for automation control point drags: prevent overlapping points in time.

* control points that were already closer than one_tick_in_pixels
 were able to move beyond adjacent points, resulting in out-of-order lines

* prior code was using 'one tick' as the smallest spacing between two
 CPs, but that is larger than the default 'guard point' spacing.  this
 resulted in odd behavior because of the dxt calculation when you moved
 a point that was created as a 'guard point'

For now, use the same 64 samples we use for 'guard points'.  And change
 the 'dxt' calculation logic to more aggressively limit the points from
 overlapping.

TODO:  we might decide that 'one tick' should be the minimum automation
 period throughout ardour.  In that case we should change guard-points.

gtk2_ardour/automation_line.cc

@@ -503,13 +503,7 @@ AutomationLine::ContiguousControlPoints::compute_x_bounds (PublicEditor& e)
 
 		if (front()->view_index() > 0) {
 			before_x = line.nth (front()->view_index() - 1)->get_x();
-
-			const samplepos_t pos = e.pixel_to_sample(before_x);
-			const TempoMetric& metric = map->metric_at (pos);
-			const samplecnt_t len = ceil (metric.samples_per_bar (pos) / (Temporal::ticks_per_beat * metric.meter().divisions_per_bar()));
-			const double one_tick_in_pixels = e.sample_to_pixel_unrounded (len);
-
-			before_x += one_tick_in_pixels;
+			before_x += e.sample_to_pixel_unrounded (64);
 		}
 
 		/* if our last point has a point after it in the line,
@@ -518,13 +512,7 @@ AutomationLine::ContiguousControlPoints::compute_x_bounds (PublicEditor& e)
 
 		if (back()->view_index() < (line.npoints() - 1)) {
 			after_x = line.nth (back()->view_index() + 1)->get_x();
-
-			const samplepos_t pos = e.pixel_to_sample(after_x);
-			const TempoMetric& metric = map->metric_at (pos);
-			const samplecnt_t len = ceil (metric.samples_per_bar (pos) / (Temporal::ticks_per_beat * metric.meter().divisions_per_bar()));
-			const double one_tick_in_pixels = e.sample_to_pixel_unrounded (len);
-
-			after_x -= one_tick_in_pixels;
+			after_x -= e.sample_to_pixel_unrounded (64);
 		}
 	}
 }
@@ -650,10 +638,7 @@ AutomationLine::drag_motion (double const x, float fraction, bool ignore_x, bool
 
 	if (dx < 0 || ((dx > 0) && !with_push)) {
 		for (vector<CCP>::iterator ccp = contiguous_points.begin(); ccp != contiguous_points.end(); ++ccp) {
-			double dxt = (*ccp)->clamp_dx (dx);
-			if (fabs (dxt) < fabs (dx)) {
-				dx = dxt;
-			}
+			dx = (*ccp)->clamp_dx (dx);
 		}
 	}
 

gtk2_ardour/editor_drag.cc

@@ -6393,12 +6393,13 @@ NoteDrag::aborted (bool)
 }
 
 /** Make an AutomationRangeDrag for lines in an AutomationTimeAxisView */
-AutomationRangeDrag::AutomationRangeDrag (Editor* editor, AutomationTimeAxisView* atv, list<TimelineRange> const & r)
+AutomationRangeDrag::AutomationRangeDrag (Editor* editor, AutomationTimeAxisView* atv, float initial_value, list<TimelineRange> const & r)
 	: Drag (editor, atv->base_item (), editor->default_time_domain()) /* XXX NUTEMPO FIX TIME DOMAIN */
 	, _ranges (r)
 	, _y_origin (atv->y_position())
 	, _y_height (atv->effective_height()) // or atv->lines()->front()->height() ?!
 	, _nothing_to_drag (false)
+	, _initial_value (initial_value)
 {
 	DEBUG_TRACE (DEBUG::Drags, "New AutomationRangeDrag\n");
 	setup (atv->lines ());
@@ -6486,6 +6487,10 @@ AutomationRangeDrag::y_fraction (double global_y) const
 double
 AutomationRangeDrag::value (boost::shared_ptr<AutomationList> list, timepos_t const & x) const
 {
+	if (list->size () == 0) {
+		return _initial_value;
+	}
+
 	const double v = list->eval(x);
 	return _integral ? rint(v) : v;
 }
@@ -6562,9 +6567,14 @@ AutomationRangeDrag::motion (GdkEvent*, bool first_move)
 					p.set_time_domain (the_list->time_domain());
 					q.set_time_domain (the_list->time_domain());
 
+					/* get start&end values to use for guard points *before* we add points to the list */
+					/* in the case where no data exists on the line, p_value = q_value = initial_value */
+					float p_value = value (the_list, p);
+					float q_value = value (the_list, q);
+
 					XMLNode &before = the_list->get_state();
-					bool const add_p = the_list->editor_add (p, value (the_list, p), false);
-					bool const add_q = the_list->editor_add (q, value (the_list, q), false);
+					bool const add_p = the_list->editor_add (p, p_value, false);
+					bool const add_q = the_list->editor_add (q, q_value, false);
 
 					if (add_p || add_q) {
 						_editor->session()->add_command (

gtk2_ardour/editor_drag.h

@@ -1362,7 +1362,7 @@ private:
 class AutomationRangeDrag : public Drag
 {
 public:
-	AutomationRangeDrag (Editor *, AutomationTimeAxisView *, std::list<ARDOUR::TimelineRange> const &);
+	AutomationRangeDrag (Editor *, AutomationTimeAxisView *, float initial_value, std::list<ARDOUR::TimelineRange> const &);
 	AutomationRangeDrag (Editor *, std::list<RegionView*> const &, std::list<ARDOUR::TimelineRange> const &, double y_origin, double y_height);
 
 	void start_grab (GdkEvent *, Gdk::Cursor* c = 0);
@@ -1394,6 +1394,7 @@ private:
 	double          _y_height;
 	bool            _nothing_to_drag;
 	bool            _integral;
+	float_t			_initial_value;
 };
 
 /** Drag of one edge of an xfade

gtk2_ardour/editor_mouse.cc

@@ -1214,8 +1214,10 @@ Editor::button_press_handler_1 (ArdourCanvas::Item* item, GdkEvent* event, ItemT
 				/* handle automation lanes first */
 				AutomationTimeAxisView* atv = dynamic_cast<AutomationTimeAxisView*> (tvp.first);
 				if (atv) {
-					/* smart "join" mode: drag automation */
-					_drags->set (new AutomationRangeDrag (this, atv, selection->time), event, _cursors->up_down);
+					/* if there's no line yet, AutomationRangeDrag will need to be told what the initial value of this control is */
+					float init_value = atv->control()->get_value();
+
+					_drags->set (new AutomationRangeDrag (this, atv, init_value, selection->time), event, _cursors->up_down);
 					return true;
 				}
 				if (dynamic_cast<AutomationRegionView*>(clicked_regionview)) {

libs/evoral/ControlList.cc

@@ -562,9 +562,7 @@ ControlList::add_guard_point (timepos_t const & time, timecnt_t const & offset)
 		return;
 	}
 
-	assert (offset <= timecnt_t());
-
-	if (!offset.is_zero()) {
+	if (offset.is_negative()) {
 		/* check if there are points between when + offset .. when */
 		ControlEvent cp (when + offset, 0.0);
 		iterator s;
@@ -577,6 +575,19 @@ ControlList::add_guard_point (timepos_t const & time, timecnt_t const & offset)
 				return;
 			}
 		}
+	} else {
+		/* check if there are points between when + offset .. when */
+		ControlEvent cp (when + offset, 0.0);
+		iterator s;
+		iterator e;
+		if ((s = upper_bound (_events.begin(), _events.end(), &cp, time_comparator)) != _events.end()) {
+			cp.when = when;
+			e = upper_bound (_events.begin(), _events.end(), &cp, time_comparator);
+			if (s != e) {
+				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 add_guard_point, none added, found event between %2 and %3\n", this, when.earlier (offset), when));
+				return;
+			}
+		}
 	}
 
 	/* don't do this again till the next write pass,
@@ -1890,7 +1901,7 @@ ControlList::cut_copy_clear (timepos_t const & start_time, timepos_t const & end
 
 			double val = unlocked_eval (start);
 
-			if (op == 0) { // cut
+			if (op != 1) { // cut/clear
 				if (start > _events.front()->when) {
 					_events.insert (s, (new ControlEvent (start, val)));
 				}
@@ -1920,15 +1931,12 @@ ControlList::cut_copy_clear (timepos_t const & start_time, timepos_t const & end
 
 		if (e == _events.end() || (*e)->when != end) {
 
-			/* only add a boundary point if there is a point after "end"
-			 */
-
-			if (op == 0 && (e != _events.end() && end < (*e)->when)) { // cut
+			if (op != 1) { // cut/clear
 				_events.insert (e, new ControlEvent (end, end_value));
 			}
 
-			if (op != 2 && (e != _events.end() && end < (*e)->when)) { // cut/copy
-				nal->_events.push_back (new ControlEvent (timepos_t (start.distance (start)), end_value));
+			if (op != 2) { // cut/copy
+				nal->_events.push_back (new ControlEvent (timepos_t (start.distance (end)), end_value));
 			}
 		}
 
@@ -1970,6 +1978,11 @@ ControlList::paste (const ControlList& alist, timepos_t const &  time)
 		return false;
 	}
 
+	/* when pasting a range of automation, first add guard points so the automation data before and after this range is retained */
+	const ControlEvent* last = alist.back();
+	add_guard_point (time, -GUARD_POINT_DELTA);
+	add_guard_point (time + last->when, GUARD_POINT_DELTA);
+
 	{
 		Glib::Threads::RWLock::WriterLock lm (_lock);
 		iterator where;