a few changes to fix region dragging, all related to coordinate system handling, which is now much simpler with the new canvas; more debugging output when asked for

4 files changed, 95 insertions, 62 deletions

diff --git a/libs/canvas/canvas.cc b/libs/canvas/canvas.cc
index b849894bf0..da34a73c21 100644
--- a/libs/canvas/canvas.cc
+++ b/libs/canvas/canvas.cc
@@ -100,18 +100,6 @@ Canvas::render (Rect const & area, Cairo::RefPtr<Cairo::Context> const & context
 	// checkpoint ("render", "-> render");
 	render_count = 0;
 	
-#ifdef CANVAS_DEBUG
-	if (getenv ("ARDOUR_HARLEQUIN_CANVAS")) {
-		/* light up the canvas to show redraws */
-		context->set_source_rgba (random()%255 / 255.0,
-					  random()%255 / 255.0,
-					  random()%255 / 255.0,
-					  255);
-		context->rectangle (area.x0, area.y0, area.width(), area.height());
-		context->fill ();
-	}
-#endif
-
 	context->save ();
 
 	/* clip to the requested area */
@@ -143,6 +131,17 @@ Canvas::render (Rect const & area, Cairo::RefPtr<Cairo::Context> const & context
 
 	context->restore ();
 
+#ifdef CANVAS_DEBUG
+	if (getenv ("ARDOUR_HARLEQUIN_CANVAS")) {
+		/* light up the canvas to show redraws */
+		context->set_source_rgba (random()%255 / 255.0,
+					  random()%255 / 255.0,
+					  random()%255 / 255.0,
+					  255);
+		context->rectangle (area.x0, area.y0, area.width(), area.height());
+		context->fill ();
+	}
+#endif
 	// checkpoint ("render", "<- render");
 }
 
@@ -225,10 +224,15 @@ void
 Canvas::item_moved (Item* item, boost::optional<Rect> pre_change_parent_bounding_box)
 {
 	if (pre_change_parent_bounding_box) {
-		/* request a redraw of where the item used to be; we have to use the
-		   parent's coordinates here as item bounding boxes do not change
-		   when the item moves.
-		*/
+		/* request a redraw of where the item used to be. The box has
+		 * to be in parent coordinate space since the bounding box of
+		 * an item does not change when moved. If we use
+		 * item->item_to_canvas() on the old bounding box, we will be
+		 * using the item's new position, and so will compute the wrong
+		 * invalidation area. If we use the parent (which has not
+		 * moved, then this will work.
+		 */
+
 		queue_draw_item_area (item->parent(), pre_change_parent_bounding_box.get ());
 	}
 
@@ -246,7 +250,9 @@ Canvas::item_moved (Item* item, boost::optional<Rect> pre_change_parent_bounding
 void
 Canvas::queue_draw_item_area (Item* item, Rect area)
 {
-	request_redraw (item->item_to_canvas (area));
+	ArdourCanvas::Rect canvas_area = item->item_to_canvas (area);
+	// cerr << "CANVAS Invalidate " << area << " TRANSLATE AS " << canvas_area << endl;
+	request_redraw (canvas_area);
 }
 
 /** @return An XML description of the canvas and its objects */
@@ -554,6 +560,7 @@ void
 GtkCanvas::request_redraw (Rect const & request)
 {
 	Rect area = canvas_to_window (request);
+	// cerr << "Invalidate " << request << " TRANSLATE AS " << area << endl;
 	queue_draw_area (floor (area.x0), floor (area.y0), ceil (area.x1) - floor (area.x0), ceil (area.y1) - floor (area.y0));
 }
 
diff --git a/libs/canvas/canvas/item.h b/libs/canvas/canvas/item.h
index fc66754ed4..7f1321b260 100644
--- a/libs/canvas/canvas/item.h
+++ b/libs/canvas/canvas/item.h
@@ -108,9 +108,12 @@ public:
 	/* XXX: it's a pity these aren't the same form as item_to_parent etc.,
 	   but it makes a bit of a mess in the rest of the code if they are not.
 	*/
-	void canvas_to_item (Coord &, Coord &) const;
+
+        void canvas_to_item (Coord &, Coord &) const;
+        Duple canvas_to_item (Duple const &) const;
 	void item_to_canvas (Coord &, Coord &) const;
 	Rect item_to_canvas (Rect const &) const;
+        Duple item_to_canvas (Duple const &) const;
 
 	void raise_to_top ();
 	void raise (int);
diff --git a/libs/canvas/group.cc b/libs/canvas/group.cc
index d12c13169e..34001bc255 100644
--- a/libs/canvas/group.cc
+++ b/libs/canvas/group.cc
@@ -278,6 +278,7 @@ Group::dump (ostream& o) const
 {
 	o << _canvas->indent();
 	o << "Group " << this << " [" << name << ']';
+	o << " @ " << position();
 	o << " Items: " << _items.size();
 	o << " Visible ? " << _visible;
 
@@ -285,6 +286,7 @@ Group::dump (ostream& o) const
 
 	if (bb) {
 		o << endl << _canvas->indent() << "  bbox: " << bb.get();
+		o << endl << _canvas->indent() << "  CANVAS bbox: " << item_to_canvas (bb.get());
 	} else {
 		o << "  bbox unset";
 	}
diff --git a/libs/canvas/item.cc b/libs/canvas/item.cc
index 2c4ec893e7..75a5718271 100644
--- a/libs/canvas/item.cc
+++ b/libs/canvas/item.cc
@@ -67,6 +67,66 @@ Item::item_to_parent (ArdourCanvas::Rect const & r) const
 	return r.translate (_position);
 }
 
+ArdourCanvas::Rect
+Item::item_to_canvas (ArdourCanvas::Rect const & r) const
+{
+	Item const * i = this;
+	Duple offset;
+
+	while (i) {
+		offset = offset.translate (i->position());
+		i = i->parent();
+	}
+
+	return r.translate (offset);
+}
+
+ArdourCanvas::Duple
+Item::item_to_canvas (ArdourCanvas::Duple const & d) const
+{
+	Item const * i = this;
+	Duple offset;
+
+	while (i) {
+		offset = offset.translate (i->position());
+		i = i->parent();
+	}
+
+	return d.translate (offset);
+}
+
+ArdourCanvas::Duple
+Item::canvas_to_item (ArdourCanvas::Duple const & d) const
+{
+	Item const * i = this;
+	Duple offset;
+
+	while (i) {
+		offset = offset.translate (-(i->position()));
+		i = i->parent();
+	}
+
+	return d.translate (offset);
+}
+
+void
+Item::item_to_canvas (Coord& x, Coord& y) const
+{
+	Duple d = item_to_canvas (Duple (x, y));
+		
+	x = d.x;
+	y = d.y;
+}
+
+void
+Item::canvas_to_item (Coord& x, Coord& y) const
+{
+	Duple d = canvas_to_item (Duple (x, y));
+
+	x = d.x;
+	y = d.y;
+}
+
 /** Set the position of this item in the parent's coordinates */
 void
 Item::set_position (Duple p)
@@ -75,6 +135,9 @@ Item::set_position (Duple p)
 	boost::optional<ArdourCanvas::Rect> pre_change_parent_bounding_box;
 
 	if (bbox) {
+		/* see the comment in Canvas::item_moved() to understand
+		 * why we use the parent's bounding box here.
+		 */
 		pre_change_parent_bounding_box = item_to_parent (bbox.get());
 	}
 	
@@ -285,50 +348,6 @@ Item::get_data (string const & key) const
 }
 
 void
-Item::item_to_canvas (Coord& x, Coord& y) const
-{
-	Duple d (x, y);
-	Item const * i = this;
-	
-	while (i) {
-		d = i->item_to_parent (d);
-		i = i->_parent;
-	}
-		
-	x = d.x;
-	y = d.y;
-}
-
-void
-Item::canvas_to_item (Coord& x, Coord& y) const
-{
-	Duple d (x, y);
-	Item const * i = this;
-
-	while (i) {
-		d = i->parent_to_item (d);
-		i = i->_parent;
-	}
-
-	x = d.x;
-	y = d.y;
-}
-
-ArdourCanvas::Rect
-Item::item_to_canvas (ArdourCanvas::Rect const & area) const
-{
-	ArdourCanvas::Rect r = area;
-	Item const * i = this;
-
-	while (i) {
-		r = i->item_to_parent (r);
-		i = i->parent ();
-	}
-
-	return r;
-}
-
-void
 Item::set_ignore_events (bool ignore)
 {
 	_ignore_events = ignore;
@@ -340,6 +359,7 @@ Item::dump (ostream& o) const
 	boost::optional<ArdourCanvas::Rect> bb = bounding_box();
 
 	o << _canvas->indent() << whatami() << ' ' << this;
+	o << " @ " << position();
 	
 #ifdef CANVAS_DEBUG
 	if (!name.empty()) {
@@ -349,6 +369,7 @@ Item::dump (ostream& o) const
 
 	if (bb) {
 		o << endl << _canvas->indent() << "\tbbox: " << bb.get();
+		o << endl << _canvas->indent() << "\tCANVAS bbox: " << item_to_canvas (bb.get());
 	} else {
 		o << " bbox unset";
 	}