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.
This commit is contained in:
parent
55e8f6ac30
commit
b074ff0dd5
@ -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();
|
||||
|
@ -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 ();
|
||||
|
@ -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();
|
||||
|
@ -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;
|
||||
}
|
||||
|
||||
|
@ -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,
|
||||
|
@ -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()
|
||||
|
Loading…
Reference in New Issue
Block a user