redesign (MIDI)RegionView display disabling around an RAII model

gtk2_ardour/audio_region_view.h

@@ -144,6 +144,8 @@ public:
 		return _end_xfade_visible;
 	}
 
+	void redisplay () {}
+
 protected:
 
 	/* this constructor allows derived types

gtk2_ardour/automation_region_view.cc

@@ -79,6 +79,8 @@ AutomationRegionView::~AutomationRegionView ()
 void
 AutomationRegionView::init (bool /*wfd*/)
 {
+	DisplaySuspender (*this);
+
 	RegionView::init (false);
 
 	reset_width_dependent_items ((double) _region->length_samples() / samples_per_pixel);
@@ -87,8 +89,6 @@ AutomationRegionView::init (bool /*wfd*/)
 
 	fill_color_name = "midi frame base";
 	set_colors ();
-
-	enable_display ();
 }
 
 void

gtk2_ardour/automation_region_view.h

@@ -67,6 +67,7 @@ public:
 
 	void set_height (double);
 	void reset_width_dependent_items(double pixel_width);
+	void redisplay () {}
 
 protected:
 	void create_line(boost::shared_ptr<ARDOUR::AutomationList> list);

gtk2_ardour/automation_streamview.cc

@@ -105,8 +105,7 @@ AutomationStreamView::add_region_view_internal (boost::shared_ptr<Region> region
 				arv->line()->set_list (list);
 			}
 			(*i)->set_valid (true);
-			(*i)->enable_display ();
+			display_region (arv);
-			display_region(arv);
 
 			return 0;
 		}
@@ -123,7 +122,6 @@ AutomationStreamView::add_region_view_internal (boost::shared_ptr<Region> region
 
 	/* follow global waveform setting */
 
-	region_view->enable_display();
 	display_region (region_view);
 
 	/* catch regionview going away */
@@ -167,10 +165,12 @@ AutomationStreamView::set_automation_state (AutoState state)
 void
 AutomationStreamView::redisplay_track ()
 {
+	vector<RegionView::DisplaySuspender> vds;
 	// Flag region views as invalid and disable drawing
 	for (list<RegionView*>::iterator i = region_views.begin(); i != region_views.end(); ++i) {
 		(*i)->set_valid (false);
-		(*i)->disable_display ();
+		vds.push_back (RegionView::DisplaySuspender (**i));
+
 	}
 
 	// Add and display region views, and flag them as valid

gtk2_ardour/midi_region_view.cc

@@ -255,6 +255,8 @@ MidiRegionView::MidiRegionView (const MidiRegionView& other, boost::shared_ptr<M
 void
 MidiRegionView::init (bool /*wfd*/)
 {
+	DisplaySuspender ds (*this);
+
 	PublicEditor::DropDownKeys.connect (sigc::mem_fun (*this, &MidiRegionView::drop_down_keys));
 
 	_model = midi_region()->midi_source(0)->model();
@@ -271,10 +273,8 @@ MidiRegionView::init (bool /*wfd*/)
 	region_resized (ARDOUR::bounds_change);
 	//region_locked ();
 
-	disable_display ();
 	set_colors ();
 	reset_width_dependent_items (_pixel_width);
-	enable_display ();
 
 	display_model (_model);
 
@@ -1078,9 +1078,21 @@ MidiRegionView::get_events (Events& e, Evoral::Sequence<Temporal::Beats>::NoteOp
 	}
 }
 
+void
+MidiRegionView::redisplay ()
+{
+	redisplay_model ();
+}
+
 void
 MidiRegionView::redisplay_model()
 {
+	if (!display_enabled()) {
+		return;
+	}
+
+	// CALLGRIND_START_INSTRUMENTATION;
+
 	group->canvas()->freeze_queue_draw ();
 
 	if (_active_notes) {
@@ -1195,7 +1207,7 @@ MidiRegionView::redisplay_model()
 
 		for (set<Evoral::event_id_t>::iterator it = _pending_note_selection.begin(); it != _pending_note_selection.end(); ++it) {
 			if ((*it) == note->id()) {
-				add_to_selection (cne);
+				// add_to_selection (cne);
 			}
 		}
 	}
@@ -1215,6 +1227,9 @@ MidiRegionView::redisplay_model()
 	_pending_note_selection.clear ();
 
 	group->canvas()->thaw_queue_draw ();
+
+	// CALLGRIND_STOP_INSTRUMENTATION;
+	// CALLGRIND_DUMP_STATS;
 }
 
 void
@@ -1372,17 +1387,6 @@ void
 MidiRegionView::region_resized (const PropertyChange& what_changed)
 {
 	RegionView::region_resized(what_changed); // calls RegionView::set_duration()
-
-#if 0
-	/* catch end and start trim so we can update the view*/
-	if (!what_changed.contains (ARDOUR::Properties::start) &&
-	    what_changed.contains (ARDOUR::Properties::length)) {
-		enable_display ();
-	} else if (what_changed.contains (ARDOUR::Properties::start) &&
-	    what_changed.contains (ARDOUR::Properties::length)) {
-		enable_display ();
-	}
-#endif
 }
 
 void
@@ -4011,18 +4015,6 @@ MidiRegionView::color_handler ()
 	/* XXX probably more to do here */
 }
 
-void
-MidiRegionView::enable_display ()
-{
-	bool was_disabled = !display_enabled();
-
-	RegionView::enable_display ();
-
-	if (was_disabled && display_enabled()) {
-		redisplay_model ();
-	}
-}
-
 void
 MidiRegionView::show_step_edit_cursor (Temporal::Beats pos)
 {

gtk2_ardour/midi_region_view.h

@@ -120,6 +120,7 @@ public:
 	void hide_step_edit_cursor ();
 	void set_step_edit_cursor_width (Temporal::Beats beats);
 
+	virtual void redisplay ();
 	void redisplay_model();
 
 	GhostRegion* add_ghost (TimeAxisView&);
@@ -297,8 +298,6 @@ public:
 
 	void selection_as_notelist (Notes& selected, bool allow_all_if_none_selected = false);
 
-	void enable_display ();
-
 	void set_channel_selector_scoped_note(NoteBase* note){ _channel_selection_scoped_note = note; }
 	NoteBase* channel_selector_scoped_note(){  return _channel_selection_scoped_note; }
 

gtk2_ardour/midi_streamview.cc

@@ -158,19 +158,18 @@ MidiStreamView::add_region_view_internal (boost::shared_ptr<Region> r, bool wait
 	}
 
 	region_views.push_front (region_view);
-	region_view->disable_display ();
+
+	{
+		RegionView::DisplaySuspender ds (*region_view);
+
 		display_region (region_view, wait_for_data);
 
 		/* fit note range if we are importing */
 		if (_trackview.session()->operation_in_progress (Operations::insert_file)) {
 			/* this will call display_region() */
 			set_note_range (ContentsRange);
-	} else {
-		/* display events and find note range */
-		region_view->redisplay_model ();
 		}
-
+	}
-	region_view->enable_display ();
 
 	/* catch regionview going away */
 	boost::weak_ptr<Region> wr (region); // make this explicit
@@ -188,7 +187,8 @@ MidiStreamView::display_region (MidiRegionView* region_view, bool load_model)
 		return;
 	}
 
-	region_view->disable_display ();
+	RegionView::DisplaySuspender ds (*region_view);
+
 	region_view->set_height (child_height());
 
 	boost::shared_ptr<MidiSource> source (region_view->midi_region()->midi_source(0));
@@ -206,7 +206,6 @@ MidiStreamView::display_region (MidiRegionView* region_view, bool load_model)
 	_range_dirty = update_data_note_range (source->model()->lowest_note(), source->model()->highest_note());
 
 	// Display region contents
-	region_view->enable_display ();
 	region_view->display_model (source->model());
 }
 
@@ -281,10 +280,12 @@ MidiStreamView::redisplay_track ()
 		_data_note_max = 71;
 	}
 
