instrument: the filter's velocity depth knob returns and multiplies the bipolar curve; the pre-v12 lift is a pure domain re-tag

2026-07-31 20:24:40 -04:00
parent cfb53aade3
commit 5e290119c5
22 changed files with 243 additions and 166 deletions
@@ -93,18 +93,16 @@ namespace reasampler::instrument::map {
 // the same shape as v7's), appended after the loop crossfade. Its y is a normalized fraction
 // of kVelocityPitchRangeSemitones (play_params.h) — a full-scale constant that lives OUTSIDE
 // this frozen ladder, so retuning it re-tunes every saved v12 project's pitch-curve throw. It
-// also RE-INTERPRETS two frozen slots inside the v9 filter tail — the byte shape is untouched,
-// only the meaning at v12+:
-//   * the filter's velocity curve is now BIPOLAR [-1,+1] and is the whole velocity->cutoff
-//     amount, not a [0,1] shape scaled by a separate depth;
-//   * the retired filter velAmount slot is written as a constant 1.0 and ignored on read.
-// PRE-v12 LIFT: the stored [0,1] filter curve has every knot's y multiplied by that blob's
-// velAmount and is re-read as bipolar. eval is homogeneous in y to within double rounding (see
-// velocity_curve.h), so the lifted curve evaluates to velAmount * oldCurve(v) — the product the
-// voice used to compute per note — and a pre-v12 project sounds identical. A pre-v12 blob
-// carries no pitch curve at all and lifts to the bipolar flat-at-zero default, which transposes
-// nothing. A DOWNGRADE to a pre-v12 binary reads the constant 1.0 depth against a curve whose
-// negative half clamps away, so it reproduces the curve's positive half only.
+// also RE-TAGS the DOMAIN of one frozen slot inside the v9 filter tail: that curve's y is read
+// as BIPOLAR [-1,+1] from v12 on, having been UNIPOLAR [0,1] before. Every other filter slot,
+// velAmount included, keeps its meaning — the cutoff contribution is still
+// velAmount * curve(velocity).
+// PRE-v12 LIFT: a domain re-tag and nothing more. A pre-v12 curve's stored y values all lie in
+// [0,1], which is inside [-1,+1], so the widened box-clamp alters no knot and eval is unchanged
+// at every velocity — a pre-v12 project sounds identical without any rounding argument. A
+// pre-v12 blob carries no pitch curve at all and lifts to the bipolar flat-at-zero default,
+// which transposes nothing. A DOWNGRADE to a pre-v12 binary re-narrows the domain, so a curve
+// drawn into the negative half comes back with that half clamped to 0.
 //
 // The two int64 slots the v5 play tail spends on the RETIRED Trigger fade pair are frozen in
 // shape and still read: a pre-v10 blob's fade-in/fade-out become the Trigger AHD that replaced
@@ -156,9 +154,9 @@ inline constexpr std::uint32_t kParamsCurveVersion = 10;
 // v10 + the loop-crossfade frame count.
 inline constexpr std::uint32_t kParamsLoopVersion = 11;

-// v11 + the velocity->pitch curve, and the version from which the filter's velocity curve is
-// bipolar and self-scaling. Both the appended tail and the filter-tail lift branch on THIS,
-// never on kParamsPayloadVersion.
+// v11 + the velocity->pitch curve; the appended tail branches on THIS, never on
+// kParamsPayloadVersion. The filter curve's v12 domain re-tag needs no branch of its own — a
+// pre-v12 curve's y values are already valid bipolar ones.
 inline constexpr std::uint32_t kParamsVelocityVersion = 12;

 // (No nominal-rate constant.) The legacy v3 payload's wall-clock frame counts convert to
@@ -93,12 +93,11 @@ void readSecondsPlayTail(ByteReader& r, InstrumentParams& p, double projectRate)
    p.play.adsr.releaseSeconds = bitsToDouble(r.u64());
 }

