Improve rendering and drawing of envelope with discontinuity

Change the drawing of wave track envelope at high zoom...
  Change evaluation of envelope near discontinuities...
  Prefer "sampleDur" to "sampleTime" for some variable names

src/Envelope.cpp

@@ -606,7 +606,7 @@ bool EnvelopeEditor::MouseEvent(const wxMouseEvent & event, wxRect & r,
    return false;
 }
 
-void Envelope::CollapseRegion( double t0, double t1, double sampleTime )
+void Envelope::CollapseRegion( double t0, double t1, double sampleDur )
 // NOFAIL-GUARANTEE
 {
    // This gets called when somebody clears samples.
@@ -615,7 +615,7 @@ void Envelope::CollapseRegion( double t0, double t1, double sampleTime )
    // For the boundaries of the interval, preserve the left-side limit at the
    // start and right-side limit at the end.
 
-   const auto epsilon = sampleTime / 2;
+   const auto epsilon = sampleDur / 2;
    t0 = std::max( 0.0, std::min( mTrackLen, t0 - mOffset ) );
    t1 = std::max( 0.0, std::min( mTrackLen, t1 - mOffset ) );
 
@@ -983,28 +983,15 @@ void Envelope::GetPoints(double *bufferWhen,
    }
 }
 
-void Envelope::Cap( double sampleTime )
+void Envelope::Cap( double sampleDur )
 {
-   auto range = EqualRange( mTrackLen, sampleTime );
+   auto range = EqualRange( mTrackLen, sampleDur );
    if ( range.first == range.second )
       InsertOrReplaceRelative( mTrackLen, GetValueRelative( mTrackLen ) );
 }
 
 // Private methods
 
