Files
reasampler/docs/COMPLETED.md
T

316 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# COMPLETED.md — ReaSampler landed milestones
Completed milestone entries removed from `PLAN.md`. Each entry preserves its
original Goal, Verify, and checklist points with boxes marked done.
This file holds the current (1.x) cycle's landed milestones only. For all
pre-1.0 (version-0) history, see `docs/ARCHIVE.md`.
### Comment-reduction pass (tree-wide, twelve parallel tracks)
Cut source comment volume tree-wide: 209 files changed, net **6,493** lines.
Comment-only lines went from 15,073 to ~8,671, a **~42% cut** — before the
pass, 37% of all source lines were comment-only. **Zero code drift**, verified
across all 209 files by comparing comment-stripped hashes; the sole
intentional exception (Daniel-approved) is two user-facing error strings in
`src/shell/capture/capture.cpp` that lost internal milestone IDs (`M8`, `M3`,
`M7+`). Build clean, 61/61 ctest pass. Code review surfaced 2 Major + 8 Minor
findings — all content cut that should have survived — and all ten were
remediated and re-gated before merge. Driver: Daniel's instruction — *"Brief
concise engineering comments. A little why, and maybe context, never WHAT."*
### CMake build-system split (ad-hoc, Daniel's request)
Split the 1423-line root `CMakeLists.txt` into a 91-line root plus 18 per-directory
`CMakeLists.txt` files under `src/`, with two shared declaration helpers
(`reasampler_pure_library`, `reasampler_test`) factored into a new
`cmake/reasampler_targets.cmake`. The root now keeps only repo-global concerns:
version/channel single-source-of-truth, `configure_file`, vendor path vars,
`LICE_SRC`, `enable_testing()`, and the `add_subdirectory` calls. Comments throughout
were rewritten to the project's comment conventions — phase/wave/ticket IDs removed,
module semantics already owned by `src/**/CLAUDE.md` deleted, load-bearing build
facts kept.
Also fixed two duplicate-object-code defects surfaced by the split: 18 `core/`
translation units were previously compiled directly into the `reaper_reasampler`
module *while also* being linked in as static libraries — those 18 source entries
were removed from the module's source list and 3 missing link edges
(`view_tree`, `guid_diff`, `lane_keys`) added so every `core/` TU now enters through
exactly one static-library link edge. A dead `bridge_marshal` link edge was also
dropped from `reaper_reasampler`, and two inaccurate comments in
`src/app/CMakeLists.txt` were corrected.
No `.cpp`, `.h`, `tests/`, or `vendor/` file was touched. Behaviour is unchanged and
was verified mechanically: same 130 targets, same 65 tests all passing,
`reaper_reasampler.dll` byte-identical at 3,477,504 bytes, both channels
(stable/beta) building to the same artifact names and locations as before.
### Θ-W1-T1 — zone-retirement
ReaSampler 9000's zone-mapping system is retired: one loaded capture, one parameter
set, playing across the full keyboard repitched from root with key-tracking — no zones,
no per-zone divergence, no keymap of captures. The dedicated zone-editing face and its
authoring affordances (add/delete zone, per-zone parameter panel, Low/High/Root zone
legend) are gone; the root note survives as a first-class parameter. `sampler_core`
split along the note-routing/per-voice-render responsibility seam (no virtual `tick()`
on the per-voice path), and the Sample face split into chrome/waveform/decks bands with
a shared band-stack allocator, discharging the wave's two structural deliverables.
Migration adopts a saved multi-zone instance's first zone; single-zone instances lift
losslessly.
**Deviations from spec:**
- The key-range open question (**[propose]**) is answered outright rather than left
open: no key-range concept survives at all — `KeyZone`/`lowNote`/`highNote` are gone
from the engine, the write format, and the strip. A low/high pair remains re-addable
later as two ordinary parameters. Θ-W2-T3 consumes this decision.
- **"Which zone is first" (**[verify]**) confirmed:** `PerformanceMap::zones` was an
ordered vector and `Keymap::resolve` was first-match-in-order, so index 0 was the
audible zone. Migration adopts index 0, and that zone's `sampleId` supersedes the
envelope's stored `selectionId`.
- The control row (root strip, preview, velocity knob, curve button, Mono|Stereo) moved
into the chrome band, directly under the title rather than above the deck —
user-visible and deliberate; it's what makes Θ-W2's band-disjointness real.
- Loading a capture now clears the three capture-anchored overrides (root, loop span,
start frame) while keeping the shaping parameters — strictly less destructive than the
previous whole-zone drop.
- The embed strip became a read-only readout; it lost its click handler since with no
zones there is nothing to select.
- **Migration side-effect:** a previously-zoned instance with implicit channel mode and
a stereo capture persisted as Mono will reopen as **Stereo**. It converges on the
documented rule and is reachable only for old blobs, but "sounds identical" was an
acceptance bar, so it's a real deviation.
- `sampler_core.{h,cpp}`'s 956-line documented hot-path exception is retired, not
relocated — the tree now carries no over-ceiling exception at all.
- `note_entry` was deleted as dead code (its only consumer was the removed zone editor).
- **Still unverified:** a real pre-change project reopening through REAPER's `setState`
has not been exercised in the DAW; migration is proven only in the pure domain against
hand-laid legacy bytes.
### Θ-W1-T2 — capture-handoff-bugs
Fixed both extension-side capture-handoff defects: drag-out now delivers the capture's
audio at the drop target every time (previously intermittent, retry-fixable); dropping a
capture onto an FX container now loads the instrument with the capture, matching the
FX-button drop path.
**Deviations from spec:**
- Item 5's root cause was three defects, not one: non-atomic COM refcounts racing a drop
target's background copy; an unverified assumption that REAPER had already called
`OleInitialize` on the calling thread; and a teardown-before-payload-check ordering bug
that let an unresolvable payload consume the gesture.
- Item 6's fix rests on an unconfirmed hypothesis — that a bare `instantiate = -1` left
placement to REAPER's ambient FX-chain insert point, which a container-focused chain
window moves. It is now pinned to an explicit top-level position; nobody could confirm
the mechanism without the DAW.
- The opportunistic rider was partly taken: `src/ingest.{h,cpp}` re-homed to
`src/shell/actions/`. `ext_keys.h` was declined (most consumers sit in another track's
exclusive surface); `resource.h` was declined (it's a build input paired with
`src/resource.rc` and the SWELL resgen step).
- **Neither acceptance criterion has actually been met yet.** Item 5's gate is
explicitly a soak ("a single pass is not a gate") and item 6 needs a live container
drop; both require REAPER and are outstanding. The code is merged; the acceptance
gates are not closed.
### Θ-W1-T3 — filter-dsp-port
Lands the per-voice resonant filter as a standalone pure module
(`core/instrument/engine/filter/`, five files: `filter_params`, `filter_coeffs`,
`filter_morph`, `filter_saturate`, `voice_filter`) — concrete `VoiceFilter` type, no
vtable, no allocation in `process()`. **No call site**; Θ-W2-T1 wires it into the voice
path.
**Deviations from spec — the spec itself changed mid-flight, headline first:**
- **The Cortex-M4 biquad port was superseded entirely by a TPT/SVF topology, Daniel's
call.** Measurement found the firmware's high-pass resonance feedback tap vestigial:
its stated rationale was inverted (the HP numerator approaches 1 as cutoff falls, not
zero — it's the LP numerator that collapses), it *reduced* HP resonance everywhere it
ran, and it carried unwanted sample-rate and input-level dependence. It was a Q15
fixed-point workaround for a ~17-bit cancellation float32 doesn't suffer. Daniel's
ruling on the resulting level-dependent resonance bloom: *"was a feature on the
hardware (one knob colorful HP for master FX), wrong choice for this approach."*
- **Two discrete modes (HP/LP) became a continuous morph, with two selectable morph
laws** — HP→BP→LP (default) and HP→notch→LP (Oberheim SEM) — chosen at `prepare()`
via a `MorphLaw` enum on `FilterSettings`. Zero per-sample cost, verified by diffing
emitted assembly (byte-identical between laws).
- **A configurable drive stage was added:** an in-loop soft limiter on the band-pass
integrator state, normalized 0..1, with a radial dial planned for Θ-W2. Drive at 0 is
bit-exact linear.
- **`Biquad1PoleLP` was struck (Daniel's call) and never ported.**
- **Q spans the full 0.110 with √2 at the control centre**, replacing the firmware's
0.707-floored mapping — as originally specified.
- **No reference sample rate exists anywhere in the module** — rate reaches the DSP
only via `g = tan(π·fc/sr)`. An interim fix that anchored a feedback tap to a
`1/48000` constant was superseded by the rewrite.
- **The rewrite fixed a float32 conditioning defect the biquad carried:** Direct Form I
measured 27% peak error at 20 Hz / 192 kHz; TPT measures +0.034%.
- **Still open, deliberately:** drive's maximum depth (4.0, set by measurement — at 64
the resonant peak inverted below passband) and the absence of makeup gain both await
an ear pass against the real dial.
- **The module has no call site** — integration is Θ-W2-T1, which also carries three
recorded decisions of its own: the envelope-order choice (drive is level-dependent, so
pre- vs post-envelope placement is sound-defining), the drive dial's calibration, and
the fact that drive authority varies ~11 dB across the morph sweep.
### Θ-W2-T1 — filter-voice-path
Wires the pure filter module into ReaSampler 9000's per-voice signal path as a new fixed
processing point between the pitch envelope and the amp stage, gives it its own
knob-deck group, and relays the deck row in signal-flow order (pitch → filter → amp).
Each `Voice` owns its own `VoiceFilter` and a second `AdsrEnvelope` instance — per-voice,
never shared — and neither adds allocation or virtual dispatch to the per-sample path.
Parameters — morph position, cutoff, Q, drive, mod amount (bipolar ±100%, targeting
cutoff), velocity and key-tracking modulation, then AHDSR — live in the one parameter
set; `FilterParams` stores the filter module's own `FilterSettings` by value rather than
a parallel copy of the normalized positions. The filter is off by default and bit-exact
off — a project saved before the change reopens sounding identical, pinned by a
bit-equality test. `ComponentState`'s params payload moves v8 → v9, appending the filter
tail; a v8 blob is a strict prefix and lifts to the off/neutral filter default, with
non-finite filter fields falling back to neutral, pinned by a golden byte-literal
fixture. Deck composition was extracted into a new pure module
`core/instrument/ui/deck_groups`, with the pitch → filter → amp row order pinned by
test; the filter's AHDSR ships as its own `FILTER ENV` group sibling to `FILTER`,
mirroring the existing `PITCH` / `PITCH ENV` split, and a morph-law toggle (`Band` |
`Notch`) ships as the FILTER group's row toggle.
**Deviations from spec:**
- **An initial mod-quantizer was added, then rejected and removed.** A
`kFilterModSteps = 2048` step gate on the coefficient re-solve stair-stepped the
corner (~5.8 cents per step); Daniel rejected it. Replaced by a cutoff-only re-solve
(`VoiceFilter::setCutoffNorm`) that re-derives only `g = tan(π·fc/sr)` — Q's parabola,
the morph's cos/sin, and the folded mix are all cutoff-independent and stay cached
from `prepare()`; `filter_params` hoists its constant logs. Measured at 48 kHz,
Release, net of the sweep generator: kernel alone 2.8 ns/frame, full `prepare()` 56.9
ns, cutoff-only re-solve 15.5 ns — about 1.2% of a core for 16 continuously-swept
voices. The corner now sweeps continuously rather than stair-stepping.
- **The editor floor was raised to 840×620**, which is also its new default size, up
from a 560×460 floor at which the grown deck wrapped to four rows and pushed
`FILTER ENV` / `AMP ENVELOPE` / `VOICE` / `MASTER` off-screen with no scroll.
`kEditorMinWidth`/`kEditorMinHeight` now live in `sample_bands`, read by both the
shell's `checkSizeConstraint` and the opening `ViewRect`. Consequence worth
recording: a host with a saved editor rect below 840×620 is clamped up on reopen.
### Θ-W2-T2 — stereo-waveform-lanes
Delivered as specified: two stacked lanes, L above R, in stereo mode; one lane in mono;
overlays draw once at full stacked height.
**Deviations from spec:**
- **A mono source under stereo mode draws one lane**, not two — lane count keys off
`stereoMode && sourceChannels >= 2`, not channel mode alone, since a mono capture in
stereo mode is dual-mono and a second lane would be the redundant duplicate the spec
forbids. Review confirmed this is the only reading consistent with the decode path.
- The full-height overlay contract Θ-W3 and Θ-W4 consume is **type-enforced, not
merely documented**: an `OverlayArea` wrapper type that lane rects cannot satisfy.
- A single-slot per-channel PCM cache was added to the editor session, invalidating
alongside the existing PCM cache.
### Θ-W2-T3 — toolbar-and-piano-strip
Delivered as specified: one toolbar font, no zone-count label, full-width piano strip,
uniform key widths, note-name tooltips, root displayed and settable.
**Deviations from spec:**
- **The non-uniform key widths were integer quantisation, not aliasing** — the old
`keyEdgeToX` truncated an exact rational, alternating 6px/7px. Θ-W6's general
antialiasing audit inherits nothing on key *widths* as a result, though key *edges*
are still drawn unantialiased.
- **Uniform integer key widths and gap-free edge-to-edge tiling are mutually
exclusive** — 75 white keys do not divide an arbitrary width. The residue now lands
in symmetric end gutters, 37px each side at the shipped 840px default — the maximum
of a sawtooth with period 75px of window width. Daniel accepted this provisionally,
pending how it looks in REAPER.
- The whole control run moved into the toolbar row, not just preview and mono/stereo —
the full-width strip left the velocity knob and curve button nowhere else to go.
- Root drag became absolute-tracking rather than pixel-delta, since black keys
overlaying white give no single pixels-per-semitone rate.
- **Width uniformity is guaranteed in client pixels only.** Nothing in the instrument
consumes a DPI scale factor, so host-side scaling is unverified — Θ-W6 already
carries a "confirm the fix survives DPI scaling" item and now genuinely inherits it.
- A stale-hover latch was fixed across **all** drag kinds and both drag-termination
paths, wider than the strip work that surfaced it.
### Θ-W3-T1 — live-parameter-delivery
Continuous playback controls are now delivered live to sounding voices instead of being
latched at note-on. New `src/core/instrument/engine/live_params.{h,cpp}` holds a
seqlock-published `LiveValues` block owned at **processor-instance scope**, above
`LoadedInstrument`, so `live_` and `draining_` observe the same one (a drain-slot voice
tracks the knob, which is the desired behavior). `foldLive(const PlayParams&)` is the
single derivation from the value type; `PlayParams` stays a plain copyable value type.
**Daniel's two decisions, both implemented:**
- **Reload tier = Grouping B.** Continuous knobs live (filter cutoff/Q/morph/drive/mod
amount/key-track; every stage time and level on all three envelopes). Root note, loop
span, and start frame still trigger a full reload.
- **Mid-stage rule = candidate (iv), hold normalized stage position.** φ =
elapsed/duration held fixed across a duration change, then advancing at
1/newDuration — expressed over normalized position specifically so Θ-W3-T2's
per-segment curve exponent composes with it.
**Deviations from spec:**
- **Trigger's %-length and fades are NOT live** — they are baked into `SampleData` at
build, so reload is the only tier that can deliver them. Consequence: a Trigger-mode
instance gets zero live amp delivery until Θ-W3-T2 folds the fade pair into the AHD.
The five non-live exclusions (`kKeyTrack`, `kFilterVel`, `kTrigLength`,
`kTrigFadeIn`, `kTrigFadeOut`) are documented in `src/core/instrument/ui/deck_groups.h`,
now their single home.
- Open question 3 resolved as **F2 + seqlock**; open question 4 (sub-block resolution)
was not built but not foreclosed — the writer interface assumes no UI thread; open
question 5 verified — a live edit still persists, `commitLive` keeps the
`setInstrumentParams` write.
- A filter envelope only advances while its depth is non-zero (the exact-skip at
`modAmount == 0`), which is what keeps the at-rest path byte-identical.
### Ξ-W1-T1 — tracking-consolidation
Consolidates the provenance/usage territory into one system: the retired
`owned_manifest` gives way to a new `src/core/tracking/` directory holding
`origin_ledger` (the record family — `OriginRecord`/`OriginKind`, the insertion-ordered
`OriginLedger`, its JSON codec, and the `Fresh`/`Loaded`/`Unreadable`/`FutureVersion`
load classification) and `tracking_authority` (the one decision surface:
`pruneProtection` and `tiedUsageExists`). Both prune's protected set and the resample's
replace-vs-add decision are computed from one borrowed `TrackingState`, so the two
safety-critical consumers cannot drift apart. `isAbsolutePath` was hoisted out to a new
`src/core/util/relative_path.h`, shared with `bank_model`'s `Sample.relativePath`.
**Deviations from spec:**
- The deferred persisted-instance-identity fix was **not** folded in — open question 5
resolved as "restate the deferral." `docs/TODO.md` already carries the sharpened
rationale (the session-epoch candidate and its sibling-drop flaw); not duplicated here.
- `sample_usage` deliberately **stays in `core/wire`** — the consolidation is of the
*decisions*, not the codecs.
- A realtime record interrupted by a project switch strands an untracked WAV in the old
project's bank folder. Resolved as document-don't-delete (prune is the exclusive
deletion authority); `docs/TODO.md` carries the entry.
- `PruneReport` fields were renamed; a malformed ledger is now reported as a distinct
blocker with its own recovery instructions.
### Ξ-W1-T2 — note-program-model
Lands the programmed-capture-signal model as a new pure module directory,
`src/core/instrument/note/` — a fourth peer of `engine/`/`map/`/`ui/` under
`core/instrument/` — holding `musical_division` (the 1/6464/1 ladder with
dotted/triplet multipliers, the 39-entry picker order), `tempo` (validated BPM plus
every beats↔seconds↔ms conversion), and `note_program` (`Velocity`, the denominated
`OffsetAmount`, the anchored `StartOffset`/`EndOffset`, the `NoteProgram` record, and
`resolveNote`).
**Deviations / resolutions from spec:**
- Open question "negative offsets" resolved: both directions are legal and the sign is
uniform (positive is later in time); only an *inverted* window is refused, reported
via `ResolvedNote::windowCollapsed`.
- Open question "denomination seam" confirmed: note length is musical-division-only;
the ms/beats duality belongs to the offsets alone. An offset stores the denomination
it was **entered in**, deriving the other view on demand, so a beats offset follows a
tempo change and a ms offset holds still.
- Module name/location resolved as `src/core/instrument/note/` — three modules, not
one, with the layering enforced by the CMake link line.
- **Beyond spec:** every value type closes its domain at construction behind a single
normalizing door (`makeDivision`, `offsetOf`, `Tempo::fromBpm`, `Velocity::of`), with
private value constructors. Consequence: `resolveNote` needs no failure path and
`ResolvedNote` no validity flag, because every returned field is finite for every
constructible program and tempo. Junk detection is relocated to the future codec,
which sees both the bytes it read and the value construction produced. `NoteProgram`
deliberately carries no MIDI note number — render pitch is deferred to Ξ-W2 as an
additive field.