diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index 6f09ad1..cd737c8 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -410,3 +410,53 @@ 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). diff --git a/docs/PLAN.md b/docs/PLAN.md index 8f714e6..6b0300d 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -203,57 +203,24 @@ button in the chrome band. Neither touches the other's band. #### Θ-W4-T1 — `gate-loop-sustain` -**Goal.** Establish (or restore) loop points, and make a Gate-mode loop function as the -sustain — indefinite playback until note-off, with a crossfaded seam. - -**Consolidates item 9 (both halves: the suspected regression and the feature spec).** - -**Surface boundary — owns:** the voice-render loop read path (post-W1 seam), -`core/instrument/map/component_state_io` (loop parameters), `core/instrument/ui/ -waveform_view` markers, and the **waveform band's marker layer**. Reads W2-T2's lane -geometry and W3's overlay contract; re-owns neither. - -**Behavior.** -- **Regression half.** Daniel's observation: loop points appear to have been lost. - Whether they were genuinely removed from playback or are merely unexposed in the - current UI **must be verified as the first act of implementation** — it was not - guessable in the source doc, which was written under a no-code-reads constraint. -- **Feature half.** In **Gate mode**: a loopable section with **parameterized start and - end points** and a **parameterized crossfade on loop reset**. The loop functions as the - sustain — indefinite playback cycling the loop until note-off, then release. -- **Loop-sustain is a Staged/Gate-mode feature.** Gate mode is unavailable while a - Spline EG is active (item 3, Θ-W5), so Spline mode remains full-sample-length playback - with no loop. This track must not assume Gate is always available. -- **Storage:** loop parameters belong to the instrument's one parameter set. There is no - per-zone side to confirm. -- **Preserve-mode contract (existing, do not break):** *loop the source, shift the - output* — loop points stay source-frame facts. -- **Reset-scope note for Ξ:** item 15 classifies loop points as **baked-in** — they - shaped the render and old positions are meaningless against new audio — so they reset - on a resample. Nothing to build here; recorded so Ξ-W2-T1 does not re-derive it. - -**Acceptance criteria.** -- In Gate mode with a loop defined, a held note sustains indefinitely, audibly cycling - the loop section; note-off exits into the release stage. -- With a nonzero crossfade the loop seam is smooth — **no click at the loop reset** — and - crossfade length audibly follows its parameter. -- Loop start, end, and crossfade are user-parameterized, editable, and persisted across - save/reload. -- Whatever the regression finding, the end state is loop points exposed and functional - per this spec. -- Looping adds no allocation to `process()`; the crossfade is computed on the per-sample - path without a branch-heavy or virtual detour. - -**Open questions.** -- **Regression verification — removed vs. unexposed [verify].** The first act of - implementation. Report the finding; the end state is the same either way. -- **Crossfade units and range [propose].** Unspecified. Surface a proposed default at - implementation review (ms is the likely denomination given item 10 puts time constants - in ms). -- **Editing surface for loop start/end [propose].** Waveform markers, knobs, or both, is - unspecified. The waveform display is the natural home for range markers — and - `waveform_view` already owns generic named draggable markers with clamp and - zero-crossing snap, which is a strong lean — but Daniel has not said. Propose at review. +**Has landed** — see `docs/COMPLETED.md` for the full narrative. Loop points are now a +usable feature and a Gate-mode loop functions as the sustain. 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`, and 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. Fixed by moving the ghost +span to `defaultLoopBounds` (last quarter, both handles clear), making a collapsed span +the explicit OFF gesture, and adding a parameterized crossfade — stored in source +frames, not ms (a deliberate divergence from the plan's ms lean, following +`sample_map.h`'s source-frame rule), linear rather than equal-power, and normalized over +`crossfade − 1` (a review Major fixed a residual seam discontinuity) so the last +rendered frame lands exactly on the incoming tap. New pure module +`src/core/instrument/engine/loop/` (`loop_span`) holds the loop math; the editing +surface is the waveform markers plus a new top-strip `markerHandleRect` grab tab, a +general fix for markers sharing a frame. Params payload is v11 (`kParamsLoopVersion`); +slot 12 is reserved for T2. Left to Daniel: whether the seam sounds smooth on real +material, the top-strip tab's discoverability, the LICE rendering of the tab and +crossfade fill, and whether the crossfade default should stay 0. ---