deep fix to the way automation control point drags are handled/computed

The old code could not snap to the grid, because it had a lot of confusion about pixels vs. time,
and between line-origin-relative time and absolute time

gtk2_ardour/automation_line.cc

@@ -485,7 +485,7 @@ struct ControlPointSorter
 };
 
 AutomationLine::ContiguousControlPoints::ContiguousControlPoints (AutomationLine& al)
-	: line (al), before_x (0), after_x (DBL_MAX)
+	: line (al), before_x (timepos_t (line.the_list()->time_domain())), after_x (timepos_t::max (line.the_list()->time_domain()))
 {
 }
 
@@ -502,8 +502,8 @@ AutomationLine::ContiguousControlPoints::compute_x_bounds (PublicEditor& e)
 		*/
 
 		if (front()->view_index() > 0) {
-			before_x = line.nth (front()->view_index() - 1)->get_x();
+			before_x = (*line.nth (front()->view_index() - 1)->model())->when;
-			before_x += e.sample_to_pixel_unrounded (64);
+			before_x += timepos_t (64);
 		}
 
 		/* if our last point has a point after it in the line,
@@ -511,57 +511,65 @@ AutomationLine::ContiguousControlPoints::compute_x_bounds (PublicEditor& e)
 		*/
 
 		if (back()->view_index() < (line.npoints() - 1)) {
-			after_x = line.nth (back()->view_index() + 1)->get_x();
+			after_x = (*line.nth (back()->view_index() + 1)->model())->when;
-			after_x -= e.sample_to_pixel_unrounded (64);
+			after_x.shift_earlier (timepos_t (64));
 		}
 	}
 }
 
