From 7f74b11dceb7b894103a5b0af1a9f35a4b1fdc80 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 09:16:30 -0400 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20legible=20ReaSampler=209000=20edito?= =?UTF-8?q?r=20=E2=80=94=20bigger=20knobs,=20ms=20time=20constants,=20per-?= =?UTF-8?q?ring=20double-click=20reset,=20and=20an=20antialiased=20draw=20?= =?UTF-8?q?pass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/product/visual-design-language.md | 61 +++++ src/core/instrument/CLAUDE.md | 13 +- src/core/instrument/ui/CMakeLists.txt | 9 + src/core/instrument/ui/deck_values.cpp | 211 +++++++++++++++ src/core/instrument/ui/deck_values.h | 52 ++++ src/core/instrument/ui/knob_deck.cpp | 24 ++ src/core/instrument/ui/knob_deck.h | 27 +- src/core/instrument/ui/sample_bands.h | 6 +- src/core/instrument/ui/sample_chrome.cpp | 6 +- src/shell/instrument/CLAUDE.md | 9 +- src/shell/instrument/CMakeLists.txt | 3 +- src/shell/instrument/editor_controls.cpp | 253 +++--------------- src/shell/instrument/editor_input.cpp | 13 + src/shell/instrument/editor_input_chrome.cpp | 12 + src/shell/instrument/editor_input_deck.cpp | 37 +++ src/shell/instrument/editor_internal.h | 37 ++- src/shell/instrument/editor_paint_chrome.cpp | 8 + src/shell/instrument/editor_paint_curve.cpp | 4 +- src/shell/instrument/editor_paint_deck.cpp | 8 +- .../instrument/editor_paint_waveform.cpp | 10 +- src/shell/instrument/editor_platform.cpp | 21 +- src/shell/instrument/reasampler_editor.h | 23 +- src/shell/panel/draw_kit.cpp | 26 +- tests/test_deck_groups.cpp | 18 +- tests/test_deck_values.cpp | 175 ++++++++++++ tests/test_knob_deck.cpp | 74 ++++- tests/test_sample_chrome.cpp | 4 +- 27 files changed, 859 insertions(+), 285 deletions(-) create mode 100644 src/core/instrument/ui/deck_values.cpp create mode 100644 src/core/instrument/ui/deck_values.h create mode 100644 tests/test_deck_values.cpp diff --git a/docs/product/visual-design-language.md b/docs/product/visual-design-language.md index c1d6aee..36b7b07 100644 --- a/docs/product/visual-design-language.md +++ b/docs/product/visual-design-language.md @@ -761,3 +761,64 @@ re-skin).** font obligation. - **Phase S is not gated on Phase L** — S7–S13 proceeded in parallel; they adopted the kit via L3 when it landed. Phase L is complete (L1–L7 all landed). + +--- + +## 8. Antialiasing disposition — the drawn-surface audit + +A standing inventory of every class of drawn surface and how it answers antialiasing, so the +audit is re-runnable rather than a one-off sweep. **The rule the table applies:** an +axis-aligned fill or hairline has no aliasing to remove — LICE's `aa` flag is inert on a pure +horizontal or vertical run — so "already clean" there is a statement about geometry, not a +concession. Everything with a slope or a curve must draw through a primitive that antialiases. + +**Primitive gotchas this audit established (verified in `vendor/WDL/WDL/lice/`):** + +- `LICE_Line` takes INTEGER endpoints. `aa=true` antialiases the span, but the endpoints are + still quantized; `LICE_FLine` keeps float endpoints and `LICE_ThickFLine` is *always* + antialiased and adds width. +- `LICE_FillTriangle` takes **no** `aa` parameter at all — its sloped edges alias, and the + only fix inside the kit is to re-stroke those edges with an AA line in the same ink. +- LICE has no thick-arc primitive. A wider ring is drawn as adjacent 1 px `LICE_Arc` calls at + stepped radii, which keeps every ring antialiased. +- A min/max waveform column plot cannot be antialiased by the column fill itself (the columns + are vertical). The outline is what reads as jagged, so it is stroked separately. + +| Surface | Where | Disposition | +|---|---|---| +| Radial knob track + value arc | `editor_internal.h` `drawKnobFace` | Was AA (`LICE_Arc`, 1 px). **Widened** to a 3 px stacked-radius ring; the bigger knob is what made 1 px read thin. | +| Knob needle | `drawKnobFace` | **Fixed** — was integer-endpoint `LICE_Line`; now `LICE_ThickFLine` (always AA, float endpoints, 2 px). | +| Inner curve dial arc + needle | `drawInnerDial` | **Fixed** — 2 px stacked-radius arc; needle moved to `LICE_FLine` with float endpoints. | +| Staged envelope segment slopes | `editor_paint_waveform.cpp` | **Fixed** — `LICE_ThickFLine` at 2 px, replacing integer-endpoint `LICE_Line`. | +| Spline (drawn EG) contour | `editor_paint_waveform.cpp` `paintSplineOverlay` | **Fixed** — same treatment, one trace grammar. | +| Velocity-curve popup trace | `editor_paint_curve.cpp` | **Fixed** — same treatment. | +| Velocity-curve mini thumbnail | `editor_paint_curve.cpp` | Left at 1 px AA `LICE_Line` — a 2 px trace blots at thumbnail scale. | +| Waveform min/max columns | `draw_kit.cpp` `drawWaveform` | **Fixed** — column fill unchanged (it cannot alias), plus an AA `LICE_FLine` stroke joining each column's extremes to its neighbour's, in the same ink. Shared with the docked bank panel and the browser cards. | +| Preview play triangle | `editor_paint_chrome.cpp` | **Fixed** — `LICE_FillTriangle` has no `aa`; its two sloped edges are re-stroked with AA `LICE_FLine`. | +| Envelope/spline node handles (squares) | `editor_paint_waveform.cpp` | Already clean — axis-aligned `LICE_FillRect`. | +| Envelope curve knots (circles) | `editor_paint_waveform.cpp` | Already clean — `LICE_FillCircle` with `aa=true`. | +| Knob body disc | `drawKnobFace` / `drawInnerDial` | Already clean — `LICE_FillCircle` with `aa=true`. | +| Buttons | `draw_kit.cpp` `drawButton` | Already clean — `LICE_RoundRect` with `aa=true`. | +| Piano key faces + edges | `editor_paint_chrome.cpp` `drawKeyboard` | Already clean — axis-aligned fills and a vertical hairline. **See §8.1.** | +| Loop span, crossfade region, marker bars, grab tab | `editor_paint_waveform.cpp` | Already clean — axis-aligned fills. | +| Group fences, card/tab/tooltip borders, focus rings | deck, browse, panel painters | Already clean — `LICE_DrawRect`, axis-aligned. | +| Surface fills + inner edge highlights | `draw_kit.cpp` `fillSurface` | Already clean — `LICE_GradRect` + axis-aligned hairlines. | +| Embed strip (TCP/MCP) | `reasampler_embed.cpp` | Already clean — axis-aligned fills only. | +| Docked bank panel chrome | `panel_render.cpp` | Already clean — axis-aligned fills, rects and hairlines. Its only exposure to this pass is the shared `drawWaveform`. | +| Text | `draw_kit.cpp` `text` | Already clean — `LICE_CachedFont` AA glyph cache (§1.1). | + +### 8.1 Was the piano-key width defect an aliasing artifact? + +**No.** Every piano key is an axis-aligned `LICE_FillRect` with an integer width, so there is +no sloped or curved edge for aliasing to act on — the defect could not have had that cause. +It was integer-division residue: `keyboard_strip` tiles same-class keys at one integer width +and the indivisible remainder of the band width has to go *somewhere*. The fix put it in +symmetric end margins instead of in a key, which is arithmetic, not rasterization. + +**Does the fix survive DPI scaling?** At the client-pixel level, yes — key widths are uniform +by construction at every client width the strip's test sweep covers. Above that level it is +**unverified**, and for a structural reason worth keeping visible: nothing in the instrument +implements `IPlugViewContentScaleSupport`, so a host that scales the plugin window resamples +the already-rasterized uniform widths at the physical-pixel level, where the guarantee no +longer applies. That is a host-scaling question, not an antialiasing one, and it is recorded +as a gotcha in `src/core/instrument/CLAUDE.md`. diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index 012f62d..9492dc4 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -309,7 +309,18 @@ anything for a trigger shape. - `browser_scroll` — scroll + type-to-filter layered over `capture_browser`: vertical scroll offset, scrollbar thumb, thumb-drag mapping, and name-substring search. - `param_slider` — parameter control-panel: vertical stack of TOGGLE (two-segment selector) and SLIDER (horizontal track) rows; maps normalized value to/from handle pixel. - `embed_strip` — compact single-row control layout for embed mode in the track FX chain. -- `knob_deck` — pure knob-deck layout + hit-test (FB1): group-box / caption-row / compact-toggle / knob-cell geometry, deterministic whole-group wrap, `DeckLayout` / `DeckHit`. Mirror of `action_bar`/`param_slider`; no LICE or REAPER types. A group carries TWO caption-toggle slots, laid right-to-left: the second exists because a group whose knob row is wider than its caption row has caption slack a toggle can occupy for free, and the deck has six pixels of headroom on its first row at the editor's floor width — a `rowToggle` would widen the GROUP and wrap the deck to a fourth row, past what the minimum window holds. **A group's cell run is a RESERVED WIDTH, not a fixed cell size**: a `-1` id reserves one cell's width without a cell, and the cells present divide the whole run between them at one uniform integer width (residue in symmetric end margins). That is what lets a mode flip drop controls from a face — Trigger's AMP and FILTER ENV lose their Sustain/Release stages — without either reflowing the deck or leaving dead slots in the box; a face with fewer controls simply gets roomier cells. Do not reintroduce fixed-width cells with blank slots. +- `knob_deck` — pure knob-deck layout + hit-test (FB1): group-box / caption-row / compact-toggle / knob-cell geometry, deterministic whole-group wrap, `DeckLayout` / `DeckHit`. Mirror of `action_bar`/`param_slider`; no LICE or REAPER types. Carries a SECOND hit-test, `hitTestKnobFace`, resolved against the drawn CIRCLES rather than the cell: a double-click reset is aimed at a dial, so the label band and the cell margins must miss where a drag grab deliberately does not, and only a radial resolve can tell the inner curve dial from the outer ring it sits inside. **The cell/knob/label sizes and `sample_bands`' editor floor move as a pair** — wider cells need a wider floor width or the deck wraps to a fourth row. A group carries TWO caption-toggle slots, laid right-to-left: the second exists because a group whose knob row is wider than its caption row has caption slack a toggle can occupy for free, and the deck has six pixels of headroom on its first row at the editor's floor width — a `rowToggle` would widen the GROUP and wrap the deck to a fourth row, past what the minimum window holds. **A group's cell run is a RESERVED WIDTH, not a fixed cell size**: a `-1` id reserves one cell's width without a cell, and the cells present divide the whole run between them at one uniform integer width (residue in symmetric end margins). That is what lets a mode flip drop controls from a face — Trigger's AMP and FILTER ENV lose their Sustain/Release stages — without either reflowing the deck or leaving dead slots in the box; a face with fewer controls simply gets roomier cells. Do not reintroduce fixed-width cells with blank slots. +- `deck_values` — the deck's control-id ↔ parameter-set BINDING and its display units, split + from the editor shell on the same axis `deck_groups` was split from `knob_deck`: `deck_groups` + says which controls exist, this says what each one's value MEANS. Holds `deckParamNorm` / + `setDeckParam` (the normalized ↔ stored-seconds/fraction/position maps and their clamps), + `resetDeckParam` (the double-click reset — the defaults are READ off a default-constructed + `PlaySeconds`, so there is no second table of defaults to drift), and `formatEnvTimeMs`, the + ONE time-constant formatter: every displayed time constant reads in **ms**, never seconds, so + two stage times are comparable at a glance. A display-unit decision only — nothing about the + stored representation changes. Links the map layer because `PlaySeconds` is what a deck edits. + The shell keeps only the controls the parameter set does not carry (key-track, voice count, + master gain, preview velocity) and the labels for them. - `deck_groups` — also home to `isLiveDeckParam` and `liveCommitFor`, the editor's whole commit-tier routing decision (see "Live parameter delivery" above), and to `OverlayEnv` + `nextOverlaySelection`/`overlayEnvEnabled`/`overlayEnvInert`, the whole overlay-selection state machine (exclusivity, the none resting state, and which selections a disabled or DRAWN group makes inert); WHICH groups the Sample face's deck carries, split from `knob_deck`'s HOW they lay out: the `DeckParam` control-id space (the editor's `ParamControl` is an alias of it), the `DeckGroupId` list, `sampleDeckGroups` in signal-flow order (**pitch → filter → amp**, then velocity/voice/master), and the deck's bipolar-knob law. Reads `PlayMode` for the AMP group's Gate/Trigger face, which is why this and not `knob_deck` is the module that touches the engine's value layer. Also home to `CurveTarget` + `curveTargetFor` — the VELOCITY group's three cells are popup openers, not dials, and that predicate is the ONE place they are named, so paint, hit-test routing and the popup's title all agree. MASTER is reserved for post-voice-mixer concerns, which is why the curves sit in their own group immediately left of VOICE rather than there. - `spline_edit` — THE point-editing grammar, and the one place it is written down: left-click grabs a node and adds one in empty space, right-click deletes, control-click toggles hard/smooth. Both spline consumers — the velocity-curve popup and the spline EG overlay — route their mouse-down through `resolveSplineEdit`, so the two cannot drift into two grammars. The endpoint and point-count rules are NOT restated here: `deletePoint` and `addPoint` own them, and the caller applies the resolved action to the curve. Also home to `splineOverlayBox`, the contour's mapping box inside the waveform overlay — the FULL area, no inset, so the drawn contour stays 1:1 with the sample's time axis. - `curve_popup` — pure curve-popup geometry + dismissal test (FB1): centered sheet over the Sample face — width/height clamps, title row, Close button rect, curve-box rect, outside-sheet dismissal test. Mirror of `overflow_menu`; no LICE or REAPER types. diff --git a/src/core/instrument/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index cff632f..affe9ec 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -68,6 +68,15 @@ reasampler_pure_library(spline_edit # kWaveformMinHeight floor. reasampler_test(spline_edit LINK spline_edit waveform_view sample_bands) +# The deck's VALUE binding, split from its composition on the same axis deck_groups was split +# from knob_deck. Links sample_map because PlaySeconds — the thing a deck knob edits — is +# declared there; no sample_map symbol is called, only its value types. Same for the filter's +# MorphLaw — an enum, so no filter symbol is linked. +reasampler_pure_library(deck_values + SOURCES deck_values.cpp + LINK PUBLIC deck_groups sample_map envelope_overlay) +reasampler_test(deck_values LINK deck_values) + reasampler_pure_library(curve_popup SOURCES curve_popup.cpp LINK PUBLIC editor_geometry) # velocity_curve is linked for the test only: the sheet's geometry is domain-agnostic, and # proving that takes a curve of each domain mapped through the one curveBox. diff --git a/src/core/instrument/ui/deck_values.cpp b/src/core/instrument/ui/deck_values.cpp new file mode 100644 index 0000000..97c58c4 --- /dev/null +++ b/src/core/instrument/ui/deck_values.cpp @@ -0,0 +1,211 @@ +// deck_values.cpp — see deck_values.h. Pure value math; no host types. + +#include "core/instrument/ui/deck_values.h" + +#include +#include + +#include "core/instrument/engine/filter/filter_morph.h" // MorphLaw (the law toggle's value) +#include "core/util/clamp01.h" +#include "core/util/curve_law.h" // the ONE curve-exponent domain + +namespace reasampler::instrument::ui { + +using engine::filter::MorphLaw; +using util::clamp01; + +namespace { + +double secToNorm(double seconds) { return clamp01(seconds / kEnvTimeMaxSeconds); } +double normToSec(double norm) { return clamp01(norm) * kEnvTimeMaxSeconds; } + +} // namespace + +double deckParamNorm(DeckParam id, const PlaySeconds& play) { + switch (id) { + case DeckParam::kPlayMode: return play.playMode == PlayMode::Trigger ? 1.0 : 0.0; + case DeckParam::kAmpEnvMode: return play.ampSpline.mode == EnvMode::Spline ? 1.0 : 0.0; + case DeckParam::kPitchEnvMode: return play.pitchSpline.mode == EnvMode::Spline ? 1.0 : 0.0; + case DeckParam::kFilterEnvMode: return play.filterSpline.mode == EnvMode::Spline ? 1.0 : 0.0; + case DeckParam::kPitchEngine: return play.pitchEngine == PitchEngine::Preserve ? 1.0 : 0.0; + case DeckParam::kAttack: return secToNorm(play.adsr.attackSeconds); + case DeckParam::kHold: return secToNorm(play.adsr.holdSeconds); + case DeckParam::kDecay: return secToNorm(play.adsr.decaySeconds); + case DeckParam::kSustain: return clamp01(play.adsr.sustainLevel); + case DeckParam::kRelease: return secToNorm(play.adsr.releaseSeconds); + case DeckParam::kAttackCurve: return util::knobNormFromCurve(play.adsr.attackCurve); + case DeckParam::kDecayCurve: return util::knobNormFromCurve(play.adsr.decayCurve); + case DeckParam::kReleaseCurve: return util::knobNormFromCurve(play.adsr.releaseCurve); + case DeckParam::kTrigLength: return clamp01(play.trigger.lengthFraction); + case DeckParam::kTrigAttack: return secToNorm(play.trigAhd.attackSeconds); + case DeckParam::kTrigHold: return clamp01(play.trigAhd.holdFraction); + case DeckParam::kTrigDecay: return secToNorm(play.trigAhd.decaySeconds); + case DeckParam::kTrigAttackCurve: return util::knobNormFromCurve(play.trigAhd.attackCurve); + case DeckParam::kTrigDecayCurve: return util::knobNormFromCurve(play.trigAhd.decayCurve); + case DeckParam::kPitchEnvEnable: return play.pitchEnv.enabled ? 1.0 : 0.0; + case DeckParam::kPitchEnvAttack: return secToNorm(play.pitchEnv.shape.attackSeconds); + case DeckParam::kPitchEnvHold: return clamp01(play.pitchEnv.shape.holdFraction); + case DeckParam::kPitchEnvDecay: return secToNorm(play.pitchEnv.shape.decaySeconds); + case DeckParam::kPitchEnvAttackCurve: + return util::knobNormFromCurve(play.pitchEnv.shape.attackCurve); + case DeckParam::kPitchEnvDecayCurve: + return util::knobNormFromCurve(play.pitchEnv.shape.decayCurve); + case DeckParam::kPitchEnvDepth: + // Signed depth centred at 0.5 (0.5 == 0 semitones). + return clamp01(0.5 + play.pitchEnv.peakSemitones / (2.0 * kPitchDepthMaxSemis)); + // Filter. The four tone controls ARE the module's normalized positions — stored and + // shown as-is, so the knob travel is exactly filter_params' own law. + case DeckParam::kFilterEnable: return play.filter.enabled ? 1.0 : 0.0; + case DeckParam::kFilterLaw: + return play.filter.settings.morphLaw == MorphLaw::HighNotchLow ? 1.0 : 0.0; + case DeckParam::kFilterMorph: return clamp01(play.filter.settings.morphNorm); + case DeckParam::kFilterCutoff: return clamp01(play.filter.settings.cutoffNorm); + case DeckParam::kFilterQ: return clamp01(play.filter.settings.resonanceNorm); + case DeckParam::kFilterDrive: return clamp01(play.filter.settings.driveNorm); + case DeckParam::kFilterModAmt: return deckNormFromBipolar(play.filter.modAmount); + case DeckParam::kFilterVel: return deckNormFromBipolar(play.filter.velAmount); + case DeckParam::kFilterKeyTrack: return clamp01(play.filter.keyTrack / kKeyTrackMax); + case DeckParam::kFilterEnvAttack: return secToNorm(play.filter.env.attackSeconds); + case DeckParam::kFilterEnvHold: return secToNorm(play.filter.env.holdSeconds); + case DeckParam::kFilterEnvDecay: return secToNorm(play.filter.env.decaySeconds); + case DeckParam::kFilterEnvSustain: return clamp01(play.filter.env.sustainLevel); + case DeckParam::kFilterEnvRelease: return secToNorm(play.filter.env.releaseSeconds); + case DeckParam::kFilterEnvAttackCurve: + return util::knobNormFromCurve(play.filter.env.attackCurve); + case DeckParam::kFilterEnvDecayCurve: + return util::knobNormFromCurve(play.filter.env.decayCurve); + case DeckParam::kFilterEnvReleaseCurve: + return util::knobNormFromCurve(play.filter.env.releaseCurve); + case DeckParam::kFilterTrigAttack: return secToNorm(play.filter.trigEnv.attackSeconds); + case DeckParam::kFilterTrigHold: return clamp01(play.filter.trigEnv.holdFraction); + case DeckParam::kFilterTrigDecay: return secToNorm(play.filter.trigEnv.decaySeconds); + case DeckParam::kFilterTrigAttackCurve: + return util::knobNormFromCurve(play.filter.trigEnv.attackCurve); + case DeckParam::kFilterTrigDecayCurve: + return util::knobNormFromCurve(play.filter.trigEnv.decayCurve); + default: return 0.0; + } +} + +void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment) { + switch (id) { + case DeckParam::kPlayMode: + // Gate is refused while any EG is drawn — see splineActive (play_params.h). The + // segment paints Disabled for the same reason, so the refusal is never a surprise. + if (segment == 0 && splineActive(play)) break; + play.playMode = (segment == 1) ? PlayMode::Trigger : PlayMode::Gate; + break; + // Switching TO Spline drops Gate, which the spline model has no place for. Switching + // back does NOT restore it: the previous mode is not stored, and silently re-gating an + // instrument the user has since heard as a one-shot is the worse surprise. + case DeckParam::kAmpEnvMode: + case DeckParam::kPitchEnvMode: + case DeckParam::kFilterEnvMode: { + const EnvMode m = (segment == 1) ? EnvMode::Spline : EnvMode::Staged; + if (id == DeckParam::kAmpEnvMode) play.ampSpline.mode = m; + else if (id == DeckParam::kPitchEnvMode) play.pitchSpline.mode = m; + else play.filterSpline.mode = m; + break; + } + case DeckParam::kPitchEngine: + play.pitchEngine = (segment == 1) ? PitchEngine::Preserve : PitchEngine::Varispeed; + break; + case DeckParam::kAttack: play.adsr.attackSeconds = normToSec(value); break; + case DeckParam::kHold: play.adsr.holdSeconds = normToSec(value); break; + case DeckParam::kDecay: play.adsr.decaySeconds = normToSec(value); break; + case DeckParam::kSustain: play.adsr.sustainLevel = clamp01(value); break; + case DeckParam::kRelease: play.adsr.releaseSeconds = normToSec(value); break; + case DeckParam::kAttackCurve: play.adsr.attackCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kDecayCurve: play.adsr.decayCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kReleaseCurve: play.adsr.releaseCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kTrigLength: + // lengthFraction is (0,1]; keep a small floor so a zero-length trigger never plays + // nothing. + play.trigger.lengthFraction = (std::max)(0.01, clamp01(value)); + break; + case DeckParam::kTrigAttack: play.trigAhd.attackSeconds = normToSec(value); break; + case DeckParam::kTrigHold: play.trigAhd.holdFraction = clamp01(value); break; + case DeckParam::kTrigDecay: play.trigAhd.decaySeconds = normToSec(value); break; + case DeckParam::kTrigAttackCurve: + play.trigAhd.attackCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kTrigDecayCurve: + play.trigAhd.decayCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kPitchEnvEnable: play.pitchEnv.enabled = (segment == 1); break; + case DeckParam::kPitchEnvAttack: + play.pitchEnv.shape.attackSeconds = normToSec(value); break; + case DeckParam::kPitchEnvHold: + play.pitchEnv.shape.holdFraction = clamp01(value); break; + case DeckParam::kPitchEnvDecay: + play.pitchEnv.shape.decaySeconds = normToSec(value); break; + case DeckParam::kPitchEnvAttackCurve: + play.pitchEnv.shape.attackCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kPitchEnvDecayCurve: + play.pitchEnv.shape.decayCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kPitchEnvDepth: + play.pitchEnv.peakSemitones = (clamp01(value) - 0.5) * 2.0 * kPitchDepthMaxSemis; + break; + case DeckParam::kFilterEnable: play.filter.enabled = (segment == 1); break; + case DeckParam::kFilterLaw: + play.filter.settings.morphLaw = + (segment == 1) ? MorphLaw::HighNotchLow : MorphLaw::HighBandLow; + break; + case DeckParam::kFilterMorph: + play.filter.settings.morphNorm = static_cast(clamp01(value)); break; + case DeckParam::kFilterCutoff: + play.filter.settings.cutoffNorm = static_cast(clamp01(value)); break; + case DeckParam::kFilterQ: + play.filter.settings.resonanceNorm = static_cast(clamp01(value)); break; + case DeckParam::kFilterDrive: + play.filter.settings.driveNorm = static_cast(clamp01(value)); break; + case DeckParam::kFilterModAmt: play.filter.modAmount = deckBipolarFromNorm(value); break; + case DeckParam::kFilterVel: play.filter.velAmount = deckBipolarFromNorm(value); break; + case DeckParam::kFilterKeyTrack: + play.filter.keyTrack = clamp01(value) * kKeyTrackMax; break; + case DeckParam::kFilterEnvAttack: + play.filter.env.attackSeconds = normToSec(value); break; + case DeckParam::kFilterEnvHold: + play.filter.env.holdSeconds = normToSec(value); break; + case DeckParam::kFilterEnvDecay: + play.filter.env.decaySeconds = normToSec(value); break; + case DeckParam::kFilterEnvSustain: + play.filter.env.sustainLevel = clamp01(value); break; + case DeckParam::kFilterEnvRelease: + play.filter.env.releaseSeconds = normToSec(value); break; + case DeckParam::kFilterEnvAttackCurve: + play.filter.env.attackCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kFilterEnvDecayCurve: + play.filter.env.decayCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kFilterEnvReleaseCurve: + play.filter.env.releaseCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kFilterTrigAttack: + play.filter.trigEnv.attackSeconds = normToSec(value); break; + case DeckParam::kFilterTrigHold: + play.filter.trigEnv.holdFraction = clamp01(value); break; + case DeckParam::kFilterTrigDecay: + play.filter.trigEnv.decaySeconds = normToSec(value); break; + case DeckParam::kFilterTrigAttackCurve: + play.filter.trigEnv.attackCurve = util::curveFromKnobNorm(value); break; + case DeckParam::kFilterTrigDecayCurve: + play.filter.trigEnv.decayCurve = util::curveFromKnobNorm(value); break; + default: break; + } + // ONE normalization point for every control that can flip splineActive — a mode toggle + // (above) or an enable toggle (kPitchEnvEnable/kFilterEnable), whose enabling can make an + // already-Spline pitch/filter envelope newly active. Applying it once here, rather than at + // each site that could cause the flip, is what keeps a future such control from reopening + // the same hole. `resolvePlay` (sample_map.cpp) is the other caller of the shared helper. + enforceGateUnavailableWhileDrawn(play); +} + +void resetDeckParam(DeckParam id, PlaySeconds& play) { + const PlaySeconds defaults; + setDeckParam(id, play, deckParamNorm(id, defaults), 0); +} + +void formatEnvTimeMs(double seconds, char* buf, std::size_t len) { + if (!buf || len == 0) return; + const double ms = seconds * 1000.0; + std::snprintf(buf, len, ms < 10.0 ? "%.1f ms" : "%.0f ms", ms); +} + +} // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/deck_values.h b/src/core/instrument/ui/deck_values.h new file mode 100644 index 0000000..98c61cf --- /dev/null +++ b/src/core/instrument/ui/deck_values.h @@ -0,0 +1,52 @@ +// deck_values.h — the deck's control-id <-> parameter-set BINDING and its display units: the +// normalized 0..1 a knob shows, the write back into the stored seconds/fractions/positions, the +// double-click reset, and the ms time-constant formatter. Split from the editor shell so the +// whole domain map is provable without a host; deck_groups owns WHICH controls exist, this owns +// what each one's value MEANS. Links the map layer because PlaySeconds is what a deck edits. + +#pragma once + +#include + +#include "core/instrument/map/sample_map.h" // PlaySeconds (the deck's edit target) +#include "core/instrument/ui/deck_groups.h" // DeckParam +#include "core/instrument/ui/envelope_overlay.h" // kGateStageMaxSeconds + +namespace reasampler::instrument::ui { + +using map::PlaySeconds; + +// Every stage-time knob spans [0, kEnvTimeMaxSeconds] seconds — rate-free, exactly what the +// parameter set stores. READ from the overlay's schematic scale rather than restated: the AHDSR +// schematic anchors a maxed knob at the canvas edge, which only holds while the two agree. +inline constexpr double kEnvTimeMaxSeconds = kGateStageMaxSeconds; + +// Pitch depth throw: +/-kVelocityPitchRangeSemitones, centred. The one throw the pitch +// envelope's peak and the velocity->pitch curve's full scale both speak (play_params.h). +inline constexpr double kPitchDepthMaxSemis = kVelocityPitchRangeSemitones; + +// Key-track knob ceiling (0..200%), shared by the pitch and filter key-track controls. +inline constexpr double kKeyTrackMax = 2.0; + +// The normalized [0,1] a control shows: seconds over the ceiling, levels and fractions as-is, +// signed depths centred at 0.5, curve exponents over their logarithmic travel. Controls backed +// by per-instance state rather than the parameter set (voice count, master gain, the pitch +// key-track scalar, preview velocity) are not here — the shell reads those from the processor. +double deckParamNorm(DeckParam id, const PlaySeconds& play); + +// Applies a committed interaction: a knob's normalized `value`, or a toggle's `segment` (0/1). +// Mutates `play` in place, touching exactly the one field the control names. +void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment); + +// Resets `id` to its default. The default IS what a fresh PlaySeconds carries, so there is no +// second table of defaults to drift from the real one. Exact for every shipped default: they +// are all 0, 1, or the curve neutral, and the seconds ceiling is a power of two, so the +// norm round trip loses nothing. For knob-valued controls — a toggle has no reset gesture. +void resetDeckParam(DeckParam id, PlaySeconds& play); + +// A time constant as MILLISECONDS, e.g. "12 ms". Never switches to seconds: the editor reads in +// one unit so two stage times are comparable at a glance. Sub-10 ms keeps one decimal so a short +// attack is not rounded to a bare "0 ms". Writes at most `len` bytes including the terminator. +void formatEnvTimeMs(double seconds, char* buf, std::size_t len); + +} // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/knob_deck.cpp b/src/core/instrument/ui/knob_deck.cpp index 3327627..f2aabf3 100644 --- a/src/core/instrument/ui/knob_deck.cpp +++ b/src/core/instrument/ui/knob_deck.cpp @@ -190,4 +190,28 @@ DeckHit hitTestDeck(const DeckLayout& layout, int x, int y) { return {}; } +namespace { +// Squared distance from a rect's centre, in the rect's own pixel units. +double distSqFromCentre(const Rect& r, int x, int y) { + const double dx = x - (r.x + r.width / 2.0); + const double dy = y - (r.y + r.height / 2.0); + return dx * dx + dy * dy; +} +} // namespace + +DeckFaceHit hitTestKnobFace(const DeckLayout& layout, int x, int y) { + for (const DeckGroupLayout& g : layout.groups) { + if (!contains(g.box, x, y)) continue; + for (const DeckCellLayout& c : g.cells) { + const double rOuter = c.knob.width / 2.0; + if (distSqFromCentre(c.knob, x, y) >= rOuter * rOuter) continue; + const double rInner = c.inner.width / 2.0; + const bool inner = distSqFromCentre(c.inner, x, y) < rInner * rInner; + return {c.id, inner}; + } + return {}; // inside the group but off every dial + } + return {}; +} + } // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/knob_deck.h b/src/core/instrument/ui/knob_deck.h index af371c7..ab01f54 100644 --- a/src/core/instrument/ui/knob_deck.h +++ b/src/core/instrument/ui/knob_deck.h @@ -22,11 +22,13 @@ namespace reasampler::instrument::ui { -// Fixed deck metrics, exposed so the shell and tests agree. -inline constexpr int kDeckCellW = 48; // one knob cell -inline constexpr int kDeckCellH = 58; -inline constexpr int kDeckKnobSize = 28; // knob diameter inside the cell -inline constexpr int kDeckCellLabelH = 12; // the Micro label band under the knob +// Fixed deck metrics, exposed so the shell and tests agree. The cell/knob/label sizes were +// raised together for legibility at high pixel densities; the editor's floor width +// (sample_bands) is what absorbs the wider cells, so the two move as a pair. +inline constexpr int kDeckCellW = 60; // one knob cell +inline constexpr int kDeckCellH = 74; +inline constexpr int kDeckKnobSize = 40; // knob diameter inside the cell +inline constexpr int kDeckCellLabelH = 16; // the label band under the knob inline constexpr int kDeckCaptionH = 20; // the group caption row inline constexpr int kDeckToggleH = 18; // compact toggle segment height inline constexpr int kDeckGroupPadX = 6; // group box horizontal inner padding @@ -39,7 +41,7 @@ inline constexpr int kDeckRadioSize = 12; // the caption-row corner radio sq // The knob cell's INNER dial: a concentric sub-disc that edits a second, related value while // the outer ring keeps editing the cell's own. Geometry only — WHICH cells carry one is // deck_groups' call, so a cell without an inner value simply resolves an inner hit as a knob. -inline constexpr int kDeckInnerDialSize = 14; +inline constexpr int kDeckInnerDialSize = 20; // One group box: padding + caption + gap + cell row + padding. inline constexpr int kDeckGroupH = kDeckGroupPadY + kDeckCaptionH + kDeckCaptionGap + kDeckCellH + kDeckGroupPadY; @@ -145,4 +147,17 @@ struct DeckHit { // the caption-row corner radio. Everything else — fence, padding, outside — misses. DeckHit hitTestDeck(const DeckLayout& layout, int x, int y); +// The knob FACE a point lands on. id -1 is a miss. +struct DeckFaceHit { + int id = -1; + bool inner = false; // inside the concentric inner disc +}; + +// Resolved against the drawn CIRCLES, not the cell: a reset is aimed at a dial, so the label +// band and the cell margins must miss where a drag grab deliberately does not. Both radii are +// boundary-EXCLUSIVE, one rule for both rings — a point exactly on the inner radius is an +// outer-ring hit, one exactly on the outer radius is a miss. Whether a cell actually carries an +// inner value is deck_groups' call, exactly as with DeckHit::inner. +DeckFaceHit hitTestKnobFace(const DeckLayout& layout, int x, int y); + } // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/sample_bands.h b/src/core/instrument/ui/sample_bands.h index ef5c812..3210520 100644 --- a/src/core/instrument/ui/sample_bands.h +++ b/src/core/instrument/ui/sample_bands.h @@ -16,13 +16,13 @@ inline constexpr int kPad = 8; // exactly this, and there is no scroll, so anything smaller pushes the deck band off the // window bottom (computeSampleBands' waveform-floor-wins degrade). Growing is fine — the // waveform band is the elastic one. Both the enforced minimum and the opening rect read this. -inline constexpr int kEditorMinWidth = 840; -inline constexpr int kEditorMinHeight = 620; +inline constexpr int kEditorMinWidth = 980; +inline constexpr int kEditorMinHeight = 680; // Chrome band: the toolbar row (title + nav) stacked over the control row (piano strip, // preview, velocity knob, channel toggle). sample_chrome partitions it. inline constexpr int kTitleHeight = 26; -inline constexpr int kChromeRowHeight = 52; +inline constexpr int kChromeRowHeight = 64; // Waveform band floor: two stacked lanes plus the seam between them. The band never shrinks // below this — a window too short for it clips the bands beneath instead, so the waveform diff --git a/src/core/instrument/ui/sample_chrome.cpp b/src/core/instrument/ui/sample_chrome.cpp index cd93273..590a31b 100644 --- a/src/core/instrument/ui/sample_chrome.cpp +++ b/src/core/instrument/ui/sample_chrome.cpp @@ -13,14 +13,14 @@ namespace { // The toolbar row carries the whole control run, so it is taller than the Browse modal's // plain kTitleHeight bar — the velocity knob cell (knob over label) sets the floor. Both // rows still fit the band the allocator hands out (kTitleHeight + kChromeRowHeight). -constexpr int kToolbarHeight = 44; +constexpr int kToolbarHeight = 58; constexpr int kStripBandHeight = 30; constexpr int kRunGap = 6; // between adjacent items of the toolbar run constexpr int kChanSegW = 52; constexpr int kChanSegH = 18; -constexpr int kVelCellW = 44; -constexpr int kVelLabelH = 12; +constexpr int kVelCellW = 56; +constexpr int kVelLabelH = 16; constexpr int kPreviewBtnW = 64; constexpr int kRunButtonH = 24; // Browse and Preview diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index a2b3b55..c71c743 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -11,7 +11,7 @@ The pure engine/geometry core this shell wraps (`sampler_core`, `pitch_shift`, `sample_map`, `component_state_io`, `play_params.h`, `editor_geometry`, `sample_bands`, `sample_chrome`, `keyboard_strip`, `waveform_view`, `capture_browser`, `browser_scroll`, `param_slider`, `trigger_seam`, `velocity_curve`, `embed_strip`, `knob_deck`, -`deck_groups`, `curve_popup`, `spline_edit`, `master_gain`, `reasampler_uid.h`) lives in `core/instrument/*` and +`deck_groups`, `deck_values`, `curve_popup`, `spline_edit`, `master_gain`, `reasampler_uid.h`) lives in `core/instrument/*` and `core/wire` and is documented there — this directory consumes it but does not own it. ## Invariants @@ -101,7 +101,7 @@ declared ahead of the instrument slots at that member in `reasampler_processor.h - `reaper_bridge` — READ-ONLY bank consumer: receives bank snapshots from the extension and exposes them as a read-only view. **Never writes to the extension's bank** — this is a load-bearing invariant; no mutation path exists in this module. **pS-usage:** gains `writeUsageExtState` (prefix-guarded — accepts only `rsusage_`-prefixed keys, refuses all others) so the processor can publish usage without weakening the read-only-bank invariant. - `reasampler_processor` (`shell/instrument/`: `reasampler_processor.cpp` lifecycle + `process()`, `processor_state.cpp` component-state I/O + UI-thread parameter accessors, `processor_reload.cpp` the off-audio-thread `reloadInstrument`/publish family — Q-W2v, T4-12 split; `process()` and its per-block work stay ONE TU on purpose, no cross-TU call on the per-sample path) — VST3 `SingleComponentEffect` shell: declares event-input bus + **permanently stereo** output (GA fix: dynamic mono↔stereo bus renegotiation deleted; `ChannelMode` is now decode-only), marshals MIDI note-on/off into the VoiceEngine, renders audio; owns off-audio-thread `reloadInstrument` + atomic pointer swap so `process()` does no allocation, no file I/O, no bridge calls. The instance state is `{loaded capture id, one InstrumentParams}`, and `reloadInstrument` resolves + decodes exactly that one capture into the `SampleData` the engine plays. **Self-contained playback (pS):** `ComponentState` v10 adds a `SampleRefs` table — per referenced sample, a project-relative path + decode intrinsics (root, loop, channels, displayName); `reloadInstrument` decodes directly from `SampleRefs`, bank-free (plays with the extension absent). The bank/bridge is a browser source: loading a capture copies its reference in; the reopen-heal timer + poll-to-play apparatus are removed. `retireIdleDrain()` retires fully-idle drain snapshots on the UI-timer cadence. Voice-param edits (`setVoiceCount`/`setVoiceMode`/`setMonoTrigger`) rebuild the engine from the already-decoded `SampleData` via the drain-slot swap — no bank re-read, no WAV re-decode, no audible cut to ringing tails. **FB1:** applies the post-mixer `masterGainLinear` (from `ComponentState` v8) as a per-sample ramp over the summed output — no zipper noise. **GA v9:** `channelModeExplicit_` flag persisted; `channelModeFor()` auto-defaults the mode from the loaded capture's channel count when the flag is not set. **pS:** `ComponentState` bumped v9→v10 (`SampleRefs` table); pre-v10 blobs lift to empty refs and re-save self-contained. **pS-usage:** publishes instance usage (held `SampleRefs` paths) to `rsusage_` at the tail of `reloadInstrument` (off audio thread) via `reaper_bridge::writeUsageExtState`; `ComponentState` bumped v10→**v11** (`instanceGuid` field); pre-v11 blobs mint guid on first publish. -- `reasampler_editor` — VST3 `IPlugView` LICE editor shell: hosts a LICE-drawn child window; the Sample face is home and Browse is a modal picker over it. Split on the Sample face's BAND axis, mirroring the pure `sample_bands` allocator: `editor_session` (session/bridge state, caches, commit-and-reload), `editor_controls` (the control-value domain maps + the node-drag bounds that must match them, plus the ONE `faceLayout` band resolve every paint and hit-test path shares), `editor_models` (the orthogonal half: which stored struct each transient editor selection names — the staged-envelope pack/unpack, the drawn contour, and the three velocity curves), then matching paint and input sets — `editor_paint`/`editor_input` (dispatch + drag router + hover dispatch), `_chrome`, `_waveform`, `_deck` — plus the two band-independent surfaces (`_browse` for the modal picker, `_curve` for the velocity-curve popup) and `editor_platform` (IPlugView/Win32 window plumbing). Shared internals in `editor_internal.h`, no TU of its own. Drop-onto-editor ingest is NOT shipped (deferred). +- `reasampler_editor` — VST3 `IPlugView` LICE editor shell: hosts a LICE-drawn child window; the Sample face is home and Browse is a modal picker over it. Split on the Sample face's BAND axis, mirroring the pure `sample_bands` allocator: `editor_session` (session/bridge state, caches, commit-and-reload), `editor_controls` (the ONE `faceLayout` band resolve every paint and hit-test path shares, the node-drag bounds, the value labels, and the per-instance controls the parameter set does not carry — the parameter-set binding itself is the pure `core/instrument/ui/deck_values` module this only adapts int ids onto), `editor_models` (the orthogonal half: which stored struct each transient editor selection names — the staged-envelope pack/unpack, the drawn contour, and the three velocity curves), then matching paint and input sets — `editor_paint`/`editor_input` (dispatch + drag router + hover dispatch), `_chrome`, `_waveform`, `_deck` — plus the two band-independent surfaces (`_browse` for the modal picker, `_curve` for the velocity-curve popup) and `editor_platform` (IPlugView/Win32 window plumbing). Shared internals in `editor_internal.h`, no TU of its own. Drop-onto-editor ingest is NOT shipped (deferred). - `reasampler_embed` — implements `IReaperUIEmbedInterface` so the instrument draws inline in the TCP/MCP without a plugin-owned HWND; delegates layout to `embed_strip`. A read-only readout: the loaded capture across the keyboard span with its root marked, plus the activity level. It takes no mouse input (there is nothing on the strip to select). - `vst_entry` — VST3 entry point: `GetPluginFactory` export, class registration, channel-forked class UIDs. - `editor_internal.h` — INTERNAL shared helpers for the `reasampler_editor` TU family, included only by the editor's own shell TUs (`editor_session` / `editor_controls` / `editor_paint_*` / `editor_input_*` / `editor_platform`), never a public seam: the `Rect`↔kit adapters, small draw primitives (knob face / title band), label helpers, and the velocity-curve box derivation — the helpers more than one band TU needs. The deck's control ids, group ids and group composition are the pure `deck_groups` module's, not this file's. The piano-strip and root-key draws live in `editor_paint_chrome`, their only consumer, not here. @@ -111,6 +111,11 @@ declared ahead of the instrument slots at that member in `reasampler_processor.h - `editor_internal.h` is include-only — it has no TU of its own and must never become a public seam; only the `reasampler_editor` band-axis TUs include it. +- **The editor window class carries `CS_DBLCLKS`, which REPLACES the second button-down of + a double-click** with `WM_?BUTTONDBLCLK`. Every surface that counts two downs — Browse's + load accelerator, the spline surfaces' right-click delete — survives only because both + DBLCLK handlers fall through to the ordinary down handler. Adding a new double-click + consumer means preserving that fall-through, not bypassing it. - The two VST3 class UIDs (`core/wire/reasampler_uid.h`, consumed via `reasampler_vst.h`) are FOREVER-FROZEN — never regenerate an already-shipped UID. - The UID selection `#ifdef` in `reasampler_vst.h` is the one deliberate exception to diff --git a/src/shell/instrument/CMakeLists.txt b/src/shell/instrument/CMakeLists.txt index 6bb35d9..6309fb1 100644 --- a/src/shell/instrument/CMakeLists.txt +++ b/src/shell/instrument/CMakeLists.txt @@ -86,7 +86,8 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") capture_browser keyboard_strip sample_bands sample_chrome waveform_view bank_sync browser_scroll param_slider tooltip theme component_geometry bank_grid trigger_seam envelope_overlay envelope_edit - knob_deck deck_groups curve_popup spline_edit master_gain sample_usage file_bytes curve_law) + knob_deck deck_groups deck_values curve_popup spline_edit master_gain sample_usage + file_bytes curve_law) # SDK_INC gives the REAPER VST3 interfaces + API header for the bridge; WDL_INC gives # LICE for the editor. The VST3 SDK headers arrive via vst3_sdk PUBLIC. target_include_directories(reasampler_vst PRIVATE ${REASAMPLER_SRC_DIR} ${SDK_INC} ${WDL_INC}) diff --git a/src/shell/instrument/editor_controls.cpp b/src/shell/instrument/editor_controls.cpp index 1a1126d..7290124 100644 --- a/src/shell/instrument/editor_controls.cpp +++ b/src/shell/instrument/editor_controls.cpp @@ -1,9 +1,9 @@ // editor_controls.cpp — the ReaSamplerEditor's parameter plumbing: the band-stack layout -// resolve every paint/hit-test path shares, the control-value domain maps (controlValue / -// applyControl — seconds/fraction/frames <-> normalized 0..1), the control-id<->value binding -// against the pure `deck_groups` module's descriptors, and the node-drag clamp bounds that must -// match those domains. The orthogonal half — which stored struct each editor selection names — -// is editor_models. Value logic only: no painting, no window plumbing. +// resolve every paint/hit-test path shares, the shell's half of the control-value binding (the +// per-instance controls the parameter set does not carry — key-track, voice count, master gain, +// preview velocity — plus the value labels), and the node-drag clamp bounds. The parameter-set +// half is the pure `deck_values` module. The orthogonal half — which stored struct each editor +// selection names — is editor_models. Value logic only: no painting, no window plumbing. #include "shell/instrument/reasampler_editor.h" @@ -16,6 +16,7 @@ #include "core/instrument/engine/filter/filter_params.h" // the filter's own control laws #include "core/instrument/engine/master_gain.h" // master-gain dB<->linear<->knob taper #include "core/instrument/ui/deck_groups.h" // sampleDeckGroups (the deck's composition) +#include "core/instrument/ui/deck_values.h" // the parameter-set binding + its ms units #include "core/instrument/ui/knob_deck.h" // deckHeight / kDeckKnobSize (the band's own height) #include "core/util/clamp01.h" #include "core/util/curve_law.h" // the ONE curve-exponent domain @@ -30,32 +31,22 @@ using instrument::ui::chromeRects; using instrument::ui::deckHeight; using instrument::ui::kDeckKnobSize; using instrument::ui::kPad; -using instrument::ui::deckBipolarFromNorm; -using instrument::ui::deckNormFromBipolar; +using instrument::ui::deckParamNorm; +using instrument::ui::formatEnvTimeMs; +using instrument::ui::kEnvTimeMaxSeconds; +using instrument::ui::kKeyTrackMax; +using instrument::ui::resetDeckParam; using instrument::ui::sampleDeckGroups; +using instrument::ui::setDeckParam; using instrument::engine::formatMasterGainLabel; using instrument::engine::masterGainLinearFromNorm; using instrument::engine::masterGainNormFromLinear; -using instrument::engine::filter::MorphLaw; using instrument::engine::filter::filterCutoffHzFromNorm; using instrument::engine::filter::filterDriveDepthFromNorm; using instrument::engine::filter::filterQFromNorm; using util::clamp01; namespace { -// Control-surface value domains (the shell owns these — param_slider is engine-free and maps -// only 0..1). Every stage-time knob spans [0, kEnvTimeMaxSeconds] seconds — rate-free, exactly -// what the parameter set stores; the build resolves seconds->frames at the live rate. No knob -// on this surface stores a source-frame count any more, so none needs a rate to draw. -// -// The ceiling is READ from the overlay's schematic scale rather than restated: the AHDSR -// schematic anchors a maxed knob at the canvas edge, which only holds while the two agree. -constexpr double kEnvTimeMaxSeconds = instrument::ui::kGateStageMaxSeconds; -// Pitch depth throw: +/-kVelocityPitchRangeSemitones, centered. The one throw the pitch -// envelope's peak and the velocity->pitch curve's full scale both speak (play_params.h). -constexpr double kPitchDepthMaxSemis = kVelocityPitchRangeSemitones; -constexpr double kKeyTrackMax = 2.0; // key-track slider ceiling (0..200%) - // The raw stored curve exponent for a curve-dial control id, read DIRECTLY off the field — // never round-tripped through curveFromKnobNorm(knobNormFromCurve(x)): the knob-norm law's // centre detent (curve_law.h) snaps anything near-neutral back to exactly 1.0, so a round trip @@ -91,191 +82,25 @@ ReaSamplerEditor::FaceLayout ReaSamplerEditor::faceLayout(int w, int h) const { return fl; } +// The parameter-set binding is the pure deck_values module's; these three are the shell's thin +// int-id adapters onto it (ParamControl is an alias of DeckParam). double ReaSamplerEditor::controlValue(int id, const PlaySeconds& play) const { - // Wall-clock seconds -> normalized over the seconds ceiling; fractions and normalized - // control positions pass through; curve exponents take the log travel. - const auto secToNorm = [](double s) { return clamp01(s / kEnvTimeMaxSeconds); }; - switch (static_cast(id)) { - case ParamControl::kPlayMode: return play.playMode == PlayMode::Trigger ? 1.0 : 0.0; - case ParamControl::kAmpEnvMode: - return play.ampSpline.mode == EnvMode::Spline ? 1.0 : 0.0; - case ParamControl::kPitchEnvMode: - return play.pitchSpline.mode == EnvMode::Spline ? 1.0 : 0.0; - case ParamControl::kFilterEnvMode: - return play.filterSpline.mode == EnvMode::Spline ? 1.0 : 0.0; - case ParamControl::kPitchEngine: return play.pitchEngine == PitchEngine::Preserve ? 1.0 : 0.0; - case ParamControl::kAttack: return secToNorm(play.adsr.attackSeconds); - case ParamControl::kHold: return secToNorm(play.adsr.holdSeconds); - case ParamControl::kDecay: return secToNorm(play.adsr.decaySeconds); - case ParamControl::kSustain: return clamp01(play.adsr.sustainLevel); - case ParamControl::kRelease: return secToNorm(play.adsr.releaseSeconds); - case ParamControl::kAttackCurve: return util::knobNormFromCurve(play.adsr.attackCurve); - case ParamControl::kDecayCurve: return util::knobNormFromCurve(play.adsr.decayCurve); - case ParamControl::kReleaseCurve: return util::knobNormFromCurve(play.adsr.releaseCurve); - case ParamControl::kTrigLength: return clamp01(play.trigger.lengthFraction); - case ParamControl::kTrigAttack: return secToNorm(play.trigAhd.attackSeconds); - case ParamControl::kTrigHold: return clamp01(play.trigAhd.holdFraction); - case ParamControl::kTrigDecay: return secToNorm(play.trigAhd.decaySeconds); - case ParamControl::kTrigAttackCurve: return util::knobNormFromCurve(play.trigAhd.attackCurve); - case ParamControl::kTrigDecayCurve: return util::knobNormFromCurve(play.trigAhd.decayCurve); - case ParamControl::kPitchEnvEnable:return play.pitchEnv.enabled ? 1.0 : 0.0; - case ParamControl::kPitchEnvAttack:return secToNorm(play.pitchEnv.shape.attackSeconds); - case ParamControl::kPitchEnvHold: return clamp01(play.pitchEnv.shape.holdFraction); - case ParamControl::kPitchEnvDecay: return secToNorm(play.pitchEnv.shape.decaySeconds); - case ParamControl::kPitchEnvAttackCurve: - return util::knobNormFromCurve(play.pitchEnv.shape.attackCurve); - case ParamControl::kPitchEnvDecayCurve: - return util::knobNormFromCurve(play.pitchEnv.shape.decayCurve); - case ParamControl::kPitchEnvDepth: - // Signed depth centered at 0.5 (0.5 == 0 semitones). - return clamp01(0.5 + play.pitchEnv.peakSemitones / (2.0 * kPitchDepthMaxSemis)); - // Filter. The four tone controls ARE the module's normalized positions — stored and - // shown as-is, so the knob travel is exactly filter_params' own law. - case ParamControl::kFilterEnable: return play.filter.enabled ? 1.0 : 0.0; - case ParamControl::kFilterLaw: - return play.filter.settings.morphLaw == MorphLaw::HighNotchLow ? 1.0 : 0.0; - case ParamControl::kFilterMorph: return clamp01(play.filter.settings.morphNorm); - case ParamControl::kFilterCutoff: return clamp01(play.filter.settings.cutoffNorm); - case ParamControl::kFilterQ: return clamp01(play.filter.settings.resonanceNorm); - case ParamControl::kFilterDrive: return clamp01(play.filter.settings.driveNorm); - case ParamControl::kFilterModAmt: return deckNormFromBipolar(play.filter.modAmount); - case ParamControl::kFilterVel: return deckNormFromBipolar(play.filter.velAmount); - case ParamControl::kFilterKeyTrack:return clamp01(play.filter.keyTrack / kKeyTrackMax); - case ParamControl::kFilterEnvAttack: return secToNorm(play.filter.env.attackSeconds); - case ParamControl::kFilterEnvHold: return secToNorm(play.filter.env.holdSeconds); - case ParamControl::kFilterEnvDecay: return secToNorm(play.filter.env.decaySeconds); - case ParamControl::kFilterEnvSustain: return clamp01(play.filter.env.sustainLevel); - case ParamControl::kFilterEnvRelease: return secToNorm(play.filter.env.releaseSeconds); - case ParamControl::kFilterEnvAttackCurve: - return util::knobNormFromCurve(play.filter.env.attackCurve); - case ParamControl::kFilterEnvDecayCurve: - return util::knobNormFromCurve(play.filter.env.decayCurve); - case ParamControl::kFilterEnvReleaseCurve: - return util::knobNormFromCurve(play.filter.env.releaseCurve); - case ParamControl::kFilterTrigAttack: return secToNorm(play.filter.trigEnv.attackSeconds); - case ParamControl::kFilterTrigHold: return clamp01(play.filter.trigEnv.holdFraction); - case ParamControl::kFilterTrigDecay: return secToNorm(play.filter.trigEnv.decaySeconds); - case ParamControl::kFilterTrigAttackCurve: - return util::knobNormFromCurve(play.filter.trigEnv.attackCurve); - case ParamControl::kFilterTrigDecayCurve: - return util::knobNormFromCurve(play.filter.trigEnv.decayCurve); - default: return 0.0; - } + return deckParamNorm(static_cast(id), play); } void ReaSamplerEditor::applyControl(int id, PlaySeconds& play, double value, int segment) const { - const auto normToSec = [](double v) { return clamp01(v) * kEnvTimeMaxSeconds; }; - switch (static_cast(id)) { - case ParamControl::kPlayMode: - // Gate is refused while any EG is drawn — see splineActive (play_params.h). The - // segment paints Disabled for the same reason, so the refusal is never a surprise. - if (segment == 0 && splineActive(play)) break; - play.playMode = (segment == 1) ? PlayMode::Trigger : PlayMode::Gate; - break; - // Switching TO Spline drops Gate, which the spline model has no place for. Switching - // back does NOT restore it: the previous mode is not stored, and silently re-gating an - // instrument the user has since heard as a one-shot is the worse surprise. - case ParamControl::kAmpEnvMode: - case ParamControl::kPitchEnvMode: - case ParamControl::kFilterEnvMode: { - const EnvMode m = (segment == 1) ? EnvMode::Spline : EnvMode::Staged; - switch (static_cast(id)) { - case ParamControl::kAmpEnvMode: play.ampSpline.mode = m; break; - case ParamControl::kPitchEnvMode: play.pitchSpline.mode = m; break; - default: play.filterSpline.mode = m; break; - } - break; - } - case ParamControl::kPitchEngine: - play.pitchEngine = (segment == 1) ? PitchEngine::Preserve : PitchEngine::Varispeed; - break; - case ParamControl::kAttack: play.adsr.attackSeconds = normToSec(value); break; - case ParamControl::kHold: play.adsr.holdSeconds = normToSec(value); break; - case ParamControl::kDecay: play.adsr.decaySeconds = normToSec(value); break; - case ParamControl::kSustain: play.adsr.sustainLevel = clamp01(value); break; - case ParamControl::kRelease: play.adsr.releaseSeconds = normToSec(value); break; - case ParamControl::kAttackCurve: play.adsr.attackCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kDecayCurve: play.adsr.decayCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kReleaseCurve: play.adsr.releaseCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kTrigLength: - // lengthFraction is (0,1]; keep a small floor so a zero-length trigger never plays nothing. - play.trigger.lengthFraction = (std::max)(0.01, clamp01(value)); - break; - case ParamControl::kTrigAttack: play.trigAhd.attackSeconds = normToSec(value); break; - case ParamControl::kTrigHold: play.trigAhd.holdFraction = clamp01(value); break; - case ParamControl::kTrigDecay: play.trigAhd.decaySeconds = normToSec(value); break; - case ParamControl::kTrigAttackCurve: - play.trigAhd.attackCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kTrigDecayCurve: - play.trigAhd.decayCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kPitchEnvEnable: - play.pitchEnv.enabled = (segment == 1); - break; - case ParamControl::kPitchEnvAttack: - play.pitchEnv.shape.attackSeconds = normToSec(value); break; - case ParamControl::kPitchEnvHold: - play.pitchEnv.shape.holdFraction = clamp01(value); break; - case ParamControl::kPitchEnvDecay: - play.pitchEnv.shape.decaySeconds = normToSec(value); break; - case ParamControl::kPitchEnvAttackCurve: - play.pitchEnv.shape.attackCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kPitchEnvDecayCurve: - play.pitchEnv.shape.decayCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kPitchEnvDepth: - play.pitchEnv.peakSemitones = (clamp01(value) - 0.5) * 2.0 * kPitchDepthMaxSemis; - break; - case ParamControl::kFilterEnable: play.filter.enabled = (segment == 1); break; - case ParamControl::kFilterLaw: - play.filter.settings.morphLaw = - (segment == 1) ? MorphLaw::HighNotchLow : MorphLaw::HighBandLow; - break; - case ParamControl::kFilterMorph: - play.filter.settings.morphNorm = static_cast(clamp01(value)); break; - case ParamControl::kFilterCutoff: - play.filter.settings.cutoffNorm = static_cast(clamp01(value)); break; - case ParamControl::kFilterQ: - play.filter.settings.resonanceNorm = static_cast(clamp01(value)); break; - case ParamControl::kFilterDrive: - play.filter.settings.driveNorm = static_cast(clamp01(value)); break; - case ParamControl::kFilterModAmt: play.filter.modAmount = deckBipolarFromNorm(value); break; - case ParamControl::kFilterVel: play.filter.velAmount = deckBipolarFromNorm(value); break; - case ParamControl::kFilterKeyTrack: - play.filter.keyTrack = clamp01(value) * kKeyTrackMax; break; - case ParamControl::kFilterEnvAttack: - play.filter.env.attackSeconds = normToSec(value); break; - case ParamControl::kFilterEnvHold: - play.filter.env.holdSeconds = normToSec(value); break; - case ParamControl::kFilterEnvDecay: - play.filter.env.decaySeconds = normToSec(value); break; - case ParamControl::kFilterEnvSustain: - play.filter.env.sustainLevel = clamp01(value); break; - case ParamControl::kFilterEnvRelease: - play.filter.env.releaseSeconds = normToSec(value); break; - case ParamControl::kFilterEnvAttackCurve: - play.filter.env.attackCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kFilterEnvDecayCurve: - play.filter.env.decayCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kFilterEnvReleaseCurve: - play.filter.env.releaseCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kFilterTrigAttack: - play.filter.trigEnv.attackSeconds = normToSec(value); break; - case ParamControl::kFilterTrigHold: - play.filter.trigEnv.holdFraction = clamp01(value); break; - case ParamControl::kFilterTrigDecay: - play.filter.trigEnv.decaySeconds = normToSec(value); break; - case ParamControl::kFilterTrigAttackCurve: - play.filter.trigEnv.attackCurve = util::curveFromKnobNorm(value); break; - case ParamControl::kFilterTrigDecayCurve: - play.filter.trigEnv.decayCurve = util::curveFromKnobNorm(value); break; - default: break; + setDeckParam(static_cast(id), play, value, segment); +} + +void ReaSamplerEditor::resetParamControl(int id) { + // kKeyTrack is the one knob whose value sits beside the play bundle, so it defaults from + // InstrumentParams rather than from PlaySeconds — the same split applyParamControl draws. + if (id == static_cast(ParamControl::kKeyTrack)) { + params_.keyTrack = InstrumentParams{}.keyTrack; + return; } - // ONE normalization point for every control that can flip splineActive — a mode toggle - // (above) or an enable toggle (kPitchEnvEnable/kFilterEnable), whose enabling can make an - // already-Spline pitch/filter envelope newly active. Applying it once here, rather than at - // each site that could cause the flip, is what keeps a future such control from reopening - // the same hole. `resolvePlay` (sample_map.cpp) is the other caller of the shared helper. - enforceGateUnavailableWhileDrawn(play); + resetDeckParam(static_cast(id), params_.play); } double ReaSamplerEditor::liveSampleRate() const { @@ -348,29 +173,29 @@ std::string ReaSamplerEditor::deckValueLabel(int id) const { const PlaySeconds& play = params_.play; switch (id == -2 ? ParamControl::kCount : static_cast(id)) { case ParamControl::kAttack: - snprintf(buf, sizeof(buf), "%.3fs", play.adsr.attackSeconds); break; + formatEnvTimeMs(play.adsr.attackSeconds, buf, sizeof(buf)); break; case ParamControl::kHold: - snprintf(buf, sizeof(buf), "%.3fs", play.adsr.holdSeconds); break; + formatEnvTimeMs(play.adsr.holdSeconds, buf, sizeof(buf)); break; case ParamControl::kDecay: - snprintf(buf, sizeof(buf), "%.3fs", play.adsr.decaySeconds); break; + formatEnvTimeMs(play.adsr.decaySeconds, buf, sizeof(buf)); break; case ParamControl::kSustain: snprintf(buf, sizeof(buf), "%.0f%%", play.adsr.sustainLevel * 100.0); break; case ParamControl::kRelease: - snprintf(buf, sizeof(buf), "%.3fs", play.adsr.releaseSeconds); break; + formatEnvTimeMs(play.adsr.releaseSeconds, buf, sizeof(buf)); break; case ParamControl::kTrigLength: snprintf(buf, sizeof(buf), "%.0f%%", play.trigger.lengthFraction * 100.0); break; case ParamControl::kTrigAttack: - snprintf(buf, sizeof(buf), "%.3fs", play.trigAhd.attackSeconds); break; + formatEnvTimeMs(play.trigAhd.attackSeconds, buf, sizeof(buf)); break; case ParamControl::kTrigHold: snprintf(buf, sizeof(buf), "%.0f%%", play.trigAhd.holdFraction * 100.0); break; case ParamControl::kTrigDecay: - snprintf(buf, sizeof(buf), "%.3fs", play.trigAhd.decaySeconds); break; + formatEnvTimeMs(play.trigAhd.decaySeconds, buf, sizeof(buf)); break; case ParamControl::kPitchEnvAttack: - snprintf(buf, sizeof(buf), "%.3fs", play.pitchEnv.shape.attackSeconds); break; + formatEnvTimeMs(play.pitchEnv.shape.attackSeconds, buf, sizeof(buf)); break; case ParamControl::kPitchEnvHold: snprintf(buf, sizeof(buf), "%.0f%%", play.pitchEnv.shape.holdFraction * 100.0); break; case ParamControl::kPitchEnvDecay: - snprintf(buf, sizeof(buf), "%.3fs", play.pitchEnv.shape.decaySeconds); break; + formatEnvTimeMs(play.pitchEnv.shape.decaySeconds, buf, sizeof(buf)); break; case ParamControl::kPitchEnvDepth: snprintf(buf, sizeof(buf), "%+.1fst", play.pitchEnv.peakSemitones); break; case ParamControl::kKeyTrack: @@ -407,21 +232,21 @@ std::string ReaSamplerEditor::deckValueLabel(int id) const { case ParamControl::kFilterKeyTrack: snprintf(buf, sizeof(buf), "%.0f%%", play.filter.keyTrack * 100.0); break; case ParamControl::kFilterEnvAttack: - snprintf(buf, sizeof(buf), "%.3fs", play.filter.env.attackSeconds); break; + formatEnvTimeMs(play.filter.env.attackSeconds, buf, sizeof(buf)); break; case ParamControl::kFilterEnvHold: - snprintf(buf, sizeof(buf), "%.3fs", play.filter.env.holdSeconds); break; + formatEnvTimeMs(play.filter.env.holdSeconds, buf, sizeof(buf)); break; case ParamControl::kFilterEnvDecay: - snprintf(buf, sizeof(buf), "%.3fs", play.filter.env.decaySeconds); break; + formatEnvTimeMs(play.filter.env.decaySeconds, buf, sizeof(buf)); break; case ParamControl::kFilterEnvSustain: snprintf(buf, sizeof(buf), "%.0f%%", play.filter.env.sustainLevel * 100.0); break; case ParamControl::kFilterEnvRelease: - snprintf(buf, sizeof(buf), "%.3fs", play.filter.env.releaseSeconds); break; + formatEnvTimeMs(play.filter.env.releaseSeconds, buf, sizeof(buf)); break; case ParamControl::kFilterTrigAttack: - snprintf(buf, sizeof(buf), "%.3fs", play.filter.trigEnv.attackSeconds); break; + formatEnvTimeMs(play.filter.trigEnv.attackSeconds, buf, sizeof(buf)); break; case ParamControl::kFilterTrigHold: snprintf(buf, sizeof(buf), "%.0f%%", play.filter.trigEnv.holdFraction * 100.0); break; case ParamControl::kFilterTrigDecay: - snprintf(buf, sizeof(buf), "%.3fs", play.filter.trigEnv.decaySeconds); break; + formatEnvTimeMs(play.filter.trigEnv.decaySeconds, buf, sizeof(buf)); break; // Every curve exponent reads the same way: the neutral shows as 1.00. case ParamControl::kAttackCurve: case ParamControl::kDecayCurve: diff --git a/src/shell/instrument/editor_input.cpp b/src/shell/instrument/editor_input.cpp index 2396a14..ae7c8db 100644 --- a/src/shell/instrument/editor_input.cpp +++ b/src/shell/instrument/editor_input.cpp @@ -40,6 +40,19 @@ void ReaSamplerEditor::onMouseDown(int x, int y) { mouseDownWaveform(fl, x, y); } +bool ReaSamplerEditor::onMouseDoubleClick(int x, int y) { + // Only the Sample face's radial knobs claim the gesture. Everything else — Browse's + // load accelerator, the spline surfaces' point grammar — is left to the caller's + // fall-through to onMouseDown, which is the path those surfaces were built on. + if (!processor_ || view_ != View::kSample || curvePopup_ != CurveTarget::kNone) return false; + RECT cr{}; + GetClientRect(childHwnd_, &cr); + const FaceLayout fl = faceLayout(cr.right - cr.left, cr.bottom - cr.top); + if (doubleClickChrome(fl, x, y)) return true; + if (selectedId_.empty()) return false; + return doubleClickDeck(fl, x, y); +} + void ReaSamplerEditor::onMouseMove(int x, int y) { if (drag_ == DragKind::kNone) return; dragCurX_ = x; // keep the live cursor position for drag-state draw cues (e.g. drag-off warn) diff --git a/src/shell/instrument/editor_input_chrome.cpp b/src/shell/instrument/editor_input_chrome.cpp index cb52885..932b3e6 100644 --- a/src/shell/instrument/editor_input_chrome.cpp +++ b/src/shell/instrument/editor_input_chrome.cpp @@ -80,6 +80,18 @@ bool ReaSamplerEditor::mouseDownChrome(const FaceLayout& fl, int x, int y) { return contains(cr.controls, x, y); } +bool ReaSamplerEditor::doubleClickChrome(const FaceLayout& fl, int x, int y) { + // The preview-velocity dial answers the same reset gesture the deck's knobs do — it is a + // radial knob drawn by the same primitive, so a user who learns the gesture there expects + // it here. Resolved against the whole cell, not the circle: the cell IS the knob's target + // on this surface (there is no neighbouring control to steal from). + if (selectedId_.empty() || !processor_) return false; + if (!contains(fl.chrome.velCell, x, y)) return false; + processor_->setPreviewVelocity(kPreviewVelocityDefault); + invalidate(); + return true; +} + void ReaSamplerEditor::dragChrome(const FaceLayout& fl, int x, int y) { const Rect& stripArea = fl.chrome.rootStrip; if (stripArea.empty()) return; diff --git a/src/shell/instrument/editor_input_deck.cpp b/src/shell/instrument/editor_input_deck.cpp index 46f1b3e..ae487c6 100644 --- a/src/shell/instrument/editor_input_deck.cpp +++ b/src/shell/instrument/editor_input_deck.cpp @@ -108,6 +108,43 @@ bool ReaSamplerEditor::mouseDownDeck(const FaceLayout& fl, int x, int y) { return true; } +bool ReaSamplerEditor::doubleClickDeck(const FaceLayout& fl, int x, int y) { + const Rect& band = fl.bands.decks; + if (!contains(band, x, y)) return false; + const DeckLayout dl = layoutDeck(fl.deckDescs, band.x, band.y, band.width); + // Resolved against the drawn CIRCLES (hitTestKnobFace), not the cell a drag grabs anywhere + // in: the two rings are concentric, so only a radial resolve can tell "reset the exponent" + // from "reset the value". + const DeckFaceHit hit = hitTestKnobFace(dl, x, y); + if (hit.id < 0) return false; + if (deckKnobDisabled(hit.id)) return true; // drawn-but-dead: swallow, change nothing + // A VELOCITY cell is a popup opener with no scalar value to reset. + if (curveTargetFor(hit.id) != CurveTarget::kNone) return true; + + const ParamControl curve = curveParamFor(static_cast(hit.id)); + const bool inner = hit.inner && curve != ParamControl::kCount; + const int target = inner ? static_cast(curve) : hit.id; + // The processor-side knobs own their own defaults — they never reach the parameter set. + if (target == static_cast(ParamControl::kVoiceCount)) { + voiceCount_ = kDefaultVoiceCount; + processor_->setVoiceCount(voiceCount_); + invalidate(); + return true; + } + if (target == static_cast(ParamControl::kMasterGain)) { + processor_->setMasterGainLinear(1.0); + invalidate(); + return true; + } + resetParamControl(target); + // Same commit tiering a drag of this control takes, so a reset reaches the sounding note + // exactly as a drag to the same value would. + if (dragCommitsLive(DragKind::kDeckKnob, target)) commitLive(); + else commitAndReload(); + invalidate(); + return true; +} + void ReaSamplerEditor::dragDeck(int x, int y) { // Radial knob: grab-anchored vertical drag — knobDragValue maps the y delta from the // value at grab (up = increase), so the value tracks relative motion and never jumps on diff --git a/src/shell/instrument/editor_internal.h b/src/shell/instrument/editor_internal.h index bcb493e..438492d 100644 --- a/src/shell/instrument/editor_internal.h +++ b/src/shell/instrument/editor_internal.h @@ -111,6 +111,13 @@ inline void drawTitleBand(LICE_IBitmap* bmp, const instrument::ui::Rect& title, kitText(bmp, titleText, readout.c_str(), Font::Title, ui::Role::TextPrimary); } +// Stroke widths for the radial faces. The value arc is drawn as adjacent 1px AA arcs rather +// than one thick primitive — LICE has no thick-arc call, and stacking radii is what keeps every +// ring antialiased. +inline constexpr int kKnobValueArcPx = 3; +inline constexpr int kInnerDialArcPx = 2; +inline constexpr int kKnobNeedlePx = 2; + // Draws one radial knob face: param_slider owns the value<->angle map; this turns it into // LICE calls. LICE takes radians, and drawing the 7->5 o'clock sweep through the top needs // a continuous angle span, so degrees convert as (deg - 360) * pi/180, mapping 210..510 @@ -144,16 +151,19 @@ inline void drawKnobFace(LICE_IBitmap* bmp, const instrument::ui::Rect& knobRect (arc.startDeg + v * instrument::ui::knobSweepDeg(arc) - 360.0) * kDegToRad); const ui::Role valueRole = disabled ? ui::Role::TextDim : (hot ? ui::Role::AccentHot : ui::Role::AccentPrimary); - LICE_Arc(bmp, cx, cy, rOuter, a0, av, toLice(ui::roleColor(valueRole)), 1.0f, 0, true); + const LICE_pixel valueCol = toLice(ui::roleColor(valueRole)); + for (int i = 0; i < kKnobValueArcPx; ++i) { + LICE_Arc(bmp, cx, cy, rOuter - static_cast(i), a0, av, valueCol, 1.0f, 0, true); + } } - // Needle: from ~35% radius out to the rim at the value's angle. + // Needle: from ~35% radius out to the rim at the value's angle. ThickFLine keeps the float + // endpoints AND is always antialiased, so the needle is smooth at every angle. const KnobPoint tip = instrument::ui::knobNeedlePoint(kg, arc, v); - const float ix = cx + static_cast((tip.x - kg.centerX) * 0.35); - const float iy = cy + static_cast((tip.y - kg.centerY) * 0.35); + const double ix = kg.centerX + (tip.x - kg.centerX) * 0.35; + const double iy = kg.centerY + (tip.y - kg.centerY) * 0.35; const ui::Role needleRole = disabled ? ui::Role::TextDim : ui::Role::TextPrimary; - LICE_Line(bmp, static_cast(ix + 0.5f), static_cast(iy + 0.5f), - static_cast(tip.x + 0.5f), static_cast(tip.y + 0.5f), - toLice(ui::roleColor(needleRole)), 1.0f, 0, true); + LICE_ThickFLine(bmp, ix, iy, tip.x, tip.y, toLice(ui::roleColor(needleRole)), 1.0f, 0, + kKnobNeedlePx); } // The concentric INNER dial: a second value on the same cell, drawn in the categorical @@ -184,12 +194,15 @@ inline void drawInnerDial(LICE_IBitmap* bmp, const instrument::ui::Rect& innerRe (arc.startDeg + v * instrument::ui::knobSweepDeg(arc) - 360.0) * kDegToRad); const ui::Role arcRole = disabled ? ui::Role::TextDim : (hot ? ui::Role::AccentHot : ui::Role::AccentTertiary); - LICE_Arc(bmp, cx, cy, r, a0, av, toLice(ui::roleColor(arcRole)), 1.0f, 0, true); + const LICE_pixel arcCol = toLice(ui::roleColor(arcRole)); + for (int i = 0; i < kInnerDialArcPx; ++i) { + LICE_Arc(bmp, cx, cy, r - static_cast(i), a0, av, arcCol, 1.0f, 0, true); + } const KnobPoint tip = instrument::ui::knobNeedlePoint(kg, arc, v); - LICE_Line(bmp, static_cast(cx + 0.5f), static_cast(cy + 0.5f), - static_cast(tip.x + 0.5f), static_cast(tip.y + 0.5f), - toLice(ui::roleColor(disabled ? ui::Role::TextDim : ui::Role::AccentTertiary)), - 1.0f, 0, true); + LICE_FLine(bmp, static_cast(kg.centerX), static_cast(kg.centerY), + static_cast(tip.x), static_cast(tip.y), + toLice(ui::roleColor(disabled ? ui::Role::TextDim : ui::Role::AccentTertiary)), + 1.0f, 0, true); } #endif // _WIN32 diff --git a/src/shell/instrument/editor_paint_chrome.cpp b/src/shell/instrument/editor_paint_chrome.cpp index 81ad6e8..1ca8514 100644 --- a/src/shell/instrument/editor_paint_chrome.cpp +++ b/src/shell/instrument/editor_paint_chrome.cpp @@ -145,6 +145,14 @@ void ReaSamplerEditor::paintChrome(LICE_IBitmap* bmp, const FaceLayout& fl, bool const LICE_pixel ink = toLice(roleColor(buttonLabelRole(st))); LICE_FillTriangle(bmp, g.leftX, g.topY, g.leftX, g.bottomY, g.apexX, g.apexY, ink, 1.0f, 0); + // LICE_FillTriangle takes no aa flag, so its two sloped edges come out stepped. + // Re-stroke them in the same ink: an AA line over the fill's own boundary softens + // the step without changing the shape. The vertical edge needs none. + const float lx = static_cast(g.leftX); + const float ax = static_cast(g.apexX); + const float ay = static_cast(g.apexY); + LICE_FLine(bmp, lx, static_cast(g.topY), ax, ay, ink, 1.0f, 0, true); + LICE_FLine(bmp, lx, static_cast(g.bottomY), ax, ay, ink, 1.0f, 0, true); } } diff --git a/src/shell/instrument/editor_paint_curve.cpp b/src/shell/instrument/editor_paint_curve.cpp index c695a3e..5b26534 100644 --- a/src/shell/instrument/editor_paint_curve.cpp +++ b/src/shell/instrument/editor_paint_curve.cpp @@ -121,7 +121,9 @@ void ReaSamplerEditor::paintVelocityCurve(LICE_IBitmap* bmp, const Rect& r) { const int cx = box.left + px; const double vel = curve.pointFromPixel(box, cx, box.top).velocity; const int cy = curve.pixelFromPoint(box, {vel, curve.eval(vel)}).y; - if (px > 0) LICE_Line(bmp, prevX, prevY, cx, cy, line, 1.0f, 0, true); + // Same weight and antialiasing the envelope traces use — one trace grammar across every + // curve surface (editor_paint_waveform.cpp owns why ThickFLine and not LICE_Line). + if (px > 0) LICE_ThickFLine(bmp, prevX, prevY, cx, cy, line, 1.0f, 0, 2); prevX = cx; prevY = cy; } diff --git a/src/shell/instrument/editor_paint_deck.cpp b/src/shell/instrument/editor_paint_deck.cpp index 780c0fe..3a7bfbf 100644 --- a/src/shell/instrument/editor_paint_deck.cpp +++ b/src/shell/instrument/editor_paint_deck.cpp @@ -20,6 +20,10 @@ namespace reasampler::vst { using namespace reasampler::ui; // kit vocabulary using namespace reasampler::instrument::ui; // deck geometry +// The knob's own name/value band. One size up from the group captions and the toggles, which +// are chrome you read once — this is the readout you read while turning something. +constexpr Font kCellLabelFont = Font::Label; + void ReaSamplerEditor::paintDeck(LICE_IBitmap* bmp, const FaceLayout& fl) { const Rect& deckArea = fl.bands.decks; if (deckArea.width <= 0 || deckArea.height <= 0) return; @@ -194,7 +198,7 @@ void ReaSamplerEditor::paintDeck(LICE_IBitmap* bmp, const FaceLayout& fl) { paintCurveButton(bmp, c.knob, curveCell, disabled, !disabled && isHovered(HoverKind::kControl, c.id)); kitTextCentered(bmp, c.label, knobName(static_cast(c.id)), - Font::Micro, Role::TextDim); + kCellLabelFont, Role::TextDim); continue; } const bool dragging = (drag_ == DragKind::kDeckKnob && dragParamId_ == c.id); @@ -226,7 +230,7 @@ void ReaSamplerEditor::paintDeck(LICE_IBitmap* bmp, const FaceLayout& fl) { if (innerDragging || innerHov) label = deckValueLabel(static_cast(curve)); else if (dragging || hov) label = deckValueLabel(c.id); else label = std::string(knobName(static_cast(c.id))); - kitTextCentered(bmp, c.label, label.c_str(), Font::Micro, Role::TextDim); + kitTextCentered(bmp, c.label, label.c_str(), kCellLabelFont, Role::TextDim); } } } diff --git a/src/shell/instrument/editor_paint_waveform.cpp b/src/shell/instrument/editor_paint_waveform.cpp index 432dc0f..833b6ec 100644 --- a/src/shell/instrument/editor_paint_waveform.cpp +++ b/src/shell/instrument/editor_paint_waveform.cpp @@ -41,6 +41,12 @@ constexpr Role kRoleLoopMarker = Role::AccentSecondary; constexpr int kEnvHandleRadius = 3; constexpr int kEnvHandleGrabbedRadius = 5; constexpr int kEnvHandleRingPx = 2; + +// Both envelope traces — staged and drawn — are one grammar and one weight. LICE_ThickFLine is +// ALWAYS antialiased (unlike LICE_Line, whose aa flag does nothing on an axis-aligned run), and +// the second pixel of width is what stops a shallow slope reading as a staircase over the +// waveform behind it. +constexpr int kEnvTracePx = 2; } // namespace void ReaSamplerEditor::paintWaveform(LICE_IBitmap* bmp, const Rect& band) { @@ -148,7 +154,7 @@ void ReaSamplerEditor::paintSplineOverlay(LICE_IBitmap* bmp, const OverlayArea& const int cx = box.left + px; const double t = curve.pointFromPixel(box, cx, box.top).velocity; const int cy = curve.pixelFromPoint(box, {t, curve.eval(t)}).y; - if (px > 0) LICE_Line(bmp, prevX, prevY, cx, cy, line, 1.0f, 0, true); + if (px > 0) LICE_ThickFLine(bmp, prevX, prevY, cx, cy, line, 1.0f, 0, kEnvTracePx); prevX = cx; prevY = cy; } @@ -213,7 +219,7 @@ void ReaSamplerEditor::paintEnvelopeOverlay(LICE_IBitmap* bmp, const OverlayArea if (prev != nullptr) { const int x0 = (std::max)(area.x, (std::min)(area.right() - 1, prev->x)); const int x1 = (std::max)(area.x, (std::min)(area.right() - 1, v.x)); - LICE_Line(bmp, x0, prev->y, x1, v.y, line, 1.0f, 0, true); + LICE_ThickFLine(bmp, x0, prev->y, x1, v.y, line, 1.0f, 0, kEnvTracePx); } prev = &v; } diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index 70e11cf..4ff47ef 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -79,7 +79,10 @@ void ReaSamplerEditor::attachedToParent() { wc.hInstance = hInst; wc.lpszClassName = kChildClassName; wc.hCursor = LoadCursor(nullptr, IDC_ARROW); - wc.style = CS_HREDRAW | CS_VREDRAW; + // CS_DBLCLKS is what makes WM_LBUTTONDBLCLK arrive at all. It also REPLACES the second + // WM_LBUTTONDOWN of a double-click, so every surface that counted two downs (Browse's + // load accelerator) depends on the DBLCLK handler falling through to onMouseDown. + wc.style = CS_HREDRAW | CS_VREDRAW | CS_DBLCLKS; RegisterClassW(&wc); classRegistered = true; } @@ -150,6 +153,17 @@ LRESULT CALLBACK ReaSamplerEditor::wndProc(HWND hwnd, UINT msg, WPARAM wParam, self->onMouseDown(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); } return 0; + case WM_LBUTTONDBLCLK: + if (self) { + SetCapture(hwnd); + SetFocus(hwnd); + const int mx = GET_X_LPARAM(lParam); + const int my = GET_Y_LPARAM(lParam); + // Knob reset first; anything it does not claim is the second click of the pair + // this message stands in for (see the class-style note above). + if (!self->onMouseDoubleClick(mx, my)) self->onMouseDown(mx, my); + } + return 0; case WM_MOUSEMOVE: if (self) { const int mx = GET_X_LPARAM(lParam); @@ -202,6 +216,11 @@ LRESULT CALLBACK ReaSamplerEditor::wndProc(HWND hwnd, UINT msg, WPARAM wParam, // explicitly (the child wndproc historically handled only left-button). if (self) self->onMouseRDown(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); return 0; + case WM_RBUTTONDBLCLK: + // CS_DBLCLKS replaces the second RIGHT-button down too, so the spline surfaces' + // repeat-delete needs this peer or a fast double right-click drops one delete. + if (self) self->onMouseRDown(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); + return 0; case WM_RBUTTONUP: return 0; // claimed so the pair never reaches DefWindowProc (no context menu) case WM_CAPTURECHANGED: diff --git a/src/shell/instrument/reasampler_editor.h b/src/shell/instrument/reasampler_editor.h index 99b55c7..5ad2b12 100644 --- a/src/shell/instrument/reasampler_editor.h +++ b/src/shell/instrument/reasampler_editor.h @@ -195,6 +195,13 @@ private: bool mouseDownDeck(const FaceLayout& fl, int x, int y); void mouseDownBrowse(int w, int h, int x, int y); + // Double-click = reset the radial knob under the pointer to its default. Returns false when + // no knob claims the point, and the caller then replays it as an ordinary mouse-down (the + // platform note at the window class explains why that fall-through is load-bearing). + bool onMouseDoubleClick(int x, int y); + bool doubleClickChrome(const FaceLayout& fl, int x, int y); + bool doubleClickDeck(const FaceLayout& fl, int x, int y); + // Whether deck knob `id` belongs to a group whose enable toggle is off. The ONE predicate // behind both the Disabled paint and the inert grab, so they cannot disagree. bool deckKnobDisabled(int id) const; @@ -335,16 +342,12 @@ private: void beginMarkerDrag(WaveMarker which, const SetupMarkers& m, std::int64_t frames, int x); // Deck knobs edit the parameter set's PlaySeconds (play mode + AHDSR; pitch engine + AD - // pitch envelope) — wall-clock seconds, rate-free; the build resolves to frames. - - // The normalized [0,1] display value for control `id` given `play` (seconds -> 0..1 over - // a fixed ceiling, levels and fractions as-is, semitone depth centered at 0.5, curve - // exponents over their logarithmic travel). + // pitch envelope) — wall-clock seconds, rate-free; the build resolves to frames. The three + // adapters below are thin int-id wrappers over the pure `deck_values` module, which owns + // what each control's value means; see its header rather than restating the domains here. double controlValue(int id, const PlaySeconds& play) const; - - // Applies a committed control interaction to `play`: a knob's normalized `value` or a - // toggle's `segment` (0/1). Mutates `play` in place. void applyControl(int id, PlaySeconds& play, double value, int segment) const; + void resetParamControl(int id); // Applies a knob/toggle interaction to the ONE parameter set for control `id`: ordinary // controls route through applyControl; kKeyTrack writes the keyTrack scalar (0..200% @@ -385,8 +388,8 @@ private: // params edit, no reload). void applyDeckKnob(int id, double norm); - // The knob's live value label shown during hover/drag: seconds, percents, source - // frames, signed semitones, a voice count, or the master-gain dB. + // The knob's live value label shown during hover/drag: milliseconds, percents, Hz, signed + // semitones, a curve exponent, a voice count, or the master-gain dB. std::string deckValueLabel(int id) const; ReaSamplerProcessor* processor_ = nullptr; diff --git a/src/shell/panel/draw_kit.cpp b/src/shell/panel/draw_kit.cpp index 4b516e7..ffa7d1b 100644 --- a/src/shell/panel/draw_kit.cpp +++ b/src/shell/panel/draw_kit.cpp @@ -298,17 +298,33 @@ void drawWaveform(LICE_IBitmap* bmp, const KitBox& box, const Envelope& env) { if (bins.empty() || innerW <= 0) continue; - // One filled span per pixel column (see draw_kit.h — gap-free via columnMinMax). + // One filled span per pixel column (see draw_kit.h — gap-free via columnMinMax), then + // an antialiased stroke joining each column's extremes to its neighbour's. The fill + // alone leaves the outline stepped — a vertical span has no aa to apply — and where two + // adjacent columns differ sharply it reads as a comb rather than one envelope. The + // stroke is the SAME ink as the fill it edges, so it can only soften the boundary. + double prevTop = 0.0; + double prevBottom = 0.0; for (int col = 0; col < innerW; ++col) { const MinMax mm = columnMinMax(bins, innerW, col); const int x = box.x + 2 + col; - int yMax = midY - static_cast( - compressAmplitudeForDisplay(mm.max) * halfSpan); - int yMin = midY - static_cast( - compressAmplitudeForDisplay(mm.min) * halfSpan); + const double topF = midY - compressAmplitudeForDisplay(mm.max) * halfSpan; + const double bottomF = midY - compressAmplitudeForDisplay(mm.min) * halfSpan; + int yMax = static_cast(topF); + int yMin = static_cast(bottomF); if (yMax < bandTop) yMax = bandTop; if (yMin > bandTop + bandH - 1) yMin = bandTop + bandH - 1; LICE_Line(bmp, x, yMin, x, yMax, waveCol, 1.0f, 0, false); + if (col > 0) { + const float xPrev = static_cast(x - 1); + const float xF = static_cast(x); + LICE_FLine(bmp, xPrev, static_cast(prevTop), xF, + static_cast(topF), waveCol, 1.0f, 0, true); + LICE_FLine(bmp, xPrev, static_cast(prevBottom), xF, + static_cast(bottomF), waveCol, 1.0f, 0, true); + } + prevTop = topF; + prevBottom = bottomF; } } } diff --git a/tests/test_deck_groups.cpp b/tests/test_deck_groups.cpp index dbbb202..2c4c30b 100644 --- a/tests/test_deck_groups.cpp +++ b/tests/test_deck_groups.cpp @@ -238,11 +238,11 @@ static void testAmpGroupWidthSurvivesAGateTriggerFlip() { static void testWrappedDeckHeightAtTheEditorFloorWidth() { const std::vector g = sampleDeckGroups(PlayMode::Gate); - // At the floor (== default) 840 the deck takes three rows: PITCH + PITCH ENV + FILTER fill - // the first (818 of the 824 available — six px of headroom, so one more FILTER cell would - // wrap the group and reflow everything under it), FILTER ENV + AMP + VELOCITY the second, - // VOICE + MASTER the third. Two rows cannot hold the eight groups in ANY order at this - // width: 1666 px of group plus 72 px of gaps against a 1648 px two-row capacity. + // At the floor (== default) 980 the deck takes three rows: PITCH + PITCH ENV + FILTER fill + // the first (950 of the 964 available — fourteen px of headroom, so one more FILTER cell + // would wrap the group and reflow everything under it), FILTER ENV + AMP + VELOCITY the + // second, VOICE + MASTER the third. Two rows cannot hold the eight groups in ANY order at + // this width: 1978 px of group plus 72 px of gaps against a 1928 px two-row capacity. CHECK(deckRowCount(g, kAvailAtMinWidth) == 3); CHECK(deckHeight(g, kAvailAtMinWidth) == 3 * kDeckGroupH + 2 * kDeckRowGap); @@ -258,7 +258,7 @@ static void testWrappedDeckHeightAtTheEditorFloorWidth() { // The guard the raised floor exists to provide: at the smallest window the host can produce, // the deck band still lands inside the client area AND the waveform still gets its two-lane -// floor. Growing the deck past what 620 px can hold fails HERE instead of silently pushing +// floor. Growing the deck past what the floor height can hold fails HERE instead of silently pushing // FILTER ENV / AMP / VOICE / MASTER off-screen, where there is no scroll to reach them. static void testDeckFitsInsideTheEnforcedMinimumWindow() { for (PlayMode mode : {PlayMode::Gate, PlayMode::Trigger}) { @@ -574,9 +574,9 @@ static void testNoFaceLeavesSlackWhereItsDroppedControlsWere() { static void testGateModeWidthsAndRowAssignmentAreUnchanged() { const std::vector g = sampleDeckGroups(PlayMode::Gate); const struct { int id; int width; int row; } want[] = { - {kGroupPitch, 150, 0}, {kGroupPitchEnv, 204, 0}, {kGroupFilter, 440, 0}, - {kGroupFilterEnv, 252, 1}, {kGroupAmpEnv, 252, 1}, {kGroupVelocity, 156, 1}, - {kGroupVoice, 152, 2}, {kGroupMaster, 60, 2}, + {kGroupPitch, 150, 0}, {kGroupPitchEnv, 252, 0}, {kGroupFilter, 524, 0}, + {kGroupFilterEnv, 312, 1}, {kGroupAmpEnv, 312, 1}, {kGroupVelocity, 192, 1}, + {kGroupVoice, 164, 2}, {kGroupMaster, 72, 2}, }; CHECK(g.size() == sizeof(want) / sizeof(want[0])); const DeckLayout dl = layoutDeck(g, kPad, 0, kAvailAtMinWidth); diff --git a/tests/test_deck_values.cpp b/tests/test_deck_values.cpp new file mode 100644 index 0000000..77eca88 --- /dev/null +++ b/tests/test_deck_values.cpp @@ -0,0 +1,175 @@ +// Standalone tests for reasampler::instrument::ui::deck_values — no VST3, no REAPER, no +// framework. Covers the deck's parameter-set binding: the norm <-> stored-value round trip on a +// representative control of each domain, the DOUBLE-CLICK RESET (each ring of a dual-ring knob +// resetting only its own field), and the ms time-constant formatter across its whole range. + +#include "../src/core/instrument/ui/deck_values.h" + +#include +#include +#include + +using namespace reasampler; +using namespace reasampler::instrument::ui; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +static std::string msLabel(double seconds) { + char buf[24]; + formatEnvTimeMs(seconds, buf, sizeof(buf)); + return std::string(buf); +} + +// Every domain the binding maps: a stage time over the seconds ceiling, a level, a fraction, +// a normalized filter position, a bipolar depth, and a curve exponent over its log travel. +static void testNormRoundTripsThroughEveryValueDomain() { + PlaySeconds p; + setDeckParam(DeckParam::kAttack, p, 0.25, 0); + CHECK(p.adsr.attackSeconds == 0.25 * kEnvTimeMaxSeconds); + CHECK(deckParamNorm(DeckParam::kAttack, p) == 0.25); + + setDeckParam(DeckParam::kSustain, p, 0.4, 0); + CHECK(p.adsr.sustainLevel == 0.4); + CHECK(deckParamNorm(DeckParam::kSustain, p) == 0.4); + + setDeckParam(DeckParam::kTrigHold, p, 0.75, 0); + CHECK(p.trigAhd.holdFraction == 0.75); + CHECK(deckParamNorm(DeckParam::kTrigHold, p) == 0.75); + + setDeckParam(DeckParam::kFilterCutoff, p, 0.5, 0); + CHECK(deckParamNorm(DeckParam::kFilterCutoff, p) == 0.5); + + // Bipolar: the centre detent is exact in BOTH directions, so a knob parked at centre + // persists no depth at all. + setDeckParam(DeckParam::kFilterModAmt, p, 0.5, 0); + CHECK(p.filter.modAmount == 0.0); + CHECK(deckParamNorm(DeckParam::kFilterModAmt, p) == 0.5); + setDeckParam(DeckParam::kFilterModAmt, p, 1.0, 0); + CHECK(p.filter.modAmount == 1.0); + + // A curve exponent off neutral survives the round trip; the centre snaps to exactly 1.0. + setDeckParam(DeckParam::kAttackCurve, p, 1.0, 0); + CHECK(p.adsr.attackCurve > 1.0); + CHECK(deckParamNorm(DeckParam::kAttackCurve, p) == 1.0); + setDeckParam(DeckParam::kAttackCurve, p, 0.5, 0); + CHECK(p.adsr.attackCurve == 1.0); + + // Out-of-range norms clamp rather than writing an out-of-domain param. + setDeckParam(DeckParam::kDecay, p, 2.0, 0); + CHECK(p.adsr.decaySeconds == kEnvTimeMaxSeconds); + setDeckParam(DeckParam::kDecay, p, -1.0, 0); + CHECK(p.adsr.decaySeconds == 0.0); +} + +// The dual-ring reset contract: the outer ring resets the stage VALUE and the inner dial resets +// the EXPONENT, each leaving the other exactly as it was. Both fields are asserted in both +// directions — checking only the field that changed would pass even if the reset clobbered its +// neighbour. +static void testResetTouchesOnlyItsOwnRingOnADualRingKnob() { + const PlaySeconds defaults; + const struct { DeckParam knob; DeckParam curve; } pairs[] = { + {DeckParam::kAttack, DeckParam::kAttackCurve}, + {DeckParam::kDecay, DeckParam::kDecayCurve}, + {DeckParam::kRelease, DeckParam::kReleaseCurve}, + {DeckParam::kTrigAttack, DeckParam::kTrigAttackCurve}, + {DeckParam::kPitchEnvDecay, DeckParam::kPitchEnvDecayCurve}, + {DeckParam::kFilterEnvRelease, DeckParam::kFilterEnvReleaseCurve}, + }; + for (const auto& pr : pairs) { + // Dial BOTH rings well away from their defaults. + PlaySeconds p; + setDeckParam(pr.knob, p, 0.6, 0); + setDeckParam(pr.curve, p, 0.9, 0); + const double dialledValue = deckParamNorm(pr.knob, p); + const double dialledCurve = deckParamNorm(pr.curve, p); + CHECK(dialledValue != deckParamNorm(pr.knob, defaults)); + CHECK(dialledCurve != deckParamNorm(pr.curve, defaults)); + + // INNER: the exponent goes to exactly the linear neutral, the value does not move. + PlaySeconds inner = p; + resetDeckParam(pr.curve, inner); + CHECK(deckParamNorm(pr.curve, inner) == deckParamNorm(pr.curve, defaults)); + CHECK(deckParamNorm(pr.curve, inner) == 0.5); // the exponent itself is 1.0 + CHECK(deckParamNorm(pr.knob, inner) == dialledValue); + + // OUTER: the value goes to its default, the exponent does not move. + PlaySeconds outer = p; + resetDeckParam(pr.knob, outer); + CHECK(deckParamNorm(pr.knob, outer) == deckParamNorm(pr.knob, defaults)); + CHECK(deckParamNorm(pr.curve, outer) == dialledCurve); + } +} + +// The exponent reset is specified as EXACTLY 1.0 — the identity curveMap short-circuits on +// (curve_law.h), not merely something that rounds to it. +static void testInnerResetLandsOnTheExactLinearNeutral() { + PlaySeconds p; + setDeckParam(DeckParam::kAttackCurve, p, 0.2, 0); + CHECK(p.adsr.attackCurve < 1.0); + resetDeckParam(DeckParam::kAttackCurve, p); + CHECK(p.adsr.attackCurve == 1.0); + + setDeckParam(DeckParam::kFilterTrigDecayCurve, p, 0.95, 0); + CHECK(p.filter.trigEnv.decayCurve > 1.0); + resetDeckParam(DeckParam::kFilterTrigDecayCurve, p); + CHECK(p.filter.trigEnv.decayCurve == 1.0); +} + +// A reset lands on the field's own stored default, exactly — the defaults are read off a fresh +// PlaySeconds rather than from a second table. +static void testResetLandsOnTheStoredDefaultOfEachControl() { + const PlaySeconds defaults; + PlaySeconds p; + setDeckParam(DeckParam::kSustain, p, 0.1, 0); + setDeckParam(DeckParam::kTrigLength, p, 0.3, 0); + setDeckParam(DeckParam::kFilterKeyTrack, p, 0.9, 0); + setDeckParam(DeckParam::kPitchEnvDepth, p, 1.0, 0); + + resetDeckParam(DeckParam::kSustain, p); + resetDeckParam(DeckParam::kTrigLength, p); + resetDeckParam(DeckParam::kFilterKeyTrack, p); + resetDeckParam(DeckParam::kPitchEnvDepth, p); + + CHECK(p.adsr.sustainLevel == defaults.adsr.sustainLevel); + CHECK(p.trigger.lengthFraction == defaults.trigger.lengthFraction); + CHECK(p.filter.keyTrack == defaults.filter.keyTrack); + CHECK(p.pitchEnv.peakSemitones == defaults.pitchEnv.peakSemitones); +} + +// One unit, everywhere, across the formatter's whole range: a sub-millisecond value keeps a +// decimal rather than reading as a bare zero, and a multi-second one stays in ms rather than +// switching units mid-deck. +static void testTimeConstantsAlwaysReadInMilliseconds() { + CHECK(msLabel(0.0) == "0.0 ms"); + CHECK(msLabel(0.0005) == "0.5 ms"); // sub-millisecond + CHECK(msLabel(0.0094) == "9.4 ms"); + CHECK(msLabel(0.012) == "12 ms"); // the use case's own reading + CHECK(msLabel(0.25) == "250 ms"); + CHECK(msLabel(1.5) == "1500 ms"); // multi-second, still ms + CHECK(msLabel(kEnvTimeMaxSeconds) == "2000 ms"); + // The 10 ms hinge belongs to the integer form, not the decimal one. + CHECK(msLabel(0.01) == "10 ms"); + CHECK(msLabel(0.0099) == "9.9 ms"); + + // Never overruns a short buffer, and always terminates. + char tiny[4]; + std::memset(tiny, 'x', sizeof(tiny)); + formatEnvTimeMs(1.5, tiny, sizeof(tiny)); + CHECK(tiny[3] == '\0'); +} + +int main() { + testNormRoundTripsThroughEveryValueDomain(); + testResetTouchesOnlyItsOwnRingOnADualRingKnob(); + testInnerResetLandsOnTheExactLinearNeutral(); + testResetLandsOnTheStoredDefaultOfEachControl(); + testTimeConstantsAlwaysReadInMilliseconds(); + if (g_fail) { + std::printf("%d FAILURE(S)\n", g_fail); + return 1; + } + std::printf("deck_values tests passed\n"); + return 0; +} diff --git a/tests/test_knob_deck.cpp b/tests/test_knob_deck.cpp index c86bf62..0ce3f89 100644 --- a/tests/test_knob_deck.cpp +++ b/tests/test_knob_deck.cpp @@ -9,6 +9,8 @@ // always places; deckHeight consistency with deckRowCount. // * hit-test — knob cell hit (whole cell), toggle segment 0/1 boundaries, fence padding // misses, outside-deck misses. +// * knob-FACE hit-test — the reset resolve against the drawn circles: inner disc, outer ring, +// both exclusive boundaries, and the points where it deliberately disagrees with the cell. #include "../src/core/instrument/ui/knob_deck.h" @@ -183,7 +185,7 @@ static void testHitTest() { // not cover is smaller than one pixel per cell. static void testReservedCellWidthGoesToTheCellsPresent() { const DeckGroupDesc full{0, 78, {}, {100, 44}, {}, {20, 21, 22, 23, 24}, {}}; - // Three, four, and a lone cell against the same five-slot reserve — 240/3, 240/4, 240/1. + // Three, four, and a lone cell against the same five-slot reserve. const std::vector> faces = { {20, 21, 22, -1, -1}, {20, 21, 22, 23, -1}, {20, -1, -1, -1, -1}}; for (const std::vector& ids : faces) { @@ -202,7 +204,13 @@ static void testReservedCellWidthGoesToTheCellsPresent() { const DeckCellLayout& c = lay.cells[static_cast(i)]; CHECK(c.cell.width == lay.cells[0].cell.width); // uniform CHECK(c.knob.width == kDeckKnobSize); // the dial itself is fixed - CHECK(c.knob.x - c.cell.x == c.cell.right() - c.knob.right()); + // Centred as exactly as integers allow: a cell whose spare width is odd cannot + // split it evenly, and the layout's integer division gives the odd pixel to the + // RIGHT margin. Pinned as a directional identity rather than a tolerance, so a + // future off-by-one on the other side would still fail here. + const int leftGap = c.knob.x - c.cell.x; + const int rightGap = c.cell.right() - c.knob.right(); + CHECK(rightGap - leftGap == (c.cell.width - kDeckKnobSize) % 2); if (i > 0) CHECK(c.cell.x == lay.cells[static_cast(i - 1)].cell.right()); } // Uncovered run is the indivisible residue only, split evenly at the two ends. @@ -222,21 +230,21 @@ static void testReservedCellWidthGoesToTheCellsPresent() { layoutDeck(b, 0, 0, 824).groups[0].rowToggle.seg0); } -// The three faces above (240/3, 240/4, 240/1) all divide their run evenly, so none of them -// actually exercises "residue in symmetric end margins". An 8-slot reserve with 5 present -// (384/5 = 76 r4) does: residue 4 is the smallest case that can tell a symmetric split (2/2) -// apart from a trailing-only one (0/4) — a residue of 1 (0/1 vs 1/0... i.e. 0/1) can't, since -// leadPad = residue/2 rounds to 0 either way, which is exactly why this seam's earlier test -// passed without pinning the rule it was named for. +// The three faces above all divide their run evenly, so none of them actually exercises +// "residue in symmetric end margins". An 8-slot reserve with 7 present (480/7 = 68 r4) does: +// residue 4 is the smallest case that can tell a symmetric split (2/2) apart from a +// trailing-only one (0/4) — a residue of 1 can't, since leadPad = residue/2 rounds to 0 either +// way, which is exactly why this seam's earlier test passed without pinning the rule it was +// named for. static void testIndivisibleResidueSplitsSymmetricallyAcrossBothEnds() { - const DeckGroupDesc g{0, 78, {}, {100, 44}, {}, {20, 21, 22, 23, 24, -1, -1, -1}, {}}; + const DeckGroupDesc g{0, 78, {}, {100, 44}, {}, {20, 21, 22, 23, 24, 25, 26, -1}, {}}; std::vector gs{g}; const DeckLayout dl = layoutDeck(gs, 0, 0, 824); const DeckGroupLayout& lay = dl.groups[0]; - CHECK(lay.cells.size() == 5); + CHECK(lay.cells.size() == 7); const int run = 8 * kDeckCellW; - const int present = 5; + const int present = 7; const int cellW = run / present; // 76: the same integer division the layout uses const int expectedResidue = run - cellW * present; // 4 CHECK(expectedResidue == 4); @@ -331,6 +339,49 @@ static void testCaptionToggle2() { CHECK(h.kind == DeckHitKind::CaptionToggle && h.id == 302 && h.segment == 1); } +// The double-click RESET resolve. Unlike hitTestDeck's whole-cell grab, this one answers the +// drawn circles: inner disc -> inner target, outer ring -> outer target, anything off the dial +// (the label band, the cell margin, outside the deck) -> neither. Both boundaries are exclusive. +static void testKnobFaceResolvesInnerRingOuterRingAndMisses() { + const std::vector g = shellLikeDeck(); + const DeckLayout dl = layoutDeck(g, 0, 0, 900); + const DeckCellLayout& c = dl.groups[0].cells[0]; + const int cx = c.knob.x + c.knob.width / 2; + const int cy = c.knob.y + c.knob.height / 2; + const int rOuter = c.knob.width / 2; + const int rInner = c.inner.width / 2; + CHECK(rInner > 0 && rInner < rOuter); + + // Dead centre is the inner target; just inside the inner radius still is. + DeckFaceHit h = hitTestKnobFace(dl, cx, cy); + CHECK(h.id == c.id && h.inner); + h = hitTestKnobFace(dl, cx + rInner - 1, cy); + CHECK(h.id == c.id && h.inner); + // EXACTLY on the inner radius is the outer ring — the boundary belongs to neither disc. + h = hitTestKnobFace(dl, cx + rInner, cy); + CHECK(h.id == c.id && !h.inner); + // Just inside the rim is still the outer ring... + h = hitTestKnobFace(dl, cx + rOuter - 1, cy); + CHECK(h.id == c.id && !h.inner); + // ...and EXACTLY on the rim is a miss, by the same exclusive rule. + h = hitTestKnobFace(dl, cx + rOuter, cy); + CHECK(h.id == -1 && !h.inner); + + // The cell corner is inside the CELL (hitTestDeck resolves it as a grab) but outside the + // circle — the two resolves deliberately disagree there. + CHECK(hitTestDeck(dl, c.cell.x + 1, c.cell.y + 1).kind == DeckHitKind::Knob); + CHECK(hitTestKnobFace(dl, c.cell.x + 1, c.cell.y + 1).id == -1); + // The label band under the knob: a grab anchor, never a reset target. + CHECK(hitTestDeck(dl, c.label.x + 2, c.label.y + 2).kind == DeckHitKind::Knob); + CHECK(hitTestKnobFace(dl, c.label.x + 2, c.label.y + 2).id == -1); + // Off the deck entirely. + CHECK(hitTestKnobFace(dl, -50, -50).id == -1); + // A diagonal at 45 degrees inside the rim: proves the resolve is radial, not the inscribed + // square a rect test would accept — this point is inside the knob RECT but outside the disc. + const int diag = static_cast(rOuter * 0.75) + 1; // dist ~ 1.06 * rOuter + CHECK(hitTestKnobFace(dl, cx + diag, cy + diag).id == -1); +} + static void testEmptyDeck() { const std::vector none; CHECK(deckRowCount(none, 800) == 0); @@ -349,6 +400,7 @@ int main() { testIndivisibleResidueSplitsSymmetricallyAcrossBothEnds(); testCaptionRadioGeometryAndHit(); testInnerDialHit(); + testKnobFaceResolvesInnerRingOuterRingAndMisses(); testCaptionToggle2(); testEmptyDeck(); if (g_fail) { diff --git a/tests/test_sample_chrome.cpp b/tests/test_sample_chrome.cpp index ed00485..8d5c75a 100644 --- a/tests/test_sample_chrome.cpp +++ b/tests/test_sample_chrome.cpp @@ -21,9 +21,9 @@ static int g_fail = 0; #define CHECK(cond) do { if(!(cond)) { \ std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) -static constexpr int kKnob = 26; // stands in for knob_deck's kDeckKnobSize +static constexpr int kKnob = 40; // stands in for knob_deck's kDeckKnobSize -static Rect chromeBand(int w = 840, int h = 620) { +static Rect chromeBand(int w = kEditorMinWidth, int h = kEditorMinHeight) { return computeSampleBands(w, h, 120).chrome; } From ca464397b24816747d853d56da9fd01a0aeb1e20 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 09:47:07 -0400 Subject: [PATCH 2/4] fix: restore waveform symmetry about the midline, cut deck_values' link to the bank model, and unit-test the column arithmetic The waveform column's vertical extents move to pure component_geometry so the shared primitive stops being untested; PlaySeconds hoists into a header-only play_seconds target. --- docs/product/visual-design-language.md | 2 +- src/core/instrument/CLAUDE.md | 9 +- src/core/instrument/map/CMakeLists.txt | 12 ++- src/core/instrument/map/play_seconds.h | 88 ++++++++++++++++++++ src/core/instrument/map/sample_map.h | 77 +---------------- src/core/instrument/ui/CMakeLists.txt | 12 +-- src/core/instrument/ui/deck_values.h | 12 +-- src/core/instrument/ui/knob_deck.cpp | 19 ++--- src/core/instrument/ui/knob_deck.h | 15 +++- src/core/ui/component_geometry.cpp | 27 ++++++ src/core/ui/component_geometry.h | 29 ++++++- src/shell/instrument/editor_input_chrome.cpp | 8 +- src/shell/panel/CLAUDE.md | 2 +- src/shell/panel/draw_kit.cpp | 34 ++++---- tests/test_component_geometry.cpp | 75 ++++++++++++++++- tests/test_deck_values.cpp | 22 ++++- tests/test_sample_chrome.cpp | 5 +- 17 files changed, 310 insertions(+), 138 deletions(-) create mode 100644 src/core/instrument/map/play_seconds.h diff --git a/docs/product/visual-design-language.md b/docs/product/visual-design-language.md index 36b7b07..15a58ee 100644 --- a/docs/product/visual-design-language.md +++ b/docs/product/visual-design-language.md @@ -793,7 +793,7 @@ concession. Everything with a slope or a curve must draw through a primitive tha | Spline (drawn EG) contour | `editor_paint_waveform.cpp` `paintSplineOverlay` | **Fixed** — same treatment, one trace grammar. | | Velocity-curve popup trace | `editor_paint_curve.cpp` | **Fixed** — same treatment. | | Velocity-curve mini thumbnail | `editor_paint_curve.cpp` | Left at 1 px AA `LICE_Line` — a 2 px trace blots at thumbnail scale. | -| Waveform min/max columns | `draw_kit.cpp` `drawWaveform` | **Fixed** — column fill unchanged (it cannot alias), plus an AA `LICE_FLine` stroke joining each column's extremes to its neighbour's, in the same ink. Shared with the docked bank panel and the browser cards. | +| Waveform min/max columns | `draw_kit.cpp` `drawWaveform` | **Fixed** — column fill unchanged (it cannot alias), plus an AA `LICE_FLine` stroke joining each column's extremes to its neighbour's, in the same ink. Shared with the docked bank panel and the browser cards. **Measured cost** (Release, MSVC 14.44, real LICE, 24 stereo cards × 136 columns = 6528 columns): fill alone 0.070 ms per full-grid repaint, fill+stroke 0.48 ms — the stroke is ~0.41 ms, about 2.5% of a 60 Hz frame, and the grid repaints on hover/scroll/drag, not continuously. | | Preview play triangle | `editor_paint_chrome.cpp` | **Fixed** — `LICE_FillTriangle` has no `aa`; its two sloped edges are re-stroked with AA `LICE_FLine`. | | Envelope/spline node handles (squares) | `editor_paint_waveform.cpp` | Already clean — axis-aligned `LICE_FillRect`. | | Envelope curve knots (circles) | `editor_paint_waveform.cpp` | Already clean — `LICE_FillCircle` with `aa=true`. | diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index 9492dc4..9c16dc3 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -291,6 +291,7 @@ anything for a trigger shape. ### `map/` - `sample_map` — the bank blob → selected capture resolve, the channel policy (downmix / dual-mono / L-R split), `InstrumentParams` (the ONE parameter set: root/loop/start overrides, keyTrack, velocity curve, `PlaySeconds`), the single override-beats-intrinsic fold (`resolveCapture`, shared by the bank and refs paths so they cannot drift), and the `SampleData` build. **Wall-clock times stored as rate-free SECONDS, resolved against the live project rate — NO hardcoded sample rates in `src/`** (Daniel's standing ruling, load-bearing). Deliberately does NOT link the voice engine: the build's product is plain `SampleData`. +- `play_seconds` — the stored, wall-clock-SECONDS value layer (`PlaySeconds` + `AdsrSeconds` / `AhdSeconds` / `PitchEnvSeconds` / `FilterSeconds`), header-only and split from `sample_map` so a consumer that only edits those values reaches them without the bank model and the WAV codec. `resolvePlay`, which turns them into the engine's frame domain, stays with the rest of the mapping. - `component_state_io` (`core/instrument/map`) — the `ComponentState` envelope + params-payload binary codec (envelope v1…v11, params payload v1…v13), split out of `sample_map` (Q-W2v, T4-13 ≡ T2-07) so BOTH artifacts can link the codec without the extension pulling in the whole voice engine to serialize one preset blob — the extension's `instrument_drop` and the instrument's processor read/write the identical bytes, so the cross-artifact contract cannot drift. Payload v1…v7 are the RETIRED per-zone lists: still read, lifting by adopting zone one's capture + parameters (that first zone is what the old first-match resolve actually played, so it is also what supersedes the envelope's stored selection id). Payload v9 appends the per-voice filter tail; a v8 blob is a strict prefix of it and lifts to the off/neutral filter default. Every tail since is a strict suffix on the same discipline — v10 the staged curves, v11 the loop crossfade, v12 the velocity→pitch curve, v13 the dual Staged/Spline state (the three contours, plus hard-flag tails for the three velocity curves — their v7/v9/v12 blocks are frozen at 16 bytes/point and had no room for a per-point flag). v12 also RE-TAGS the y DOMAIN of one frozen slot inside the v9 filter tail — its velocity curve reads bipolar from v12 on, unipolar before — which needs no version branch, because a pre-v12 curve's y values are already valid bipolar ones; every other filter slot, `velAmount` included, keeps its meaning. - `params_payload` — the PARAMS-PAYLOAD half of that codec, split from the envelope half on the axis the format already has: the payload carries its own version and grows independently, so the two version ladders are two responsibilities. An INTERNAL seam — the public entry points stay `serialize`/`deserializeComponentState`. The prose ladder and every version constant stay in `component_state_io.h`, their one home. - `bank_sync` — generation change-detection + assignment-request consume: owns the yes/no decision logic so the rules are provable without a host. The processor shell owns cadence and side effects. @@ -318,9 +319,11 @@ anything for a trigger shape. `PlaySeconds`, so there is no second table of defaults to drift), and `formatEnvTimeMs`, the ONE time-constant formatter: every displayed time constant reads in **ms**, never seconds, so two stage times are comparable at a glance. A display-unit decision only — nothing about the - stored representation changes. Links the map layer because `PlaySeconds` is what a deck edits. - The shell keeps only the controls the parameter set does not carry (key-track, voice count, - master gain, preview velocity) and the labels for them. + stored representation changes. Links the header-only `play_seconds`, deliberately not + `sample_map`: `PlaySeconds` is the whole of what a deck edits, and linking the mapping would + drag the bank model and the WAV codec in behind it. The shell keeps only the controls the + parameter set does not carry (key-track, voice count, master gain, preview velocity) and the + labels for them. - `deck_groups` — also home to `isLiveDeckParam` and `liveCommitFor`, the editor's whole commit-tier routing decision (see "Live parameter delivery" above), and to `OverlayEnv` + `nextOverlaySelection`/`overlayEnvEnabled`/`overlayEnvInert`, the whole overlay-selection state machine (exclusivity, the none resting state, and which selections a disabled or DRAWN group makes inert); WHICH groups the Sample face's deck carries, split from `knob_deck`'s HOW they lay out: the `DeckParam` control-id space (the editor's `ParamControl` is an alias of it), the `DeckGroupId` list, `sampleDeckGroups` in signal-flow order (**pitch → filter → amp**, then velocity/voice/master), and the deck's bipolar-knob law. Reads `PlayMode` for the AMP group's Gate/Trigger face, which is why this and not `knob_deck` is the module that touches the engine's value layer. Also home to `CurveTarget` + `curveTargetFor` — the VELOCITY group's three cells are popup openers, not dials, and that predicate is the ONE place they are named, so paint, hit-test routing and the popup's title all agree. MASTER is reserved for post-voice-mixer concerns, which is why the curves sit in their own group immediately left of VOICE rather than there. - `spline_edit` — THE point-editing grammar, and the one place it is written down: left-click grabs a node and adds one in empty space, right-click deletes, control-click toggles hard/smooth. Both spline consumers — the velocity-curve popup and the spline EG overlay — route their mouse-down through `resolveSplineEdit`, so the two cannot drift into two grammars. The endpoint and point-count rules are NOT restated here: `deletePoint` and `addPoint` own them, and the caller applies the resolved action to the curve. Also home to `splineOverlayBox`, the contour's mapping box inside the waveform overlay — the FULL area, no inset, so the drawn contour stays 1:1 with the sample's time axis. - `curve_popup` — pure curve-popup geometry + dismissal test (FB1): centered sheet over the Sample face — width/height clamps, title row, Close button rect, curve-box rect, outside-sheet dismissal test. Mirror of `overflow_menu`; no LICE or REAPER types. diff --git a/src/core/instrument/map/CMakeLists.txt b/src/core/instrument/map/CMakeLists.txt index 4f409d1..95f983e 100644 --- a/src/core/instrument/map/CMakeLists.txt +++ b/src/core/instrument/map/CMakeLists.txt @@ -26,10 +26,20 @@ reasampler_pure_library(component_state_io # proof the codec is engine-free, which is what keeps engine object code out of the extension. reasampler_test(component_state_io LINK component_state_io) +# The stored seconds value layer, header-only (hence INTERFACE) — PlaySeconds and the four +# stage-time structs it composes. Split from sample_map so a consumer that only edits those +# values reaches them WITHOUT the bank model and the WAV codec: the editor's deck_values +# binding is exactly that consumer, and linking sample_map for one value struct would put +# bank_book + wav_codec into a test whose subject is a knob. Links the same value-layer set +# play_params.h needs (velocity_curve's out-of-line zero() is a default member initializer). +add_library(play_seconds INTERFACE) +target_include_directories(play_seconds INTERFACE ${REASAMPLER_SRC_DIR}) +target_link_libraries(play_seconds INTERFACE velocity_curve peaks curve_law) + # The mapping's product is plain SampleData, so the voice engine is not a dependency. reasampler_pure_library(sample_map SOURCES sample_map.cpp - LINK PUBLIC bank_book wav_codec velocity_curve peaks curve_law) + LINK PUBLIC bank_book wav_codec play_seconds velocity_curve peaks curve_law) # Links only sample_map + component_state_io: the same plain-data-boundary proof, spanning # both halves of the mapping/codec split where the frozen-format assertions live. reasampler_test(sample_map LINK sample_map component_state_io) diff --git a/src/core/instrument/map/play_seconds.h b/src/core/instrument/map/play_seconds.h new file mode 100644 index 0000000..b91cd78 --- /dev/null +++ b/src/core/instrument/map/play_seconds.h @@ -0,0 +1,88 @@ +#pragma once +// play_seconds — the STORED, wall-clock-SECONDS value layer the instrument edits and +// serializes: PlaySeconds and the four stage-time structs it composes. Header-only, and split +// from sample_map so a consumer that only edits these values (the editor's deck binding) does +// not link the bank model and the WAV codec to reach one value struct. `resolvePlay`, which +// turns them into the engine's frame domain, stays in sample_map with the rest of the mapping. + +#include "core/instrument/engine/play_params.h" // PlayMode / TriggerParams / SplineEnv / … + +namespace reasampler::instrument::map { + +using instrument::engine::VelocityCurve; + +// Daniel's standing ruling: no hardcoded sample rate anywhere in the program. The +// instrument stores/edits wall-clock performance times (AHDSR A/H/D/R, pitch-env A/D) as +// SECONDS, rate-free; the engine receives FRAMES resolved from the LIVE sample rate at +// build. Quantities anchored to the source file's timeline (start point, loop points, +// Trigger %-length + fades) stay in source frames/fractions, carried through unchanged +// (TriggerParams reused verbatim). +// +// The stored AHDSR times (seconds). sustainLevel is dimensionless (0..1), not a time; the +// three curve exponents are dimensionless too (curve_law.h owns their domain). +struct AdsrSeconds { + double attackSeconds = 0.003; // tier-0 default + double holdSeconds = 0.0; + double decaySeconds = 0.0; + double sustainLevel = 1.0; + double releaseSeconds = 0.060; // tier-0 default + double attackCurve = util::kCurveNeutral; + double decayCurve = util::kCurveNeutral; + double releaseCurve = util::kCurveNeutral; +}; + +// The stored sustain-less AHD: wall-clock stage times in SECONDS, Hold as a FRACTION of the +// span left after them (AhdParams owns why a fraction, not a time). +struct AhdSeconds { + double attackSeconds = 0.0; + double decaySeconds = 0.0; + double holdFraction = 1.0; + double attackCurve = util::kCurveNeutral; + double decayCurve = util::kCurveNeutral; +}; + +// The stored AHD pitch envelope. enabled + peakSemitones are dimensionless. The hold fraction +// defaults to 0 so an instance predating the stage plays as its attack-decay predecessor did. +struct PitchEnvSeconds { + bool enabled = false; + double peakSemitones = 0.0; // signed depth at the peak + AhdSeconds shape{0.0, 0.0, /*holdFraction=*/0.0, util::kCurveNeutral, util::kCurveNeutral}; +}; + +// The stored mirror of the engine's FilterParams (play_params.h, which owns what each field +// MEANS). Only the envelope differs between the two: the control positions and depths are +// rate-free already, so this block is a seconds/frames split of one field, not of the whole +// struct. The env default is a flat unity, so `enabled` is the only thing standing between a +// loaded blob and the pre-filter sound. +struct FilterSeconds { + bool enabled = false; + engine::filter::FilterSettings settings; + double modAmount = 0.0; + double velAmount = 0.0; + double keyTrack = 0.0; + AdsrSeconds env{0.0, 0.0, 0.0, 1.0, 0.0}; // Gate + AhdSeconds trigEnv; // Trigger + VelocityCurve velocityCurve = VelocityCurve::zero(); +}; + +// The stored play bundle: wall-clock times in SECONDS, source-timeline quantities in +// frames/fractions (TriggerParams). Instrument-owned, serialized, editor-facing — distinct +// from the engine-facing PlayParams (frames). +struct PlaySeconds { + PlayMode playMode = PlayMode::Gate; + AdsrSeconds adsr; // Gate amp: AHDSR (seconds) + TriggerParams trigger; // Trigger play span (%-length) + AhdSeconds trigAhd; // Trigger amp: AHD (seconds + fraction) + PitchEngine pitchEngine = kDefaultPitchEngine; // product default: Preserve + PitchEnvSeconds pitchEnv; // AHD pitch modulation, off by default + VelocityCurve pitchVelocityCurve = VelocityCurve::zero(); // velocity -> pitch, off by default + FilterSeconds filter; // per-voice filter, off by default + // The three drawn contours, in the same slots the engine bundle carries them (play_params.h + // owns why they sit beside the envelopes rather than inside them). Normalized over the + // sample's own length, so resolvePlay needs no rate for them. + SplineEnv ampSpline; + SplineEnv pitchSpline; + SplineEnv filterSpline; +}; + +} // namespace reasampler::instrument::map diff --git a/src/core/instrument/map/sample_map.h b/src/core/instrument/map/sample_map.h index ead32c4..18eb7a0 100644 --- a/src/core/instrument/map/sample_map.h +++ b/src/core/instrument/map/sample_map.h @@ -14,6 +14,7 @@ #include "core/model/bank_book.h" // BankBook::deserialize (shared bank JSON parse) #include "core/instrument/engine/play_params.h" // SampleData, SampleLoop, PlayParams +#include "core/instrument/map/play_seconds.h" // PlaySeconds (the stored seconds value layer) #include "core/capture/wav_codec.h" // parseWavLayout, extractFloatFrames (shared WAV parse) namespace reasampler::instrument::map { @@ -134,81 +135,7 @@ std::vector downmixToMono(const std::vector& interleav std::vector extractChannel(const std::vector& interleaved, int channelCount, int which); -// --- Stored (wall-clock SECONDS) play params ---------------------------------- -// -// Daniel's standing ruling: no hardcoded sample rate anywhere in the program. The -// instrument stores/edits wall-clock performance times (AHDSR A/H/D/R, pitch-env A/D) as -// SECONDS, rate-free; the engine receives FRAMES resolved from the LIVE sample rate at -// build. Quantities anchored to the source file's timeline (start point, loop points, -// Trigger %-length + fades) stay in source frames/fractions, carried through unchanged -// (TriggerParams reused verbatim). -// -// The stored AHDSR times (seconds). sustainLevel is dimensionless (0..1), not a time; the -// three curve exponents are dimensionless too (curve_law.h owns their domain). -struct AdsrSeconds { - double attackSeconds = 0.003; // tier-0 default - double holdSeconds = 0.0; - double decaySeconds = 0.0; - double sustainLevel = 1.0; - double releaseSeconds = 0.060; // tier-0 default - double attackCurve = util::kCurveNeutral; - double decayCurve = util::kCurveNeutral; - double releaseCurve = util::kCurveNeutral; -}; - -// The stored sustain-less AHD: wall-clock stage times in SECONDS, Hold as a FRACTION of the -// span left after them (AhdParams owns why a fraction, not a time). -struct AhdSeconds { - double attackSeconds = 0.0; - double decaySeconds = 0.0; - double holdFraction = 1.0; - double attackCurve = util::kCurveNeutral; - double decayCurve = util::kCurveNeutral; -}; - -// The stored AHD pitch envelope. enabled + peakSemitones are dimensionless. The hold fraction -// defaults to 0 so an instance predating the stage plays as its attack-decay predecessor did. -struct PitchEnvSeconds { - bool enabled = false; - double peakSemitones = 0.0; // signed depth at the peak - AhdSeconds shape{0.0, 0.0, /*holdFraction=*/0.0, util::kCurveNeutral, util::kCurveNeutral}; -}; - -// The stored mirror of the engine's FilterParams (play_params.h, which owns what each field -// MEANS). Only the envelope differs between the two: the control positions and depths are -// rate-free already, so this block is a seconds/frames split of one field, not of the whole -// struct. The env default is a flat unity, so `enabled` is the only thing standing between a -// loaded blob and the pre-filter sound. -struct FilterSeconds { - bool enabled = false; - engine::filter::FilterSettings settings; - double modAmount = 0.0; - double velAmount = 0.0; - double keyTrack = 0.0; - AdsrSeconds env{0.0, 0.0, 0.0, 1.0, 0.0}; // Gate - AhdSeconds trigEnv; // Trigger - VelocityCurve velocityCurve = VelocityCurve::zero(); -}; - -// The stored play bundle: wall-clock times in SECONDS, source-timeline quantities in -// frames/fractions (TriggerParams). Instrument-owned, serialized, editor-facing — distinct -// from the engine-facing PlayParams (frames). -struct PlaySeconds { - PlayMode playMode = PlayMode::Gate; - AdsrSeconds adsr; // Gate amp: AHDSR (seconds) - TriggerParams trigger; // Trigger play span (%-length) - AhdSeconds trigAhd; // Trigger amp: AHD (seconds + fraction) - PitchEngine pitchEngine = kDefaultPitchEngine; // product default: Preserve - PitchEnvSeconds pitchEnv; // AHD pitch modulation, off by default - VelocityCurve pitchVelocityCurve = VelocityCurve::zero(); // velocity -> pitch, off by default - FilterSeconds filter; // per-voice filter, off by default - // The three drawn contours, in the same slots the engine bundle carries them (play_params.h - // owns why they sit beside the envelopes rather than inside them). Normalized over the - // sample's own length, so resolvePlay needs no rate for them. - SplineEnv ampSpline; - SplineEnv pitchSpline; - SplineEnv filterSpline; -}; +// --- Seconds -> frames -------------------------------------------------------- // Resolve a stored seconds bundle to the engine's frame-domain PlayParams against a live // sample rate (frames = round(seconds * rate)). Source-timeline fields carry through diff --git a/src/core/instrument/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index affe9ec..7a5c44d 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -7,7 +7,9 @@ reasampler_pure_library(sample_bands SOURCES sample_bands.cpp LINK PUBLIC editor reasampler_test(sample_bands LINK sample_bands) reasampler_pure_library(sample_chrome SOURCES sample_chrome.cpp LINK PUBLIC sample_bands) -reasampler_test(sample_chrome LINK sample_chrome) +# knob_deck is linked for the test only: the knob size the shell hands chromeRects is +# kDeckKnobSize, and the test reads the real constant rather than copying its value. +reasampler_test(sample_chrome LINK sample_chrome knob_deck) reasampler_pure_library(embed_strip SOURCES embed_strip.cpp LINK PUBLIC editor_geometry) reasampler_test(embed_strip LINK embed_strip) @@ -69,12 +71,12 @@ reasampler_pure_library(spline_edit reasampler_test(spline_edit LINK spline_edit waveform_view sample_bands) # The deck's VALUE binding, split from its composition on the same axis deck_groups was split -# from knob_deck. Links sample_map because PlaySeconds — the thing a deck knob edits — is -# declared there; no sample_map symbol is called, only its value types. Same for the filter's -# MorphLaw — an enum, so no filter symbol is linked. +# from knob_deck. Links the header-only play_seconds, NOT sample_map: PlaySeconds is all a deck +# knob edits, and sample_map would drag the bank model and the WAV codec in behind it. Same for +# the filter's MorphLaw — an enum, so no filter symbol is linked. reasampler_pure_library(deck_values SOURCES deck_values.cpp - LINK PUBLIC deck_groups sample_map envelope_overlay) + LINK PUBLIC deck_groups play_seconds envelope_overlay) reasampler_test(deck_values LINK deck_values) reasampler_pure_library(curve_popup SOURCES curve_popup.cpp LINK PUBLIC editor_geometry) diff --git a/src/core/instrument/ui/deck_values.h b/src/core/instrument/ui/deck_values.h index 98c61cf..8512c46 100644 --- a/src/core/instrument/ui/deck_values.h +++ b/src/core/instrument/ui/deck_values.h @@ -2,13 +2,13 @@ // normalized 0..1 a knob shows, the write back into the stored seconds/fractions/positions, the // double-click reset, and the ms time-constant formatter. Split from the editor shell so the // whole domain map is provable without a host; deck_groups owns WHICH controls exist, this owns -// what each one's value MEANS. Links the map layer because PlaySeconds is what a deck edits. +// what each one's value MEANS. #pragma once #include -#include "core/instrument/map/sample_map.h" // PlaySeconds (the deck's edit target) +#include "core/instrument/map/play_seconds.h" // PlaySeconds (the deck's edit target) #include "core/instrument/ui/deck_groups.h" // DeckParam #include "core/instrument/ui/envelope_overlay.h" // kGateStageMaxSeconds @@ -39,9 +39,11 @@ double deckParamNorm(DeckParam id, const PlaySeconds& play); void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment); // Resets `id` to its default. The default IS what a fresh PlaySeconds carries, so there is no -// second table of defaults to drift from the real one. Exact for every shipped default: they -// are all 0, 1, or the curve neutral, and the seconds ceiling is a power of two, so the -// norm round trip loses nothing. For knob-valued controls — a toggle has no reset gesture. +// second table of defaults to drift from the real one. It arrives via the norm round trip, so +// landing EXACTLY on a stage time (0.003 s attack, 0.060 s release) depends on +// kEnvTimeMaxSeconds being a power of two — x/2^n*2^n is lossless, an arbitrary ceiling is not. +// Move that ceiling off a power of two and a reset lands a mantissa bit off its own default. +// For knob-valued controls — a toggle has no reset gesture. void resetDeckParam(DeckParam id, PlaySeconds& play); // A time constant as MILLISECONDS, e.g. "12 ms". Never switches to seconds: the editor reads in diff --git a/src/core/instrument/ui/knob_deck.cpp b/src/core/instrument/ui/knob_deck.cpp index f2aabf3..89c9ca2 100644 --- a/src/core/instrument/ui/knob_deck.cpp +++ b/src/core/instrument/ui/knob_deck.cpp @@ -190,24 +190,19 @@ DeckHit hitTestDeck(const DeckLayout& layout, int x, int y) { return {}; } -namespace { -// Squared distance from a rect's centre, in the rect's own pixel units. -double distSqFromCentre(const Rect& r, int x, int y) { - const double dx = x - (r.x + r.width / 2.0); - const double dy = y - (r.y + r.height / 2.0); - return dx * dx + dy * dy; +bool inKnobFace(const Rect& knob, int x, int y) { + const double dx = x - (knob.x + knob.width / 2.0); + const double dy = y - (knob.y + knob.height / 2.0); + const double r = knob.width / 2.0; + return dx * dx + dy * dy < r * r; } -} // namespace DeckFaceHit hitTestKnobFace(const DeckLayout& layout, int x, int y) { for (const DeckGroupLayout& g : layout.groups) { if (!contains(g.box, x, y)) continue; for (const DeckCellLayout& c : g.cells) { - const double rOuter = c.knob.width / 2.0; - if (distSqFromCentre(c.knob, x, y) >= rOuter * rOuter) continue; - const double rInner = c.inner.width / 2.0; - const bool inner = distSqFromCentre(c.inner, x, y) < rInner * rInner; - return {c.id, inner}; + if (!inKnobFace(c.knob, x, y)) continue; + return {c.id, inKnobFace(c.inner, x, y)}; } return {}; // inside the group but off every dial } diff --git a/src/core/instrument/ui/knob_deck.h b/src/core/instrument/ui/knob_deck.h index ab01f54..e618eee 100644 --- a/src/core/instrument/ui/knob_deck.h +++ b/src/core/instrument/ui/knob_deck.h @@ -153,11 +153,18 @@ struct DeckFaceHit { bool inner = false; // inside the concentric inner disc }; +// A point inside the circle inscribed in `knob`, boundary-EXCLUSIVE. THE target rule for the +// reset gesture wherever a radial knob is drawn — the deck's own faces and the chrome's +// preview-velocity dial both resolve through it, so one gesture cannot grow two target rules. +bool inKnobFace(const Rect& knob, int x, int y); + // Resolved against the drawn CIRCLES, not the cell: a reset is aimed at a dial, so the label -// band and the cell margins must miss where a drag grab deliberately does not. Both radii are -// boundary-EXCLUSIVE, one rule for both rings — a point exactly on the inner radius is an -// outer-ring hit, one exactly on the outer radius is a miss. Whether a cell actually carries an -// inner value is deck_groups' call, exactly as with DeckHit::inner. +// band and the cell margins must miss where a drag grab deliberately does not. One rule for +// both rings — a point exactly on the inner radius is an outer-ring hit, one exactly on the +// outer radius is a miss. Whether a cell actually carries an inner value is deck_groups' call, +// exactly as with DeckHit::inner. Unlike hitTestDeck this runs NO toggle/radio precedence pass +// first, which is only correct while no toggle rect overlaps a knob circle — a layout change +// that lets them overlap has to give this the same precedence order. DeckFaceHit hitTestKnobFace(const DeckLayout& layout, int x, int y); } // namespace reasampler::instrument::ui diff --git a/src/core/ui/component_geometry.cpp b/src/core/ui/component_geometry.cpp index 6157990..8a9a8a1 100644 --- a/src/core/ui/component_geometry.cpp +++ b/src/core/ui/component_geometry.cpp @@ -97,4 +97,31 @@ int waveformColumnCount(const KitBox& box) { return w > 0 ? w : 0; } +WaveformBand waveformBand(int bandTop, int bandHeight) { + WaveformBand b; + b.top = bandTop; + b.height = bandHeight; + b.midY = bandTop + bandHeight / 2; + b.halfSpan = (bandHeight / 2) - 2; // matches drawWaveform's 2px vertical breathing room + return b; +} + +WaveformColumnSpan waveformColumnSpan(const WaveformBand& band, + double compressedMax, double compressedMin) { + const int lo = band.top; + const int hi = band.top + band.height - 1; + + WaveformColumnSpan s; + s.top = band.midY - static_cast(compressedMax * band.halfSpan); + s.bottom = band.midY - static_cast(compressedMin * band.halfSpan); + s.topF = band.midY - compressedMax * band.halfSpan; + s.bottomF = band.midY - compressedMin * band.halfSpan; + + if (s.top < lo) s.top = lo; + if (s.bottom > hi) s.bottom = hi; + if (s.topF < lo) s.topF = lo; + if (s.bottomF > hi) s.bottomF = hi; + return s; +} + } // namespace reasampler::ui diff --git a/src/core/ui/component_geometry.h b/src/core/ui/component_geometry.h index 89876e6..7e8e4f8 100644 --- a/src/core/ui/component_geometry.h +++ b/src/core/ui/component_geometry.h @@ -75,11 +75,38 @@ ListRowBox computeListRow(const KitBox& list, int index, int rowHeight); // `rowCount` rows). rowCount bounds the hit so blank space past the last row is a clean miss. int hitTestListRow(int px, int py, const KitBox& list, int rowHeight, int rowCount); -// --- Waveform column count --------------------------------------------------- +// --- Waveform columns -------------------------------------------------------- // Pixel columns drawWaveform renders inside `box` (its fixed 2px side insets), never negative. // Pass directly as peaks::computeEnvelope's binCount — one bin per column is correct resolution; // overbinning doesn't improve render quality and wastes memory/CPU. int waveformColumnCount(const KitBox& box); +// One channel band's shared vertical metrics: the zero line every column mirrors about, and the +// pixel height a full-scale amplitude reaches (inset so a peak keeps the band's breathing room). +struct WaveformBand { + int top = 0; + int height = 0; + int midY = 0; + double halfSpan = 0.0; +}; +WaveformBand waveformBand(int bandTop, int bandHeight); + +// The vertical extents of one column: the integer edges of the filled span, and the same edges +// at sub-pixel precision for the antialiased outline stroke that joins a column's extremes to +// its neighbour's. Both take the band clamp, so the stroke cannot leave the band the fill is +// confined to. Amplitudes arrive already through the display curve. +struct WaveformColumnSpan { + int top = 0; + int bottom = 0; + double topF = 0.0; + double bottomF = 0.0; +}; + +// Truncation is applied to the SCALED AMPLITUDE and then mirrored about midY — never to the +// resulting y, which would round the two edges in opposite directions and leave a column with +// |max| == |min| a pixel taller above the zero line than below. +WaveformColumnSpan waveformColumnSpan(const WaveformBand& band, + double compressedMax, double compressedMin); + } // namespace reasampler::ui diff --git a/src/shell/instrument/editor_input_chrome.cpp b/src/shell/instrument/editor_input_chrome.cpp index 932b3e6..83cb4de 100644 --- a/src/shell/instrument/editor_input_chrome.cpp +++ b/src/shell/instrument/editor_input_chrome.cpp @@ -7,6 +7,7 @@ #ifdef _WIN32 #include "core/instrument/ui/keyboard_strip.h" // keyAtPoint / resolveDragNote (root key) +#include "core/instrument/ui/knob_deck.h" // inKnobFace (the shared reset target rule) #include "shell/instrument/editor_internal.h" #include "shell/instrument/reasampler_processor.h" @@ -82,11 +83,10 @@ bool ReaSamplerEditor::mouseDownChrome(const FaceLayout& fl, int x, int y) { bool ReaSamplerEditor::doubleClickChrome(const FaceLayout& fl, int x, int y) { // The preview-velocity dial answers the same reset gesture the deck's knobs do — it is a - // radial knob drawn by the same primitive, so a user who learns the gesture there expects - // it here. Resolved against the whole cell, not the circle: the cell IS the knob's target - // on this surface (there is no neighbouring control to steal from). + // radial knob drawn by the same primitive, so it resolves through the same target rule + // (inKnobFace), against the drawn circle rather than the cell's label band. if (selectedId_.empty() || !processor_) return false; - if (!contains(fl.chrome.velCell, x, y)) return false; + if (!inKnobFace(fl.chrome.velKnob, x, y)) return false; processor_->setPreviewVelocity(kPreviewVelocityDefault); invalidate(); return true; diff --git a/src/shell/panel/CLAUDE.md b/src/shell/panel/CLAUDE.md index 59ac294..b8da4c2 100644 --- a/src/shell/panel/CLAUDE.md +++ b/src/shell/panel/CLAUDE.md @@ -56,7 +56,7 @@ live in `shell/bank_ops`, a sibling directory, not here. - `panel_thumbnails` — the PCM→envelope thumbnail cache + the bank-change fingerprint pass. - `panel_audition` — the preview-playback engine. - `panel_bank_ops` — the menu/prompt UX skin over the promptless `shell/bank_ops` verbs. -- `draw_kit` — shared LICE draw shell: `fillSurface`, `drawButton`/`drawSlider`/`drawListRow`/`drawWaveform`, cached-font `text()`, full interaction-state model, double-buffer preserved. Consumes `theme` + `component_geometry`. +- `draw_kit` — shared LICE draw shell: `fillSurface`, `drawButton`/`drawSlider`/`drawListRow`/`drawWaveform`, cached-font `text()`, full interaction-state model, double-buffer preserved. Consumes `theme` + `component_geometry`. Its antialiasing contract — which surfaces alias, which cannot, and the LICE primitive each answers with — is the disposition table in `docs/product/visual-design-language.md` §8, which governs `drawWaveform` as much as this doc does. `drawWaveform`'s per-column vertical arithmetic is NOT here: it is the pure, unit-tested `component_geometry::waveformBand`/`waveformColumnSpan` pair, because this TU is DAW-verified and not unit-tested. ## Gotchas diff --git a/src/shell/panel/draw_kit.cpp b/src/shell/panel/draw_kit.cpp index ffa7d1b..22ac842 100644 --- a/src/shell/panel/draw_kit.cpp +++ b/src/shell/panel/draw_kit.cpp @@ -27,6 +27,10 @@ using ui::compressAmplitudeForDisplay; using ui::roleColor; using ui::roleColorState; using ui::spectralColor; +using ui::WaveformBand; +using ui::waveformBand; +using ui::WaveformColumnSpan; +using ui::waveformColumnSpan; // The one place a pure KitColor becomes a LICE_pixel (LICE_RGBA(r,g,b,a), verified against // lice.h). The theme owns the color; the shell owns the packing. @@ -289,11 +293,9 @@ void drawWaveform(LICE_IBitmap* bmp, const KitBox& box, const Envelope& env) { for (int ch = 0; ch < channels; ++ch) { const ChannelEnvelope& bins = env[static_cast(ch)]; - const int bandTop = box.y + ch * bandH; - const int midY = bandTop + bandH / 2; - const double halfSpan = (bandH / 2) - 2; + const WaveformBand band = waveformBand(box.y + ch * bandH, bandH); - LICE_Line(bmp, box.x + 2, midY, box.x + box.width - 2, midY, + LICE_Line(bmp, box.x + 2, band.midY, box.x + box.width - 2, band.midY, midCol, 1.0f, 0, false); if (bins.empty() || innerW <= 0) continue; @@ -303,28 +305,22 @@ void drawWaveform(LICE_IBitmap* bmp, const KitBox& box, const Envelope& env) { // alone leaves the outline stepped — a vertical span has no aa to apply — and where two // adjacent columns differ sharply it reads as a comb rather than one envelope. The // stroke is the SAME ink as the fill it edges, so it can only soften the boundary. - double prevTop = 0.0; - double prevBottom = 0.0; + WaveformColumnSpan prev; for (int col = 0; col < innerW; ++col) { const MinMax mm = columnMinMax(bins, innerW, col); const int x = box.x + 2 + col; - const double topF = midY - compressAmplitudeForDisplay(mm.max) * halfSpan; - const double bottomF = midY - compressAmplitudeForDisplay(mm.min) * halfSpan; - int yMax = static_cast(topF); - int yMin = static_cast(bottomF); - if (yMax < bandTop) yMax = bandTop; - if (yMin > bandTop + bandH - 1) yMin = bandTop + bandH - 1; - LICE_Line(bmp, x, yMin, x, yMax, waveCol, 1.0f, 0, false); + const WaveformColumnSpan s = waveformColumnSpan( + band, compressAmplitudeForDisplay(mm.max), compressAmplitudeForDisplay(mm.min)); + LICE_Line(bmp, x, s.bottom, x, s.top, waveCol, 1.0f, 0, false); if (col > 0) { const float xPrev = static_cast(x - 1); const float xF = static_cast(x); - LICE_FLine(bmp, xPrev, static_cast(prevTop), xF, - static_cast(topF), waveCol, 1.0f, 0, true); - LICE_FLine(bmp, xPrev, static_cast(prevBottom), xF, - static_cast(bottomF), waveCol, 1.0f, 0, true); + LICE_FLine(bmp, xPrev, static_cast(prev.topF), xF, + static_cast(s.topF), waveCol, 1.0f, 0, true); + LICE_FLine(bmp, xPrev, static_cast(prev.bottomF), xF, + static_cast(s.bottomF), waveCol, 1.0f, 0, true); } - prevTop = topF; - prevBottom = bottomF; + prev = s; } } } diff --git a/tests/test_component_geometry.cpp b/tests/test_component_geometry.cpp index a793ac4..bb5e764 100644 --- a/tests/test_component_geometry.cpp +++ b/tests/test_component_geometry.cpp @@ -4,8 +4,10 @@ // Covers (brief §L1 point 2 + §test cases): button box inset + graceful suppression; slider // track/filled/handle geometry for representative values incl. endpoints, value->px inverse, // too-small/degenerate suppression; list-row rect for representative indices, partial last -// row, hover hit-test returns the right row and "no hit" outside/past the last row; and the -// shared half-open box hit-test agrees with layout (no double-claimed pixel). +// row, hover hit-test returns the right row and "no hit" outside/past the last row; the +// shared half-open box hit-test agrees with layout (no double-claimed pixel); and the waveform +// column's vertical extents — symmetry about the zero line, monotonicity, and the band clamp +// on both the fill and the antialiased stroke, which draw_kit's shell cannot cover. #include "../src/core/ui/component_geometry.h" @@ -190,6 +192,70 @@ static void testWaveformColumnCount() { CHECK(waveformColumnCount(KitBox{0, 0, 0, 40}) == 0); } +// --- waveform column span ---------------------------------------------------- + +// The regression this exists to catch: rounding applied to the resulting y instead of to the +// scaled amplitude draws a symmetric column one pixel taller above the zero line than below. +static void testSymmetricColumnDrawsEqualHeightAboveAndBelowTheZeroLine() { + const WaveformBand band = waveformBand(0, 41); // odd height -> fractional half-span + // Sweep amplitudes whose scaled value is fractional, which is where the two edges can + // round in opposite directions. + for (int i = 1; i <= 20; ++i) { + const double a = i / 20.0; + const WaveformColumnSpan s = waveformColumnSpan(band, a, -a); + CHECK(band.midY - s.top == s.bottom - band.midY); + // The stroke's edges are symmetric to sub-ULP, not bit-exactly: midY +/- x rounds the + // two sides independently. Any REAL asymmetry here would be a whole pixel. + const double above = band.midY - s.topF; + const double below = s.bottomF - band.midY; + CHECK(above - below < 1e-9 && below - above < 1e-9); + } +} + +// A column with no signal collapses onto the zero line rather than spanning a pixel of it. +static void testSilentColumnCollapsesOntoTheZeroLine() { + const WaveformBand band = waveformBand(10, 40); + const WaveformColumnSpan s = waveformColumnSpan(band, 0.0, 0.0); + CHECK(s.top == band.midY && s.bottom == band.midY); + CHECK(s.topF == band.midY && s.bottomF == band.midY); +} + +// Amplitude grows the span monotonically, and a bigger amplitude never draws shorter. +static void testTallerAmplitudeNeverDrawsAShorterColumn() { + const WaveformBand band = waveformBand(0, 40); + int prevHeight = -1; + for (int i = 0; i <= 20; ++i) { + const WaveformColumnSpan s = waveformColumnSpan(band, i / 20.0, -(i / 20.0)); + const int h = s.bottom - s.top; + CHECK(h >= prevHeight); + prevHeight = h; + } +} + +// The clamp is the band's boundary for BOTH the fill and the antialiased stroke — a stroke +// vertex outside the band would draw into the neighbouring channel's lane. +static void testBothEdgesAndTheStrokeClampToTheBand() { + const WaveformBand band = waveformBand(100, 40); + const int lo = 100; + const int hi = 139; + // Past full scale in both directions (the display curve's own range is [-1,1], so this is + // the defensive case, not a reachable one). + const WaveformColumnSpan s = waveformColumnSpan(band, 8.0, -8.0); + CHECK(s.top == lo && s.bottom == hi); + CHECK(s.topF == lo && s.bottomF == hi); + // Full scale sits INSIDE the band by the half-span's 2px inset — the clamp is a guard, + // not the thing that produces the normal drawn height. + const WaveformColumnSpan full = waveformColumnSpan(band, 1.0, -1.0); + CHECK(full.top > lo && full.bottom < hi); +} + +static void testBandMetricsMirrorTheDrawnInset() { + const WaveformBand b = waveformBand(50, 40); + CHECK(b.top == 50 && b.height == 40); + CHECK(b.midY == 70); + CHECK(b.halfSpan == 18.0); // half the band, less the 2px breathing room +} + int main() { testHitTestBoxHalfOpen(); testButtonBoxInset(); @@ -208,6 +274,11 @@ int main() { testListRowHitTestBoundedByCount(); testListRowLayoutHitAgreement(); testWaveformColumnCount(); + testSymmetricColumnDrawsEqualHeightAboveAndBelowTheZeroLine(); + testSilentColumnCollapsesOntoTheZeroLine(); + testTallerAmplitudeNeverDrawsAShorterColumn(); + testBothEdgesAndTheStrokeClampToTheBand(); + testBandMetricsMirrorTheDrawnInset(); if (g_fail == 0) std::printf("component_geometry: all tests passed\n"); else std::printf("component_geometry: %d CHECK(s) FAILED\n", g_fail); diff --git a/tests/test_deck_values.cpp b/tests/test_deck_values.cpp index 77eca88..1a0963f 100644 --- a/tests/test_deck_values.cpp +++ b/tests/test_deck_values.cpp @@ -38,8 +38,12 @@ static void testNormRoundTripsThroughEveryValueDomain() { CHECK(p.trigAhd.holdFraction == 0.75); CHECK(deckParamNorm(DeckParam::kTrigHold, p) == 0.75); - setDeckParam(DeckParam::kFilterCutoff, p, 0.5, 0); - CHECK(deckParamNorm(DeckParam::kFilterCutoff, p) == 0.5); + // Named field, not just a round trip: cutoff and morph are both normalized positions with + // the same 1.0 default, so a getter+setter pair that swapped them would round-trip cleanly. + setDeckParam(DeckParam::kFilterCutoff, p, 0.25, 0); + CHECK(p.filter.settings.cutoffNorm == 0.25f); + CHECK(p.filter.settings.morphNorm == 1.0f); + CHECK(deckParamNorm(DeckParam::kFilterCutoff, p) == 0.25); // Bipolar: the centre detent is exact in BOTH directions, so a knob parked at centre // persists no depth at all. @@ -117,8 +121,10 @@ static void testInnerResetLandsOnTheExactLinearNeutral() { CHECK(p.filter.trigEnv.decayCurve == 1.0); } -// A reset lands on the field's own stored default, exactly — the defaults are read off a fresh -// PlaySeconds rather than from a second table. +// A reset lands on the field's own stored default, EXACTLY — the defaults are read off a fresh +// PlaySeconds and arrive through the norm round trip, so the two stage times whose defaults are +// neither 0 nor 1 are the cases that actually exercise that exactness (see resetDeckParam's +// note on what the seconds ceiling has to be for it to hold). static void testResetLandsOnTheStoredDefaultOfEachControl() { const PlaySeconds defaults; PlaySeconds p; @@ -126,16 +132,24 @@ static void testResetLandsOnTheStoredDefaultOfEachControl() { setDeckParam(DeckParam::kTrigLength, p, 0.3, 0); setDeckParam(DeckParam::kFilterKeyTrack, p, 0.9, 0); setDeckParam(DeckParam::kPitchEnvDepth, p, 1.0, 0); + setDeckParam(DeckParam::kAttack, p, 0.5, 0); + setDeckParam(DeckParam::kRelease, p, 0.5, 0); + CHECK(p.adsr.attackSeconds != defaults.adsr.attackSeconds); + CHECK(p.adsr.releaseSeconds != defaults.adsr.releaseSeconds); resetDeckParam(DeckParam::kSustain, p); resetDeckParam(DeckParam::kTrigLength, p); resetDeckParam(DeckParam::kFilterKeyTrack, p); resetDeckParam(DeckParam::kPitchEnvDepth, p); + resetDeckParam(DeckParam::kAttack, p); + resetDeckParam(DeckParam::kRelease, p); CHECK(p.adsr.sustainLevel == defaults.adsr.sustainLevel); CHECK(p.trigger.lengthFraction == defaults.trigger.lengthFraction); CHECK(p.filter.keyTrack == defaults.filter.keyTrack); CHECK(p.pitchEnv.peakSemitones == defaults.pitchEnv.peakSemitones); + CHECK(p.adsr.attackSeconds == defaults.adsr.attackSeconds); + CHECK(p.adsr.releaseSeconds == defaults.adsr.releaseSeconds); } // One unit, everywhere, across the formatter's whole range: a sub-millisecond value keeps a diff --git a/tests/test_sample_chrome.cpp b/tests/test_sample_chrome.cpp index 8d5c75a..8612986 100644 --- a/tests/test_sample_chrome.cpp +++ b/tests/test_sample_chrome.cpp @@ -8,6 +8,7 @@ // toolbar overlapping any other; degenerate bands yielding no inverted rects; and the preview // button's play-triangle glyph, which sits inside the button without changing its rect. +#include "../src/core/instrument/ui/knob_deck.h" #include "../src/core/instrument/ui/sample_bands.h" #include "../src/core/instrument/ui/sample_chrome.h" @@ -21,7 +22,9 @@ static int g_fail = 0; #define CHECK(cond) do { if(!(cond)) { \ std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) -static constexpr int kKnob = 40; // stands in for knob_deck's kDeckKnobSize +// The real constant, not a copy: the shell passes kDeckKnobSize into chromeRects, and a +// hand-copied stand-in here had already drifted from it once. +static constexpr int kKnob = kDeckKnobSize; static Rect chromeBand(int w = kEditorMinWidth, int h = kEditorMinHeight) { return computeSampleBands(w, h, 120).chrome; From 47f2a063e79d42631aa4398ae299e725c8218a62 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 10:01:40 -0400 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20close=20five=20=CE=98-W6-T1=20review?= =?UTF-8?q?=20minors=20=E2=80=94=20headroom=20figure,=20knob-face=20radius?= =?UTF-8?q?,=20degenerate=20band=20clamp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns inKnobFace's hit radius with computeKnob's draw-side min(w,h) rule and adds a non-square-rect test; clamps halfSpan for degenerate waveform bands with a test; fixes stale docs/comments and annotates an uncommitted perf measurement. --- docs/product/visual-design-language.md | 2 +- src/core/instrument/CLAUDE.md | 2 +- src/core/instrument/map/sample_map.h | 4 ++-- src/core/instrument/ui/knob_deck.cpp | 4 +++- src/core/instrument/ui/knob_deck.h | 8 +++++--- src/core/ui/component_geometry.cpp | 8 +++++++- tests/test_component_geometry.cpp | 17 ++++++++++++++++- tests/test_knob_deck.cpp | 22 ++++++++++++++++++++++ 8 files changed, 57 insertions(+), 10 deletions(-) diff --git a/docs/product/visual-design-language.md b/docs/product/visual-design-language.md index 15a58ee..000aeea 100644 --- a/docs/product/visual-design-language.md +++ b/docs/product/visual-design-language.md @@ -793,7 +793,7 @@ concession. Everything with a slope or a curve must draw through a primitive tha | Spline (drawn EG) contour | `editor_paint_waveform.cpp` `paintSplineOverlay` | **Fixed** — same treatment, one trace grammar. | | Velocity-curve popup trace | `editor_paint_curve.cpp` | **Fixed** — same treatment. | | Velocity-curve mini thumbnail | `editor_paint_curve.cpp` | Left at 1 px AA `LICE_Line` — a 2 px trace blots at thumbnail scale. | -| Waveform min/max columns | `draw_kit.cpp` `drawWaveform` | **Fixed** — column fill unchanged (it cannot alias), plus an AA `LICE_FLine` stroke joining each column's extremes to its neighbour's, in the same ink. Shared with the docked bank panel and the browser cards. **Measured cost** (Release, MSVC 14.44, real LICE, 24 stereo cards × 136 columns = 6528 columns): fill alone 0.070 ms per full-grid repaint, fill+stroke 0.48 ms — the stroke is ~0.41 ms, about 2.5% of a 60 Hz frame, and the grid repaints on hover/scroll/drag, not continuously. | +| Waveform min/max columns | `draw_kit.cpp` `drawWaveform` | **Fixed** — column fill unchanged (it cannot alias), plus an AA `LICE_FLine` stroke joining each column's extremes to its neighbour's, in the same ink. Shared with the docked bank panel and the browser cards. **Measured cost** (Release, MSVC 14.44, real LICE, 24 stereo cards × 136 columns = 6528 columns): fill alone 0.070 ms per full-grid repaint, fill+stroke 0.48 ms — the stroke is ~0.41 ms, about 2.5% of a 60 Hz frame, and the grid repaints on hover/scroll/drag, not continuously. One-off scratchpad measurement, 2026-08-01, harness not committed — not a standing regression guard; re-measure before relying on it again. | | Preview play triangle | `editor_paint_chrome.cpp` | **Fixed** — `LICE_FillTriangle` has no `aa`; its two sloped edges are re-stroked with AA `LICE_FLine`. | | Envelope/spline node handles (squares) | `editor_paint_waveform.cpp` | Already clean — axis-aligned `LICE_FillRect`. | | Envelope curve knots (circles) | `editor_paint_waveform.cpp` | Already clean — `LICE_FillCircle` with `aa=true`. | diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index 9c16dc3..d7fc685 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -310,7 +310,7 @@ anything for a trigger shape. - `browser_scroll` — scroll + type-to-filter layered over `capture_browser`: vertical scroll offset, scrollbar thumb, thumb-drag mapping, and name-substring search. - `param_slider` — parameter control-panel: vertical stack of TOGGLE (two-segment selector) and SLIDER (horizontal track) rows; maps normalized value to/from handle pixel. - `embed_strip` — compact single-row control layout for embed mode in the track FX chain. -- `knob_deck` — pure knob-deck layout + hit-test (FB1): group-box / caption-row / compact-toggle / knob-cell geometry, deterministic whole-group wrap, `DeckLayout` / `DeckHit`. Mirror of `action_bar`/`param_slider`; no LICE or REAPER types. Carries a SECOND hit-test, `hitTestKnobFace`, resolved against the drawn CIRCLES rather than the cell: a double-click reset is aimed at a dial, so the label band and the cell margins must miss where a drag grab deliberately does not, and only a radial resolve can tell the inner curve dial from the outer ring it sits inside. **The cell/knob/label sizes and `sample_bands`' editor floor move as a pair** — wider cells need a wider floor width or the deck wraps to a fourth row. A group carries TWO caption-toggle slots, laid right-to-left: the second exists because a group whose knob row is wider than its caption row has caption slack a toggle can occupy for free, and the deck has six pixels of headroom on its first row at the editor's floor width — a `rowToggle` would widen the GROUP and wrap the deck to a fourth row, past what the minimum window holds. **A group's cell run is a RESERVED WIDTH, not a fixed cell size**: a `-1` id reserves one cell's width without a cell, and the cells present divide the whole run between them at one uniform integer width (residue in symmetric end margins). That is what lets a mode flip drop controls from a face — Trigger's AMP and FILTER ENV lose their Sustain/Release stages — without either reflowing the deck or leaving dead slots in the box; a face with fewer controls simply gets roomier cells. Do not reintroduce fixed-width cells with blank slots. +- `knob_deck` — pure knob-deck layout + hit-test (FB1): group-box / caption-row / compact-toggle / knob-cell geometry, deterministic whole-group wrap, `DeckLayout` / `DeckHit`. Mirror of `action_bar`/`param_slider`; no LICE or REAPER types. Carries a SECOND hit-test, `hitTestKnobFace`, resolved against the drawn CIRCLES rather than the cell: a double-click reset is aimed at a dial, so the label band and the cell margins must miss where a drag grab deliberately does not, and only a radial resolve can tell the inner curve dial from the outer ring it sits inside. **The cell/knob/label sizes and `sample_bands`' editor floor move as a pair** — wider cells need a wider floor width or the deck wraps to a fourth row. A group carries TWO caption-toggle slots, laid right-to-left: the second exists because a group whose knob row is wider than its caption row has caption slack a toggle can occupy for free, and the deck has fourteen pixels of headroom on its first row at the editor's floor width — a `rowToggle` would widen the GROUP and wrap the deck to a fourth row, past what the minimum window holds. **A group's cell run is a RESERVED WIDTH, not a fixed cell size**: a `-1` id reserves one cell's width without a cell, and the cells present divide the whole run between them at one uniform integer width (residue in symmetric end margins). That is what lets a mode flip drop controls from a face — Trigger's AMP and FILTER ENV lose their Sustain/Release stages — without either reflowing the deck or leaving dead slots in the box; a face with fewer controls simply gets roomier cells. Do not reintroduce fixed-width cells with blank slots. - `deck_values` — the deck's control-id ↔ parameter-set BINDING and its display units, split from the editor shell on the same axis `deck_groups` was split from `knob_deck`: `deck_groups` says which controls exist, this says what each one's value MEANS. Holds `deckParamNorm` / diff --git a/src/core/instrument/map/sample_map.h b/src/core/instrument/map/sample_map.h index 18eb7a0..57da0df 100644 --- a/src/core/instrument/map/sample_map.h +++ b/src/core/instrument/map/sample_map.h @@ -4,8 +4,8 @@ // The bank is read over the live-state seam, audio over the file seam; both raw inputs // cross the bridge/file boundary in the shell, everything after (bank parse via the shared // bank_book JSON path, sample pick, channel policy, SampleData build) is pure and -// unit-tested here. Links bank_book, wav_codec, and play_params (all pure) — deliberately -// NOT the voice engine: the build's product is plain SampleData. +// unit-tested here. Links bank_book, wav_codec, play_params, and play_seconds (all pure) — +// deliberately NOT the voice engine: the build's product is plain SampleData. #include #include diff --git a/src/core/instrument/ui/knob_deck.cpp b/src/core/instrument/ui/knob_deck.cpp index 89c9ca2..66480ea 100644 --- a/src/core/instrument/ui/knob_deck.cpp +++ b/src/core/instrument/ui/knob_deck.cpp @@ -193,7 +193,9 @@ DeckHit hitTestDeck(const DeckLayout& layout, int x, int y) { bool inKnobFace(const Rect& knob, int x, int y) { const double dx = x - (knob.x + knob.width / 2.0); const double dy = y - (knob.y + knob.height / 2.0); - const double r = knob.width / 2.0; + // Matches computeKnob's radius rule (param_slider.cpp) so the hit rule never claims more + // circle than is actually drawn when a caller's rect is non-square (e.g. a squashed chrome row). + const double r = (std::min)(knob.width, knob.height) / 2.0; return dx * dx + dy * dy < r * r; } diff --git a/src/core/instrument/ui/knob_deck.h b/src/core/instrument/ui/knob_deck.h index e618eee..f13b995 100644 --- a/src/core/instrument/ui/knob_deck.h +++ b/src/core/instrument/ui/knob_deck.h @@ -153,9 +153,11 @@ struct DeckFaceHit { bool inner = false; // inside the concentric inner disc }; -// A point inside the circle inscribed in `knob`, boundary-EXCLUSIVE. THE target rule for the -// reset gesture wherever a radial knob is drawn — the deck's own faces and the chrome's -// preview-velocity dial both resolve through it, so one gesture cannot grow two target rules. +// A point inside the circle inscribed in `knob` — radius is min(width, height)/2, same rule as +// computeKnob's draw-side circle, so a non-square rect can never claim a hit past the drawn disc +// — boundary-EXCLUSIVE. THE target rule for the reset gesture wherever a radial knob is drawn — +// the deck's own faces and the chrome's preview-velocity dial both resolve through it, so one +// gesture cannot grow two target rules. bool inKnobFace(const Rect& knob, int x, int y); // Resolved against the drawn CIRCLES, not the cell: a reset is aimed at a dial, so the label diff --git a/src/core/ui/component_geometry.cpp b/src/core/ui/component_geometry.cpp index 8a9a8a1..48f6173 100644 --- a/src/core/ui/component_geometry.cpp +++ b/src/core/ui/component_geometry.cpp @@ -102,7 +102,13 @@ WaveformBand waveformBand(int bandTop, int bandHeight) { b.top = bandTop; b.height = bandHeight; b.midY = bandTop + bandHeight / 2; - b.halfSpan = (bandHeight / 2) - 2; // matches drawWaveform's 2px vertical breathing room + // matches drawWaveform's 2px vertical breathing room; clamped at 0 so a degenerate band + // (height <= 3, where this would otherwise go negative) can't flip a positive peak below + // the zero line. + const int rawHalfSpan = (bandHeight / 2) - 2; + b.halfSpan = rawHalfSpan > 0 ? rawHalfSpan : 0; + // lo/hi (waveformColumnSpan) are midY-height/2 .. midY+height/2-1 — asymmetric by one px for + // even heights. Unreachable while |compressAmplitudeForDisplay| <= 1, so left as-is. return b; } diff --git a/tests/test_component_geometry.cpp b/tests/test_component_geometry.cpp index bb5e764..4a5a5b8 100644 --- a/tests/test_component_geometry.cpp +++ b/tests/test_component_geometry.cpp @@ -197,7 +197,9 @@ static void testWaveformColumnCount() { // The regression this exists to catch: rounding applied to the resulting y instead of to the // scaled amplitude draws a symmetric column one pixel taller above the zero line than below. static void testSymmetricColumnDrawsEqualHeightAboveAndBelowTheZeroLine() { - const WaveformBand band = waveformBand(0, 41); // odd height -> fractional half-span + // Odd height so midY sits equidistant from lo/hi — the fractional half-span below comes + // from the amplitude product, not the height's parity. + const WaveformBand band = waveformBand(0, 41); // Sweep amplitudes whose scaled value is fractional, which is where the two edges can // round in opposite directions. for (int i = 1; i <= 20; ++i) { @@ -256,6 +258,18 @@ static void testBandMetricsMirrorTheDrawnInset() { CHECK(b.halfSpan == 18.0); // half the band, less the 2px breathing room } +// A band too short for the 2px breathing room (height <= 4, so height/2 - 2 <= 0) must clamp +// halfSpan to 0 rather than go negative and mirror every positive column below the zero line. +static void testDegenerateBandClampsHalfSpanToZero() { + CHECK(waveformBand(0, 4).halfSpan == 0.0); + CHECK(waveformBand(0, 3).halfSpan == 0.0); + CHECK(waveformBand(0, 0).halfSpan == 0.0); + // A zero half-span collapses every column onto the zero line regardless of amplitude sign. + const WaveformBand b = waveformBand(10, 4); + const WaveformColumnSpan s = waveformColumnSpan(b, 1.0, -1.0); + CHECK(s.top == b.midY && s.bottom == b.midY); +} + int main() { testHitTestBoxHalfOpen(); testButtonBoxInset(); @@ -279,6 +293,7 @@ int main() { testTallerAmplitudeNeverDrawsAShorterColumn(); testBothEdgesAndTheStrokeClampToTheBand(); testBandMetricsMirrorTheDrawnInset(); + testDegenerateBandClampsHalfSpanToZero(); if (g_fail == 0) std::printf("component_geometry: all tests passed\n"); else std::printf("component_geometry: %d CHECK(s) FAILED\n", g_fail); diff --git a/tests/test_knob_deck.cpp b/tests/test_knob_deck.cpp index 0ce3f89..23fe0d4 100644 --- a/tests/test_knob_deck.cpp +++ b/tests/test_knob_deck.cpp @@ -382,6 +382,27 @@ static void testKnobFaceResolvesInnerRingOuterRingAndMisses() { CHECK(hitTestKnobFace(dl, cx + diag, cy + diag).id == -1); } +// A non-square knob rect (e.g. a squashed chrome row clamps knob height below its width) must +// resolve against min(width, height)/2 — the same radius computeKnob draws — never against +// width alone, or the hit disc would claim territory above/below where nothing is drawn. +static void testInKnobFaceUsesTheSmallerDimensionOnANonSquareRect() { + const Rect wide{0, 0, 40, 20}; // width > height: draws a 10px-radius disc, not 20px + const int cx = wide.x + wide.width / 2; + const int cy = wide.y + wide.height / 2; + CHECK(inKnobFace(wide, cx, cy)); // dead centre always hits + CHECK(inKnobFace(wide, cx, cy + 9)); // just inside the drawn (height-limited) radius + CHECK(!inKnobFace(wide, cx, cy + 10)); // on the drawn rim: exclusive miss + CHECK(!inKnobFace(wide, cx, cy + 15)); // inside the RECT but outside the smaller-radius disc + CHECK(!inKnobFace(wide, cx + 15, cy)); // same check along the wider axis + + const Rect tall{0, 0, 20, 40}; // height > width: draws a 10px-radius disc, not 20px + const int tcx = tall.x + tall.width / 2; + const int tcy = tall.y + tall.height / 2; + CHECK(inKnobFace(tall, tcx, tcy)); + CHECK(!inKnobFace(tall, tcx + 15, tcy)); + CHECK(!inKnobFace(tall, tcx, tcy + 15)); +} + static void testEmptyDeck() { const std::vector none; CHECK(deckRowCount(none, 800) == 0); @@ -401,6 +422,7 @@ int main() { testCaptionRadioGeometryAndHit(); testInnerDialHit(); testKnobFaceResolvesInnerRingOuterRingAndMisses(); + testInKnobFaceUsesTheSmallerDimensionOnANonSquareRect(); testCaptionToggle2(); testEmptyDeck(); if (g_fail) { From d445cfdae36b03ede997cafab88187c2fc3ecd4e Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 10:07:47 -0400 Subject: [PATCH 4/4] test: tighten waveform-collapse and knob-face rect assertions Height-3 case pins the halfSpan clamp itself (height-4 passed pre-fix too); dropped the vacuous tall-rect mirror since min() is symmetric and wide already discriminates. --- tests/test_component_geometry.cpp | 7 ++++++- tests/test_knob_deck.cpp | 8 ++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_component_geometry.cpp b/tests/test_component_geometry.cpp index 4a5a5b8..14991ce 100644 --- a/tests/test_component_geometry.cpp +++ b/tests/test_component_geometry.cpp @@ -265,9 +265,14 @@ static void testDegenerateBandClampsHalfSpanToZero() { CHECK(waveformBand(0, 3).halfSpan == 0.0); CHECK(waveformBand(0, 0).halfSpan == 0.0); // A zero half-span collapses every column onto the zero line regardless of amplitude sign. - const WaveformBand b = waveformBand(10, 4); + // height 3 (not 4): pre-clamp this rawHalfSpan is -1, so this is the case that pins the + // clamp itself, not just a halfSpan-already-zero band. + const WaveformBand b = waveformBand(10, 3); const WaveformColumnSpan s = waveformColumnSpan(b, 1.0, -1.0); CHECK(s.top == b.midY && s.bottom == b.midY); + // Domain note, not a bug: waveformBand(t, 0) still has lo=t, hi=t-1 (an inverted span) even + // though halfSpan clamps to 0 — unreachable via the band allocator (draw_kit.cpp never + // divides down to a 0-height band) and harmless if it were (LICE clips a reversed 1px line). } int main() { diff --git a/tests/test_knob_deck.cpp b/tests/test_knob_deck.cpp index 23fe0d4..a43c3ec 100644 --- a/tests/test_knob_deck.cpp +++ b/tests/test_knob_deck.cpp @@ -395,12 +395,8 @@ static void testInKnobFaceUsesTheSmallerDimensionOnANonSquareRect() { CHECK(!inKnobFace(wide, cx, cy + 15)); // inside the RECT but outside the smaller-radius disc CHECK(!inKnobFace(wide, cx + 15, cy)); // same check along the wider axis - const Rect tall{0, 0, 20, 40}; // height > width: draws a 10px-radius disc, not 20px - const int tcx = tall.x + tall.width / 2; - const int tcy = tall.y + tall.height / 2; - CHECK(inKnobFace(tall, tcx, tcy)); - CHECK(!inKnobFace(tall, tcx + 15, tcy)); - CHECK(!inKnobFace(tall, tcx, tcy + 15)); + // No mirrored tall{20,40} case: min() is symmetric in its two arguments, so a tall rect + // can't discriminate width/2 from min(w,h)/2 any differently than wide already does. } static void testEmptyDeck() {