Hopefully fix up automation control point selection (finally).

git-svn-id: svn://localhost/ardour2/branches/3.0@7592 d708f5d6-7413-0410-9779-e7cbd77b26cf

11 files changed, 170 insertions, 130 deletions

diff --git a/gtk2_ardour/automation_line.cc b/gtk2_ardour/automation_line.cc
index 9a22c1ebad..bf8b5c51b1 100644
--- a/gtk2_ardour/automation_line.cc
+++ b/gtk2_ardour/automation_line.cc
@@ -41,7 +41,6 @@
 #include "selection.h"
 #include "time_axis_view.h"
 #include "point_selection.h"
-#include "automation_selectable.h"
 #include "automation_time_axis.h"
 #include "public_editor.h"
 
@@ -204,6 +203,16 @@ AutomationLine::nth (uint32_t n)
 	}
 }
 
+ControlPoint const *
+AutomationLine::nth (uint32_t n) const
+{
+	if (n < control_points.size()) {
+		return control_points[n];
+	} else {
+		return 0;
+	}
+}
+
 void
 AutomationLine::modify_point_y (ControlPoint& cp, double y)
 {
@@ -582,9 +591,9 @@ AutomationLine::start_drag_single (ControlPoint* cp, double x, float fraction)
 	_drag_points.clear ();
 	_drag_points.push_back (cp);
 
-	if (cp->selected ()) {
+	if (cp->get_selected ()) {
 		for (vector<ControlPoint*>::iterator i = control_points.begin(); i != control_points.end(); ++i) {
-			if (*i != cp && (*i)->selected()) {
+			if (*i != cp && (*i)->get_selected()) {
 				_drag_points.push_back (*i);
 			}
 		}
@@ -952,70 +961,25 @@ AutomationLine::remove_point (ControlPoint& cp)
  *  @param end End position in session frames.
  *  @param bot Bottom y range, as a fraction of line height, where 0 is the bottom of the line.
  *  @param top Top y range, as a fraction of line height, where 0 is the bottom of the line.
- *  @param result Filled in with selectable things.
+ *  @param result Filled in with selectable things; in this case, ControlPoints.
  */
 void
 AutomationLine::get_selectables (
 	framepos_t start, framepos_t end, double botfrac, double topfrac, list<Selectable*>& results
 	)
 {
-	/* these two are in AutomationList model coordinates */
-	double nstart;
-	double nend;
-	
-	bool collecting = false;
-
 	/* convert fractions to display coordinates with 0 at the top of the track */
 	double const bot_track = (1 - topfrac) * trackview.current_height ();
 	double const top_track = (1 - botfrac) * trackview.current_height ();
 
-	nstart = DBL_MAX;
-	nend = 0;
-
 	for (vector<ControlPoint*>::iterator i = control_points.begin(); i != control_points.end(); ++i) {
 		double const model_when = (*(*i)->model())->when;
 		framepos_t const session_frames_when = _time_converter.to (model_when) + _time_converter.origin_b ();
 
-		if (session_frames_when >= start && session_frames_when <= end) {
-
-			if ((*i)->get_y() >= bot_track && (*i)->get_y() <= top_track) {
-
-				(*i)->show();
-				(*i)->set_visible(true);
-				collecting = true;
-				nstart = min (nstart, model_when);
-				nend = max (nend, model_when);
-
-			} else {
-
-				if (collecting) {
-
-					AutomationSelectable* s = new AutomationSelectable (nstart, nend, botfrac, topfrac, &trackview);
-					PointSelection& ps = trackview.editor().get_selection().points;
-					if (find (ps.begin(), ps.end(), *s) != ps.end()) {
-						s->set_selected (true);
-					}
-
-					results.push_back (s);
-					collecting = false;
-					nstart = DBL_MAX;
-					nend = 0;
-				}
-			}
+		if (session_frames_when >= start && session_frames_when <= end && (*i)->get_y() >= bot_track && (*i)->get_y() <= top_track) {
+			results.push_back (*i);
 		}
 	}
-
-	if (collecting) {
-		AutomationSelectable* s = new AutomationSelectable (nstart, nend, botfrac, topfrac, &trackview);
-		
-		PointSelection& ps = trackview.editor().get_selection().points;
-		if (find (ps.begin(), ps.end(), *s) != ps.end()) {
-			s->set_selected (true);
-		}
-		
-		results.push_back (s);
-	}
-
 }
 
 void
@@ -1184,7 +1148,7 @@ AutomationLine::hide_all_but_selected_control_points ()
 	points_visible = false;
 
 	for (vector<ControlPoint*>::iterator i = control_points.begin(); i != control_points.end(); ++i) {
-		if (!(*i)->selected()) {
+		if (!(*i)->get_selected()) {
 			(*i)->set_visible (false);
 		}
 	}
diff --git a/gtk2_ardour/automation_line.h b/gtk2_ardour/automation_line.h
index 7377ab92a0..6578e12ea4 100644
--- a/gtk2_ardour/automation_line.h
+++ b/gtk2_ardour/automation_line.h
@@ -82,6 +82,7 @@ class AutomationLine : public sigc::trackable, public PBD::StatefulDestructible
 	virtual void end_drag ();
 
 	ControlPoint* nth (uint32_t);
+	ControlPoint const * nth (uint32_t) const;
 	uint32_t npoints() const { return control_points.size(); }
 
 	std::string  name()    const { return _name; }
diff --git a/gtk2_ardour/automation_selectable.h b/gtk2_ardour/automation_range.h
index 2d5c6fc4b3..e13ad378a5 100644
--- a/gtk2_ardour/automation_selectable.h
+++ b/gtk2_ardour/automation_range.h
@@ -17,14 +17,12 @@
 
 */
 
-#ifndef __ardour_gtk_automation_selectable_h__
-#define __ardour_gtk_automation_selectable_h__
-
-#include "selectable.h"
+#ifndef __ardour_gtk_automation_range_h__
+#define __ardour_gtk_automation_range_h__
 
 class TimeAxisView;
 
-/** One or more selected automation points, expressed as a rectangle.
+/** A rectangular range of an automation line, used to express a selected area.
  *
  *  x coordinates start/end are in AutomationList model coordinates.
  *  y coordinates are a expressed as a fraction of the AutomationTimeAxisView's height, where 0 is the
@@ -35,8 +33,10 @@ class TimeAxisView;
  *  visible; it is not trivial to convert from one of these to the
  *  other, so the AutomationSelectable is a kind of "best and worst of
  *  both worlds".
+ *
+ *  It offers a zoom-independent representation of a selected area of automation.
  */
-struct AutomationSelectable : public Selectable
+struct AutomationRange
 {
 	double start;
 	double end;
@@ -44,16 +44,8 @@ struct AutomationSelectable : public Selectable
 	double high_fract;
 	TimeAxisView* track; // ref would be better, but ARDOUR::SessionHandlePtr is non-assignable
 	
-	AutomationSelectable (double s, double e, double l, double h, TimeAxisView* atv)
+	AutomationRange (double s, double e, double l, double h, TimeAxisView* atv)
 		: start (s), end (e), low_fract (l), high_fract (h), track (atv) {}
-	
-	bool operator== (const AutomationSelectable& other) {
-		return start == other.start &&
-			end == other.end &&
-			low_fract == other.low_fract &&
-			high_fract == other.high_fract &&
-			track == other.track;
-	}
 };
 
-#endif /* __ardour_gtk_automation_selectable_h__ */
+#endif /* __ardour_gtk_automation_range_h__ */
diff --git a/gtk2_ardour/automation_time_axis.cc b/gtk2_ardour/automation_time_axis.cc
index b9edc4ef63..0eaa1aefc8 100644
--- a/gtk2_ardour/automation_time_axis.cc
+++ b/gtk2_ardour/automation_time_axis.cc
@@ -35,7 +35,6 @@
 #include "simplerect.h"
 #include "selection.h"
 #include "rgb_macros.h"
-#include "automation_selectable.h"
 #include "point_selection.h"
 #include "canvas_impl.h"
 #include "utils.h"
diff --git a/gtk2_ardour/control_point.cc b/gtk2_ardour/control_point.cc
index f40a809a4a..7632642adc 100644
--- a/gtk2_ardour/control_point.cc
+++ b/gtk2_ardour/control_point.cc
@@ -40,7 +40,6 @@ ControlPoint::ControlPoint (AutomationLine& al)
 	_y = 0;
 	_shape = Full;
 	_size = 4.0;
-	_selected = false;
 
 	_item = new Canvas::SimpleRect (_line.canvas_group());
 	_item->property_draw() = true;
@@ -69,7 +68,6 @@ ControlPoint::ControlPoint (const ControlPoint& other, bool /*dummy_arg_to_force
 	_y = other._y;
 	_shape = other._shape;
 	_size = other._size;
-	_selected = false;
 
 	_item = new Canvas::SimpleRect (_line.canvas_group());
 	_item->property_fill() = false;
diff --git a/gtk2_ardour/control_point.h b/gtk2_ardour/control_point.h
index 0264015f93..2dbf225d86 100644
--- a/gtk2_ardour/control_point.h
+++ b/gtk2_ardour/control_point.h
@@ -26,6 +26,7 @@
 
 #include "canvas.h"
 #include "simplerect.h"
+#include "selectable.h"
 
 class AutomationLine;
 class ControlPoint;
@@ -42,7 +43,7 @@ namespace Gnome {
 	}
 }
 
-class ControlPoint
+class ControlPoint : public Selectable
 {
   public:
 	ControlPoint (AutomationLine& al);
@@ -74,8 +75,6 @@ class ControlPoint
 
 	bool     can_slide() const          { return _can_slide; }
 	void     set_can_slide(bool yn)     { _can_slide = yn; }
-	bool     selected() const           { return _selected; }
-	void     set_selected(bool yn)      { _selected = yn; }
 	uint32_t view_index() const         { return _view_index; }
 	void     set_view_index(uint32_t i) { _view_index = i; }
 
@@ -92,7 +91,6 @@ class ControlPoint
 	ARDOUR::AutomationList::iterator _model;
 	uint32_t _view_index;
 	bool     _can_slide;
-	bool     _selected;
 
 	virtual bool event_handler (GdkEvent*);
 
diff --git a/gtk2_ardour/editor_mouse.cc b/gtk2_ardour/editor_mouse.cc
index 6a3eaa9084..576279290b 100644
--- a/gtk2_ardour/editor_mouse.cc
+++ b/gtk2_ardour/editor_mouse.cc
@@ -1685,7 +1685,7 @@ Editor::leave_handler (ArdourCanvas::Item* item, GdkEvent* event, ItemType item_
 	case ControlPointItem:
 		cp = reinterpret_cast<ControlPoint*>(item->get_data ("control_point"));
 		if (cp->line().the_list()->interpolation() != AutomationList::Discrete) {
-			if (cp->line().npoints() > 1 && !cp->selected()) {
+			if (cp->line().npoints() > 1 && !cp->get_selected()) {
 				cp->set_visible (false);
 			}
 		}
diff --git a/gtk2_ardour/editor_selection.cc b/gtk2_ardour/editor_selection.cc
index a72d595b2f..7569c3e0a7 100644
--- a/gtk2_ardour/editor_selection.cc
+++ b/gtk2_ardour/editor_selection.cc
@@ -240,30 +240,23 @@ Editor::set_selected_control_point_from_click (Selection::Operation op, bool /*n
 	if (!clicked_control_point) {
 		return false;
 	}
-
-	/* We know the ControlPoint that was clicked, but (as discussed in automation_selectable.h)
-	 * selected automation data are described by areas on the AutomationLine.  A ControlPoint
-	 * represents any model points in the space that it takes up, so the AutomationSelectable
-	 * needs to be the size of the ControlPoint.
-	 */
-
-	double const size = clicked_control_point->size ();
-	AutomationLine& line = clicked_control_point->line ();
 	
-	nframes64_t const x1 = pixel_to_frame (clicked_control_point->get_x() - size / 2) + line.time_converter().origin_b ();
-	nframes64_t const x2 = pixel_to_frame (clicked_control_point->get_x() + size / 2) + line.time_converter().origin_b ();
- 	double y1 = clicked_control_point->get_y() - size / 2;
-	double y2 = clicked_control_point->get_y() + size / 2;
-
-	/* convert the y values to trackview space */
-	double dummy = 0;
-	clicked_control_point->line().parent_group().i2w (dummy, y1);
-	clicked_control_point->line().parent_group().i2w (dummy, y2);
-	_trackview_group->w2i (dummy, y1);
-	_trackview_group->w2i (dummy, y2);
-
-	/* and set up the selection */
-	return select_all_within (x1, x2, y1, y2, selection->tracks, op, true);
+	switch (op) {
+	case Selection::Set:
+		selection->set (clicked_control_point);
+		break;
+	case Selection::Add:
+		selection->add (clicked_control_point);
+		break;
+	case Selection::Toggle:
+		selection->toggle (clicked_control_point);
+		break;
+	case Selection::Extend:
+		/* XXX */
+		break;
+	}
+
+	return true;
 }
 
 void
diff --git a/gtk2_ardour/point_selection.h b/gtk2_ardour/point_selection.h
index 1b442d279b..85acc90ec8 100644
--- a/gtk2_ardour/point_selection.h
+++ b/gtk2_ardour/point_selection.h
@@ -23,9 +23,9 @@
 #include <list>
 #include <boost/noncopyable.hpp>
 
-#include "automation_selectable.h"
+#include "automation_range.h"
 
-struct PointSelection : public std::list<AutomationSelectable>
+struct PointSelection : public std::list<AutomationRange>
 {
 };
 
diff --git a/gtk2_ardour/selection.cc b/gtk2_ardour/selection.cc
index 2e5be77cde..786749ce72 100644
--- a/gtk2_ardour/selection.cc
+++ b/gtk2_ardour/selection.cc
@@ -33,6 +33,7 @@
 #include "time_axis_view.h"
 #include "automation_time_axis.h"
 #include "public_editor.h"
+#include "control_point.h"
 
 #include "i18n.h"
 
@@ -817,34 +818,35 @@ Selection::empty (bool internal_selection)
 }
 
 void
-Selection::toggle (const vector<AutomationSelectable*>& autos)
+Selection::toggle (ControlPoint* cp)
 {
-	for (vector<AutomationSelectable*>::const_iterator x = autos.begin(); x != autos.end(); ++x) {
-		if ((*x)->get_selected()) {
-			points.remove (**x);
-		} else {
-			points.push_back (**x);
-		}
+	cp->set_selected (!cp->get_selected ());
+	set_point_selection_from_line (cp->line ());
+}
 
-		delete *x;
+void
+Selection::toggle (vector<ControlPoint*> const & cps)
+{
+	for (vector<ControlPoint*>::const_iterator i = cps.begin(); i != cps.end(); ++i) {
+		(*i)->set_selected (!(*i)->get_selected ());
 	}
 
-	PointsChanged (); /* EMIT SIGNAL */
+	set_point_selection_from_line (cps.front()->line ());
 }
 
 void
 Selection::toggle (list<Selectable*> const & selectables)
 {
 	RegionView* rv;
-	AutomationSelectable* as;
+	ControlPoint* cp;
 	vector<RegionView*> rvs;
-	vector<AutomationSelectable*> autos;
+	vector<ControlPoint*> cps;
 
 	for (std::list<Selectable*>::const_iterator i = selectables.begin(); i != selectables.end(); ++i) {
 		if ((rv = dynamic_cast<RegionView*> (*i)) != 0) {
 			rvs.push_back (rv);
-		} else if ((as = dynamic_cast<AutomationSelectable*> (*i)) != 0) {
-			autos.push_back (as);
+		} else if ((cp = dynamic_cast<ControlPoint*> (*i)) != 0) {
+			cps.push_back (cp);
 		} else {
 			fatal << _("programming error: ")
 			      << X_("unknown selectable type passed to Selection::toggle()")
@@ -857,8 +859,8 @@ Selection::toggle (list<Selectable*> const & selectables)
 		toggle (rvs);
 	}
 
-	if (!autos.empty()) {
-		toggle (autos);
+	if (!cps.empty()) {
+		toggle (cps);
 	}
 }
 
@@ -875,15 +877,15 @@ void
 Selection::add (list<Selectable*> const & selectables)
 {
 	RegionView* rv;
-	AutomationSelectable* as;
+	ControlPoint* cp;
 	vector<RegionView*> rvs;
-	vector<AutomationSelectable*> autos;
+	vector<ControlPoint*> cps;
 
 	for (std::list<Selectable*>::const_iterator i = selectables.begin(); i != selectables.end(); ++i) {
 		if ((rv = dynamic_cast<RegionView*> (*i)) != 0) {
 			rvs.push_back (rv);
-		} else if ((as = dynamic_cast<AutomationSelectable*> (*i)) != 0) {
-			autos.push_back (as);
+		} else if ((cp = dynamic_cast<ControlPoint*> (*i)) != 0) {
+			cps.push_back (cp);
 		} else {
 			fatal << _("programming error: ")
 			      << X_("unknown selectable type passed to Selection::add()")
@@ -896,8 +898,8 @@ Selection::add (list<Selectable*> const & selectables)
 		add (rvs);
 	}
 
-	if (!autos.empty()) {
-		add (autos);
+	if (!cps.empty()) {
+		add (cps);
 	}
 }
 
@@ -911,13 +913,33 @@ Selection::clear_points ()
 }
 
 void
-Selection::add (vector<AutomationSelectable*>& autos)
+Selection::add (ControlPoint* cp)
+{
+	cp->set_selected (true);
+	set_point_selection_from_line (cp->line ());
+}
+
+void
+Selection::add (vector<ControlPoint*> const & cps)
+{
+	for (vector<ControlPoint*>::const_iterator i = cps.begin(); i != cps.end(); ++i) {
+		(*i)->set_selected (true);
+	}
+
+	set_point_selection_from_line (cps.front()->line ());
+}
+
+void
+Selection::set (ControlPoint* cp)
 {
-	for (vector<AutomationSelectable*>::iterator i = autos.begin(); i != autos.end(); ++i) {
-		points.push_back (**i);
+	if (cp->get_selected()) {
+		return;
 	}
 
-	PointsChanged ();
+	points.clear ();
+	vector<ControlPoint*> cps;
+	cps.push_back (cp);
+	add (cps);
 }
 
 void
@@ -986,3 +1008,70 @@ MarkerSelection::range (nframes64_t& s, nframes64_t& e)
 	s = std::min (s, e);
 	e = std::max (s, e);
 }
+
+/** Automation control point selection is mostly manipulated using the selected state
+ *  of the ControlPoints themselves.  For example, to add a point to a selection, its
+ *  ControlPoint is marked as selected and then this method is called.  It sets up
+ *  our PointSelection from the selected ControlPoints of a given AutomationLine.
+ *
+ *  We can't use ControlPoints directly in the selection, as we need to express a
+ *  selection of not just a visible ControlPoint but also (possibly) some invisible
+ *  points nearby.  Hence the selection stores AutomationRanges, and these are synced
+ *  with ControlPoint selection state using AutomationLine::set_selected_points.
+ */
+
+void
+Selection::set_point_selection_from_line (AutomationLine const & line)
+{
+	points.clear ();
+	
+	AutomationRange current (DBL_MAX, 0, 1, 0, &line.trackview);
+
+	for (uint32_t i = 0; i < line.npoints(); ++i) {
+		ControlPoint const * cp = line.nth (i);
+
+		if (cp->get_selected()) {
+			/* x and y position of this control point in coordinates suitable for
+			   an AutomationRange (ie model time and fraction of track height)
+			*/
+			double const x = (*(cp->model()))->when;
+			double const y = 1 - (cp->get_y() / line.trackview.current_height ());
+
+			/* work out the position of a rectangle the size of a control point centred
+			   on this point
+			*/
+
+			double const size = cp->size ();
+			double const x_size = line.time_converter().from (line.trackview.editor().pixel_to_frame (size));
+			double const y_size = size / line.trackview.current_height ();
+			
+			double const x1 = x - x_size / 2;
+			double const x2 = x + x_size / 2;
+			double const y1 = y - y_size / 2;
+			double const y2 = y + y_size / 2;
+
+			/* extend the current AutomationRange to put this point in */
+			current.start = min (current.start, x1);
+			current.end = max (current.end, x2);
+			current.low_fract = min (current.low_fract, y1);
+			current.high_fract = max (current.high_fract, y2);
+
+		} else {
+			/* this point isn't selected; if the current AutomationRange has some
+			   stuff in it, push it onto the list and make a new one
+			*/
+			if (current.start < DBL_MAX) {
+				points.push_back (current);
+				current = AutomationRange (DBL_MAX, 0, 1, 0, &line.trackview);
+			}
+		}
+	}
+
+	/* Maybe push the current AutomationRange, as above */
+	if (current.start < DBL_MAX) {
+		points.push_back (current);
+		current = AutomationRange (DBL_MAX, 0, 1, 0, &line.trackview);
+	}
+
+	PointsChanged (); /* EMIT SIGNAL */
+}
diff --git a/gtk2_ardour/selection.h b/gtk2_ardour/selection.h
index ca678fd832..e81e1bdea8 100644
--- a/gtk2_ardour/selection.h
+++ b/gtk2_ardour/selection.h
@@ -43,6 +43,9 @@ class RegionView;
 class Selectable;
 class PublicEditor;
 class MidiRegionView;
+class AutomationLine;
+class ControlPoint;
+
 
 namespace ARDOUR {
 	class Region;
@@ -122,7 +125,7 @@ class Selection : public sigc::trackable, public PBD::ScopedConnectionList
 	void set (boost::shared_ptr<Evoral::ControlList>);
 	void set (boost::shared_ptr<ARDOUR::Playlist>);
 	void set (const std::list<boost::shared_ptr<ARDOUR::Playlist> >&);
-	void set (AutomationSelectable*);
+	void set (ControlPoint *);
 	void set (Marker*);
 	void set (const RegionSelection&);
 
@@ -137,7 +140,8 @@ class Selection : public sigc::trackable, public PBD::ScopedConnectionList
 	void toggle (ARDOUR::AutomationList*);
 	void toggle (boost::shared_ptr<ARDOUR::Playlist>);
 	void toggle (const std::list<boost::shared_ptr<ARDOUR::Playlist> >&);
-	void toggle (const std::vector<AutomationSelectable*>&);
+	void toggle (ControlPoint *);
+	void toggle (std::vector<ControlPoint*> const &);
 	void toggle (Marker*);
 
 	void add (TimeAxisView*);
@@ -151,6 +155,8 @@ class Selection : public sigc::trackable, public PBD::ScopedConnectionList
 	void add (boost::shared_ptr<Evoral::ControlList>);
 	void add (boost::shared_ptr<ARDOUR::Playlist>);
 	void add (const std::list<boost::shared_ptr<ARDOUR::Playlist> >&);
+	void add (ControlPoint *);
+	void add (std::vector<ControlPoint*> const &);
 	void add (Marker*);
 	void add (const std::list<Marker*>&);
 	void add (const RegionSelection&);
@@ -186,10 +192,10 @@ class Selection : public sigc::trackable, public PBD::ScopedConnectionList
 	template<class A> void foreach_region (void (ARDOUR::Region::*method)(A), A arg);
 
   private:
+	void set_point_selection_from_line (AutomationLine const &);
+
 	PublicEditor const * editor;
 	uint32_t next_time_id;
-	
-	void add (std::vector<AutomationSelectable*>&);
 };
 
 bool operator==(const Selection& a, const Selection& b);