Revert "add Group::clear(), do not clear _canvas member of Item when unparented (only the parent is changed)"

This reverts commit a4df65a56f83823aba27432685977f420458a213.

gtk2_ardour/tempo_lines.cc

@@ -25,30 +25,42 @@
 
 using namespace std;
 
+#define MAX_CACHED_LINES 128
+
 TempoLines::TempoLines (ArdourCanvas::Canvas& canvas, ArdourCanvas::Group* group, double screen_height)
 	: _canvas (canvas)
-	, _group (group)
-	, _height (screen_height)
+	, _group(group)
+	, _clean_left(DBL_MAX)
+	, _clean_right(0.0)
+	, _height(screen_height)
 {
 }
 
 void
 TempoLines::tempo_map_changed()
 {
-	/* remove all lines from the group, put them in the cache (to avoid
-	 * unnecessary object destruction+construction later), and clear _lines
-	 */
-	 
-	_group->clear ();
-	_cache.insert (_cache.end(), _lines.begin(), _lines.end());
-	_lines.clear ();
+	_clean_left = DBL_MAX;
+	_clean_right = 0.0;
+
+	double_t d = 1.0;
+	// TODO: Dirty/slow, but 'needed' for zoom :(
+	for (Lines::iterator i = _lines.begin(); i != _lines.end(); d += 1.0) {
+		Lines::iterator next = i;
+		++next;
+		i->second->set_x0 (-d);
+		i->second->set_x1 (-d);
+		ArdourCanvas::Line* f = i->second;
+		_lines.erase(i);
+		_lines.insert(make_pair(- d, f));
+		i = next;
+	}
 }
 
 void
 TempoLines::show ()
 {
 	for (Lines::iterator i = _lines.begin(); i != _lines.end(); ++i) {
-		(*i)->show();
+		i->second->show();
 	}
 }
 
@@ -56,7 +68,7 @@ void
 TempoLines::hide ()
 {
 	for (Lines::iterator i = _lines.begin(); i != _lines.end(); ++i) {
-		(*i)->hide();
+		i->second->hide();
 	}
 }
 
