672 lines
44 KiB
Markdown
672 lines
44 KiB
Markdown
# 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.1–10 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.
|
||
|
||
### Θ-W3-T2 — staged-envelope-curves
|
||
|
||
Grows the envelope-overlay editor from an amp-only fixture into the shared graphical
|
||
surface for all three envelopes (amp, pitch, filter): a corner radio switch per deck
|
||
selects which envelope is overlay-active (none by default, exclusive); every sloped
|
||
stage on every envelope (Attack/Decay/Release — Hold and Sustain stay flat) gets an
|
||
editable curve exponent (0.1–10, 1.0 the linear neutral) via a paired inner knob dial
|
||
and a round mid-segment overlay knot, both resolving through the one curve law in
|
||
`src/core/util/curve_law.h`; the pitch envelope becomes AHD (Attack → Hold → Decay,
|
||
Hold a fraction of the time remaining after Attack and Decay, so A+H+D ≤ span holds by
|
||
construction, no clamp); AHDSR envelopes get a right-anchored release, dragged from
|
||
its top node with the bottom-right corner fixed; and the Trigger amp/filter
|
||
fade-in/fade-out pair is retired in favor of a Trigger AHD, consolidating what were
|
||
two staged-shape mechanisms into one — item 8's rule (pitch always AHD; amp and filter
|
||
AHDSR in Gate, AHD in Trigger) governs all three. The Trigger × Preserve end-of-sample
|
||
click is fixed at its root cause: `freezeTail()` stopping the pitch shifter's writer a
|
||
full window before the read head arrives.
|
||
|
||
**Open question resolved — per-mode stage-value state.** Gate and Trigger keep
|
||
SEPARATE stored stage values, on both the amp (`PlaySeconds::adsr` +
|
||
`PlaySeconds::trigAhd`) and the filter (`FilterSeconds::env` + `FilterSeconds::trigEnv`).
|
||
Migration forces it: an old instance carries both an AHDSR and a fade pair, and one
|
||
shared set cannot preserve both modes' prior sound. Cost: ~160 bytes of persisted
|
||
state per instance, 6 additional `DeckParam` ids.
|
||
|
||
**Deviations from spec:**
|
||
- **The migration exponent is FITTED, not neutral — Daniel's explicit ruling,
|
||
resolving a spec contradiction.** PLAN.md stated both "pre-existing instances load
|
||
at exponent 1.0" and "exponents at whatever reproduces the prior fade shape";
|
||
those conflict, and the fix resolves toward the second, since it carries the
|
||
migration guarantee. Attack lifts at **p = 0.6133**, decay at **q = 1.7437**; max
|
||
deviation from the retired equal-power (sin/cos) fade shape drops from 0.2105 to
|
||
0.0875. Every non-migrated curve still lifts to the 1.0 neutral.
|
||
- **Item 4's fix is deliberately WIDER than spec.** The spec scoped the end-of-sample
|
||
click fix to Trigger × Preserve; the landed fix is not mode-scoped, so Gate ×
|
||
Preserve × source-exhaustion also now rings out (~4 ms) where it previously
|
||
hard-cut. A held Gate note whose source runs out with no loop is cut at sustain
|
||
level, landing on the same recycled synthetic tail — scoping the fix to Trigger
|
||
alone would have knowingly left that click.
|
||
- **Migration is lossy under a sample-rate mismatch** — a documented bound, not a
|
||
bug. The retired fades were source frames; the lift divides by the project rate
|
||
while the AHD rebuilds at decode rate, so a rate mismatch shifts migrated stage
|
||
lengths by that ratio. Documented in the v10 version ladder
|
||
(`component_state_io.h`) with a test.
|
||
- **Payload version is v10.** `component_state_io.cpp` was split on the format seam
|
||
into `component_state_io.cpp` + a new `params_payload.{h,cpp}`.
|
||
- **New pure module:** `src/core/util/curve_law.h` — the one per-segment curve law
|
||
(exponent domain, normalized-position→level map, the mid-segment inverse an overlay
|
||
knot drags through, and the knob's norm↔exponent travel with an exact centre
|
||
detent). The neutral exponent is a bit-identity. Measured cost of a non-neutral
|
||
exponent: ~4.7 ns per evaluation, +224 ns/output frame worst case at 16 voices —
|
||
3.1% → 4.1% of one core at 44.1 kHz.
|
||
- **`OverlayEnv` and the overlay-selection state machine live in
|
||
`core/instrument/ui/deck_groups`**, not the shell.
|
||
- **The knot-creation gesture differs from spec.** Spec said dragging a segment
|
||
*adds* a knot; the landed behavior draws the knot unconditionally on every sloped
|
||
non-zero segment and responds to a drag within the grab radius. Daniel confirmed
|
||
this reading stands.
|
||
- **Loop markers moved from `AccentTertiary` to `AccentSecondary`** — they collided
|
||
exactly with the envelope trace (RGB delta 0) in the same overlay rect. Daniel
|
||
ruled. The palette has since settled: `AccentSecondary` is `#38A8A0` (see the
|
||
palette-rework entry below and `src/core/ui/CLAUDE.md`).
|
||
|
||
**Left open by this track, resolved later.** The envelope overlay's contrast against
|
||
the waveform (tertiary purple, measured 1.37:1, below the 3:1 indicator floor) awaited
|
||
Daniel's eye on a build; pinned as a flagged deviation in `tests/test_theme.cpp` at the
|
||
time. Resolved by the ad-hoc palette rework below (`91f71f9`/`a19d645`): the trace moved
|
||
off `AccentTertiary` onto a new `Role::OverlayTrace` (`#816AA6`), clearing the floor at
|
||
3.07:1 — the mathematical ceiling for the pairing. See the palette-rework entry below and
|
||
`src/core/ui/CLAUDE.md`.
|
||
|
||
### Ξ-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/64–64/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.
|
||
|
||
### Palette rework — accent/secondary darkening + overlay/trace role (ad-hoc, Daniel's request)
|
||
|
||
Two commits (`91f71f9`, `a19d645`) resolve the envelope-overlay contrast wart Θ-W3-T2 left
|
||
open (see above). `accent/secondary` darkened `#84D6D0` → `#38A8A0`; the keyboard strip's
|
||
spectral mid stop decoupled from `accent/secondary` into its own constant, since the
|
||
darkening had inverted the ramp's lo→mid→hi luminance ordering. A new `Role::OverlayTrace`
|
||
(`#816AA6`) was added and the envelope trace + handles repointed onto it: the trace now
|
||
measures **3.07:1** against the waveform — the mathematical ceiling for any single color
|
||
sitting between the primary accent and `bg/base` (9.41:1 apart; `sqrt(9.41) ≈ 3.068`),
|
||
`#816AA6` landing at 99.94% of that optimum. Two below-floor pairs remain deliberately
|
||
accepted — the trace inside the 20%-alpha loop-span fill (2.25:1) and against the
|
||
waveform's `line/hairline` zero-line (1.92:1) — both asserted as pinned ranges in
|
||
`tests/test_theme.cpp` so either direction of drift fails the build.
|
||
|
||
Separately: the bank panel's region title enlarged into WCAG large class via a new
|
||
`Font::RegionTitle` (19px bold); `theme.h`'s large-text thresholds were corrected (a prior
|
||
revision had them ~25% low, letting 15px semibold self-classify as Large); `compositeOver`
|
||
was added to `theme` (the composited-fill arithmetic the loop-span-fill contrast pair
|
||
depends on); and the grabbed envelope handle was repointed off hue onto a size + ring
|
||
treatment, since no two values that clear the overlay-trace ceiling differ enough to carry
|
||
a state by color alone.
|
||
|
||
Full detail — the two-neighbour contrast rule, the WCAG threshold correction, and the
|
||
accepted below-floor pairs — lives in `src/core/ui/CLAUDE.md` and
|
||
`docs/product/visual-design-language.md` §4 Direction B; not duplicated here.
|
||
|
||
### Θ-W4-T1 — gate-loop-sustain
|
||
|
||
Establishes loop points as a usable feature and makes a Gate-mode loop function as the
|
||
sustain — indefinite playback until note-off, with a crossfaded seam. The regression
|
||
half resolved as **present but unreachable, not removed**: nothing in any capture path
|
||
ever wrote `Sample::loop`, so every capture opened with `hasLoop == false`; the ghost
|
||
default parked `loopStart` at frame 0 directly under the start marker, where
|
||
`markerAtPoint`'s first-in-draw-order tie-break made the handle ungrabbable; and no
|
||
crossfade existed at all. Fixed by moving the ghost span to `defaultLoopBounds` (last
|
||
quarter of the sample, both handles clear), making a collapsed span the explicit OFF
|
||
gesture, and adding a parameterized crossfade.
|
||
|
||
New pure module `src/core/instrument/engine/loop/` (`loop_span`, its own CMake target,
|
||
its own `CLAUDE.md`, `loop_span_tests`) holds `resolveLoop`, `defaultLoopBounds`,
|
||
`maxCrossfade`, `crossfadeWeight`, `lerpSource`, `crossfadedSource`. Params payload
|
||
bumped to **v11** (`kParamsLoopVersion`), appended at the tail; slot 12 is reserved for
|
||
Θ-W4-T2.
|
||
|
||
**Open questions resolved:**
|
||
- **Crossfade units and range.** Stored in source FRAMES, not ms — deliberately against
|
||
the plan's ms lean, because `sample_map.h`'s rule keeps source-timeline quantities in
|
||
source frames and the seconds path is documented lossy under a sample-rate mismatch.
|
||
Default 0 frames (a hard seam, which is what makes the migration bar hold by
|
||
construction); range is the derived `[0, min(loopStart, loopEnd − loopStart)]`.
|
||
- **Editing surface.** The waveform markers, plus a new `markerHandleRect` top-strip
|
||
grab tab (top 10px, hit-tested before the full-height marker columns) so markers
|
||
sharing a frame stay independently grabbable — a general fix for the tie-break
|
||
defect, not a crossfade special case.
|
||
- **Crossfade shape.** Settled during implementation, not specified in the source doc:
|
||
linear, not equal-power (correlated taps one loop length apart; no transcendental on
|
||
the per-sample path), with a decorrelated full-mix/stem exception recorded in the
|
||
module's own `CLAUDE.md`.
|
||
|
||
**Deviations from spec / code review:**
|
||
- Code review found one Major: the crossfade normalizer left an avoidable residual
|
||
seam discontinuity, and the module's own `CLAUDE.md` had enshrined that limitation as
|
||
a mathematical impossibility. Remediated — `crossfadeWeight` now normalizes over
|
||
`crossfade − 1` so the last rendered frame lands exactly on the incoming tap, the
|
||
false invariant was corrected, and the seam test now asserts against the material's
|
||
natural one-frame step rather than a proportionality band. Six review minors were
|
||
also fixed.
|
||
- `voice.h` sits at ~650 lines after `lerpSource`/`crossfadedSource` moved out to
|
||
`loop_span.h` — still over the ~600-line ceiling under the standing documented
|
||
hot-path exception.
|
||
|
||
**Left open by this track, deferred to Daniel (not defects):** whether the seam sounds
|
||
smooth on real material, whether the top-strip tab is discoverable, and the LICE
|
||
rendering of the tab and crossfade fill. Also open: whether the crossfade default
|
||
should stay 0 (a smooth seam becomes opt-in).
|
||
|
||
### Θ-W4-T2 — velocity-deck-and-bipolar-curves
|
||
|
||
Gives the three velocity-curve popups (amp, pitch, filter) one home — a new deck group
|
||
labelled VELOCITY — and makes the pitch and filter transfer curves bipolar. No
|
||
velocity-curve button remains in MASTER, PITCH, or Filter. Pitch and filter curves now
|
||
run y range [−1, 1], default flat at 0, so velocity modulation of pitch and filter is off
|
||
until the user draws a curve; amp stays unipolar [0, 1] with its flat-unity default
|
||
unchanged. The domain is modelled as a `CurveDomain { Unipolar, Bipolar }` field on
|
||
`VelocityCurve`, with `curveYMin`/`curveNeutral` deriving from it; `VelocityPoint::amp`
|
||
was renamed to `value`. A velocity→pitch transfer curve is new — it did not previously
|
||
exist. Full scale is `kVelocityPitchRangeSemitones = 24.0`, now the single constant the
|
||
shell's pitch-depth control also consumes; it folds into `baseRatio_` once at note-on, so
|
||
`process()` gains no per-frame work. The preview button's text is replaced by a drawn
|
||
play triangle — `previewGlyph()` returns three vertices from the pure layer, the shell
|
||
passes them to `LICE_FillTriangle`, which was already in the build: no new dependency, no
|
||
asset. Params payload is **v12** (`kParamsVelocityVersion = 12`), appending the
|
||
velocity→pitch curve after Θ-W4-T1's loop block.
|
||
|
||
**Daniel's ruling — the depth knob stays.** The implementation initially *removed*
|
||
`FilterParams::velAmount` and the `kFilterVel` depth knob, arguing a bipolar curve is
|
||
both shape and amount. Daniel rejected that: the knob scalar AND the curve both apply.
|
||
The depth control was restored, and the filter's velocity contribution is
|
||
`velAmount × curve.eval(v)` with the curve bipolar. Consequence: with `velAmount`
|
||
surviving, the pre-v12 migration became a **pure domain re-tag** — a pre-v12 unipolar
|
||
curve's y values already sit inside [−1, +1], so `velAmount` and every knot carry
|
||
forward bit-identically, with no scaling transform and no version branch in the reader.
|
||
The earlier fold-and-rescale approach (and its degree-1-homogeneity argument, which was
|
||
only exact to within double rounding) was removed entirely.
|
||
|
||
**`kFilterVel` also crossed from non-live to live** — a user-visible contract change
|
||
beyond simple restoration. Rationale: it is a depth over a latched value, the same shape
|
||
as `kFilterKeyTrack`, live since Θ-W3; the note latches `curve.eval(velocity)` and the
|
||
depth multiply happens in `applyLive` at block boundaries, gliding through the existing
|
||
cutoff ramp at zero per-sample cost.
|
||
|
||
**Deviations from spec / code review:** Code review ran on two surfaces
|
||
(engine/persistence, UI/editor) and found one Critical plus two actionable Majors and ten
|
||
Minors, all remediated. The Critical: `editedCurve()`'s `kNone` fallback let
|
||
Esc-during-a-curve-node-drag write the pitch or filter curve — bipolar domain and all —
|
||
over the amp gain curve and persist it. Fixed on both routes (the popup close now
|
||
cancels the drag; the mutable accessor refuses `kNone`). It has **no automated
|
||
regression pin** — `src/shell/instrument/` has no test target, and the bug is shell
|
||
state-machine coupling with no pure-layer equivalent.
|
||
|
||
### Θ-W5-T1 — spline-egs
|
||
|
||
Ships a free-drawn alternative to every staged envelope: the pitch, filter, and amp EGs
|
||
can each switch Staged → Spline and have their contour drawn directly on the waveform
|
||
overlay. The one shared monotone-spline implementation
|
||
(`core/instrument/engine/velocity_curve`) gained **hard points** as a per-segment rule —
|
||
a hard point does no curve smoothing on either adjacent segment, so the natural sharp
|
||
angle stands instead of a continuous derivative — and the enhancement flows to every
|
||
consumer, including the existing velocity→amp transfer curve, with no fork.
|
||
|
||
- **Dual state, save-but-inactive.** Both the Staged and Spline state persist
|
||
simultaneously; switching modes never converts or discards the inactive one, so
|
||
Staged↔Spline round-trips losslessly. Params payload reached **v13**; v12 projects
|
||
still load.
|
||
- **Gate unavailable in Spline mode.** A Spline EG's contour always covers the full
|
||
sample length as a pure time function (the Trigger/one-shot playback model), so Gate
|
||
is not selectable while it's active.
|
||
- **Point-editing grammar converged**: left-click adds a point, right-click deletes it,
|
||
control-click toggles hard/smooth — one grammar shared by both spline consumers (the
|
||
EG overlay and the velocity-curve popup), matching the popup's already-shipped
|
||
right-click delete.
|
||
- **Point-count ceiling: 128 — a musical bound, not a performance one.** Segment lookup
|
||
is an indexed binary search (≤7 steps at 128 points); the cap exists so long rhythmic
|
||
phrases (roughly two points per articulation event) aren't limited, not because the
|
||
evaluator is expensive.
|
||
- **Staged controls disabled while Spline is active** — that envelope's segment knobs
|
||
and their inner curve dials render disabled and reject edits; the dormant staged state
|
||
is edited only by switching back to Staged.
|
||
- The overlay's contour is normalized to the full sample length and drawn 1:1 with the
|
||
sample's time axis; a different-length capture rescales the stored contour
|
||
proportionally.
|
||
|
||
A follow-on change in the same track reworked deck cell width: `-1` in `cellIds` changed
|
||
meaning from "a blank cell holding geometry" to **one cell's width, reserved and
|
||
redistributed** — a Trigger face that drops Sustain and Release now gets wider cells
|
||
instead of 144 px of dead slots. Group widths, row packing, deck height, and Gate-mode
|
||
cell widths are unchanged.
|
||
|
||
**Deviations from spec / code review:**
|
||
- A pure `resolveWaveformClaim` predicate (`core/instrument/ui/spline_edit`) now resolves
|
||
competing waveform-band clicks — contour node, crossfade tab, marker column, staged
|
||
envelope node — by **smallest nominal target area among candidates that actually
|
||
contain the click**, replacing resolution by check order.
|
||
- The Gate-unavailable-while-drawn rule was consolidated into
|
||
`enforceGateUnavailableWhileDrawn` (`core/instrument/engine/play_params.h`), now the
|
||
single home of that rule, called by both `resolvePlay` and the editor's
|
||
`applyControl`.
|
||
|
||
### Θ-W6-T1 — legibility-and-antialiasing
|
||
|
||
Made the editor legible, then audited every drawn surface for high-DPI clean rendering
|
||
— sequenced sizing first, audit second, since the audit's disposition list needed a
|
||
surface that had stopped moving.
|
||
|
||
- **Sizing.** Knobs grew 28→40 px (inner curve dial 14→20), the deck cell 48×58→60×74,
|
||
and the label band 12→16 px, now drawn in `Font::Label` rather than `Font::Micro`.
|
||
Group captions and toggle segments deliberately stay `Font::Micro` — bumping them
|
||
would grow the per-group `captionWidth` reserves, and row 1 has only 14 px of
|
||
headroom at the floor width.
|
||
- **Editor default/minimum size 840×620 → 980×680**, because the deck cannot pack
|
||
three rows at the old floor with the wider cells. An existing saved instance's
|
||
window grows on open. The floor is validated by a derived test rather than
|
||
literals.
|
||
- **All 14 time-constant labels now read in ms**; internal representation untouched
|
||
(`formatEnvTimeMs` is display-only). `holdFraction` knobs and `Len %` stay `%` —
|
||
they are fractions, not times. The bank panel's clip-length readout is a duration,
|
||
not a parameter time constant, and stayed out of scope.
|
||
- **Double-click reset, per ring.** Outer ring resets the value, inner dial resets
|
||
the exponent to 1.0, independently. The window class gained `CS_DBLCLKS`;
|
||
`WM_RBUTTONDBLCLK` was added as its peer so the spline right-click delete survives,
|
||
and both DBLCLK handlers fall through to the ordinary down handler. The chrome's
|
||
preview-velocity knob answers reset too, resolving against the drawn circle via a
|
||
shared `inKnobFace` rule now used by both the deck and the chrome.
|
||
- **Antialiasing pass.** Fixed: knob track/value arcs (widened to 3 px stacked-radius
|
||
AA arcs), knob needle (`LICE_ThickFLine`), inner dial arc and needle, staged
|
||
envelope slopes, spline contour, velocity-popup trace, waveform outline, preview
|
||
triangle. Already clean: node handles, curve knots, knob discs, buttons, piano
|
||
keys, loop markers, borders, gradients, text. The full disposition table is a
|
||
standing artifact in `docs/product/visual-design-language.md` §8.
|
||
- Three LICE facts the audit established: `LICE_Line` takes integer endpoints so
|
||
`aa=true` still quantizes; `LICE_FillTriangle` has no `aa` parameter at all; LICE
|
||
has no thick-arc call, so a wider ring is stacked 1 px arcs.
|
||
- **Measured cost:** the new AA waveform stroke adds ~0.41 ms per full-grid panel
|
||
repaint (0.070 → 0.48 ms over a 24-card × 2-band × 136-column grid), ~2.5% of a
|
||
60 Hz frame. Recorded in the §8 table and annotated as a one-off scratchpad
|
||
measurement, not a standing regression guard.
|
||
- **The piano-key open question is answered: not aliasing.** Every key is an
|
||
axis-aligned integer-width `LICE_FillRect`, so there was no sloped edge for
|
||
aliasing to act on; the defect was integer-division residue in the tiling, and
|
||
W2-T3's fix (remainder moved into symmetric end margins) is arithmetic. Above
|
||
client-pixel scaling it is unverified — nothing implements
|
||
`IPlugViewContentScaleSupport`.
|
||
|
||
**Two structural changes forced by review.** The waveform column's vertical
|
||
arithmetic moved into a pure, unit-tested `waveformColumnSpan` in
|
||
`core/ui/component_geometry` — the first pass had silently broken symmetry about
|
||
the midline in the shared `draw_kit` primitive that also feeds the docked bank
|
||
panel and browse thumbnails. And `PlaySeconds` plus its `AdsrSeconds`/
|
||
`AhdSeconds`/`PitchEnvSeconds`/`FilterSeconds` companions hoisted out of
|
||
`sample_map.h` into a header-only `play_seconds` INTERFACE target, so the new
|
||
`core/instrument/ui/deck_values` module stops transitively linking the bank model
|
||
and WAV codec. `deck_values` itself is an extraction of `controlValue`/
|
||
`applyControl`/`resetDeckParam`/the ms formatter out of the editor shell, making
|
||
reset semantics unit-testable; `editor_controls.cpp` dropped 469→293 lines.
|
||
|
||
All visual outcomes remain **pending Daniel's by-eye sign-off on `dev`** — sizes,
|
||
arc weight, and whether the waveform stroke improves or thickens the docked panel.
|
||
Not recorded as accepted.
|
||
|
||
### Θ-W7-T1 — arc-and-spline-aa
|
||
|
||
Two defects Daniel found by eye once Θ-W6-T1's antialiasing pass shipped — diagnosing
|
||
both corrected the initial reading of each.
|
||
|
||
- **Arcs never reached opacity.** `LICE_Arc` rasterizes a whole circle clipped per 90°
|
||
chunk and splits ink across two pixels by the fractional part of the radius;
|
||
`rOuter = radius - 0.5f` is half-integer, so no pixel in the ring was ever opaque —
|
||
measured peak alpha 138/255. The three stacked radii also did not tile: spacing
|
||
dilates from 1.0 px to 1.41 px at 45°, leaving partial-coverage holes. It read as
|
||
fuzz, but it was a stroke that never fully inked.
|
||
- **Splines were fully aliased, not gapped.** The apparent dotting was not missing
|
||
ink: `LICE_ThickFLine` steps the major axis and structurally cannot gap. The paint
|
||
loop passed **integer** `cx`/`cy`, so LICE had no sub-pixel position to interpolate
|
||
— every pixel was full or empty with no AA fringe, and integer `cy` quantized the
|
||
slope into an alternating 1/2 px staircase that reads as beading at 100%.
|
||
- **The cheap fix was rejected.** Float endpoints plus `LICE_ThickFLine` fixes
|
||
opacity and the staircase, but `ThickFLine` lays width along the *minor* axis, so
|
||
perpendicular weight is `wid·cosθ` — a measured 42% ripple dipping at every 45°
|
||
diagonal.
|
||
- **What landed:** one pure analytic thick-stroke rasterizer. Coverage is
|
||
distance-to-polyline, accumulated with `max()` into a scratch buffer and blended
|
||
**once** — the single blend is what structurally prevents the compositing fringe
|
||
build-up behind the first defect. An arc is just a polyline, so one code path
|
||
replaces the stacked arcs, both spline traces, and the two needles. Pure coverage
|
||
math in a new `core/ui/stroke_aa`; the blend loop in a new
|
||
`shell/instrument/editor_stroke`. `shell/panel/draw_kit` was deliberately **not**
|
||
touched, keeping the docked bank panel and browse cards entirely out of the blast
|
||
radius.
|
||
- **Measured, before → after:** arc peak alpha 138/255 → 255/255; arc perpendicular
|
||
weight 1.62–3.24 px (67% ripple) → 2.95–3.11 px (5%); spline weight 1.41–2.00 px
|
||
(29%) → 1.95–2.01 px (3%). Cost: **+0.09 ms per full editor repaint** (30 arcs
|
||
0.113 → 0.169 ms; 500 px contour 0.013 → 0.047 ms), a knowing regression on an
|
||
interaction-driven surface, measured in Release against real LICE in an
|
||
uncommitted harness.
|
||
- **`velocity_curve` gained `subpixelFromPoint`** — sub-pixel y was unavoidable since
|
||
integer `cy` was the root cause. The existing integer map now *rounds* the new
|
||
float map rather than forking a second formula, so hit-testing is unchanged.
|
||
- **Daniel then ruled that every sub-2 px stroker width be enlarged**, because the
|
||
stroker can only guarantee an opaque core at width >= 2 px (an opaque pixel needs
|
||
`d <= halfWidth − 0.5`, and the worst-case pixel-centre-to-centreline distance is
|
||
0.5). The knob track arc, the inner-dial needle, and the deck's mini velocity trace
|
||
all moved 1.0 → 2.0 px. A test pinning the sub-opaque behaviour at 1 px was kept as
|
||
a guard against reintroduction.
|
||
- **The audit's method was the root failure, not its output.**
|
||
`docs/product/visual-design-language.md` §8 had claimed stacked 1 px `LICE_Arc`
|
||
calls "keep every ring antialiased" — false. The Θ-W6 audit verified *which
|
||
primitive was called* rather than *what it rasterized*, which is how both surfaces
|
||
were signed off clean while never producing an opaque pixel. That sentence is
|
||
deleted, the rows are re-dispositioned with measurements, and the methodological
|
||
lesson is recorded in §8 as a standing blockquote.
|
||
|
||
All visual outcomes remain **pending Daniel's by-eye sign-off on `dev`** — nothing was
|
||
verified in a live REAPER window; all measurement was against an offscreen bitmap in a
|
||
standalone harness. Not recorded as accepted.
|