-// Read a velocity curve tail into `curve`, interpreting its y values in `domain` and scaling
-// them by `yScale` (the pre-v12 filter lift folds a retired depth in that way — see
-// component_state_io.h). fromPoints repairs the X-order/endpoint invariant defensively; a
-// truncated read leaves `curve` at whatever default it came in with.
+// Read a velocity curve tail into `curve`, interpreting its y values in `domain` — the domain
+// is not on the wire, it is a property of the slot. fromPoints repairs the X-order/endpoint
+// invariant defensively; a truncated read leaves `curve` at whatever default it came in with.
 void readCurveTail(ByteReader& r, VelocityCurve& curve,
-                   reasampler::instrument::engine::CurveDomain domain, double yScale) {
+                   reasampler::instrument::engine::CurveDomain domain) {
    const std::uint32_t ptCount = r.u32();
    std::vector<VelocityPoint> pts;
    // Bound the reserve to what the blob can hold (16 bytes/point) so a corrupt huge count
@@ -108,7 +107,7 @@ void readCurveTail(ByteReader& r, VelocityCurve& curve,
    for (std::uint32_t i = 0; i < ptCount && r.ok; ++i) {
        const double vel = bitsToDouble(r.u64());
        const double value = bitsToDouble(r.u64());
-        pts.push_back(VelocityPoint{vel, value * yScale});
+        pts.push_back(VelocityPoint{vel, value});
    }
    if (r.ok) {
        curve = reasampler::instrument::engine::VelocityCurve::fromPoints(std::move(pts), domain);
@@ -116,10 +115,10 @@ void readCurveTail(ByteReader& r, VelocityCurve& curve,
 }

 // Read the v9 filter tail into `p`. A blob that stops short leaves the off/neutral default,
-// which is what makes a v8 blob play bit-identically under the new codec. `preVelocityVersion`
-// selects the pre-v12 lift: the frozen velAmount slot is folded into the curve's knots instead
-// of being kept as a separate depth.
-void readFilterTail(ByteReader& r, InstrumentParams& p, bool preVelocityVersion) {
+// which is what makes a v8 blob play bit-identically under the new codec. The curve reads as
+// bipolar at EVERY version — a pre-v12 blob's y values are already valid bipolar ones, so its
+// v12 domain re-tag needs no version branch (see component_state_io.h).
+void readFilterTail(ByteReader& r, InstrumentParams& p) {
    FilterSeconds& f = p.play.filter;
    f.enabled = (r.u8() != 0);
    f.settings.cutoffNorm    = static_cast<float>(bitsToDouble(r.u64()));
@@ -128,27 +127,20 @@ void readFilterTail(ByteReader& r, InstrumentParams& p, bool preVelocityVersion)
    f.settings.driveNorm     = static_cast<float>(bitsToDouble(r.u64()));
    f.settings.morphLaw = (r.u8() != 0) ? engine::filter::MorphLaw::HighNotchLow
                                        : engine::filter::MorphLaw::HighBandLow;
-    // Same non-finite-falls-back-to-neutral guard as the v8 master gain above: these reach
-    // Voice::tickFilterCutoff's clamp compares and a static_cast<int>, both UB on NaN.
+    // Same non-finite-falls-back-to-neutral guard as the v8 master gain above: these three
+    // reach Voice::tickFilterCutoff's clamp compares and a static_cast<int>, both UB on NaN.
    double modAmount = bitsToDouble(r.u64());
    double velAmount = bitsToDouble(r.u64());
    double keyTrack  = bitsToDouble(r.u64());
    f.modAmount = std::isfinite(modAmount) ? modAmount : 0.0;
+    f.velAmount = std::isfinite(velAmount) ? velAmount : 0.0;
    f.keyTrack  = std::isfinite(keyTrack) ? keyTrack : 0.0;
    f.env.attackSeconds  = bitsToDouble(r.u64());
    f.env.holdSeconds    = bitsToDouble(r.u64());
    f.env.decaySeconds   = bitsToDouble(r.u64());
    f.env.sustainLevel   = bitsToDouble(r.u64());
    f.env.releaseSeconds = bitsToDouble(r.u64());
-    // velAmount feeds a MULTIPLIER on the stored curve's knots (below), not a param the engine
-    // clamps on its own — the UI never dials it outside [-1,1] (deckBipolarFromNorm), so a
-    // corrupt-but-finite blob value outside that range must clamp here rather than silently
-    // scaling the lifted curve past what fromPoints' own [-1,1] box-clamp would then truncate.
-    const double velFold =
-        preVelocityVersion ? std::clamp(std::isfinite(velAmount) ? velAmount : 0.0, -1.0, 1.0)
-                            : 1.0;
-    readCurveTail(r, f.velocityCurve, reasampler::instrument::engine::CurveDomain::Bipolar,
-                  velFold);
+    readCurveTail(r, f.velocityCurve, reasampler::instrument::engine::CurveDomain::Bipolar);
 }

 // A curve exponent off the wire. A corrupt/non-finite value degrades to the LINEAR neutral
@@ -255,7 +247,7 @@ PayloadRead readLegacyZonePayload(ByteReader& r, std::uint32_t pv, double projec
        if (keyTrackTail) p.keyTrack = bitsToDouble(r.u64());
        if (curveTail) {
            readCurveTail(r, p.velocityCurve,
-                          reasampler::instrument::engine::CurveDomain::Unipolar, 1.0);
+                          reasampler::instrument::engine::CurveDomain::Unipolar);
        }
        // Payload version 4 (a branch-only frames tail, never shipped) and any unknown pv
        // leave the seconds product defaults on p.play.
@@ -315,9 +307,7 @@ void putParamsPayload(std::vector<std::uint8_t>& out, const InstrumentParams& p)
    putLE(out, doubleToBits(static_cast<double>(f.settings.driveNorm)));
    out.push_back(f.settings.morphLaw == engine::filter::MorphLaw::HighNotchLow ? 1 : 0);
    putLE(out, doubleToBits(f.modAmount));
-    // The retired filter velAmount's frozen slot: a constant 1.0 so a pre-v12 binary reading
-    // this blob scales the curve by unity rather than silencing it (see component_state_io.h).
-    putLE(out, doubleToBits(1.0));
+    putLE(out, doubleToBits(f.velAmount));
    putLE(out, doubleToBits(f.keyTrack));
    putLE(out, doubleToBits(f.env.attackSeconds));
    putLE(out, doubleToBits(f.env.holdSeconds));
@@ -370,10 +360,8 @@ PayloadRead readParamsPayload(ByteReader& r, double projectRate) {
    if (hasStart) p.startPoint = r.i64();
    readSecondsPlayTail(r, p, projectRate);
    p.keyTrack = bitsToDouble(r.u64());
-    readCurveTail(r, p.velocityCurve, reasampler::instrument::engine::CurveDomain::Unipolar, 1.0);
-    if (pv >= kParamsFilterVersion) {
-        readFilterTail(r, p, /*preVelocityVersion=*/pv < kParamsVelocityVersion);
-    }
+    readCurveTail(r, p.velocityCurve, reasampler::instrument::engine::CurveDomain::Unipolar);
+    if (pv >= kParamsFilterVersion) readFilterTail(r, p);
    if (pv >= kParamsCurveVersion) readCurveStageTail(r, p);
    if (pv >= kParamsLoopVersion) {
        // A negative fade is meaningless and would reach resolveLoop's clamp anyway; refusing
@@ -383,7 +371,7 @@ PayloadRead readParamsPayload(ByteReader& r, double projectRate) {
    }
    if (pv >= kParamsVelocityVersion) {
        readCurveTail(r, p.play.pitchVelocityCurve,
-                      reasampler::instrument::engine::CurveDomain::Bipolar, 1.0);
+                      reasampler::instrument::engine::CurveDomain::Bipolar);
    }
    // A truncated record leaves whatever parsed plus construction defaults for the rest —
    // the same degrade-don't-throw contract the zone ladder always had.
@@ -244,6 +244,7 @@ PlayParams resolvePlay(const PlaySeconds& stored, int sampleRate) {
    out.filter.enabled   = stored.filter.enabled;
    out.filter.settings  = stored.filter.settings;
    out.filter.modAmount = stored.filter.modAmount;
+    out.filter.velAmount = stored.filter.velAmount;
    out.filter.keyTrack  = stored.filter.keyTrack;
    out.filter.velocityCurve   = stored.filter.velocityCurve;
    out.filter.env.attackFrames  = secToFrames(stored.filter.env.attackSeconds);
@@ -183,6 +183,7 @@ struct FilterSeconds {
    bool enabled = false;
    engine::filter::FilterSettings settings;
    double modAmount = 0.0;
+    double velAmount = 0.0;
    double keyTrack  = 0.0;
    AdsrSeconds env{0.0, 0.0, 0.0, 1.0, 0.0};  // Gate
    AhdSeconds  trigEnv;                       // Trigger