use window-based coordinates when picking current item so that we get per-item (per-scroll-group,really) computation of position and coverage.

libs/canvas/arc.cc

@@ -126,7 +126,7 @@ Arc::set_start (double deg)
 bool
 Arc::covers (Duple const & point) const
 {
-	Duple p = canvas_to_item (point);
+	Duple p = window_to_item (point);
 
 	double angle_degs = atan (p.y/p.x) * 2.0 * M_PI;
 	double radius = sqrt (p.x * p.x + p.y * p.y);

libs/canvas/canvas.cc

@@ -315,7 +315,7 @@ GtkCanvas::pick_current_item (int state)
 		return;
 	}
 
-	pick_current_item (window_to_canvas (Duple (x, y)), state);
+	pick_current_item (Duple (x, y), state);
 }
 		
 void
@@ -388,6 +388,9 @@ GtkCanvas::pick_current_item (Duple const & point, int state)
 	}
 }
 
+/** Deliver a series of enter & leave events based on the pointer position being at window
+ * coordinate @param point, and pointer @param state (modifier keys, etc)
+ */
 void
 GtkCanvas::deliver_enter_leave (Duple const & point, int state)
 {
@@ -449,7 +452,6 @@ GtkCanvas::deliver_enter_leave (Duple const & point, int state)
 		 * heirarchy between current and new_current.
 		 */
 		
-
 		for (i = _current_item->parent(); i && i != _new_current_item; i = i->parent()) {
 			items_to_leave_virtual.push_back (i);
 		}
@@ -658,7 +660,10 @@ GtkCanvas::on_button_press_event (GdkEventButton* ev)
 	/* translate event coordinates from window to canvas */
 
 	GdkEvent copy = *((GdkEvent*)ev);
-	Duple where = window_to_canvas (Duple (ev->x, ev->y));
+	Duple winpos = Duple (ev->x, ev->y);
+	Duple where = window_to_canvas (winpos);
+	
+	pick_current_item (winpos, ev->state);
 
 	copy.button.x = where.x;
 	copy.button.y = where.y;
@@ -667,7 +672,6 @@ GtkCanvas::on_button_press_event (GdkEventButton* ev)
 	   for scroll if this GtkCanvas is in a GtkCanvasViewport.
 	*/
 
-	pick_current_item (where, ev->state);
 	DEBUG_TRACE (PBD::DEBUG::CanvasEvents, string_compose ("canvas button press @ %1, %2 => %3\n", ev->x, ev->y, where));
 	return deliver_event (reinterpret_cast<GdkEvent*>(&copy));
 }
@@ -682,9 +686,10 @@ GtkCanvas::on_button_release_event (GdkEventButton* ev)
 	/* translate event coordinates from window to canvas */
 
 	GdkEvent copy = *((GdkEvent*)ev);
-	Duple where = window_to_canvas (Duple (ev->x, ev->y));
+	Duple winpos = Duple (ev->x, ev->y);
+	Duple where = window_to_canvas (winpos);
 	
-	pick_current_item (where, ev->state);
+	pick_current_item (winpos, ev->state);
 
 	copy.button.x = where.x;
 	copy.button.y = where.y;
@@ -719,7 +724,7 @@ GtkCanvas::on_motion_notify_event (GdkEventMotion* ev)
 
 	// DEBUG_TRACE (PBD::DEBUG::CanvasEvents, string_compose ("canvas motion @ %1, %2\n", ev->x, ev->y));
 
-	pick_current_item (where, ev->state);
+	pick_current_item (point, ev->state);
 
 	/* Now deliver the motion event.  It may seem a little inefficient
 	   to recompute the items under the event, but the enter notify/leave
@@ -733,8 +738,7 @@ GtkCanvas::on_motion_notify_event (GdkEventMotion* ev)
 bool
 GtkCanvas::on_enter_notify_event (GdkEventCrossing* ev)
 {
-	Duple where = window_to_canvas (Duple (ev->x, ev->y));
-	pick_current_item (where, ev->state);
+	pick_current_item (Duple (ev->x, ev->y), ev->state);
 	return true;
 }
 
@@ -742,8 +746,7 @@ bool
 GtkCanvas::on_leave_notify_event (GdkEventCrossing* ev)
 {
 	_new_current_item = 0;
-	Duple where = window_to_canvas (Duple (ev->x, ev->y));
-	deliver_enter_leave (where, ev->state);
+	deliver_enter_leave (Duple (ev->x, ev->y), ev->state);
 	return true;
 }
 

libs/canvas/canvas/item.h

@@ -73,7 +73,16 @@ public:
 	 */
 	virtual void render (Rect const & area, Cairo::RefPtr<Cairo::Context>) const = 0;
 
