Remove GhostRegion::CatchDeletion signal to reduce session close times

Currently when a GhostRegion is deleted by its "parent" RegionView it emits the
static GhostRegion::CatchDeletion signal which is connected to the
RegionView::remove_ghost method of every RegionView instance.

With a static GhostRegion::CatchDeletion signal a particular test session
causes 31 Million calls of RegionView::remove_ghost on Session deletion and the
session takes 70 seconds to close with a debug build.

The lifetime of a ghost region is tied to both the TimeAxisView(TAV) and
RegionView(RV) in that when a RegionView is deleted all GhostRegion instances
associated with the RegionView should be deleted or when a TimeAxisView is
deleted all ghost regions that are contained in the view should be deleted.

This means that there needs to be notification between GhostRegion and both
classes. Instead of using a signal for this as we know there are only two
listeners and GhostRegion already holds a reference to the TimeAxisView, also
take a reference to the parent RegionView in the GhostRegion constructor and
use it to notify the RegionView when GhostRegion destructor is called so it can
drop any references it holds.

Using a direct function call in the GhostRegion destructor to notify the
TimeAxisView and RegionView "parents" brings the unload/close time down for the
test session from 70 seconds to 4.5 seconds.

The GhostRegion also references canvas items that are added to the TimeAxisView
canvas group or at least a canvas group that it manages. So when the
TimeAxisView is destroyed and the canvas group that is the parent of those
items is destroyed, the GhostRegion's canvas items will also be
deleted/destroyed by the parent canvas item/group. This means the GhostRegions
must be destroyed when the TimeAxisView they are contained in is destroyed or
there will be dangling references to canvas items that have already been
deleted and trying to delete them again will be bad.

diff --git a/gtk2_ardour/audio_region_view.cc b/gtk2_ardour/audio_region_view.cc
index cbf1619188b484664ced6cdfb69a8b53b1d6ae10..d3a82dba12b684d7cfca2b5afa98629f6d62129c 100644 (file)
--- a/gtk2_ardour/audio_region_view.cc
+++ b/gtk2_ardour/audio_region_view.cc
@@ -1384,7 +1384,7 @@ AudioRegionView::add_ghost (TimeAxisView& tv)
        assert(rtv);
 
        double unit_position = _region->position () / samples_per_pixel;
-       AudioGhostRegion* ghost = new AudioGhostRegion (tv, trackview, unit_position);
+       AudioGhostRegion* ghost = new AudioGhostRegion (*this, tv, trackview, unit_position);
        uint32_t nchans;
 
        nchans = rtv->track()->n_channels().n_audio();

diff --git a/gtk2_ardour/ghostregion.cc b/gtk2_ardour/ghostregion.cc
index 7c697a1b0f4c9165c5ca732f226df0616476fd81..127ab64d41f3bfa095e27ab58b498d51a615bc5b 100644 (file)
--- a/gtk2_ardour/ghostregion.cc
+++ b/gtk2_ardour/ghostregion.cc
@@ -30,6 +30,7 @@
 #include "ghostregion.h"
 #include "midi_streamview.h"
 #include "midi_time_axis.h"
+#include "region_view.h"
 #include "rgb_macros.h"
 #include "note.h"
 #include "hit.h"
@@ -40,11 +41,14 @@ using namespace Editing;
 using namespace ArdourCanvas;
 using namespace ARDOUR;
 
