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;