Changes in WaveClip construction...

... Eliminate CreateFromCopy, add new one-step constructor instead.
It was wasteful to create a copy only to re-create the Sequence at once.

Sequence::Copy is a factory returning a unique_ptr.

Some error checks are removed, but there will be exceptions instead later.

src/Sequence.cpp

@@ -385,12 +385,10 @@ bool Sequence::GetRMS(sampleCount start, sampleCount len,
    return true;
 }
 
-bool Sequence::Copy(sampleCount s0, sampleCount s1, std::unique_ptr<Sequence> &dest) const
+std::unique_ptr<Sequence> Sequence::Copy(sampleCount s0, sampleCount s1) const
 {
-   dest.reset();
-
    if (s0 >= s1 || s0 >= mNumSamples || s1 < 0)
-      return false;
+      return {};
 
    int numBlocks = mBlock.size();
 
@@ -402,7 +400,7 @@ bool Sequence::Copy(sampleCount s0, sampleCount s1, std::unique_ptr<Sequence> &d
    wxUnusedVar(numBlocks);
    wxASSERT(b0 <= b1);
 
-   dest = std::make_unique<Sequence>(mDirManager, mSampleFormat);
+   auto dest = std::make_unique<Sequence>(mDirManager, mSampleFormat);
    dest->mBlock.reserve(b1 - b0 + 1);
 
    SampleBuffer buffer(mMaxSamples, mSampleFormat);
@@ -445,7 +443,10 @@ bool Sequence::Copy(sampleCount s0, sampleCount s1, std::unique_ptr<Sequence> &d
          dest->AppendBlock(block); // Increase ref count or duplicate file
    }
 
-   return ConsistencyCheck(wxT("Sequence::Copy()"));
+   if (! ConsistencyCheck(wxT("Sequence::Copy()")))
+      return {};
+
+   return dest;
 }
 
 namespace {

src/Sequence.h

@@ -103,7 +103,7 @@ class PROFILE_DLL_API Sequence final : public XMLTagHandler{
    bool GetWaveDisplay(float *min, float *max, float *rms, int* bl,
                        size_t len, const sampleCount *where);
 
-   bool Copy(sampleCount s0, sampleCount s1, std::unique_ptr<Sequence> &dest) const;
+   std::unique_ptr<Sequence> Copy(sampleCount s0, sampleCount s1) const;
    bool Paste(sampleCount s0, const Sequence *src);
 
    size_t GetIdealAppendLen() const;

src/WaveClip.cpp

@@ -302,16 +302,14 @@ static void ComputeSpectrumUsingRealFFTf
 
 WaveClip::WaveClip(const std::shared_ptr<DirManager> &projDirManager, sampleFormat format, int rate)
 {
-   mOffset = 0;
    mRate = rate;
    mSequence = std::make_unique<Sequence>(projDirManager, format);
+
    mEnvelope = std::make_unique<Envelope>();
+
    mWaveCache = std::make_unique<WaveCache>();
    mSpecCache = std::make_unique<SpecCache>();
    mSpecPxCache = std::make_unique<SpecPxCache>(1);
-   mAppendBufferLen = 0;
-   mDirty = 0;
-   mIsPlaceholder = false;
 }
 
 WaveClip::WaveClip(const WaveClip& orig, const std::shared_ptr<DirManager> &projDirManager)
@@ -323,10 +321,12 @@ WaveClip::WaveClip(const WaveClip& orig, const std::shared_ptr<DirManager> &proj
    mOffset = orig.mOffset;
    mRate = orig.mRate;
    mSequence = std::make_unique<Sequence>(*orig.mSequence, projDirManager);
+
    mEnvelope = std::make_unique<Envelope>();
    mEnvelope->Paste(0.0, orig.mEnvelope.get());
    mEnvelope->SetOffset(orig.GetOffset());
    mEnvelope->SetTrackLen((orig.mSequence->GetNumSamples().as_double()) / orig.mRate);
+
    mWaveCache = std::make_unique<WaveCache>();
    mSpecCache = std::make_unique<SpecCache>();
    mSpecPxCache = std::make_unique<SpecPxCache>(1);
@@ -334,11 +334,38 @@ WaveClip::WaveClip(const WaveClip& orig, const std::shared_ptr<DirManager> &proj
    for (const auto &clip: orig.mCutLines)
       mCutLines.push_back(make_movable<WaveClip>(*clip, projDirManager));
 
-   mAppendBufferLen = 0;
-   mDirty = 0;
    mIsPlaceholder = orig.GetIsPlaceholder();
 }
 
+WaveClip::WaveClip(const WaveClip& orig,
+                   const std::shared_ptr<DirManager> &projDirManager,
+                   double t0, double t1)
+{
+   // Copy only a range of the other WaveClip
+
+   mOffset = orig.mOffset;
+   mRate = orig.mRate;
+
+   mWaveCache = std::make_unique<WaveCache>();
+   mSpecCache = std::make_unique<SpecCache>();
+   mSpecPxCache = std::make_unique<SpecPxCache>(1);
+
+   mIsPlaceholder = orig.GetIsPlaceholder();
+
+   sampleCount s0, s1;
+
+   orig.TimeToSamplesClip(t0, &s0);
+   orig.TimeToSamplesClip(t1, &s1);
+
+   mSequence = orig.mSequence->Copy(s0, s1);
+
+   mEnvelope = std::make_unique<Envelope>();
+   mEnvelope->CopyFrom(orig.mEnvelope.get(),
+                       mOffset + s0.as_double()/mRate,
+                       mOffset + s1.as_double()/mRate);
+}
+
+
 WaveClip::~WaveClip()
 {
 }
@@ -1480,30 +1507,6 @@ void WaveClip::WriteXML(XMLWriter &xmlFile) const
    xmlFile.EndTag(wxT("waveclip"));
 }
 
-bool WaveClip::CreateFromCopy(double t0, double t1, const WaveClip* other)
-{
-   sampleCount s0, s1;
-
-   other->TimeToSamplesClip(t0, &s0);
-   other->TimeToSamplesClip(t1, &s1);
-
-   std::unique_ptr<Sequence> oldSequence = std::move(mSequence);
-   if (!other->mSequence->Copy(s0, s1, mSequence))
-   {
-      mSequence = std::move(oldSequence);
-      return false;
-   }
-
-   mEnvelope = std::make_unique<Envelope>();
-   mEnvelope->CopyFrom(other->mEnvelope.get(),
-      mOffset + s0.as_double()/mRate,
-      mOffset + s1.as_double()/mRate);
-
-   MarkChanged();
-
-   return true;
-}
-
 bool WaveClip::Paste(double t0, const WaveClip* other)
 {
    const bool clipNeedsResampling = other->mRate != mRate;
@@ -1638,18 +1641,11 @@ bool WaveClip::ClearAndAddCutLine(double t0, double t1)
    if (t0 > GetEndTime() || t1 < GetStartTime())
       return true; // time out of bounds
 
-   auto newClip = make_movable<WaveClip>
+   const double clip_t0 = std::max( t0, GetStartTime() );
-      (mSequence->GetDirManager(), mSequence->GetSampleFormat(), mRate);
+   const double clip_t1 = std::min( t1, GetEndTime() );
-   double clip_t0 = t0;
-   double clip_t1 = t1;
-   if (clip_t0 < GetStartTime())
-      clip_t0 = GetStartTime();
-   if (clip_t1 > GetEndTime())
-      clip_t1 = GetEndTime();
 
-   newClip->SetOffset(this->mOffset);
+   auto newClip = make_movable<WaveClip>
-   if (!newClip->CreateFromCopy(clip_t0, clip_t1, this))
+      (*this, mSequence->GetDirManager(), clip_t0, clip_t1);
-      return false;
    newClip->SetOffset(clip_t0-mOffset);
 
    // Sort out cutlines that belong to the NEW cutline

src/WaveClip.h

@@ -217,6 +217,11 @@ public:
    // from one project to another
    WaveClip(const WaveClip& orig, const std::shared_ptr<DirManager> &projDirManager);
 
+   // Copy only a range from the given WaveClip
+   WaveClip(const WaveClip& orig,
+            const std::shared_ptr<DirManager> &projDirManager,
+            double t0, double t1);
+
    virtual ~WaveClip();
 
    void ConvertToSampleFormat(sampleFormat format);
@@ -266,9 +271,6 @@ public:
     * has changed, like when member functions SetSamples() etc. are called. */
    void MarkChanged() { mDirty++; }
 
-   /// Create clip from copy, discarding previous information in the clip
-   bool CreateFromCopy(double t0, double t1, const WaveClip* other);
-
    /** Getting high-level data from the for screen display and clipping
     * calculations and Contrast */
    bool GetWaveDisplay(WaveDisplay &display,
@@ -367,35 +369,35 @@ public:
    XMLTagHandler *HandleXMLChild(const wxChar *tag) override;
    void WriteXML(XMLWriter &xmlFile) const /* not override */;
 
-   // Cache of values to colour pixels of Spectrogram - used by TrackArtist
-   mutable std::unique_ptr<SpecPxCache> mSpecPxCache;
-
    // AWD, Oct 2009: for pasting whitespace at the end of selection
    bool GetIsPlaceholder() const { return mIsPlaceholder; }
    void SetIsPlaceholder(bool val) { mIsPlaceholder = val; }
 
-protected:
+public:
-   mutable wxRect mDisplayRect;
+   // Cache of values to colour pixels of Spectrogram - used by TrackArtist
+   mutable std::unique_ptr<SpecPxCache> mSpecPxCache;
 
-   double mOffset;
+protected:
+   mutable wxRect mDisplayRect {};
+
+   double mOffset { 0 };
    int mRate;
-   int mDirty;
+   int mDirty { 0 };
-   bool mIsCutLine;
    std::unique_ptr<Sequence> mSequence;
    std::unique_ptr<Envelope> mEnvelope;
 
    mutable std::unique_ptr<WaveCache> mWaveCache;
-   mutable ODLock       mWaveCacheMutex;
+   mutable ODLock       mWaveCacheMutex {};
    mutable std::unique_ptr<SpecCache> mSpecCache;
-   SampleBuffer  mAppendBuffer;
+   SampleBuffer  mAppendBuffer {};
-   size_t        mAppendBufferLen;
+   size_t        mAppendBufferLen { 0 };
 
    // Cut Lines are nothing more than ordinary wave clips, with the
    // offset relative to the start of the clip.
-   WaveClipHolders mCutLines;
+   WaveClipHolders mCutLines {};
 
    // AWD, Oct. 2009: for whitespace-at-end-of-selection pasting
-   bool mIsPlaceholder;
+   bool mIsPlaceholder { false };
 };
 
 #endif

src/WaveTrack.cpp

@@ -654,20 +654,17 @@ Track::Holder WaveTrack::Copy(double t0, double t1, bool forClipboard) const
          newClip->RemoveAllCutLines();
          newClip->Offset(-t0);
       }
-      else
+      else if (t1 > clip->GetStartTime() && t0 < clip->GetEndTime())
-      if (t1 > clip->GetStartTime() && t0 < clip->GetEndTime())
       {
          // Clip is affected by command
          //printf("copy: clip %i is affected by command\n", (int)clip);
 
-         auto newClip = make_movable<WaveClip>(*clip, mDirManager);
+         const double clip_t0 = std::max(t0, clip->GetStartTime());
+         const double clip_t1 = std::min(t1, clip->GetEndTime());
+
+         auto newClip = make_movable<WaveClip>
+            (*clip, mDirManager, clip_t0, clip_t1);
          newClip->RemoveAllCutLines();
-         double clip_t0 = t0;
-         double clip_t1 = t1;
-         if (clip_t0 < clip->GetStartTime())
-            clip_t0 = clip->GetStartTime();
-         if (clip_t1 > clip->GetEndTime())
-            clip_t1 = clip->GetEndTime();
 
          //printf("copy: clip_t0=%f, clip_t1=%f\n", clip_t0, clip_t1);
 
@@ -675,23 +672,9 @@ Track::Holder WaveTrack::Copy(double t0, double t1, bool forClipboard) const
          if (newClip->GetOffset() < 0)
             newClip->SetOffset(0);
 
-         //printf("copy: clip offset is now %f\n", newClip->GetOffset());
-
-         if (!newClip->CreateFromCopy(clip_t0, clip_t1, clip.get()))
-         {
-            //printf("paste: CreateFromCopy(%f, %f, %i) returns false, quitting\n",
-            //   clip_t0, clip_t1, (int)clip);
-            // JKC: July 2007, previously we did 'return false' here which
-            // could leave *dest undefined.
-            // I think this is dealing with clips that don't have any sequence content
-            // i.e. we don't copy cut lines and such - anyone like to explain more?
-         }
-         else
-         {
          newTrack->mClips.push_back(std::move(newClip)); // transfer ownership
       }
    }
-   }
 
    // AWD, Oct 2009: If the selection ends in whitespace, create a placeholder
    // clip representing that whitespace