-PBD::Signal1<void,GhostRegion*> GhostRegion::CatchDeletion;
-
-GhostRegion::GhostRegion (ArdourCanvas::Container* parent, TimeAxisView& tv, TimeAxisView& source_tv, double initial_pos)
-       : trackview (tv)
-       , source_trackview (source_tv)
+GhostRegion::GhostRegion(RegionView& rv,
+                         ArdourCanvas::Container* parent,
+                         TimeAxisView& tv,
+                         TimeAxisView& source_tv,
+                         double initial_pos)
+    : parent_rv(rv)
+    , trackview(tv)
+    , source_trackview(source_tv)
 {
        group = new ArdourCanvas::Container (parent);
        CANVAS_DEBUG_NAME (group, "ghost region");
@@ -70,7 +74,8 @@ GhostRegion::GhostRegion (ArdourCanvas::Container* parent, TimeAxisView& tv, Tim
 
 GhostRegion::~GhostRegion ()
 {
-       CatchDeletion (this);
+       parent_rv.remove_ghost(this);
+       trackview.erase_ghost(this);
        delete base_rect;
        delete group;
 }
@@ -108,8 +113,11 @@ GhostRegion::is_automation_ghost()
        return (dynamic_cast<AutomationTimeAxisView*>(&trackview)) != 0;
 }
 
-AudioGhostRegion::AudioGhostRegion(TimeAxisView& tv, TimeAxisView& source_tv, double initial_unit_pos)
-       : GhostRegion(tv.ghost_group(), tv, source_tv, initial_unit_pos)
+AudioGhostRegion::AudioGhostRegion(RegionView& rv,
+                                   TimeAxisView& tv,
+                                   TimeAxisView& source_tv,
+                                   double initial_unit_pos)
+    : GhostRegion(rv, tv.ghost_group(), tv, source_tv, initial_unit_pos)
 {
 
 }
@@ -162,12 +170,16 @@ AudioGhostRegion::set_colors ()
 /** The general constructor; called when the destination timeaxisview doesn't have
  *  a midistreamview.
  *
+ *  @param rv The parent RegionView that is being ghosted.
  *  @param tv TimeAxisView that this ghost region is on.
  *  @param source_tv TimeAxisView that we are the ghost for.
  */
-MidiGhostRegion::MidiGhostRegion(TimeAxisView& tv, TimeAxisView& source_tv, double initial_unit_pos)
-       : GhostRegion(tv.ghost_group(), tv, source_tv, initial_unit_pos)
-       , _optimization_iterator (events.end ())
+MidiGhostRegion::MidiGhostRegion(RegionView& rv,
+                                 TimeAxisView& tv,
+                                 TimeAxisView& source_tv,
+                                 double initial_unit_pos)
+    : GhostRegion(rv, tv.ghost_group(), tv, source_tv, initial_unit_pos)
+    , _optimization_iterator(events.end())
 {
        base_rect->lower_to_bottom();
        update_range ();
@@ -176,12 +188,20 @@ MidiGhostRegion::MidiGhostRegion(TimeAxisView& tv, TimeAxisView& source_tv, doub
 }
 
 /**
+ *  @param rv The parent RegionView being ghosted.
  *  @param msv MidiStreamView that this ghost region is on.
  *  @param source_tv TimeAxisView that we are the ghost for.
  */
-MidiGhostRegion::MidiGhostRegion(MidiStreamView& msv, TimeAxisView& source_tv, double initial_unit_pos)
-       : GhostRegion(msv.midi_underlay_group, msv.trackview(), source_tv, initial_unit_pos)
-       , _optimization_iterator (events.end ())
+MidiGhostRegion::MidiGhostRegion(RegionView& rv,
+                                 MidiStreamView& msv,
+                                 TimeAxisView& source_tv,
+                                 double initial_unit_pos)
+    : GhostRegion(rv,
+                  msv.midi_underlay_group,
+                  msv.trackview(),
+                  source_tv,
+                  initial_unit_pos)
+    , _optimization_iterator(events.end())
 {
        base_rect->lower_to_bottom();
        update_range ();

diff --git a/gtk2_ardour/ghostregion.h b/gtk2_ardour/ghostregion.h
index a62d8d5b25c7c65771ca4c640b4803763dba7af5..8a4a8284c0df964cb4a30b788503ce4026f662da 100644 (file)
--- a/gtk2_ardour/ghostregion.h
+++ b/gtk2_ardour/ghostregion.h
@@ -32,11 +32,17 @@ class Note;
 class Hit;
 class MidiStreamView;
 class TimeAxisView;
+class RegionView;
 
 class GhostRegion : public sigc::trackable
 {
 public:
-       GhostRegion(ArdourCanvas::Container* parent, TimeAxisView& tv, TimeAxisView& source_tv, double initial_unit_pos);
+       GhostRegion(RegionView& rv,
+                   ArdourCanvas::Container* parent,
+                   TimeAxisView& tv,
+                   TimeAxisView& source_tv,
+                   double initial_unit_pos);
+
        virtual ~GhostRegion();
 
        virtual void set_samples_per_pixel (double) = 0;
@@ -48,19 +54,21 @@ public:
        guint source_track_color(unsigned char alpha = 0xff);
        bool is_automation_ghost();
 
+       RegionView& parent_rv;
        /** TimeAxisView that is the AutomationTimeAxisView that we are on */
        TimeAxisView& trackview;
        /** TimeAxisView that we are a ghost for */
        TimeAxisView& source_trackview;
        ArdourCanvas::Container* group;
        ArdourCanvas::Rectangle* base_rect;
-
-       static PBD::Signal1<void,GhostRegion*> CatchDeletion;
 };
 
 class AudioGhostRegion : public GhostRegion {
 public:
-       AudioGhostRegion(TimeAxisView& tv, TimeAxisView& source_tv, double initial_unit_pos);
+       AudioGhostRegion(RegionView& rv,
+                        TimeAxisView& tv,
+                        TimeAxisView& source_tv,
+                        double initial_unit_pos);
 
        void set_samples_per_pixel (double);
        void set_height();
@@ -80,8 +88,16 @@ public:
            ArdourCanvas::Item* item;
        };
 
-       MidiGhostRegion(TimeAxisView& tv, TimeAxisView& source_tv, double initial_unit_pos);
-       MidiGhostRegion(MidiStreamView& msv, TimeAxisView& source_tv, double initial_unit_pos);
+       MidiGhostRegion(RegionView& rv,
+                       TimeAxisView& tv,
+                       TimeAxisView& source_tv,
+                       double initial_unit_pos);
+
+       MidiGhostRegion(RegionView& rv,
+                       MidiStreamView& msv,
+                       TimeAxisView& source_tv,
+                       double initial_unit_pos);
+
        ~MidiGhostRegion();
 
        MidiStreamView* midi_view();

diff --git a/gtk2_ardour/midi_region_view.cc b/gtk2_ardour/midi_region_view.cc
index 682f02365c89400dceeab8d1a44616d8f86e7e10..ba8c72120e55315249ac2c6b5a26a7799429a4e4 100644 (file)
--- a/gtk2_ardour/midi_region_view.cc
+++ b/gtk2_ardour/midi_region_view.cc
@@ -1532,9 +1532,9 @@ MidiRegionView::add_ghost (TimeAxisView& tv)
                /* if ghost is inserted into midi track, use a dedicated midi ghost canvas group
                   to allow having midi notes on top of note lines and waveforms.
                */
-               ghost = new MidiGhostRegion (*mtv->midi_view(), trackview, unit_position);
+               ghost = new MidiGhostRegion (*this, *mtv->midi_view(), trackview, unit_position);
        } else {
-               ghost = new MidiGhostRegion (tv, trackview, unit_position);
+               ghost = new MidiGhostRegion (*this, tv, trackview, unit_position);
        }
 
        for (Events::iterator i = _events.begin(); i != _events.end(); ++i) {
@@ -1545,8 +1545,6 @@ MidiRegionView::add_ghost (TimeAxisView& tv)
        ghost->set_duration (_region->length() / samples_per_pixel);
        ghosts.push_back (ghost);
 
-       GhostRegion::CatchDeletion.connect (*this, invalidator (*this), boost::bind (&RegionView::remove_ghost, this, _1), gui_context());
-
        return ghost;
 }
 

diff --git a/gtk2_ardour/region_view.cc b/gtk2_ardour/region_view.cc
index 7ddca8b574824b101faa548dc5a9e859a47a4dca..3312fc392950c7244d6d728541921ce772eba3c0 100644 (file)
--- a/gtk2_ardour/region_view.cc
+++ b/gtk2_ardour/region_view.cc
@@ -84,7 +84,6 @@ RegionView::RegionView (ArdourCanvas::Container*          parent,
        , wait_for_data(false)
        , _silence_text (0)
 {
-       GhostRegion::CatchDeletion.connect (*this, invalidator (*this), boost::bind (&RegionView::remove_ghost, this, _1), gui_context());
 }
 
 RegionView::RegionView (const RegionView& other)
@@ -98,8 +97,6 @@ RegionView::RegionView (const RegionView& other)
        current_visible_sync_position = other.current_visible_sync_position;
        valid = false;
        _pixel_width = other._pixel_width;
-
-       GhostRegion::CatchDeletion.connect (*this, invalidator (*this), boost::bind (&RegionView::remove_ghost, this, _1), gui_context());
 }
 
 RegionView::RegionView (const RegionView& other, boost::shared_ptr<Region> other_region)
@@ -117,8 +114,6 @@ RegionView::RegionView (const RegionView& other, boost::shared_ptr<Region> other
        current_visible_sync_position = other.current_visible_sync_position;
        valid = false;
        _pixel_width = other._pixel_width;
-
-       GhostRegion::CatchDeletion.connect (*this, invalidator (*this), boost::bind (&RegionView::remove_ghost, this, _1), gui_context());
 }
 
 RegionView::RegionView (ArdourCanvas::Container*          parent,

diff --git a/gtk2_ardour/time_axis_view.cc b/gtk2_ardour/time_axis_view.cc
index 436e161583f01dad8322ba9c866bd649a0e1b8e2..4ad476b73442c775e6018862c8f7adb60664aed6 100644 (file)
--- a/gtk2_ardour/time_axis_view.cc
+++ b/gtk2_ardour/time_axis_view.cc
@@ -221,8 +221,6 @@ TimeAxisView::TimeAxisView (ARDOUR::Session* sess, PublicEditor& ed, TimeAxisVie
        top_hbox.pack_start (scroomer_placeholder, false, false); // OR pack_end to move after meters ?
 
        UIConfiguration::instance().ColorsChanged.connect (sigc::mem_fun (*this, &TimeAxisView::color_handler));
-
-       GhostRegion::CatchDeletion.connect (*this, invalidator (*this), boost::bind (&TimeAxisView::erase_ghost, this, _1), gui_context());
 }
 
 TimeAxisView::~TimeAxisView()