-double
+Temporal::timecnt_t
-AutomationLine::ContiguousControlPoints::clamp_dx (double dx, double region_limit)
+AutomationLine::ContiguousControlPoints::clamp_dt (timecnt_t const & dt, timepos_t const & line_limit)
 {
 	if (empty()) {
-		return dx;
+		return dt;
 	}
 
 	/* get the maximum distance we can move any of these points along the x-axis
 	 */
 
-	double tx; /* possible position a point would move to, given dx */
+	ControlPoint* reference_point;
-	ControlPoint* cp;
 
-	if (dx > 0) {
+	if (dt.magnitude() > 0) {
 		/* check the last point, since we're moving later in time */
-		cp = back();
+		reference_point = back();
 	} else {
 		/* check the first point, since we're moving earlier in time */
-		cp = front();
+		reference_point = front();
 	}
 
-	tx = cp->get_x() + dx; // new possible position if we just add the motion
+	/* possible position the "reference" point would move to, given dx */
+	Temporal::timepos_t possible_pos = (*reference_point->model())->when + dt; // new possible position if we just add the motion
 
-	tx = max (tx, 0.);
+	/* Now clamp that position so that:
-	tx = min (tx, region_limit);
+	 *
+	 * - it is not before the origin (zero)
+	 * - it is not beyond the line's own limit (e.g. for region automation)
+	 * - it is not before the preceding point
+	 * - it is not after the folloing point
+	 */
 
-	tx = max (tx, before_x); // can't move later than following point
+	possible_pos = max (possible_pos, Temporal::timepos_t (possible_pos.time_domain()));
-	tx = min (tx, after_x);  // can't move earlier than preceding point
+	possible_pos = min (possible_pos, line_limit);
 
-	return  tx - cp->get_x ();
+	possible_pos = max (possible_pos, before_x); // can't move later than following point
+	possible_pos = min (possible_pos, after_x);  // can't move earlier than preceding point
+
+	return (*reference_point->model())->when.distance (possible_pos);
 }
 
 void
-AutomationLine::ContiguousControlPoints::move (double dx, double dvalue)
+AutomationLine::ContiguousControlPoints::move (timecnt_t const & dt, double dvalue)
 {
-	for (std::list<ControlPoint*>::iterator i = begin(); i != end(); ++i) {
+	for (auto & cp : *this) {
 		// compute y-axis delta
-		double view_y = 1.0 - (*i)->get_y() / line.height();
+		double view_y = 1.0 - cp->get_y() / line.height();
 		line.view_to_model_coord_y (view_y);
 		line.apply_delta (view_y, dvalue);
 		view_y = line.model_to_view_coord_y (view_y);
 		view_y = (1.0 - view_y) * line.height();
 
-		(*i)->move_to ((*i)->get_x() + dx, view_y, ControlPoint::Full);
+		cp->move_to (line.dt_to_dx ((*cp->model())->when, dt), view_y, ControlPoint::Full);
-		line.reset_line_coords (**i);
+		line.reset_line_coords (*cp);
 	}
 }
 
@@ -572,8 +580,6 @@ AutomationLine::ContiguousControlPoints::move (double dx, double dvalue)
 void
 AutomationLine::start_drag_common (double x, float fraction)
 {
-	_drag_x = x;
-	_drag_distance = 0;
 	_last_drag_fraction = fraction;
 	_drag_had_movement = false;
 	did_push = false;
@@ -583,20 +589,39 @@ AutomationLine::start_drag_common (double x, float fraction)
 	_drag_points.sort (ControlPointSorter());
 }
 
+/** Takes a relative-to-origin position, moves it by dt, and returns a
+ *  relative-to-origin pixel count.
+ */
+double
+AutomationLine::dt_to_dx (timepos_t const & pos, timecnt_t const & dt)
+{
+	/* convert a shift of pos by dt into an absolute timepos */
+	timepos_t const new_pos ((pos + dt + get_origin()).shift_earlier (offset()));
+	/* convert to pixels */
+	double px = trackview.editor().time_to_pixel_unrounded (new_pos);
+	/* convert back to pixels-relative-to-origin */
+	px -= trackview.editor().time_to_pixel_unrounded (get_origin());
+	return px;
+}
 
 /** Should be called to indicate motion during a drag.
- *  @param x New x position of the drag in canvas units, or undefined if ignore_x == true.
+ *  @param x New x position of the drag in canvas units relative to origin, or undefined if ignore_x == true.
  *  @param fraction New y fraction.
  *  @return x position and y fraction that were actually used (once clamped).
  */
 pair<float, float>
-AutomationLine::drag_motion (double const x, float fraction, bool ignore_x, bool with_push, uint32_t& final_index)
+AutomationLine::drag_motion (timecnt_t const & pdt, float fraction, bool ignore_x, bool with_push, uint32_t& final_index)
 {
 	if (_drag_points.empty()) {
 		return pair<double,float> (fraction, _desc.is_linear () ? 0 : 1);
 	}
 
-	double dx = ignore_x ? 0 : (x - _drag_x);
+	timecnt_t dt (pdt);
+
+	if (ignore_x) {
+		dt = timecnt_t (pdt.time_domain());
+	}
+
 	double dy = fraction - _last_drag_fraction;
 
 	if (!_drag_had_movement) {
@@ -641,11 +666,10 @@ AutomationLine::drag_motion (double const x, float fraction, bool ignore_x, bool
 	 * since all later points will move too.
 	 */
 
-	if (dx < 0 || ((dx > 0) && !with_push)) {
+	if (dt.is_negative() || (dt.is_positive() && !with_push)) {
-		const timepos_t rl (maximum_time() + _offset);
+		const timepos_t line_limit = get_origin() + maximum_time() + _offset;
-		double region_limit = trackview.editor().duration_to_pixels_unrounded (timecnt_t (rl, get_origin()));
 		for (auto const & ccp : contiguous_points){
-			dx = ccp->clamp_dx (dx, region_limit);
+			dt = ccp->clamp_dt (dt, line_limit);
 		}
 	}
 
@@ -679,18 +703,22 @@ AutomationLine::drag_motion (double const x, float fraction, bool ignore_x, bool
 		}
 	}
 
-	if (dx || dy) {
+	if (!dt.is_zero() || dy) {
 		/* and now move each section */
+
+
 		for (vector<CCP>::iterator ccp = contiguous_points.begin(); ccp != contiguous_points.end(); ++ccp) {
-			(*ccp)->move (dx, delta_value);
+			(*ccp)->move (dt, delta_value);
 		}
 
 		if (with_push) {
 			final_index = contiguous_points.back()->back()->view_index () + 1;
 			ControlPoint* p;
 			uint32_t i = final_index;
+
 			while ((p = nth (i)) != 0 && p->can_slide()) {
-				p->move_to (p->get_x() + dx, p->get_y(), ControlPoint::Full);
+
+				p->move_to (dt_to_dx ((*p->model())->when, dt), p->get_y(), ControlPoint::Full);
 				reset_line_coords (*p);
 				++i;
 			}
@@ -716,8 +744,6 @@ AutomationLine::drag_motion (double const x, float fraction, bool ignore_x, bool
 	}
 
 	double const result_frac = _last_drag_fraction + dy;
-	_drag_distance += dx;
-	_drag_x += dx;
 	_last_drag_fraction = result_frac;
 	did_push = with_push;
 

gtk2_ardour/automation_line.h

@@ -92,7 +92,7 @@ public:
 	virtual void start_drag_single (ControlPoint*, double, float);
 	virtual void start_drag_line (uint32_t, uint32_t, float);
 	virtual void start_drag_multiple (std::list<ControlPoint*>, float, XMLNode *);
-	virtual std::pair<float, float> drag_motion (double, float, bool, bool with_push, uint32_t& final_index);
+	virtual std::pair<float, float> drag_motion (Temporal::timecnt_t const &, float, bool, bool with_push, uint32_t& final_index);
 	virtual void end_drag (bool with_push, uint32_t final_index);
 
 	ControlPoint* nth (uint32_t);
@@ -167,6 +167,8 @@ public:
 	Temporal::timepos_t session_position (Temporal::timepos_t const &) const;
 	void dump (std::ostream&) const;
 
+	double dt_to_dx (Temporal::timepos_t const &, Temporal::timecnt_t const &);
+
 protected:
 
 	std::string    _name;
@@ -194,13 +196,13 @@ protected:
 	class ContiguousControlPoints : public std::list<ControlPoint*> {
 public:
 		ContiguousControlPoints (AutomationLine& al);
-		double clamp_dx (double dx, double region_limit);
+		Temporal::timecnt_t clamp_dt (Temporal::timecnt_t const & dx, Temporal::timepos_t const & region_limit);
-		void move (double dx, double dvalue);
+		void move (Temporal::timecnt_t const &, double dvalue);
 		void compute_x_bounds (PublicEditor& e);
 private:
 		AutomationLine& line;
-		double before_x;
+		Temporal::timepos_t before_x;
-		double after_x;
+		Temporal::timepos_t after_x;
 	};
 
 	friend class ContiguousControlPoints;
@@ -221,8 +223,6 @@ private:
 	std::list<ControlPoint*> _drag_points; ///< points we are dragging
 	std::list<ControlPoint*> _push_points; ///< additional points we are dragging if "push" is enabled
 	bool _drag_had_movement; ///< true if the drag has seen movement, otherwise false
-	double _drag_x; ///< last x position of the drag, in units
-	double _drag_distance; ///< total x movement of the drag, in canvas units
 	double _last_drag_fraction; ///< last y position of the drag, as a fraction
 	/** offset from the start of the automation list to the start of the line, so that
 	 *  a +ve offset means that the 0 on the line is at _offset in the list

gtk2_ardour/editor_drag.cc

@@ -4718,6 +4718,30 @@ ControlPointDrag::ControlPointDrag (Editor* e, ArdourCanvas::Item* i)
 	set_time_domain (_point->line().the_list()->time_domain());
 }
 
+Temporal::timecnt_t
+ControlPointDrag::total_dt (GdkEvent* event) const
+{
+	if (_x_constrained) {
+		return timecnt_t::zero (Temporal::BeatTime);
+	}
+
+	/* x-axis delta in absolute samples, because we can't do any better */
+	timecnt_t const dx = timecnt_t (pixel_to_time (_drags->current_pointer_x() - grab_x()), _point->line().get_origin());
+
+	/* control point time in absolute time, using natural time domain */
+	timepos_t const point_absolute = (*_point->model())->when + _point->line().get_origin().shift_earlier (_point->line().offset());
+
+	/* Now adjust the absolute time by dx, and snap
+	 */
+	timepos_t snap = point_absolute + dx + snap_delta (event->button.state);
+	_editor->snap_to_with_modifier (snap, event);
+
+	/* Now measure the distance between the actual point position and
+	 * dragged one (possibly snapped), then subtract the snap delta again.
+	 */
+
+	return timecnt_t (point_absolute.distance (snap) - snap_delta (event->button.state));
+}
 
 void
 ControlPointDrag::start_grab (GdkEvent* event, Gdk::Cursor* /*cursor*/)
@@ -4726,6 +4750,13 @@ ControlPointDrag::start_grab (GdkEvent* event, Gdk::Cursor* /*cursor*/)
 
 	// start the grab at the center of the control point so
 	// the point doesn't 'jump' to the mouse after the first drag
+
+	/* The point coordinates are in canvas-item-relative space, so x==9
+	 * represents the start of the line. That start could be absolute zero
+	 * (for a track-level automation line) or the position of a region on
+	 * the timline (e.g. for MIDI CC data exposed as automation)
+	 */
+
 	_fixed_grab_x = _point->get_x() + _editor->time_to_pixel_unrounded (timepos_t (_point->line().offset()));
 	_fixed_grab_y = _point->get_y();
 
@@ -4746,58 +4777,42 @@ ControlPointDrag::start_grab (GdkEvent* event, Gdk::Cursor* /*cursor*/)
 void
 ControlPointDrag::motion (GdkEvent* event, bool first_motion)
 {
-	double dx = _drags->current_pointer_x() - last_pointer_x();
+	/* First y */
+
 	double dy = current_pointer_y() - last_pointer_y();
-	bool need_snap = true;
 
 	if (Keyboard::modifier_state_equals (event->button.state, ArdourKeyboard::fine_adjust_modifier ())) {
-		dx *= 0.1;
 		dy *= 0.1;
-		need_snap = false;
 	}
 
-	/* coordinate in pixels relative to the start of the region (for region-based automation)
-	   or track (for track-based automation) */
-	double cx = _fixed_grab_x + _cumulative_x_drag + dx;
 	double cy = _fixed_grab_y + _cumulative_y_drag + dy;
-
-	// calculate zero crossing point. back off by .01 to stay on the
-	// positive side of zero
 	double const zero_gain_y = (1.0 - _zero_gain_fraction) * _point->line().height() - .01;
 
-	if (!_point->can_slide ()) {
-		cx = _fixed_grab_x;
-	}
 	if (_y_constrained) {
 		cy = _fixed_grab_y;
 	}
 
-	_cumulative_x_drag = cx - _fixed_grab_x;
 	_cumulative_y_drag = cy - _fixed_grab_y;
 
-	cx = max (0.0, cx);
 	cy = max (0.0, cy);
 	cy = min ((double) _point->line().height(), cy);
 
 	// make sure we hit zero when passing through
+
 	if ((cy < zero_gain_y && (cy - dy) > zero_gain_y) || (cy > zero_gain_y && (cy - dy) < zero_gain_y)) {
 		cy = zero_gain_y;
 	}
 
-	/* cx_pos is in absolute timeline units */
-	timepos_t cx_pos (timepos_t (pixel_to_time (cx)) + snap_delta (event->button.state));
-
-	if (need_snap) {
-		_editor->snap_to_with_modifier (cx_pos, event);
-	}
-
-	cx_pos.shift_earlier (snap_delta (event->button.state));
-
-	/* total number of pixels (canvas window units) to move */
-	double px = _editor->time_to_pixel_unrounded (cx_pos);
-
 	float const fraction = 1.0 - (cy / _point->line().height());
 
+	/* Now x axis */
+
+	timecnt_t dt;
+
+	if (_point->can_slide ()) {
+		dt = total_dt (event);
+	}
+
 	if (first_motion) {
 		float const initial_fraction = 1.0 - (_fixed_grab_y / _point->line().height());
 		_editor->begin_reversible_command (_("automation event move"));
@@ -4805,7 +4820,7 @@ ControlPointDrag::motion (GdkEvent* event, bool first_motion)
 	}
 
 	pair<float, float> result;
-	result = _point->line().drag_motion (px, fraction, false, _pushing, _final_index);
+	result = _point->line().drag_motion (dt, fraction, false, _pushing, _final_index);
 	show_verbose_cursor_text (_point->line().get_verbose_cursor_relative_string (result.first, result.second));
 }
 
@@ -4941,7 +4956,7 @@ LineDrag::motion (GdkEvent* event, bool first_move)
 	/* we are ignoring x position for this drag, so we can just pass in anything */
 	pair<float, float> result;
 
-	result = _line->drag_motion (0, fraction, true, false, ignored);
+	result = _line->drag_motion (timecnt_t (time_domain()), fraction, true, false, ignored);
 	show_verbose_cursor_text (_line->get_verbose_cursor_relative_string (result.first, result.second));
 }
 
@@ -6514,7 +6529,7 @@ AutomationRangeDrag::motion (GdkEvent*, bool first_move)
 		/* we are ignoring x position for this drag, so we can just pass in anything */
 		pair<float, float> result;
 		uint32_t ignored;
-		result = l->line->drag_motion (0, f, true, false, ignored);
+		result = l->line->drag_motion (timecnt_t (time_domain()), f, true, false, ignored);
 		show_verbose_cursor_text (l->line->get_verbose_cursor_relative_string (result.first, result.second));
 	}
 }

gtk2_ardour/editor_drag.h

@@ -1151,6 +1151,8 @@ private:
 	bool     _pushing;
         uint32_t _final_index;
 	static double _zero_gain_fraction;
+
+	Temporal::timecnt_t total_dt (GdkEvent*) const;
 };
 
 /** Gain or automation line drag */