a set of inter-related subtle changes to get vertical autoscrolling to work, or at least work better.

4 files changed, 79 insertions, 49 deletions

diff --git a/gtk2_ardour/editor_canvas.cc b/gtk2_ardour/editor_canvas.cc
index 2b6d8731fd..8ad2abdfad 100644
--- a/gtk2_ardour/editor_canvas.cc
+++ b/gtk2_ardour/editor_canvas.cc
@@ -493,18 +493,46 @@ Editor::drop_paths (const RefPtr<Gdk::DragContext>& context,
 void
 Editor::maybe_autoscroll (bool allow_horiz, bool allow_vert, bool from_headers)
 {
-	if (!Config->get_autoscroll_editor ()) {
+	if (!Config->get_autoscroll_editor () || autoscroll_active ()) {
 		return;
 	}
 
+	/* define a rectangular boundary for scrolling. If the mouse moves
+	 * outside of this area and/or continue to be outside of this area,
+	 * then we will continuously auto-scroll the canvas in the appropriate
+	 * direction(s)
+	 *
+	 * the boundary is defined in coordinates relative to the toplevel
+	 * window since that is what we're going to call ::get_pointer() on
+	 * during autoscrolling to determine if we're still outside the
+	 * boundary or not.
+	 */
+
 	ArdourCanvas::Rect scrolling_boundary;
 	Gtk::Allocation alloc;
-	
+	int cx, cy;
+
 	if (from_headers) {
 		alloc = controls_layout.get_allocation ();
-	} else {
+	} else {	
 		alloc = _track_canvas_viewport->get_allocation ();
+		cx = alloc.get_x();
+		cy = alloc.get_y();
+
+		/* reduce height by the height of the timebars, which happens
+		   to correspond to the position of the hv_scroll_group.
+		*/
 		
+		alloc.set_height (alloc.get_height() - hv_scroll_group->position().y);
+		alloc.set_y (alloc.get_y() + hv_scroll_group->position().y);
+
+		/* now reduce it again so that we start autoscrolling before we
+		 * move off the top or bottom of the canvas
+		 */
+
+		alloc.set_height (alloc.get_height() - 20);
+		alloc.set_y (alloc.get_y() + 10);
+
 		/* the effective width of the autoscroll boundary so
 		   that we start scrolling before we hit the edge.
 		   
@@ -517,11 +545,10 @@ Editor::maybe_autoscroll (bool allow_horiz, bool allow_vert, bool from_headers)
 			alloc.set_width (alloc.get_width() - 20);
 			alloc.set_x (alloc.get_x() + 10);
 		} 
+
 	}
 	
-	scrolling_boundary = ArdourCanvas::Rect (alloc.get_x(), alloc.get_y(), 
-						 alloc.get_x() + alloc.get_width(), 
-						 alloc.get_y() + alloc.get_height());
+	scrolling_boundary = ArdourCanvas::Rect (alloc.get_x(), alloc.get_y(), alloc.get_x() + alloc.get_width(), alloc.get_y() + alloc.get_height());
 	
 	int x, y;
 	Gdk::ModifierType mask;
@@ -552,6 +579,7 @@ Editor::autoscroll_canvas ()
 	get_window()->get_pointer (x, y, mask);
 
 	VisualChange vc;
+	bool vertical_motion = false;
 
 	if (autoscroll_horizontal_allowed) {
 
@@ -600,7 +628,7 @@ Editor::autoscroll_canvas ()
 	if (autoscroll_vertical_allowed) {
 		
 		// const double vertical_pos = vertical_adjustment.get_value();
-		const int speed_factor = 20;
+		const int speed_factor = 10;
 
 		/* vertical */ 
 		
@@ -610,20 +638,21 @@ Editor::autoscroll_canvas ()
 
 			if (autoscroll_cnt && (autoscroll_cnt % speed_factor == 0)) {
 				y_motion = scroll_up_one_track ();
+				vertical_motion = true;
 			}
 
 		} else if (y > autoscroll_boundary.y1) {
 
 			if (autoscroll_cnt && (autoscroll_cnt % speed_factor == 0)) {
 				y_motion = scroll_down_one_track ();
-				
+				vertical_motion = true;
 			}
 		}
 
 		no_stop = true;
 	}
 
-	if (vc.pending) {
+	if (vc.pending || vertical_motion) {
 
 		/* change horizontal first */
 
@@ -642,26 +671,33 @@ Editor::autoscroll_canvas ()
 		
 		/* the motion handler expects events in canvas coordinate space */
 
-		/* first convert from Editor window coordinates to canvas
-		 * window coordinates
+		/* we asked for the mouse position above (::get_pointer()) via
+		 * our own top level window (we being the Editor). Convert into 
+		 * coordinates within the canvas window.
 		 */
 
 		int cx;
 		int cy;
 
-		/* clamp x and y to remain within the visible area */
+		translate_coordinates (*_track_canvas, x, y, cx, cy);
 
-		x = min (max ((ArdourCanvas::Coord) x, autoscroll_boundary.x0), autoscroll_boundary.x1);
-		y = min (max ((ArdourCanvas::Coord) y, autoscroll_boundary.y0), autoscroll_boundary.y1);
+		/* clamp x and y to remain within the autoscroll boundary,
+		 * which is defined in window coordinates
+		 */
 
-		translate_coordinates (*_track_canvas_viewport, x, y, cx, cy);
+		x = min (max ((ArdourCanvas::Coord) cx, autoscroll_boundary.x0), autoscroll_boundary.x1);
+		y = min (max ((ArdourCanvas::Coord) cy, autoscroll_boundary.y0), autoscroll_boundary.y1);
+
+		/* now convert from Editor window coordinates to canvas
+		 * window coordinates
+		 */
 
 		ArdourCanvas::Duple d = _track_canvas->window_to_canvas (ArdourCanvas::Duple (cx, cy));
 		ev.x = d.x;
 		ev.y = d.y;
 
 		motion_handler (0, (GdkEvent*) &ev, true);
-
+		
 	} else if (no_stop) {
 
 		/* not changing visual state but pointer is outside the scrolling boundary
@@ -701,7 +737,7 @@ Editor::autoscroll_canvas ()
 		ev.y = d.y;
 
 		motion_handler (0, (GdkEvent*) &ev, true);
-
+		
 	} else {
 		stop_canvas_autoscroll ();
 		return false;
diff --git a/gtk2_ardour/editor_drag.cc b/gtk2_ardour/editor_drag.cc
index f6f05925de..9abbdba28b 100644
--- a/gtk2_ardour/editor_drag.cc
+++ b/gtk2_ardour/editor_drag.cc
@@ -177,6 +177,12 @@ DragManager::motion_handler (GdkEvent* e, bool from_autoscroll)
 {
 	bool r = false;
 
+	/* calling this implies that we expect the event to have canvas
+	 * coordinates 
+	 *
+	 * Can we guarantee that this is true?
+	 */
+
 	_current_pointer_frame = _editor->canvas_event_sample (e, &_current_pointer_x, &_current_pointer_y);
 
 	for (list<Drag*>::iterator i = _drags.begin(); i != _drags.end(); ++i) {
@@ -194,24 +200,6 @@ DragManager::motion_handler (GdkEvent* e, bool from_autoscroll)
 }
 
 bool
-DragManager::window_motion_handler (GdkEvent* e, bool from_autoscroll)
-{
-	bool r = false;
-
-	_current_pointer_frame = _editor->canvas_event_sample (e, &_current_pointer_x, &_current_pointer_y);
-
-	for (list<Drag*>::iterator i = _drags.begin(); i != _drags.end(); ++i) {
-		bool const t = (*i)->motion_handler (e, from_autoscroll);
-		if (t) {
-			r = true;
-		}
-
-	}
-
-	return r;
-}
-
-bool
 DragManager::have_item (ArdourCanvas::Item* i) const
 {
 	list<Drag*>::const_iterator j = _drags.begin ();
@@ -398,6 +386,7 @@ Drag::motion_handler (GdkEvent* event, bool from_autoscroll)
 			return true;
 		}
 	}
+
 	return false;
 }
 
@@ -711,7 +700,7 @@ RegionMotionDrag::motion (GdkEvent* event, bool first_move)
 
 		delta_time_axis_view = current_pointer_time_axis_view - _last_pointer_time_axis_view;
 		delta_layer = current_pointer_layer - _last_pointer_layer;
-	}
+	} 
 	
 	/* Work out the change in x */
 	framepos_t pending_region_position;
