diff --git a/src/WaveTrack.cpp b/src/WaveTrack.cpp index d86a4960d..c8755cd6e 100644 --- a/src/WaveTrack.cpp +++ b/src/WaveTrack.cpp @@ -979,11 +979,16 @@ std::shared_ptr WaveTrack::RemoveAndReturnClip(WaveClip* clip) return {}; } -void WaveTrack::AddClip(std::shared_ptr &&clip) +bool WaveTrack::AddClip(const std::shared_ptr &clip) { + if (clip->GetSequence()->GetFactory() != this->mpFactory) + return false; + // Uncomment the following line after we correct the problem of zero-length clips //if (CanInsertClip(clip)) - mClips.push_back(std::move(clip)); // transfer ownership + mClips.push_back(clip); // transfer ownership + + return true; } /*! @excsafety{Strong} */ @@ -2254,7 +2259,8 @@ bool WaveTrack::CanOffsetClip(WaveClip* clip, double amount, return true; } -bool WaveTrack::CanInsertClip(WaveClip* clip, double &slideBy, double &tolerance) +bool WaveTrack::CanInsertClip( + WaveClip* clip, double &slideBy, double &tolerance) const { for (const auto &c : mClips) { diff --git a/src/WaveTrack.h b/src/WaveTrack.h index 4ac2a45b5..627fcaa69 100644 --- a/src/WaveTrack.h +++ b/src/WaveTrack.h @@ -474,14 +474,14 @@ private: // Before moving a clip into a track (or inserting a clip), use this // function to see if the times are valid (i.e. don't overlap with // existing clips). - bool CanInsertClip(WaveClip* clip, double &slideBy, double &tolerance); + bool CanInsertClip(WaveClip* clip, double &slideBy, double &tolerance) const; // Remove the clip from the track and return a SMART pointer to it. // You assume responsibility for its memory! std::shared_ptr RemoveAndReturnClip(WaveClip* clip); - // Append a clip to the track - void AddClip(std::shared_ptr &&clip); // Call using std::move + //! Append a clip to the track; which must have the same block factory as this track; return success + bool AddClip(const std::shared_ptr &clip); // Merge two clips, that is append data from clip2 to clip1, // then remove clip2 from track. diff --git a/src/effects/Reverse.cpp b/src/effects/Reverse.cpp index ab6297437..d8cccbe90 100644 --- a/src/effects/Reverse.cpp +++ b/src/effects/Reverse.cpp @@ -201,12 +201,13 @@ bool EffectReverse::ProcessOneWave(int count, WaveTrack * track, sampleCount sta // PRL: I don't think that matters, the sequence of storage of clips in the track // is not elsewhere assumed to be by time { - for (auto it = revClips.rbegin(), revEnd = revClips.rend(); it != revEnd; ++it) - track->AddClip(std::move(*it)); + for (auto it = revClips.rbegin(), revEnd = revClips.rend(); rValue && it != revEnd; ++it) + rValue = track->AddClip(*it); } for (auto &clip : otherClips) - track->AddClip(std::move(clip)); + if (!(rValue = track->AddClip(clip))) + break; return rValue; } diff --git a/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp b/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp index 80de1288f..6878fddeb 100644 --- a/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp +++ b/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp @@ -10,6 +10,8 @@ Paul Licameli split from TrackPanel.cpp #include "WaveTrackView.h" +#include + #include "CutlineHandle.h" #include "../../../../Experimental.h" @@ -1344,6 +1346,11 @@ public: bool SyncLocks() override { return true; } + bool MayMigrateTo(Track &other) override + { + return TrackShifter::CommonMayMigrateTo(other); + } + double HintOffsetLarger(double desiredOffset) override { // set it to a sample point, and minimum of 1 sample point @@ -1358,8 +1365,66 @@ public: return desiredOffset; } + Intervals Detach() override + { + for ( auto &interval: mMoving ) { + auto pData = static_cast( interval.Extra() ); + auto pClip = pData->GetClip().get(); + // interval will still hold the clip, so ignore the return: + (void) mpTrack->RemoveAndReturnClip(pClip); + mMigrated.erase(pClip); + } + return std::move( mMoving ); + } + + bool AdjustFit( + const Track &otherTrack, const Intervals &intervals, + double &desiredOffset, double tolerance) override + { + bool ok = true; + auto pOtherWaveTrack = static_cast(&otherTrack); + for ( auto &interval: intervals ) { + auto pData = + static_cast( interval.Extra() ); + auto pClip = pData->GetClip().get(); + ok = pOtherWaveTrack->CanInsertClip( + pClip, desiredOffset, tolerance ); + if( !ok ) + break; + } + return ok; + } + + bool Attach( Intervals intervals ) override + { + for (auto &interval : intervals) { + auto pData = static_cast( interval.Extra() ); + auto pClip = pData->GetClip(); + if ( !mpTrack->AddClip( pClip ) ) + return false; + mMigrated.insert( pClip.get() ); + mMoving.emplace_back( std::move( interval ) ); + } + return true; + } + + bool FinishMigration() override + { + auto rate = mpTrack->GetRate(); + for (auto pClip : mMigrated) { + // Now that user has dropped the clip into a different track, + // make sure the sample rate matches the destination track. + pClip->Resample(rate); + pClip->MarkChanged(); + } + return true; + } + private: std::shared_ptr mpTrack; + + // Clips that may require resampling + std::unordered_set mMigrated; }; using MakeWaveTrackShifter = MakeTrackShifter::Override; diff --git a/src/tracks/ui/TimeShiftHandle.cpp b/src/tracks/ui/TimeShiftHandle.cpp index 946860760..dd0356ee8 100644 --- a/src/tracks/ui/TimeShiftHandle.cpp +++ b/src/tracks/ui/TimeShiftHandle.cpp @@ -164,46 +164,6 @@ namespace viewInfo.selectedRegion.t1() ); } - WaveTrack *NthChannel(WaveTrack &leader, int nn) - { - if (nn < 0) - return nullptr; - return *TrackList::Channels( &leader ).begin().advance(nn); - } - - int ChannelPosition(const Track *pChannel) - { - return static_cast( - TrackList::Channels( pChannel ) - .EndingAfter( pChannel ).size() - ) - 1; - } - - // Don't count right channels. - WaveTrack *NthAudioTrack(TrackList &list, int nn) - { - if (nn >= 0) { - for ( auto pTrack : list.Leaders< WaveTrack >() ) - if (nn -- == 0) - return pTrack; - } - - return NULL; - } - - // Don't count right channels. - int TrackPosition(TrackList &list, const Track *pFindTrack) - { - pFindTrack = *list.FindLeader(pFindTrack); - int nn = 0; - for ( auto pTrack : list.Leaders< const WaveTrack >() ) { - if (pTrack == pFindTrack) - return nn; - ++nn; - } - return -1; - } - WaveClip *FindClipAtTime(WaveTrack *pTrack, double time) { if (pTrack) { @@ -279,6 +239,63 @@ double TrackShifter::HintOffsetLarger(double desiredOffset) return desiredOffset; } +bool TrackShifter::MayMigrateTo(Track &) +{ + return false; +} + +bool TrackShifter::CommonMayMigrateTo(Track &otherTrack) +{ + auto &track = GetTrack(); + + // Both tracks need to be owned to decide this + auto pMyList = track.GetOwner().get(); + auto pOtherList = otherTrack.GetOwner().get(); + if (pMyList && pOtherList) { + + // Can migrate to another track of the same kind... + if ( otherTrack.SameKindAs( track ) ) { + + // ... with the same number of channels ... + auto myChannels = TrackList::Channels( &track ); + auto otherChannels = TrackList::Channels( &otherTrack ); + if (myChannels.size() == otherChannels.size()) { + + // ... and where this track and the other have corresponding + // positions + return myChannels.size() == 1 || + std::distance(myChannels.first, pMyList->Find(&track)) == + std::distance(otherChannels.first, pOtherList->Find(&otherTrack)); + + } + + } + + } + return false; +} + +auto TrackShifter::Detach() -> Intervals +{ + return {}; +} + +bool TrackShifter::AdjustFit( + const Track &, const Intervals&, double &, double) +{ + return false; +} + +bool TrackShifter::Attach( Intervals ) +{ + return true; +} + +bool TrackShifter::FinishMigration() +{ + return true; +} + void TrackShifter::InitIntervals() { mMoving.clear(); @@ -724,63 +741,89 @@ namespace { } } + using Correspondence = std::unordered_map< Track*, Track* >; + bool FindCorrespondence( - TrackList &trackList, Track &track, Track &capturedTrack, + Correspondence &correspondence, + TrackList &trackList, Track &track, ClipMoveState &state) { - const int diff = - TrackPosition(trackList, &track) - - TrackPosition(trackList, &capturedTrack); - for ( auto &trackClip : state.capturedClipArray ) { - if (trackClip.clip) { - // Move all clips up or down by an equal count of audio tracks. - // Can only move between tracks with equal numbers of channels, - // and among corresponding channels. + auto &capturedTrack = state.mCapturedTrack; + auto sameType = [&]( auto pTrack ){ + return capturedTrack->SameKindAs( *pTrack ); + }; + if (!sameType(&track)) + return false; + + // All tracks of the same kind as the captured track + auto range = trackList.Any() + sameType; - Track *const pSrcTrack = trackClip.track; - auto pDstTrack = NthAudioTrack(trackList, - diff + TrackPosition(trackList, pSrcTrack)); - if (!pDstTrack) + // Find how far this track would shift down among those (signed) + const auto myPosition = + std::distance( range.first, trackList.Find( capturedTrack.get() ) ); + const auto otherPosition = + std::distance( range.first, trackList.Find( &track ) ); + auto diff = otherPosition - myPosition; + + // Point to destination track + auto iter = range.first.advance( diff > 0 ? diff : 0 ); + + for (auto pTrack : range) { + auto &pShifter = state.shifters[pTrack]; + if ( !pShifter->MovingIntervals().empty() ) { + // One of the interesting tracks + + auto pOther = *iter; + if ( diff < 0 || !pOther ) + // No corresponding track return false; - if (TrackList::Channels(pSrcTrack).size() != - TrackList::Channels(pDstTrack).size()) + if ( !pShifter->MayMigrateTo(*pOther) ) + // Rejected for other reason return false; - auto pDstChannel = NthChannel( - *pDstTrack, ChannelPosition(pSrcTrack)); - - if (!pDstChannel) { - wxASSERT(false); - return false; - } - - trackClip.dstTrack = pDstChannel; + correspondence[ pTrack ] = pOther; } + + if ( diff < 0 ) + ++diff; // Still consuming initial tracks + else + ++iter; // Safe to increment TrackIter even at end of range } + + // Record the correspondence in TrackClip + for ( auto &trackClip: state.capturedClipArray ) + if ( trackClip.clip ) + trackClip.dstTrack = + dynamic_cast(correspondence[ trackClip.track ]); + return true; } + using DetachedIntervals = + std::unordered_map; + bool CheckFit( - const ViewInfo &viewInfo, wxCoord xx, ClipMoveState &state, + ClipMoveState &state, const Correspondence &correspondence, + const DetachedIntervals &intervals, double tolerance, double &desiredSlideAmount ) { - (void)xx;// Compiler food - (void)viewInfo;// Compiler food bool ok = true; double firstTolerance = tolerance; - + // The desiredSlideAmount may change and the tolerance may get used up. for ( unsigned iPass = 0; iPass < 2 && ok; ++iPass ) { - for ( auto &trackClip : state.capturedClipArray ) { - WaveClip *const pSrcClip = trackClip.clip; - if (pSrcClip) { - ok = trackClip.dstTrack->CanInsertClip( - pSrcClip, desiredSlideAmount, tolerance ); - if( !ok ) - break; - } + for ( auto &pair : state.shifters ) { + auto *pSrcTrack = pair.first; + auto iter = correspondence.find( pSrcTrack ); + if ( iter != correspondence.end() ) + if( auto *pOtherTrack = iter->second ) + if ( !(ok = pair.second->AdjustFit( + *pOtherTrack, intervals.at(pSrcTrack), + desiredSlideAmount /*in,out*/, tolerance)) ) + break; } + // If it fits ok, desiredSlideAmount could have been updated to get // the clip to fit. // Check again, in the new position, this time with zero tolerance. @@ -793,19 +836,20 @@ namespace { return ok; } + [[noreturn]] void MigrationFailure() { + // Tracks may be in an inconsistent state; throw to the application + // handler which restores consistency from undo history + throw SimpleMessageBoxException{ + XO("Could not shift between tracks")}; + } + struct TemporaryClipRemover { TemporaryClipRemover( ClipMoveState &clipMoveState ) : state( clipMoveState ) { // Pluck the moving clips out of their tracks - for ( auto &trackClip : state.capturedClipArray ) { - WaveClip *const pSrcClip = trackClip.clip; - if (pSrcClip) - trackClip.holder = - // Assume track is wave because it has a clip - static_cast(trackClip.track)-> - RemoveAndReturnClip(pSrcClip); - } + for (auto &pair : state.shifters) + detached[pair.first] = pair.second->Detach(); } void Fail() @@ -813,9 +857,11 @@ namespace { // Cause destructor to put all clips back where they came from for ( auto &trackClip : state.capturedClipArray ) trackClip.dstTrack = static_cast(trackClip.track); + failed = true; } - ~TemporaryClipRemover() + void Reinsert( + std::unordered_map< Track*, Track* > &correspondence ) { // Complete (or roll back) the vertical move. // Put moving clips into their destination tracks @@ -824,22 +870,33 @@ namespace { WaveClip *const pSrcClip = trackClip.clip; if (pSrcClip) { const auto dstTrack = trackClip.dstTrack; - dstTrack->AddClip(std::move(trackClip.holder)); trackClip.track = dstTrack; } } + + for (auto &pair : detached) { + auto pTrack = pair.first; + if (!failed && correspondence.count(pTrack)) + pTrack = correspondence[pTrack]; + auto &pShifter = state.shifters[pTrack]; + if (!pShifter->Attach( std::move( pair.second ) )) + MigrationFailure(); + } } ClipMoveState &state; + DetachedIntervals detached; + bool failed = false; }; } bool TimeShiftHandle::DoSlideVertical ( ViewInfo &viewInfo, wxCoord xx, - ClipMoveState &state, TrackList &trackList, Track &capturedTrack, + ClipMoveState &state, TrackList &trackList, Track &dstTrack, double &desiredSlideAmount ) { - if (!FindCorrespondence( trackList, dstTrack, capturedTrack, state)) + Correspondence correspondence; + if (!FindCorrespondence( correspondence, trackList, dstTrack, state )) return false; // Having passed that test, remove clips temporarily from their @@ -854,42 +911,19 @@ bool TimeShiftHandle::DoSlideVertical // i.e. one pixel tolerance at current zoom. double tolerance = viewInfo.PositionToTime(xx + 1) - viewInfo.PositionToTime(xx); - bool ok = CheckFit( viewInfo, xx, state, tolerance, desiredSlideAmount ); + bool ok = CheckFit( state, correspondence, remover.detached, + tolerance, desiredSlideAmount /*in,out*/ ); if (!ok) { // Failure, even with using tolerance. remover.Fail(); - - // Failure -- we'll put clips back where they were - // ok will next indicate if a horizontal slide is OK. - tolerance = 0.0; - desiredSlideAmount = slide; - ok = CheckFit( viewInfo, xx, state, tolerance, desiredSlideAmount ); - for ( auto &trackClip : state.capturedClipArray) { - WaveClip *const pSrcClip = trackClip.clip; - if (pSrcClip){ - - // Attempt to move to a new track did not work. - // Put the clip back appropriately shifted! - if( ok) - trackClip.holder->Offset(slide); - } - } - // Make the offset permanent; start from a "clean slate" - if( ok ) { - state.mMouseClickX = xx; - if (state.movingSelection) { - // Slide the selection, too - viewInfo.selectedRegion.move( slide ); - } - state.hSlideAmount = 0; - } - + remover.Reinsert( correspondence ); return false; } // Make the offset permanent; start from a "clean slate" state.mMouseClickX = xx; + remover.Reinsert( correspondence ); return true; } @@ -951,33 +985,25 @@ UIHandle::Result TimeShiftHandle::Drag *mClipMoveState.mCapturedTrack, *pTrack ); // Scroll during vertical drag. - // EnsureVisible(pTrack); //vvv Gale says this has problems on Linux, per bug 393 thread. Revert for 2.0.2. - bool slidVertically = false; - // If the mouse is over a track that isn't the captured track, // decide which tracks the captured clips should go to. - bool fail = ( + // EnsureVisible(pTrack); //vvv Gale says this has problems on Linux, per bug 393 thread. Revert for 2.0.2. + bool slidVertically = ( mClipMoveState.capturedClip && pTrack != mClipMoveState.mCapturedTrack /* && !mCapturedClipIsSelection*/ && pTrack->TypeSwitch( [&] (WaveTrack *) { - if ( DoSlideVertical( viewInfo, event.m_x, mClipMoveState, - trackList, *mClipMoveState.mCapturedTrack, *pTrack, desiredSlideAmount ) ) { - mClipMoveState.mCapturedTrack = pTrack; - mDidSlideVertically = true; - } - else - return true; - - // Not done yet, check for horizontal movement. - slidVertically = true; + if ( DoSlideVertical( viewInfo, event.m_x, mClipMoveState, + trackList, *pTrack, desiredSlideAmount ) ) { + mClipMoveState.mCapturedTrack = pTrack; + mDidSlideVertically = true; + return true; + } + else return false; - }) + }) ); - if (fail) - return RefreshAll; - if (desiredSlideAmount == 0.0) return RefreshAll; @@ -1026,26 +1052,11 @@ UIHandle::Result TimeShiftHandle::Release if ( !mDidSlideVertically && mClipMoveState.hSlideAmount == 0 ) return result; - - for ( auto &trackClip : mClipMoveState.capturedClipArray ) - { - WaveClip* pWaveClip = trackClip.clip; - // Note that per AddClipsToCaptured(Track *t, double t0, double t1), - // in the non-WaveTrack case, the code adds a NULL clip to capturedClipArray, - // so we have to check for that any time we're going to deref it. - // Previous code did not check it here, and that caused bug 367 crash. - if (pWaveClip && - trackClip.track != trackClip.origTrack) - { - // Now that user has dropped the clip into a different track, - // make sure the sample rate matches the destination track. - // Assume the clip was dropped in a wave track - pWaveClip->Resample - (static_cast(trackClip.track)->GetRate()); - pWaveClip->MarkChanged(); - } - } + for ( auto &pair : mClipMoveState.shifters ) + if ( !pair.second->FinishMigration() ) + MigrationFailure(); + TranslatableString msg; bool consolidate; if (mDidSlideVertically) { diff --git a/src/tracks/ui/TimeShiftHandle.h b/src/tracks/ui/TimeShiftHandle.h index a353daa2f..e70e7a65c 100644 --- a/src/tracks/ui/TimeShiftHandle.h +++ b/src/tracks/ui/TimeShiftHandle.h @@ -75,10 +75,49 @@ public: */ virtual double HintOffsetLarger( double desiredOffset ); + //! Whether intervals may migrate to the other track, not yet checking all placement constraints */ + /*! Default implementation returns false */ + virtual bool MayMigrateTo( Track &otherTrack ); + + //! Remove all moving intervals from the track, if possible + /*! Default implementation does nothing */ + virtual Intervals Detach(); + + //! Test whether intervals can fit into another track, maybe adjusting the offset slightly + /*! Default implementation does nothing and returns false */ + virtual bool AdjustFit( + const Track &otherTrack, + const Intervals &intervals, /*!< + Assume these came from Detach() and only after MayMigrateTo returned true for otherTrack */ + double &desiredOffset, //!< [in,out] + double tolerance //! Nonnegative ceiling for allowed changes in fabs(desiredOffset) + ); + + //! Put moving intervals into the track, which may have migrated from another + /*! @return success + + In case of failure, track states are unspecified + + Default implementation does nothing and returns true */ + virtual bool Attach( Intervals intervals ); + + //! When dragging is done, do (once) the final steps of migration (which may be expensive) + /*! @return success + + In case of failure, track states are unspecified + + Default implementation does nothing and returns true */ + virtual bool FinishMigration(); + protected: /*! Unfix any of the intervals that intersect the given one; may be useful to override `SelectInterval()` */ void CommonSelectInterval( const TrackInterval &interval ); + /*! May be useful to override `MayMigrateTo()`, if certain other needed overrides are given. + Returns true, iff: tracks have same type, and corresponding positions in their channel groups, + which have same size */ + bool CommonMayMigrateTo( Track &otherTrack ); + //! Derived class constructor can initialize all intervals reported by the track as fixed, none moving /*! This can't be called by the base class constructor, when GetTrack() isn't yet callable */ void InitIntervals(); @@ -124,7 +163,6 @@ public: // These fields are used only during time-shift dragging WaveTrack *dstTrack; - std::shared_ptr holder; }; using TrackClipArray = std::vector ; @@ -191,10 +229,10 @@ public: // Try to move clips from one WaveTrack to another, before also moving // by some horizontal amount, which may be slightly adjusted to fit the // destination tracks. - static bool DoSlideVertical - ( ViewInfo &viewInfo, wxCoord xx, - ClipMoveState &state, TrackList &trackList, Track &capturedTrack, - Track &dstTrack, double &desiredSlideAmount ); + static bool DoSlideVertical( + ViewInfo &viewInfo, wxCoord xx, + ClipMoveState &state, TrackList &trackList, + Track &dstTrack, double &desiredSlideAmount ); static UIHandlePtr HitAnywhere (std::weak_ptr &holder,