fix: S12 R2 — marker-drag sets selectedZone_, ADSR rate-resolve, gateAdsr doc

Marker drag now sets selectedZone_ so single-capture controls stay reachable. Lifted/default ADSR rescales nominal 44100-Hz frame counts by sampleRate/44100 via adsrNeedsRateResolve; v4-authored zones skip rescale. gateAdsr documented vestigial.
This commit is contained in:
2026-07-27 02:01:43 -04:00
parent 7fb778206c
commit 1d338318e7
7 changed files with 192 additions and 41 deletions
+37 -18
View File
@@ -269,32 +269,31 @@ ReaSamplerEditor::SetupMarkers ReaSamplerEditor::pickedMarkers(std::int64_t fram
return m;
}
void ReaSamplerEditor::upsertPickedOverride(const SetupMarkers& m) {
int ReaSamplerEditor::upsertPickedOverride(const SetupMarkers& m) {
// Find-or-append the zone for selectedId_ and write the loop/start override fields.
// The bank intrinsic is NEVER written (read-only bank consumer, D-B). selectedId_ must
// be non-empty; callers are responsible for that guard.
// Returns the zone index (0-based) so callers can update selectedZone_.
SampleLoop loop;
loop.hasLoop = m.hasLoop;
loop.start = m.loopStart;
loop.end = m.loopEnd;
bool found = false;
for (PerformanceZone& z : map_.zones) {
for (int i = 0; i < static_cast<int>(map_.zones.size()); ++i) {
PerformanceZone& z = map_.zones[static_cast<std::size_t>(i)];
if (z.sampleId == selectedId_) {
z.loopOverride = loop;
z.startPoint = m.start;
found = true;
break;
return i;
}
}
if (!found) {
PerformanceZone z;
z.sampleId = selectedId_;
z.lowNote = 0;
z.highNote = 127;
z.loopOverride = loop;
z.startPoint = m.start;
map_.zones.push_back(z);
}
PerformanceZone z;
z.sampleId = selectedId_;
z.lowNote = 0;
z.highNote = 127;
z.loopOverride = loop;
z.startPoint = m.start;
map_.zones.push_back(z);
return static_cast<int>(map_.zones.size()) - 1;
}
namespace {
@@ -1322,6 +1321,14 @@ void ReaSamplerEditor::onMouseDown(int x, int y) {
dragParamPanel_ = panel;
dragStartMap_ = map_;
applyControl(id, z.play, valueAtPoint(r.control, x), 0);
// An explicit ADSR slider touch commits a rate-resolved value (the slider
// maps 0..1 -> editor-domain frames at kEnvTimeMaxFrames, not nominal 44100-Hz
// counts). Mark the zone as no longer needing rate-resolve so buildZonedKeymap
// does not re-rescale the value at reload time.
if (id >= static_cast<int>(ParamControl::kAttack) &&
id <= static_cast<int>(ParamControl::kRelease)) {
z.adsrNeedsRateResolve = false;
}
invalidate(); // live feedback; commit on WM_LBUTTONUP
}
break;
@@ -1352,9 +1359,18 @@ void ReaSamplerEditor::onMouseMove(int x, int y) {
// zone over the whole keyboard) and round-trips through the v3 component state; the
// zone becomes visible if the user opens the Zones panel. Upsert by the picked id so a
// repeated drag edits the same zone rather than stacking duplicates.
// Upsert the root override on the picked id; track the zone index so the control panel
// stays visible after the zone is materialized on the single-capture face (fix: without
// setting selectedZone_ here, selectedZone_==-1 with a non-empty map hides controls).
bool found = false;
for (PerformanceZone& z : map_.zones) {
if (z.sampleId == selectedId_) { z.rootOverride = note; found = true; break; }
for (int i = 0; i < static_cast<int>(map_.zones.size()); ++i) {
PerformanceZone& z = map_.zones[static_cast<std::size_t>(i)];
if (z.sampleId == selectedId_) {
z.rootOverride = note;
selectedZone_ = i;
found = true;
break;
}
}
if (!found) {
PerformanceZone z;
@@ -1363,6 +1379,7 @@ void ReaSamplerEditor::onMouseMove(int x, int y) {
z.highNote = 127;
z.rootOverride = note;
map_.zones.push_back(z);
selectedZone_ = static_cast<int>(map_.zones.size()) - 1;
}
invalidate(); // live feedback; the commit lands on WM_LBUTTONUP
return;
@@ -1407,8 +1424,10 @@ void ReaSamplerEditor::onMouseMove(int x, int y) {
if (m.start > frames - 1) m.start = frames - 1;
// Upsert the override on the picked id (mirror of the root-marker path); commit lands on
// release, this is live feedback.
upsertPickedOverride(m);
// release, this is live feedback. Set selectedZone_ so the control panel stays visible
// after the zone is materialized (fix: without this, selectedZone_==-1 with a non-empty
// map hides controls after the first marker drag on the single-capture face).
selectedZone_ = upsertPickedOverride(m);
invalidate();
return;
}
+3 -2
View File
@@ -172,8 +172,9 @@ private:
// Write `m` as a loop/start override upsert into map_ for selectedId_ (find-or-append).
// Does NOT call commitAndReload — callers decide whether this is a live-drag update or a
// final commit. selectedId_ must be non-empty before calling.
void upsertPickedOverride(const SetupMarkers& m);
// final commit. selectedId_ must be non-empty before calling. Returns the zone index
// (0-based) that was updated or appended, so callers can set selectedZone_.
int upsertPickedOverride(const SetupMarkers& m);
// --- S12/S15/S16 parameter surface (Zones panel, keyed to selectedZone_) ------
//
+35
View File
@@ -147,6 +147,23 @@ Keymap buildTier0Keymap(std::vector<AudioSample> frames, int sampleRate,
data.rootNote = rootNote;
data.loop = loop;
data.play = play; // S15/S16 single-capture play params (product defaults unless overridden)
// The default `play` arg carries 44100-Hz nominal ADSR frame counts (kTier0Nominal*).
// Rescale A/D/R by (sampleRate / 44100) so the wall-clock ADSR matches tier0Adsr(sampleRate)
// exactly. Sustain (a level, not a frame count) is unchanged. At 44100 the factor is 1.0 —
// bit-identical to the pre-fix build. The tier-0 single-capture path never carries user-edited
// ADSR (users edit ADSR through zones, which go through buildZonedKeymap), so always rescaling
// here is correct and safe.
if (data.sampleRate != 44100) {
const double factor = static_cast<double>(data.sampleRate) / 44100.0;
data.play.adsr.attackFrames = static_cast<std::int64_t>(
static_cast<double>(data.play.adsr.attackFrames) * factor + 0.5);
data.play.adsr.decayFrames = static_cast<std::int64_t>(
static_cast<double>(data.play.adsr.decayFrames) * factor + 0.5);
data.play.adsr.releaseFrames = static_cast<std::int64_t>(
static_cast<double>(data.play.adsr.releaseFrames) * factor + 0.5);
}
return Keymap::singleSampleChromatic(std::move(data));
}
@@ -190,6 +207,8 @@ ResolvedPerformance resolvePerformance(const std::string& banksJson,
// S15/S16 per-zone play params carry through unchanged (they are instrument state, not
// resolved against the bank) so the keymap build can stamp them onto the SampleData.
rz.play = z.play;
// Carry the rate-resolve flag so buildZonedKeymap can rescale 44100-nominal ADSR counts.
rz.adsrNeedsRateResolve = z.adsrNeedsRateResolve;
out.zones.push_back(std::move(rz));
}
return out;
@@ -215,6 +234,20 @@ Keymap buildZonedKeymap(const std::vector<ResolvedZone>& zones,
data.loop = zones[i].loop;
data.startFrame = zones[i].startFrame; // S11 effective start (override, else 0)
data.play = zones[i].play; // S15/S16 per-zone play mode + engine + envelopes
// If the zone's ADSR came from a v3 lift or a new-zone default (adsrNeedsRateResolve),
// its A/D/R frame counts are 44100-Hz nominals. Rescale to the WAV's actual rate so
// wall-clock ADSR durations match tier0Adsr(sampleRate) exactly. v4 zones (user-edited
// frame counts) carry adsrNeedsRateResolve=false and are left unchanged.
if (zones[i].adsrNeedsRateResolve && data.sampleRate != 44100) {
const double factor = static_cast<double>(data.sampleRate) / 44100.0;
data.play.adsr.attackFrames = static_cast<std::int64_t>(
static_cast<double>(data.play.adsr.attackFrames) * factor + 0.5);
data.play.adsr.decayFrames = static_cast<std::int64_t>(
static_cast<double>(data.play.adsr.decayFrames) * factor + 0.5);
data.play.adsr.releaseFrames = static_cast<std::int64_t>(
static_cast<double>(data.play.adsr.releaseFrames) * factor + 0.5);
}
const std::size_t sampleIndex = km.samples.size();
km.samples.push_back(std::move(data));
KeyZone zone;
@@ -420,10 +453,12 @@ void readZonesPayload(ByteReader& r, PerformanceMap& map) {
}
if (hasAdsr) {
// S12 review fix (v4): the full per-zone A/D/S/R tail — read in the emit order.
// These values were authored at the DAW's rate; mark them resolved (no rescaling).
z.play.adsr.attackFrames = r.i64();
z.play.adsr.decayFrames = r.i64();
z.play.adsr.sustainLevel = bitsToDouble(r.u64());
z.play.adsr.releaseFrames = r.i64();
z.adsrNeedsRateResolve = false; // already rate-resolved; do NOT rescale at keymap build
} else if (hasPlay) {
// v3 blob: A/D/S/R fields are absent. Lift to the tier-0 nominal defaults (44100 Hz)
// so a voice playing this zone sounds bit-identical to the pre-v4 build (back-compat).
+20 -7
View File
@@ -108,11 +108,13 @@ std::vector<AudioSample> extractChannel(const std::vector<AudioSample>& interlea
int channelCount, int which);
// Nominal tier-0 ADSR defaults (frames at 44100 Hz) — used when lifting a pre-v4 zones payload
// blob whose per-zone A/D/S/R fields are absent. These reproduce the instrument-wide tier0Adsr
// behavior (0.003 s attack, 0.0 s decay, sustain 1.0, 0.060 s release at 44100 Hz) so that a
// zone loaded from an old blob sounds bit-identical to the pre-v4 build. DAWs at other sample
// rates will be close but not exact (the same approximation the instrument already makes when
// building tier0Adsr from its compile-time kAttackSeconds / kReleaseSeconds constants).
// blob whose per-zone A/D/S/R fields are absent, and as the initializer for new zones / the
// buildTier0Keymap default play arg. These are 44100-Hz nominal frame counts; buildTier0Keymap
// and buildZonedKeymap both rescale the A/D/R frame counts by (sampleRate / 44100) at build
// time when adsrNeedsRateResolve is set on the zone — so a v3-lifted or default zone plays with
// the same wall-clock ADSR as tier0Adsr(liveRate) did before the fix. Bit-identical at 44100 Hz.
// Zones loaded from a v4 blob (explicitly user-edited) carry adsrNeedsRateResolve=false and are
// never rescaled — their stored frame counts already reflect the rate at which they were authored.
// Must be declared before buildTier0Keymap (default arg) and PerformanceZone / ResolvedZone
// (member initializers) — both of which reference these values.
inline constexpr std::int64_t kTier0NominalAttackFrames = 132; // 0.003 * 44100, rounded
@@ -129,8 +131,9 @@ inline constexpr std::int64_t kTier0NominalReleaseFrames = 2646; // 0.060 * 441
// pair never half-plays. `sampleRate` is the WAV's rate.
// `play` carries the S15/S16 per-zone play params for the single-capture path; it defaults to
// the PRODUCT defaults (Gate + Preserve engine, S16-F1) so a picked single capture plays under
// the same default engine as a zone would. The editor will surface per-capture overrides later
// (S15-F2 one-zone-map lean); until then this is the one place the single-capture default lives.
// the same default engine as a zone would. The A/D/R frame counts in the default play arg are
// 44100-Hz nominals; this function always rescales them by (sampleRate / 44100) before stamping
// them on the SampleData so the ADSR wall-clock durations match tier0Adsr(sampleRate) exactly.
Keymap buildTier0Keymap(std::vector<AudioSample> frames, int sampleRate,
int rootNote, const SampleLoop& loop,
std::vector<AudioSample> framesR = {},
@@ -184,6 +187,13 @@ struct PerformanceZone {
kTier0NominalDecayFrames, kTier0NominalSustainLevel,
kTier0NominalReleaseFrames},
TriggerParams{}, kDefaultPitchEngine, PitchEnvParams{}};
// When true, the A/D/R frame counts in play.adsr are 44100-Hz nominals (either lifted from a
// v3 blob or defaulted for a new zone) that must be rescaled by (sampleRate / 44100) at keymap
// build time (buildZonedKeymap). Set to false when a v4 blob explicitly provides A/D/S/R (the
// stored counts already reflect the DAW rate at the time the user edited them) or when the user
// edits an ADSR slider (the committed value is already editor-domain). Never serialized.
bool adsrNeedsRateResolve = true;
};
// The instrument's performance map: an ordered list of zones. Order is authoritative for
@@ -213,6 +223,9 @@ struct ResolvedZone {
kTier0NominalReleaseFrames},
TriggerParams{}, kDefaultPitchEngine,
PitchEnvParams{}}; // S15/S16 per-zone play params (carried through as-is)
// Carried from PerformanceZone::adsrNeedsRateResolve — buildZonedKeymap rescales A/D/R
// frames by (sampleRate / 44100) when true. False for v4-explicit or user-edited values.
bool adsrNeedsRateResolve = true;
};
// The result of resolving a performance map against the live bank blob. `zones` are the
+8 -9
View File
@@ -279,16 +279,15 @@ void Voice::start(int note, int velocity, const SampleData& sample, int rootNote
// --- Amplitude envelope: Gate = AHDSR (fully per-zone: A/H/D/S/R all read from the zone's
// play.adsr); Trigger = the time-boxed fade-in/out over the % play length.
//
// gateAdsr (instrument-wide tier0) is the fallback for zones deserialized from a pre-v4
// payload blob (see sample_map zones-payload v4): those zones carry their adsr fields
// pre-populated at deserialize time with the tier0 nominal defaults so back-compat holds
// (see sample_map.cpp readZonesPayload). For a new or fully-round-tripped zone the field
// is purely from the zone's own state; gateAdsr is still passed here for the Tier-0
// single-capture path (buildTier0Keymap supplies a ZonePlayParams that already seeds the
// ADSR defaults from the product defaults; the voice reads those directly).
// All five AHDSR fields come from sample.play.adsr, stamped by buildTier0Keymap /
// buildZonedKeymap at reload time (with rate-rescaling for v3-lifted / default zones).
// gateAdsr (the VoiceEngine's instrument-wide ADSR) is accepted for interface compat
// but is NOT read here — it is vestigial since the S12 review fix moved A/D/S/R fully
// onto the per-zone SampleData.
//
// Back-compat invariant: a zone whose adsr fields carry the product defaults (the tier0
// nominal at 44100) sounds bit-identical to the pre-fix build. ---
// Back-compat invariant: a zone whose adsr fields carry the tier-0 nominal values
// (rescaled to the live sample rate by buildZonedKeymap) sounds bit-identical to the
// pre-fix build at every DAW rate. ---
if (playMode_ == PlayMode::Gate) {
env_.configure(p.adsr);
env_.noteOn();
+7 -5
View File
@@ -340,11 +340,13 @@ class Voice {
public:
// Starts this voice on `note` at `velocity`, playing `sample` (a stable reference
// the caller must keep alive for the voice's lifetime — the Keymap owns it), repitched
// from `rootNote`. `gateAdsr` is the effective Gate AHDSR (the engine supplies the
// instrument-wide attack/decay/sustain/release timing; the per-zone HOLD stage comes from
// sample.play.adsr.holdFrames, folded in here). The S15 play MODE + Trigger params and the
// S16 pitch ENGINE + pitch envelope are read from `sample.play`. The Preserve shifters MUST
// already be pre-sized (presizePreserveShifters, off-thread) — start() only reset()s + warm()s
// from `rootNote`. All five AHDSR fields (A/H/D/S/R) are read directly from
// sample.play.adsr — the per-zone values stamped by buildTier0Keymap / buildZonedKeymap.
// `gateAdsr` is the VoiceEngine's instrument-wide ADSR parameter, accepted for interface
// compatibility but NOT used by start() (vestigial since the S12 review fix moved A/D/S/R
// onto the per-zone SampleData). The S15 play MODE + Trigger params and the S16 pitch
// ENGINE + pitch envelope are read from `sample.play`. The Preserve shifters MUST already
// be pre-sized (presizePreserveShifters, off-thread) — start() only reset()s + warm()s
// them (RT-safe, no allocation) since it runs on the audio thread inside process(). The warm
// silence pass settles the OLA taps before the first output frame (no cold-start click).
// Byte-identical to the pre-S15 engine when sample.play is default (Gate + Varispeed + no
+82
View File
@@ -1194,6 +1194,84 @@ static void testV3BlobLiftsAdsrToNominalDefaults() {
CHECK(a.decayFrames == kTier0NominalDecayFrames); // 0
CHECK(a.sustainLevel == kTier0NominalSustainLevel); // 1.0
CHECK(a.releaseFrames == kTier0NominalReleaseFrames); // 2646 (0.060 s at 44100)
// The lifted zone must be flagged for rate-resolve so buildZonedKeymap rescales at the live rate.
CHECK(back.zones[0].adsrNeedsRateResolve == true);
}
// A v4 blob (explicit A/D/S/R tail) must NOT set adsrNeedsRateResolve — those values
// were authored at the DAW's rate and must not be rescaled again at keymap build time.
static void testV4BlobClearsAdsrNeedsRateResolve() {
PerformanceMap m;
PerformanceZone z = zone("pad", 0, 127);
z.play.adsr.attackFrames = 441;
z.play.adsr.releaseFrames = 8820;
m.zones.push_back(z);
// A round-trip through serialize/deserialize writes a v4 payload (current version).
const PerformanceMap back = deserializePerformance(serializePerformance(m));
CHECK(back.zones.size() == 1);
if (back.zones.size() != 1) return;
// v4 tail was explicitly read — adsrNeedsRateResolve must be false.
CHECK(back.zones[0].adsrNeedsRateResolve == false);
}
// buildTier0Keymap at 48k must produce ADSR frame counts equal to tier0Adsr(48000):
// attack = round(kTier0NominalAttackFrames * 48000 / 44100) = round(143.67) = 144,
// release = round(kTier0NominalReleaseFrames * 48000 / 44100) = round(2880.0) = 2880.
// This is the "pre-fix, the Gate voice used tier0Adsr(sampleRate_)" invariant restored
// for the single-capture fast path at any DAW rate.
static void testBuildTier0KeymapRescalesAdsrAt48k() {
const Keymap km = buildTier0Keymap({0.5f}, 48000, 60, SampleLoop{});
CHECK(km.samples.size() == 1);
if (km.samples.empty()) return;
const AdsrParams& a = km.samples[0].play.adsr;
// Rescaled from 44100-nominal at 48000 Hz:
CHECK(a.attackFrames == 144); // round(132 * 48000.0 / 44100.0)
CHECK(a.decayFrames == 0); // 0 * factor = 0 (no change)
CHECK(a.sustainLevel == 1.0); // level, not frames (no rescale)
CHECK(a.releaseFrames == 2880); // round(2646 * 48000.0 / 44100.0)
}
// buildZonedKeymap at 48k with adsrNeedsRateResolve=true must rescale ADSR to match
// tier0Adsr(48000), mirroring what Gate voices saw before the per-zone-ADSR fix.
static void testBuildZonedKeymapRescalesNominalAdsrAt48k() {
// Build a zone with nominal 44100-Hz ADSR and the needs-resolve flag (the default).
ResolvedZone z;
z.lowNote = 0; z.highNote = 127; z.rootNote = 60;
z.adsrNeedsRateResolve = true; // 44100-nominal values, rescale needed
z.play.adsr.attackFrames = kTier0NominalAttackFrames; // 132
z.play.adsr.decayFrames = kTier0NominalDecayFrames; // 0
z.play.adsr.sustainLevel = kTier0NominalSustainLevel; // 1.0
z.play.adsr.releaseFrames = kTier0NominalReleaseFrames; // 2646
const DecodedZonePcm pcm{{0.5f}, 48000}; // 48k WAV
const Keymap km = buildZonedKeymap({z}, {pcm});
CHECK(km.samples.size() == 1);
if (km.samples.empty()) return;
const AdsrParams& a = km.samples[0].play.adsr;
CHECK(a.attackFrames == 144); // round(132 * 48000.0 / 44100.0)
CHECK(a.decayFrames == 0); // 0 * factor = 0
CHECK(a.sustainLevel == 1.0); // level, not rescaled
CHECK(a.releaseFrames == 2880); // round(2646 * 48000.0 / 44100.0)
}
// buildZonedKeymap must NOT rescale a zone whose adsrNeedsRateResolve is false —
// those frame counts are explicitly user-authored at the DAW's rate.
static void testBuildZonedKeymapDoesNotRescaleV4Adsr() {
ResolvedZone z;
z.lowNote = 0; z.highNote = 127; z.rootNote = 60;
z.adsrNeedsRateResolve = false; // v4 blob or user-edited — do not rescale
z.play.adsr.attackFrames = 441; // 0.01 s at 44100 Hz (non-nominal)
z.play.adsr.decayFrames = 4410;
z.play.adsr.sustainLevel = 0.7;
z.play.adsr.releaseFrames = 8820;
const DecodedZonePcm pcm{{0.5f}, 48000}; // 48k WAV — rescale would change values
const Keymap km = buildZonedKeymap({z}, {pcm});
CHECK(km.samples.size() == 1);
if (km.samples.empty()) return;
const AdsrParams& a = km.samples[0].play.adsr;
CHECK(a.attackFrames == 441); // unchanged (not rescaled)
CHECK(a.decayFrames == 4410);
CHECK(a.sustainLevel == 0.7);
CHECK(a.releaseFrames == 8820);
}
int main() {
@@ -1249,6 +1327,10 @@ int main() {
testPlayParamsThroughComponentEnvelope();
testFullAdsrV4RoundTrip();
testV3BlobLiftsAdsrToNominalDefaults();
testV4BlobClearsAdsrNeedsRateResolve();
testBuildTier0KeymapRescalesAdsrAt48k();
testBuildZonedKeymapRescalesNominalAdsrAt48k();
testBuildZonedKeymapDoesNotRescaleV4Adsr();
testComponentStateRoundTrip();
testComponentStateLoopStartRoundTrip();
testComponentStateSelectionOnlyNoZones();