@@ -66,13 +78,18 @@ TempoLines::draw (const ARDOUR::TempoMap::BBTPointList::const_iterator& begin,
 		  double samples_per_pixel)
 {
 	ARDOUR::TempoMap::BBTPointList::const_iterator i;
-	ArdourCanvas::Rect const visible = _canvas.visible_area ();
+	ArdourCanvas::Line *line = 0;
+	gdouble xpos;
 	double  beat_density;
 
 	uint32_t beats = 0;
 	uint32_t bars = 0;
 	uint32_t color;
 
+	const size_t needed = distance (begin, end);
+
+	ArdourCanvas::Rect const visible = _canvas.visible_area ();
+
 	/* get the first bar spacing */
 
 	i = end;
@@ -88,7 +105,31 @@ TempoLines::draw (const ARDOUR::TempoMap::BBTPointList::const_iterator& begin,
 		return;
 	}
 
-	tempo_map_changed ();
+	xpos = rint(((framepos_t)(*i).frame) / (double)samples_per_pixel);
+	const double needed_right = xpos;
+
+	i = begin;
+
+	xpos = rint(((framepos_t)(*i).frame) / (double)samples_per_pixel);
+	const double needed_left = xpos;
+
+	Lines::iterator left = _lines.lower_bound(xpos); // first line >= xpos
+
+	bool exhausted = (left == _lines.end());
+	Lines::iterator li = left;
+	if (li != _lines.end())
+		line = li->second;
+
+	// Tempo map hasn't changed and we're entirely within a clean
+	// range, don't need to do anything.  Yay.
+	if (needed_left >= _clean_left && needed_right <= _clean_right) {
+		// cerr << endl << "*** LINE CACHE PERFECT HIT" << endl;
+		return;
+	}
+
+	// cerr << endl << "*** LINE CACHE MISS" << endl;
+
+	bool invalidated = false;
 
 	for (i = begin; i != end; ++i) {
 
@@ -101,24 +142,109 @@ TempoLines::draw (const ARDOUR::TempoMap::BBTPointList::const_iterator& begin,
 			color = ARDOUR_UI::config()->get_canvasvar_MeasureLineBeat();
 		}
 
-		ArdourCanvas::Coord xpos = rint(((framepos_t)(*i).frame) / (double)samples_per_pixel);
+		xpos = rint(((framepos_t)(*i).frame) / (double)samples_per_pixel);
 
-		ArdourCanvas::Line* line;
+		li = _lines.lower_bound(xpos); // first line >= xpos
 
-		if (!_cache.empty()) {
-			line = _cache.back ();
-			_cache.pop_back ();
-			line->reparent (_group);
+		line = (li != _lines.end()) ? li->second : 0;
+		assert(!line || line->x0() == li->first);
+		
+		Lines::iterator next = li;
+		if (next != _lines.end())
+			++next;
+		
+		exhausted = (next == _lines.end());
+
+		// Hooray, line is perfect
+		if (line && line->x0() == xpos) {
+			if (li != _lines.end())
+				++li;
+			
+			line->set_outline_color (color);
+
+			// Use existing line, moving if necessary
+		} else if (!exhausted) {
+			Lines::iterator steal = _lines.end();
+			--steal;
+			
+			// Steal from the right
+			if (left->first > needed_left && li != steal && steal->first > needed_right) {
+				// cerr << "*** STEALING FROM RIGHT" << endl;
+				double const x = steal->first;
+				line = steal->second;
+				_lines.erase(steal);
+				line->set_x0 (xpos);
+				line->set_x1 (xpos);
+				line->set_outline_color (color);
+				_lines.insert(make_pair(xpos, line));
+				invalidated = true;
+				
+				// Shift clean range left
+				_clean_left = min(_clean_left, xpos);
+				_clean_right = min(_clean_right, x);
+				
+				// Move this line to where we need it
+			} else {
+				Lines::iterator existing = _lines.find(xpos);
+				if (existing != _lines.end()) {
+					//cout << "*** EXISTING LINE" << endl;
+					li = existing;
+					li->second->set_outline_color (color);
+				} else {
+					//cout << "*** MOVING LINE" << endl;
+					const double x1 = line->x0();
+					const bool was_clean = x1 >= _clean_left && x1 <= _clean_right;
+					invalidated = invalidated || was_clean;
+					// Invalidate clean portion (XXX: too harsh?)
+					_clean_left  = needed_left;
+					_clean_right = needed_right;
+					_lines.erase(li);
+					line->set_outline_color (color);
+					line->set_x0 (xpos);
+					line->set_x1 (xpos);
+					_lines.insert(make_pair(xpos, line));
+				}
+			}
+			
+			// Create a new line
+		} else if (_lines.size() < needed || _lines.size() < MAX_CACHED_LINES) {
+			// cerr << "*** CREATING LINE" << endl;
+			/* if we already have a line there ... don't sweat it */
+			if (_lines.find (xpos) == _lines.end()) {
+				line = new ArdourCanvas::Line (_group);
+				line->set_x0 (xpos);
+				line->set_x1 (xpos);
+				line->set_y0 (0.0);
+				line->set_y1 (ArdourCanvas::COORD_MAX);
+				line->set_outline_color (color);
+				_lines.insert(make_pair(xpos, line));
+			}
+			
+			// Steal from the left
 		} else {
-			line = new ArdourCanvas::Line (_group);
+			// cerr << "*** STEALING FROM LEFT" << endl;
+			if (_lines.find (xpos) == _lines.end()) {
+				Lines::iterator steal = _lines.begin();
+				double const x = steal->first;
+				line = steal->second;
+				_lines.erase(steal);
+				line->set_outline_color (color);
+				line->set_x0 (xpos);
+				line->set_x1 (xpos);
+				_lines.insert(make_pair(xpos, line));
+				invalidated = true;
+			
+				// Shift clean range right
+				_clean_left = max(_clean_left, x);
+				_clean_right = max(_clean_right, xpos);
+			}
 		}
+	}
 
-		line->set_x0 (xpos);
-		line->set_x1 (xpos);
-		line->set_y0 (0.0);
-		line->set_y1 (_height);
-		line->set_outline_color (color);
-		line->show ();
+	// Extend range to what we've 'fixed'
+	if (!invalidated) {
+		_clean_left  = min(_clean_left, needed_left);
+		_clean_right = max(_clean_right, needed_right);
 	}
 }
 

gtk2_ardour/tempo_lines.h

@@ -19,29 +19,39 @@
 #ifndef __ardour_tempo_lines_h__
 #define __ardour_tempo_lines_h__
 
-#include <list>
+#include <map>
+#include <boost/pool/pool.hpp>
+#include <boost/pool/pool_alloc.hpp>
 #include "ardour/tempo.h"
 
+typedef boost::fast_pool_allocator<
+		std::pair<const double, ArdourCanvas::Line*>,
+		boost::default_user_allocator_new_delete,
+		boost::details::pool::null_mutex,
+		8192>
+	MapAllocator;
+
 class TempoLines {
 public:
 	TempoLines(ArdourCanvas::Canvas& canvas, ArdourCanvas::Group* group, double screen_height);
 
 	void tempo_map_changed();
 
-	void draw (const ARDOUR::TempoMap::BBTPointList::const_iterator& begin, 
-		   const ARDOUR::TempoMap::BBTPointList::const_iterator& end, 
-		   double frames_per_unit);
+	void draw(const ARDOUR::TempoMap::BBTPointList::const_iterator& begin, 
+		  const ARDOUR::TempoMap::BBTPointList::const_iterator& end, 
+		  double frames_per_unit);
 
 	void show();
 	void hide();
 
 private:
-        typedef std::list<ArdourCanvas::Line*> Lines;
+	typedef std::map<double, ArdourCanvas::Line*, std::less<double>, MapAllocator> Lines;
 	Lines _lines;
-        Lines _cache;
 
         ArdourCanvas::Canvas& _canvas;
 	ArdourCanvas::Group*  _group;
+	double                _clean_left;
+	double                _clean_right;
 	double                _height;
 };
 

libs/canvas/canvas/group.h

@@ -40,7 +40,6 @@ public:
 
 	void add (Item *);
 	void remove (Item *);
-        void clear (bool with_delete = false);
 	std::list<Item*> const & items () const {
 		return _items;
 	}

libs/canvas/group.cc

@@ -170,8 +170,6 @@ Group::compute_bounding_box () const
 void
 Group::add (Item* i)
 {
-	/* XXX should really notify canvas about this */
-
 	_items.push_back (i);
 	invalidate_lut ();
 	_bounding_box_dirty = true;
@@ -182,40 +180,11 @@ Group::add (Item* i)
 void
 Group::remove (Item* i)
 {
-
-	if (i->parent() != this) {
-		return;
-	}
-
-	begin_change ();
-
-	i->unparent ();
 	_items.remove (i);
 	invalidate_lut ();
 	_bounding_box_dirty = true;
 	
-	end_change ();
-}
-
-void
-Group::clear (bool with_delete)
-{
-	begin_change ();
-
-	for (list<Item*>::iterator i = _items.begin(); i != _items.end(); ++i) {
-		if (with_delete) {
-			delete *i;
-		} else {
-			(*i)->unparent ();
-		}
-	}
-
-	_items.clear ();
-
-	invalidate_lut ();
-	_bounding_box_dirty = true;
-
-	end_change ();
+	DEBUG_TRACE (PBD::DEBUG::CanvasItemsDirtied, "canvas item dirty: group remove\n");
 }
 
 void

libs/canvas/item.cc

@@ -236,14 +236,13 @@ Item::parent_to_item (ArdourCanvas::Rect const & d) const
 void
 Item::unparent ()
 {
+	_canvas = 0;
 	_parent = 0;
 }
 
 void
 Item::reparent (Group* new_parent)
 {
-	assert (_canvas == _parent->canvas ());
-
 	if (_parent) {
 		_parent->remove (this);
 	}
@@ -251,6 +250,7 @@ Item::reparent (Group* new_parent)
 	assert (new_parent);
 
 	_parent = new_parent;
+	_canvas = _parent->canvas ();
 	_parent->add (this);
 }