+	vector<RegionView::DisplaySuspender> vds;
+
 	// Flag region views as invalid and disable drawing
 	for (i = region_views.begin(); i != region_views.end(); ++i) {
-		(*i)->set_valid(false);
+		(*i)->set_valid (false);
-		(*i)->disable_display ();
+		vds.push_back (RegionView::DisplaySuspender (**i));
 	}
 
 	// Add and display region views, and flag them as valid
@@ -294,7 +295,7 @@ MidiStreamView::redisplay_track ()
 	layer_regions();
 
 	// Update note range (not regions which are correct) and draw note lines
-	apply_note_range(_lowest_note, _highest_note, false);
+	apply_note_range (_lowest_note, _highest_note, false);
 }
 
 void

gtk2_ardour/region_view.cc

@@ -89,7 +89,7 @@ RegionView::RegionView (ArdourCanvas::Container*          parent,
 	, editor(0)
 	, current_visible_sync_position(0.0)
 	, valid(false)
-	, _disable_display (1)
+	, _disable_display (0)
 	, _pixel_width(1.0)
 	, in_destructor(false)
 	, wait_for_data(false)
@@ -170,7 +170,7 @@ RegionView::RegionView (ArdourCanvas::Container*          parent,
 	, editor(0)
 	, current_visible_sync_position(0.0)
 	, valid(false)
-	, _disable_display (1)
+	, _disable_display (0)
 	, _pixel_width(1.0)
 	, in_destructor(false)
 	, wait_for_data(false)
@@ -238,10 +238,6 @@ RegionView::init (bool wfd)
 	_cue_markers_visible = false;
 	update_cue_markers ();
 
-	if (wfd) {
-		enable_display ();
-	}
-
 	/* derived class calls set_height () including RegionView::set_height() in ::init() */
 	//set_height (trackview.current_height());
 
@@ -1263,6 +1259,9 @@ RegionView::enable_display ()
 	std::cerr << "EnableDisplay " << this << " currently " << _disable_display << std::endl;
 	if (_disable_display) {
 		_disable_display--;
+		if (_disable_display == 0) {
+			redisplay ();
+		}
 	}
 }
 

gtk2_ardour/region_view.h

@@ -101,9 +101,19 @@ public:
 	virtual void entered () {}
 	virtual void exited () {}
 
-	virtual void enable_display();
-	virtual void disable_display();
 	bool display_enabled() const;
+	virtual void redisplay() = 0;
+
+	struct DisplaySuspender {
+		DisplaySuspender (RegionView& rv) : region_view (rv) {
+			region_view.disable_display ();
+		}
+		~DisplaySuspender () {
+			region_view.enable_display ();
+		}
+		RegionView& region_view;
+	};
+
 	virtual void update_coverage_frame (LayerDisplay);
 
 	static PBD::Signal1<void,RegionView*> RegionViewGoingAway;
@@ -228,6 +238,11 @@ private:
 	typedef std::list<ViewCueMarker*> ViewCueMarkers;
 	ViewCueMarkers _cue_markers;
 	bool _cue_markers_visible;
+
+  private:
+	friend class DisplaySuspender;
+	void enable_display();
+	void disable_display();
 };
 
 #endif /* __gtk_ardour_region_view_h__ */