-// We no longer tolerate multiple envelope control points at the exact
-// same t; the behavior can be well-defined, but it is still incorrect
-// in that it vastly complicates paste operations behaving as a user
-// reasonably expects.  The most common problem occurs pasting an
-// envelope into another track; the boundary behavior causes the
-// t=insert_point envelope level of the insertee to apply to sample 0
-// of the inserted sample, causing a pop.  This most visibly manifests
-// itself in undo and mixing when a v=1.0 sample magically shows
-// up at boundaries causing a pop.
-
-// Although this renders the name a slight misnomer, a duplicate
-// 'replaces' the current control point.
-
 /** @brief Add a control point to the envelope
  *
  * @param when the time in seconds when the envelope point should be created.
@@ -1061,13 +1048,13 @@ int Envelope::InsertOrReplaceRelative(double when, double value)
    return i;
 }
 
-std::pair<int, int> Envelope::EqualRange( double when, double sampleTime ) const
+std::pair<int, int> Envelope::EqualRange( double when, double sampleDur ) const
 {
    // Find range of envelope points matching the given time coordinate
-   // (within an interval of length sampleTime)
+   // (within an interval of length sampleDur)
    // by binary search; if empty, it still indicates where to
    // insert.
-   const auto tolerance = sampleTime / 2;
+   const auto tolerance = sampleDur / 2;
    auto begin = mEnv.begin();
    auto end = mEnv.end();
    auto first = std::lower_bound(
@@ -1090,11 +1077,11 @@ void Envelope::SetOffset(double newOffset)
    mOffset = newOffset;
 }
 
-void Envelope::SetTrackLen( double trackLen, double sampleTime )
+void Envelope::SetTrackLen( double trackLen, double sampleDur )
 // NOFAIL-GUARANTEE
 {
    // Preserve the left-side limit at trackLen.
-   auto range = EqualRange( trackLen, sampleTime );
+   auto range = EqualRange( trackLen, sampleDur );
    bool needPoint = ( range.first == range.second && trackLen < mTrackLen );
    double value;
    if ( needPoint )
@@ -1127,12 +1114,12 @@ void Envelope::RescaleTimes( double newLength )
 }
 
 // Accessors
-double Envelope::GetValue(double t) const
+double Envelope::GetValue( double t, double sampleDur ) const
 {
    // t is absolute time
    double temp;
 
-   GetValues(&temp, 1, t, 1.0);
+   GetValues( &temp, 1, t, sampleDur );
    return temp;
 }
 
@@ -1140,7 +1127,7 @@ double Envelope::GetValueRelative(double t) const
 {
    double temp;
 
-   GetValuesRelative(&temp, 1, t, 1.0);
+   GetValuesRelative(&temp, 1, t, 0.0);
    return temp;
 }
 
@@ -1219,9 +1206,14 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen,
    // JC: If bufferLen ==0 we have probably just allocated a zero sized buffer.
    // wxASSERT( bufferLen > 0 );
 
+   const auto epsilon = tstep / 2;
    int len = mEnv.size();
 
    double t = t0;
+   double increment = 0;
+   if ( len > 0 && t <= mEnv[0].GetT() && mEnv[0].GetT() == mEnv[1].GetT() )
+      increment = epsilon;
+
    double tprev, vprev, tnext = 0, vnext, vstep = 0;
 
    for (int b = 0; b < bufferLen; b++) {
@@ -1233,20 +1225,24 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen,
          t += tstep;
          continue;
       }
+
+      auto tplus = t + increment;
+
       // IF before envelope THEN first value
-      if (t <= mEnv[0].GetT()) {
+      if ( tplus <= mEnv[0].GetT() ) {
          buffer[b] = mEnv[0].GetVal();
          t += tstep;
          continue;
       }
       // IF after envelope THEN last value
-      if (t >= mEnv[len - 1].GetT()) {
+      if ( tplus >= mEnv[len - 1].GetT() ) {
          buffer[b] = mEnv[len - 1].GetVal();
          t += tstep;
          continue;
       }
 
-      if (b == 0 || t > tnext) {
+      // Note >= not > , to get the right limit in case epsilon == 0
+      if ( b == 0 || tplus >= tnext ) {
 
          // We're beyond our tnext, so find the next one.
          // Don't just increment lo or hi because we might
@@ -1254,12 +1250,23 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen,
          // points to move over.  That's why we binary search.
 
          int lo,hi;
-         BinarySearchForTime( lo, hi, t );
+         BinarySearchForTime( lo, hi, tplus );
-         // mEnv[0] is before t because of eliminations above, therefore lo >= 0
+         // mEnv[0] is before tplus because of eliminations above, therefore lo >= 0
-         // mEnv[len - 1] is after t, therefore hi <= len - 1
+         // mEnv[len - 1] is after tplus, therefore hi <= len - 1
+
          tprev = mEnv[lo].GetT();
          tnext = mEnv[hi].GetT();
 
+         if ( hi + 1 < len && tnext == mEnv[ hi + 1 ].GetT() )
+            // There is a discontinuity after this point-to-point interval.
+            // Will stop evaluating in this interval when time is slightly
+            // before tNext, then use the right limit.  This is the right intent
+            // in case small roundoff errors cause a sample time to be a little
+            // before the envelope point time.
+            increment = epsilon;
+         else
+            increment = 0;
+
          vprev = GetInterpolationStartValueAtPoint( lo );
          vnext = GetInterpolationStartValueAtPoint( hi );
 
@@ -1299,10 +1306,45 @@ void Envelope::GetValuesRelative(double *buffer, int bufferLen,
 }
 
 void Envelope::GetValues
-   (double *buffer, int bufferLen, int leftOffset, const ZoomInfo &zoomInfo) const
+   ( double alignedTime, double sampleDur,
+     double *buffer, int bufferLen, int leftOffset,
+     const ZoomInfo &zoomInfo )
+   const
 {
-   for (int xx = 0; xx < bufferLen; ++xx)
+   // Getting many envelope values, corresponding to pixel columns, which may
-      buffer[xx] = GetValue(zoomInfo.PositionToTime(xx, -leftOffset));
+   // not be uniformly spaced in time when there is a fisheye.
+
+   double prevDiscreteTime, prevSampleVal, nextSampleVal;
+   for ( int xx = 0; xx < bufferLen; ++xx ) {
+      auto time = zoomInfo.PositionToTime( xx, -leftOffset );
+      if ( sampleDur <= 0 )
+         // Sample interval not defined (as for time track)
+         buffer[xx] = GetValue( time );
+      else {
+         // The level of zoom-in may resolve individual samples.
+         // If so, then instead of evaluating the envelope directly,
+         // we draw a piecewise curve with knees at each sample time.
+         // This actually makes clearer what happens as you drag envelope
+         // points and make discontinuities.
+         auto leftDiscreteTime = alignedTime +
+            sampleDur * floor( ( time - alignedTime ) / sampleDur );
+         if ( xx == 0 || leftDiscreteTime != prevDiscreteTime ) {
+            prevDiscreteTime = leftDiscreteTime;
+            prevSampleVal =
+               GetValue( prevDiscreteTime, sampleDur );
+            nextSampleVal =
+               GetValue( prevDiscreteTime + sampleDur, sampleDur );
+         }
+         auto ratio = ( time - leftDiscreteTime ) / sampleDur;
+         if ( GetExponential() )
+            buffer[ xx ] = exp(
+               ( 1.0 - ratio ) * log( prevSampleVal )
+                  + ratio * log( nextSampleVal ) );
+         else
+            buffer[ xx ] =
+               ( 1.0 - ratio ) * prevSampleVal + ratio * nextSampleVal;
+      }
+   }
 }
 
 // relative time

src/Envelope.h

@@ -118,9 +118,9 @@ public:
       float zoomMin, float zoomMax, bool mirrored) const;
 
    // Handling Cut/Copy/Paste events
-   // sampleTime determines when the endpoint of the collapse is near enough
+   // sampleDur determines when the endpoint of the collapse is near enough
    // to an endpoint of the domain, that an extra control point is not needed.
-   void CollapseRegion(double t0, double t1, double sampleTime);
+   void CollapseRegion(double t0, double t1, double sampleDur);
    void Paste(double t0, const Envelope *e);
 
    void InsertSpace(double t0, double tlen);
@@ -128,13 +128,13 @@ public:
 
    // Control
    void SetOffset(double newOffset);
-   void SetTrackLen( double trackLen, double sampleTime = 0.0 );
+   void SetTrackLen( double trackLen, double sampleDur = 0.0 );
    void RescaleValues(double minValue, double maxValue);
    void RescaleTimes( double newLength );
 
    // Accessors
    /** \brief Get envelope value at time t */
