diff --git a/src/core/instrument/engine/play_params.h b/src/core/instrument/engine/play_params.h index 2ce2ad0..433d62a 100644 --- a/src/core/instrument/engine/play_params.h +++ b/src/core/instrument/engine/play_params.h @@ -96,10 +96,8 @@ struct PitchEnvParams { // normalized control positions verbatim rather than a parallel set, so no control range is // re-derived here; `filter_params.h` owns every law that maps them to Hz/Q/depth. // -// The three modulation depths all land in that same normalized cutoff domain and sum before a -// single clamp: `modAmount` scales the per-frame filter envelope, `velAmount` scales the -// note-on velocity through `velocityCurve`, and `keyTrack` moves cutoff by octaves per octave -// above the root. All three are zero/neutral by default. +// The three modulation depths below land in that same normalized cutoff domain and sum +// before a single clamp; all three are zero/neutral by default. struct FilterParams { bool enabled = false; instrument::engine::filter::FilterSettings settings; @@ -109,7 +107,10 @@ struct FilterParams { AdsrParams env; // the same staged AHDSR the amp runs; frames // Shapes velocity before velAmount scales it. Linear rather than the amp's flat() default // because a flat curve under a depth control would make every velocity the same offset; - // the no-op at rest is velAmount == 0, not the curve. + // the no-op at rest is velAmount == 0, not the curve. NOTE: this default only governs a + // FRESH FilterParams — the shared codec's corrupt/truncated-point-list repair + // (VelocityCurve::fromPoints, used for both this curve and the amp's) still degrades to + // flat() regardless, since that repair has no curve-specific fallback. VelocityCurve velocityCurve = VelocityCurve::linear(); }; diff --git a/src/core/instrument/engine/voice.cpp b/src/core/instrument/engine/voice.cpp index d755b82..66e6b99 100644 --- a/src/core/instrument/engine/voice.cpp +++ b/src/core/instrument/engine/voice.cpp @@ -92,8 +92,8 @@ void Voice::start(int note, int velocity, const SampleData& sample, bool declick pitchEnv_.configure(p.pitchEnv); pitchEnv_.noteOn(); - // Filter: fresh integrators per note (prepare() preserves state on purpose, so a note-on - // is the one place that must clear it). Velocity maps through the curve once here, off the + // Filter: reset() clears integrator state for the new note (prepare() preserves it — + // voice_filter.h / filter/CLAUDE.md). Velocity maps through the curve once here, off the // per-frame path, exactly as the amp's velocityGain_ does. filterOn_ = p.filter.enabled; if (filterOn_) { diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index e92b6ff..071062e 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -54,8 +54,9 @@ inline double filterNormPerOctave() { // A modulated cutoff re-solves the SVF coefficients, which costs a tan() plus the morph's // cos/sin — so the solve is gated on the modulated position crossing one step of this // quantization of the sweep. 2048 steps over three decades is ~0.06 semitone, far under the -// ear's resolution for a filter corner, and it collapses the solve to nothing across a static -// envelope stage: an unmodulated voice pays one integer compare per frame. +// ear's resolution for a filter corner. tickFilterCutoff's own modAmount==0 early-out (see +// there) skips the envelope/clamp/quantize math too, so a static, already-solved voice pays +// two cheap compares per frame there, never the tan/cos/sin. inline constexpr int kFilterModSteps = 2048; // Takeover declick: a restart of a sounding voice (mono retrigger takeover/fallback or a @@ -178,9 +179,16 @@ private: } // Advances the filter envelope and re-solves the filter's coefficients when the modulated - // cutoff has moved a whole quantization step (see kFilterModSteps). prepare() deliberately - // preserves integrator state, so a moving cutoff glides rather than clicking. + // cutoff has moved a whole quantization step (see kFilterModSteps); state preservation + // across that solve is voice_filter's own contract (voice_filter.h / filter/CLAUDE.md). + // + // filterModAmount_ is fixed for the note's lifetime (set once in start()), so once the + // first solve has run (filterModStep_ != -1) a zero depth can only ever re-derive the same + // cutoff — skip the envelope tick, clamp, and quantization entirely rather than pay them + // to land on the answer already solved. filterModStep_ == -1 (forced by start()/retune() + // via updateFilterCutoffBase) still falls through here so the base cutoff's own solve runs. void tickFilterCutoff() { + if (filterModAmount_ == 0.0 && filterModStep_ != -1) return; double cut = static_cast(filterBaseCutoff_) + filterModAmount_ * filterEnv_.tick(); if (cut < 0.0) cut = 0.0; diff --git a/src/core/instrument/map/CMakeLists.txt b/src/core/instrument/map/CMakeLists.txt index 91836e9..f4d761f 100644 --- a/src/core/instrument/map/CMakeLists.txt +++ b/src/core/instrument/map/CMakeLists.txt @@ -15,6 +15,8 @@ reasampler_test(bank_sync LINK bank_sync) # The state codec is shared with the extension's preset-blob path, so it must link WITHOUT # the voice engine: velocity_curve (the curve field) and master_gain (the wire gain cap) only. +# play_params.h also pulls in filter/'s headers (FilterSettings, MorphLaw) for the v9 filter +# tail -- plain value types, so no filter symbol is linked and this stays true. reasampler_pure_library(component_state_io SOURCES component_state_io.cpp LINK PUBLIC velocity_curve master_gain) diff --git a/src/core/instrument/map/component_state_io.cpp b/src/core/instrument/map/component_state_io.cpp index a03848e..8e1acee 100644 --- a/src/core/instrument/map/component_state_io.cpp +++ b/src/core/instrument/map/component_state_io.cpp @@ -162,9 +162,14 @@ void readFilterTail(ByteReader& r, InstrumentParams& p) { f.settings.driveNorm = static_cast(bitsToDouble(r.u64())); f.settings.morphLaw = (r.u8() != 0) ? engine::filter::MorphLaw::HighNotchLow : engine::filter::MorphLaw::HighBandLow; - f.modAmount = bitsToDouble(r.u64()); - f.velAmount = bitsToDouble(r.u64()); - f.keyTrack = bitsToDouble(r.u64()); + // 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, 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()); diff --git a/src/core/instrument/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index b267d41..4919aa2 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -44,8 +44,11 @@ reasampler_test(envelope_edit LINK envelope_edit) reasampler_pure_library(knob_deck SOURCES knob_deck.cpp LINK PUBLIC editor_geometry) reasampler_test(knob_deck LINK knob_deck) -# The deck's group COMPOSITION, split from its layout: knob_deck stays engine-free, while this -# names the controls and so reads PlayMode (velocity_curve comes along with play_params.h). +# The deck's group COMPOSITION, split from its layout: knob_deck stays engine-free (see +# core/instrument/CLAUDE.md's deck_groups entry for why this module, not knob_deck, reads +# PlayMode). velocity_curve is the filter's own curve field; peaks is play_params.h's +# AudioSample dependency. play_params.h also drags in filter/'s headers (FilterSettings, +# MorphLaw) for the v9 filter tail -- plain value types, no filter symbol linked. reasampler_pure_library(deck_groups SOURCES deck_groups.cpp LINK PUBLIC knob_deck velocity_curve peaks) diff --git a/src/core/instrument/ui/deck_groups.cpp b/src/core/instrument/ui/deck_groups.cpp index 57d8a41..bfb266b 100644 --- a/src/core/instrument/ui/deck_groups.cpp +++ b/src/core/instrument/ui/deck_groups.cpp @@ -72,7 +72,7 @@ std::vector sampleDeckGroups(PlayMode playMode) { id(DeckParam::kRelease)}; } else { // Trigger, time-ordered left-to-right (Fade In / Length % / Fade Out — matches - // the drawn envelope), plus the two reserved blanks that hold the Gate width. + // the drawn envelope), plus two blanks (see knob_deck.h's blank-cell contract). amp.cellIds = {id(DeckParam::kTrigFadeIn), id(DeckParam::kTrigLength), id(DeckParam::kTrigFadeOut), -1, -1}; } diff --git a/src/core/instrument/ui/deck_groups.h b/src/core/instrument/ui/deck_groups.h index 1477d9d..69256d6 100644 --- a/src/core/instrument/ui/deck_groups.h +++ b/src/core/instrument/ui/deck_groups.h @@ -68,9 +68,8 @@ enum DeckGroupId { }; // The deck's groups, left to right, in SIGNAL-FLOW order: pitch -> filter -> amp, then the -// two instance-wide groups. `playMode` picks the AMP group's face; its width is -// mode-independent (Trigger leaves two blank cells) so a mode flip never reflows the -// neighbouring groups. +// two instance-wide groups. `playMode` picks the AMP group's face, via knob_deck's blank-cell +// reservation (knob_deck.h) so a mode flip never reflows the neighbouring groups. std::vector sampleDeckGroups(PlayMode playMode); // The deck's BIPOLAR knob law: 0.5 of the knob's travel is zero depth, the ends are -1 and diff --git a/src/shell/instrument/editor_controls.cpp b/src/shell/instrument/editor_controls.cpp index c35bfa0..cbf83eb 100644 --- a/src/shell/instrument/editor_controls.cpp +++ b/src/shell/instrument/editor_controls.cpp @@ -1,8 +1,8 @@ // editor_controls.cpp — the ReaSamplerEditor's parameter plumbing: the band-stack layout // resolve every paint/hit-test path shares, the control-value domain maps (controlValue / -// applyControl — seconds/fraction/frames <-> normalized 0..1), the knob-deck group -// descriptors + control-id<->value binding, and the envelope pack/unpack (the trigger-seam -// converter). Value logic only — no painting, no window plumbing. +// applyControl — seconds/fraction/frames <-> normalized 0..1), the control-id<->value binding +// against the pure `deck_groups` module's descriptors, and the envelope pack/unpack (the +// trigger-seam converter). Value logic only — no painting, no window plumbing. #include "shell/instrument/reasampler_editor.h" diff --git a/src/shell/instrument/editor_session.cpp b/src/shell/instrument/editor_session.cpp index 8137f03..795deb4 100644 --- a/src/shell/instrument/editor_session.cpp +++ b/src/shell/instrument/editor_session.cpp @@ -38,8 +38,9 @@ using util::readFileBytes; ReaSamplerEditor::ReaSamplerEditor(ReaSamplerProcessor* processor) : CPluginView(nullptr), processor_(processor) { - // Default view size, tuned to the three band heights: chrome + two-lane waveform + - // deck row. 840x620 clears the full face without scroll on 1080p. + // Default view size, tuned to the band heights: chrome + two-lane waveform + the deck + // (now two rows since the filter group). 840x620 clears the full face without scroll on + // 1080p. ViewRect r(0, 0, 840, 620); setRect(r); } diff --git a/tests/test_component_state_io.cpp b/tests/test_component_state_io.cpp index 94f212c..4521589 100644 --- a/tests/test_component_state_io.cpp +++ b/tests/test_component_state_io.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -608,6 +609,30 @@ static void testFilterTailRoundTripsLosslessly() { reasampler::instrument::engine::VelocityCurve::flat())); } +// A non-finite modAmount/velAmount/keyTrack (a corrupt blob, or any writer that skipped the +// same guard the v8 master gain already applies) must lift to the neutral default rather than +// reach Voice::tickFilterCutoff, where both clamp compares are false against NaN and the +// static_cast is UB. Mirrors testCorruptFieldsFallBackToDefaults' per-field precedent. +static void testNonFiniteFilterFieldsLiftToTheNeutralDefault() { + ComponentState in; + in.selectionId = "pad"; + FilterSeconds& f = in.params.play.filter; + f.enabled = true; + f.modAmount = std::numeric_limits::quiet_NaN(); + f.velAmount = std::numeric_limits::infinity(); + f.keyTrack = -std::numeric_limits::infinity(); + + const ComponentState out = + deserializeComponentState(serializeComponentState(in), 48000.0); + const FilterSeconds& g = out.params.play.filter; + const FilterSeconds def; + CHECK(g.modAmount == def.modAmount); + CHECK(g.velAmount == def.velAmount); + CHECK(g.keyTrack == def.keyTrack); + // The fallback is per-field, not per-record: the untouched fields still round-trip. + CHECK(g.enabled); +} + // The WRITER emits the CURRENT payload version, and the marker + version sit at the head of // the payload — the self-describing property every legacy branch depends on. Asserted // against the semantic constants, not literals. @@ -1080,6 +1105,7 @@ int main() { testTruncationDegradesCleanly(); testV8RecordLiftsToTheOffNeutralFilter(); testFilterTailRoundTripsLosslessly(); + testNonFiniteFilterFieldsLiftToTheNeutralDefault(); if (failures == 0) { std::printf("component_state_io_tests: all tests passed\n"); return 0; diff --git a/tests/test_deck_groups.cpp b/tests/test_deck_groups.cpp index 7174ead..e3d40c3 100644 --- a/tests/test_deck_groups.cpp +++ b/tests/test_deck_groups.cpp @@ -96,7 +96,8 @@ static void testWrappedDeckHeightAtThePinnedEditorWidths() { // the remaining four fit the second. CHECK(deckRowCount(g, kAvailAtDefaultWidth) == 2); CHECK(deckHeight(g, kAvailAtDefaultWidth) == 2 * kDeckGroupH + kDeckRowGap); - // At the 560 floor it takes four; FILTER is wider than the row on its own. + // At the 560 floor it takes four: FILTER (440px) fits the row alone, but not alongside + // PITCH + PITCH ENV (318 + 12 + 440 = 770 > 544), so it wraps to its own row. CHECK(deckRowCount(g, kAvailAtMinWidth) == 4); CHECK(deckHeight(g, kAvailAtMinWidth) == 4 * kDeckGroupH + 3 * kDeckRowGap); diff --git a/tests/test_knob_deck.cpp b/tests/test_knob_deck.cpp index 4852617..88ba3a9 100644 --- a/tests/test_knob_deck.cpp +++ b/tests/test_knob_deck.cpp @@ -51,19 +51,6 @@ static void testGroupWidth() { CHECK(deckGroupWidth(master) == kDeckCellW + 2 * kDeckGroupPadX); } -static void testShellDeckFitsOneRowAtDefaultWidth() { - // The r11 default window is 840 with kPad=8 margins -> 824 available. The five shell - // groups must fit ONE deck row there (the elastic hero keeps ~430px — the layout spec's - // premise). Locks the constants against accidental growth. - const auto deck = shellLikeDeck(); - int total = 0; - for (const auto& g : deck) total += deckGroupWidth(g); - total += (static_cast(deck.size()) - 1) * kDeckGroupGap; - CHECK(total <= 824); - CHECK(deckRowCount(deck, 824) == 1); - CHECK(deckHeight(deck, 824) == kDeckGroupH); -} - static void testWrapAtNarrowWidthIsDeterministic() { // At the 560x460 checkSizeConstraint floor (544 available) the deck wraps to TWO rows, // whole trailing groups only. @@ -185,7 +172,6 @@ static void testEmptyDeck() { int main() { testGroupWidth(); - testShellDeckFitsOneRowAtDefaultWidth(); testWrapAtNarrowWidthIsDeterministic(); testFirstGroupAlwaysPlaces(); testGroupInnerGeometry(); diff --git a/tests/test_sampler_filter.cpp b/tests/test_sampler_filter.cpp index eb4fa7b..b24e932 100644 --- a/tests/test_sampler_filter.cpp +++ b/tests/test_sampler_filter.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include using namespace reasampler; @@ -67,6 +68,22 @@ static std::vector render(SampleData& sample, int note, int velocity, in return out; } +// Renders via renderFrameStereo — exercises voice.h's stereo filter block (both the +// filter_.process(1, ...) call for a genuinely stereo source and the dual-mono mirror for a +// mono one), never touched by the mono `render` helper above. +static std::vector> renderStereo(SampleData& sample, int note, + int velocity, int frames) { + Voice v; + v.start(note, velocity, sample); + std::vector> out(static_cast(frames)); + for (int i = 0; i < frames; ++i) { + AudioSample l = 0.0f, r = 0.0f; + v.renderFrameStereo(l, r); + out[static_cast(i)] = {static_cast(l), static_cast(r)}; + } + return out; +} + static double rms(const std::vector& x, std::size_t from, std::size_t to) { double acc = 0.0; for (std::size_t i = from; i < to; ++i) acc += x[i] * x[i]; @@ -218,7 +235,8 @@ static void testModAmountPolarityDrivesCutoffFromOppositeEnds() { const std::vector falling = sweep(1.0f, -1.0); CHECK(rms(falling, 10000, 12000) < 0.05 * rms(falling, 0, 2000)); - // Zero depth: the envelope is still running, but it must not reach cutoff at all. + // Zero depth: tickFilterCutoff's early-out skips the envelope entirely, so cutoff must + // never move regardless of how long the (unread) envelope stage runs. const std::vector steady = sweep(0.5f, 0.0); const double early = rms(steady, 2000, 4000); const double late = rms(steady, 10000, 12000); @@ -282,6 +300,50 @@ static void testNoteOnResetsTheFilterSoAPreviousNoteCannotLeak() { for (std::size_t i = 0; i < fresh.size(); ++i) CHECK(restarted[i] == fresh[i]); } +// --------------------------------------------------------------------------- +// Stereo: the two filter paths renderFrame() (mono) never reaches. +// --------------------------------------------------------------------------- + +static void testStereoRenderOfAMonoSampleMirrorsTheMonoResultExactly() { + // A mono SampleData in a stereo render never calls filter_.process(1, ...) — voice.h's + // dual-mono block copies channel 0's already-filtered result to channel 1 instead. Pin + // both channels bit-identical to the plain mono render. + SampleData s = periodicSine(4000, 64); + s.play.adsr = flatAdsr(); + s.play.filter = engagedFilter(0.3f, 0.85f, 1.0f); + s.play.filter.settings.driveNorm = 0.6f; + + SampleData monoSrc = s; + const std::vector mono = render(monoSrc, 60, 100, 2000); + + SampleData stereoSrc = s; + const auto stereo = renderStereo(stereoSrc, 60, 100, 2000); + for (std::size_t i = 0; i < stereo.size(); ++i) { + CHECK(stereo[i].first == mono[i]); + CHECK(stereo[i].second == mono[i]); + } +} + +static void testStereoFilterChannel1MatchesChannel0ForIdenticalLRInput() { + // A genuinely stereo sample whose channels are identical drives filter_.process(1, ...) + // for real (haveR true). Identical input plus identical reset() state must give identical + // output on both channels — the mirror's correctness argument, pinned against the actual + // per-channel path instead of only asserted in a comment. + SampleData s = periodicSine(4000, 64); + s.framesR = s.frames; // genuinely stereo, L == R + s.play.adsr = flatAdsr(); + s.play.filter = engagedFilter(0.3f, 0.85f, 1.0f); + s.play.filter.settings.driveNorm = 0.6f; + + const auto stereo = renderStereo(s, 60, 100, 2000); + bool sawSignal = false; + for (const auto& lr : stereo) { + CHECK(lr.first == lr.second); + if (std::fabs(lr.first) > 1e-6) sawSignal = true; + } + CHECK(sawSignal); +} + int main() { testDisengagedFilterIsBitInertEvenWithExtremeSettingsStored(); testFilterSeesThePreAmpSignalSoAmpGainScalesTheResultLinearly(); @@ -289,6 +351,8 @@ int main() { testModAmountPolarityDrivesCutoffFromOppositeEnds(); testVelocityAndKeyTrackingReachCutoffAndAreNoOpsAtTheirDefaults(); testNoteOnResetsTheFilterSoAPreviousNoteCannotLeak(); + testStereoRenderOfAMonoSampleMirrorsTheMonoResultExactly(); + testStereoFilterChannel1MatchesChannel0ForIdenticalLRInput(); if (g_fail == 0) std::printf("sampler_filter: all tests passed\n"); return g_fail == 0 ? 0 : 1; }