diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index cc82441..ada6c38 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -334,7 +334,7 @@ anything for a trigger shape. parameter set does not carry (key-track, voice count, master gain, preview velocity) and the labels for them. - `deck_groups` — also home to `isLiveDeckParam` and `liveCommitFor`, the editor's whole commit-tier routing decision (see "Live parameter delivery" above), and to `OverlayEnv` + `nextOverlaySelection`/`overlayEnvEnabled`/`overlayEnvInert`, the whole overlay-selection state machine (exclusivity, the none resting state, and which selections a disabled or DRAWN group makes inert); WHICH groups the Sample face's deck carries, split from `knob_deck`'s HOW they lay out: the `DeckParam` control-id space (the editor's `ParamControl` is an alias of it), the `DeckGroupId` list, `sampleDeckGroups` in signal-flow order (**pitch → filter → amp**, then velocity/voice/master), and the deck's bipolar-knob law. Reads `PlayMode` for the AMP group's Gate/Trigger face, which is why this and not `knob_deck` is the module that touches the engine's value layer. Also home to `CurveTarget` + `curveTargetFor` — the VELOCITY group's three cells are popup openers, not dials, and that predicate is the ONE place they are named, so paint, hit-test routing and the popup's title all agree. MASTER is reserved for post-voice-mixer concerns, which is why the curves sit in their own group immediately left of VOICE rather than there. -- `spline_edit` — THE point-editing grammar, and the one place it is written down: left-click grabs a node and adds one in empty space, right-click deletes, control-click toggles hard/smooth. Both spline consumers — the velocity-curve popup and the spline EG overlay — route their mouse-down through `resolveSplineEdit`, so the two cannot drift into two grammars. The endpoint and point-count rules are NOT restated here: `deletePoint` and `addPoint` own them, and the caller applies the resolved action to the curve. Also home to `splineOverlayBox`, the contour's mapping box inside the waveform overlay — the FULL area, no inset, so the drawn contour stays 1:1 with the sample's time axis. +- `spline_edit` — THE point-editing grammar, and the one place it is written down: left-click grabs a node and adds one in empty space, right-click deletes, control-click toggles hard/smooth. Both spline consumers — the velocity-curve popup and the spline EG overlay — route their mouse-down through `resolveSplineEdit`, so the two cannot drift into two grammars. The endpoint and point-count rules are NOT restated here: `deletePoint` and `addPoint` own them, and the caller applies the resolved action to the curve. Also home to `splineOverlayBox`, the contour's mapping box inside the waveform overlay — the FULL area, no inset, so the drawn contour stays 1:1 with the sample's time axis. Spline points are excluded from `param_taper`'s Shift/Ctrl modifier law like waveform markers are: a point is a normalized position with no displayed unit, and control-click there is already claimed by the hard/smooth toggle above. - `curve_popup` — pure curve-popup geometry + dismissal test (FB1): centered sheet over the Sample face — width/height clamps, title row, Close button rect, curve-box rect, outside-sheet dismissal test. Mirror of `overflow_menu`; no LICE or REAPER types. - `envelope_overlay` — pure staged-envelope→polyline geometry for the Sample-view overlay (read from `envelope_overlay.h`): maps a `StageEnvelope` to a polyline inside a rect under whichever of TWO layout policies its `EnvKind` selects — an AHDSR draws a bounded param-domain schematic with its release RIGHT-ANCHORED to the canvas edge, an AHD draws 1:1 over the waveform's own time axis — plus a round mid-segment knot on every sloped stage that has a duration. Every vertex clamped in-canvas. Shares the `EnvNode`/`StageEnvelope`/`timeToX`/`levelToY` vocabulary with `envelope_edit` so the drawn handle and its grab region agree pixel-for-pixel. No VST3/REAPER/LICE types at the boundary. - `envelope_edit` — pure node hit-test + pixel-delta→clamped-param inverse map for the draggable envelope nodes and their curve knots (read from `envelope_edit.h`): `nodeAtPoint` resolves a grab to the nearest node within a pick radius (Chebyshev distance, draw-order tie-break, knots appended last so a coincident endpoint handle wins); `resolveNodeDrag` maps a pixel delta since grab to a new `StageEnvelope` under the same caller-supplied per-param clamp bounds the knobs use — a drag can never produce a param a knob couldn't. Mirror of `card_drag`/`waveform_view`; the inverse of `envelope_overlay`'s params→polyline forward map, so node-drag, knot-drag and knob-edit read/write one shared model and can never diverge. diff --git a/src/core/instrument/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index 6138e9e..114ac7b 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -100,4 +100,7 @@ reasampler_test(curve_popup LINK curve_popup velocity_curve) # envelope_overlay and deck_values all read it — which is exactly why it could not stay inside # deck_values, which sits above envelope_overlay. reasampler_pure_library(param_taper SOURCES param_taper.cpp LINK PUBLIC curve_law) -reasampler_test(param_taper LINK param_taper) +# envelope_overlay and sample_bands are linked for the test only: the finest-drag-step assertion +# is judged against the envelope node drag at the editor's own floor width (the sharper of the +# taper's two consumers), read from the allocator/overlay rather than copied as a number. +reasampler_test(param_taper LINK param_taper envelope_overlay sample_bands) diff --git a/src/core/instrument/ui/deck_values.cpp b/src/core/instrument/ui/deck_values.cpp index a391fc5..782ea2f 100644 --- a/src/core/instrument/ui/deck_values.cpp +++ b/src/core/instrument/ui/deck_values.cpp @@ -16,7 +16,6 @@ namespace reasampler::instrument::ui { using engine::filter::MorphLaw; using util::clamp01; - double deckParamNorm(DeckParam id, const PlaySeconds& play) { switch (id) { case DeckParam::kPlayMode: return play.playMode == PlayMode::Trigger ? 1.0 : 0.0; @@ -197,12 +196,10 @@ void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment) { enforceGateUnavailableWhileDrawn(play); } -namespace { - -// The ADDRESS of the one stored field a knob id owns. deckParamNorm and setDeckParam carry each -// id's MAP — which taper, which clamp; this carries only its LOCATION, which is the whole -// mechanism of the taper-free reset. A toggle, radio or curve cell has no reset gesture and -// resolves to null. +// deckParamNorm and setDeckParam carry each id's MAP — which taper, which clamp; these two carry +// only its LOCATION, which is the whole mechanism of the taper-free reset (see deck_values.h for +// why they are exposed beyond that one caller). A toggle, radio or curve cell has no reset gesture +// and resolves to null. double* deckDoubleField(DeckParam id, PlaySeconds& p) { switch (id) { case DeckParam::kAttack: return &p.adsr.attackSeconds; @@ -257,8 +254,6 @@ float* deckFloatField(DeckParam id, PlaySeconds& p) { } } -} // namespace - void resetDeckParam(DeckParam id, PlaySeconds& play) { PlaySeconds defaults; if (double* dst = deckDoubleField(id, play)) { diff --git a/src/core/instrument/ui/deck_values.h b/src/core/instrument/ui/deck_values.h index 8c23804..b27b46f 100644 --- a/src/core/instrument/ui/deck_values.h +++ b/src/core/instrument/ui/deck_values.h @@ -48,6 +48,16 @@ void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment); // For knob-valued controls — a toggle has no reset gesture. void resetDeckParam(DeckParam id, PlaySeconds& play); +// The ADDRESS of the one stored field `id` owns — the mechanism resetDeckParam bypasses the taper +// with. Exposed beyond that one caller so a test can verify a reset (or any other mutation) +// against the actual stored field rather than its normalized read-back, which deckParamNorm does +// not guarantee is injective. Null for a control with no reset gesture (a toggle, radio, or +// curve-popup cell) or one whose value lives outside PlaySeconds (master gain, key-track). +double* deckDoubleField(DeckParam id, PlaySeconds& p); +// The filter's four tone controls store their normalized position as float — see deckFloatField's +// definition for why that is a second resolver rather than a widened first one. +float* deckFloatField(DeckParam id, PlaySeconds& p); + // THE snap-unit table: which whole unit Shift snaps each control to. Includes the deck's // processor-side ids (voice count, master gain), which have no entry in the two functions above // because their VALUE lives outside the parameter set — the unit does not. diff --git a/src/core/instrument/ui/param_taper.cpp b/src/core/instrument/ui/param_taper.cpp index 647d589..59cc49f 100644 --- a/src/core/instrument/ui/param_taper.cpp +++ b/src/core/instrument/ui/param_taper.cpp @@ -8,14 +8,18 @@ namespace reasampler::instrument::ui { namespace { -// The output quanta (header: EXACT PREIMAGE). Powers of TEN on purpose: nearbyint(v*S)/S is the +// The output quanta (header: EXACT PREIMAGE). Powers of TEN on purpose: std::round(v*S)/S is the // correctly-rounded double of k/S, which is the same double a decimal literal of k/S parses to — // so a default written as 0.003 or 0.060 lands on the grid exactly. A power-of-two quantum would // not have that property against decimal literals. constexpr double kSecondsPerQuantum = 1e9; // 1 ns constexpr double kSemitonesPerQuantum = 1e6; // 1 micro-semitone -double resolveTo(double value, double perUnit) { return std::nearbyint(value * perUnit) / perUnit; } +// std::nearbyint reads the CURRENT FP rounding mode (MXCSR) — not exclusively ours on a DAW's UI +// thread. Under round-toward-zero it can drop a grid value by a whole quantum, which is exactly +// what the quantization scheme exists to prevent. std::round (half-away-from-zero) is the same +// regardless of that mode, which is what makes the EXACT PREIMAGE guarantee (header) structural. +double resolveTo(double value, double perUnit) { return std::round(value * perUnit) / perUnit; } // The shifted-log offsets. Both are FITTED AGAINST THE CEILING above them, which is why the // ceiling could not be raised in a later track: doing the two apart means fitting twice. @@ -64,19 +68,19 @@ double depthSemitonesFromNorm(double norm, double maxSemitones) { double snapSecondsToWholeMs(double seconds) { if (!(seconds > 0.0)) return 0.0; - return std::nearbyint(seconds * 1000.0) / 1000.0; + return std::round(seconds * 1000.0) / 1000.0; } double snapFractionToWholePercent(double fraction) { - return std::nearbyint(fraction * 100.0) / 100.0; + return std::round(fraction * 100.0) / 100.0; } -double snapSemitonesToWhole(double semitones) { return std::nearbyint(semitones); } +double snapSemitonesToWhole(double semitones) { return std::round(semitones); } // Rounding lands on 1..10; anything under half a unit clamps to the domain floor rather than to // zero, which is not an exponent. 1.0, the linear neutral, is therefore one snap from centre. double snapExponentToWhole(double exponent) { - return util::clampCurve(std::nearbyint(util::clampCurve(exponent))); + return util::clampCurve(std::round(util::clampCurve(exponent))); } } // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/param_taper.h b/src/core/instrument/ui/param_taper.h index 248e3fe..cbfc703 100644 --- a/src/core/instrument/ui/param_taper.h +++ b/src/core/instrument/ui/param_taper.h @@ -43,12 +43,15 @@ enum class UnitCategory { // as toPlain(defaultNorm) with no editor-side bypass available, so every default must satisfy // toPlain(toNormalized(d)) == d BITWISE. No transcendental map delivers that at an arbitrary // interior point — the image of toPlain is sparser there than the doubles around it — so both -// maps below resolve their output onto a fixed decimal quantum. That turns the guarantee into -// "every value on the quantum grid round-trips exactly" instead of a libm coincidence that a -// compiler upgrade could take away. Both quanta sit four or more orders below the finest -// reachable drag step, so nothing observable is quantized. The converse, -// toNormalized(toPlain(n)) == n at arbitrary n, is NOT required and must not be demanded: no log -// map satisfies it in double, and requiring it would rule out the shape the range needs. +// maps below resolve their output onto a fixed decimal quantum, via std::round rather than +// std::nearbyint: round is half-away-from-zero regardless of the caller's FP rounding mode, so +// the quantization is mode-independent, not just decimal-exact. That turns the guarantee into +// "every value on the quantum grid round-trips exactly" instead of a libm/MXCSR coincidence that +// a compiler upgrade or a host's UI thread could take away. Both quanta sit roughly 3.7-4 orders +// below the finest reachable drag step (time ~3.98, depth ~3.71), so nothing observable is +// quantized. The converse, toNormalized(toPlain(n)) == n at arbitrary n, is NOT required and must +// not be demanded: no log map satisfies it in double, and requiring it would rule out the shape +// the range needs. // The stage-time domain's upper end — the value at norm 1, and the one home of that number: // envelope_overlay's kGateStageMaxSeconds and deck_values' kEnvTimeMaxSeconds are both aliases diff --git a/tests/test_deck_values.cpp b/tests/test_deck_values.cpp index fc85595..d82090b 100644 --- a/tests/test_deck_values.cpp +++ b/tests/test_deck_values.cpp @@ -175,8 +175,11 @@ static void testResetLandsOnTheStoredDefaultOfEachControl() { // EVERY knob resets to its own stored default, not just the six dual-ring pairs above. Swept // over the whole control-id space so a control added later cannot quietly miss the reset table: // perturb, reset, and require the control to read exactly what a fresh PlaySeconds reads. +// Compared against the STORED FIELD directly (deckDoubleField/deckFloatField), not the +// normalized read-back: deckParamNorm is not guaranteed injective, so a norm match is weaker +// than the criterion — verification against a default-constructed PlaySeconds. static void testEveryKnobIdResetsToItsDefault() { - const PlaySeconds defaults; + PlaySeconds defaults; for (int i = 0; i < static_cast(DeckParam::kCount); ++i) { const DeckParam id = static_cast(i); if (deckParamUnit(id) == UnitCategory::None) continue; // no reset gesture @@ -184,9 +187,17 @@ static void testEveryKnobIdResetsToItsDefault() { PlaySeconds p; setDeckParam(id, p, 0.37, 0); setDeckParam(id, p, 0.83, 0); // two writes: one of the two is off every default - CHECK(deckParamNorm(id, p) != deckParamNorm(id, defaults)); - resetDeckParam(id, p); - CHECK(deckParamNorm(id, p) == deckParamNorm(id, defaults)); + if (double* pd = deckDoubleField(id, p)) { + CHECK(*pd != *deckDoubleField(id, defaults)); + resetDeckParam(id, p); + CHECK(*pd == *deckDoubleField(id, defaults)); + } else if (float* pf = deckFloatField(id, p)) { + CHECK(*pf != *deckFloatField(id, defaults)); + resetDeckParam(id, p); + CHECK(*pf == *deckFloatField(id, defaults)); + } else { + CHECK(false); // every non-None, non-excluded id must own a reset field + } } } @@ -279,8 +290,10 @@ static void testAValueStoredUnderTheOldCeilingIsReadNotRewritten() { CHECK(p.adsr.releaseSeconds == 2.0); CHECK(normDecay > 0.0 && normDecay < 1.0); // still on the knob, just at a new angle CHECK(deckParamNorm(DeckParam::kRelease, p) > normDecay); - // And it survives the norm the knob would hand back, so a no-op touch of the control does - // not quantize a legacy value away. + // And a no-op touch survives the norm the knob would hand back — for THIS value, which is + // exactly on the taper's output quantum grid (1.75 s parses to a grid-aligned double). A + // legacy value off the grid (e.g. 1.2345678912345) WOULD be re-quantized on first touch; + // that is correct, intended behaviour, not a gap this test is claiming to cover. setDeckParam(DeckParam::kDecay, p, normDecay, 0); CHECK(p.adsr.decaySeconds == 1.75); } diff --git a/tests/test_param_taper.cpp b/tests/test_param_taper.cpp index 49dab04..4cf38f5 100644 --- a/tests/test_param_taper.cpp +++ b/tests/test_param_taper.cpp @@ -9,6 +9,10 @@ #include "../src/core/instrument/ui/param_taper.h" +#include "../src/core/instrument/ui/envelope_overlay.h" // gateStageSlotPx: finest drag surface +#include "../src/core/instrument/ui/sample_bands.h" // kEditorMinWidth/kPad: the editor floor + +#include #include #include @@ -78,10 +82,15 @@ static void testStageTimeIsMonotone() { } } -// The FINEST drag a user can make — Ctrl's 1/20 rate over the 128 px knob travel — must still -// move the value, or the output quantum would be observable as a dead zone. +// The FINEST drag a user can make on ANY surface this taper serves — not the knob's own 128 px +// travel, which is coarser than the AHDSR schematic's node drag at the editor floor. Derived from +// the floor constant and the overlay's own slot-width formula, so a later floor change sharpens +// (or coarsens) the step this test exercises automatically instead of leaving a copied number +// silently stale. static void testEveryFinestDragStepMovesTheValue() { - const int steps = static_cast(1.0 / kFineDragScale) * 128; + const Rect floorArea = Rect::ltrb(0, 0, kEditorMinWidth - 2 * kPad, 100); + const double slot = gateStageSlotPx(floorArea); + const int steps = static_cast(slot / kFineDragScale); for (int i = 0; i < steps; ++i) { const double lo = timeSecondsFromNorm(static_cast(i) / steps); const double hi = timeSecondsFromNorm(static_cast(i + 1) / steps); @@ -108,6 +117,32 @@ static void testEveryWholeMicrosecondRoundTripsExactly() { } } +// MODE-INDEPENDENCE, the whole point of resolveTo's std::round over std::nearbyint. First shows +// the defect directly, generically: under round-toward-zero, the RETIRED std::nearbyint reads +// that mode and truncates a value whose fraction is well past half, while std::round (specified +// to round half-away-from-zero REGARDLESS of the current mode) does not. Then proves the +// production round trip itself — not a stand-in — survives the same hostile mode across the grid. +static void testRoundingSurvivesAHostileFpRoundingMode() { + const int saved = std::fegetround(); + CHECK(std::fesetround(FE_TOWARDZERO) == 0); + + CHECK(std::nearbyint(12.9) == 12.0); // the RETIRED behaviour: mode-dependent, wrong here + CHECK(std::round(12.9) == 13.0); // the fix: mode-independent, rounds to nearest + + for (int us = 0; us <= 200000; us += 7) { + const double seconds = static_cast(us) / 1e6; + CHECK(timeSecondsFromNorm(timeNormFromSeconds(seconds)) == seconds); + if (timeSecondsFromNorm(timeNormFromSeconds(seconds)) != seconds) break; + } + for (int milli = -24000; milli <= 24000; milli += 37) { + const double d = static_cast(milli) / 1000.0; + CHECK(depthSemitonesFromNorm(depthNormFromSemitones(d, kDepth), kDepth) == d); + if (depthSemitonesFromNorm(depthNormFromSemitones(d, kDepth), kDepth) != d) break; + } + + std::fesetround(saved); // restore — every other test in this binary assumes the default +} + // The converse round trip is NOT required, but its residual is worth pinning: it is bounded by // the output quantum read back through the map, which stays four orders below one drag pixel. // Pinned so a future quantum change cannot make the needle visibly lag the hand unnoticed. @@ -239,6 +274,7 @@ int main() { testStageTimeIsMonotone(); testEveryFinestDragStepMovesTheValue(); testEveryWholeMicrosecondRoundTripsExactly(); + testRoundingSurvivesAHostileFpRoundingMode(); testNormRoundTripResidualStaysBelowOneDragPixel(); testTheStageTimeDefaultsRoundTripExactly();