From 834a6ddcc7194823b2842e59f19fc673240b71ac Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 30 Jul 2026 19:44:15 -0400 Subject: [PATCH 1/4] =?UTF-8?q?note:=20land=20the=20programmed=20capture-s?= =?UTF-8?q?ignal=20model=20=E2=80=94=20division=20ladder,=20tempo=20resolu?= =?UTF-8?q?tion,=20anchored=20offsets,=20one=20record=20and=20one=20resolv?= =?UTF-8?q?er?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/core/instrument/CMakeLists.txt | 1 + src/core/instrument/note/CLAUDE.md | 63 +++ src/core/instrument/note/CMakeLists.txt | 14 + src/core/instrument/note/musical_division.cpp | 68 ++++ src/core/instrument/note/musical_division.h | 50 +++ src/core/instrument/note/note_program.cpp | 68 ++++ src/core/instrument/note/note_program.h | 99 +++++ src/core/instrument/note/tempo.cpp | 27 ++ src/core/instrument/note/tempo.h | 37 ++ tests/test_musical_division.cpp | 177 +++++++++ tests/test_note_program.cpp | 359 ++++++++++++++++++ tests/test_tempo.cpp | 135 +++++++ 12 files changed, 1098 insertions(+) create mode 100644 src/core/instrument/note/CLAUDE.md create mode 100644 src/core/instrument/note/CMakeLists.txt create mode 100644 src/core/instrument/note/musical_division.cpp create mode 100644 src/core/instrument/note/musical_division.h create mode 100644 src/core/instrument/note/note_program.cpp create mode 100644 src/core/instrument/note/note_program.h create mode 100644 src/core/instrument/note/tempo.cpp create mode 100644 src/core/instrument/note/tempo.h create mode 100644 tests/test_musical_division.cpp create mode 100644 tests/test_note_program.cpp create mode 100644 tests/test_tempo.cpp diff --git a/src/core/instrument/CMakeLists.txt b/src/core/instrument/CMakeLists.txt index 16d3af6..2f6f6ba 100644 --- a/src/core/instrument/CMakeLists.txt +++ b/src/core/instrument/CMakeLists.txt @@ -1,3 +1,4 @@ add_subdirectory(engine) add_subdirectory(map) +add_subdirectory(note) add_subdirectory(ui) diff --git a/src/core/instrument/note/CLAUDE.md b/src/core/instrument/note/CLAUDE.md new file mode 100644 index 0000000..4f3c7c2 --- /dev/null +++ b/src/core/instrument/note/CLAUDE.md @@ -0,0 +1,63 @@ +# src/core/instrument/note — the programmed capture signal + +## Scope + +The pure model of the note the sampler plays to itself when it resamples: how long it +sounds, how hard, and how far around it the capture window opens. A fourth peer of +`engine/` / `map/` / `ui/` under `core/instrument/`, and pure by the same rule — no REAPER +types, no VST3 types, no host at all. + +It exists as its own directory because it is neither engine (it renders nothing), mapping +(it resolves no capture and builds no `SampleData`), nor UI (it computes no geometry). It +is a performance *description* plus its arithmetic, read by two consumers that must not +diverge: the capture-signal popup that edits it and the bake that renders it. + +## Invariants + +- **One record, one resolver.** `NoteProgram` is the single source of truth and + `resolveNote` the single way to turn it into times. A preview that computes its own + window, or a bake that does, is the exact divergence this module exists to prevent — the + criterion is structural (one path), not "the numbers looked close." +- **The tempo comes in as a parameter.** The BPM in effect at the project cursor is read by + the shell. Nothing here may reach for it, and no tempo is hardcoded anywhere in the + directory — `Tempo` has no default and cannot be constructed without one. +- **Resolved times are rate-free seconds.** The standing ruling: no sample rate appears + here; the caller converts seconds to frames against the live rate. +- **Note length is musical-division-only.** Offsets carry the ms/beats duality; the note + length does not. A free-duration note length would make two records describe the same + performance at one tempo and different performances at another. +- **A division persists as its `{quarterExponent, modifier}` pair, never as its picker + index.** The index is presentation order and would silently re-map every saved record if + the ladder ever gained a rung or a modifier. +- **An offset stores the denomination it was entered in.** The other view is derived on + demand. Storing resolved seconds instead would make a beats-denominated offset stop + following the tempo, which is the only reason to express one in beats. + +## Modules + +- `musical_division` — the note-length ladder: 1/64 through 64/1 (a rung is the base-2 + exponent of its length in quarter notes, -4..8), each straight, dotted (x3/2), or triplet + (x2/3); the 39-entry picker order; and the `"1/8."` / `"1/4t"` label notation. Lengths in + beats only, so it links no tempo. +- `tempo` — a validated project tempo plus every beats <-> seconds <-> ms conversion. + Construction (`Tempo::fromBpm`) is the only place a bad BPM is rejected, which is what + lets each conversion be total and every downstream resolver be failure-free. +- `note_program` — `Velocity` (clamped 1..127), the denominated `OffsetAmount` and its unit + toggle, the anchored `StartOffset` / `EndOffset`, the `NoteProgram` record, and + `resolveNote`. + +## Gotchas + +- **A beat is a quarter note.** REAPER states project tempo in quarter notes per minute + regardless of time signature, so divisions resolve with no time signature in sight. A + beats *readout* that should track a compound meter's dotted-quarter pulse would need the + time signature threaded in — it is not, deliberately. +- **`StartOffset` and `EndOffset` are distinct types on purpose.** They hold the same + payload and differ only in what they anchor to (note-on and note-off respectively); + collapsing them into one type with an anchor field makes the swap a runtime bug instead + of a compile error. +- **Signs are uniform: positive is later in time.** So Daniel's "capture from 20 ms before + note-on" is a *negative* start offset, and a negative end offset truncates before release. + Both are legal; `resolveNote` only refuses to invert the window. +- **ms <-> beats round-trips are lossless to double precision, not bit-identical.** The + conversion is a multiply/divide pair; compare with an epsilon. diff --git a/src/core/instrument/note/CMakeLists.txt b/src/core/instrument/note/CMakeLists.txt new file mode 100644 index 0000000..25dee5e --- /dev/null +++ b/src/core/instrument/note/CMakeLists.txt @@ -0,0 +1,14 @@ +reasampler_pure_library(musical_division SOURCES musical_division.cpp) +# Links only musical_division: the ladder is beats-only, so it must prove out with no tempo +# in the link line at all. +reasampler_test(musical_division LINK musical_division) + +reasampler_pure_library(tempo SOURCES tempo.cpp) +reasampler_test(tempo LINK tempo) + +# The record composes the ladder and the tempo and nothing else — the programmed signal is +# plain data, provable without the engine, the bank, or a host. +reasampler_pure_library(note_program + SOURCES note_program.cpp + LINK PUBLIC musical_division tempo) +reasampler_test(note_program LINK note_program) diff --git a/src/core/instrument/note/musical_division.cpp b/src/core/instrument/note/musical_division.cpp new file mode 100644 index 0000000..22de6e7 --- /dev/null +++ b/src/core/instrument/note/musical_division.cpp @@ -0,0 +1,68 @@ +// musical_division.cpp — see musical_division.h. Pure; standard library only. + +#include "core/instrument/note/musical_division.h" + +#include +#include + +namespace reasampler::instrument::note { +namespace { + +double modifierFactor(DivisionModifier m) { + switch (m) { + case DivisionModifier::Dotted: return 1.5; + case DivisionModifier::Triplet: return 2.0 / 3.0; + case DivisionModifier::Straight: break; + } + return 1.0; +} + +int clampExponent(int quarterExponent) { + return (std::max)(kMinQuarterExponent, (std::min)(kMaxQuarterExponent, quarterExponent)); +} + +} // namespace + +bool operator==(Division a, Division b) { + return a.quarterExponent == b.quarterExponent && a.modifier == b.modifier; +} + +bool operator!=(Division a, Division b) { return !(a == b); } + +Division makeDivision(int quarterExponent, DivisionModifier modifier) { + Division d; + d.quarterExponent = static_cast(clampExponent(quarterExponent)); + d.modifier = modifier; + return d; +} + +double divisionBeats(Division d) { + const Division legal = makeDivision(d.quarterExponent, d.modifier); + return std::ldexp(1.0, legal.quarterExponent) * modifierFactor(legal.modifier); +} + +Division divisionAt(int index) { + const int clamped = (std::max)(0, (std::min)(kDivisionCount - 1, index)); + return makeDivision(kMinQuarterExponent + clamped / kModifierCount, + static_cast(clamped % kModifierCount)); +} + +int divisionIndex(Division d) { + const Division legal = makeDivision(d.quarterExponent, d.modifier); + return (legal.quarterExponent - kMinQuarterExponent) * kModifierCount + + static_cast(legal.modifier); +} + +std::string divisionLabel(Division d) { + const Division legal = makeDivision(d.quarterExponent, d.modifier); + const int e = legal.quarterExponent; + // Both branches meet at e == 2 ("1/1"): a division's written form is its length in + // whole notes, which is 2^(e-2). + std::string label = e <= 2 ? "1/" + std::to_string(1 << (2 - e)) + : std::to_string(1 << (e - 2)) + "/1"; + if (legal.modifier == DivisionModifier::Dotted) label += '.'; + else if (legal.modifier == DivisionModifier::Triplet) label += 't'; + return label; +} + +} // namespace reasampler::instrument::note diff --git a/src/core/instrument/note/musical_division.h b/src/core/instrument/note/musical_division.h new file mode 100644 index 0000000..afeaa0d --- /dev/null +++ b/src/core/instrument/note/musical_division.h @@ -0,0 +1,50 @@ +// musical_division — the note-length ladder the capture signal is programmed from: 1/64 +// through 64/1, each straight, dotted, or triplet. Lengths are in BEATS only; the tempo +// resolution belongs to `tempo`, which keeps this ladder provable without one. + +#pragma once + +#include +#include + +namespace reasampler::instrument::note { + +enum class DivisionModifier : std::uint8_t { + Straight, + Dotted, // x 3/2 + Triplet, // x 2/3 +}; + +// A rung of the ladder is the base-2 exponent of its length in quarter notes: -4 is 1/64, +// 0 is 1/4, 2 is 1/1, 8 is 64/1. Holding the exponent rather than a table of literal beat +// counts keeps every straight and dotted length exactly representable in double. +inline constexpr int kMinQuarterExponent = -4; +inline constexpr int kMaxQuarterExponent = 8; +inline constexpr int kRungCount = kMaxQuarterExponent - kMinQuarterExponent + 1; +inline constexpr int kModifierCount = 3; +inline constexpr int kDivisionCount = kRungCount * kModifierCount; + +struct Division { + std::int8_t quarterExponent = 0; // 1/4 + DivisionModifier modifier = DivisionModifier::Straight; +}; + +bool operator==(Division a, Division b); +bool operator!=(Division a, Division b); + +// Off-ladder exponents clamp rather than reject: the only ways to reach one are a corrupt +// persisted record or a picker bug, and the nearest legal length beats a nonsense duration. +Division makeDivision(int quarterExponent, DivisionModifier modifier); + +// Length in beats (quarter notes). Always > 0. +double divisionBeats(Division d); + +// Picker order: shortest rung first, straight/dotted/triplet within each rung. Index is +// presentation order only — see this directory's CLAUDE.md before persisting one. +Division divisionAt(int index); +int divisionIndex(Division d); + +// The notation divisions are named in: "1/16", "1/8.", "1/4t", "4/1". +std::string divisionLabel(Division d); + +} // namespace reasampler::instrument::note diff --git a/src/core/instrument/note/note_program.cpp b/src/core/instrument/note/note_program.cpp new file mode 100644 index 0000000..f5a4be5 --- /dev/null +++ b/src/core/instrument/note/note_program.cpp @@ -0,0 +1,68 @@ +// note_program.cpp — see note_program.h. Pure; standard library only. + +#include "core/instrument/note/note_program.h" + +#include + +namespace reasampler::instrument::note { + +Velocity Velocity::of(int value) { + Velocity v; + v.value_ = static_cast((std::max)(kMin, (std::min)(kMax, value))); + return v; +} + +bool operator==(Velocity a, Velocity b) { return a.value() == b.value(); } + +bool operator==(OffsetAmount a, OffsetAmount b) { + return a.magnitude == b.magnitude && a.denomination == b.denomination; +} + +bool operator!=(OffsetAmount a, OffsetAmount b) { return !(a == b); } + +OffsetAmount offsetFromMs(double ms) { return {ms, Denomination::Milliseconds}; } + +OffsetAmount offsetFromBeats(double beats) { return {beats, Denomination::Beats}; } + +double offsetMs(OffsetAmount amount, Tempo tempo) { + return amount.denomination == Denomination::Milliseconds ? amount.magnitude + : tempo.beatsToMs(amount.magnitude); +} + +double offsetBeats(OffsetAmount amount, Tempo tempo) { + return amount.denomination == Denomination::Beats ? amount.magnitude + : tempo.msToBeats(amount.magnitude); +} + +double offsetSeconds(OffsetAmount amount, Tempo tempo) { + return amount.denomination == Denomination::Beats + ? tempo.beatsToSeconds(amount.magnitude) + : msToSeconds(amount.magnitude); +} + +OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo) { + if (amount.denomination == to) return amount; + return to == Denomination::Beats ? offsetFromBeats(offsetBeats(amount, tempo)) + : offsetFromMs(offsetMs(amount, tempo)); +} + +bool operator==(const NoteProgram& a, const NoteProgram& b) { + return a.length == b.length && a.start.amount() == b.start.amount() + && a.end.amount() == b.end.amount() && a.velocity == b.velocity; +} + +bool operator!=(const NoteProgram& a, const NoteProgram& b) { return !(a == b); } + +ResolvedNote resolveNote(const NoteProgram& program, Tempo tempo) { + ResolvedNote out; + out.noteOffSeconds = tempo.beatsToSeconds(divisionBeats(program.length)); + out.captureStartSeconds = offsetSeconds(program.start.amount(), tempo); + out.captureEndSeconds = out.noteOffSeconds + offsetSeconds(program.end.amount(), tempo); + // An inverted window has no meaning to a renderer, so a far-negative end offset yields a + // zero-length capture the caller can reject rather than a negative one it cannot. + out.captureEndSeconds = (std::max)(out.captureEndSeconds, out.captureStartSeconds); + out.velocity = program.velocity.value(); + return out; +} + +} // namespace reasampler::instrument::note diff --git a/src/core/instrument/note/note_program.h b/src/core/instrument/note/note_program.h new file mode 100644 index 0000000..02aacad --- /dev/null +++ b/src/core/instrument/note/note_program.h @@ -0,0 +1,99 @@ +// note_program — the programmed capture signal: one note length, one velocity, two anchored +// offsets, and the one resolver a preview and a bake must share. +// +// Resolved times are rate-free SECONDS relative to note-on; the caller converts against the +// live sample rate. + +#pragma once + +#include + +#include "core/instrument/note/musical_division.h" +#include "core/instrument/note/tempo.h" + +namespace reasampler::instrument::note { + +class Velocity { +public: + static constexpr int kMin = 1; // 0 is note-off in MIDI; a programmed note must sound + static constexpr int kMax = 127; + + Velocity() = default; + static Velocity of(int value); // clamped into [kMin, kMax] + + std::uint8_t value() const { return value_; } + +private: + std::uint8_t value_ = 100; +}; + +bool operator==(Velocity a, Velocity b); + +enum class Denomination : std::uint8_t { Milliseconds, Beats }; + +// One magnitude, in the denomination it was entered in; the other view is derived on demand +// and never stored. Which one was entered is itself the intent: a beats offset must follow a +// tempo change and a ms offset must hold still, and only a stored denomination says which. +struct OffsetAmount { + double magnitude = 0.0; + Denomination denomination = Denomination::Milliseconds; +}; + +bool operator==(OffsetAmount a, OffsetAmount b); +bool operator!=(OffsetAmount a, OffsetAmount b); + +OffsetAmount offsetFromMs(double ms); +OffsetAmount offsetFromBeats(double beats); + +double offsetMs(OffsetAmount amount, Tempo tempo); +double offsetBeats(OffsetAmount amount, Tempo tempo); +double offsetSeconds(OffsetAmount amount, Tempo tempo); + +// The unit toggle: the same instant restated in the other denomination. +OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo); + +// Two types rather than one carrying an anchor field: the anchor is then unswappable at +// compile time. Sign is uniform — positive is later in time — so a capture that opens before +// the note is a negative start offset, and a negative end offset truncates before release. +class StartOffset { +public: + StartOffset() = default; + explicit StartOffset(OffsetAmount amount) : amount_(amount) {} + OffsetAmount amount() const { return amount_; } + +private: + OffsetAmount amount_{}; +}; + +class EndOffset { +public: + EndOffset() = default; + explicit EndOffset(OffsetAmount amount) : amount_(amount) {} + OffsetAmount amount() const { return amount_; } + +private: + OffsetAmount amount_{}; +}; + +struct NoteProgram { + Division length{}; + StartOffset start{}; + EndOffset end{}; + Velocity velocity{}; +}; + +bool operator==(const NoteProgram& a, const NoteProgram& b); +bool operator!=(const NoteProgram& a, const NoteProgram& b); + +struct ResolvedNote { + double noteOffSeconds = 0.0; // == the note's sounding length, note-on being 0 + double captureStartSeconds = 0.0; // negative when the capture opens before the note + double captureEndSeconds = 0.0; + std::uint8_t velocity = 1; + + double captureLengthSeconds() const { return captureEndSeconds - captureStartSeconds; } +}; + +ResolvedNote resolveNote(const NoteProgram& program, Tempo tempo); + +} // namespace reasampler::instrument::note diff --git a/src/core/instrument/note/tempo.cpp b/src/core/instrument/note/tempo.cpp new file mode 100644 index 0000000..7f75d7d --- /dev/null +++ b/src/core/instrument/note/tempo.cpp @@ -0,0 +1,27 @@ +// tempo.cpp — see tempo.h. Pure; standard library only. + +#include "core/instrument/note/tempo.h" + +#include + +namespace reasampler::instrument::note { +namespace { +constexpr double kSecondsPerMinute = 60.0; +} // namespace + +std::optional Tempo::fromBpm(double beatsPerMinute) { + if (!std::isfinite(beatsPerMinute) || beatsPerMinute <= 0.0) return std::nullopt; + return Tempo(beatsPerMinute); +} + +double Tempo::secondsPerBeat() const { return kSecondsPerMinute / bpm_; } + +double Tempo::beatsToSeconds(double beats) const { return beats * secondsPerBeat(); } + +double Tempo::secondsToBeats(double seconds) const { return seconds / secondsPerBeat(); } + +double Tempo::beatsToMs(double beats) const { return secondsToMs(beatsToSeconds(beats)); } + +double Tempo::msToBeats(double ms) const { return secondsToBeats(msToSeconds(ms)); } + +} // namespace reasampler::instrument::note diff --git a/src/core/instrument/note/tempo.h b/src/core/instrument/note/tempo.h new file mode 100644 index 0000000..706a18e --- /dev/null +++ b/src/core/instrument/note/tempo.h @@ -0,0 +1,37 @@ +// tempo — a validated project tempo and every beats <-> seconds <-> ms conversion a +// beat-denominated capture value resolves through. +// +// A BEAT IS A QUARTER NOTE — REAPER states project tempo in quarter notes per minute +// regardless of time signature, so a division resolves without one. + +#pragma once + +#include + +namespace reasampler::instrument::note { + +inline constexpr double kMsPerSecond = 1000.0; + +constexpr double msToSeconds(double ms) { return ms / kMsPerSecond; } +constexpr double secondsToMs(double seconds) { return seconds * kMsPerSecond; } + +class Tempo { +public: + // The only place a bad BPM is rejected, which is what lets every conversion below be + // total — no resolver downstream needs a failure path. + static std::optional fromBpm(double beatsPerMinute); + + double bpm() const { return bpm_; } + double secondsPerBeat() const; + + double beatsToSeconds(double beats) const; + double secondsToBeats(double seconds) const; + double beatsToMs(double beats) const; + double msToBeats(double ms) const; + +private: + explicit Tempo(double beatsPerMinute) : bpm_(beatsPerMinute) {} + double bpm_; +}; + +} // namespace reasampler::instrument::note diff --git a/tests/test_musical_division.cpp b/tests/test_musical_division.cpp new file mode 100644 index 0000000..449bb53 --- /dev/null +++ b/tests/test_musical_division.cpp @@ -0,0 +1,177 @@ +// Standalone tests for reasampler::instrument::note::musical_division — no VST3, no REAPER, +// no framework. Same fast assert loop as the sibling pure tests. +// +// Covers: the beat length of all 39 divisions against a literal rung table (NOT the module's +// own exponent formula); the 1/64 and 64/1 extremes; the four named example divisions; the +// label notation; picker order and index round-trip; off-ladder clamping. + +#include "../src/core/instrument/note/musical_division.h" + +#include + +using namespace reasampler::instrument::note; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +static bool almostEqual(double a, double b) { + const double d = a - b; + return (d < 0 ? -d : d) < 1e-12; +} + +// Length in beats (quarter notes) of each straight rung, written out rather than computed, +// so a broken exponent formula cannot agree with its own mistake. +static const double kStraightBeats[kRungCount] = { + 0.0625, // 1/64 + 0.125, // 1/32 + 0.25, // 1/16 + 0.5, // 1/8 + 1.0, // 1/4 + 2.0, // 1/2 + 4.0, // 1/1 + 8.0, // 2/1 + 16.0, // 4/1 + 32.0, // 8/1 + 64.0, // 16/1 + 128.0, // 32/1 + 256.0, // 64/1 +}; + +static const char* const kStraightLabels[kRungCount] = { + "1/64", "1/32", "1/16", "1/8", "1/4", "1/2", "1/1", + "2/1", "4/1", "8/1", "16/1", "32/1", "64/1", +}; + +// --- The ladder --------------------------------------------------------------- + +static void testLadderSpansSixtyfourthToSixtyFourWhole() { + CHECK(kRungCount == 13); + CHECK(kDivisionCount == 39); + CHECK(divisionLabel(divisionAt(0)) == "1/64"); + CHECK(divisionLabel(divisionAt(kDivisionCount - 1)) == "64/1t"); +} + +static void testEveryStraightRungHasItsWrittenBeatLength() { + for (int rung = 0; rung < kRungCount; ++rung) { + const Division d = makeDivision(kMinQuarterExponent + rung, DivisionModifier::Straight); + CHECK(almostEqual(divisionBeats(d), kStraightBeats[rung])); + CHECK(divisionLabel(d) == kStraightLabels[rung]); + } +} + +static void testDottedIsHalfAgainAndTripletIsTwoThirds() { + for (int rung = 0; rung < kRungCount; ++rung) { + const int e = kMinQuarterExponent + rung; + CHECK(almostEqual(divisionBeats(makeDivision(e, DivisionModifier::Dotted)), + kStraightBeats[rung] * 1.5)); + CHECK(almostEqual(divisionBeats(makeDivision(e, DivisionModifier::Triplet)), + kStraightBeats[rung] * 2.0 / 3.0)); + } +} + +static void testExtremes() { + // 1/64 straight is the shortest rung; 64/1 straight is the longest. + CHECK(almostEqual(divisionBeats(makeDivision(kMinQuarterExponent, DivisionModifier::Straight)), + 0.0625)); + CHECK(almostEqual(divisionBeats(makeDivision(kMaxQuarterExponent, DivisionModifier::Straight)), + 256.0)); + // The dotted 64/1 is the single longest programmable note. + CHECK(almostEqual(divisionBeats(makeDivision(kMaxQuarterExponent, DivisionModifier::Dotted)), + 384.0)); + // The 1/64 triplet is the shortest. + CHECK(almostEqual(divisionBeats(makeDivision(kMinQuarterExponent, DivisionModifier::Triplet)), + 0.0625 * 2.0 / 3.0)); +} + +// --- The four named examples -------------------------------------------------- + +static void testNamedExamples() { + // 1/8. — an eighth is half a beat, dotted is three quarters of one. + const Division dottedEighth = makeDivision(-1, DivisionModifier::Dotted); + CHECK(almostEqual(divisionBeats(dottedEighth), 0.75)); + CHECK(divisionLabel(dottedEighth) == "1/8."); + + // 1/4t — a quarter is one beat, the triplet is two thirds of one. + const Division quarterTriplet = makeDivision(0, DivisionModifier::Triplet); + CHECK(almostEqual(divisionBeats(quarterTriplet), 2.0 / 3.0)); + CHECK(divisionLabel(quarterTriplet) == "1/4t"); + + // 1/16 — a quarter of a beat. + const Division sixteenth = makeDivision(-2, DivisionModifier::Straight); + CHECK(almostEqual(divisionBeats(sixteenth), 0.25)); + CHECK(divisionLabel(sixteenth) == "1/16"); + + // 4/1 — four whole notes, sixteen beats. + const Division fourWhole = makeDivision(4, DivisionModifier::Straight); + CHECK(almostEqual(divisionBeats(fourWhole), 16.0)); + CHECK(divisionLabel(fourWhole) == "4/1"); +} + +// --- Picker order ------------------------------------------------------------- + +static void testPickerOrderIsShortestFirst() { + // Straight lengths ascend across rungs; within a rung the order is straight, dotted, + // triplet (so the index is not itself sorted by duration — only the rungs are). + for (int rung = 1; rung < kRungCount; ++rung) { + const double prev = divisionBeats(divisionAt((rung - 1) * kModifierCount)); + const double here = divisionBeats(divisionAt(rung * kModifierCount)); + CHECK(here > prev); + } + CHECK(divisionAt(0) == makeDivision(kMinQuarterExponent, DivisionModifier::Straight)); + CHECK(divisionAt(1) == makeDivision(kMinQuarterExponent, DivisionModifier::Dotted)); + CHECK(divisionAt(2) == makeDivision(kMinQuarterExponent, DivisionModifier::Triplet)); +} + +static void testIndexRoundTripsOverTheWholeSet() { + for (int i = 0; i < kDivisionCount; ++i) { + CHECK(divisionIndex(divisionAt(i)) == i); + } +} + +static void testEverySetMemberIsDistinct() { + // No two indices name the same division, so the picker offers 39 real choices. + for (int i = 0; i < kDivisionCount; ++i) { + for (int j = i + 1; j < kDivisionCount; ++j) { + CHECK(divisionAt(i) != divisionAt(j)); + } + } +} + +// --- Clamping ----------------------------------------------------------------- + +static void testOffLadderExponentClampsToTheNearestRung() { + CHECK(makeDivision(-99, DivisionModifier::Straight) + == makeDivision(kMinQuarterExponent, DivisionModifier::Straight)); + CHECK(makeDivision(99, DivisionModifier::Triplet) + == makeDivision(kMaxQuarterExponent, DivisionModifier::Triplet)); + // A record carrying an off-ladder exponent still resolves to a real length. + Division corrupt; + corrupt.quarterExponent = 120; + CHECK(almostEqual(divisionBeats(corrupt), 256.0)); +} + +static void testOutOfRangeIndexClampsIntoTheSet() { + CHECK(divisionAt(-1) == divisionAt(0)); + CHECK(divisionAt(kDivisionCount) == divisionAt(kDivisionCount - 1)); +} + +int main() { + testLadderSpansSixtyfourthToSixtyFourWhole(); + testEveryStraightRungHasItsWrittenBeatLength(); + testDottedIsHalfAgainAndTripletIsTwoThirds(); + testExtremes(); + + testNamedExamples(); + + testPickerOrderIsShortestFirst(); + testIndexRoundTripsOverTheWholeSet(); + testEverySetMemberIsDistinct(); + + testOffLadderExponentClampsToTheNearestRung(); + testOutOfRangeIndexClampsIntoTheSet(); + + if (g_fail == 0) std::printf("musical_division: all tests passed\n"); + else std::printf("musical_division: %d FAILED\n", g_fail); + return g_fail == 0 ? 0 : 1; +} diff --git a/tests/test_note_program.cpp b/tests/test_note_program.cpp new file mode 100644 index 0000000..03a5afa --- /dev/null +++ b/tests/test_note_program.cpp @@ -0,0 +1,359 @@ +// Standalone tests for reasampler::instrument::note::note_program — no VST3, no REAPER, no +// framework. Same fast assert loop as the sibling pure tests. +// +// Covers: velocity clamping; the ms/beats denomination seam and its round-trip; anchoring +// (start to note-on, end to note-off); the resolved window against hand-computed values; +// every division resolving to its duration in seconds; proportionality across two tempos; +// record equality and copy round-trip. + +#include "../src/core/instrument/note/note_program.h" + +#include +#include + +using namespace reasampler::instrument::note; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +static bool almostEqual(double a, double b, double eps = 1e-9) { + return std::fabs(a - b) < eps; +} + +static Tempo at(double bpm) { + const std::optional t = Tempo::fromBpm(bpm); + if (!t) { std::printf("FAIL: fixture tempo %f rejected\n", bpm); ++g_fail; } + return t.value_or(Tempo::fromBpm(120.0).value()); +} + +// Beats per straight rung, written out rather than computed — see test_musical_division. +static const double kStraightBeats[kRungCount] = { + 0.0625, 0.125, 0.25, 0.5, 1.0, 2.0, 4.0, 8.0, 16.0, 32.0, 64.0, 128.0, 256.0, +}; + +static NoteProgram program(Division length, OffsetAmount start, OffsetAmount end, int velocity) { + NoteProgram p; + p.length = length; + p.start = StartOffset(start); + p.end = EndOffset(end); + p.velocity = Velocity::of(velocity); + return p; +} + +// --- Velocity ------------------------------------------------------------------ + +static void testVelocityCarriesInRange() { + CHECK(Velocity::of(1).value() == 1); + CHECK(Velocity::of(96).value() == 96); + CHECK(Velocity::of(127).value() == 127); +} + +static void testVelocityClampsOutOfRange() { + // 0 is note-off in MIDI: a programmed note that does not sound is never the intent. + CHECK(Velocity::of(0).value() == 1); + CHECK(Velocity::of(-40).value() == 1); + CHECK(Velocity::of(128).value() == 127); + CHECK(Velocity::of(9000).value() == 127); +} + +static void testResolvedNoteCarriesTheProgrammedVelocity() { + const Tempo t = at(120.0); + CHECK(resolveNote(program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(0.0), + offsetFromMs(0.0), 96), + t) + .velocity + == 96); + CHECK(resolveNote(program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(0.0), + offsetFromMs(0.0), 0), + t) + .velocity + == 1); +} + +// --- The denomination seam ----------------------------------------------------- + +static void testMsOffsetReadsBackInBothDenominations() { + // 120 BPM: one beat is 500 ms, so 250 ms is half a beat. + const Tempo t = at(120.0); + const OffsetAmount a = offsetFromMs(250.0); + CHECK(almostEqual(offsetMs(a, t), 250.0)); + CHECK(almostEqual(offsetBeats(a, t), 0.5)); + CHECK(almostEqual(offsetSeconds(a, t), 0.25)); +} + +static void testBeatsOffsetReadsBackInBothDenominations() { + // 80 BPM: one beat is 750 ms. + const Tempo t = at(80.0); + const OffsetAmount a = offsetFromBeats(2.0); + CHECK(almostEqual(offsetBeats(a, t), 2.0)); + CHECK(almostEqual(offsetMs(a, t), 1500.0, 1e-6)); + CHECK(almostEqual(offsetSeconds(a, t), 1.5)); +} + +static void testRedenominationRoundTripsLosslessly() { + const double bpms[] = {44.0, 91.7, 120.0, 200.0}; + const double magnitudes[] = {-500.0, -20.0, 0.0, 0.25, 333.0}; + for (double bpm : bpms) { + const Tempo t = at(bpm); + for (double ms : magnitudes) { + const OffsetAmount original = offsetFromMs(ms); + const OffsetAmount there = redenominate(original, Denomination::Beats, t); + const OffsetAmount back = redenominate(there, Denomination::Milliseconds, t); + CHECK(there.denomination == Denomination::Beats); + CHECK(back.denomination == Denomination::Milliseconds); + CHECK(almostEqual(back.magnitude, ms, 1e-9 + 1e-9 * std::fabs(ms))); + // Re-denominating never moves the instant it names. + CHECK(almostEqual(offsetSeconds(there, t), offsetSeconds(original, t))); + } + for (double beats : magnitudes) { + const OffsetAmount original = offsetFromBeats(beats); + const OffsetAmount back = + redenominate(redenominate(original, Denomination::Milliseconds, t), + Denomination::Beats, t); + CHECK(almostEqual(back.magnitude, beats, 1e-9 + 1e-9 * std::fabs(beats))); + } + } +} + +static void testRedenominatingToTheSameUnitIsIdentity() { + const Tempo t = at(120.0); + const OffsetAmount a = offsetFromMs(37.0); + CHECK(redenominate(a, Denomination::Milliseconds, t) == a); +} + +static void testStoredDenominationDecidesWhetherAnOffsetFollowsTheTempo() { + // The whole reason the denomination is stored: at half the tempo the beats offset is + // twice as long in seconds, the ms offset unchanged. + const OffsetAmount inMs = offsetFromMs(500.0); + const OffsetAmount inBeats = offsetFromBeats(1.0); + const Tempo fast = at(120.0); + const Tempo slow = at(60.0); + CHECK(almostEqual(offsetSeconds(inMs, fast), offsetSeconds(inMs, slow))); + CHECK(almostEqual(offsetSeconds(inBeats, slow), 2.0 * offsetSeconds(inBeats, fast))); +} + +// --- Note length in seconds ---------------------------------------------------- + +static void testEveryDivisionResolvesToItsDuration() { + // 120 BPM: one beat is 0.5 s, so a division's length in seconds is half its beats. + const Tempo t = at(120.0); + const OffsetAmount none = offsetFromMs(0.0); + for (int rung = 0; rung < kRungCount; ++rung) { + const int e = kMinQuarterExponent + rung; + const double straight = kStraightBeats[rung] * 0.5; + CHECK(almostEqual( + resolveNote(program(makeDivision(e, DivisionModifier::Straight), none, none, 100), t) + .noteOffSeconds, + straight, 1e-9 + 1e-9 * straight)); + CHECK(almostEqual( + resolveNote(program(makeDivision(e, DivisionModifier::Dotted), none, none, 100), t) + .noteOffSeconds, + straight * 1.5, 1e-9 + 1e-9 * straight)); + CHECK(almostEqual( + resolveNote(program(makeDivision(e, DivisionModifier::Triplet), none, none, 100), t) + .noteOffSeconds, + straight * 2.0 / 3.0, 1e-9 + 1e-9 * straight)); + } +} + +static void testExtremeAndNamedDivisionsInSeconds() { + // 120 BPM: one beat is 0.5 s. + const Tempo t = at(120.0); + const OffsetAmount none = offsetFromMs(0.0); + struct Case { Division d; double seconds; }; + const Case cases[] = { + {makeDivision(kMinQuarterExponent, DivisionModifier::Straight), 0.03125}, // 1/64 + {makeDivision(kMaxQuarterExponent, DivisionModifier::Straight), 128.0}, // 64/1 + {makeDivision(-1, DivisionModifier::Dotted), 0.375}, // 1/8. + {makeDivision(0, DivisionModifier::Triplet), 1.0 / 3.0}, // 1/4t + {makeDivision(-2, DivisionModifier::Straight), 0.125}, // 1/16 + {makeDivision(4, DivisionModifier::Straight), 8.0}, // 4/1 + }; + for (const Case& c : cases) { + CHECK(almostEqual(resolveNote(program(c.d, none, none, 100), t).noteOffSeconds, + c.seconds, 1e-9 + 1e-9 * c.seconds)); + } +} + +static void testNoteLengthIsProportionalToTempo() { + // Ratio only — no seconds value is asserted here, so the module's tempo-freedom is what + // is under test rather than any particular rate. + const OffsetAmount none = offsetFromMs(0.0); + const Tempo fast = at(160.0); + const Tempo slow = at(40.0); + for (int i = 0; i < kDivisionCount; ++i) { + const NoteProgram p = program(divisionAt(i), none, none, 100); + CHECK(almostEqual(resolveNote(p, slow).noteOffSeconds, + 4.0 * resolveNote(p, fast).noteOffSeconds, 1e-9)); + } +} + +// --- The resolved window ------------------------------------------------------- + +static void testWindowAnchorsStartToNoteOnAndEndToNoteOff() { + // 120 BPM, 1/4 note = 0.5 s. Daniel's case: open 20 ms before note-on, close 500 ms + // after note-off. + const Tempo t = at(120.0); + const ResolvedNote r = resolveNote(program(makeDivision(0, DivisionModifier::Straight), + offsetFromMs(-20.0), offsetFromMs(500.0), 96), + t); + CHECK(almostEqual(r.noteOffSeconds, 0.5)); + CHECK(almostEqual(r.captureStartSeconds, -0.020)); // note-on is 0, so the pre-roll is negative + CHECK(almostEqual(r.captureEndSeconds, 1.0)); // 0.5 note-off + 0.5 tail + CHECK(almostEqual(r.captureLengthSeconds(), 1.02)); + CHECK(r.velocity == 96); +} + +static void testEndOffsetMovesWithTheNoteLength() { + // The end offset anchors to note-off, so lengthening the note moves the window's end by + // the same amount and leaves its start alone. + const Tempo t = at(120.0); + const OffsetAmount start = offsetFromMs(-20.0); + const OffsetAmount end = offsetFromMs(500.0); + const ResolvedNote quarter = + resolveNote(program(makeDivision(0, DivisionModifier::Straight), start, end, 100), t); + const ResolvedNote half = + resolveNote(program(makeDivision(1, DivisionModifier::Straight), start, end, 100), t); + CHECK(almostEqual(half.captureStartSeconds, quarter.captureStartSeconds)); + CHECK(almostEqual(half.captureEndSeconds - quarter.captureEndSeconds, 0.5)); +} + +static void testBeatsDenominatedOffsetsResolveAgainstTheSuppliedTempo() { + // 1/4 note, start -1/2 beat, end +1 beat. At 120 BPM (0.5 s/beat): note-off 0.5, + // window -0.25 .. 1.0. At 60 BPM every one of those doubles. + const NoteProgram p = program(makeDivision(0, DivisionModifier::Straight), + offsetFromBeats(-0.5), offsetFromBeats(1.0), 100); + const ResolvedNote fast = resolveNote(p, at(120.0)); + CHECK(almostEqual(fast.captureStartSeconds, -0.25)); + CHECK(almostEqual(fast.captureEndSeconds, 1.0)); + + const ResolvedNote slow = resolveNote(p, at(60.0)); + CHECK(almostEqual(slow.captureStartSeconds, -0.5)); + CHECK(almostEqual(slow.captureEndSeconds, 2.0)); +} + +static void testMixedDenominationsResolveIndependently() { + // A ms pre-roll and a beats tail on one record: halving the tempo moves the tail only. + const NoteProgram p = program(makeDivision(0, DivisionModifier::Straight), + offsetFromMs(-20.0), offsetFromBeats(1.0), 100); + const ResolvedNote fast = resolveNote(p, at(120.0)); + const ResolvedNote slow = resolveNote(p, at(60.0)); + CHECK(almostEqual(fast.captureStartSeconds, -0.020)); + CHECK(almostEqual(slow.captureStartSeconds, -0.020)); + CHECK(almostEqual(fast.captureEndSeconds, 1.0)); + CHECK(almostEqual(slow.captureEndSeconds, 2.0)); +} + +static void testNegativeEndOffsetTruncatesBeforeRelease() { + // 1/2 note at 120 BPM is 1.0 s; closing 200 ms early ends the window at 0.8 s. + const Tempo t = at(120.0); + const ResolvedNote r = resolveNote(program(makeDivision(1, DivisionModifier::Straight), + offsetFromMs(0.0), offsetFromMs(-200.0), 100), + t); + CHECK(almostEqual(r.noteOffSeconds, 1.0)); + CHECK(almostEqual(r.captureEndSeconds, 0.8)); + CHECK(almostEqual(r.captureLengthSeconds(), 0.8)); +} + +static void testWindowNeverInverts() { + // An end offset past the window's own start collapses the window rather than inverting it. + const Tempo t = at(120.0); + const ResolvedNote r = resolveNote(program(makeDivision(0, DivisionModifier::Straight), + offsetFromMs(0.0), offsetFromMs(-5000.0), 100), + t); + CHECK(almostEqual(r.captureStartSeconds, 0.0)); + CHECK(almostEqual(r.captureEndSeconds, 0.0)); + CHECK(r.captureLengthSeconds() >= 0.0); +} + +// --- The record ---------------------------------------------------------------- + +static void testRecordRoundTripsAsAWhole() { + const NoteProgram original = program(makeDivision(-1, DivisionModifier::Dotted), + offsetFromMs(-20.0), offsetFromBeats(2.0), 96); + const NoteProgram copy = original; + CHECK(copy == original); + CHECK(copy.length == makeDivision(-1, DivisionModifier::Dotted)); + CHECK(copy.start.amount() == offsetFromMs(-20.0)); + CHECK(copy.end.amount() == offsetFromBeats(2.0)); + CHECK(copy.velocity.value() == 96); + + // Resolving reads the record and leaves it alone, so a preview cannot drift the state a + // later bake reads. + resolveNote(original, at(120.0)); + CHECK(copy == original); +} + +static void testRecordEqualityIsSensitiveToEveryField() { + const NoteProgram base = program(makeDivision(0, DivisionModifier::Straight), + offsetFromMs(-20.0), offsetFromMs(500.0), 96); + CHECK(base != program(makeDivision(0, DivisionModifier::Dotted), offsetFromMs(-20.0), + offsetFromMs(500.0), 96)); + CHECK(base != program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(-21.0), + offsetFromMs(500.0), 96)); + CHECK(base != program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(-20.0), + offsetFromMs(501.0), 96)); + CHECK(base != program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(-20.0), + offsetFromMs(500.0), 97)); + // Same magnitude, different denomination is a different record even where one tempo + // makes them resolve alike. + CHECK(base != program(makeDivision(0, DivisionModifier::Straight), offsetFromBeats(-20.0), + offsetFromMs(500.0), 96)); +} + +static void testRedenominatedRecordDescribesTheSameWindow() { + const Tempo t = at(133.0); + const NoteProgram original = program(makeDivision(-2, DivisionModifier::Triplet), + offsetFromMs(-35.0), offsetFromMs(420.0), 64); + NoteProgram restated = original; + restated.start = StartOffset(redenominate(original.start.amount(), Denomination::Beats, t)); + restated.end = EndOffset(redenominate(original.end.amount(), Denomination::Beats, t)); + + const ResolvedNote a = resolveNote(original, t); + const ResolvedNote b = resolveNote(restated, t); + CHECK(restated != original); // the record changed... + CHECK(almostEqual(a.captureStartSeconds, b.captureStartSeconds)); // ...the window did not + CHECK(almostEqual(a.captureEndSeconds, b.captureEndSeconds)); +} + +static void testDefaultRecordIsAQuarterNoteWithNoOffsets() { + const Tempo t = at(120.0); + const ResolvedNote r = resolveNote(NoteProgram{}, t); + CHECK(almostEqual(r.noteOffSeconds, 0.5)); + CHECK(almostEqual(r.captureStartSeconds, 0.0)); + CHECK(almostEqual(r.captureEndSeconds, 0.5)); + CHECK(r.velocity >= Velocity::kMin && r.velocity <= Velocity::kMax); +} + +int main() { + testVelocityCarriesInRange(); + testVelocityClampsOutOfRange(); + testResolvedNoteCarriesTheProgrammedVelocity(); + + testMsOffsetReadsBackInBothDenominations(); + testBeatsOffsetReadsBackInBothDenominations(); + testRedenominationRoundTripsLosslessly(); + testRedenominatingToTheSameUnitIsIdentity(); + testStoredDenominationDecidesWhetherAnOffsetFollowsTheTempo(); + + testEveryDivisionResolvesToItsDuration(); + testExtremeAndNamedDivisionsInSeconds(); + testNoteLengthIsProportionalToTempo(); + + testWindowAnchorsStartToNoteOnAndEndToNoteOff(); + testEndOffsetMovesWithTheNoteLength(); + testBeatsDenominatedOffsetsResolveAgainstTheSuppliedTempo(); + testMixedDenominationsResolveIndependently(); + testNegativeEndOffsetTruncatesBeforeRelease(); + testWindowNeverInverts(); + + testRecordRoundTripsAsAWhole(); + testRecordEqualityIsSensitiveToEveryField(); + testRedenominatedRecordDescribesTheSameWindow(); + testDefaultRecordIsAQuarterNoteWithNoOffsets(); + + if (g_fail == 0) std::printf("note_program: all tests passed\n"); + else std::printf("note_program: %d FAILED\n", g_fail); + return g_fail == 0 ? 0 : 1; +} diff --git a/tests/test_tempo.cpp b/tests/test_tempo.cpp new file mode 100644 index 0000000..dcfa06c --- /dev/null +++ b/tests/test_tempo.cpp @@ -0,0 +1,135 @@ +// Standalone tests for reasampler::instrument::note::tempo — no VST3, no REAPER, no +// framework. Same fast assert loop as the sibling pure tests. +// +// Covers: BPM validation (the only rejection point, which is what makes the conversions +// total); seconds-per-beat at several tempos; beats<->seconds and beats<->ms round-trips +// across tempos and signs; the proportionality between two tempos, asserted as a ratio +// rather than against any fixed seconds value. + +#include "../src/core/instrument/note/tempo.h" + +#include +#include +#include + +using namespace reasampler::instrument::note; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +static bool almostEqual(double a, double b, double eps = 1e-9) { + return std::fabs(a - b) < eps; +} + +static Tempo at(double bpm) { + const std::optional t = Tempo::fromBpm(bpm); + if (!t) { std::printf("FAIL: fixture tempo %f rejected\n", bpm); ++g_fail; } + return t.value_or(Tempo::fromBpm(120.0).value()); +} + +// --- Validation --------------------------------------------------------------- + +static void testUsableBpmIsAccepted() { + const std::optional t = Tempo::fromBpm(137.5); + CHECK(t.has_value()); + CHECK(t && almostEqual(t->bpm(), 137.5)); +} + +static void testUnusableBpmIsRejected() { + CHECK(!Tempo::fromBpm(0.0).has_value()); + CHECK(!Tempo::fromBpm(-120.0).has_value()); + CHECK(!Tempo::fromBpm(std::numeric_limits::quiet_NaN()).has_value()); + CHECK(!Tempo::fromBpm(std::numeric_limits::infinity()).has_value()); +} + +// --- Conversions --------------------------------------------------------------- + +static void testSecondsPerBeatFollowsBpm() { + CHECK(almostEqual(at(60.0).secondsPerBeat(), 1.0)); + CHECK(almostEqual(at(120.0).secondsPerBeat(), 0.5)); + CHECK(almostEqual(at(240.0).secondsPerBeat(), 0.25)); +} + +static void testBeatsToSecondsAtAKnownTempo() { + // 90 BPM: one beat is 2/3 s, so four beats are 8/3 s. + const Tempo t = at(90.0); + CHECK(almostEqual(t.beatsToSeconds(1.0), 2.0 / 3.0)); + CHECK(almostEqual(t.beatsToSeconds(4.0), 8.0 / 3.0)); + CHECK(almostEqual(t.secondsToBeats(8.0 / 3.0), 4.0)); +} + +static void testBeatsToMsAtAKnownTempo() { + // 150 BPM: one beat is 400 ms. + const Tempo t = at(150.0); + CHECK(almostEqual(t.beatsToMs(1.0), 400.0, 1e-6)); + CHECK(almostEqual(t.msToBeats(400.0), 1.0)); + CHECK(almostEqual(t.msToBeats(100.0), 0.25)); +} + +static void testMsAndBeatsRoundTripAcrossTemposAndSigns() { + const double bpms[] = {33.0, 77.3, 120.0, 174.6, 300.0}; + const double values[] = {-500.0, -20.0, 0.0, 0.5, 250.0, 12345.678}; + for (double bpm : bpms) { + const Tempo t = at(bpm); + for (double ms : values) { + CHECK(almostEqual(t.beatsToMs(t.msToBeats(ms)), ms, 1e-9 + 1e-9 * std::fabs(ms))); + } + for (double beats : values) { + CHECK(almostEqual(t.msToBeats(t.beatsToMs(beats)), beats, + 1e-9 + 1e-9 * std::fabs(beats))); + } + } +} + +static void testSecondsRoundTrip() { + const Tempo t = at(101.7); + CHECK(almostEqual(t.secondsToBeats(t.beatsToSeconds(3.25)), 3.25)); + CHECK(almostEqual(t.beatsToSeconds(t.secondsToBeats(-1.75)), -1.75)); +} + +// --- Proportionality ----------------------------------------------------------- + +static void testHalvingTheTempoDoublesEveryBeatDuration() { + // The ratio is the claim; no seconds value is asserted, so the test cannot encode a + // fixed tempo of its own. + const Tempo fast = at(140.0); + const Tempo slow = at(70.0); + for (double beats : {0.0625, 0.75, 2.0 / 3.0, 16.0, 256.0}) { + CHECK(almostEqual(slow.beatsToSeconds(beats), 2.0 * fast.beatsToSeconds(beats), 1e-9)); + } +} + +static void testSecondsScaleInverselyWithBpm() { + const Tempo a = at(96.0); + const Tempo b = at(123.0); + const double beats = 3.5; + CHECK(almostEqual(a.beatsToSeconds(beats) / b.beatsToSeconds(beats), 123.0 / 96.0)); +} + +static void testMillisecondsAreTempoFree() { + // The ms<->seconds pair carries no tempo — that is what lets a ms-denominated offset + // hold still while a beats-denominated one moves. + CHECK(almostEqual(msToSeconds(250.0), 0.25)); + CHECK(almostEqual(secondsToMs(1.5), 1500.0)); + CHECK(almostEqual(msToSeconds(secondsToMs(0.037)), 0.037)); +} + +int main() { + testUsableBpmIsAccepted(); + testUnusableBpmIsRejected(); + + testSecondsPerBeatFollowsBpm(); + testBeatsToSecondsAtAKnownTempo(); + testBeatsToMsAtAKnownTempo(); + testMsAndBeatsRoundTripAcrossTemposAndSigns(); + testSecondsRoundTrip(); + + testHalvingTheTempoDoublesEveryBeatDuration(); + testSecondsScaleInverselyWithBpm(); + testMillisecondsAreTempoFree(); + + if (g_fail == 0) std::printf("tempo: all tests passed\n"); + else std::printf("tempo: %d FAILED\n", g_fail); + return g_fail == 0 ? 0 : 1; +} From d923b352aed16ad8c81349d2815760d2089931c7 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 30 Jul 2026 20:11:06 -0400 Subject: [PATCH 2/4] fix: harden note-program model against corrupt tempo/division/denomination records Reject subnormal BPM that overflows to NaN, normalize Division equality, pin a single out-of-range-denomination interpretation across all readers, flag collapsed capture windows, add offset off-view editors and structural static_asserts, collapse duplicated CLAUDE.md facts. --- src/core/instrument/note/CLAUDE.md | 21 ++--- src/core/instrument/note/CMakeLists.txt | 8 +- src/core/instrument/note/musical_division.cpp | 6 +- src/core/instrument/note/note_program.cpp | 43 +++++++--- src/core/instrument/note/note_program.h | 20 ++++- src/core/instrument/note/tempo.cpp | 4 + src/core/instrument/note/tempo.h | 4 + tests/test_musical_division.cpp | 24 ++++-- tests/test_note_program.cpp | 78 +++++++++++++++++-- tests/test_tempo.cpp | 3 + 10 files changed, 175 insertions(+), 36 deletions(-) diff --git a/src/core/instrument/note/CLAUDE.md b/src/core/instrument/note/CLAUDE.md index 4f3c7c2..9a00025 100644 --- a/src/core/instrument/note/CLAUDE.md +++ b/src/core/instrument/note/CLAUDE.md @@ -29,16 +29,18 @@ diverge: the capture-signal popup that edits it and the bake that renders it. - **A division persists as its `{quarterExponent, modifier}` pair, never as its picker index.** The index is presentation order and would silently re-map every saved record if the ladder ever gained a rung or a modifier. -- **An offset stores the denomination it was entered in.** The other view is derived on - demand. Storing resolved seconds instead would make a beats-denominated offset stop - following the tempo, which is the only reason to express one in beats. +- **An offset stores the denomination it was entered in** — see `OffsetAmount` in + `note_program.h` for why. +- **Does not carry a MIDI note number.** `NoteProgram` describes timing and velocity only; + render pitch is deferred to a later additive field (Ξ-W2) rather than assumed to live + here. ## Modules - `musical_division` — the note-length ladder: 1/64 through 64/1 (a rung is the base-2 exponent of its length in quarter notes, -4..8), each straight, dotted (x3/2), or triplet - (x2/3); the 39-entry picker order; and the `"1/8."` / `"1/4t"` label notation. Lengths in - beats only, so it links no tempo. + (x2/3); the 39-entry picker order; and the `"1/8."` / `"1/4t"` label notation. Beats only + — see `musical_division.h` for why it links no tempo. - `tempo` — a validated project tempo plus every beats <-> seconds <-> ms conversion. Construction (`Tempo::fromBpm`) is the only place a bad BPM is rejected, which is what lets each conversion be total and every downstream resolver be failure-free. @@ -48,10 +50,11 @@ diverge: the capture-signal popup that edits it and the bake that renders it. ## Gotchas -- **A beat is a quarter note.** REAPER states project tempo in quarter notes per minute - regardless of time signature, so divisions resolve with no time signature in sight. A - beats *readout* that should track a compound meter's dotted-quarter pulse would need the - time signature threaded in — it is not, deliberately. +- **A beat is a quarter note** — see `tempo.h` for why. A beats *readout* that should track + a compound meter's dotted-quarter pulse would need the time signature threaded in — it is + not, deliberately. `src/core/ui/card_meta.cpp` is the sibling module that *does* fold + `timeSigDenom` into its own seconds-per-beat — a different, also-correct convention for a + different job; don't read the divergence as a bug in either. - **`StartOffset` and `EndOffset` are distinct types on purpose.** They hold the same payload and differ only in what they anchor to (note-on and note-off respectively); collapsing them into one type with an anchor field makes the swap a runtime bug instead diff --git a/src/core/instrument/note/CMakeLists.txt b/src/core/instrument/note/CMakeLists.txt index 25dee5e..dbffa83 100644 --- a/src/core/instrument/note/CMakeLists.txt +++ b/src/core/instrument/note/CMakeLists.txt @@ -1,13 +1,13 @@ reasampler_pure_library(musical_division SOURCES musical_division.cpp) -# Links only musical_division: the ladder is beats-only, so it must prove out with no tempo -# in the link line at all. +# Links only musical_division — the beats-only contract (see musical_division.h) needs no +# tempo in the link line. reasampler_test(musical_division LINK musical_division) reasampler_pure_library(tempo SOURCES tempo.cpp) reasampler_test(tempo LINK tempo) -# The record composes the ladder and the tempo and nothing else — the programmed signal is -# plain data, provable without the engine, the bank, or a host. +# note_program links exactly these two: it composes the ladder and the tempo and nothing +# else (see note_program.h). reasampler_pure_library(note_program SOURCES note_program.cpp LINK PUBLIC musical_division tempo) diff --git a/src/core/instrument/note/musical_division.cpp b/src/core/instrument/note/musical_division.cpp index 22de6e7..443d0ac 100644 --- a/src/core/instrument/note/musical_division.cpp +++ b/src/core/instrument/note/musical_division.cpp @@ -24,7 +24,11 @@ int clampExponent(int quarterExponent) { } // namespace bool operator==(Division a, Division b) { - return a.quarterExponent == b.quarterExponent && a.modifier == b.modifier; + // Normalize both sides through makeDivision first: a persisted off-ladder exponent must + // compare equal to its clamped form, the same as every other reader in this file. + const Division la = makeDivision(a.quarterExponent, a.modifier); + const Division lb = makeDivision(b.quarterExponent, b.modifier); + return la.quarterExponent == lb.quarterExponent && la.modifier == lb.modifier; } bool operator!=(Division a, Division b) { return !(a == b); } diff --git a/src/core/instrument/note/note_program.cpp b/src/core/instrument/note/note_program.cpp index f5a4be5..683a356 100644 --- a/src/core/instrument/note/note_program.cpp +++ b/src/core/instrument/note/note_program.cpp @@ -24,20 +24,31 @@ OffsetAmount offsetFromMs(double ms) { return {ms, Denomination::Milliseconds}; OffsetAmount offsetFromBeats(double beats) { return {beats, Denomination::Beats}; } +// All three readers switch on Denomination with the same default (Milliseconds, the +// struct's own default value) so a corrupt persisted record reads identically everywhere — +// a popup and a bake must never disagree on an out-of-range denomination byte. double offsetMs(OffsetAmount amount, Tempo tempo) { - return amount.denomination == Denomination::Milliseconds ? amount.magnitude - : tempo.beatsToMs(amount.magnitude); + switch (amount.denomination) { + case Denomination::Beats: return tempo.beatsToMs(amount.magnitude); + case Denomination::Milliseconds: + default: return amount.magnitude; + } } double offsetBeats(OffsetAmount amount, Tempo tempo) { - return amount.denomination == Denomination::Beats ? amount.magnitude - : tempo.msToBeats(amount.magnitude); + switch (amount.denomination) { + case Denomination::Beats: return amount.magnitude; + case Denomination::Milliseconds: + default: return tempo.msToBeats(amount.magnitude); + } } double offsetSeconds(OffsetAmount amount, Tempo tempo) { - return amount.denomination == Denomination::Beats - ? tempo.beatsToSeconds(amount.magnitude) - : msToSeconds(amount.magnitude); + switch (amount.denomination) { + case Denomination::Beats: return tempo.beatsToSeconds(amount.magnitude); + case Denomination::Milliseconds: + default: return msToSeconds(amount.magnitude); + } } OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo) { @@ -46,6 +57,18 @@ OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo) { : offsetFromMs(offsetMs(amount, tempo)); } +OffsetAmount withMsView(OffsetAmount amount, double ms, Tempo tempo) { + return amount.denomination == Denomination::Milliseconds + ? offsetFromMs(ms) + : offsetFromBeats(tempo.msToBeats(ms)); +} + +OffsetAmount withBeatsView(OffsetAmount amount, double beats, Tempo tempo) { + return amount.denomination == Denomination::Beats + ? offsetFromBeats(beats) + : offsetFromMs(tempo.beatsToMs(beats)); +} + bool operator==(const NoteProgram& a, const NoteProgram& b) { return a.length == b.length && a.start.amount() == b.start.amount() && a.end.amount() == b.end.amount() && a.velocity == b.velocity; @@ -57,10 +80,12 @@ ResolvedNote resolveNote(const NoteProgram& program, Tempo tempo) { ResolvedNote out; out.noteOffSeconds = tempo.beatsToSeconds(divisionBeats(program.length)); out.captureStartSeconds = offsetSeconds(program.start.amount(), tempo); - out.captureEndSeconds = out.noteOffSeconds + offsetSeconds(program.end.amount(), tempo); + const double rawEndSeconds = out.noteOffSeconds + offsetSeconds(program.end.amount(), tempo); // An inverted window has no meaning to a renderer, so a far-negative end offset yields a // zero-length capture the caller can reject rather than a negative one it cannot. - out.captureEndSeconds = (std::max)(out.captureEndSeconds, out.captureStartSeconds); + // windowCollapsed distinguishes that from a genuinely zero-length program. + out.windowCollapsed = rawEndSeconds < out.captureStartSeconds; + out.captureEndSeconds = (std::max)(rawEndSeconds, out.captureStartSeconds); out.velocity = program.velocity.value(); return out; } diff --git a/src/core/instrument/note/note_program.h b/src/core/instrument/note/note_program.h index 02aacad..d0973fa 100644 --- a/src/core/instrument/note/note_program.h +++ b/src/core/instrument/note/note_program.h @@ -1,12 +1,12 @@ // note_program — the programmed capture signal: one note length, one velocity, two anchored // offsets, and the one resolver a preview and a bake must share. // -// Resolved times are rate-free SECONDS relative to note-on; the caller converts against the -// live sample rate. +// Resolved times are rate-free seconds (this directory's CLAUDE.md: the standing ruling). #pragma once #include +#include #include "core/instrument/note/musical_division.h" #include "core/instrument/note/tempo.h" @@ -52,6 +52,12 @@ double offsetSeconds(OffsetAmount amount, Tempo tempo); // The unit toggle: the same instant restated in the other denomination. OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo); +// Edit the magnitude via its non-stored view without changing which denomination is stored +// — a popup's ms and beats fields both stay live no matter which one the offset was entered +// in; only `redenominate` changes the stored denomination itself. +OffsetAmount withMsView(OffsetAmount amount, double ms, Tempo tempo); +OffsetAmount withBeatsView(OffsetAmount amount, double beats, Tempo tempo); + // Two types rather than one carrying an anchor field: the anchor is then unswappable at // compile time. Sign is uniform — positive is later in time — so a capture that opens before // the note is a negative start offset, and a negative end offset truncates before release. @@ -75,6 +81,11 @@ private: OffsetAmount amount_{}; }; +static_assert(!std::is_constructible_v, + "StartOffset and EndOffset must not be interchangeable at compile time"); +static_assert(!std::is_convertible_v, + "the anchor constructor must stay explicit"); + struct NoteProgram { Division length{}; StartOffset start{}; @@ -89,7 +100,10 @@ struct ResolvedNote { double noteOffSeconds = 0.0; // == the note's sounding length, note-on being 0 double captureStartSeconds = 0.0; // negative when the capture opens before the note double captureEndSeconds = 0.0; - std::uint8_t velocity = 1; + std::uint8_t velocity = 100; // resolveNote always overwrites this; matches Velocity's own default + // True when the programmed end offset inverted the window and resolveNote collapsed it + // to zero length instead — lets a popup explain an empty window rather than just show one. + bool windowCollapsed = false; double captureLengthSeconds() const { return captureEndSeconds - captureStartSeconds; } }; diff --git a/src/core/instrument/note/tempo.cpp b/src/core/instrument/note/tempo.cpp index 7f75d7d..bb7e5e4 100644 --- a/src/core/instrument/note/tempo.cpp +++ b/src/core/instrument/note/tempo.cpp @@ -11,6 +11,10 @@ constexpr double kSecondsPerMinute = 60.0; std::optional Tempo::fromBpm(double beatsPerMinute) { if (!std::isfinite(beatsPerMinute) || beatsPerMinute <= 0.0) return std::nullopt; + // A subnormal BPM is finite and positive but overflows 60/bpm to +inf, which then turns + // any beatsToSeconds(0) into NaN downstream — reject it here so every conversion below + // stays total. + if (!std::isfinite(kSecondsPerMinute / beatsPerMinute)) return std::nullopt; return Tempo(beatsPerMinute); } diff --git a/src/core/instrument/note/tempo.h b/src/core/instrument/note/tempo.h index 706a18e..7c2f8a6 100644 --- a/src/core/instrument/note/tempo.h +++ b/src/core/instrument/note/tempo.h @@ -7,6 +7,7 @@ #pragma once #include +#include namespace reasampler::instrument::note { @@ -34,4 +35,7 @@ private: double bpm_; }; +static_assert(!std::is_default_constructible_v, + "Tempo must not be constructible without a validated BPM"); + } // namespace reasampler::instrument::note diff --git a/tests/test_musical_division.cpp b/tests/test_musical_division.cpp index 449bb53..f9cc6b9 100644 --- a/tests/test_musical_division.cpp +++ b/tests/test_musical_division.cpp @@ -111,16 +111,19 @@ static void testNamedExamples() { // --- Picker order ------------------------------------------------------------- static void testPickerOrderIsShortestFirst() { - // Straight lengths ascend across rungs; within a rung the order is straight, dotted, - // triplet (so the index is not itself sorted by duration — only the rungs are). + // Straight lengths ascend across rungs; within EVERY rung (not just rung 0) the order is + // straight, dotted, triplet — so the index is not itself sorted by duration. for (int rung = 1; rung < kRungCount; ++rung) { const double prev = divisionBeats(divisionAt((rung - 1) * kModifierCount)); const double here = divisionBeats(divisionAt(rung * kModifierCount)); CHECK(here > prev); } - CHECK(divisionAt(0) == makeDivision(kMinQuarterExponent, DivisionModifier::Straight)); - CHECK(divisionAt(1) == makeDivision(kMinQuarterExponent, DivisionModifier::Dotted)); - CHECK(divisionAt(2) == makeDivision(kMinQuarterExponent, DivisionModifier::Triplet)); + for (int rung = 0; rung < kRungCount; ++rung) { + const int e = kMinQuarterExponent + rung; + CHECK(divisionAt(rung * kModifierCount + 0) == makeDivision(e, DivisionModifier::Straight)); + CHECK(divisionAt(rung * kModifierCount + 1) == makeDivision(e, DivisionModifier::Dotted)); + CHECK(divisionAt(rung * kModifierCount + 2) == makeDivision(e, DivisionModifier::Triplet)); + } } static void testIndexRoundTripsOverTheWholeSet() { @@ -151,6 +154,16 @@ static void testOffLadderExponentClampsToTheNearestRung() { CHECK(almostEqual(divisionBeats(corrupt), 256.0)); } +static void testEqualityNormalizesOffLadderExponentsLikeEveryOtherReader() { + // divisionBeats/divisionIndex/divisionLabel all re-clamp through makeDivision; equality + // must too, or a corrupt persisted value reads as a spurious diff on every reload. + Division corrupt; + corrupt.quarterExponent = 120; + corrupt.modifier = DivisionModifier::Straight; + CHECK(corrupt == makeDivision(kMaxQuarterExponent, DivisionModifier::Straight)); + CHECK(corrupt != makeDivision(kMaxQuarterExponent, DivisionModifier::Dotted)); +} + static void testOutOfRangeIndexClampsIntoTheSet() { CHECK(divisionAt(-1) == divisionAt(0)); CHECK(divisionAt(kDivisionCount) == divisionAt(kDivisionCount - 1)); @@ -169,6 +182,7 @@ int main() { testEverySetMemberIsDistinct(); testOffLadderExponentClampsToTheNearestRung(); + testEqualityNormalizesOffLadderExponentsLikeEveryOtherReader(); testOutOfRangeIndexClampsIntoTheSet(); if (g_fail == 0) std::printf("musical_division: all tests passed\n"); diff --git a/tests/test_note_program.cpp b/tests/test_note_program.cpp index 03a5afa..92ca6c8 100644 --- a/tests/test_note_program.cpp +++ b/tests/test_note_program.cpp @@ -1,10 +1,11 @@ // Standalone tests for reasampler::instrument::note::note_program — no VST3, no REAPER, no // framework. Same fast assert loop as the sibling pure tests. // -// Covers: velocity clamping; the ms/beats denomination seam and its round-trip; anchoring -// (start to note-on, end to note-off); the resolved window against hand-computed values; -// every division resolving to its duration in seconds; proportionality across two tempos; -// record equality and copy round-trip. +// Covers: velocity clamping; the ms/beats denomination seam and its round-trip, including a +// corrupt denomination byte; anchoring (start to note-on, end to note-off); the resolved +// window against hand-computed values and its windowCollapsed flag; every division resolving +// to its duration in seconds; proportionality across two tempos; record equality and copy +// round-trip; editing an offset via its non-stored view (withMsView/withBeatsView). #include "../src/core/instrument/note/note_program.h" @@ -323,7 +324,67 @@ static void testDefaultRecordIsAQuarterNoteWithNoOffsets() { CHECK(almostEqual(r.noteOffSeconds, 0.5)); CHECK(almostEqual(r.captureStartSeconds, 0.0)); CHECK(almostEqual(r.captureEndSeconds, 0.5)); - CHECK(r.velocity >= Velocity::kMin && r.velocity <= Velocity::kMax); + CHECK(r.velocity == 100); // NoteProgram{}'s default Velocity, documented in note_program.h +} + +// --- Corrupt denomination byte -------------------------------------------------- + +static void testOutOfRangeDenominationReadsAsMillisecondsEverywhere() { + // A denomination byte outside {Milliseconds, Beats} is well-defined but unnamed; all + // three readers must default it to the same interpretation or a popup and a bake can + // report different instants for one record. + OffsetAmount corrupt; + corrupt.magnitude = 250.0; + corrupt.denomination = static_cast(7); + const Tempo t = at(120.0); + CHECK(almostEqual(offsetMs(corrupt, t), 250.0)); + CHECK(almostEqual(offsetSeconds(corrupt, t), 0.25)); + CHECK(almostEqual(offsetBeats(corrupt, t), t.msToBeats(250.0))); +} + +// --- windowCollapsed ------------------------------------------------------------- + +static void testWindowCollapsedFlagsAnInvertedWindow() { + const Tempo t = at(120.0); + const ResolvedNote inverted = resolveNote( + program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(0.0), + offsetFromMs(-5000.0), 100), + t); + CHECK(inverted.windowCollapsed); + + const ResolvedNote normal = resolveNote( + program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(-20.0), + offsetFromMs(500.0), 100), + t); + CHECK(!normal.windowCollapsed); +} + +// --- Editing via the non-stored view --------------------------------------------- + +static void testWithMsViewPreservesTheStoredDenomination() { + const Tempo t = at(120.0); // one beat is 500 ms + const OffsetAmount msOffset = offsetFromMs(10.0); + const OffsetAmount editedMs = withMsView(msOffset, 40.0, t); + CHECK(editedMs.denomination == Denomination::Milliseconds); + CHECK(almostEqual(editedMs.magnitude, 40.0)); + + const OffsetAmount beatsOffset = offsetFromBeats(1.0); + const OffsetAmount editedBeats = withMsView(beatsOffset, 250.0, t); + CHECK(editedBeats.denomination == Denomination::Beats); // stays beats-denominated + CHECK(almostEqual(offsetMs(editedBeats, t), 250.0, 1e-6)); // but reads back as 250 ms +} + +static void testWithBeatsViewPreservesTheStoredDenomination() { + const Tempo t = at(120.0); // one beat is 500 ms + const OffsetAmount beatsOffset = offsetFromBeats(0.5); + const OffsetAmount editedBeats = withBeatsView(beatsOffset, 2.0, t); + CHECK(editedBeats.denomination == Denomination::Beats); + CHECK(almostEqual(editedBeats.magnitude, 2.0)); + + const OffsetAmount msOffset = offsetFromMs(100.0); + const OffsetAmount editedMs = withBeatsView(msOffset, 1.0, t); + CHECK(editedMs.denomination == Denomination::Milliseconds); // stays ms-denominated + CHECK(almostEqual(offsetBeats(editedMs, t), 1.0)); // but reads back as 1 beat } int main() { @@ -353,6 +414,13 @@ int main() { testRedenominatedRecordDescribesTheSameWindow(); testDefaultRecordIsAQuarterNoteWithNoOffsets(); + testOutOfRangeDenominationReadsAsMillisecondsEverywhere(); + + testWindowCollapsedFlagsAnInvertedWindow(); + + testWithMsViewPreservesTheStoredDenomination(); + testWithBeatsViewPreservesTheStoredDenomination(); + if (g_fail == 0) std::printf("note_program: all tests passed\n"); else std::printf("note_program: %d FAILED\n", g_fail); return g_fail == 0 ? 0 : 1; diff --git a/tests/test_tempo.cpp b/tests/test_tempo.cpp index dcfa06c..28c2a0a 100644 --- a/tests/test_tempo.cpp +++ b/tests/test_tempo.cpp @@ -41,6 +41,9 @@ static void testUnusableBpmIsRejected() { CHECK(!Tempo::fromBpm(-120.0).has_value()); CHECK(!Tempo::fromBpm(std::numeric_limits::quiet_NaN()).has_value()); CHECK(!Tempo::fromBpm(std::numeric_limits::infinity()).has_value()); + // Finite, positive, subnormal — but 60/bpm overflows to +inf, which turns + // beatsToSeconds(0) into NaN downstream if let through. + CHECK(!Tempo::fromBpm(1e-310).has_value()); } // --- Conversions --------------------------------------------------------------- From a80eb76c1fb3ea7a9973750a99a93db6dc251f47 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 30 Jul 2026 20:37:12 -0400 Subject: [PATCH 3/4] note: close every value type's domain at construction, so resolveNote is finite for every constructible input Division and OffsetAmount get single normalizing doors and private constructors; fromBpm validates by running the conversions rather than their reciprocal. Readers drop their re-clamps and default labels. --- src/core/instrument/note/CLAUDE.md | 26 ++- src/core/instrument/note/CMakeLists.txt | 3 +- src/core/instrument/note/musical_division.cpp | 32 ++-- src/core/instrument/note/musical_division.h | 41 ++++- src/core/instrument/note/note_program.cpp | 65 ++++--- src/core/instrument/note/note_program.h | 45 ++++- src/core/instrument/note/tempo.cpp | 14 +- src/core/instrument/note/tempo.h | 10 +- tests/test_musical_division.cpp | 68 ++++++-- tests/test_note_program.cpp | 165 +++++++++++++++--- tests/test_tempo.cpp | 48 ++++- 11 files changed, 398 insertions(+), 119 deletions(-) diff --git a/src/core/instrument/note/CLAUDE.md b/src/core/instrument/note/CLAUDE.md index 9a00025..bc66354 100644 --- a/src/core/instrument/note/CLAUDE.md +++ b/src/core/instrument/note/CLAUDE.md @@ -31,6 +31,19 @@ diverge: the capture-signal popup that edits it and the bake that renders it. the ladder ever gained a rung or a modifier. - **An offset stores the denomination it was entered in** — see `OffsetAmount` in `note_program.h` for why. +- **Every value type establishes its domain at construction, and nothing downstream can + fail.** `Tempo::fromBpm` rejects, alone, because an unusable BPM has no nearest usable one + to fall to. `Division`, `OffsetAmount`, and `Velocity` clamp, because an off-ladder rung, + an unrepresentable magnitude, and an out-of-range velocity each do. Each has exactly one + door (`makeDivision`, `offsetOf`, `Velocity::of`) and a private constructor behind it, so + an out-of-domain value cannot be held, only passed in. That is what lets every reader + branch without a fallback, equality compare fields raw, and `resolveNote` return finite + times for every constructible input with no failure path and no validity flag. +- **The module will not tell a caller a record is junk, because a junk record cannot exist + here.** Corruption is only visible where raw bytes are: a codec sees both the bytes it + read and the value construction produced, and reporting the difference is the codec's job. + Do not add a validity flag to `NoteProgram` or `ResolvedNote` to carry that signal upward + — `windowCollapsed` describes a legal program, and is not the seed of an error channel. - **Does not carry a MIDI note number.** `NoteProgram` describes timing and velocity only; render pitch is deferred to a later additive field (Ξ-W2) rather than assumed to live here. @@ -41,9 +54,11 @@ diverge: the capture-signal popup that edits it and the bake that renders it. exponent of its length in quarter notes, -4..8), each straight, dotted (x3/2), or triplet (x2/3); the 39-entry picker order; and the `"1/8."` / `"1/4t"` label notation. Beats only — see `musical_division.h` for why it links no tempo. -- `tempo` — a validated project tempo plus every beats <-> seconds <-> ms conversion. - Construction (`Tempo::fromBpm`) is the only place a bad BPM is rejected, which is what - lets each conversion be total and every downstream resolver be failure-free. +- `tempo` — a validated project tempo plus every beats <-> seconds <-> ms conversion, and + `kMaxConvertibleMagnitude`, the beats-or-ms ceiling the whole directory caps its domains + to. `fromBpm` validates by running the extreme conversions rather than by testing the + `60/bpm` reciprocal they start from — that reciprocal stays finite well past the point the + multiply after it overflows. - `note_program` — `Velocity` (clamped 1..127), the denominated `OffsetAmount` and its unit toggle, the anchored `StartOffset` / `EndOffset`, the `NoteProgram` record, and `resolveNote`. @@ -64,3 +79,8 @@ diverge: the capture-signal popup that edits it and the bake that renders it. Both are legal; `resolveNote` only refuses to invert the window. - **ms <-> beats round-trips are lossless to double precision, not bit-identical.** The conversion is a multiply/divide pair; compare with an epsilon. +- **Editing the ms field of a beats-stored offset stores beats, and the ms readout will then + move with the tempo.** `withMsView` keeps the stored denomination on purpose, so typing 250 + into the ms field of a beats offset stores 0.5 beats at 120 BPM. That is the intended + semantic, but it is a UI-visible surprise worth a word in the popup: `redenominate` — the + unit toggle — is the only thing that changes which denomination is stored. diff --git a/src/core/instrument/note/CMakeLists.txt b/src/core/instrument/note/CMakeLists.txt index dbffa83..f7c5540 100644 --- a/src/core/instrument/note/CMakeLists.txt +++ b/src/core/instrument/note/CMakeLists.txt @@ -6,8 +6,7 @@ reasampler_test(musical_division LINK musical_division) reasampler_pure_library(tempo SOURCES tempo.cpp) reasampler_test(tempo LINK tempo) -# note_program links exactly these two: it composes the ladder and the tempo and nothing -# else (see note_program.h). +# note_program links exactly these two: it composes the ladder and the tempo and nothing else. reasampler_pure_library(note_program SOURCES note_program.cpp LINK PUBLIC musical_division tempo) diff --git a/src/core/instrument/note/musical_division.cpp b/src/core/instrument/note/musical_division.cpp index 443d0ac..c1babad 100644 --- a/src/core/instrument/note/musical_division.cpp +++ b/src/core/instrument/note/musical_division.cpp @@ -21,28 +21,26 @@ int clampExponent(int quarterExponent) { return (std::max)(kMinQuarterExponent, (std::min)(kMaxQuarterExponent, quarterExponent)); } +// The underlying type is unsigned, so an out-of-enum byte can only be too large. +DivisionModifier clampModifier(DivisionModifier m) { + return static_cast(m) < kModifierCount ? m : DivisionModifier::Straight; +} + } // namespace bool operator==(Division a, Division b) { - // Normalize both sides through makeDivision first: a persisted off-ladder exponent must - // compare equal to its clamped form, the same as every other reader in this file. - const Division la = makeDivision(a.quarterExponent, a.modifier); - const Division lb = makeDivision(b.quarterExponent, b.modifier); - return la.quarterExponent == lb.quarterExponent && la.modifier == lb.modifier; + return a.quarterExponent() == b.quarterExponent() && a.modifier() == b.modifier(); } bool operator!=(Division a, Division b) { return !(a == b); } Division makeDivision(int quarterExponent, DivisionModifier modifier) { - Division d; - d.quarterExponent = static_cast(clampExponent(quarterExponent)); - d.modifier = modifier; - return d; + return Division(static_cast(clampExponent(quarterExponent)), + clampModifier(modifier)); } double divisionBeats(Division d) { - const Division legal = makeDivision(d.quarterExponent, d.modifier); - return std::ldexp(1.0, legal.quarterExponent) * modifierFactor(legal.modifier); + return std::ldexp(1.0, d.quarterExponent()) * modifierFactor(d.modifier()); } Division divisionAt(int index) { @@ -52,20 +50,18 @@ Division divisionAt(int index) { } int divisionIndex(Division d) { - const Division legal = makeDivision(d.quarterExponent, d.modifier); - return (legal.quarterExponent - kMinQuarterExponent) * kModifierCount - + static_cast(legal.modifier); + return (d.quarterExponent() - kMinQuarterExponent) * kModifierCount + + static_cast(d.modifier()); } std::string divisionLabel(Division d) { - const Division legal = makeDivision(d.quarterExponent, d.modifier); - const int e = legal.quarterExponent; + const int e = d.quarterExponent(); // Both branches meet at e == 2 ("1/1"): a division's written form is its length in // whole notes, which is 2^(e-2). std::string label = e <= 2 ? "1/" + std::to_string(1 << (2 - e)) : std::to_string(1 << (e - 2)) + "/1"; - if (legal.modifier == DivisionModifier::Dotted) label += '.'; - else if (legal.modifier == DivisionModifier::Triplet) label += 't'; + if (d.modifier() == DivisionModifier::Dotted) label += '.'; + else if (d.modifier() == DivisionModifier::Triplet) label += 't'; return label; } diff --git a/src/core/instrument/note/musical_division.h b/src/core/instrument/note/musical_division.h index afeaa0d..bef6fc8 100644 --- a/src/core/instrument/note/musical_division.h +++ b/src/core/instrument/note/musical_division.h @@ -6,6 +6,7 @@ #include #include +#include namespace reasampler::instrument::note { @@ -24,19 +25,43 @@ inline constexpr int kRungCount = kMaxQuarterExponent - kMinQuarterExponent + 1; inline constexpr int kModifierCount = 3; inline constexpr int kDivisionCount = kRungCount * kModifierCount; -struct Division { - std::int8_t quarterExponent = 0; // 1/4 - DivisionModifier modifier = DivisionModifier::Straight; +// The longest programmable note — the dotted top rung — so a caller composing this ladder +// with the tempo conversions can check the two domains against each other at compile time. +inline constexpr double kMaxDivisionBeats = (1 << kMaxQuarterExponent) * 1.5; + +class Division; + +// Off-ladder inputs clamp rather than reject: the only ways to reach one are a corrupt +// persisted record or a picker bug, and the nearest legal length beats a nonsense duration. +// An unnamed modifier byte has no nearest rung to fall to, so it takes the field's default. +Division makeDivision(int quarterExponent, DivisionModifier modifier); + +// In-domain by construction — `makeDivision` is the only door and it clamps BOTH fields, so +// every reader below trusts the stored pair instead of re-clamping it, and equality compares +// the two fields raw without disagreeing with any of them. +class Division { +public: + Division() = default; // 1/4 straight + + constexpr std::int8_t quarterExponent() const { return quarterExponent_; } + constexpr DivisionModifier modifier() const { return modifier_; } + +private: + Division(std::int8_t quarterExponent, DivisionModifier modifier) + : quarterExponent_(quarterExponent), modifier_(modifier) {} + friend Division makeDivision(int quarterExponent, DivisionModifier modifier); + + std::int8_t quarterExponent_ = 0; + DivisionModifier modifier_ = DivisionModifier::Straight; }; +static_assert(!std::is_constructible_v, + "makeDivision must be the only way to give a Division a rung"); + bool operator==(Division a, Division b); bool operator!=(Division a, Division b); -// Off-ladder exponents clamp rather than reject: the only ways to reach one are a corrupt -// persisted record or a picker bug, and the nearest legal length beats a nonsense duration. -Division makeDivision(int quarterExponent, DivisionModifier modifier); - -// Length in beats (quarter notes). Always > 0. +// Length in beats (quarter notes). Always > 0, and never above kMaxDivisionBeats. double divisionBeats(Division d); // Picker order: shortest rung first, straight/dotted/triplet within each rung. Index is diff --git a/src/core/instrument/note/note_program.cpp b/src/core/instrument/note/note_program.cpp index 683a356..42f187a 100644 --- a/src/core/instrument/note/note_program.cpp +++ b/src/core/instrument/note/note_program.cpp @@ -3,6 +3,7 @@ #include "core/instrument/note/note_program.h" #include +#include namespace reasampler::instrument::note { @@ -15,56 +16,69 @@ Velocity Velocity::of(int value) { bool operator==(Velocity a, Velocity b) { return a.value() == b.value(); } bool operator==(OffsetAmount a, OffsetAmount b) { - return a.magnitude == b.magnitude && a.denomination == b.denomination; + return a.magnitude() == b.magnitude() && a.denomination() == b.denomination(); } bool operator!=(OffsetAmount a, OffsetAmount b) { return !(a == b); } -OffsetAmount offsetFromMs(double ms) { return {ms, Denomination::Milliseconds}; } +OffsetAmount offsetOf(double magnitude, Denomination denomination) { + const double bounded = + std::isnan(magnitude) ? 0.0 + : (std::max)(-kMaxConvertibleMagnitude, + (std::min)(kMaxConvertibleMagnitude, magnitude)); + const bool named = denomination == Denomination::Milliseconds + || denomination == Denomination::Beats; + return OffsetAmount(bounded, named ? denomination : Denomination::Milliseconds); +} -OffsetAmount offsetFromBeats(double beats) { return {beats, Denomination::Beats}; } +OffsetAmount offsetFromMs(double ms) { return offsetOf(ms, Denomination::Milliseconds); } -// All three readers switch on Denomination with the same default (Milliseconds, the -// struct's own default value) so a corrupt persisted record reads identically everywhere — -// a popup and a bake must never disagree on an out-of-range denomination byte. +OffsetAmount offsetFromBeats(double beats) { return offsetOf(beats, Denomination::Beats); } + +// Milliseconds is pinned AFTER the switch rather than by a `default:` inside it, so the +// switch stays exhaustive over the enum and a third denomination trips switch-exhaustiveness +// diagnostics here instead of silently resolving as ms in all three. Those diagnostics are +// off at this project's warning level, so read it as a signpost — the tests are the gate. double offsetMs(OffsetAmount amount, Tempo tempo) { - switch (amount.denomination) { - case Denomination::Beats: return tempo.beatsToMs(amount.magnitude); - case Denomination::Milliseconds: - default: return amount.magnitude; + switch (amount.denomination()) { + case Denomination::Beats: return tempo.beatsToMs(amount.magnitude()); + case Denomination::Milliseconds: break; } + return amount.magnitude(); } double offsetBeats(OffsetAmount amount, Tempo tempo) { - switch (amount.denomination) { - case Denomination::Beats: return amount.magnitude; - case Denomination::Milliseconds: - default: return tempo.msToBeats(amount.magnitude); + switch (amount.denomination()) { + case Denomination::Beats: return amount.magnitude(); + case Denomination::Milliseconds: break; } + return tempo.msToBeats(amount.magnitude()); } double offsetSeconds(OffsetAmount amount, Tempo tempo) { - switch (amount.denomination) { - case Denomination::Beats: return tempo.beatsToSeconds(amount.magnitude); - case Denomination::Milliseconds: - default: return msToSeconds(amount.magnitude); + switch (amount.denomination()) { + case Denomination::Beats: return tempo.beatsToSeconds(amount.magnitude()); + case Denomination::Milliseconds: break; } + return msToSeconds(amount.magnitude()); } OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo) { - if (amount.denomination == to) return amount; - return to == Denomination::Beats ? offsetFromBeats(offsetBeats(amount, tempo)) - : offsetFromMs(offsetMs(amount, tempo)); + // Route the requested target through the same door a stored denomination goes through, + // so an out-of-enum target lands where a corrupt stored one does. + const Denomination target = offsetOf(0.0, to).denomination(); + if (amount.denomination() == target) return amount; + return target == Denomination::Beats ? offsetFromBeats(offsetBeats(amount, tempo)) + : offsetFromMs(offsetMs(amount, tempo)); } OffsetAmount withMsView(OffsetAmount amount, double ms, Tempo tempo) { - return amount.denomination == Denomination::Milliseconds - ? offsetFromMs(ms) - : offsetFromBeats(tempo.msToBeats(ms)); + return amount.denomination() == Denomination::Beats ? offsetFromBeats(tempo.msToBeats(ms)) + : offsetFromMs(ms); } OffsetAmount withBeatsView(OffsetAmount amount, double beats, Tempo tempo) { - return amount.denomination == Denomination::Beats + return amount.denomination() == Denomination::Beats ? offsetFromBeats(beats) : offsetFromMs(tempo.beatsToMs(beats)); } @@ -83,7 +97,6 @@ ResolvedNote resolveNote(const NoteProgram& program, Tempo tempo) { const double rawEndSeconds = out.noteOffSeconds + offsetSeconds(program.end.amount(), tempo); // An inverted window has no meaning to a renderer, so a far-negative end offset yields a // zero-length capture the caller can reject rather than a negative one it cannot. - // windowCollapsed distinguishes that from a genuinely zero-length program. out.windowCollapsed = rawEndSeconds < out.captureStartSeconds; out.captureEndSeconds = (std::max)(rawEndSeconds, out.captureStartSeconds); out.velocity = program.velocity.value(); diff --git a/src/core/instrument/note/note_program.h b/src/core/instrument/note/note_program.h index d0973fa..148f53b 100644 --- a/src/core/instrument/note/note_program.h +++ b/src/core/instrument/note/note_program.h @@ -13,6 +13,11 @@ namespace reasampler::instrument::note { +// The ladder and the offsets both feed the tempo conversions, so both must sit inside the +// domain fromBpm validates — checked here because this is the one file that composes them. +static_assert(kMaxDivisionBeats <= kMaxConvertibleMagnitude, + "the note-length ladder must stay inside the tempo conversions' domain"); + class Velocity { public: static constexpr int kMin = 1; // 0 is note-off in MIDI; a programmed note must sound @@ -21,7 +26,7 @@ public: Velocity() = default; static Velocity of(int value); // clamped into [kMin, kMax] - std::uint8_t value() const { return value_; } + constexpr std::uint8_t value() const { return value_; } private: std::uint8_t value_ = 100; @@ -31,20 +36,42 @@ bool operator==(Velocity a, Velocity b); enum class Denomination : std::uint8_t { Milliseconds, Beats }; +class OffsetAmount; + +// The one door. Normalizes both fields so nothing downstream has to: a magnitude past +// +/-kMaxConvertibleMagnitude clamps to it, a NaN magnitude — which names no value to clamp +// toward — becomes zero, and a denomination outside the enum becomes Milliseconds, the +// field's own default. A corrupt persisted record therefore resolves to a plausible offset +// rather than an unrepresentable one, and no two readers can disagree about which. +OffsetAmount offsetOf(double magnitude, Denomination denomination); +OffsetAmount offsetFromMs(double ms); +OffsetAmount offsetFromBeats(double beats); + // One magnitude, in the denomination it was entered in; the other view is derived on demand // and never stored. Which one was entered is itself the intent: a beats offset must follow a // tempo change and a ms offset must hold still, and only a stored denomination says which. -struct OffsetAmount { - double magnitude = 0.0; - Denomination denomination = Denomination::Milliseconds; +class OffsetAmount { +public: + OffsetAmount() = default; + + constexpr double magnitude() const { return magnitude_; } + constexpr Denomination denomination() const { return denomination_; } + +private: + OffsetAmount(double magnitude, Denomination denomination) + : magnitude_(magnitude), denomination_(denomination) {} + friend OffsetAmount offsetOf(double magnitude, Denomination denomination); + + double magnitude_ = 0.0; + Denomination denomination_ = Denomination::Milliseconds; }; +static_assert(!std::is_constructible_v, + "offsetOf must be the only way to give an OffsetAmount a value"); + bool operator==(OffsetAmount a, OffsetAmount b); bool operator!=(OffsetAmount a, OffsetAmount b); -OffsetAmount offsetFromMs(double ms); -OffsetAmount offsetFromBeats(double beats); - double offsetMs(OffsetAmount amount, Tempo tempo); double offsetBeats(OffsetAmount amount, Tempo tempo); double offsetSeconds(OffsetAmount amount, Tempo tempo); @@ -100,7 +127,7 @@ struct ResolvedNote { double noteOffSeconds = 0.0; // == the note's sounding length, note-on being 0 double captureStartSeconds = 0.0; // negative when the capture opens before the note double captureEndSeconds = 0.0; - std::uint8_t velocity = 100; // resolveNote always overwrites this; matches Velocity's own default + std::uint8_t velocity = Velocity{}.value(); // resolveNote always overwrites this // True when the programmed end offset inverted the window and resolveNote collapsed it // to zero length instead — lets a popup explain an empty window rather than just show one. bool windowCollapsed = false; @@ -108,6 +135,8 @@ struct ResolvedNote { double captureLengthSeconds() const { return captureEndSeconds - captureStartSeconds; } }; +// Total: every field of the result is finite for every constructible program and tempo, +// which is why there is no failure path here. See this directory's CLAUDE.md. ResolvedNote resolveNote(const NoteProgram& program, Tempo tempo); } // namespace reasampler::instrument::note diff --git a/src/core/instrument/note/tempo.cpp b/src/core/instrument/note/tempo.cpp index bb7e5e4..bf660bc 100644 --- a/src/core/instrument/note/tempo.cpp +++ b/src/core/instrument/note/tempo.cpp @@ -11,11 +11,15 @@ constexpr double kSecondsPerMinute = 60.0; std::optional Tempo::fromBpm(double beatsPerMinute) { if (!std::isfinite(beatsPerMinute) || beatsPerMinute <= 0.0) return std::nullopt; - // A subnormal BPM is finite and positive but overflows 60/bpm to +inf, which then turns - // any beatsToSeconds(0) into NaN downstream — reject it here so every conversion below - // stays total. - if (!std::isfinite(kSecondsPerMinute / beatsPerMinute)) return std::nullopt; - return Tempo(beatsPerMinute); + // Guard by running the conversions, not by testing the 60/bpm reciprocal they start + // from: that reciprocal stays finite for BPMs whose beatsToMs has already overflowed, + // because the conversions scale it by up to kMaxConvertibleMagnitude. Both directions + // are checked — one overflows at an absurdly slow tempo, the other at an absurdly fast + // one. Calling them here is what keeps the guard from drifting away from what they do. + const Tempo candidate(beatsPerMinute); + if (!std::isfinite(candidate.beatsToMs(kMaxConvertibleMagnitude))) return std::nullopt; + if (!std::isfinite(candidate.msToBeats(kMaxConvertibleMagnitude))) return std::nullopt; + return candidate; } double Tempo::secondsPerBeat() const { return kSecondsPerMinute / bpm_; } diff --git a/src/core/instrument/note/tempo.h b/src/core/instrument/note/tempo.h index 7c2f8a6..a768398 100644 --- a/src/core/instrument/note/tempo.h +++ b/src/core/instrument/note/tempo.h @@ -16,10 +16,16 @@ inline constexpr double kMsPerSecond = 1000.0; constexpr double msToSeconds(double ms) { return ms / kMsPerSecond; } constexpr double secondsToMs(double seconds) { return seconds * kMsPerSecond; } +// The largest magnitude, in beats or in milliseconds, the conversions below are required to +// keep finite. `fromBpm` validates against it and every caller caps its own domain to it, so +// the two halves of the totality claim meet at one number. Astronomically above anything +// musical — a billion milliseconds is eleven days — so nothing real is excluded. +inline constexpr double kMaxConvertibleMagnitude = 1e9; + class Tempo { public: - // The only place a bad BPM is rejected, which is what lets every conversion below be - // total — no resolver downstream needs a failure path. + // Rejects rather than clamps, alone among this module's doors: an unusable BPM has no + // nearest usable one to fall to. See this directory's CLAUDE.md for the rule. static std::optional fromBpm(double beatsPerMinute); double bpm() const { return bpm_; } diff --git a/tests/test_musical_division.cpp b/tests/test_musical_division.cpp index f9cc6b9..0997af2 100644 --- a/tests/test_musical_division.cpp +++ b/tests/test_musical_division.cpp @@ -3,7 +3,8 @@ // // Covers: the beat length of all 39 divisions against a literal rung table (NOT the module's // own exponent formula); the 1/64 and 64/1 extremes; the four named example divisions; the -// label notation; picker order and index round-trip; off-ladder clamping. +// label notation; picker order and index round-trip; off-ladder clamping of BOTH persisted +// fields, measured through the readers rather than by comparing two clamped values. #include "../src/core/instrument/note/musical_division.h" @@ -76,9 +77,14 @@ static void testExtremes() { 0.0625)); CHECK(almostEqual(divisionBeats(makeDivision(kMaxQuarterExponent, DivisionModifier::Straight)), 256.0)); - // The dotted 64/1 is the single longest programmable note. + // The dotted 64/1 is the single longest programmable note, and kMaxDivisionBeats — which + // note_program checks the tempo conversions' domain against — must name exactly it. CHECK(almostEqual(divisionBeats(makeDivision(kMaxQuarterExponent, DivisionModifier::Dotted)), 384.0)); + CHECK(almostEqual(kMaxDivisionBeats, 384.0)); + for (int i = 0; i < kDivisionCount; ++i) { + CHECK(divisionBeats(divisionAt(i)) <= kMaxDivisionBeats); + } // The 1/64 triplet is the shortest. CHECK(almostEqual(divisionBeats(makeDivision(kMinQuarterExponent, DivisionModifier::Triplet)), 0.0625 * 2.0 / 3.0)); @@ -144,24 +150,47 @@ static void testEverySetMemberIsDistinct() { // --- Clamping ----------------------------------------------------------------- static void testOffLadderExponentClampsToTheNearestRung() { - CHECK(makeDivision(-99, DivisionModifier::Straight) - == makeDivision(kMinQuarterExponent, DivisionModifier::Straight)); - CHECK(makeDivision(99, DivisionModifier::Triplet) - == makeDivision(kMaxQuarterExponent, DivisionModifier::Triplet)); - // A record carrying an off-ladder exponent still resolves to a real length. - Division corrupt; - corrupt.quarterExponent = 120; - CHECK(almostEqual(divisionBeats(corrupt), 256.0)); + // Asserted through the readers, never by comparing two clamped Divisions: a clamp that + // collapsed every exponent to one rung would make Division-to-Division comparisons agree + // with their own mistake. + CHECK(almostEqual(divisionBeats(makeDivision(-99, DivisionModifier::Straight)), 0.0625)); + CHECK(divisionLabel(makeDivision(-99, DivisionModifier::Straight)) == "1/64"); + CHECK(divisionIndex(makeDivision(-99, DivisionModifier::Straight)) == 0); + + CHECK(almostEqual(divisionBeats(makeDivision(99, DivisionModifier::Triplet)), + 256.0 * 2.0 / 3.0)); + CHECK(divisionLabel(makeDivision(99, DivisionModifier::Triplet)) == "64/1t"); + CHECK(divisionIndex(makeDivision(99, DivisionModifier::Triplet)) == kDivisionCount - 1); + + // The exponent that only a corrupt persisted record could carry still names a real rung. + CHECK(almostEqual(divisionBeats(makeDivision(120, DivisionModifier::Straight)), 256.0)); } -static void testEqualityNormalizesOffLadderExponentsLikeEveryOtherReader() { - // divisionBeats/divisionIndex/divisionLabel all re-clamp through makeDivision; equality - // must too, or a corrupt persisted value reads as a spurious diff on every reload. - Division corrupt; - corrupt.quarterExponent = 120; - corrupt.modifier = DivisionModifier::Straight; - CHECK(corrupt == makeDivision(kMaxQuarterExponent, DivisionModifier::Straight)); - CHECK(corrupt != makeDivision(kMaxQuarterExponent, DivisionModifier::Dotted)); +static void testUnnamedModifierClampsToStraight() { + // The other half of the persisted pair. Neither divisionBeats nor divisionLabel can see + // an unnamed modifier — both already fall through to the straight case — so the clamp is + // measured where it does show: the picker index and equality. + const DivisionModifier junk = static_cast(7); + CHECK(divisionIndex(makeDivision(0, junk)) + == divisionIndex(makeDivision(0, DivisionModifier::Straight))); + CHECK(divisionLabel(makeDivision(0, junk)) == "1/4"); // and no junk reaches the readout + CHECK(makeDivision(0, junk) == makeDivision(0, DivisionModifier::Straight)); + CHECK(makeDivision(0, junk) != makeDivision(0, DivisionModifier::Dotted)); +} + +static void testEveryConstructibleDivisionIndexesIntoThePickerSet() { + // divisionIndex is what a picker array is subscripted with, so an out-of-set index is an + // overrun in the caller. Both corrupt fields at once is the worst case: 12*3+7 without a + // modifier clamp. + const int exponents[] = {-9000, -99, kMinQuarterExponent, 0, kMaxQuarterExponent, 120, 9000}; + for (int e : exponents) { + for (int m = 0; m < 260; ++m) { + const Division d = makeDivision(e, static_cast(m)); + const int index = divisionIndex(d); + CHECK(index >= 0 && index < kDivisionCount); + CHECK(divisionAt(index) == d); // and the picker round-trips it back + } + } } static void testOutOfRangeIndexClampsIntoTheSet() { @@ -182,7 +211,8 @@ int main() { testEverySetMemberIsDistinct(); testOffLadderExponentClampsToTheNearestRung(); - testEqualityNormalizesOffLadderExponentsLikeEveryOtherReader(); + testUnnamedModifierClampsToStraight(); + testEveryConstructibleDivisionIndexesIntoThePickerSet(); testOutOfRangeIndexClampsIntoTheSet(); if (g_fail == 0) std::printf("musical_division: all tests passed\n"); diff --git a/tests/test_note_program.cpp b/tests/test_note_program.cpp index 92ca6c8..9d1c3f9 100644 --- a/tests/test_note_program.cpp +++ b/tests/test_note_program.cpp @@ -1,16 +1,20 @@ // Standalone tests for reasampler::instrument::note::note_program — no VST3, no REAPER, no // framework. Same fast assert loop as the sibling pure tests. // -// Covers: velocity clamping; the ms/beats denomination seam and its round-trip, including a -// corrupt denomination byte; anchoring (start to note-on, end to note-off); the resolved -// window against hand-computed values and its windowCollapsed flag; every division resolving -// to its duration in seconds; proportionality across two tempos; record equality and copy -// round-trip; editing an offset via its non-stored view (withMsView/withBeatsView). +// Covers: velocity clamping; the ms/beats denomination seam and its round-trip; anchoring +// (start to note-on, end to note-off); the resolved window against hand-computed values and +// its windowCollapsed flag, including the zero-length window the flag exists to distinguish; +// every division resolving to its duration in seconds; proportionality across two tempos; +// record equality and copy round-trip; editing an offset via its non-stored view +// (withMsView/withBeatsView); what `offsetOf` does to a corrupt magnitude or denomination, +// asserted through EVERY function that branches on one; and the module's headline claim — +// that resolveNote returns finite times for every constructible input. #include "../src/core/instrument/note/note_program.h" #include #include +#include using namespace reasampler::instrument::note; @@ -101,9 +105,9 @@ static void testRedenominationRoundTripsLosslessly() { const OffsetAmount original = offsetFromMs(ms); const OffsetAmount there = redenominate(original, Denomination::Beats, t); const OffsetAmount back = redenominate(there, Denomination::Milliseconds, t); - CHECK(there.denomination == Denomination::Beats); - CHECK(back.denomination == Denomination::Milliseconds); - CHECK(almostEqual(back.magnitude, ms, 1e-9 + 1e-9 * std::fabs(ms))); + CHECK(there.denomination() == Denomination::Beats); + CHECK(back.denomination() == Denomination::Milliseconds); + CHECK(almostEqual(back.magnitude(), ms, 1e-9 + 1e-9 * std::fabs(ms))); // Re-denominating never moves the instant it names. CHECK(almostEqual(offsetSeconds(there, t), offsetSeconds(original, t))); } @@ -112,7 +116,7 @@ static void testRedenominationRoundTripsLosslessly() { const OffsetAmount back = redenominate(redenominate(original, Denomination::Milliseconds, t), Denomination::Beats, t); - CHECK(almostEqual(back.magnitude, beats, 1e-9 + 1e-9 * std::fabs(beats))); + CHECK(almostEqual(back.magnitude(), beats, 1e-9 + 1e-9 * std::fabs(beats))); } } } @@ -327,19 +331,71 @@ static void testDefaultRecordIsAQuarterNoteWithNoOffsets() { CHECK(r.velocity == 100); // NoteProgram{}'s default Velocity, documented in note_program.h } -// --- Corrupt denomination byte -------------------------------------------------- +// --- The door: what a corrupt persisted field becomes ---------------------------- -static void testOutOfRangeDenominationReadsAsMillisecondsEverywhere() { - // A denomination byte outside {Milliseconds, Beats} is well-defined but unnamed; all - // three readers must default it to the same interpretation or a popup and a bake can - // report different instants for one record. - OffsetAmount corrupt; - corrupt.magnitude = 250.0; - corrupt.denomination = static_cast(7); +static void testUnnamedDenominationBecomesMilliseconds() { + // A denomination byte outside {Milliseconds, Beats} is well-defined but unnamed. The + // door pins it, so it is not merely that the readers agree — the value they read from + // is already Milliseconds by the time any of them sees it. + const OffsetAmount corrupt = offsetOf(250.0, static_cast(7)); + CHECK(corrupt.denomination() == Denomination::Milliseconds); const Tempo t = at(120.0); CHECK(almostEqual(offsetMs(corrupt, t), 250.0)); CHECK(almostEqual(offsetSeconds(corrupt, t), 0.25)); - CHECK(almostEqual(offsetBeats(corrupt, t), t.msToBeats(250.0))); + CHECK(almostEqual(offsetBeats(corrupt, t), 0.5)); +} + +static void testEveryDenominationBranchingFunctionAgreesWithThePin() { + // The pin is worth nothing if one branching function disagrees with it: an editor that + // flipped a corrupt record to beats would silently change whether it follows the tempo, + // and an equality that saw the raw byte would report a diff on every reload. All six. + const Tempo t = at(120.0); // one beat is 500 ms + const OffsetAmount corrupt = offsetOf(250.0, static_cast(7)); + const OffsetAmount asMs = offsetFromMs(250.0); + + CHECK(corrupt == asMs); // offsetMs / offsetBeats / offsetSeconds covered above + CHECK(!(corrupt != asMs)); + CHECK(redenominate(corrupt, Denomination::Milliseconds, t) == corrupt); + CHECK(redenominate(corrupt, Denomination::Beats, t).denomination() == Denomination::Beats); + CHECK(withMsView(corrupt, 40.0, t) == withMsView(asMs, 40.0, t)); + CHECK(withMsView(corrupt, 40.0, t).denomination() == Denomination::Milliseconds); + CHECK(withBeatsView(corrupt, 1.0, t) == withBeatsView(asMs, 1.0, t)); + CHECK(withBeatsView(corrupt, 1.0, t).denomination() == Denomination::Milliseconds); + CHECK(almostEqual(withBeatsView(corrupt, 1.0, t).magnitude(), 500.0, 1e-6)); + + // An unnamed TARGET denomination pins the same way an unnamed stored one does. + CHECK(redenominate(offsetFromBeats(1.0), static_cast(7), t) + == offsetFromMs(500.0)); +} + +static void testCorruptMagnitudeIsBoundedAtTheDoor() { + const double inf = std::numeric_limits::infinity(); + const double nan = std::numeric_limits::quiet_NaN(); + // NaN names no value to clamp toward, so it takes the field's own default; an infinity + // does have a nearest representable magnitude, so it clamps like any other overshoot. + CHECK(almostEqual(offsetFromMs(nan).magnitude(), 0.0)); + CHECK(almostEqual(offsetFromBeats(nan).magnitude(), 0.0)); + CHECK(almostEqual(offsetFromMs(inf).magnitude(), kMaxConvertibleMagnitude)); + CHECK(almostEqual(offsetFromBeats(-inf).magnitude(), -kMaxConvertibleMagnitude)); + CHECK(almostEqual(offsetFromMs(1e300).magnitude(), kMaxConvertibleMagnitude)); + // A NaN offset is a value, not a hole: it equals itself, so it is not a spurious diff. + CHECK(offsetFromMs(nan) == offsetFromMs(0.0)); + // Anything inside the domain passes through untouched. + CHECK(almostEqual(offsetFromMs(-12345.678).magnitude(), -12345.678)); +} + +static void testANanMagnitudeCannotReachTheResolvedWindow() { + // The witness the door exists for: at an unremarkable tempo, a NaN magnitude used to + // make captureStart, rawEnd and captureEnd all NaN, and windowCollapsed read false. + const Tempo t = at(120.0); + const double nan = std::numeric_limits::quiet_NaN(); + const ResolvedNote r = resolveNote( + program(makeDivision(0, DivisionModifier::Straight), offsetOf(nan, Denomination::Beats), + offsetOf(nan, Denomination::Milliseconds), 100), + t); + CHECK(almostEqual(r.captureStartSeconds, 0.0)); + CHECK(almostEqual(r.captureEndSeconds, 0.5)); + CHECK(!r.windowCollapsed); } // --- windowCollapsed ------------------------------------------------------------- @@ -359,18 +415,71 @@ static void testWindowCollapsedFlagsAnInvertedWindow() { CHECK(!normal.windowCollapsed); } +static void testWindowCollapsedIsFalseForAGenuinelyZeroLengthWindow() { + // The discrimination the flag exists for. A 1/4 at 120 BPM is 500 ms, so an end offset + // of -500 ms puts the raw end EXACTLY on the start: zero-length, but programmed that way + // rather than collapsed, and a popup must be able to tell the two apart. + const Tempo t = at(120.0); + const ResolvedNote r = resolveNote(program(makeDivision(0, DivisionModifier::Straight), + offsetFromMs(0.0), offsetFromMs(-500.0), 100), + t); + CHECK(almostEqual(r.captureLengthSeconds(), 0.0)); + CHECK(!r.windowCollapsed); + // One millisecond further in is the same zero length, but collapsed. + const ResolvedNote collapsed = resolveNote( + program(makeDivision(0, DivisionModifier::Straight), offsetFromMs(0.0), + offsetFromMs(-501.0), 100), + t); + CHECK(almostEqual(collapsed.captureLengthSeconds(), 0.0)); + CHECK(collapsed.windowCollapsed); +} + +// --- Totality -------------------------------------------------------------------- + +static void testResolveNoteIsFiniteForEveryConstructibleInput() { + // The claim that lets resolveNote have no failure path, swept rather than argued: every + // division, both denominations, the magnitude extremes the door admits plus the garbage + // it normalizes, across tempos from rejected-subnormal to rejected-astronomical. + const double inf = std::numeric_limits::infinity(); + const double nan = std::numeric_limits::quiet_NaN(); + const double magnitudes[] = {-inf, -kMaxConvertibleMagnitude, -1e300, 0.0, 1e300, + kMaxConvertibleMagnitude, inf, nan}; + int accepted = 0, rejected = 0; + for (double bpm : {1e-320, 1e-306, 1e-200, 1e-6, 0.5, 120.0, 1e6, 1e100, 1e308}) { + const std::optional tempo = Tempo::fromBpm(bpm); + if (!tempo) { ++rejected; continue; } + ++accepted; + for (int i = 0; i < kDivisionCount; ++i) { + for (double m : magnitudes) { + for (Denomination d : {Denomination::Milliseconds, Denomination::Beats}) { + const ResolvedNote r = resolveNote( + program(divisionAt(i), offsetOf(m, d), offsetOf(-m, d), 100), *tempo); + CHECK(std::isfinite(r.noteOffSeconds)); + CHECK(std::isfinite(r.captureStartSeconds)); + CHECK(std::isfinite(r.captureEndSeconds)); + CHECK(std::isfinite(r.captureLengthSeconds())); + CHECK(r.captureLengthSeconds() >= 0.0); + } + } + } + } + // Neither half of the tempo sweep may be empty, or the loop above proves nothing. + CHECK(accepted > 0); + CHECK(rejected > 0); +} + // --- Editing via the non-stored view --------------------------------------------- static void testWithMsViewPreservesTheStoredDenomination() { const Tempo t = at(120.0); // one beat is 500 ms const OffsetAmount msOffset = offsetFromMs(10.0); const OffsetAmount editedMs = withMsView(msOffset, 40.0, t); - CHECK(editedMs.denomination == Denomination::Milliseconds); - CHECK(almostEqual(editedMs.magnitude, 40.0)); + CHECK(editedMs.denomination() == Denomination::Milliseconds); + CHECK(almostEqual(editedMs.magnitude(), 40.0)); const OffsetAmount beatsOffset = offsetFromBeats(1.0); const OffsetAmount editedBeats = withMsView(beatsOffset, 250.0, t); - CHECK(editedBeats.denomination == Denomination::Beats); // stays beats-denominated + CHECK(editedBeats.denomination() == Denomination::Beats); // stays beats-denominated CHECK(almostEqual(offsetMs(editedBeats, t), 250.0, 1e-6)); // but reads back as 250 ms } @@ -378,12 +487,12 @@ static void testWithBeatsViewPreservesTheStoredDenomination() { const Tempo t = at(120.0); // one beat is 500 ms const OffsetAmount beatsOffset = offsetFromBeats(0.5); const OffsetAmount editedBeats = withBeatsView(beatsOffset, 2.0, t); - CHECK(editedBeats.denomination == Denomination::Beats); - CHECK(almostEqual(editedBeats.magnitude, 2.0)); + CHECK(editedBeats.denomination() == Denomination::Beats); + CHECK(almostEqual(editedBeats.magnitude(), 2.0)); const OffsetAmount msOffset = offsetFromMs(100.0); const OffsetAmount editedMs = withBeatsView(msOffset, 1.0, t); - CHECK(editedMs.denomination == Denomination::Milliseconds); // stays ms-denominated + CHECK(editedMs.denomination() == Denomination::Milliseconds); // stays ms-denominated CHECK(almostEqual(offsetBeats(editedMs, t), 1.0)); // but reads back as 1 beat } @@ -414,13 +523,19 @@ int main() { testRedenominatedRecordDescribesTheSameWindow(); testDefaultRecordIsAQuarterNoteWithNoOffsets(); - testOutOfRangeDenominationReadsAsMillisecondsEverywhere(); + testUnnamedDenominationBecomesMilliseconds(); + testEveryDenominationBranchingFunctionAgreesWithThePin(); + testCorruptMagnitudeIsBoundedAtTheDoor(); + testANanMagnitudeCannotReachTheResolvedWindow(); testWindowCollapsedFlagsAnInvertedWindow(); + testWindowCollapsedIsFalseForAGenuinelyZeroLengthWindow(); testWithMsViewPreservesTheStoredDenomination(); testWithBeatsViewPreservesTheStoredDenomination(); + testResolveNoteIsFiniteForEveryConstructibleInput(); + if (g_fail == 0) std::printf("note_program: all tests passed\n"); else std::printf("note_program: %d FAILED\n", g_fail); return g_fail == 0 ? 0 : 1; diff --git a/tests/test_tempo.cpp b/tests/test_tempo.cpp index 28c2a0a..85de36c 100644 --- a/tests/test_tempo.cpp +++ b/tests/test_tempo.cpp @@ -2,9 +2,10 @@ // framework. Same fast assert loop as the sibling pure tests. // // Covers: BPM validation (the only rejection point, which is what makes the conversions -// total); seconds-per-beat at several tempos; beats<->seconds and beats<->ms round-trips -// across tempos and signs; the proportionality between two tempos, asserted as a ratio -// rather than against any fixed seconds value. +// total) including the tempos whose reciprocal is finite but whose conversions overflow; +// seconds-per-beat at several tempos; beats<->seconds and beats<->ms round-trips across +// tempos and signs; the proportionality between two tempos, asserted as a ratio rather than +// against any fixed seconds value. #include "../src/core/instrument/note/tempo.h" @@ -46,6 +47,44 @@ static void testUnusableBpmIsRejected() { CHECK(!Tempo::fromBpm(1e-310).has_value()); } +static void testBpmWhoseReciprocalIsFineButWhoseConversionsOverflowIsRejected() { + // The gap a guard on 60/bpm alone leaves open: the reciprocal is an ordinary finite + // double, and the multiply that follows it is what blows up. + CHECK(std::isfinite(60.0 / 1e-306)); + CHECK(!Tempo::fromBpm(1e-306).has_value()); + // The fast end fails in the other direction — the divide, not the multiply. + CHECK(!Tempo::fromBpm(1e308).has_value()); +} + +static void testTheGuardAdmitsEveryRealTempoAndFarBeyond() { + // The guard is structural, not musical, so it must not have narrowed onto the range of + // tempos anyone would type. The extremes here are orders of magnitude past that. + for (double bpm : {1e-200, 1e-6, 0.001, 1.0, 20.0, 120.0, 240.0, 960.0, 1e6, 1e100}) { + CHECK(Tempo::fromBpm(bpm).has_value()); + } +} + +static void testEveryAcceptedTempoConvertsTheWholeDomainFinitely() { + // What the guard is FOR: past it, no conversion of a magnitude the module admits can + // reach inf or NaN, in either unit or either direction. + int accepted = 0, rejected = 0; + for (double bpm : {1e-320, 1e-306, 1e-300, 1e-100, 1e-6, 0.5, 120.0, 1e6, 1e100, 1e250, + 1e308}) { + const std::optional t = Tempo::fromBpm(bpm); + if (!t) { ++rejected; continue; } + ++accepted; + for (double m : {-kMaxConvertibleMagnitude, -1.0, 0.0, 1.0, kMaxConvertibleMagnitude}) { + CHECK(std::isfinite(t->beatsToSeconds(m))); + CHECK(std::isfinite(t->beatsToMs(m))); + CHECK(std::isfinite(t->msToBeats(m))); + CHECK(std::isfinite(t->secondsToBeats(msToSeconds(m)))); + } + } + // Neither half of the sweep may be empty, or the loop above proves nothing. + CHECK(accepted > 0); + CHECK(rejected > 0); +} + // --- Conversions --------------------------------------------------------------- static void testSecondsPerBeatFollowsBpm() { @@ -121,6 +160,9 @@ static void testMillisecondsAreTempoFree() { int main() { testUsableBpmIsAccepted(); testUnusableBpmIsRejected(); + testBpmWhoseReciprocalIsFineButWhoseConversionsOverflowIsRejected(); + testTheGuardAdmitsEveryRealTempoAndFarBeyond(); + testEveryAcceptedTempoConvertsTheWholeDomainFinitely(); testSecondsPerBeatFollowsBpm(); testBeatsToSecondsAtAKnownTempo(); From dddecc57347950b93890acbad4ab8c24dfc8cca2 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 30 Jul 2026 21:03:02 -0400 Subject: [PATCH 4/4] =?UTF-8?q?note:=20close=20out=20the=20model=20?= =?UTF-8?q?=E2=80=94=20correct=20an=20inert=20mutation=20claim,=20retag=20?= =?UTF-8?q?four=20non-discriminating=20assertions,=20assert=20Tempo's=20cl?= =?UTF-8?q?osure,=20fix=20three=20doc/test=20accuracy=20gaps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No behavior change; verification-record corrections and one static_assert. --- src/core/instrument/note/CLAUDE.md | 21 +++++++++++++-------- src/core/instrument/note/note_program.cpp | 6 ++++-- src/core/instrument/note/tempo.h | 2 ++ tests/test_musical_division.cpp | 5 +++++ tests/test_note_program.cpp | 20 ++++++++++++++++++-- 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/core/instrument/note/CLAUDE.md b/src/core/instrument/note/CLAUDE.md index bc66354..895e8f5 100644 --- a/src/core/instrument/note/CLAUDE.md +++ b/src/core/instrument/note/CLAUDE.md @@ -31,14 +31,16 @@ diverge: the capture-signal popup that edits it and the bake that renders it. the ladder ever gained a rung or a modifier. - **An offset stores the denomination it was entered in** — see `OffsetAmount` in `note_program.h` for why. -- **Every value type establishes its domain at construction, and nothing downstream can - fail.** `Tempo::fromBpm` rejects, alone, because an unusable BPM has no nearest usable one - to fall to. `Division`, `OffsetAmount`, and `Velocity` clamp, because an off-ladder rung, - an unrepresentable magnitude, and an out-of-range velocity each do. Each has exactly one - door (`makeDivision`, `offsetOf`, `Velocity::of`) and a private constructor behind it, so - an out-of-domain value cannot be held, only passed in. That is what lets every reader - branch without a fallback, equality compare fields raw, and `resolveNote` return finite - times for every constructible input with no failure path and no validity flag. +- **Every value type establishes its domain at construction, so every field `resolveNote` + returns is finite for every constructible program and tempo.** `Tempo::fromBpm` rejects, + alone, because an unusable BPM has no nearest usable one to fall to. `Division`, + `OffsetAmount`, and `Velocity` clamp, because an off-ladder rung, an unrepresentable + magnitude, and an out-of-range velocity each do. Each has exactly one door (`makeDivision`, + `offsetOf`, `Velocity::of`); `Division` and `OffsetAmount` block any other path with a + private value constructor, `Velocity` with a private member that only `of()` writes — + either way an out-of-domain value cannot be held, only passed in. That is what lets every + reader branch without a fallback, equality compare fields raw, and `resolveNote` return + finite times for every constructible input with no failure path and no validity flag. - **The module will not tell a caller a record is junk, because a junk record cannot exist here.** Corruption is only visible where raw bytes are: a codec sees both the bytes it read and the value construction produced, and reporting the difference is the codec's job. @@ -79,6 +81,9 @@ diverge: the capture-signal popup that edits it and the bake that renders it. Both are legal; `resolveNote` only refuses to invert the window. - **ms <-> beats round-trips are lossless to double precision, not bit-identical.** The conversion is a multiply/divide pair; compare with an epsilon. +- **`Division` and `OffsetAmount` are trivially copyable, so a `memcpy` of a wire record + bypasses every door.** Decode field-by-field through `makeDivision`/`offsetOf` (the pattern + `src/core/wire/bytes.h` already uses) instead — never `memcpy` raw bytes into either type. - **Editing the ms field of a beats-stored offset stores beats, and the ms readout will then move with the tempo.** `withMsView` keeps the stored denomination on purpose, so typing 250 into the ms field of a beats offset stores 0.5 beats at 120 BPM. That is the intended diff --git a/src/core/instrument/note/note_program.cpp b/src/core/instrument/note/note_program.cpp index 42f187a..f2b1d73 100644 --- a/src/core/instrument/note/note_program.cpp +++ b/src/core/instrument/note/note_program.cpp @@ -64,8 +64,10 @@ double offsetSeconds(OffsetAmount amount, Tempo tempo) { } OffsetAmount redenominate(OffsetAmount amount, Denomination to, Tempo tempo) { - // Route the requested target through the same door a stored denomination goes through, - // so an out-of-enum target lands where a corrupt stored one does. + // Defense-in-depth, not a discriminating guard: the branch below already treats any + // non-Beats target as Milliseconds, so an unnamed `to` resolves the same way whether or + // not it is routed through offsetOf first. Kept because a future third denomination + // would make this the one place that still pins it. const Denomination target = offsetOf(0.0, to).denomination(); if (amount.denomination() == target) return amount; return target == Denomination::Beats ? offsetFromBeats(offsetBeats(amount, tempo)) diff --git a/src/core/instrument/note/tempo.h b/src/core/instrument/note/tempo.h index a768398..c05508e 100644 --- a/src/core/instrument/note/tempo.h +++ b/src/core/instrument/note/tempo.h @@ -43,5 +43,7 @@ private: static_assert(!std::is_default_constructible_v, "Tempo must not be constructible without a validated BPM"); +static_assert(!std::is_constructible_v, + "fromBpm must be the only way to give a Tempo a value"); } // namespace reasampler::instrument::note diff --git a/tests/test_musical_division.cpp b/tests/test_musical_division.cpp index 0997af2..e620ecf 100644 --- a/tests/test_musical_division.cpp +++ b/tests/test_musical_division.cpp @@ -175,6 +175,9 @@ static void testUnnamedModifierClampsToStraight() { == divisionIndex(makeDivision(0, DivisionModifier::Straight))); CHECK(divisionLabel(makeDivision(0, junk)) == "1/4"); // and no junk reaches the readout CHECK(makeDivision(0, junk) == makeDivision(0, DivisionModifier::Straight)); + // Measured (clamp removed from clampModifier): still passes. junk(7) != Dotted's stored + // modifier either way, clamped or raw — this discriminates a degenerate operator== that + // ignores the modifier field, not the clamp itself. CHECK(makeDivision(0, junk) != makeDivision(0, DivisionModifier::Dotted)); } @@ -184,6 +187,8 @@ static void testEveryConstructibleDivisionIndexesIntoThePickerSet() { // modifier clamp. const int exponents[] = {-9000, -99, kMinQuarterExponent, 0, kMaxQuarterExponent, 120, 9000}; for (int e : exponents) { + // 260, not 256: m=256..259 wrap modulo uint8_t back to 0..3, re-covering the four + // lowest bytes rather than reaching any byte 256 alone couldn't already reach. for (int m = 0; m < 260; ++m) { const Division d = makeDivision(e, static_cast(m)); const int index = divisionIndex(d); diff --git a/tests/test_note_program.cpp b/tests/test_note_program.cpp index 9d1c3f9..4bf496a 100644 --- a/tests/test_note_program.cpp +++ b/tests/test_note_program.cpp @@ -356,14 +356,27 @@ static void testEveryDenominationBranchingFunctionAgreesWithThePin() { CHECK(corrupt == asMs); // offsetMs / offsetBeats / offsetSeconds covered above CHECK(!(corrupt != asMs)); CHECK(redenominate(corrupt, Denomination::Milliseconds, t) == corrupt); + // Measured (mutate offsetOf to a pass-through): still passes. offsetFromBeats always + // tags its result Beats, so this holds regardless of whether corrupt was pinned — it + // does not discriminate the pin. CHECK(redenominate(corrupt, Denomination::Beats, t).denomination() == Denomination::Beats); + // Reported measured (withMsView reverted to its pre-domain-closure form): still passes. + // corrupt already equals asMs by this point, and withMsView is a pure function of its + // argument, so this line cannot discriminate anything withMsView-specific — it is a + // restatement of the equality above. CHECK(withMsView(corrupt, 40.0, t) == withMsView(asMs, 40.0, t)); CHECK(withMsView(corrupt, 40.0, t).denomination() == Denomination::Milliseconds); + // Measured (mutate offsetOf to a pass-through): still passes. withBeatsView only branches + // on `== Beats`; any non-Beats value — pinned or raw corrupt — takes the same ms-based + // else branch, so this does not discriminate the pin either. CHECK(withBeatsView(corrupt, 1.0, t) == withBeatsView(asMs, 1.0, t)); CHECK(withBeatsView(corrupt, 1.0, t).denomination() == Denomination::Milliseconds); CHECK(almostEqual(withBeatsView(corrupt, 1.0, t).magnitude(), 500.0, 1e-6)); - // An unnamed TARGET denomination pins the same way an unnamed stored one does. + // Reported measured (offsetOf(0.0, to) replaced with `= to;`): still passes, for the + // same reason as above — `target == Beats` is false whether target is pinned or raw, so + // this always takes the ms branch and cannot discriminate the door (see the comment on + // that line in note_program.cpp). CHECK(redenominate(offsetFromBeats(1.0), static_cast(7), t) == offsetFromMs(500.0)); } @@ -445,7 +458,10 @@ static void testResolveNoteIsFiniteForEveryConstructibleInput() { const double magnitudes[] = {-inf, -kMaxConvertibleMagnitude, -1e300, 0.0, 1e300, kMaxConvertibleMagnitude, inf, nan}; int accepted = 0, rejected = 0; - for (double bpm : {1e-320, 1e-306, 1e-200, 1e-6, 0.5, 120.0, 1e6, 1e100, 1e308}) { + // 1e-294/1e-295 bracket the accept/reject edge (measured ~3.34e-295) so the sweep + // actually approaches it rather than jumping past it by ~95 orders of magnitude. + for (double bpm : + {1e-320, 1e-306, 1e-295, 1e-294, 1e-200, 1e-6, 0.5, 120.0, 1e6, 1e100, 1e308}) { const std::optional tempo = Tempo::fromBpm(bpm); if (!tempo) { ++rejected; continue; } ++accepted;