diff --git a/src/AudacityApp.cpp b/src/AudacityApp.cpp index 6ae8b27f4..d8b40e0d3 100644 --- a/src/AudacityApp.cpp +++ b/src/AudacityApp.cpp @@ -1087,6 +1087,9 @@ bool AudacityApp::OnExceptionInMainLoop() // failed operation pProject->RollbackState(); + // Forget pending changes in the TrackList + pProject->GetTracks()->ClearPendingTracks(); + pProject->RedrawProject(); // Give the user an alert diff --git a/src/Track.cpp b/src/Track.cpp index d35b745ba..787578b3f 100644 --- a/src/Track.cpp +++ b/src/Track.cpp @@ -156,6 +156,17 @@ int Track::GetY() const void Track::SetY(int y) { + auto pList = mList.lock(); + if (pList && !pList->mPendingUpdates.empty()) { + auto orig = pList->FindById( GetId() ); + if (orig && orig != this) { + // delegate, and rely on the update to copy back + orig->SetY(y); + pList->UpdatePendingTracks(); + return; + } + } + DoSetY(y); } @@ -176,6 +187,14 @@ int Track::GetHeight() const void Track::SetHeight(int h) { auto pList = mList.lock(); + if (pList && !pList->mPendingUpdates.empty()) { + auto orig = pList->FindById( GetId() ); + if (orig && orig != this) { + // delegate, and rely on RecalcPositions to copy back + orig->SetHeight(h); + return; + } + } DoSetHeight(h); @@ -198,6 +217,14 @@ bool Track::GetMinimized() const void Track::SetMinimized(bool isMinimized) { auto pList = mList.lock(); + if (pList && !pList->mPendingUpdates.empty()) { + auto orig = pList->FindById( GetId() ); + if (orig && orig != this) { + // delegate, and rely on RecalcPositions to copy back + orig->SetMinimized(isMinimized); + return; + } + } DoSetMinimized(isMinimized); @@ -215,6 +242,14 @@ void Track::DoSetMinimized(bool isMinimized) void Track::SetLinked(bool l) { auto pList = mList.lock(); + if (pList && !pList->mPendingUpdates.empty()) { + auto orig = pList->FindById( GetId() ); + if (orig && orig != this) { + // delegate, and rely on RecalcPositions to copy back + orig->SetLinked(l); + return; + } + } DoSetLinked(l); @@ -777,20 +812,37 @@ TrackList &TrackList::operator= (TrackList &&that) void TrackList::DoAssign(const TrackList &that) { - TrackListConstIterator it(&that); - for (const Track *track = it.First(); track; track = it.Next()) - Add(track->Duplicate()); + auto copyLOT = []( + ListOfTracks &dst, const std::weak_ptr< TrackList > &self, + const ListOfTracks &src ) + { + for (const auto &ptr : src) + dst.push_back( + ListOfTracks::value_type{ ptr->Duplicate().release() } ); + for (auto it = dst.begin(), last = dst.end(); it != last; ++it) + (*it)->SetOwner(self, it); + }; + copyLOT( *this, mSelf, that ); + copyLOT( this->mPendingUpdates, mSelf, that.mPendingUpdates ); + mUpdaters = that.mUpdaters; } void TrackList::Swap(TrackList &that) { - ListOfTracks::swap(that); - for (auto it = ListOfTracks::begin(), last = ListOfTracks::end(); - it != last; ++it) - (*it)->SetOwner(this->mSelf, it); - for (auto it = that.ListOfTracks::begin(), last = that.ListOfTracks::end(); - it != last; ++it) - (*it)->SetOwner(that.mSelf, it); + auto SwapLOTs = []( + ListOfTracks &a, const std::weak_ptr< TrackList > &aSelf, + ListOfTracks &b, const std::weak_ptr< TrackList > &bSelf ) + { + a.swap(b); + for (auto it = a.begin(), last = a.end(); it != last; ++it) + (*it)->SetOwner(aSelf, it); + for (auto it = b.begin(), last = b.end(); it != last; ++it) + (*it)->SetOwner(bSelf, it); + }; + + SwapLOTs( *this, mSelf, that, that.mSelf ); + SwapLOTs( this->mPendingUpdates, mSelf, that.mPendingUpdates, that.mSelf ); + mUpdaters.swap(that.mUpdaters); } TrackList::~TrackList() @@ -821,6 +873,8 @@ void TrackList::RecalcPositions(TrackNodePointer node) t->DoSetY(y); y += t->GetHeight(); } + + UpdatePendingTracks(); } void TrackList::PermutationEvent() @@ -976,9 +1030,17 @@ void TrackList::Clear(bool sendEvent) // shared_ptrs to those tracks. for ( auto pTrack: *this ) pTrack->SetOwner( {}, {} ); + for ( auto pTrack: mPendingUpdates ) + pTrack->SetOwner( {}, {} ); ListOfTracks tempList; tempList.swap( *this ); + + ListOfTracks updating; + updating.swap( mPendingUpdates ); + + mUpdaters.clear(); + if (sendEvent) DeletionEvent(); } @@ -1374,3 +1436,157 @@ double TrackList::GetEndTime() const { return Accumulate(*this, &Track::GetEndTime, doubleMax); } + +std::shared_ptr +TrackList::RegisterPendingChangedTrack( Updater updater, Track *src ) +{ + std::shared_ptr pTrack; + if (src) + // convert from unique_ptr to shared_ptr + pTrack.reset( src->Duplicate().release() ); + + if (pTrack) { + mUpdaters.push_back( updater ); + mPendingUpdates.push_back( pTrack ); + auto n = mPendingUpdates.end(); + --n; + pTrack->SetOwner(mSelf, n); + } + + return pTrack; +} + +void TrackList::RegisterPendingNewTrack( const std::shared_ptr &pTrack ) +{ + auto copy = pTrack; + Add( std::move( copy ) ); + pTrack->SetId( TrackId{} ); +} + +void TrackList::UpdatePendingTracks() +{ + auto pUpdater = mUpdaters.begin(); + for (const auto &pendingTrack : mPendingUpdates) { + // Copy just a part of the track state, according to the update + // function + const auto &updater = *pUpdater; + auto src = FindById( pendingTrack->GetId() ); + if (pendingTrack && src) { + if (updater) + updater( *pendingTrack, *src ); + pendingTrack->DoSetY(src->GetY()); + pendingTrack->DoSetHeight(src->GetHeight()); + pendingTrack->DoSetMinimized(src->GetMinimized()); + pendingTrack->DoSetLinked(src->GetLinked()); + } + ++pUpdater; + } +} + +void TrackList::ClearPendingTracks( ListOfTracks *pAdded ) +// NOFAIL-GUARANTEE +{ + for (const auto &pTrack: mPendingUpdates) + pTrack->SetOwner( {}, {} ); + mPendingUpdates.clear(); + mUpdaters.clear(); + + if (pAdded) + pAdded->clear(); + + for (auto it = ListOfTracks::begin(), stop = ListOfTracks::end(); + it != stop;) { + if (it->get()->GetId() == TrackId{}) { + if (pAdded) + pAdded->push_back( *it ); + it = erase( it ); + } + else + ++it; + } + + if (!empty()) + RecalcPositions(ListOfTracks::begin()); +} + +bool TrackList::ApplyPendingTracks() +{ + bool result = false; + + ListOfTracks additions; + ListOfTracks updates; + { + // Always clear, even if one of the update functions throws + auto cleanup = finally( [&] { ClearPendingTracks( &additions ); } ); + UpdatePendingTracks(); + updates.swap( mPendingUpdates ); + } + + // Remaining steps must be NOFAIL-GUARANTEE so that this function + // gives STRONG-GUARANTEE + + std::vector< std::shared_ptr > reinstated; + + for (auto &pendingTrack : updates) { + if (pendingTrack) { + auto src = FindById( pendingTrack->GetId() ); + if (src) + this->Replace(src, std::move(pendingTrack)), result = true; + else + // Perhaps a track marked for pending changes got deleted by + // some other action. Recreate it so we don't lose the + // accumulated changes. + reinstated.push_back(pendingTrack); + } + } + + // If there are tracks to reinstate, append them to the list. + for (auto &pendingTrack : reinstated) + if (pendingTrack) + this->Add(std::move(pendingTrack)), result = true; + + // Put the pending added tracks back into the list, preserving their + // positions. + bool inserted = false; + ListOfTracks::iterator first; + for (auto &pendingTrack : additions) { + if (pendingTrack) { + auto iter = ListOfTracks::begin(); + std::advance( iter, pendingTrack->GetIndex() ); + iter = ListOfTracks::insert( iter, pendingTrack ); + pendingTrack->SetOwner( mSelf, iter ); + pendingTrack->SetId( TrackId{ ++sCounter } ); + if (!inserted) { + first = iter; + inserted = true; + } + } + } + if (inserted) { + RecalcPositions(first); + result = true; + } + + return result; +} + +std::shared_ptr TrackList::FindPendingChangedTrack(TrackId id) const +{ + // Linear search. Tracks in a project are usually very few. + auto it = std::find_if( mPendingUpdates.begin(), mPendingUpdates.end(), + [=](const ListOfTracks::value_type &ptr){ return ptr->GetId() == id; } ); + if (it == mPendingUpdates.end()) + return {}; + return *it; +} + +bool TrackList::HasPendingTracks() const +{ + if ( !mPendingUpdates.empty() ) + return true; + if (end() != std::find_if(begin(), end(), [](const Track *t){ + return t->GetId() == TrackId{}; + })) + return true; + return false; +} diff --git a/src/Track.h b/src/Track.h index e65e59380..5c5c702bf 100644 --- a/src/Track.h +++ b/src/Track.h @@ -16,6 +16,7 @@ #include "MemoryX.h" #include #include +#include #include #include #include @@ -68,6 +69,7 @@ class ViewInfo; // It does not persist between sessions // Default constructed value is not equal to the id of any track that has ever // been added to a TrackList, or (directly or transitively) copied from such +// (A pending additional track that is not yet applied is not considered added) // TrackIds are assigned uniquely across projects class TrackId { @@ -747,7 +749,8 @@ class TrackList final : public wxEvtHandler, public ListOfTracks private: bool isNull(TrackNodePointer p) const - { return p == ListOfTracks::end(); } + { return p == ListOfTracks::end() + || p == mPendingUpdates.end(); } TrackNodePointer getEnd() const { return const_cast(this)->ListOfTracks::end(); } TrackNodePointer getBegin() const @@ -788,6 +791,61 @@ private: // Nonpersistent. // Used to assign ids to added tracks. static long sCounter; + +public: + using Updater = std::function< void(Track &dest, const Track &src) >; + // Start a deferred update of the project. + // The return value is a duplicate of the given track. + // While ApplyPendingTracks or ClearPendingTracks is not yet called, + // there may be other direct changes to the project that push undo history. + // Meanwhile the returned object can accumulate other changes for a deferred + // push, and temporarily shadow the actual project track for display purposes. + // The Updater function, if not null, merges state (from the actual project + // into the pending track) which is not meant to be overridden by the + // accumulated pending changes. + // To keep the display consistent, the Y and Height values, minimized state, + // and Linked state must be copied, and this will be done even if the + // Updater does not do it. + // Pending track will have the same TrackId as the actual. + // Pending changed tracks will not occur in iterations. + std::shared_ptr RegisterPendingChangedTrack( + Updater updater, + Track *src + ); + + // Like the previous, but for a NEW track, not a replacement track. Caller + // supplies the track, and there are no updates. + // Pending track will have an unassigned TrackId. + // Pending changed tracks WILL occur in iterations, always after actual + // tracks, and in the sequence that they were added. They can be + // distinguished from actual tracks by TrackId. + void RegisterPendingNewTrack( const std::shared_ptr &pTrack ); + + // Invoke the updaters of pending tracks. Pass any exceptions from the + // updater functions. + void UpdatePendingTracks(); + + // Forget pending track additions and changes; + // if requested, give back the pending added tracks. + void ClearPendingTracks( ListOfTracks *pAdded = nullptr ); + + // Change the state of the project. + // Strong guarantee for project state in case of exceptions. + // Will always clear the pending updates. + // Return true if the state of the track list really did change. + bool ApplyPendingTracks(); + + // Find anything registered with RegisterPendingChangedTrack and not yet + // cleared or applied + std::shared_ptr FindPendingChangedTrack(TrackId id) const; + + bool HasPendingTracks() const; + +private: + // Need to put pending tracks into a list so that GetLink() works + ListOfTracks mPendingUpdates; + // This is in correspondence with mPendingUpdates + std::vector< Updater > mUpdaters; }; class AUDACITY_DLL_API TrackFactory diff --git a/src/TrackPanel.cpp b/src/TrackPanel.cpp index 8eec62ad6..aa1bfa50d 100644 --- a/src/TrackPanel.cpp +++ b/src/TrackPanel.cpp @@ -655,6 +655,10 @@ namespace // TODO: make a finer distinction between refreshing the track control area, // and the waveform area. As it is, redraw both whenever you must redraw either. + // Copy data from the underlying tracks to the pending tracks that are + // really displayed + panel->GetProject()->GetTracks()->UpdatePendingTracks(); + using namespace RefreshCode; if (refreshResult & DestroyedCell) { diff --git a/src/UndoManager.cpp b/src/UndoManager.cpp index 4b54223ed..2933270ee 100644 --- a/src/UndoManager.cpp +++ b/src/UndoManager.cpp @@ -236,11 +236,11 @@ void UndoManager::ModifyState(const TrackList * l, // Duplicate auto tracksCopy = TrackList::Create(); - TrackListConstIterator iter(l); - const Track *t = iter.First(); - while (t) { + for (auto t : *l) { + if ( t->GetId() == TrackId{} ) + // Don't copy a pending added track + continue; tracksCopy->Add(t->Duplicate()); - t = iter.Next(); } // Replace @@ -280,11 +280,11 @@ void UndoManager::PushState(const TrackList * l, } auto tracksCopy = TrackList::Create(); - TrackListConstIterator iter(l); - const Track *t = iter.First(); - while (t) { + for (auto t : *l) { + if ( t->GetId() == TrackId{} ) + // Don't copy a pending added track + continue; tracksCopy->Add(t->Duplicate()); - t = iter.Next(); } // Assume tags was duplicted before any changes.