From f1d784afbb0e1bcb19cd7b9fec32aa68c0c624bd Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Fri, 10 Feb 2023 11:08:50 -0700 Subject: [PATCH] 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 | 102 +++++++++++++++++++++------------ gtk2_ardour/automation_line.h | 14 ++--- gtk2_ardour/editor_drag.cc | 75 ++++++++++++++---------- gtk2_ardour/editor_drag.h | 2 + 4 files changed, 118 insertions(+), 75 deletions(-) diff --git a/gtk2_ardour/automation_line.cc b/gtk2_ardour/automation_line.cc index 020597b406..c258544b7b 100644 --- a/gtk2_ardour/automation_line.cc +++ b/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 += e.sample_to_pixel_unrounded (64); + before_x = (*line.nth (front()->view_index() - 1)->model())->when; + 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 -= e.sample_to_pixel_unrounded (64); + after_x = (*line.nth (back()->view_index() + 1)->model())->when; + after_x.shift_earlier (timepos_t (64)); } } } -double -AutomationLine::ContiguousControlPoints::clamp_dx (double dx, double region_limit) +Temporal::timecnt_t +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* cp; + ControlPoint* reference_point; - 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.); - tx = min (tx, region_limit); + /* Now clamp that position so that: + * + * - 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 - tx = min (tx, after_x); // can't move earlier than preceding point + possible_pos = max (possible_pos, Temporal::timepos_t (possible_pos.time_domain())); + 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::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); - line.reset_line_coords (**i); + cp->move_to (line.dt_to_dx ((*cp->model())->when, dt), view_y, ControlPoint::Full); + 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 -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 (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)) { - const timepos_t rl (maximum_time() + _offset); - double region_limit = trackview.editor().duration_to_pixels_unrounded (timecnt_t (rl, get_origin())); + if (dt.is_negative() || (dt.is_positive() && !with_push)) { + const timepos_t line_limit = get_origin() + maximum_time() + _offset; 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::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; diff --git a/gtk2_ardour/automation_line.h b/gtk2_ardour/automation_line.h index 492dd1a954..4eba5dbc5f 100644 --- a/gtk2_ardour/automation_line.h +++ b/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, float, XMLNode *); - virtual std::pair drag_motion (double, float, bool, bool with_push, uint32_t& final_index); + virtual std::pair 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 { public: ContiguousControlPoints (AutomationLine& al); - double clamp_dx (double dx, double region_limit); - void move (double dx, double dvalue); + Temporal::timecnt_t clamp_dt (Temporal::timecnt_t const & dx, Temporal::timepos_t const & region_limit); + void move (Temporal::timecnt_t const &, double dvalue); void compute_x_bounds (PublicEditor& e); private: AutomationLine& line; - double before_x; - double after_x; + Temporal::timepos_t before_x; + Temporal::timepos_t after_x; }; friend class ContiguousControlPoints; @@ -221,8 +223,6 @@ private: std::list _drag_points; ///< points we are dragging std::list _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 diff --git a/gtk2_ardour/editor_drag.cc b/gtk2_ardour/editor_drag.cc index d0ce08a462..aa3e798af0 100644 --- a/gtk2_ardour/editor_drag.cc +++ b/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 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 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 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)); } } diff --git a/gtk2_ardour/editor_drag.h b/gtk2_ardour/editor_drag.h index 60341acba1..f2308c17f6 100644 --- a/gtk2_ardour/editor_drag.h +++ b/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 */