From f115904e4f71ed4f72c30e3f773b2930d38b77d1 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 31 Jul 2026 20:31:53 -0400 Subject: [PATCH] docs: close out wave Theta-W4, and defer the deck layout rework to TODO --- docs/COMPLETED.md | 44 ++++++++++++++++++++++++ docs/PLAN.md | 86 +++++------------------------------------------ docs/TODO.md | 45 +++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 77 deletions(-) diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index cd737c8..41300f1 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -460,3 +460,47 @@ bumped to **v11** (`kParamsLoopVersion`), appended at the tail; slot 12 is reser 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. diff --git a/docs/PLAN.md b/docs/PLAN.md index 6b0300d..7ef26c8 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -197,83 +197,15 @@ is the Gate-mode sustain the AHDSR releases out of, and T2's bipolar pitch/filte modulate targets whose envelopes W3 just reshaped. T2 additionally depends on W2-T1 for the filter's existence and on W2-T3 for the preview button's toolbar position. -**Two tracks, in priority order.** Disjoint: T1 owns the engine loop path and the -waveform band's marker layer; T2 owns the deck band, the curve popup, and the preview -button in the chrome band. Neither touches the other's band. - -#### Θ-W4-T1 — `gate-loop-sustain` - -**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. - ---- - -#### Θ-W4-T2 — `velocity-deck-and-bipolar-curves` - -**Goal.** Give the three velocity-curve popups one home, make the pitch and filter curves -bipolar and off-by-default, and replace the preview button's text with a glyph. - -**Consolidates item 11.** - -**Surface boundary — owns:** `core/instrument/engine/velocity_curve` (bipolar y-domain), -`core/instrument/ui/curve_popup`, the **deck band** (the new VELOCITY group), and **the -preview button in the chrome band** at the toolbar position W2-T3 gave it. Does not touch -the waveform band. - -**Behavior.** -- **The VELOCITY deck.** All three velocity-curve popup buttons (amp, pitch, filter) live - **together in a new control deck group labelled "VELOCITY", placed to the left of the - VOICE group.** This **supersedes the original ask's per-section placement** — Daniel - confirmed the MASTER placement was a real contention point: **MASTER is reserved for - other, post-voice-mixer concerns** he will add to later, so the velocity curves do not - belong there. No velocity-curve button appears in MASTER, PITCH, or Filter. -- **Bipolar pitch/filter transfer functions.** Pitch and filter velocity transfer - functions are **bipolar: y range [−1, 1], default y = 0** — flat at zero, meaning - velocity modulation of pitch and filter is **off until the user draws a curve**. -- **Amp stays unipolar at [0, 1]**, and its existing flat-unity default is unchanged. -- **Storage:** the curves live in the instrument's one parameter set. -- **Preview glyph.** The preview button's inner text is replaced with a glyph. Proposed - at product level: a right-pointing **play triangle** — the universal audition read. - **Daniel's constraint: no new dependencies** — a statically embedded bitmap or - equivalent that plays nicely with the existing LICE drawing path. - -**Acceptance criteria.** -- The three velocity-curve buttons sit together in a deck group labelled **VELOCITY**, - immediately to the left of the VOICE group; no velocity-curve button appears in MASTER, - PITCH, or Filter. -- Opening the pitch or filter curve shows a **bipolar editor ([−1, 1]) defaulted flat at - y = 0**; played velocities produce no pitch/filter modulation until a curve is drawn, - then audibly follow it. -- The amp curve's domain ([0, 1]) and flat-unity default are unchanged. -- The velocity curves persist in the one parameter set and round-trip save/reload. -- The preview button shows the glyph (no text) and stays legible in **all** interaction - states; **no new build or runtime dependency is introduced.** - -**Open questions.** -- **Does a user-facing pitch velocity transfer curve already exist, or does this item - introduce it? [verify]** Unverifiable in the source doc under its no-code-reads - constraint. If absent, this track introduces it. Check before scoping. -- **Bipolar rendering in the shared spline editor [propose].** Item 3 (Θ-W5) enhances the - one spline implementation for hard points; this track must render and edit a bipolar - y-domain alongside the amp curve's unipolar one. Land the bipolar domain here in the - existing editor; Θ-W5 then adds hard points to the same editor without re-deciding the - domain. +**Both tracks have landed** — Θ-W4-T1 (`gate-loop-sustain`) and Θ-W4-T2 +(`velocity-deck-and-bipolar-curves`) — see `docs/COMPLETED.md` for the full narrative of +each. Between them: loop points are now a usable feature, with a Gate-mode loop acting +as the sustain and a parameterized crossfade at the seam; and the three velocity-curve +popups (amp, pitch, filter) now live together in a new VELOCITY deck group, with the +pitch and filter curves bipolar and flat-by-default so their modulation is off until +drawn, while the amp curve stays unipolar and unchanged, and the preview button's text +is replaced by a drawn play-triangle glyph. Params payload reached v11 with T1's loop +block and v12 with T2's velocity→pitch curve appended after it. --- diff --git a/docs/TODO.md b/docs/TODO.md index 5f904dc..cb464b9 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -179,3 +179,48 @@ Forward-looking follow-ups. Deferred by decision, not oversight — each entry r **Priority / risk.** Low / deferred. Mitigated in the meantime: the console message names the stranded file's project-relative path, so the operator can recover or remove it rather than discovering it later as an unexplained orphan. **Done looks like.** Switching projects mid-record leaves the recorded file with a ledger record in the project it belongs to, so a later prune of that project can reclaim it normally. + +## The deck layout needs a real rework — one row, taller decks, controls stacked within a deck + +**Context (what shipped — Θ-W4-T2).** The VELOCITY deck group landed with the amp, pitch, +and filter velocity-curve buttons together, but the group placement lands wherever the +existing per-row deck layout happens to put it. Daniel does not want multiple rows of +decks. He wants **one row**, with the decks **much taller** to accommodate the controls — +knobs stacked *within* a deck rather than strung out in a long line of small knobs. His +example: the filter's static control knobs go **above** the filter envelope knobs. +**He has further design notes coming**, so this is not ready to implement — it is +recorded here so it is not lost and not attempted piecemeal. + +**The wart.** Θ-W4-T2's acceptance criterion "VELOCITY sits immediately to the left of +the VOICE group" is **not met at the default window size**. This is **superseded by the +deferred rework**, not silently dropped — recorded here so a later reader does not see an +unmet criterion with no explanation. + +**Intended fix.** Not yet specified beyond the shape above (one row, taller decks, +within-deck knob stacking) — Daniel's further design notes are still to come. + +**The constraint the fix MUST handle.** Not yet known — this is exactly why the rework is +deferred rather than attempted against the current spec. + +**The measured geometry — the data the rework starts from.** Measured at the 840 px +floor window, `availWidth = 824`, Gate mode. Group widths: PITCH 150, PITCH ENV 204, +FILTER 440, FILTER ENV 252, AMP 252, VELOCITY 156, VOICE 152, MASTER 60. Row 1 = PITCH + +PITCH ENV + FILTER (818 of 824). Row 2 = FILTER ENV + AMP + VELOCITY (684). Row 3 = VOICE ++ MASTER (224). Deck height 280 px; waveform band 246 px against a 150 px two-lane floor, +so 96 px of margin. No reordering can fit two rows — 1666 px of groups plus 72 px of gaps +against a 1648 px two-row capacity. Two rows first fit at a 936 px window. + +**Unresolved measurement discrepancy.** The VELOCITY↔VOICE adjacency threshold was +measured at a 864 px window by the implementation and stated as 872 px by code review; +the implementation flagged the difference rather than asserting the reviewer wrong, and +it was not re-measured because the deferral made it moot. + +**Priority / risk.** Deliberate deferral, not oversight. Daniel wants to finish his +design notes before this is attempted, to avoid building a layout that gets reworked +piecemeal. + +**Done looks like.** One row of decks, each much taller than today's, with each deck's +controls stacked internally (e.g. the filter's static control knobs above its envelope +knobs) per Daniel's design notes once they land. The VELOCITY↔VOICE adjacency criterion +is re-evaluated (or explicitly retired) against the new layout rather than the old +per-row one.