From 0a1298663924f97cd6c66e7d72844c69e28df91f Mon Sep 17 00:00:00 2001 From: Ben Loftis Date: Fri, 22 Sep 2023 18:19:38 +0200 Subject: [PATCH] Preserve existing region-group relationships This solves several issues related to splitting or pasting regions, when there is more than one layer. Rather than assign a new group-id for "all the regions on the right of a split", only ions that had a *prior* group-relationship should be propagated into the new group. Signed-off-by: Robin Gareus --- libs/ardour/ardour/region.h | 34 ++++++++++++++++++++++++---------- libs/ardour/midi_playlist.cc | 2 +- libs/ardour/playlist.cc | 8 ++++---- libs/ardour/region.cc | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/libs/ardour/ardour/region.h b/libs/ardour/ardour/region.h index 00e575ce9a..789790b1a1 100644 --- a/libs/ardour/ardour/region.h +++ b/libs/ardour/ardour/region.h @@ -91,6 +91,12 @@ enum LIBARDOUR_API RegionEditState { EditChangesID = 2 }; +enum LIBARDOUR_API RegionOperationFlag { + LeftOfSplit = 0, + InnerSplit = 1, // when splitting a Range, there's left/center/right parts of the split + RightOfSplit = 2, + Paste = 4 +}; class LIBARDOUR_API Region : public SessionObject @@ -170,9 +176,11 @@ public: struct RegionGroupRetainer { RegionGroupRetainer () { + Glib::Threads::Mutex::Lock lm (_operation_rgroup_mutex); if (_retained_group_id == 0) { - _next_group_id++; - _retained_group_id = _next_group_id << 4; + ++_next_group_id; + _operation_rgroup_map.clear (); // this is used for split & paste operations that honor the region's prior grouping + _retained_group_id = _next_group_id << 4; // this is used for newly created regions via recording or importing _clear_on_destruction = true; } else { _clear_on_destruction = false; @@ -181,7 +189,9 @@ public: ~RegionGroupRetainer () { if (_clear_on_destruction) { + Glib::Threads::Mutex::Lock lm (_operation_rgroup_mutex); _retained_group_id = 0; + _operation_rgroup_map.clear(); } } bool _clear_on_destruction; @@ -190,14 +200,16 @@ public: static uint64_t next_group_id () { return _next_group_id; } static void set_next_group_id (uint64_t ngid) { _next_group_id = ngid; } - /* access the shared group-id, potentially with an Alt identifier (for left/center/right splits) */ - static uint64_t get_retained_group_id (bool alt = false) - { - return _retained_group_id | (alt ? Alt : NoGroup); + /* access the retained group-id for actions like Recording, Import */ + static uint64_t get_retained_group_id () { + return _retained_group_id; } + /* access the group-id for an operation on a region, honoring the existing region's group status */ + static uint64_t get_region_operation_group_id (uint64_t old_region_group, RegionOperationFlag flags); + uint64_t region_group () const { return _reg_group; } - void set_region_group (bool explicitly, bool alt = false) { _reg_group = get_retained_group_id (alt) | (explicitly ? Explicit : NoGroup); } + void set_region_group (bool explicitly) { _reg_group = get_retained_group_id () | (explicitly ? Explicit : NoGroup); } void unset_region_group () { _reg_group = Explicit; } bool is_explicitly_grouped() { return (_reg_group & Explicit) == Explicit; } @@ -599,13 +611,15 @@ private: void use_sources (SourceList const &); enum RegionGroupFlags : uint64_t { - NoGroup = 0x0, //no flag: implicitly grouped if the id is nonzero; or implicitly 'un-grouped' if the group-id is zero - Explicit = 0x1, //the user has explicitly grouped or ungrouped this region. explicitly grouped regions can cross track-group boundaries - Alt = 0x8, //this accommodates some operations (separate) that generate left/center/right region groups. add more bits here, if needed + NoGroup = 0x0, // no flag: implicitly grouped if the id is nonzero; or implicitly 'un-grouped' if the group-id is zero + Explicit = 0x1, // the user has explicitly grouped or ungrouped this region. explicitly grouped regions can cross track-group boundaries }; static uint64_t _retained_group_id; static uint64_t _next_group_id; + static Glib::Threads::Mutex _operation_rgroup_mutex; + static std::map _operation_rgroup_map; + std::atomic _source_deleted; Glib::Threads::Mutex _source_list_lock; PBD::ScopedConnectionList _source_deleted_connections; diff --git a/libs/ardour/midi_playlist.cc b/libs/ardour/midi_playlist.cc index 0fd984a9cd..83644446a3 100644 --- a/libs/ardour/midi_playlist.cc +++ b/libs/ardour/midi_playlist.cc @@ -233,7 +233,7 @@ MidiPlaylist::_split_region (std::shared_ptr region, timepos_t const & p plist.add (Properties::length, after); plist.add (Properties::name, after_name); plist.add (Properties::right_of_split, true); - plist.add (Properties::reg_group, Region::get_retained_group_id()); + plist.add (Properties::reg_group, Region::get_region_operation_group_id (region->region_group(), RightOfSplit)); /* same note as above */ right = RegionFactory::create (region, before, plist, true, &thawlist); diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc index 42a3bd89c3..583c45790f 100644 --- a/libs/ardour/playlist.cc +++ b/libs/ardour/playlist.cc @@ -1063,7 +1063,7 @@ Playlist::partition_internal (timepos_t const & start, timepos_t const & end, bo plist.add (Properties::automatic, true); plist.add (Properties::left_of_split, true); plist.add (Properties::right_of_split, true); - plist.add (Properties::reg_group, Region::get_retained_group_id(true)); + plist.add (Properties::reg_group, Region::get_region_operation_group_id (current->region_group(), InnerSplit)); /* see note in ::_split_region() */ @@ -1083,7 +1083,7 @@ Playlist::partition_internal (timepos_t const & start, timepos_t const & end, bo plist.add (Properties::name, new_name); plist.add (Properties::automatic, true); plist.add (Properties::right_of_split, true); - plist.add (Properties::reg_group, Region::get_retained_group_id()); + plist.add (Properties::reg_group, Region::get_region_operation_group_id (current->region_group(), RightOfSplit)); region = RegionFactory::create (current, pos1.distance (pos3), plist, true, &thawlist ); @@ -1161,7 +1161,7 @@ Playlist::partition_internal (timepos_t const & start, timepos_t const & end, bo plist.add (Properties::name, new_name); plist.add (Properties::automatic, true); plist.add (Properties::right_of_split, true); - plist.add (Properties::reg_group, Region::get_retained_group_id()); + plist.add (Properties::reg_group, Region::get_region_operation_group_id (current->region_group(), RightOfSplit)); region = RegionFactory::create (current, plist, true, &thawlist); @@ -1539,7 +1539,7 @@ Playlist::_split_region (std::shared_ptr region, timepos_t const & play plist.add (Properties::length, after); plist.add (Properties::name, after_name); plist.add (Properties::right_of_split, true); - plist.add (Properties::reg_group, Region::get_retained_group_id()); + plist.add (Properties::reg_group, Region::get_region_operation_group_id (region->region_group(), RightOfSplit)); /* same note as above */ right = RegionFactory::create (region, before, plist, true, &thawlist); diff --git a/libs/ardour/region.cc b/libs/ardour/region.cc index 471c40bcc3..38084879ee 100644 --- a/libs/ardour/region.cc +++ b/libs/ardour/region.cc @@ -95,6 +95,39 @@ PBD::Signal2,const PropertyChange&> Reg uint64_t Region::_retained_group_id = 0; uint64_t Region::_next_group_id = 0; +std::map Region::_operation_rgroup_map; +Glib::Threads::Mutex Region::_operation_rgroup_mutex; + +/* access the group-id for an operation on a region, honoring the existing region's group status */ +uint64_t +Region::get_region_operation_group_id (uint64_t old_region_group, RegionOperationFlag flags) { + /* if the region was ungrouped, stay ungrouped */ + if ((old_region_group == NoGroup) || (old_region_group == Explicit)) { + return old_region_group; + } + + /* separate and preserve the Explicit flag: */ + bool expl = (old_region_group & Explicit) == Explicit; + + /* remove all flags */ + old_region_group = (old_region_group >> 4) << 4; + + /* apply flags to create a key, which will be used to recognize regions that belong in the same group */ + uint64_t region_group_key = old_region_group | flags; + + /* since the GUI is single-threaded, and it's hard/impossible to edit + * during a rec-stop, this 'should' never be contentious + */ + Glib::Threads::Mutex::Lock lm (_operation_rgroup_mutex); + + /* if a region group has not been assigned for this key, assign one */ + if (_operation_rgroup_map.find (region_group_key) == _operation_rgroup_map.end ()) { + _operation_rgroup_map[region_group_key] = _next_group_id++; + } + + return ((_operation_rgroup_map[region_group_key] << 4) | expl); +} + void Region::make_property_quarks () {