-   double GetValue(double t) const;
+   double GetValue( double t, double sampleDur = 0 ) const;
 
    /** \brief Get many envelope points at once.
     *
@@ -142,13 +142,16 @@ public:
     * more than one value in a row. */
    void GetValues(double *buffer, int len, double t0, double tstep) const;
 
-   /** \brief Get many envelope points at once, but don't assume uniform time step.
+   /** \brief Get many envelope points for pixel columns at once,
+    * but don't assume uniform time per pixel.
    */
    void GetValues
-      (double *buffer, int bufferLen, int leftOffset, const ZoomInfo &zoomInfo) const;
+      ( double aligned_time, double sampleDur,
+        double *buffer, int bufferLen, int leftOffset,
+        const ZoomInfo &zoomInfo) const;
 
    // Guarantee an envelope point at the end of the domain.
-   void Cap( double sampleTime );
+   void Cap( double sampleDur );
 
 private:
    double GetValueRelative(double t) const;
@@ -198,7 +201,7 @@ private:
       return mEnv[index];
    }
 
-   std::pair<int, int> EqualRange( double when, double sampleTime ) const;
+   std::pair<int, int> EqualRange( double when, double sampleDur ) const;
 
 public:
    /** \brief Returns the sets of when and value pairs */

src/TimeTrack.cpp

@@ -281,7 +281,8 @@ void TimeTrack::Draw(wxDC & dc, const wxRect & r, const ZoomInfo &zoomInfo) cons
    mRuler->Draw(dc, this);
 
    Doubles envValues{ size_t(mid.width) };
-   GetEnvelope()->GetValues(envValues.get(), mid.width, 0, zoomInfo);
+   GetEnvelope()->GetValues
+      ( 0, 0, envValues.get(), mid.width, 0, zoomInfo );
 
    dc.SetPen(AColor::envelopePen);
 

src/TrackArtist.cpp

@@ -1352,7 +1352,10 @@ void TrackArtist::DrawIndividualSamples(wxDC &dc, int leftOffset, const wxRect &
             (int)(zoomInfo.TimeToPosition(time, -leftOffset))));
       xpos[s] = xx;
 
-      const double tt = buffer[s] * clip->GetEnvelope()->GetValue(time);
+      // Calculate sample as it would be rendered, so quantize time
+      double value =
+         clip->GetEnvelope()->GetValue( time, 1.0 / clip->GetRate() );
+      const double tt = buffer[s] * value;
 
       if (clipped && mShowClipping && ((tt <= -MAX_AUDIO) || (tt >= MAX_AUDIO)))
          clipped[clipcnt++] = xx;
@@ -1778,7 +1781,8 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track,
 
    std::vector<double> vEnv(mid.width);
    double *const env = &vEnv[0];