-	virtual void add_items_at_point (Duple, std::vector<Item const *>& items) const {
+	/** Adds one or more items to the vector @param items based on their
+	 * covering @param point which is in **window** coordinates
+	 *
+	 * Note that Item::add_items_at_window_point() is only intended to be 
+	 * called on items already looked up in a LookupTable (i.e. by a
+	 * parent group) and thus known to cover @param point already.
+	 *
+	 * Derived classes may add more items than themselves (e.g. Group).
+	 */
+	virtual void add_items_at_point (Duple /*point*/, std::vector<Item const *>& items) const {
 		items.push_back (this);
 	}
 

libs/canvas/curve.cc

@@ -235,7 +235,7 @@ Curve::render (Rect const & area, Cairo::RefPtr<Cairo::Context> context) const
 bool
 Curve::covers (Duple const & pc) const
 {
-	Duple point = canvas_to_item (pc);
+	Duple point = window_to_item (pc);
 
 	/* O(N) N = number of points, and not accurate */
 

libs/canvas/group.cc

@@ -346,9 +346,9 @@ Group::add_items_at_point (Duple const point, vector<Item const *>& items) const
 {
 	boost::optional<Rect> const bbox = bounding_box ();
 
-	/* Point is in canvas coordinate system */
+	/* Point is in window coordinate system */
 
-	if (!bbox || !item_to_canvas (bbox.get()).contains (point)) {
+	if (!bbox || !item_to_window (bbox.get()).contains (point)) {
 		return;
 	}
 

libs/canvas/item.cc

@@ -318,6 +318,11 @@ Item::find_scroll_parent ()
 	Item const * i = this;
 	ScrollGroup const * last_scroll_group = 0;
 
+	/* Don't allow a scroll group to find itself as its own scroll parent
+	 */
+
+	i = i->parent ();
+
 	while (i) {
 		ScrollGroup const * sg = dynamic_cast<ScrollGroup const *> (i);
 		if (sg) {
@@ -591,7 +596,7 @@ Item::depth () const
 bool
 Item::covers (Duple const & point) const
 {
-	Duple p = canvas_to_item (point);
+	Duple p = window_to_item (point);
 
 	if (_bounding_box_dirty) {
 		compute_bounding_box ();

libs/canvas/line.cc

@@ -159,7 +159,7 @@ Line::set_y1 (Coord y1)
 bool
 Line::covers (Duple const & point) const
 {
-	const Duple p = canvas_to_item (point);
+	const Duple p = window_to_item (point);
 	static const Distance threshold = 2.0;
 
 	/* this quick check works for vertical and horizontal lines, which are

libs/canvas/lookup_table.cc

@@ -52,7 +52,7 @@ DumbLookupTable::get (Rect const &)
 vector<Item *>
 DumbLookupTable::items_at_point (Duple const & point) const
 {
-	/* Point is in canvas coordinate system */
+	/* Point is in window coordinate system */
 
 	list<Item *> const & items (_group.items ());
 	vector<Item *> vitems;
@@ -71,7 +71,7 @@ DumbLookupTable::items_at_point (Duple const & point) const
 bool
 DumbLookupTable::has_item_at_point (Duple const & point) const
 {
-	/* Point is in canvas coordinate system */
+	/* Point is in window coordinate system */
 
 	list<Item *> const & items (_group.items ());
 	vector<Item *> vitems;

libs/canvas/poly_line.cc

@@ -46,7 +46,7 @@ PolyLine::render (Rect const & area, Cairo::RefPtr<Cairo::Context> context) cons
 bool
 PolyLine::covers (Duple const & point) const
 {
-	Duple p = canvas_to_item (point);
+	Duple p = window_to_item (point);
 
 	const Points::size_type npoints = _points.size();
 	

libs/canvas/polygon.cc

@@ -103,7 +103,7 @@ Polygon::cache_shape_computation () const
 bool 
 Polygon::covers (Duple const & point) const
 {
-	Duple p = canvas_to_item (point);
+	Duple p = window_to_item (point);
 
 	Points::size_type npoints = _points.size();