@@ -1932,8 +1921,6 @@ TrimDrag::start_grab (GdkEvent* event, Gdk::Cursor*)
 
 	framepos_t const pf = adjusted_current_frame (event);
 
-	cerr << "Button state = " << hex << event->button.state << dec << endl;
-
 	if (Keyboard::modifier_state_equals (event->button.state, Keyboard::PrimaryModifier)) {
 		/* Move the contents of the region around without changing the region bounds */
 		_operation = ContentsTrim;
@@ -1997,8 +1984,6 @@ TrimDrag::motion (GdkEvent* event, bool first_move)
 	pair<set<boost::shared_ptr<Playlist> >::iterator,bool> insert_result;
 	frameoffset_t frame_delta = 0;
 
-	cerr << "trim drag @ " << this << " motion\n";
-
 	if (tv && tv->is_track()) {
 		speed = tv->track()->speed();
 	}
diff --git a/gtk2_ardour/editor_drag.h b/gtk2_ardour/editor_drag.h
index 7649813269..de10ed0787 100644
--- a/gtk2_ardour/editor_drag.h
+++ b/gtk2_ardour/editor_drag.h
@@ -54,7 +54,6 @@ public:
 	~DragManager ();
 
 	bool motion_handler (GdkEvent *, bool);
-	bool window_motion_handler (GdkEvent *, bool);
 
 	void abort ();
 	void add (Drag *);
diff --git a/gtk2_ardour/editor_ops.cc b/gtk2_ardour/editor_ops.cc
index 0151df872f..e7618d9168 100644
--- a/gtk2_ardour/editor_ops.cc
+++ b/gtk2_ardour/editor_ops.cc
@@ -1334,20 +1334,24 @@ Editor::scroll_down_one_track ()
 {
 	TrackViewList::reverse_iterator next = track_views.rend();
 	std::pair<TimeAxisView*,double> res;
+	const double bottom_of_trackviews = vertical_adjustment.get_value() + vertical_adjustment.get_page_size() - 1;
 
 	for (TrackViewList::reverse_iterator t = track_views.rbegin(); t != track_views.rend(); ++t) {
 		if ((*t)->hidden()) {
 			continue;
 		}
 		
-		/* find the trackview at the bottom of the trackview group */
-		res = (*t)->covers_y_position (_visible_canvas_height);
+		/* If this is the bottom visible trackview, we want to display
+		   the next one.
+		*/
+
+		res = (*t)->covers_y_position (bottom_of_trackviews);
 
 		if (res.first) {
 			break;
 		}
 
-		next = t;
+		++next; // moves "next" towards the "front" since it is a reverse iterator
 	}
 
 	/* move to the track below the first one that covers the */
@@ -1363,7 +1367,7 @@ Editor::scroll_down_one_track ()
 bool
 Editor::scroll_up_one_track ()
 {
-	// double vertical_pos = vertical_adjustment.get_value ();
+	double vertical_pos = vertical_adjustment.get_value ();
 
 	TrackViewList::iterator prev = track_views.end();
 	std::pair<TimeAxisView*,double> res;
@@ -1375,9 +1379,10 @@ Editor::scroll_up_one_track ()
 		}
 
 		/* find the trackview at the top of the trackview group */
-		res = (*t)->covers_y_position (0);
+		res = (*t)->covers_y_position (vertical_pos);
 		
 		if (res.first) {
+			cerr << res.first->name() << " covers the top\n";
 			break;
 		}
 
@@ -5733,16 +5738,21 @@ Editor::select_prev_route()
 void
 Editor::ensure_track_visible(TimeAxisView *track)
 {
-	if (track->hidden())
+	if (track->hidden()) {
 		return;
+	}
+
+	/* compute visible area of trackview group, as offsets from top of
+	 * trackview group.
+	 */
 
 	double const current_view_min_y = vertical_adjustment.get_value();
-	double const current_view_max_y = vertical_adjustment.get_value() + vertical_adjustment.get_page_size();
+	double const current_view_max_y = current_view_min_y + vertical_adjustment.get_page_size();
 
 	double const track_min_y = track->y_position ();
 	double const track_max_y = track->y_position () + track->effective_height ();
 
-	if (track_min_y >= current_view_min_y &&
+	if (track_min_y > current_view_min_y &&
 	    track_max_y <= current_view_max_y) {
 		return;
 	}