-   clip->GetEnvelope()->GetValues(env, mid.width, leftOffset, zoomInfo);
+   clip->GetEnvelope()->GetValues
+      ( tOffset, 1.0 / rate, env, mid.width, leftOffset, zoomInfo );
 
    // Draw the background of the track, outlining the shape of
    // the envelope and using a colored pen for the selected
@@ -1909,7 +1913,8 @@ void TrackArtist::DrawClipWaveform(const WaveTrack *track,
          if (!showIndividualSamples) {
             std::vector<double> vEnv2(rect.width);
             double *const env2 = &vEnv2[0];
-            clip->GetEnvelope()->GetValues(env2, rect.width, leftOffset, zoomInfo);
+            clip->GetEnvelope()->GetValues
+               ( tOffset, 1.0 / rate, env2, rect.width, leftOffset, zoomInfo );
             DrawMinMaxRMS(dc, rect, env2,
                zoomMin, zoomMax,
                dB, dBRange,

src/TrackPanel.cpp

@@ -4625,7 +4625,8 @@ float TrackPanel::FindSampleEditingLevel(wxMouseEvent &event, double dBRange, do
    Envelope *const env = mDrawingTrack->GetEnvelopeAtX(event.m_x);
    if (env)
    {
-      double envValue = env->GetValue(t0);
+      // Calculate sample as it would be rendered, so quantize time
+      double envValue = env->GetValue( t0, 1.0 / mDrawingTrack->GetRate() );
       if (envValue > 0)
          newLevel /= envValue;
       else
@@ -6904,7 +6905,7 @@ bool TrackPanel::HitTestEnvelope(Track *track, const wxRect &rect, const wxMouse
 
    // Get envelope point, range 0.0 to 1.0
    const bool dB = !wavetrack->GetWaveformSettings().isLinear();
-   // Convert x to time.
+
    const double envValue = envelope->GetValue(mViewInfo->PositionToTime(event.m_x, rect.x));
 
    float zoomMin, zoomMax;
@@ -6992,7 +6993,8 @@ bool TrackPanel::HitTestSamples(Track *track, const wxRect &rect, const wxMouseE
    double envValue = 1.0;
    Envelope* env = wavetrack->GetEnvelopeAtX(event.GetX());
    if (env)
-      envValue = env->GetValue(tt);
+      // Calculate sample as it would be rendered, so quantize time
+      envValue = env->GetValue( tt, 1.0 / wavetrack->GetRate() );
 
    int yValue = GetWaveYPos( oneSample * envValue,
       zoomMin, zoomMax,

src/WaveTrack.cpp

@@ -2130,7 +2130,7 @@ void WaveTrack::GetEnvelopeValues(double *buffer, size_t bufferLen,
    // Since this does not guarantee that the entire buffer is filled with values we need
    // to initialize the entire buffer to a default value.
    //
-   // This does mean that, in the cases where a usuable clip is located, the buffer value will
+   // This does mean that, in the cases where a usable clip is located, the buffer value will
    // be set twice.  Unfortunately, there is no easy way around this since the clips are not
    // stored in increasing time order.  If they were, we could just track the time as the
    // buffer is filled.
@@ -2180,6 +2180,8 @@ void WaveTrack::GetEnvelopeValues(double *buffer, size_t bufferLen,
             rlen = limitSampleBufferSize( rlen, nClipLen );
             rlen = std::min(rlen, size_t(floor(0.5 + (dClipEndTime - rt0) / tstep)));
          }
+         // Samples are obtained for the purpose of rendering a wave track,
+         // so quantize time
          clip->GetEnvelope()->GetValues(rbuf, rlen, rt0, tstep);
       }
    }

src/WaveTrack.h

@@ -259,8 +259,12 @@ class AUDACITY_DLL_API WaveTrack final : public PlayableTrack {
                    fillFormat fill = fillZero, bool mayThrow = true) const;
    void Set(samplePtr buffer, sampleFormat format,
                    sampleCount start, size_t len);
+
+   // Fetch envelope values corresponding to uniformly separated sample times
+   // starting at the given time.
    void GetEnvelopeValues(double *buffer, size_t bufferLen,
                          double t0) const;
+
    std::pair<float, float> GetMinMax(
       double t0, double t1, bool mayThrow = true) const;
    float GetRMS(double t0, double t1, bool mayThrow = true) const;