Files
reasampler/docs/COMPLETED.md
T

193 lines
12 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-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.