View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003098 | ardour | bugs | public | 2010-04-27 23:34 | 2010-11-14 22:59 |
| Reporter | oofus | Assigned To | cth103 | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | Dell D830 core2duo T9300 2.5GHz | OS | Mandriva | OS Version | 2010 |
| Target Version | 3.0-beta1 | ||||
| Summary | 0003098: Moving a region doesn't update its region list entry for positional info. | ||||
| Description | Moving a region doesn't update its region list entry for positional info. | ||||
| Tags | No tags attached. | ||||
|
|
rev 7013 contains a basic fix for this, and shows the way forward, so to speak. it should probably be extended to cover other region properties. in theory, it would be nice to have 1 method to set the display of every field in the list, but this may be hard or impractical to do. maybe just calling ::populate_row() on any change is just fine. |
|
|
Yesterday I was working on a fix to the RegionFactory::create(region) This function does not signal new regions coming into play in a number of cases such as: copy/paste, duplicate, drag region to another track, ctl+drag region. These ops do not create a new entry in the region list and also the new regions are not being named correctly. I can push a patch with fixes for this issue in a couple of hours. |
|
|
the changes mentioned in my initial note are now in svn |
|
2010-09-20 22:56
|
region-copy-notify.patch (10,148 bytes)
Index: gtk2_ardour/editor_regions.cc
===================================================================
--- gtk2_ardour/editor_regions.cc (revision 7816)
+++ gtk2_ardour/editor_regions.cc (working copy)
@@ -346,18 +346,41 @@
our_interests.add (ARDOUR::Properties::opaque);
our_interests.add (ARDOUR::Properties::fade_in);
our_interests.add (ARDOUR::Properties::fade_out);
+
+ if (last_row != NULL){
+ TreeModel::iterator j = _model->get_iter (last_row.get_path());
+ boost::shared_ptr<Region> c = (*j)[_columns.region];
+
+ if (c == r) {
+ populate_row (r, (*j));
+
+ if (what_changed.contains (ARDOUR::Properties::hidden)) {
+ redisplay ();
+ }
+
+ return;
+ }
+ }
+
+
if (what_changed.contains (our_interests)) {
/* find the region in our model and update its row */
TreeModel::Children rows = _model->children ();
TreeModel::iterator i = rows.begin ();
+
while (i != rows.end ()) {
+
TreeModel::Children children = (*i)->children ();
TreeModel::iterator j = children.begin ();
+
while (j != children.end()) {
+
boost::shared_ptr<Region> c = (*j)[_columns.region];
+
if (c == r) {
+ last_row = TreeRowReference(_model, TreePath(j));
break;
}
++j;
@@ -403,7 +426,6 @@
++i;
}
-
}
if (what_changed.contains (ARDOUR::Properties::hidden)) {
@@ -438,7 +460,6 @@
boost::shared_ptr<Region> region = (*iter)[_columns.region];
// they could have clicked on a row that is just a placeholder, like "Hidden"
-
if (region) {
if (region->automatic()) {
@@ -464,10 +485,12 @@
void
EditorRegions::set_selected (RegionSelection& regions)
{
+ TreeModel::Children rows = _model->children();
+
for (RegionSelection::iterator iter = regions.begin(); iter != regions.end(); ++iter) {
TreeModel::iterator i;
- TreeModel::Children rows = _model->children();
+
boost::shared_ptr<Region> r ((*iter)->region());
for (i = rows.begin(); i != rows.end(); ++i) {
@@ -509,6 +532,7 @@
}
}
}
+
return false;
}
Index: gtk2_ardour/editor_drag.cc
===================================================================
--- gtk2_ardour/editor_drag.cc (revision 7816)
+++ gtk2_ardour/editor_drag.cc (working copy)
@@ -692,7 +692,7 @@
MidiRegionView* mrv = dynamic_cast<MidiRegionView*>(rv);
const boost::shared_ptr<const Region> original = rv->region();
- boost::shared_ptr<Region> region_copy = RegionFactory::create (original);
+ boost::shared_ptr<Region> region_copy = RegionFactory::create (original, true);
region_copy->set_position (original->position(), this);
RegionView* nrv;
@@ -915,7 +915,7 @@
/* insert into new playlist */
RegionView* new_view = insert_region_into_playlist (
- RegionFactory::create (rv->region ()), dest_rtv, dest_layer, where, modified_playlists
+ RegionFactory::create (rv->region (), true), dest_rtv, dest_layer, where, modified_playlists
);
if (new_view == 0) {
Index: gtk2_ardour/editor_ops.cc
===================================================================
--- gtk2_ardour/editor_ops.cc (revision 7816)
+++ gtk2_ardour/editor_ops.cc (working copy)
@@ -2218,7 +2218,7 @@
begin_reversible_command (_("insert dragged region"));
playlist->clear_changes ();
- playlist->add_region (RegionFactory::create (region), where, 1.0);
+ playlist->add_region (RegionFactory::create (region, true), where, 1.0);
_session->add_command(new StatefulDiffCommand (playlist));
commit_reversible_command ();
}
@@ -2297,7 +2297,7 @@
begin_reversible_command (_("insert region"));
playlist->clear_changes ();
- playlist->add_region ((RegionFactory::create (region)), get_preferred_edit_position(), times);
+ playlist->add_region ((RegionFactory::create (region, true)), get_preferred_edit_position(), times);
_session->add_command(new StatefulDiffCommand (playlist));
commit_reversible_command ();
}
@@ -3106,7 +3106,7 @@
}
pl->clear_changes ();
- pl->add_region (RegionFactory::create (region), region->last_frame(), times);
+ pl->add_region (RegionFactory::create (region, true), region->last_frame(), times);
_session->add_command (new StatefulDiffCommand (pl));
}
@@ -3150,7 +3150,7 @@
}
playlist->clear_changes ();
- playlist->add_region (RegionFactory::create (region), start, times);
+ playlist->add_region (RegionFactory::create (region, true), start, times);
_session->add_command (new StatefulDiffCommand (playlist));
}
Index: gtk2_ardour/editor_regions.h
===================================================================
--- gtk2_ardour/editor_regions.h (revision 7816)
+++ gtk2_ardour/editor_regions.h (working copy)
@@ -40,16 +40,20 @@
void reset_sort_type (Editing::RegionListSortType, bool);
void set_selected (RegionSelection &);
void selection_mapover (sigc::slot<void,boost::shared_ptr<ARDOUR::Region> >);
+
boost::shared_ptr<ARDOUR::Region> get_dragged_region ();
boost::shared_ptr<ARDOUR::Region> get_single_selection ();
+
Editing::RegionListSortType sort_type () const {
return _sort_type;
}
+
void redisplay ();
void suspend_redisplay () {
_no_redisplay = true;
}
+
void resume_redisplay () {
_no_redisplay = false;
redisplay ();
@@ -108,12 +112,17 @@
};
Columns _columns;
+
+ Gtk::TreeModel::RowReference last_row;
void region_changed (boost::shared_ptr<ARDOUR::Region>, PBD::PropertyChange const &);
void selection_changed ();
+
sigc::connection _change_connection;
+
bool set_selected_in_subrow (boost::shared_ptr<ARDOUR::Region>, Gtk::TreeModel::Row const &, int);
bool selection_filter (const Glib::RefPtr<Gtk::TreeModel>& model, const Gtk::TreeModel::Path& path, bool yn);
+
void name_edit (const std::string&, const std::string&);
void locked_changed (std::string const &);
void glued_changed (std::string const &);
Index: libs/ardour/ardour/region_factory.h
===================================================================
--- libs/ardour/ardour/region_factory.h (revision 7816)
+++ libs/ardour/ardour/region_factory.h (working copy)
@@ -56,7 +56,7 @@
static PBD::Signal1<void,boost::shared_ptr<Region> > CheckNewRegion;
/** create a "pure copy" of Region @param other */
- static boost::shared_ptr<Region> create (boost::shared_ptr<const Region> other);
+ static boost::shared_ptr<Region> create (boost::shared_ptr<const Region> other, bool announce = false);
/** create a region from a single Source */
static boost::shared_ptr<Region> create (boost::shared_ptr<Source>,
Index: libs/ardour/audioregion.cc
===================================================================
--- libs/ardour/audioregion.cc (revision 7816)
+++ libs/ardour/audioregion.cc (working copy)
@@ -116,8 +116,10 @@
{
register_properties ();
+ suspend_property_changes();
set_default_fades ();
set_default_envelope ();
+ resume_property_changes();
listen_to_my_curves ();
connect_to_analysis_changed ();
@@ -958,6 +960,8 @@
_envelope->truncate_end (_length);
_envelope->set_max_xval (_length);
_envelope->thaw ();
+
+ suspend_property_changes();
if (_left_of_split) {
set_default_fade_out ();
@@ -971,6 +975,8 @@
_fade_in->extend_to (_length);
send_change (PropertyChange (Properties::fade_in));
}
+
+ resume_property_changes();
}
void
@@ -979,6 +985,8 @@
/* as above, but the shift was from the front */
_envelope->truncate_start (_length);
+
+ suspend_property_changes();
if (_right_of_split) {
set_default_fade_in ();
@@ -992,6 +1000,8 @@
_fade_out->extend_to (_length);
send_change (PropertyChange (Properties::fade_out));
}
+
+ resume_property_changes();
}
int
Index: libs/ardour/playlist.cc
===================================================================
--- libs/ardour/playlist.cc (revision 7816)
+++ libs/ardour/playlist.cc (working copy)
@@ -324,7 +324,7 @@
RegionLock rlock (const_cast<Playlist *> (this));
for (RegionList::const_iterator i = regions.begin(); i != regions.end(); ++i) {
- newlist.push_back (RegionFactory::RegionFactory::create (*i));
+ newlist.push_back (RegionFactory::RegionFactory::create (*i, true));
}
}
@@ -709,7 +709,7 @@
*/
for (int i = 0; i < itimes; ++i) {
- boost::shared_ptr<Region> copy = RegionFactory::create (region);
+ boost::shared_ptr<Region> copy = RegionFactory::create (region, true);
add_region_internal (copy, pos);
pos += region->length();
}
@@ -1247,7 +1247,7 @@
while (itimes--) {
for (RegionList::iterator i = other->regions.begin(); i != other->regions.end(); ++i) {
- boost::shared_ptr<Region> copy_of_region = RegionFactory::create (*i);
+ boost::shared_ptr<Region> copy_of_region = RegionFactory::create (*i, true);
/* put these new regions on top of all existing ones, but preserve
the ordering they had in the original playlist.
@@ -1283,7 +1283,7 @@
framepos_t pos = position + 1;
while (itimes--) {
- boost::shared_ptr<Region> copy = RegionFactory::create (region);
+ boost::shared_ptr<Region> copy = RegionFactory::create (region, true);
add_region_internal (copy, pos);
pos += region->length();
}
Index: libs/ardour/region_factory.cc
===================================================================
--- libs/ardour/region_factory.cc (revision 7816)
+++ libs/ardour/region_factory.cc (working copy)
@@ -45,7 +45,7 @@
std::map<std::string, uint32_t> RegionFactory::region_name_map;
boost::shared_ptr<Region>
-RegionFactory::create (boost::shared_ptr<const Region> region)
+RegionFactory::create (boost::shared_ptr<const Region> region, bool announce)
{
boost::shared_ptr<Region> ret;
boost::shared_ptr<const AudioRegion> ar;
@@ -72,12 +72,17 @@
}
if (ret) {
+ ret->set_name (new_region_name(ret->name()));
map_add (ret);
/* pure copy constructor - no property list */
/* pure copy constructor - no CheckNewRegion emitted */
+ if (announce) {
+ CheckNewRegion (ret);
+ }
}
+
return ret;
}
|
|
|
Attached my patch to get notification going for a number of operations that were not being accounted for. Note that this will make the region editor show show many more regions which basically reflects what is going on internally. Also there is a small optimization by storing the RowReference to the region being manipulated. This makes single region operations in a session with many regions much smoother. Does not cater for a multi track region (say drums in a group). Over all maintenance of the region list is a major source of slowdown in big session. I am working on a 'delete unused regions' to help out after, say, a big edit session that ends with a final consolidated region. At present we carry all created regions forever. They may not go away since the source is still active in session. |
|
|
Patch has been applied to SVN. |
|
|
Not quite fixed. If you hide automatic regions in the list then there is no updating when a region is moved. |
|
|
Fixed in SVN. |
|
|
Appears fixed |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2010-04-27 23:34 | oofus | New Issue | |
| 2010-04-28 14:53 | paul | Note Added: 0007640 | |
| 2010-04-28 22:28 | cth103 | Status | new => confirmed |
| 2010-07-21 02:40 | cth103 | cost | => 0.00 |
| 2010-07-21 02:40 | cth103 | Target Version | => 3.0-beta1 |
| 2010-09-20 15:46 | lincoln | Note Added: 0009117 | |
| 2010-09-20 18:53 | paul | Note Added: 0009119 | |
| 2010-09-20 22:56 | lincoln | File Added: region-copy-notify.patch | |
| 2010-09-20 23:03 | lincoln | Note Added: 0009122 | |
| 2010-09-21 23:47 | cth103 | Note Added: 0009131 | |
| 2010-09-21 23:47 | cth103 | Status | confirmed => resolved |
| 2010-09-21 23:47 | cth103 | Resolution | open => fixed |
| 2010-09-21 23:47 | cth103 | Assigned To | => cth103 |
| 2010-09-25 13:12 | oofus | Note Added: 0009166 | |
| 2010-09-25 13:12 | oofus | Status | resolved => feedback |
| 2010-09-25 13:12 | oofus | Resolution | fixed => reopened |
| 2010-11-14 16:54 | cth103 | Note Added: 0009405 | |
| 2010-11-14 16:54 | cth103 | Status | feedback => resolved |
| 2010-11-14 16:54 | cth103 | Resolution | reopened => fixed |
| 2010-11-14 22:59 | oofus | Note Added: 0009425 | |
| 2010-11-14 22:59 | oofus | Status | resolved => closed |
