From 0a77b0071dce03f3f4ceb65fa26083aaac2f2173 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:37:06 -0400 Subject: [PATCH 01/12] Docs: list PLAN.md and VERIFICATION.md in CLAUDE.md, correct TODO-1.0.md's role --- CLAUDE.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 88d0491..0493f0a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co **ReaSampler** is a per-project audio sample-bank capture tool that builds two artifacts: the REAPER extension (`reaper_reasampler`) and **ReaSampler 9000**, a Windows-only VST3 sampler instrument (`reasampler_9000.vst3`, `core/instrument/` + `shell/instrument/`, second CMake target `reasampler_vst`, gated on the vendored `vendor/vst3sdk` submodule slice). The pure-testable-core / REAPER-facing-shell discipline is preserved throughout: `core/` never includes REAPER or VST3 SDK types, `shell/` is where those hosts are actually touched, `app/` is the extension entry point. Every REAPER API name cited in project docs is correct-by-intent; verify argument order, types, and flag values against `vendor/reaper-sdk/sdk/reaper_plugin_functions.h` before use. -Per-module detail — what each file owns, its invariants — lives in the twenty-eight per-directory `src/**/CLAUDE.md` files; see the compact map in "Architecture: the load-bearing split" below to find the right one. Landed-phase history lives in `docs/ARCHIVE.md`; current work lives in `docs/COMPLETED.md`, `docs/TODO.md`, and `docs/TODO-1.0.md` — see "Project docs" below. +Per-module detail — what each file owns, its invariants — lives in the twenty-eight per-directory `src/**/CLAUDE.md` files; see the compact map in "Architecture: the load-bearing split" below to find the right one. Landed-phase history lives in `docs/ARCHIVE.md`; current work lives in `docs/PLAN.md`, `docs/COMPLETED.md`, `docs/TODO.md`, and `docs/TODO-1.0.md` — see "Project docs" below. ## Settled decisions @@ -201,8 +201,10 @@ Files: `bank-package.md`, `capture-tail.md`, `code-organization.md`, `code-quali Plan-style docs live under `docs/`: - `docs/ARCHIVE.md` — pre-1.0 history, rarely read, not a source of context for current work. - `docs/COMPLETED.md` — 1.x landed items. +- `docs/PLAN.md` — the active post-1.0 roadmap: phases, waves, tracks, and rulings that implementation specialists are dispatched against directly. - `docs/TODO.md` — deferred follow-ups with recorded rationale. -- `docs/TODO-1.0.md` — the raw 1.x work list, not yet structured into a plan. +- `docs/TODO-1.0.md` — retained as PLAN.md's verbatim-provenance appendix: Daniel's raw asks and every answer round, unedited, with a traceability table mapping each item onto the PLAN.md track that owns it. +- `docs/VERIFICATION.md` — the in-REAPER hand-test checklist for post-1.0 work no unit test can close, plus per-feature companion scripts (`docs/verify-*.md`, e.g. `verify-track-scope-multitrack.md`, `verify-package-transfer.md`). ## The load-bearing principle From fe1aec5925ccf56d258810f9f0e441a0c1696005 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:39:39 -0400 Subject: [PATCH 02/12] Hand a card dragged onto a plugin's own editor to the OS as a file drop; withdraw the OLE drag when the pointer returns to REAPER A code-ownership proof (window class procedure's module, via the pure window_origin) plus REAPER's FX-surface token routes a plugin editor to OLE; FX chrome keeps the instrument drop. The OLE loop re-asks the law each turn, so passing a plugin editor stays reversible. --- src/app/CMakeLists.txt | 2 +- src/core/ui/CLAUDE.md | 35 ++++-- src/core/ui/CMakeLists.txt | 3 + src/core/ui/drag_out.cpp | 29 +++-- src/core/ui/drag_out.h | 50 +++++++-- src/core/ui/window_origin.cpp | 48 ++++++++ src/core/ui/window_origin.h | 40 +++++++ src/core/wire/instrument_drop.h | 7 +- src/shell/actions/CLAUDE.md | 4 +- src/shell/actions/drag_out_win.cpp | 152 ++++++++++++++++++++++--- src/shell/actions/drag_out_win.h | 34 +++++- src/shell/panel/panel_drag.cpp | 70 +++++++++--- tests/test_drag_out.cpp | 175 +++++++++++++++++++++++++---- tests/test_window_origin.cpp | 162 ++++++++++++++++++++++++++ 14 files changed, 720 insertions(+), 91 deletions(-) create mode 100644 src/core/ui/window_origin.cpp create mode 100644 src/core/ui/window_origin.h create mode 100644 tests/test_window_origin.cpp diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index d07fb5e..1a801c0 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -62,7 +62,7 @@ add_library(reaper_reasampler MODULE ${REASAMPLER_SRC_DIR}/shell/actions/insert_fx_action.cpp ${REASAMPLER_SRC_DIR}/shell/persist/usage_scan.cpp ) -target_link_libraries(reaper_reasampler PRIVATE json wire file_bytes bank_model capture_paths capture_name peaks bank_grid mode_switch tab_strip view_mode_model view_tree fx_state applied_state guid_diff lane_keys solo_cache insert_plan render_settings render_window track_topology batch_capture tail_control capture_realtime bank_book wav_codec origin_ledger tracking_authority prune_reconcile prune_button app_version provenance drag_out instrument_drop theme component_geometry action_bar footer_bar overflow_menu mode_enable insert_fx_enable tooltip card_meta card_drag assignment_request bank_sync sample_usage bake_wire resample_name export_bank package_pickers) +target_link_libraries(reaper_reasampler PRIVATE json wire file_bytes bank_model capture_paths capture_name peaks bank_grid mode_switch tab_strip view_mode_model view_tree fx_state applied_state guid_diff lane_keys solo_cache insert_plan render_settings render_window track_topology batch_capture tail_control capture_realtime bank_book wav_codec origin_ledger tracking_authority prune_reconcile prune_button app_version provenance drag_out window_origin instrument_drop theme component_geometry action_bar footer_bar overflow_menu mode_enable insert_fx_enable tooltip card_meta card_drag assignment_request bank_sync sample_usage bake_wire resample_name export_bank package_pickers) # NOT linked here, deliberately: sampler_core / pitch_shift / the filter / limiter. The # instrument renders its own bake in its own process, which is what keeps the extension's # link graph free of the voice engine — a link edge to it here means the design drifted. diff --git a/src/core/ui/CLAUDE.md b/src/core/ui/CLAUDE.md index bfc5a22..6c2c720 100644 --- a/src/core/ui/CLAUDE.md +++ b/src/core/ui/CLAUDE.md @@ -86,16 +86,28 @@ L7 sub-pass, 2026-07-27): - **The drag-out law is per-move and stateless.** The class is resolved from the current pointer on every move and again at the release point; nothing is latched between evaluations. That is what makes every transition reversible and what - makes drag speed (WM_MOUSEMOVE coalescing) unable to change an outcome. Leaving - REAPER entirely is the one irreversible transition, because the OS hand-off goes - modal — so it must be PROVEN (a window-ownership test in the shell), never inferred - from a hit-test token REAPER could not name; an unrecognised surface *inside* REAPER - refuses. That proof is necessary but NOT sufficient: a plugin bridged into - `reaper_host*.exe` fails the process test while still being REAPER's own UI, so the - hand-off additionally requires REAPER's hit-test to have named nothing — a recognised - token can only ever veto the hand-off, never produce one. A first-move class lock and - a drag-lifetime "cannot hand off" latch both existed here and were removed — do not - reintroduce either. + makes drag speed (WM_MOUSEMOVE coalescing) unable to change an outcome. The OS + hand-off goes modal, so it must be PROVEN by a window test in the shell, never + inferred from a hit-test token REAPER could not name; an unrecognised surface + *inside* REAPER refuses. There are exactly two proofs, each necessary but NOT + sufficient, each paired with a hit-test condition: + - **Leaving REAPER** (process ownership). A plugin bridged into `reaper_host*.exe` + fails the process test while still being inside REAPER, so this route also requires + REAPER's hit-test to have named nothing — here a recognised token can only VETO. + - **A plugin's own editor** (code ownership, `window_origin`). The proof alone would + hand off over any extension window docked in REAPER, so this route also REQUIRES the + FX-surface token; within an FX window the proof is what separates the plugin's editor + (OS hand-off — the plugin takes the files as it would from Explorer) from REAPER's + chrome (instrument drop, unchanged). + + Once handed off, the law keeps running: on Windows the OS drag loop re-asks it every + turn (`decideOsDragTurn`) and WITHDRAWS the moment it stops answering `OsHandoff`, + giving the gesture back to the panel — or, at a release, committing REAPER's own + outcome at that point. So neither a pass over a plugin's editor nor a return from + outside REAPER costs a REAPER-internal outcome. SWELL's file drag exposes no such hook, + so off Windows the hand-off stays one-way (and there is no plugin-editor route there). + A first-move class lock and a drag-lifetime "cannot hand off" latch both existed here + and were removed — do not reintroduce either. ## Modules @@ -103,7 +115,8 @@ L7 sub-pass, 2026-07-27): - `bank_grid` — REAPER-free grid layout, selection, keyboard-nav, and thumbnail-cache-key logic for the docked bank panel. - `tab_strip` — REAPER-free scrollable tab-strip layout + hit-test for the named-banks strip. - `prune_button` — pure layout/hit-test for the `bank_panel` footer Prune button. -- `drag_out` — the pure drag-out gesture law plus path-list assembly. Owns the `ReaperSurface` vocabulary (TrackPanel / FxSurface / FxEmbed / Arrange / Other — `core/wire/instrument_drop` classifies REAPER's info token INTO it; there is deliberately no off-REAPER member, since no token can carry that fact), `decideDropClass` (surface × single-vs-multi payload → Internal / InstrumentDrop / ArrangeInsert / Refuse / OsHandoff / None), and `cueForDropClass`. `OsHandoff` is reachable ONLY through `DropContext::pointerOffHost` — a positive window-ownership proof the shell owes the law (`drag_out_win::pointerOverHostWindow`) — AND only where REAPER's own hit-test named nothing (`Other`, no track), gated ahead of the surface switch, so neither evidence source can give a live drag away to OLE on its own. **No `DropClass` means "nothing happens"**: a surface with no defined outcome for the payload resolves to `Refuse`, which the shell shows as a cursor, so "no silent no-op release" is a property of the enumeration rather than of any call site. +- `drag_out` — the pure drag-out gesture law plus path-list assembly. Owns the `ReaperSurface` vocabulary (TrackPanel / FxSurface / FxEmbed / Arrange / Other — `core/wire/instrument_drop` classifies REAPER's info token INTO it; there is deliberately no off-REAPER member, since no token can carry that fact), `decideDropClass` (surface × single-vs-multi payload → Internal / InstrumentDrop / ArrangeInsert / Refuse / OsHandoff / None), and `cueForDropClass`. `OsHandoff` is reachable ONLY through the two proof members of `DropContext` — `pointerOffHost` (`drag_out_win::pointerOverHostWindow`) where REAPER's hit-test named nothing (`Other`, no track), and `pointerOverPluginEditor` (`drag_out_win::pointerOverPluginEditor`) where it named `FxSurface` — both gated ahead of the surface switch, so no evidence source can give a live drag away to OLE on its own (the invariant above states why each pairing). Also owns `decideOsDragTurn`, the OS drag loop's per-turn Continue / Drop / Cancel / Withdraw / WithdrawAtRelease verdict. **No `DropClass` means "nothing happens"**: a surface with no defined outcome for the payload resolves to `Refuse`, which the shell shows as a cursor, so "no silent no-op release" is a property of the enumeration rather than of any call site. +- `window_origin` — whose code implements a window (Host / System / Plugin / Unknown, from the owning process and the module holding the window CLASS's procedure) and `isPluginEditorChain`, the rule behind `pointerOverPluginEditor`: any Plugin window from the hit window up to its root. Unknown is never proof, and without the OS directory every non-host module degrades to Unknown — both keep the failure direction toward REAPER's own outcome, never toward the modal hand-off. The shell reads the facts; the known misses are listed at `drag_out_win.h`'s `pointerOverPluginEditor`. - `theme` — pure palette module: role→color mapping, REAPER-grey neutral ladder + the pastel accent system, the keyboard strip's spectral ramp, WCAG contrast-floor helpers + `compositeOver` (the effective color of a translucent fill, so alpha overlays are testable). Only the ramp's MID stop is its own constant; lo/hi are still aliases of `accent/primary`/`accent/tertiary`, so a categorical accent move CAN still reorder the ramp — `testSpectralRampLuminanceIsMonotonic` is the build-time catch, not the structure. - `component_geometry` — pure button/slider/list-row geometry + hover hit-test helpers. - `action_bar` — pure task-grouped action-bar layout/hit-test: clusters (Capture / Placement / Maintenance / Tagging / Switching). diff --git a/src/core/ui/CMakeLists.txt b/src/core/ui/CMakeLists.txt index 78a7860..f099cf7 100644 --- a/src/core/ui/CMakeLists.txt +++ b/src/core/ui/CMakeLists.txt @@ -10,6 +10,9 @@ reasampler_test(prune_button LINK prune_button) reasampler_pure_library(drag_out SOURCES drag_out.cpp) reasampler_test(drag_out LINK drag_out) +reasampler_pure_library(window_origin SOURCES window_origin.cpp) +reasampler_test(window_origin LINK window_origin) + reasampler_pure_library(theme SOURCES theme.cpp) reasampler_test(theme LINK theme) diff --git a/src/core/ui/drag_out.cpp b/src/core/ui/drag_out.cpp index 9917eeb..934f307 100644 --- a/src/core/ui/drag_out.cpp +++ b/src/core/ui/drag_out.cpp @@ -24,17 +24,24 @@ DropClass decideDropClass(int px, int py, const PanelClientRect& client, if (!ctx.drag.dragging || !ctx.drag.hasArmedSamples) return DropClass::None; if (insideClient(px, py, client)) return DropClass::Internal; - // The ONE route to OsHandoff, ahead of the surface switch, and it needs BOTH halves. The - // shell's ownership proof alone is not sufficient: REAPER runs a bridged plugin's UI in a - // separate process, so a window failing that test can still be REAPER's own UI. A recognised - // token therefore VETOES the hand-off instead of producing one — a veto can only move the - // answer toward staying inside REAPER, so pointerOffHost remains the only member whose true - // value can reach OLE. An off-host point REAPER DID name falls through to that surface's own - // outcome below: over a bridged FX window that is the instrument drop, as it is for a native - // one. + // The two routes to OsHandoff, both ahead of the surface switch, each needing BOTH halves. + // + // Leaving REAPER. The process proof alone is not sufficient: REAPER runs a bridged plugin's UI + // in a separate process, so a window failing that test can still be inside REAPER. A + // recognised token therefore VETOES this route instead of producing it — a veto can only move + // the answer toward staying inside REAPER. An off-host point REAPER DID name falls through to + // the second route, then to that surface's own outcome below. if (ctx.pointerOffHost && ctx.surface == ReaperSurface::Other && !ctx.haveTrack) return DropClass::OsHandoff; + // A plugin's own editor, with the reverse pairing: REAPER's naming is REQUIRED here, not a + // veto. The code proof alone would hand off over any extension window docked in REAPER; the + // FX-surface token confines it to FX windows, and the proof picks the plugin's editor out of + // that window's REAPER chrome, which keeps its instrument outcome below. A bridged editor + // lands here too — the token that vetoed the first route admits this one. + if (ctx.pointerOverPluginEditor && ctx.surface == ReaperSurface::FxSurface) + return DropClass::OsHandoff; + switch (ctx.surface) { case ReaperSurface::TrackPanel: case ReaperSurface::FxSurface: @@ -100,4 +107,10 @@ OsHandoff decideOsHandoff(const std::vector& paths) { return out; } +OsDragTurn decideOsDragTurn(bool escapePressed, bool buttonDown, bool lawHandsOff) { + if (escapePressed) return OsDragTurn::Cancel; + if (buttonDown) return lawHandsOff ? OsDragTurn::Continue : OsDragTurn::Withdraw; + return lawHandsOff ? OsDragTurn::Drop : OsDragTurn::WithdrawAtRelease; +} + } // namespace reasampler::ui diff --git a/src/core/ui/drag_out.h b/src/core/ui/drag_out.h index 96d8b9e..961dabe 100644 --- a/src/core/ui/drag_out.h +++ b/src/core/ui/drag_out.h @@ -4,9 +4,10 @@ // hit-testing, cursor setting, and outcome execution stay in the shell (panel_drag.cpp + // instrument_drop_win / arrange_drop_win / drag_out_win). // -// THE LAW: the class is resolved from what is under the cursor on EVERY move; every transition -// is reversible until release or until the pointer leaves REAPER entirely; the OS hand-off is -// reserved for leaving REAPER (DropContext::pointerOffHost owns what proves that) and every +// THE LAW: the class is resolved from what is under the cursor on EVERY move — and on every turn +// of the OS drag loop once handed off (decideOsDragTurn), so every transition is reversible until +// release wherever the OS loop can be re-asked; the OS hand-off is reserved for targets that are +// not REAPER's own UI (DropContext's two proof members own what establishes that) and every // REAPER-internal target executes natively on release. Do not reintroduce a first-move class // lock or a drag-lifetime "blocked" latch. // @@ -55,7 +56,7 @@ enum class DropClass { InstrumentDrop, // a track's panel or FX surface — add ReaSampler 9000 preloaded ArrangeInsert, // the timeline — place items at the pointer's track and time Refuse, // a REAPER surface with no defined outcome for this payload — cue it - OsHandoff, // the pointer left REAPER — hand the file list to the OS + OsHandoff, // not REAPER's own UI (off-host, or a plugin's editor) — hand the files to the OS }; // Everything the law reads. Nothing here is remembered between evaluations: two evaluations at @@ -65,15 +66,22 @@ struct DropContext { bool singlePayload = false; // payload holds EXACTLY one capture (arms InstrumentDrop) ReaperSurface surface = ReaperSurface::Other; // only read outside the client rect bool haveTrack = false; // GetThingFromPoint returned a non-null MediaTrack* - // THE ONE HOME for the hand-off's evidence rule. This is the only member whose true value - // can reach the irreversible OS hand-off, and it is a POSITIVE proof the shell owes the law - // — a window-ownership test (drag_out_win::pointerOverHostWindow), never "the hit-test named - // nothing I recognise", which is equally true of the transport, the toolbar and the docker - // chrome; reading an unnamed token as an exit is what handed live drags to OLE mid-gesture. - // Defaults false so an unset context stays inside REAPER: the failure mode of forgetting to - // resolve it must be a refusal, never a drag given away to OLE. NECESSARY but not sufficient - // — decideDropClass's gate carries the second half and why it needs one. + // THE ONE HOME for the hand-off's evidence rule. This member and pointerOverPluginEditor are + // the only two whose true value can reach the modal OS hand-off, and each is a POSITIVE proof + // the shell owes the law — a window test, never "the hit-test named nothing I recognise", + // which is equally true of the transport, the toolbar and the docker chrome; reading an + // unnamed token as an exit is what handed live drags to OLE mid-gesture. Both default false + // so an unset context stays inside REAPER: forgetting to resolve one must be a refusal, never + // a drag given away to OLE. Each is NECESSARY but not sufficient — decideDropClass pairs each + // with a hit-test condition, and says why. + // + // Process ownership (drag_out_win::pointerOverHostWindow): the window under the pointer is not + // the host process's. bool pointerOffHost = false; + // Code ownership (drag_out_win::pointerOverPluginEditor, deciding via ui::window_origin): the + // window under the pointer, or one of its ancestors, is implemented by plugin code — a plugin's + // own editor, which takes files the way it takes them from Explorer. + bool pointerOverPluginEditor = false; }; // Resolves the class for a drag at pointer (px, py) over `client`. Position-and-context only, @@ -131,4 +139,22 @@ struct OsHandoff { // means the internal drag stays live rather than dying half-torn-down. OsHandoff decideOsHandoff(const std::vector& paths); +// --- The OS drag loop, once handed off --------------------------------------------- + +// One turn of the OS drag loop. The hand-off does NOT end the law: the loop re-asks it at the +// current pointer every turn, and the moment it stops answering OsHandoff the OS drag is +// WITHDRAWN and the gesture given back to the panel — which is what keeps a pass over a plugin's +// editor, or a return from outside REAPER, from costing the user every REAPER-internal outcome. +enum class OsDragTurn { + Continue, // still over an OS target, button held + Drop, // released over an OS target: the OS delivers the files + Cancel, // Escape: the whole drag ends, nothing happens (unchanged) + Withdraw, // button held over a REAPER surface: resume the internal drag + WithdrawAtRelease, // released over a REAPER surface: resume, then commit it at that point +}; + +// Escape outranks everything, so a cancel can never be read as a release anywhere. +// `lawHandsOff` is decideDropClass(...) == OsHandoff at the current pointer; ignored on Escape. +OsDragTurn decideOsDragTurn(bool escapePressed, bool buttonDown, bool lawHandsOff); + } // namespace reasampler::ui diff --git a/src/core/ui/window_origin.cpp b/src/core/ui/window_origin.cpp new file mode 100644 index 0000000..6248ad8 --- /dev/null +++ b/src/core/ui/window_origin.cpp @@ -0,0 +1,48 @@ +// window_origin — pure implementation. See window_origin.h. + +#include "core/ui/window_origin.h" + +#include + +namespace reasampler::ui { + +namespace { + +// Case-folded (ASCII only — Windows' own directory names are ASCII), separator-unified, with the +// "\\?\" long-path prefix GetModuleFileName may report dropped, so the same directory compares +// equal however each API chose to spell it. +std::string normalizePath(const std::string& p) { + std::string s = p.rfind("\\\\?\\", 0) == 0 ? p.substr(4) : p; + for (char& c : s) { + if (c == '/') c = '\\'; + else if (c >= 'A' && c <= 'Z') c = static_cast(c - 'A' + 'a'); + } + while (!s.empty() && s.back() == '\\') s.pop_back(); + return s; +} + +// Directory containment, not string prefix: C:\Windows must not claim C:\WindowsApps. +bool isBeneath(const std::string& path, const std::string& dir) { + const std::string p = normalizePath(path); + const std::string d = normalizePath(dir); + return !d.empty() && p.size() > d.size() && p.compare(0, d.size(), d) == 0 && + p[d.size()] == '\\'; +} + +} // namespace + +WindowCode classifyWindowCode(const WindowFacts& facts, const std::string& systemRoot) { + // Another process's procedure address means nothing in ours; the process boundary is itself + // the proof. Inside an FX window that is the bridged plugin's editor. + if (facts.otherProcess) return WindowCode::Plugin; + if (!facts.procResolved) return WindowCode::Unknown; + if (facts.hostModule) return WindowCode::Host; + if (normalizePath(systemRoot).empty()) return WindowCode::Unknown; + return isBeneath(facts.modulePath, systemRoot) ? WindowCode::System : WindowCode::Plugin; +} + +bool isPluginEditorChain(const std::vector& chain) { + return std::find(chain.begin(), chain.end(), WindowCode::Plugin) != chain.end(); +} + +} // namespace reasampler::ui diff --git a/src/core/ui/window_origin.h b/src/core/ui/window_origin.h new file mode 100644 index 0000000..cc10c07 --- /dev/null +++ b/src/core/ui/window_origin.h @@ -0,0 +1,40 @@ +#pragma once +// window_origin — whose code implements a window, and whether the window chain under the pointer +// is a plugin's own editor rather than REAPER's chrome. The shell reads the facts (owning process, +// the module holding the window CLASS's procedure); this module decides, so the rule is provable +// without a DAW. Evidence feeding ui::DropContext::pointerOverPluginEditor. + +#include +#include + +namespace reasampler::ui { + +enum class WindowCode { + Host, // the host executable itself — REAPER's own chrome and containers + System, // an OS module (standard controls, dialog class): says nothing about who placed it + Plugin, // any other code — a plugin binary, or a window of another process (bridged plugin) + Unknown, // the procedure resolved to no loaded module — no evidence either way +}; + +// What the shell observed about ONE window. The CLASS procedure, not the instance one, because an +// extension subclassing a REAPER window swaps only the instance procedure — reading that would +// make REAPER's own chrome look like foreign code. +struct WindowFacts { + bool otherProcess = false; // the window belongs to a process other than the host's + bool procResolved = false; // the class procedure lies inside a loaded module + bool hostModule = false; // ...and that module is the host executable + std::string modulePath; // that module's file path, UTF-8 +}; + +// `systemRoot` is the OS directory (GetSystemWindowsDirectory); a module beneath it is System. +// An empty root makes every non-host module Unknown rather than Plugin: without it a standard +// button in REAPER's chrome would be indistinguishable from plugin code. +WindowCode classifyWindowCode(const WindowFacts& facts, const std::string& systemRoot); + +// `chain` runs from the window under the pointer up to and including its top-level root. True iff +// any window in it is Plugin code: a plugin's editor sits beneath REAPER's container, and a +// standard control INSIDE that editor is still under the plugin's own window. Unknown is never +// proof — the hand-off this gates is modal, so it needs positive evidence. +bool isPluginEditorChain(const std::vector& chain); + +} // namespace reasampler::ui diff --git a/src/core/wire/instrument_drop.h b/src/core/wire/instrument_drop.h index 720e779..bfcf6fc 100644 --- a/src/core/wire/instrument_drop.h +++ b/src/core/wire/instrument_drop.h @@ -1,8 +1,9 @@ #pragma once // instrument_drop — pure payload-construction core of drop-and-load: dropping a -// bank capture onto a track's panel or FX surface instantiates ReaSampler 9000 on -// that track already playing that capture. Also the classifier that names those -// surfaces. No REAPER/SWELL/VST3 SDK/vendor includes +// bank capture onto a track's panel or an FX window's REAPER chrome instantiates +// ReaSampler 9000 on that track already playing that capture (a plugin's own editor +// inside an FX window takes the files instead — ui::decideDropClass). Also the +// classifier that names those surfaces. No REAPER/SWELL/VST3 SDK/vendor includes // (+ the pure sample_map it reuses and the SDK-free UID macros in // reasampler_uid.h); unit-tested outside the DAW. // diff --git a/src/shell/actions/CLAUDE.md b/src/shell/actions/CLAUDE.md index 3e1e13e..561f479 100644 --- a/src/shell/actions/CLAUDE.md +++ b/src/shell/actions/CLAUDE.md @@ -57,8 +57,8 @@ is owned by other directories and only skinned here. - `shell/actions` (`action_registry` / `design_view_actions` / `bank_actions` / `prune_action`) — the bindable action families, all routed via the `command_id`/`gaccel`/`hookcommand` contract. `action_registry` owns the shared registration plumbing (interned channel-qualified id strings; register and mirror-unregister present the identical pointer) **and the Q-W6 registration TABLE**: `main.cpp`'s own family (capture scopes, panel toggle, insert, batch, realtime, recapture, version) is one `ActionTableRow` array — suffix, phrase, flat function-pointer handler — that registration, hookcommand dispatch, and the unload mirror-unregister all iterate, so adding an action touches the table only (OCP). Bank mutations flow through the promptless `shell/bank_ops` verbs (`bankOp*` + `persistBankOp`, taking `ReaSamplerSession&`), which the panel menus and `bank_actions` consume as thin UX skins; `src/shell/bank_ops/CLAUDE.md` owns the one-op-one-Ctrl-Z rule those verbs enforce. The prune action (`prune_action`, `BANK_PRUNE_FOLDER`) is **the ONLY file-deletion action in the system**; it opens no undo point (file deletion is not REAPER-undoable). `BANK_PRUNE_FOLDER` halts on `blockedByTracking` and prints each blocker that fired, with recovery instructions. - `package_export_action` — the "export bank as package" skin: survey and report first, confirm what is absent (and, separately, a destination being replaced), pick a destination, write. Every prompt in the flow lives here so `shell/package/export_bank` stays promptless. Read-only against the project — it holds the session by `const&`, so no ext-state write, generation bump or undo point is reachable. Registration rides `main.cpp`'s action table (`EXPORT_BANK_PACKAGE`); the panel's tab menu is the second skin over the same body. -- `drag_out_win` — the host/OS boundary of drag-out: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. Also owns `pointerOverHostWindow`, the window-ownership test behind `ui::DropContext::pointerOffHost` (`core/ui/CLAUDE.md` owns the hand-off rule it feeds) — process identity on Windows (`WindowFromPoint` + `GetWindowThreadProcessId`), non-null `WindowFromPoint` under SWELL, whose window list is already this-process-only. Its shipped exception: a plugin bridged into `reaper_host*.exe` reports off-host though the pointer never left REAPER. -- `instrument_drop_win` — instrument-drop shell: `probeDropTarget` resolves a screen point to a track + a `ReaperSurface` (via the pure `wire::classifyReaperSurface`, whose token rules `core/wire/CLAUDE.md` owns) — it answers only what REAPER named, never whether the pointer is still in REAPER — and the drop half adds a ReaSampler 9000 instance and applies the dragged capture's state via a transient `.vstpreset` + `TrackFX_SetPreset` (the former `TrackFX_SetNamedConfigParm` "vst_chunk" write was silently unappliable for VST3). Exposes `loadInstrumentOntoTrack` (inner half, no own undo block) and `performInstrumentDrop` (wraps in its own undo block). **Never captures, never writes the bank, never inserts a timeline item.** +- `drag_out_win` — the host/OS boundary of drag-out: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. Owns the two window tests behind `ui::DropContext`'s hand-off proofs (`core/ui/CLAUDE.md` owns the rule they feed). `pointerOverHostWindow` is process identity on Windows (`WindowFromPoint` + `GetWindowThreadProcessId`), non-null `WindowFromPoint` under SWELL, whose window list is already this-process-only; its shipped exception: a plugin bridged into `reaper_host*.exe` reports off-host though the pointer never left REAPER. `pointerOverPluginEditor` walks from the hit window to its root reading each window's owning process and the module of its CLASS procedure (`GetClassLongPtr(GCLP_WNDPROC)` → `GetModuleHandleEx(FROM_ADDRESS)`), and lets `ui::window_origin` decide; its failure modes are listed at the declaration, and it is always false under SWELL. The OLE drop source re-asks the gesture law on every `QueryContinueDrag` (a function pointer the panel supplies) and withdraws the drag per `ui::decideOsDragTurn`, reporting it as `OsDragEnd::Withdrawn*` so the panel resumes its own gesture. A drop into a plugin writes nothing to the bank and inserts no timeline item — the plugin receives a copy-only path list, exactly as from Explorer. +- `instrument_drop_win` — instrument-drop shell: `probeDropTarget` resolves a screen point to a track + a `ReaperSurface` (via the pure `wire::classifyReaperSurface`, whose token rules `core/wire/CLAUDE.md` owns) — it answers only what REAPER named, never whether the pointer is still in REAPER or over a plugin's own editor (so an FX surface reaches the instrument drop only as its chrome; a plugin's editor inside it hands off to the OS) — and the drop half adds a ReaSampler 9000 instance and applies the dragged capture's state via a transient `.vstpreset` + `TrackFX_SetPreset` (the former `TrackFX_SetNamedConfigParm` "vst_chunk" write was silently unappliable for VST3). Exposes `loadInstrumentOntoTrack` (inner half, no own undo block) and `performInstrumentDrop` (wraps in its own undo block). **Never captures, never writes the bank, never inserts a timeline item.** - `insert_fx_action` — the "Insert as FX" verb: a second ENTRY POINT to `instrument_drop_win`'s drop body (the bank panel's Placement button and a bindable action), loading the panel's focused capture onto the first selected track. Owns no insert logic of its own — it resolves the selection, folds `core/ui/insert_fx_enable`, and calls `performInstrumentDrop` (which keeps the one undo block). Refuses visibly on every failing precondition; the panel button additionally paints dead for the two panel-local ones. - `arrange_drop_win` — the drag-out gesture's arrange outcome: `arrangeTimeAtScreenX` (pointer column → time via `GetSet_ArrangeView2`'s one-pixel-span reading — inferred, not SDK-documented) and `performArrangeDrop` (snap the drop time, then one `InsertMedia` per capture on the pointer's track — assumed, not confirmed, to land end-to-end via REAPER's own cursor advance — in ONE undo block, counting only InsertMedia's reported successes, with the caller's track selection and edit cursor restored). The one timeline-placing shell here, per the invariant above; it never captures and never writes the bank. - `package_import_action` — the bindable / bank-menu / file-drop skin over `shell/package`'s `importBankPackage`. Owns the **ledger gate**, which runs BEFORE the file picker (a refusal must not cost the user a file choice) and is keyed on the session's `LedgerStatus` alone — never on `PruneReport::blockedByTracking`, whose undecodable-`rsusage_*` arm governs deletion-time protection and would refuse an import that only writes birth records. Builds and shows every message the import produces, but the ledger-refusal body itself is `core/package::ledgerRefusalMessage` — a pure fold this TU only supplies the channel-correct namespace to — so the wording is assertable without a DAW. `doImportBankPackage`/`doImportBankPackageFile` return the minted bank id on a landed import (empty otherwise) so a caller can focus it; the verb itself is promptless. diff --git a/src/shell/actions/drag_out_win.cpp b/src/shell/actions/drag_out_win.cpp index cb2d171..49a4fe3 100644 --- a/src/shell/actions/drag_out_win.cpp +++ b/src/shell/actions/drag_out_win.cpp @@ -12,6 +12,9 @@ #include // DROPFILES, CF_HDROP #include +#include "core/ui/drag_out.h" // decideOsDragTurn — the OS loop's per-turn verdict +#include "core/ui/window_origin.h" // classifyWindowCode / isPluginEditorChain + namespace reasampler { namespace { @@ -86,10 +89,16 @@ bool ensureOleForThisThread() { return state > 0; } -// Minimal IDropSource: continue until the (left) button releases or Escape cancels; always -// request the copy cursor. This is the standard textbook drop source — no custom feedback. +// IDropSource: the textbook button/Escape source, plus the law re-asked every turn so the drag can +// be withdrawn back to the panel (ui::decideOsDragTurn). Always requests the default cursors. +// [verify — DAW] QueryContinueDrag's cadence: MSDN words it as called on a keyboard or +// mouse-BUTTON state change, so on plain moves it may not run; if so a mid-drag withdrawal waits +// for the next state change, but the release — a button change — is still decided here. class DropSource final : public IDropSource { public: + explicit DropSource(bool (*lawHandsOff)()) : lawHandsOff_(lawHandsOff) {} + ui::OsDragTurn withdrawal() const { return withdrawal_; } + // IUnknown HRESULT STDMETHODCALLTYPE QueryInterface(REFIID riid, void** ppv) override { if (riid == IID_IUnknown || riid == IID_IDropSource) { @@ -108,15 +117,33 @@ public: } // IDropSource HRESULT STDMETHODCALLTYPE QueryContinueDrag(BOOL escapePressed, DWORD keyState) override { - if (escapePressed) return DRAGDROP_S_CANCEL; - if (!(keyState & MK_LBUTTON)) return DRAGDROP_S_DROP; // released -> drop - return S_OK; // keep dragging + const bool escape = escapePressed != FALSE; + // Escape ignores the law, so it is not probed then; no callback is the textbook source. + const bool lawHandsOff = escape || !lawHandsOff_ || lawHandsOff_(); + const ui::OsDragTurn turn = + ui::decideOsDragTurn(escape, (keyState & MK_LBUTTON) != 0, lawHandsOff); + switch (turn) { + case ui::OsDragTurn::Continue: + return S_OK; + case ui::OsDragTurn::Drop: + return DRAGDROP_S_DROP; + case ui::OsDragTurn::Cancel: + return DRAGDROP_S_CANCEL; + case ui::OsDragTurn::Withdraw: + case ui::OsDragTurn::WithdrawAtRelease: + withdrawal_ = turn; // the target sees a cancel (DragLeave), never a drop + return DRAGDROP_S_CANCEL; + } + return DRAGDROP_S_CANCEL; } HRESULT STDMETHODCALLTYPE GiveFeedback(DWORD /*effect*/) override { return DRAGDROP_S_USEDEFAULTCURSORS; // let OLE draw the standard copy cursor } private: volatile LONG refs_ = 1; + bool (*lawHandsOff_)() = nullptr; + // Continue doubles as "not withdrawn": it is never the turn that ends the loop. + ui::OsDragTurn withdrawal_ = ui::OsDragTurn::Continue; }; // Minimal IDataObject exposing exactly one format (CF_HDROP / TYMED_HGLOBAL). The HDROP is @@ -214,25 +241,34 @@ private: } // namespace -bool initiateDragOut(HWND__* /*panelHwnd*/, const std::vector& absolutePaths) { - if (absolutePaths.empty()) return false; - if (!ensureOleForThisThread()) return false; +OsDragEnd initiateDragOut(HWND__* /*panelHwnd*/, const std::vector& absolutePaths, + bool (*lawHandsOff)()) { + if (absolutePaths.empty()) return OsDragEnd::Concluded; + if (!ensureOleForThisThread()) return OsDragEnd::Concluded; HGLOBAL hdrop = buildHDrop(absolutePaths); - if (!hdrop) return false; + if (!hdrop) return OsDragEnd::Concluded; auto* data = new HDropDataObject(hdrop); // takes ownership of hdrop - auto* source = new DropSource(); + auto* source = new DropSource(lawHandsOff); DWORD effect = 0; // COPY-ONLY: the allowed-effects mask is DROPEFFECT_COPY alone. MOVE is never // offered, so no drop target can relocate (delete) the bank file. - const HRESULT hr = DoDragDrop(data, source, DROPEFFECT_COPY, &effect); + DoDragDrop(data, source, DROPEFFECT_COPY, &effect); + const ui::OsDragTurn withdrawal = source->withdrawal(); source->Release(); data->Release(); // frees the source HGLOBAL via HDropDataObject's dtor - return hr == DRAGDROP_S_DROP && effect == DROPEFFECT_COPY; + switch (withdrawal) { + case ui::OsDragTurn::Withdraw: return OsDragEnd::Withdrawn; + case ui::OsDragTurn::WithdrawAtRelease: return OsDragEnd::WithdrawnAtRelease; + case ui::OsDragTurn::Continue: + case ui::OsDragTurn::Drop: + case ui::OsDragTurn::Cancel: break; + } + return OsDragEnd::Concluded; } bool canInitiateDragOut(const std::vector& absolutePaths) { @@ -265,6 +301,85 @@ bool pointerOverHostWindow(int screenX, int screenY) { return pid == GetCurrentProcessId(); } +namespace { + +std::string toUtf8(const std::wstring& w) { + if (w.empty()) return {}; + const int n = WideCharToMultiByte(CP_UTF8, 0, w.c_str(), static_cast(w.size()), nullptr, + 0, nullptr, nullptr); + if (n <= 0) return {}; + std::string s(static_cast(n), '\0'); + WideCharToMultiByte(CP_UTF8, 0, w.c_str(), static_cast(w.size()), &s[0], n, nullptr, + nullptr); + return s; +} + +std::string moduleFileName(HMODULE mod) { + std::wstring buf(MAX_PATH, L'\0'); + for (;;) { + const DWORD n = GetModuleFileNameW(mod, &buf[0], static_cast(buf.size())); + if (n == 0) return {}; + if (n < buf.size()) { buf.resize(n); return toUtf8(buf); } + if (buf.size() >= 32768) return {}; // truncated at the long-path limit: no evidence + buf.resize(buf.size() * 2); + } +} + +// Empty on failure, which window_origin reads as "cannot tell OS code from plugin code". +const std::string& systemRoot() { + static const std::string root = [] { + wchar_t dir[MAX_PATH] = {0}; + const UINT n = GetSystemWindowsDirectoryW(dir, MAX_PATH); + return (n > 0 && n < MAX_PATH) ? toUtf8(std::wstring(dir, n)) : std::string{}; + }(); + return root; +} + +ui::WindowFacts windowFacts(HWND hwnd) { + ui::WindowFacts f; + DWORD pid = 0; + GetWindowThreadProcessId(hwnd, &pid); + if (pid != GetCurrentProcessId()) { + f.otherProcess = true; + return f; + } + // The charset-matched read returns the procedure's address; a mismatched one can return a + // handle that lies in no module, which is left as unresolved rather than guessed at. + const ULONG_PTR proc = IsWindowUnicode(hwnd) ? GetClassLongPtrW(hwnd, GCLP_WNDPROC) + : GetClassLongPtrA(hwnd, GCLP_WNDPROC); + HMODULE mod = nullptr; + if (!proc || !GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | + GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + reinterpret_cast(proc), &mod) || + !mod) { + return f; + } + f.procResolved = true; + f.hostModule = mod == GetModuleHandleW(nullptr); + f.modulePath = moduleFileName(mod); + return f; +} + +} // namespace + +bool pointerOverPluginEditor(int screenX, int screenY) { + // The CLASS procedure's module rather than the window's hInstance: a bare-SDK plugin may + // create its editor with the hInstance of the parent REAPER hands it (ReaSampler 9000's own + // editor does), so hInstance would read that plugin as REAPER. + const POINT pt{screenX, screenY}; + const HWND hit = WindowFromPoint(pt); + if (!hit) return false; + const HWND root = GetAncestor(hit, GA_ROOT); + + constexpr std::size_t kMaxDepth = 64; // a guard against a malformed tree, not a real bound + std::vector chain; + for (HWND h = hit; h && chain.size() < kMaxDepth; h = GetAncestor(h, GA_PARENT)) { + chain.push_back(ui::classifyWindowCode(windowFacts(h), systemRoot())); + if (h == root) break; + } + return ui::isPluginEditorChain(chain); +} + } // namespace reasampler #else // ---- macOS / Linux (SWELL) ----------------------------------------------------- @@ -278,8 +393,9 @@ namespace reasampler { // SWELL_InitiateDragDropOfFileList initiates a copy-style file drag from the given // window. Unlike OLE it exposes no per-source effect mask, so the copy-only // guarantee here rests on SWELL's copy semantics rather than an explicit mask. -bool initiateDragOut(HWND__* panelHwnd, const std::vector& absolutePaths) { - if (absolutePaths.empty() || !panelHwnd) return false; +OsDragEnd initiateDragOut(HWND__* panelHwnd, const std::vector& absolutePaths, + bool (* /*lawHandsOff*/)()) { + if (absolutePaths.empty() || !panelHwnd) return OsDragEnd::Concluded; std::vector ptrs; ptrs.reserve(absolutePaths.size()); @@ -288,7 +404,8 @@ bool initiateDragOut(HWND__* panelHwnd, const std::vector& absolute // srcrect null: SWELL positions the drag image at the current event. No custom icon. SWELL_InitiateDragDropOfFileList(reinterpret_cast(panelHwnd), nullptr, ptrs.data(), static_cast(ptrs.size()), nullptr); - return true; // fire-and-forget; SWELL owns the drag from here (no accept/cancel return) + // Fire-and-forget; SWELL owns the drag from here, with no per-turn hook to re-ask the law. + return OsDragEnd::Concluded; } // SWELL exposes no readiness probe ahead of SWELL_InitiateDragDropOfFileList (which itself @@ -307,6 +424,11 @@ bool pointerOverHostWindow(int screenX, int screenY) { return WindowFromPoint(pt) != nullptr; } +// No plugin-editor hand-off off Windows: every FX window keeps its instrument outcome, as before. +bool pointerOverPluginEditor(int /*screenX*/, int /*screenY*/) { + return false; +} + } // namespace reasampler #endif diff --git a/src/shell/actions/drag_out_win.h b/src/shell/actions/drag_out_win.h index f13a9b9..7129ac7 100644 --- a/src/shell/actions/drag_out_win.h +++ b/src/shell/actions/drag_out_win.h @@ -1,6 +1,7 @@ #pragma once // drag_out_win — the host/OS boundary of native OS drag-out: whether the pointer has -// crossed it, whether the OS is ready to take the drag, and the hand-off itself, which +// crossed it (left the host process, or reached a plugin's own editor), whether the OS is +// ready to take the drag, and the hand-off itself, which // gives a resolved existing-file path list to the OS's drag-drop machinery so the user can // drop bank samples into Explorer / another app / another DAW. The pure gesture-boundary // decision + path-list assembly live in drag_out.*. @@ -21,11 +22,22 @@ struct HWND__; // avoid dragging windows.h into every includer; the shell casts namespace reasampler { +// How the OS drag ended, as far as the caller must act on it. Whether a Concluded drag was +// accepted is not reported — nothing acts on it. +enum class OsDragEnd { + Concluded, // dropped, cancelled, or failed to start: the gesture is over + Withdrawn, // ui::OsDragTurn::Withdraw — the button is still down + WithdrawnAtRelease, // ui::OsDragTurn::WithdrawAtRelease — the button came up over REAPER +}; + // `absolutePaths` must already be resolved/existing/de-duped (drag_out::assemblePathList // output). No-op when empty. BLOCKING on Windows: OLE DoDragDrop runs its own modal -// message loop until drop/cancel. Returns true iff the drop was accepted -// (DROPEFFECT_COPY); the return is advisory — a failed drag surfaces no error. -bool initiateDragOut(HWND__* panelHwnd, const std::vector& absolutePaths); +// message loop, re-asking `lawHandsOff` (the gesture law at the current pointer) on every +// IDropSource::QueryContinueDrag and withdrawing per ui::decideOsDragTurn. macOS/Linux: SWELL's +// file drag is fire-and-forget with no such hook, so it always reports Concluded — the hand-off +// stays irreversible there. +OsDragEnd initiateDragOut(HWND__* panelHwnd, const std::vector& absolutePaths, + bool (*lawHandsOff)()); // Side-effect-free readiness probe for initiateDragOut. Call BEFORE tearing down internal drag // state — an OS that isn't ready (OLE unavailable, HDROP build failure) must not consume the @@ -44,4 +56,18 @@ bool canInitiateDragOut(const std::vector& absolutePaths); // Side-effect-free; safe on every mouse-move. bool pointerOverHostWindow(int screenX, int screenY); +// True iff plugin code implements the window under the screen point or one of its ancestors — +// the POSITIVE proof behind ui::DropContext::pointerOverPluginEditor. Windows reads, per window +// from the hit window up to its root, the owning process and the module holding the window +// CLASS's procedure, and lets ui::window_origin decide. The failure modes, all toward REAPER's +// own outcome (the instrument drop) rather than toward OLE: +// - a plugin that draws straight into the parent REAPER hands it, creating no window of its +// own, leaves nothing of its code in the chain; +// - a plugin binary installed beneath the Windows directory reads as an OS module; +// - a class procedure that resolves to no module (a charset-mismatch handle) is no evidence. +// [verify — DAW] that REAPER's GetThingFromPoint names an fx_* token over the plugin's own child +// window — the law requires it, so an unnamed editor would refuse rather than hand off. +// SWELL: always false (no plugin-editor hand-off off Windows). Side-effect-free. +bool pointerOverPluginEditor(int screenX, int screenY); + } // namespace reasampler diff --git a/src/shell/panel/panel_drag.cpp b/src/shell/panel/panel_drag.cpp index 001143b..a5bb4fe 100644 --- a/src/shell/panel/panel_drag.cpp +++ b/src/shell/panel/panel_drag.cpp @@ -199,10 +199,11 @@ LiveDrop resolveLiveDrop(int x, int y) { const DropProbe probe = probeDropTarget(sp.x, sp.y); ctx.surface = probe.surface; ctx.haveTrack = probe.track != nullptr; - // Two probes because they answer two independent questions: what REAPER named, and - // whether we are still inside REAPER at all. The law's hand-off gate reads both — see - // ui::DropContext::pointerOffHost. + // Separate probes because they answer independent questions: what REAPER named, whether + // we are still inside REAPER at all, and whether plugin code owns the window. The law's + // hand-off gates pair them — see ui::DropContext::pointerOffHost. ctx.pointerOffHost = !pointerOverHostWindow(sp.x, sp.y); + ctx.pointerOverPluginEditor = pointerOverPluginEditor(sp.x, sp.y); out.track = probe.track; out.screenX = sp.x; } @@ -219,16 +220,46 @@ LiveDrop resolveLiveDrop(int x, int y) { return out; } -// The law's one irreversible transition: DoDragDrop takes mouse capture and runs its own modal -// loop, so the internal drag must be fully wound down first, and only once the payload is known -// to be hand-off-able. This runs on every qualifying move — do not memoize a failed attempt. -// -// Residual (accepted): once the pointer has genuinely left every REAPER window, dragging back IN -// mid-modal-loop delivers a CF_HDROP to REAPER's own file-import drop target rather than to our -// gesture law. NOT confirmed by experiment — inferred from REAPER's handling of external file -// drops, and the inferred outcome (an item at the drop point) coincides with what our own -// arrange path would have done. This is now the ONLY way to see that import ghost mid-drag; -// reaching it from a surface still inside REAPER was the defect pointerOffHost closed. +// Re-asked by the OS drag loop on every turn. The payload fields the law reads survive +// resetDragState, so it still sees the drag the hand-off wound down. With the panel gone +// mid-loop there is no gesture left to give back, so the OS keeps the drag. +bool lawHandsOffAtCursor() { + if (!g_panel.hwnd) return true; + POINT p{}; + GetCursorPos(&p); + ScreenToClient(g_panel.hwnd, &p); + return resolveLiveDrop(p.x, p.y).cls == DropClass::OsHandoff; +} + +// The OS loop gave the gesture back (ui::OsDragTurn::Withdraw*): restore exactly the fields +// resetDragState cleared that the drag still needs, then either keep dragging under our own +// capture or — the release was consumed by the OS loop, so no WM_LBUTTONUP will follow — commit +// at the release point now. No move is replayed here: the next WM_MOUSEMOVE re-resolves the cue, +// and replaying one could re-enter handOffToOs from inside itself. +void resumeInternalDrag(const std::string& primaryId, bool released) { + if (!g_panel.hwnd) return; + g_panel.dragArmed = true; + g_panel.dragging = true; + g_panel.dragPrimaryId = primaryId; + + POINT p{}; + GetCursorPos(&p); + ScreenToClient(g_panel.hwnd, &p); + if (released) { + onLBtnUp(p.x, p.y); + return; + } + // [verify — DAW] that SetCapture re-takes the mouse mid-press after DoDragDrop released it. + SetCapture(g_panel.hwnd); + invalidatePanel(); +} + +// The hand-off: DoDragDrop takes mouse capture and runs its own modal loop, so the internal drag +// must be fully wound down first, and only once the payload is known to be hand-off-able. This +// runs on every qualifying move — do not memoize a failed attempt. The OS loop may hand the +// gesture back (ui::OsDragTurn owns why); a side effect worth keeping is that, on Windows, a +// return trip can no longer drop a CF_HDROP on REAPER's file import or on this panel's own +// WM_DROPFILES ingest — the release there is withdrawn, not dropped. void handOffToOs() { // Resolve BEFORE tearing anything down (the resolver reads the live drag payload), then let // the pure rule couple the two side effects: an unresolvable payload leaves the internal @@ -242,11 +273,22 @@ void handOffToOs() { return; } + const std::string primaryId = g_panel.dragPrimaryId; // resetDragState clears it if (GetCapture() == g_panel.hwnd) ReleaseCapture(); resetDragState(); invalidatePanel(); - initiateDragOut(g_panel.hwnd, paths); // COPY-ONLY; blocking on Windows + // COPY-ONLY; blocking on Windows. + switch (initiateDragOut(g_panel.hwnd, paths, &lawHandsOffAtCursor)) { + case OsDragEnd::Withdrawn: + resumeInternalDrag(primaryId, /*released=*/false); + return; + case OsDragEnd::WithdrawnAtRelease: + resumeInternalDrag(primaryId, /*released=*/true); + return; + case OsDragEnd::Concluded: + break; + } // The drag state was reset above, so the WM_LBUTTONUP that ends the modal loop takes // onLBtnUp's not-dragging branch and never restores the arrow — do it here instead. diff --git a/tests/test_drag_out.cpp b/tests/test_drag_out.cpp index 6ebb259..af4338e 100644 --- a/tests/test_drag_out.cpp +++ b/tests/test_drag_out.cpp @@ -3,8 +3,9 @@ // // Covers: // * The full class matrix: every ReaperSurface x {single, multi} x {track, no track} x -// {over a host window, off it}, inside and outside the client rect, plus the half-open edge -// and a non-zero panel origin. +// {over a host window, off it} x {plugin code under the pointer, not}, inside and outside the +// client rect, plus the half-open edge and a non-zero panel origin. +// * The OS drag loop's per-turn verdict (decideOsDragTurn), exhaustively. // * Reversibility and speed-independence, stated as properties: a class transition sequence // resolves the same forwards and backwards, and one unresolvable evaluation cannot change // any later evaluation's outcome. @@ -50,6 +51,16 @@ static DropContext offHost(ReaperSurface surface = ReaperSurface::Other, bool si return c; } +// The same drag with the shell's plugin-code proof set: the window under the pointer is a plugin's +// own editor. Defaults to where it matters — an FX surface REAPER attributed to a track. +static DropContext overPlugin(ReaperSurface surface = ReaperSurface::FxSurface, bool single = true, + bool haveTrack = true, bool offHostToo = false) { + DropContext c = ctx(surface, single, haveTrack); + c.pointerOverPluginEditor = true; + c.pointerOffHost = offHostToo; + return c; +} + // Every surface the law enumerates, so the matrix tests iterate rather than list. static const ReaperSurface kAllSurfaces[] = { ReaperSurface::TrackPanel, ReaperSurface::FxSurface, ReaperSurface::FxEmbed, @@ -78,10 +89,11 @@ static void testInsideClientIsAlwaysInternal() { CHECK(decideDropClass(200, 150, kPanel, ctx(s, single)) == DropClass::Internal); CHECK(decideDropClass(0, 0, kPanel, ctx(s, single)) == DropClass::Internal); CHECK(decideDropClass(399, 299, kPanel, ctx(s, single)) == DropClass::Internal); - // Even a stale off-host proof cannot reinterpret a point inside our own client rect: - // the inside-client answer is resolved ahead of the hand-off gate, so re-entering the - // panel always resumes the bank-to-bank gesture. + // Even a stale proof of either kind cannot reinterpret a point inside our own client + // rect: the inside-client answer is resolved ahead of both hand-off gates, so + // re-entering the panel always resumes the bank-to-bank gesture. CHECK(decideDropClass(200, 150, kPanel, offHost(s, single)) == DropClass::Internal); + CHECK(decideDropClass(200, 150, kPanel, overPlugin(s, single)) == DropClass::Internal); } } } @@ -119,7 +131,8 @@ static void testSingleOverTrackPanelIsInstrumentDrop() { DropClass::InstrumentDrop); } -// The FX chain / floating-FX windows keep their existing outcome. +// The FX chain / floating-FX windows' own chrome — REAPER's preset bar, the chain list — keeps its +// existing outcome: no plugin-code proof, so the instrument drop. static void testSingleOverFxSurfaceIsInstrumentDrop() { CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxSurface)) == DropClass::InstrumentDrop); @@ -198,7 +211,8 @@ static void testNullTrackWithSurfaceRefuses() { // --- The hand-off gate --------------------------------------------------------- // THE REGRESSION FLOOR. No reading of REAPER's hit-test can reach the modal OLE loop while the -// pointer is over a host window — asserted over EVERY surface the probe can report, both payload +// pointer is over a host window and no plugin code owns it (ctx() sets neither proof) — asserted +// over EVERY surface the probe can report, both payload // sizes and both track verdicts, not just the toolbar/transport cell that was reported. The // defect was structural (an unnamed token read as "the user left REAPER"), so the guarantee has // to be structural too. @@ -231,9 +245,10 @@ static void testOffHostHandsOffOnlyWhereReaperNamedNothing() { } // The shipped exception the token condition exists for: REAPER runs a bridged plugin's UI in -// reaper_host*.exe, so the window-ownership test reports off-host over a floating bridged FX -// editor even though the pointer never left REAPER. There the recognised token vetoes the -// hand-off and the surface's own REAPER-internal outcome stands — identical to the native case. +// reaper_host*.exe, so the process test reports off-host over a bridged FX window even though the +// pointer never left REAPER. The recognised token vetoes the PROCESS route, so on its own that +// proof changes nothing over a named surface — whether the bridged editor then hands off is the +// plugin-code route's call alone (testBridgedEditorHandsOffThroughThePluginRoute). static void testOffHostOverANamedSurfaceKeepsTheReaperOutcome() { CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost(ReaperSurface::FxSurface, true, true)) == DropClass::InstrumentDrop); @@ -264,13 +279,71 @@ static void testOffHostOverANamedSurfaceKeepsTheReaperOutcome() { } } +// --- The plugin-editor route --------------------------------------------------- + +// THE FEATURE: over a plugin's own editor inside an FX window, the files go to the plugin through +// the OS — whatever the payload size and whether or not REAPER attributed a track, since the +// plugin, not a track, is the receiver. +static void testPluginEditorOverFxSurfaceHandsOff() { + for (bool single : {true, false}) { + for (bool haveTrack : {true, false}) { + CHECK(decideDropClass(kOutX, kOutY, kPanel, + overPlugin(ReaperSurface::FxSurface, single, haveTrack)) == + DropClass::OsHandoff); + } + } +} + +// The same point WITHOUT the proof is the FX window's chrome: the instrument drop for a single +// card, the multi refusal otherwise. The proof is the whole difference, in both directions. +static void testSameFxSurfaceWithoutTheProofKeepsItsOutcome() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxSurface)) == + DropClass::InstrumentDrop); + CHECK(decideDropClass(kOutX, kOutY, kPanel, overPlugin(ReaperSurface::FxSurface)) == + DropClass::OsHandoff); + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxSurface, false)) == + DropClass::Refuse); + CHECK(decideDropClass(kOutX, kOutY, kPanel, overPlugin(ReaperSurface::FxSurface, false)) == + DropClass::OsHandoff); +} + +// Plugin code under the pointer anywhere REAPER did not name an FX surface — the TCP, the MCP, the +// embed strip, the arrange, an extension window docked in REAPER (Other) — changes nothing: the +// token confines the route to FX windows. Asserted over every such cell, on-host and off. +static void testPluginProofOffAnFxSurfaceChangesNothing() { + for (ReaperSurface s : kAllSurfaces) { + if (s == ReaperSurface::FxSurface) continue; + for (bool single : {true, false}) { + for (bool haveTrack : {true, false}) { + for (bool off : {false, true}) { + const DropContext without = + off ? offHost(s, single, haveTrack) : ctx(s, single, haveTrack); + CHECK(decideDropClass(kOutX, kOutY, kPanel, + overPlugin(s, single, haveTrack, off)) == + decideDropClass(kOutX, kOutY, kPanel, without)); + } + } + } + } +} + +// A bridged editor: the process proof AND the plugin proof are both set, REAPER names the FX +// window. The token vetoed the process route; the plugin route still hands off. +static void testBridgedEditorHandsOffThroughThePluginRoute() { + for (bool haveTrack : {true, false}) { + CHECK(decideDropClass(kOutX, kOutY, kPanel, + overPlugin(ReaperSurface::FxSurface, true, haveTrack, + /*offHostToo=*/true)) == DropClass::OsHandoff); + } +} + // --- Not-a-drag ---------------------------------------------------------------- // No armed samples, or not dragging -> None regardless of position or surface. static void testNoDragOrNoSamplesIsNone() { for (ReaperSurface s : kAllSurfaces) { - for (bool off : {false, true}) { - DropContext c = off ? offHost(s) : ctx(s); + for (int proof = 0; proof < 3; ++proof) { // none, off-host, plugin editor + DropContext c = proof == 1 ? offHost(s) : proof == 2 ? overPlugin(s) : ctx(s); c.drag = DragState{/*dragging=*/true, /*hasArmedSamples=*/false}; CHECK(decideDropClass(kOutX, kOutY, kPanel, c) == DropClass::None); CHECK(decideDropClass(200, 150, kPanel, c) == DropClass::None); @@ -299,12 +372,22 @@ static void testClassTransitionsAreReversible() { CHECK(decideDropClass(kOutX, kOutY, kPanel, arrange) == DropClass::ArrangeInsert); CHECK(decideDropClass(200, 150, kPanel, inside) == DropClass::Internal); - // The hand-off leg is irreversible only because the SHELL goes modal on it. The law itself - // stays stateless across it: evaluating an off-host context leaves the next in-REAPER - // evaluation exactly where it was. + // The hand-off legs stay stateless too: evaluating an off-host or plugin-editor context leaves + // the next in-REAPER evaluation exactly where it was — which is what lets the OS loop withdraw + // on the law's own answer (decideOsDragTurn) rather than on anything remembered. CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost()) == DropClass::OsHandoff); CHECK(decideDropClass(kOutX, kOutY, kPanel, arrange) == DropClass::ArrangeInsert); CHECK(decideDropClass(200, 150, kPanel, inside) == DropClass::Internal); + + // Through a plugin's editor and back out over its own window's chrome: the instrument drop is + // still there after the pass, and so is everything after it. + CHECK(decideDropClass(kOutX, kOutY, kPanel, overPlugin()) == DropClass::OsHandoff); + CHECK(decideDropClass(kOutX, kOutY, kPanel, fx) == DropClass::InstrumentDrop); + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::TrackPanel)) == + DropClass::InstrumentDrop); + CHECK(decideDropClass(kOutX, kOutY, kPanel, arrange) == DropClass::ArrangeInsert); + CHECK(decideDropClass(kOutX, kOutY, kPanel, overPlugin()) == DropClass::OsHandoff); + CHECK(decideDropClass(200, 150, kPanel, inside) == DropClass::Internal); } // One unresolvable evaluation (a surface with no track, which refuses) followed by a resolvable @@ -331,10 +414,11 @@ static void testDragSpeedCannotChangeTheOutcome() { const DropContext destination = ctx(ReaperSurface::TrackPanel); const DropClass flick = decideDropClass(kOutX, kOutY, kPanel, destination); - // The slow path crosses everything else first, off-host legs included. + // The slow path crosses everything else first, off-host and plugin-editor legs included. for (ReaperSurface s : kAllSurfaces) { (void)decideDropClass(kOutX - 10, kOutY, kPanel, ctx(s)); (void)decideDropClass(kOutX - 10, kOutY, kPanel, offHost(s)); + (void)decideDropClass(kOutX - 10, kOutY, kPanel, overPlugin(s)); (void)decideDropClass(200, 150, kPanel, ctx(s)); // and back through the client } CHECK(decideDropClass(kOutX, kOutY, kPanel, destination) == flick); @@ -350,11 +434,14 @@ static void testNoLiveOutsideCombinationResolvesToNone() { for (bool single : {true, false}) { for (bool haveTrack : {true, false}) { for (bool off : {false, true}) { - const DropClass c = decideDropClass( - kOutX, kOutY, kPanel, - off ? offHost(s, single, haveTrack) : ctx(s, single, haveTrack)); - CHECK(c != DropClass::None); - CHECK(c != DropClass::Internal); + for (bool plugin : {false, true}) { + DropContext c0 = + off ? offHost(s, single, haveTrack) : ctx(s, single, haveTrack); + c0.pointerOverPluginEditor = plugin; + const DropClass c = decideDropClass(kOutX, kOutY, kPanel, c0); + CHECK(c != DropClass::None); + CHECK(c != DropClass::Internal); + } } } } @@ -490,6 +577,44 @@ static void testAllSkippedSelectionKeepsInternalDrag() { CHECK(!h.handOffToOs); } +// --- The OS drag loop, once handed off --------------------------------------------- + +// The whole truth table, one cell per line so a regression names its cell. +static void testOsDragTurnTruthTable() { + // Button held: stay with the OS while the law still hands off, withdraw the moment it doesn't. + CHECK(decideOsDragTurn(false, true, true) == OsDragTurn::Continue); + CHECK(decideOsDragTurn(false, true, false) == OsDragTurn::Withdraw); + // Released: the OS drops only where the law hands off; over a REAPER surface the release is + // given back to the panel, so REAPER's own outcome runs instead of a CF_HDROP import. + CHECK(decideOsDragTurn(false, false, true) == OsDragTurn::Drop); + CHECK(decideOsDragTurn(false, false, false) == OsDragTurn::WithdrawAtRelease); + // Escape ends the drag outright in every cell — never read as a release, never a withdrawal + // that would resume a drag the user just cancelled. + CHECK(decideOsDragTurn(true, true, true) == OsDragTurn::Cancel); + CHECK(decideOsDragTurn(true, true, false) == OsDragTurn::Cancel); + CHECK(decideOsDragTurn(true, false, true) == OsDragTurn::Cancel); + CHECK(decideOsDragTurn(true, false, false) == OsDragTurn::Cancel); +} + +// End to end with the law: a hand-off started over a plugin's editor, then the pointer moves onto +// that FX window's chrome with the button held — withdrawn, and the resumed drag's release there is +// the instrument drop, not an OS drop. Moving back onto the editor keeps the OS drag. +static void testPassOverAPluginEditorIsWithdrawnOnReaperSurfaces() { + auto handsOff = [](const DropContext& c) { + return decideDropClass(kOutX, kOutY, kPanel, c) == DropClass::OsHandoff; + }; + CHECK(decideOsDragTurn(false, true, handsOff(overPlugin())) == OsDragTurn::Continue); + CHECK(decideOsDragTurn(false, true, handsOff(ctx(ReaperSurface::FxSurface))) == + OsDragTurn::Withdraw); + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxSurface)) == + DropClass::InstrumentDrop); + CHECK(decideOsDragTurn(false, false, handsOff(ctx(ReaperSurface::Arrange))) == + OsDragTurn::WithdrawAtRelease); + CHECK(decideOsDragTurn(false, false, handsOff(overPlugin())) == OsDragTurn::Drop); + // Leaving REAPER entirely is handed off by the other route and held the same way. + CHECK(decideOsDragTurn(false, true, handsOff(offHost())) == OsDragTurn::Continue); +} + int main() { testInsideClientIsAlwaysInternal(); testBoundaryHalfOpen(); @@ -513,6 +638,11 @@ int main() { testOffHostOverANamedSurfaceKeepsTheReaperOutcome(); testNoDragOrNoSamplesIsNone(); + testPluginEditorOverFxSurfaceHandsOff(); + testSameFxSurfaceWithoutTheProofKeepsItsOutcome(); + testPluginProofOffAnFxSurfaceChangesNothing(); + testBridgedEditorHandsOffThroughThePluginRoute(); + testClassTransitionsAreReversible(); testUnresolvableEvaluationDoesNotAffectTheNext(); testDragSpeedCannotChangeTheOutcome(); @@ -532,6 +662,9 @@ int main() { testResolvablePayloadHandsOff(); testAllSkippedSelectionKeepsInternalDrag(); + testOsDragTurnTruthTable(); + testPassOverAPluginEditorIsWithdrawnOnReaperSurfaces(); + if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; } diff --git a/tests/test_window_origin.cpp b/tests/test_window_origin.cpp new file mode 100644 index 0000000..4290313 --- /dev/null +++ b/tests/test_window_origin.cpp @@ -0,0 +1,162 @@ +// Standalone tests for reasampler::ui::window_origin — no REAPER, no Win32, no test framework. +// Asserts the per-window code fold and the chain rule the plugin-editor hand-off proof rests on. + +#include "../src/core/ui/window_origin.h" + +#include +#include +#include + +using namespace reasampler::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 const std::string kWinDir = "C:\\Windows"; + +static WindowFacts inModule(const std::string& path, bool host = false) { + WindowFacts f; + f.procResolved = true; + f.hostModule = host; + f.modulePath = path; + return f; +} + +// --- classifyWindowCode -------------------------------------------------------- + +// REAPER's own chrome and containers: the class procedure lives in the executable. +static void testHostExecutableIsHost() { + CHECK(classifyWindowCode(inModule("C:\\Program Files\\REAPER (x64)\\reaper.exe", true), + kWinDir) == WindowCode::Host); +} + +// Standard controls and the dialog class are implemented by OS modules, wherever the OS keeps +// them — System32, SysWOW64 for a 32-bit host, and the side-by-side store comctl32 v6 loads from. +static void testOsModulesAreSystem() { + CHECK(classifyWindowCode(inModule("C:\\Windows\\System32\\user32.dll"), kWinDir) == + WindowCode::System); + CHECK(classifyWindowCode(inModule("C:\\Windows\\SysWOW64\\user32.dll"), kWinDir) == + WindowCode::System); + CHECK(classifyWindowCode( + inModule("C:\\Windows\\WinSxS\\amd64_microsoft.windows.common-controls_6595b64144ccf1df" + "_6.0.26100.1_none_x\\comctl32.dll"), + kWinDir) == WindowCode::System); +} + +// The same directory spelled differently by two APIs still compares equal: case, separator +// direction, a trailing separator on the root, and the long-path prefix. +static void testSystemMatchIgnoresSpelling() { + CHECK(classifyWindowCode(inModule("c:/WINDOWS/system32/USER32.DLL"), kWinDir) == + WindowCode::System); + CHECK(classifyWindowCode(inModule("C:\\Windows\\System32\\user32.dll"), "C:\\WINDOWS\\") == + WindowCode::System); + CHECK(classifyWindowCode(inModule("\\\\?\\C:\\Windows\\System32\\user32.dll"), kWinDir) == + WindowCode::System); +} + +// Containment, not string prefix: a sibling directory whose name merely starts with the OS +// directory's is plugin territory, and so is the OS directory itself named as a file path. +static void testSiblingDirectoryIsNotSystem() { + CHECK(classifyWindowCode(inModule("C:\\WindowsApps\\thing.dll"), kWinDir) == + WindowCode::Plugin); + CHECK(classifyWindowCode(inModule("C:\\Windows2\\thing.dll"), kWinDir) == WindowCode::Plugin); + CHECK(classifyWindowCode(inModule("C:\\Windows"), kWinDir) == WindowCode::Plugin); +} + +// A plugin binary anywhere outside the OS directory — the VST3 bundle, a VST2 folder, a helper +// DLL a plugin loads beside itself — is Plugin. +static void testPluginBinariesArePlugin() { + CHECK(classifyWindowCode( + inModule("C:\\Program Files\\Common Files\\VST3\\Serum2.vst3\\Contents\\x86_64-win\\" + "Serum2.vst3"), + kWinDir) == WindowCode::Plugin); + CHECK(classifyWindowCode(inModule("D:\\VstPlugins\\Qt5Widgets.dll"), kWinDir) == + WindowCode::Plugin); +} + +// Another process's window is plugin code by the process boundary alone, whatever its other facts +// claim: a procedure address from another process resolved in ours is meaningless. +static void testOtherProcessIsPluginRegardlessOfModule() { + WindowFacts f = inModule("C:\\Program Files\\REAPER (x64)\\reaper.exe", true); + f.otherProcess = true; + CHECK(classifyWindowCode(f, kWinDir) == WindowCode::Plugin); + WindowFacts g; + g.otherProcess = true; // nothing resolved at all + CHECK(classifyWindowCode(g, kWinDir) == WindowCode::Plugin); +} + +// No module (a charset-mismatch handle, generated code) is no evidence — never Plugin. +static void testUnresolvedProcIsUnknown() { + WindowFacts f; + f.modulePath = "C:\\VST\\x.dll"; // ignored without a resolved procedure + CHECK(classifyWindowCode(f, kWinDir) == WindowCode::Unknown); +} + +// Without the OS directory a standard control cannot be told from plugin code, so every non-host +// module degrades to Unknown — the failure must not read REAPER's buttons as a plugin. +static void testMissingSystemRootNeverYieldsPlugin() { + CHECK(classifyWindowCode(inModule("C:\\Windows\\System32\\user32.dll"), "") == + WindowCode::Unknown); + CHECK(classifyWindowCode(inModule("C:\\VST\\x.dll"), "") == WindowCode::Unknown); + CHECK(classifyWindowCode(inModule("C:\\VST\\x.dll"), "\\") == WindowCode::Unknown); + CHECK(classifyWindowCode(inModule("C:\\REAPER\\reaper.exe", true), "") == WindowCode::Host); +} + +// --- isPluginEditorChain ------------------------------------------------------- + +// Pointer on REAPER's FX-window chrome: a standard control or a REAPER container, under the +// dialog-class root. No plugin code anywhere in the chain. +static void testChromeChainIsNotPlugin() { + CHECK(!isPluginEditorChain({WindowCode::System, WindowCode::Host, WindowCode::System})); + CHECK(!isPluginEditorChain({WindowCode::Host, WindowCode::System})); + CHECK(!isPluginEditorChain({WindowCode::Host})); +} + +// Pointer on the plugin's editor window itself. +static void testPluginWindowChainIsPlugin() { + CHECK(isPluginEditorChain({WindowCode::Plugin, WindowCode::Host, WindowCode::System})); +} + +// A standard control INSIDE the plugin's editor (a native text box) is System on its own; the +// plugin's window above it is what makes the point the plugin's. +static void testStandardControlInsidePluginIsPlugin() { + CHECK(isPluginEditorChain( + {WindowCode::System, WindowCode::Plugin, WindowCode::Host, WindowCode::System})); +} + +// A bridged plugin's editor embedded across the process boundary: the hit window is Plugin by +// process, REAPER's container and root above it are in-process. +static void testBridgedEmbeddedEditorIsPlugin() { + WindowFacts bridged; + bridged.otherProcess = true; + CHECK(isPluginEditorChain({classifyWindowCode(bridged, kWinDir), WindowCode::Host, + WindowCode::System})); +} + +// Unknown anywhere, and nothing else, is not proof; an empty chain (no window at all) is not. +static void testUnknownAndEmptyChainsAreNotPlugin() { + CHECK(!isPluginEditorChain({WindowCode::Unknown, WindowCode::Host, WindowCode::System})); + CHECK(!isPluginEditorChain({WindowCode::Unknown})); + CHECK(!isPluginEditorChain({})); +} + +int main() { + testHostExecutableIsHost(); + testOsModulesAreSystem(); + testSystemMatchIgnoresSpelling(); + testSiblingDirectoryIsNotSystem(); + testPluginBinariesArePlugin(); + testOtherProcessIsPluginRegardlessOfModule(); + testUnresolvedProcIsUnknown(); + testMissingSystemRootNeverYieldsPlugin(); + + testChromeChainIsNotPlugin(); + testPluginWindowChainIsPlugin(); + testStandardControlInsidePluginIsPlugin(); + testBridgedEmbeddedEditorIsPlugin(); + testUnknownAndEmptyChainsAreNotPlugin(); + + if (g_fail == 0) std::printf("All tests passed.\n"); + return g_fail ? 1 : 0; +} From f7505d0e538724d009decdb6f0738ae7e8031180 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:42:25 -0400 Subject: [PATCH 03/12] Make spline EG drags audible live: held and new notes follow a dragged knot Contours ride their own seqlock block into a double-buffered engine copy, and the swap step is smoothed. Each drag move publishes contours without a model write; release still commits and reloads. --- src/core/instrument/CLAUDE.md | 17 +- src/core/instrument/engine/CMakeLists.txt | 11 +- src/core/instrument/engine/live_contours.cpp | 16 + src/core/instrument/engine/live_contours.h | 50 ++ src/core/instrument/engine/live_params.h | 67 +-- src/core/instrument/engine/play_params.h | 10 +- src/core/instrument/engine/seqlock.h | 76 +++ src/core/instrument/engine/velocity_curve.h | 17 +- src/core/instrument/engine/voice.cpp | 26 ++ src/core/instrument/engine/voice.h | 41 +- src/core/instrument/engine/voice_engine.cpp | 22 + src/core/instrument/engine/voice_engine.h | 22 + src/shell/instrument/CLAUDE.md | 20 +- .../instrument/editor_input_waveform.cpp | 6 +- src/shell/instrument/editor_platform.cpp | 4 + src/shell/instrument/editor_session.cpp | 4 + src/shell/instrument/processor_reload.cpp | 5 + src/shell/instrument/processor_state.cpp | 8 + src/shell/instrument/reasampler_editor.h | 6 + src/shell/instrument/reasampler_processor.h | 19 +- tests/test_live_contours.cpp | 438 ++++++++++++++++++ 21 files changed, 788 insertions(+), 97 deletions(-) create mode 100644 src/core/instrument/engine/live_contours.cpp create mode 100644 src/core/instrument/engine/live_contours.h create mode 100644 src/core/instrument/engine/seqlock.h create mode 100644 tests/test_live_contours.cpp diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index 629b23d..f16dd02 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -203,6 +203,14 @@ automatable parameters."* It rejects the precedent, not one instance of it. - **Two genuine level steps are smoothed, not ruled away**: a sustain level moved while the voice holds it, and a stage duration dialled to exactly zero mid-stage. Both are absorbed by the envelope's own bounded offset smoother. +- **A drawn contour is live too, through a second block.** `LiveContours` (all three contours + as fixed-capacity arrays, ~9 KB) is published beside `LiveValues` rather than inside it, so a + knob publish, the automation merge and the block compare never carry it. Each `VoiceEngine` + double-buffers it, and `applyContoursToActive` (`voice_engine.h`) owns the rebind invariant + that makes the double buffer safe. A cursor `start()` left cleared (a Staged EG) stays + cleared: Staged|Spline is a note-on fact. A contour is positional, like the Trigger AHD, + so a swap is a level step at the read position, absorbed by a per-EG `StepSmoother` — one more + predicted branch per sample on a drawn EG, none on a Staged one. ### Non-goals / guardrails (instrument-specific; repo-wide invariants live in root CLAUDE.md) @@ -255,8 +263,9 @@ consequences, each with one home: - **Normalized is what makes a contour length-independent.** There are no stored seconds to rescale, so a different-length capture replays the same shape proportionally. - **The contours sit on `PlayParams`/`PlaySeconds` directly, not inside the three envelope - structs.** Those are copied whole into the live block, which must stay trivially copyable - (`live_params.h`) — and a contour is not a live control anyway: like the velocity curves it + structs.** Those are copied whole into the live block, which must stay small and trivially + copyable (`live_params.h`). A contour still reaches the note already sounding — through its + OWN block (`live_contours.h`), see "Live parameter delivery". Only the Staged|Spline mode travels by reload. **Which shape a STAGED envelope takes is decided by the play mode, not by what it modulates:** @@ -294,7 +303,7 @@ anything for a trigger shape. - The engine is the `sampler_core` CMake target over FOUR headers and TWO TUs, split on its own responsibility seam — cold note routing vs the hot per-sample render: - `play_params.h` — the value layer: `PlayParams`/`AdsrParams`/`TriggerParams`/`PitchEnvParams`/`FilterParams`, the per-instance mode enums (`ChannelMode`/`VoiceMode`/`MonoTrigger`), and `SampleData` (the ONE loaded capture: decoded PCM + root + loop + start + keyTrack + velocity curve + play params). Shared by the engine, the codec, and the editor, so a UI/codec TU reading a param struct doesn't recompile when a `Voice` member changes. `FilterParams` stores the filter module's own `FilterSettings` by value rather than a parallel copy of its normalized positions. Also the ONE home of the drawn-EG rule family — `splineActive`, `effectivePlayMode`, `enforceGateUnavailableWhileDrawn` and `effectiveLengthFraction` — all templated over the frames and seconds representations, so no consumer of either can re-read the raw fields instead. - `envelopes.h` — the three per-frame evaluators (`AdsrEnvelope` AHDSR, `AhdEnvelope` the sustain-less Attack/Hold/Decay, `PitchEnvelope` the AHD pitch offset), CONCRETE and fully header-inline. Never give them a common base or a virtual `tick()`: they are called per-voice-per-sample. Also home to `fitAhd`/`ahdLevelAt`, THE span split and shape every sustain-less envelope shares. A voice carries two of each shape — the amp's and the filter's — and its play mode picks which pair it reads. `AdsrEnvelope`/`PitchEnvelope` own `applyLive` (the φ-holding mid-stage rule), its fresh-note peer `snapLive`, and `StepSmoother`, the bounded offset that absorbs the level steps φ cannot cover; `AhdEnvelope` is POSITIONAL (evaluated at a source offset, not ticked), so it has no phase to hold and smooths a live reshape instead. - - `live_params.h` / `live_params.cpp` — the live-parameter block: `LiveValues` (the plain, trivially-copyable bundle the audio thread observes), the single-writer `LiveParams` seqlock that publishes it without a lock or a torn read, `foldLive` (the ONE derivation from `PlayParams` — every publisher goes through it so the two representations cannot drift), the block's FIELD-wise `operator==` (never a memcmp — the header owns why the padding makes a byte compare report differences that do not exist), and `ValueRamp`, the per-frame glide whose EXACT termination is what lets the filter's equality-compare cutoff skip re-engage. Links no engine: the block is a value the voice observes, not a thing the engine owns. + - `live_params.h` / `live_params.cpp` — the live-parameter block: `LiveValues` (the plain, trivially-copyable bundle the audio thread observes), the single-writer `LiveParams` seqlock that publishes it without a lock or a torn read, `foldLive` (the ONE derivation from `PlayParams` — every publisher goes through it so the two representations cannot drift), the block's FIELD-wise `operator==` (never a memcmp — the header owns why the padding makes a byte compare report differences that do not exist), and `ValueRamp`, the per-frame glide whose EXACT termination is what lets the filter's equality-compare cutoff skip re-engage. Links no engine: the block is a value the voice observes, not a thing the engine owns. The same library carries `live_contours.h`/`.cpp` (the drawn-contour block beside it, and `foldContours`, templated over both parameter representations like `splineActive`) and the header-only `seqlock.h`, the ONE seqlock both blocks publish through. - `voice.h` / `voice.cpp` — one voice. The per-SAMPLE render half (`advanceFrame` and everything it calls) is INLINE IN THE HEADER by RT constraint; the per-NOTE half (note-on setup incl. the Preserve ring prime, legato retune, gate-off, the off-thread shifter presize) is out of line in the TU. The voice owns its own `VoiceFilter` and filter envelope, run between the pitch stage and the amp multiply — see `engine/filter/CLAUDE.md`. **Documented ~600-line-ceiling exception** (root `CLAUDE.md` structural heuristic 1): `voice.h` sits over the ceiling because `advanceFrame`'s RT-inline constraint forbids the seam a split would need — a documented exception, not silent overshoot. - `voice_engine.h` / `voice_engine.cpp` — `VoiceEngine`: note routing, bounded-stealing allocation, user-parameterized voice count (1–32, default 16), `VoiceMode` Poly/Mono (last-note held-note stack, `MonoTrigger` Retrigger/Legato), two-tier panic (CC 123 = all-notes-off release, CC 120 = immediate hard-stop including Trigger one-shots), and the block render loops. Preview injects a synthetic note-on at the loaded capture's root note into the main `VoiceEngine` — no dedicated `PreviewCard`; preview obeys polyphony/mono/voice-stealing/envelopes. - `engine/loop/` — the sustain loop's ONE validity/clamp fold (`resolveLoop`) plus its pre-seam crossfade geometry and the editor's default handle span; see `engine/loop/CLAUDE.md`. The voice folds it once at note-on; the crossfade weight is header-inline because it rides the per-sample read. @@ -396,7 +405,7 @@ anything for a trigger shape. envelope that was never running. - **A live edit leaves the snapshot's own `sample.play` stale, on purpose.** The block, not the snapshot, is the audio thread's source; a new voice latches the stale copy and is corrected by - `snapLive` before its first frame. + `snapLive` (and a drawn EG by `applyContours`' snap) before its first frame. - **`keyboard_strip`'s width-uniformity guarantee is client-pixel only.** Its test sweep covers client-pixel widths (including multiples standing in for larger client areas); nothing in the instrument implements `IPlugViewContentScaleSupport`, so host-side DPI diff --git a/src/core/instrument/engine/CMakeLists.txt b/src/core/instrument/engine/CMakeLists.txt index 9c917b0..e44d042 100644 --- a/src/core/instrument/engine/CMakeLists.txt +++ b/src/core/instrument/engine/CMakeLists.txt @@ -26,10 +26,11 @@ add_subdirectory(filter) # marker layer. After filter: it links play_params' dependency set, which includes it. add_subdirectory(loop) -# The live-parameter block: the value layer plus its publication, deliberately linking no -# engine — the block is a plain value the voice observes, not a thing the engine owns. +# The live-parameter block and the drawn-contour block beside it: value layers plus their +# publication, deliberately linking no engine — a block is a plain value the voice observes, not +# a thing the engine owns. reasampler_pure_library(live_params - SOURCES live_params.cpp + SOURCES live_params.cpp live_contours.cpp LINK PUBLIC peaks velocity_curve filter curve_law) reasampler_test(live_params LINK live_params) @@ -52,6 +53,10 @@ reasampler_test(sampler_filter LINK sampler_core) # does to a voice that is already sounding, and what it must leave alone. reasampler_test(live_delivery LINK sampler_core) +# The drawn-contour half of live delivery. Its own executable because it replaces the global +# allocator to prove the audio-side swap allocates nothing, which must not leak into other tests. +reasampler_test(live_contours LINK sampler_core) + # The staged-envelope system across the same engine: per-segment curves, the sustain-less AHD # both mode shapes share, and the Trigger tail's terminal behaviour. reasampler_test(staged_envelopes LINK sampler_core) diff --git a/src/core/instrument/engine/live_contours.cpp b/src/core/instrument/engine/live_contours.cpp new file mode 100644 index 0000000..19542dc --- /dev/null +++ b/src/core/instrument/engine/live_contours.cpp @@ -0,0 +1,16 @@ +// live_contours.cpp — the VelocityCurve -> fixed-capacity copy the contour block carries. + +#include "core/instrument/engine/live_contours.h" + +namespace reasampler::instrument::engine { + +ContourPoints contourPoints(const VelocityCurve& curve) { + ContourPoints out{}; + const std::vector& pts = curve.points(); + const std::size_t n = pts.size() < kMaxCurvePoints ? pts.size() : kMaxCurvePoints; + for (std::size_t i = 0; i < n; ++i) out.points[i] = pts[i]; + out.count = static_cast(n); + return out; +} + +} // namespace reasampler::instrument::engine diff --git a/src/core/instrument/engine/live_contours.h b/src/core/instrument/engine/live_contours.h new file mode 100644 index 0000000..5db2fed --- /dev/null +++ b/src/core/instrument/engine/live_contours.h @@ -0,0 +1,50 @@ +#pragma once +// live_contours.h — the three drawn EG contours as one fixed-capacity plain value, and the block +// that publishes them to the audio thread so a sounding voice follows a contour edit. Separate +// from LiveValues on purpose: at kMaxCurvePoints x 3 this is ~9 KB, and riding the live block +// would put it on every knob publish, automation merge and field compare. + +#include +#include + +#include "core/instrument/engine/seqlock.h" +#include "core/instrument/engine/velocity_curve.h" + +namespace reasampler::instrument::engine { + +// One contour's points, copied out of its VelocityCurve so the audio thread can hold it without +// a vector. `count` never exceeds kMaxCurvePoints — the curve's own ceiling — and the points +// past it are ignored. +struct ContourPoints { + std::uint32_t count = 0; + VelocityPoint points[kMaxCurvePoints]; +}; + +// All three are carried whatever each envelope's mode: Staged|Spline is a note-on fact that +// travels by reload, so the voice decides which cursors to rebind and the block stays one shape. +struct LiveContours { + ContourPoints amp; + ContourPoints pitch; + ContourPoints filter; +}; + +static_assert(std::is_trivially_copyable_v, + "the contour block is copied under a seqlock — it must stay a plain value"); + +ContourPoints contourPoints(const VelocityCurve& curve); + +// Templated over the frames and seconds parameter representations for the same reason +// splineActive is (play_params.h): both spell the three fields identically. Off the audio thread. +template +LiveContours foldContours(const Play& p) { + LiveContours c{}; + c.amp = contourPoints(p.ampSpline.contour); + c.pitch = contourPoints(p.pitchSpline.contour); + c.filter = contourPoints(p.filterSpline.contour); + return c; +} + +// A class rather than an alias so play_params.h can forward-declare it. +class LiveContourBlock : public Seqlock {}; + +} // namespace reasampler::instrument::engine diff --git a/src/core/instrument/engine/live_params.h b/src/core/instrument/engine/live_params.h index 6b12332..b36fdd1 100644 --- a/src/core/instrument/engine/live_params.h +++ b/src/core/instrument/engine/live_params.h @@ -1,15 +1,15 @@ #pragma once // live_params.h — the live playback-parameter block: the plain value bundle the audio thread -// observes once per BLOCK, the single-writer seqlock that publishes it without a lock or a -// torn read, the ONE fold from PlayParams that keeps the two representations in step, and the +// observes once per BLOCK, its publication (the seqlock.h protocol) without a lock or a torn +// read, the ONE fold from PlayParams that keeps the two representations in step, and the // per-frame ramp that keeps a block-rate step inaudible. Ownership belongs above every // instrument snapshot (see SampleData::live). -#include #include #include #include "core/instrument/engine/play_params.h" +#include "core/instrument/engine/seqlock.h" namespace reasampler::instrument::engine { @@ -96,64 +96,9 @@ inline bool operator!=(const LiveValues& a, const LiveValues& b) { return !(a == // PlayParams — SampleData::keyTrack at the reload, InstrumentParams::keyTrack at a live commit. LiveValues foldLive(const PlayParams& params, double keyTrack); -// Single-writer / single-reader seqlock. The writer publishes a whole block between an odd -// and an even generation; the reader copies the block and re-checks the generation, retrying -// a bounded number of times, so it can never act on a half-applied edit. Wait-free for the -// reader: after the retry budget it reports "nothing new" and the caller keeps its last good -// snapshot rather than spinning on the audio thread. -// -// SINGLE-WRITER IS THE CALLER'S JOB and is load-bearing: two concurrent writers can leave the -// generation EVEN mid-write (A stores gen+1, B reads odd and stores gen+2) while both copy the -// block, and a reader then accepts a torn block as coherent. Every publisher must serialize. -// -// The plain (non-atomic) block copied across the fences is the standard pragmatic seqlock: -// the fences give correct ordering, but the concurrent read of a non-atomic object is a data -// race under the C++ object model, so TSan/UBSan will report it. That report is expected, not -// a defect — there is no clean lock-free standard-C++ alternative that keeps the block a plain -// value the audio thread can copy in one shot. -// -// The writer interface deliberately assumes NO particular thread beyond single-writer, so a -// host's own parameter-change queue (delivered on the audio thread with sample offsets) can -// drive it later without a redesign. -class LiveParams { -public: - // A generation of 0 means "never published"; the first publish lands on 2. - void publish(const LiveValues& values) { - const std::uint32_t gen = seq_.load(std::memory_order_relaxed); - seq_.store(gen + 1, std::memory_order_relaxed); // odd: a write is in progress - std::atomic_thread_fence(std::memory_order_release); - values_ = values; - std::atomic_thread_fence(std::memory_order_release); - // Skip 0 on wrap (~2^31 publishes): landing there would read as "never published" and - // stall every reader until the NEXT publish — a silent mode, unlike a loud one. - const std::uint32_t next = (gen + 2 == 0u) ? 2u : gen + 2; - seq_.store(next, std::memory_order_release); // even: complete and coherent - } - - // The last generation published, without copying the block — one relaxed load, so a reader - // that only needs "has anything moved" pays nothing for asking on a block where nothing has. - std::uint32_t generation() const { return seq_.load(std::memory_order_relaxed); } - - // Copies the block into `out` and returns the generation actually observed, or 0 when - // nothing has been published yet or the retry budget ran out (in which case `out` may hold - // a torn copy and MUST be discarded — compare the return against 0 before using it). - std::uint32_t read(LiveValues& out, int maxAttempts = 4) const { - for (int attempt = 0; attempt < maxAttempts; ++attempt) { - const std::uint32_t before = seq_.load(std::memory_order_acquire); - if (before == 0) return 0; // never published - if ((before & 1u) != 0u) continue; // writer mid-update - std::atomic_thread_fence(std::memory_order_acquire); - out = values_; - std::atomic_thread_fence(std::memory_order_acquire); - if (seq_.load(std::memory_order_relaxed) == before) return before; - } - return 0; - } - -private: - std::atomic seq_{0}; - LiveValues values_{}; -}; +// The live-parameter block's publication — seqlock.h owns the protocol and its single-writer +// contract. A class rather than an alias so play_params.h can forward-declare it. +class LiveParams : public Seqlock {}; // Linear per-frame glide with EXACT termination: once the target is within one step the value // becomes the target itself. An asymptotic smoother would leave the value forever a hair off, diff --git a/src/core/instrument/engine/play_params.h b/src/core/instrument/engine/play_params.h index c4229e9..7393241 100644 --- a/src/core/instrument/engine/play_params.h +++ b/src/core/instrument/engine/play_params.h @@ -15,7 +15,8 @@ namespace reasampler { -namespace instrument::engine { class LiveParams; } // live_params.h; SampleData holds a pointer +// SampleData holds a pointer to each (live_params.h, live_contours.h). +namespace instrument::engine { class LiveParams; class LiveContourBlock; } using audio::AudioSample; using instrument::engine::VelocityCurve; @@ -181,8 +182,8 @@ struct PlayParams { // The three drawn contours: the alternative to adsr/trigAhd, to pitchEnv.shape, and to // filter.env/trigEnv respectively. They sit HERE rather than inside the three envelope // structs because those are copied whole into the live block, which must stay trivially - // copyable (live_params.h) — and a contour is not a live control anyway: like the velocity - // curves it travels by reload. + // copyable (live_params.h). A contour reaches a sounding voice through its own block + // instead (live_contours.h); only its Staged|Spline mode travels by reload. SplineEnv ampSpline; SplineEnv pitchSpline; SplineEnv filterSpline; @@ -295,6 +296,9 @@ struct SampleData { // same knob as a live one. That is the desired behaviour — it is the note the user is // hearing. Do not "fix" it by moving ownership into the snapshot. const instrument::engine::LiveParams* live = nullptr; + // The drawn-contour peer of `live`, under the same ownership and null contract: null plays + // the contours in `play` for the note's whole life. + const instrument::engine::LiveContourBlock* liveContours = nullptr; // A framesR of a different length than frames is treated as absent — a malformed pair // never half-plays. diff --git a/src/core/instrument/engine/seqlock.h b/src/core/instrument/engine/seqlock.h new file mode 100644 index 0000000..cb36215 --- /dev/null +++ b/src/core/instrument/engine/seqlock.h @@ -0,0 +1,76 @@ +#pragma once +// seqlock.h — the lock-free publication every live block rides from its off-thread writer to +// the audio thread: a single-writer seqlock over one plain value. Generic over that value so the +// live-parameter block and the drawn-contour block share one implementation of the protocol. + +#include +#include +#include + +namespace reasampler::instrument::engine { + +// Single-writer seqlock. The writer publishes a whole block between an odd and an even +// generation; a reader copies the block and re-checks the generation, retrying a bounded number +// of times, so it can never act on a half-applied edit. Wait-free for the reader: after the +// retry budget it reports "nothing new" and the caller keeps its last good snapshot rather than +// spinning on the audio thread. Reading does not mutate, so any number of readers may share one +// block (the live and the draining engine both do). +// +// SINGLE-WRITER IS THE CALLER'S JOB and is load-bearing: two concurrent writers can leave the +// generation EVEN mid-write (A stores gen+1, B reads odd and stores gen+2) while both copy the +// block, and a reader then accepts a torn block as coherent. Every publisher must serialize. +// +// The plain (non-atomic) block copied across the fences is the standard pragmatic seqlock: +// the fences give correct ordering, but the concurrent read of a non-atomic object is a data +// race under the C++ object model, so TSan/UBSan will report it. That report is expected, not +// a defect — there is no clean lock-free standard-C++ alternative that keeps the block a plain +// value the audio thread can copy in one shot. +// +// The writer interface deliberately assumes NO particular thread beyond single-writer, so a +// host's own parameter-change queue (delivered on the audio thread with sample offsets) can +// drive it later without a redesign. +template +class Seqlock { + static_assert(std::is_trivially_copyable_v, + "the block is copied under a seqlock — it must stay a plain value"); + +public: + // A generation of 0 means "never published"; the first publish lands on 2. + void publish(const T& values) { + const std::uint32_t gen = seq_.load(std::memory_order_relaxed); + seq_.store(gen + 1, std::memory_order_relaxed); // odd: a write is in progress + std::atomic_thread_fence(std::memory_order_release); + values_ = values; + std::atomic_thread_fence(std::memory_order_release); + // Skip 0 on wrap (~2^31 publishes): landing there would read as "never published" and + // stall every reader until the NEXT publish — a silent mode, unlike a loud one. + const std::uint32_t next = (gen + 2 == 0u) ? 2u : gen + 2; + seq_.store(next, std::memory_order_release); // even: complete and coherent + } + + // The last generation published, without copying the block — one relaxed load, so a reader + // that only needs "has anything moved" pays nothing for asking on a block where nothing has. + std::uint32_t generation() const { return seq_.load(std::memory_order_relaxed); } + + // Copies the block into `out` and returns the generation actually observed, or 0 when + // nothing has been published yet or the retry budget ran out (in which case `out` may hold + // a torn copy and MUST be discarded — compare the return against 0 before using it). + std::uint32_t read(T& out, int maxAttempts = 4) const { + for (int attempt = 0; attempt < maxAttempts; ++attempt) { + const std::uint32_t before = seq_.load(std::memory_order_acquire); + if (before == 0) return 0; // never published + if ((before & 1u) != 0u) continue; // writer mid-update + std::atomic_thread_fence(std::memory_order_acquire); + out = values_; + std::atomic_thread_fence(std::memory_order_acquire); + if (seq_.load(std::memory_order_relaxed) == before) return before; + } + return 0; + } + +private: + std::atomic seq_{0}; + T values_{}; +}; + +} // namespace reasampler::instrument::engine diff --git a/src/core/instrument/engine/velocity_curve.h b/src/core/instrument/engine/velocity_curve.h index 34532ec..dc1bb75 100644 --- a/src/core/instrument/engine/velocity_curve.h +++ b/src/core/instrument/engine/velocity_curve.h @@ -224,17 +224,20 @@ private: // sample. A jump (a loop wrap, a fresh note) falls back to a binary search, <= 7 steps at the // 128-point ceiling. // -// Holds a RAW POINTER into the bound curve's point array: the caller guarantees the curve -// outlives the cursor. The voice binds against its SampleData, which has exactly that lifetime. +// Holds a RAW POINTER into the bound point array: the caller guarantees the array outlives the +// cursor, or rebinds it first. The voice binds against its SampleData, which has exactly that +// lifetime, or against its engine's live contour copy, which the engine rebinds before reusing. class SplineCursor { public: // Binds `c` if it has an evaluable segment; a shorter curve leaves the cursor inactive so // the caller's `if (active())` skips the whole spline path. - void bind(const VelocityCurve& c) { - const std::vector& pts = c.points(); - if (pts.size() < 2) { clear(); return; } - pts_ = pts.data(); - n_ = pts.size(); + void bind(const VelocityCurve& c) { bind(c.points().data(), c.points().size()); } + // The same over a raw X-ordered array (live_contours.h's fixed-capacity copy). Rebinding a + // cursor mid-read is legal: it restarts the segment cache, and the next eval re-locates. + void bind(const VelocityPoint* pts, std::size_t n) { + if (pts == nullptr || n < 2) { clear(); return; } + pts_ = pts; + n_ = n; select(0); } void clear() { pts_ = nullptr; n_ = 0; } diff --git a/src/core/instrument/engine/voice.cpp b/src/core/instrument/engine/voice.cpp index a8b4e11..ded44cc 100644 --- a/src/core/instrument/engine/voice.cpp +++ b/src/core/instrument/engine/voice.cpp @@ -109,6 +109,9 @@ void Voice::start(int note, int velocity, const SampleData& sample, bool declick } else { filterSplineCur_.clear(); } + ampSplineSmooth_.clear(); + pitchSplineSmooth_.clear(); + filterSplineSmooth_.clear(); // Amplitude envelope: Gate = AHDSR (all five fields read from play.adsr, resolved to // frames from stored seconds at load time); Trigger = the staged AHD over the % play span. @@ -330,6 +333,29 @@ void Voice::applyLive(const instrument::engine::LiveValues& live, bool snap) { rMorph_.moving() || rDrive_.moving(); } +namespace { + +// One EG's half of Voice::applyContours. `before` is read through the OLD binding, which is +// still intact: the engine only overwrites a buffer no active voice is bound to. +void rebindContour(SplineCursor& cursor, StepSmoother& smooth, + const instrument::engine::ContourPoints& pts, double phase, bool snap) { + if (!cursor.active()) return; + const double before = snap ? 0.0 : cursor.eval(phase); + cursor.bind(pts.points, pts.count); + if (snap || !cursor.active()) return; + const double after = cursor.eval(phase); + if (after != before) smooth.absorb(before - after); +} + +} // namespace + +void Voice::applyContours(const instrument::engine::LiveContours& contours, bool snap) { + const double phase = splinePhase(); + rebindContour(ampSplineCur_, ampSplineSmooth_, contours.amp, phase, snap); + rebindContour(pitchSplineCur_, pitchSplineSmooth_, contours.pitch, phase, snap); + rebindContour(filterSplineCur_, filterSplineSmooth_, contours.filter, phase, snap); +} + void Voice::retune(int note) { // Mono legato takeover: move the pitch, touch NOTHING else — the amplitude envelope keeps // running (no re-attack), the read head keeps its position, the shifter keeps its ring diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index 7a135e6..6e225c1 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -15,6 +15,7 @@ #include "core/instrument/engine/envelopes.h" #include "core/instrument/engine/filter/filter_params.h" #include "core/instrument/engine/filter/voice_filter.h" +#include "core/instrument/engine/live_contours.h" #include "core/instrument/engine/live_params.h" #include "core/instrument/engine/loop/loop_span.h" #include "core/instrument/engine/pitch_shift.h" @@ -179,6 +180,14 @@ public: // pitch ratio, and the decoded PCM stay latched at note-on. void applyLive(const instrument::engine::LiveValues& live, bool snap); + // Re-points every drawn EG this note is ALREADY reading at `contours` — a cursor start() + // left cleared stays cleared, because Staged|Spline is a note-on fact. Block-boundary only, + // like applyLive. Without `snap` the level step at the current read position is absorbed + // by that EG's StepSmoother, the same way a live reshape of a positional AHD is; with it + // (a voice that has rendered nothing) the new contour is taken outright. `contours` must + // outlive the binding or be rebound first — VoiceEngine's double buffer owns that. + void applyContours(const instrument::engine::LiveContours& contours, bool snap); + // Pre-sizes this voice's Preserve pitch shifters (both channels) to `windowFrames`, off // the audio thread (allocates; also sizes the prime scratch buffer), so start() — which // runs inside process() — never allocates. <= 1 leaves the shifters pass-through. @@ -268,6 +277,14 @@ private: // its opening value. double splinePhase() const { return readPos_ * splineScale_; } + // One drawn EG's value this frame: the contour at the read position plus whatever step a + // live contour swap left to absorb (applyContours). The smoother is inactive at rest, so a + // contour nobody is editing costs one predicted branch over the bare cursor read. + double readSpline(SplineCursor& cursor, StepSmoother& smooth) { + const double v = cursor.eval(splinePhase()); + return smooth.active() ? v + smooth.advance() : v; + } + // This frame's amplitude in [0,1] from the active envelope. Spline: the drawn contour read // at the normalized position (one cached-segment compare per frame). Gate: AHDSR ticks once // per output frame (envelope time is wall-clock, independent of read rate). Trigger: the AHD @@ -284,7 +301,7 @@ private: if (ampSplineCur_.active() && playMode_ == PlayMode::Trigger) { // Early-free at a genuine permanent terminus (the spline analogue of a staged AHD's // finished()) — onFinalSegment()/segmentEndValue()'s own doc comments own the why. - amp = ampSplineCur_.eval(splinePhase()); + amp = readSpline(ampSplineCur_, ampSplineSmooth_); if (amp == 0.0 && ampSplineCur_.onFinalSegment() && ampSplineCur_.segmentEndValue() == 0.0) { amplitudeDone_ = true; @@ -321,7 +338,7 @@ private: // the source-offset AHD in Trigger. playMode_ is fixed for the note's lifetime, so the // branch is perfectly predicted. const double envOut = filterSplineCur_.active() - ? filterSplineCur_.eval(splinePhase()) + ? readSpline(filterSplineCur_, filterSplineSmooth_) : ((playMode_ == PlayMode::Gate) ? filterEnv_.tick() : filterAhd_.amplitudeAt(sourceOffset())); @@ -514,9 +531,10 @@ private: seedTerminalDeclick(); } const double gain = amp * velocityGain_; - const double pitchEnvSemis = pitchSplineCur_.active() - ? pitchSplineDepth_ * pitchSplineCur_.eval(splinePhase()) - : pitchEnv_.tick(); + const double pitchEnvSemis = + pitchSplineCur_.active() + ? pitchSplineDepth_ * readSpline(pitchSplineCur_, pitchSplineSmooth_) + : pitchEnv_.tick(); // 2^(semis/12); when the envelope is off (semis exactly 0) this is 1.0 and skips the // pow entirely — no per-frame transcendental on the common path. @@ -748,13 +766,18 @@ private: bool amplitudeDone_ = false; // set when the active amplitude envelope finished // The three drawn contours, bound at note-on to the loaded capture's own point arrays (the - // SampleData outlives the voice — same contract as sample_). A Staged EG leaves its cursor - // inactive, so a purely staged instrument's per-sample path gains three predicted branches - // and nothing else. splineScale_ is 1/frameCount, the readPos -> [0,1] map every contour - // shares; pitchSplineDepth_ is the pitch envelope's peak, zero while it is disabled. + // SampleData outlives the voice — same contract as sample_) and, when the engine observes a + // contour block, re-pointed at its engine's live copy (applyContours). A Staged EG leaves its + // cursor inactive, so a purely staged instrument's per-sample path gains three predicted + // branches and nothing else. splineScale_ is 1/frameCount, the readPos -> [0,1] map every + // contour shares; pitchSplineDepth_ is the pitch envelope's peak, zero while it is disabled. + // Each smoother holds the step a live contour swap left, in the contour's own y domain. SplineCursor ampSplineCur_; SplineCursor pitchSplineCur_; SplineCursor filterSplineCur_; + StepSmoother ampSplineSmooth_; + StepSmoother pitchSplineSmooth_; + StepSmoother filterSplineSmooth_; double splineScale_ = 0.0; double pitchSplineDepth_ = 0.0; diff --git a/src/core/instrument/engine/voice_engine.cpp b/src/core/instrument/engine/voice_engine.cpp index f7ab3cf..eed4e4c 100644 --- a/src/core/instrument/engine/voice_engine.cpp +++ b/src/core/instrument/engine/voice_engine.cpp @@ -52,7 +52,25 @@ void VoiceEngine::applyLiveToActive() { } } +void VoiceEngine::applyContoursToActive() { + const instrument::engine::LiveContourBlock* block = sample_.liveContours; + if (block == nullptr) return; // bare engine: every voice keeps its snapshot's contours + const std::uint32_t published = block->generation(); + if (published == 0 || published == contourGeneration_) return; + const std::size_t back = contourFront_ ^ 1u; + const std::uint32_t observed = block->read(contours_[back]); + if (observed == 0) return; // torn: the back buffer is discarded, retried next boundary + contourFront_ = back; + contourGeneration_ = observed; + haveContours_ = true; + for (Voice& voice : voices_) { + if (voice.active()) voice.applyContours(contours_[contourFront_], /*snap=*/false); + } +} + void VoiceEngine::startVoice(Voice& voice, int note, int velocity) { + // FIRST, so no sounding voice is left bound to a buffer the next read overwrites. + applyContoursToActive(); refreshLive(); // THE read of the note-on-latched commit class, and the only one: a published block outranks // the snapshot's own copy (a live edit deliberately leaves that stale), and applyLive below @@ -65,6 +83,8 @@ void VoiceEngine::startVoice(Voice& voice, int note, int velocity) { voice.start(note, velocity, sample_, /*declickTakeover=*/takeoverDeclick_, rate, keyTrack, lengthFraction); if (haveLive_) voice.applyLive(live_, /*snap=*/true); + // The snapshot's contours are stale after a live edit, exactly as its play params are. + if (haveContours_) voice.applyContours(contours_[contourFront_], /*snap=*/true); voice.setStartOrder(nextStartOrder_++); } @@ -258,6 +278,7 @@ void VoiceEngine::render(AudioSample* out, std::size_t frameCount) { // audio thread never touches the heap. if (out == nullptr || frameCount == 0) return; applyLiveToActive(); // block boundary, once — never inside the frame loop + applyContoursToActive(); for (Voice& voice : voices_) { if (!voice.active()) continue; for (std::size_t f = 0; f < frameCount; ++f) { @@ -274,6 +295,7 @@ void VoiceEngine::render(AudioSample* left, AudioSample* right, std::size_t fram // discipline; only the per-frame call differs (renderFrameStereo vs renderFrame). if (left == nullptr || right == nullptr || frameCount == 0) return; applyLiveToActive(); // block boundary, once — never inside the frame loop + applyContoursToActive(); for (Voice& voice : voices_) { if (!voice.active()) continue; for (std::size_t f = 0; f < frameCount; ++f) { diff --git a/src/core/instrument/engine/voice_engine.h b/src/core/instrument/engine/voice_engine.h index e06995a..f437926 100644 --- a/src/core/instrument/engine/voice_engine.h +++ b/src/core/instrument/engine/voice_engine.h @@ -10,6 +10,7 @@ #include #include "core/audio/peaks.h" +#include "core/instrument/engine/live_contours.h" #include "core/instrument/engine/live_params.h" #include "core/instrument/engine/play_params.h" #include "core/instrument/engine/voice.h" @@ -57,6 +58,11 @@ public: MonoTrigger monoTrigger = MonoTrigger::Retrigger, bool takeoverDeclick = false); + // Voices may hold raw pointers into this engine's own contour buffers, so a copy would + // leave them reading the original's. + VoiceEngine(const VoiceEngine&) = delete; + VoiceEngine& operator=(const VoiceEngine&) = delete; + // MIDI note-on. Allocates a free voice, or steals one per the policy above. Returns the // index of the voice used, or kNoVoice when nothing is playable (no decoded PCM, an // out-of-range note, or a Preserve note-on past the cap) — a defined no-play, not an error. @@ -124,6 +130,22 @@ private: std::uint32_t liveGeneration_ = 0; // last generation observed; 0 = none yet bool haveLive_ = false; + // --- Drawn-contour observation (live_contours.h) --- + // Reads a new contour generation, when one landed, into the buffer no voice is bound to and + // rebinds EVERY active voice to it before returning — which is what keeps the other buffer + // free for the next read. That invariant is why this, and never a bare read, is what both + // the block boundary and startVoice call. + void applyContoursToActive(); + + // Double-buffered because a voice's SplineCursor points INTO the buffer: reading over it + // would change a contour under a sounding voice without the rebind that absorbs the step, + // and a torn read (which the seqlock only reports after the fact) must land somewhere no + // voice reads. ~9 KB each, in the engine, so nothing on the audio thread allocates. + std::array contours_{}; + std::size_t contourFront_ = 0; + std::uint32_t contourGeneration_ = 0; // last generation observed; 0 = none yet + bool haveContours_ = false; + // Count of active Preserve-engine voices (for the Preserve cap). Rescanned per note-on // (cheap: bounded by maxVoices) rather than maintained as a running tally. std::size_t activePreserveVoices() const; diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index d21cd82..1e34d86 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -84,6 +84,16 @@ The editor's `commitLive` is the tier-3 peer of `commitAndReload`; why it still parameter set is recorded at its declaration in `reasampler_editor.h`, and why `liveParams_` is declared ahead of the instrument slots at that member in `reasampler_processor.h`. +**The drawn-contour drag is the one gesture that publishes WITHOUT committing.** Each move +couriers the editor's contours to `liveContours_` (`publishLiveContours`) and writes nothing +else — no parameter set, no generation, no host callback — and its release is the +`commitAndReload` it always was. That is sound because no contour is a host parameter, so no +host-facing reader needs the model to carry it mid-gesture; it is what keeps a motion event off +the host entirely (an unlatched `setInstrumentParams` opens a group edit even when nothing +exposed moved). `reloadInstrument` republishes the model's contours, so a release, a click-edit +(add/delete/hard) and a restore all reach the voices through the reload; capture-lost, the one +exit that commits nothing, re-couriers the restored `params_`. + **The VST3 parameter surface is a THIRD surface onto the one model, never a second copy.** The pure half — the frozen id table, the exposed set, the plain-value layer, the formatter — is `core/instrument/param` and is documented there; this directory only adapts it. @@ -109,7 +119,8 @@ pure half — the frozen id table, the exposed set, the plain-value layer, the f the audio thread and may allocate on the way; `process()` merges that block with the host's automation points into `automationLive_`, which is what `SampleData::live` points at. Two blocks rather than one because the seqlock's single-writer contract is load-bearing and the two - writers genuinely differ in thread. + writers genuinely differ in thread. The drawn-contour block `liveContours_` has no such pair: + no host automates a contour, so the engines read it straight from its off-thread writers. ### THE AUTHORITY MODEL — who may write a parameter's value, and until when @@ -123,6 +134,7 @@ the single authority.** Everything else that holds these values is a cache or a | Writer | Authority begins | Authority ends | |---|---|---| | Editor gesture (`commitLive` / `commitAndReload`) | mouse-down | the commit lands in the model | +| Contour drag's courier (`publishLiveContours`) | mouse-down | never writes the model — the release's `commitAndReload` does | | Host controller write (`setParamNormalized`) | the call | the call returns (it writes the model) | | State restore (`setState`) | the call | the call returns | | Bake reset (`adoptBakedCapture`) | the call | the call returns | @@ -130,9 +142,9 @@ the single authority.** Everything else that holds these values is a cache or a | Reload seed (`reloadInstrument`) | never — it does not write `params_` | it only republishes a live block folded from whatever the model already holds | | **Host automation point** (`IParameterChanges`) | the block it lands in | **the UI thread has folded it into the model and republished** | -Every writer above except the last two writes the model directly, so for those "authority ends" -is just "the write happened". Reload seed is not itself a model write — `reloadInstrument` never -touches `params_`; the only write in the tree is `setInstrumentParams`'s, `processor_state.cpp:193` +Every writer above except the contour courier and the last two writes the model directly, so for +those "authority ends" is just "the write happened". Reload seed is not itself a model write — +`reloadInstrument` never touches `params_`; the only write in the tree is `setInstrumentParams`'s, `processor_state.cpp:193` — which is why its row states no authority window of its own. The automation lane is the only one that cannot write directly: the SDK delivers it on the audio thread, where the model path allocates (`resolvePlay` copies velocity curves and spline contours). So it patches the diff --git a/src/shell/instrument/editor_input_waveform.cpp b/src/shell/instrument/editor_input_waveform.cpp index 27332cb..6eb0d3c 100644 --- a/src/shell/instrument/editor_input_waveform.cpp +++ b/src/shell/instrument/editor_input_waveform.cpp @@ -178,6 +178,7 @@ bool ReaSamplerEditor::splineOverlayClick(const OverlayArea& waveArea, int x, in // At the ceiling: refused, contour untouched, nothing to roll back. Swallow the click // rather than letting it fall through to a marker grab under the cursor. if (index < 0) return true; + publishContoursLive(); // the new point is heard before the first move, not after it } drag_ = DragKind::kSplineNode; curvePointIndex_ = index; @@ -188,7 +189,7 @@ bool ReaSamplerEditor::splineOverlayClick(const OverlayArea& waveArea, int x, in dragCurveRect_ = waveArea.rect; dragStartX_ = x; dragStartY_ = y; - invalidate(); // live feedback; the commit lands on WM_LBUTTONUP + invalidate(); // the model commit lands on WM_LBUTTONUP return true; } @@ -235,7 +236,8 @@ void ReaSamplerEditor::dragWaveform(const FaceLayout& fl, int x, int y) { splineFor(overlayEnv_) = VelocityCurve::resolvePointDrag( dragStartCurve_, static_cast(curvePointIndex_), splineOverlayBox(overlay), dx, y - dragStartY_); - invalidate(); // live feedback; commit on release + publishContoursLive(); // heard now; the model write stays the release's commit + invalidate(); return; } diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index d329fe5..5631266 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -312,6 +312,10 @@ LRESULT CALLBACK ReaSamplerEditor::wndProc(HWND hwnd, UINT msg, WPARAM wParam, // drag used. if (self->dragCommitsLive(self->drag_, self->dragParamId_)) { self->commitLive(); + } else if (self->drag_ == DragKind::kSplineNode) { + // The contour drag reached the voices without the model; restore + // the voices to it the same way (see publishContoursLive). + self->publishContoursLive(); } } // Peer of onMouseUp's bracket close, and after the rollback for the same diff --git a/src/shell/instrument/editor_session.cpp b/src/shell/instrument/editor_session.cpp index 2b82324..984cf5c 100644 --- a/src/shell/instrument/editor_session.cpp +++ b/src/shell/instrument/editor_session.cpp @@ -251,6 +251,10 @@ void ReaSamplerEditor::commitLive() { processor_->publishLiveParams(); } +void ReaSamplerEditor::publishContoursLive() { + if (processor_) processor_->publishLiveContours(params_.play); +} + bool ReaSamplerEditor::deckKnobIsProcessorSide(int paramId) { return paramId == -2 || paramId == static_cast(ParamControl::kVoiceCount) || diff --git a/src/shell/instrument/processor_reload.cpp b/src/shell/instrument/processor_reload.cpp index 5c32d62..e4306de 100644 --- a/src/shell/instrument/processor_reload.cpp +++ b/src/shell/instrument/processor_reload.cpp @@ -153,6 +153,7 @@ std::string ReaSamplerProcessor::reloadInstrument() { // top of the first block after this, which is where the swap below becomes visible // too — so the new snapshot's first note reads it. sample.live = &automationLive_; + sample.liveContours = &liveContours_; // BEFORE the publish: the publish is what makes the audio thread re-merge, and the // merge resolves every held automation stage time against this rate. Stored after, // a merge in the window between them would resolve them against the previous @@ -164,6 +165,10 @@ std::string ReaSamplerProcessor::reloadInstrument() { // publishLiveParams never holds reloadMutex_, so this is the only nesting. std::lock_guard lp(livePublishMutex_); liveParams_.publish(instrument::engine::foldLive(sample.play, sample.keyTrack)); + // The model's contours, so a reload-tier contour edit (a point added, deleted or + // made hard, a drag's release) reaches the drain's sounding voices too, and a + // courier publish from an abandoned gesture cannot outlive the reload. + liveContours_.publish(instrument::engine::foldContours(sample.play)); } resolvedId = selId; // the concrete pick that resolved } diff --git a/src/shell/instrument/processor_state.cpp b/src/shell/instrument/processor_state.cpp index c33fa27..b562c70 100644 --- a/src/shell/instrument/processor_state.cpp +++ b/src/shell/instrument/processor_state.cpp @@ -293,6 +293,14 @@ AutomationChannel::ReleaseProof ReaSamplerProcessor::publishLiveParams() { return AutomationChannel::ReleaseProof::fromPublish(before, liveParams_.generation()); } +void ReaSamplerProcessor::publishLiveContours(const PlaySeconds& play) { + // Folded outside the lock, as publishLiveParams' block is. A contour is normalized time, so + // unlike that block it needs no rate and publishes even before anything is built. + const instrument::engine::LiveContours contours = instrument::engine::foldContours(play); + std::lock_guard lock(livePublishMutex_); + liveContours_.publish(contours); +} + SampleRefs ReaSamplerProcessor::sampleRefs() { std::lock_guard lock(refsMutex_); return sampleRefs_; diff --git a/src/shell/instrument/reasampler_editor.h b/src/shell/instrument/reasampler_editor.h index 8750798..1302581 100644 --- a/src/shell/instrument/reasampler_editor.h +++ b/src/shell/instrument/reasampler_editor.h @@ -267,6 +267,12 @@ private: // the knob. Does not repaint; callers already do. UI thread only. void commitLive(); + // The drawn-contour drag's per-move publish: params_' three contours straight to the voices, + // with NO parameter-set write — why is this directory's CLAUDE.md (the contour-drag note + // under the commit tiers). Every path that ends such a drag without its release's + // commitAndReload — capture-lost — must call this again after restoring params_. UI thread. + void publishContoursLive(); + // Whether an in-flight drag commits live rather than through a reload. A deck knob is // live per deckParamCommit; an envelope-node drag is live in EITHER mode — see // liveCommitFor (deck_groups.h) for why. diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index 8b0f177..71361ae 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -23,6 +23,7 @@ #include "core/instrument/map/sample_map.h" // InstrumentParams (the one parameter set) #include "core/instrument/map/component_state_io.h" // ComponentState codec #include "core/instrument/engine/limiter.h" // the master bus's post-gain limiter +#include "core/instrument/engine/live_contours.h" // LiveContourBlock (the drawn-contour block) #include "core/instrument/engine/live_params.h" // LiveParams (the live-parameter block) #include "core/instrument/engine/meter_accumulate.h" // the meter's block-rate folds + consume #include "core/instrument/param/param_id.h" // the frozen ParamId space + DeckParam binding @@ -31,6 +32,7 @@ namespace reasampler::vst { using instrument::map::ComponentState; using instrument::map::InstrumentParams; +using instrument::map::PlaySeconds; using instrument::map::SampleRefEntry; using instrument::map::SampleRefs; using instrument::map::kPreviewVelocityDefault; @@ -206,6 +208,11 @@ public: // bakes and publishes). UI thread; serialized against reloadInstrument's own publish. Returns drainAutomationToModel's release proof (automation_channel.h); other callers ignore it. AutomationChannel::ReleaseProof publishLiveParams(); + // Publishes `play`'s three drawn contours to every sounding voice WITHOUT writing the + // model — the contour drag's courier (this directory's CLAUDE.md, the authority table). + // UI thread; serialized against reloadInstrument's own contour publish. + void publishLiveContours(const PlaySeconds& play); + // Per-instance channel mode (mono | stereo), guarded by channelModeMutex_, never read // on the audio thread. Decode policy only (downmix vs L/R split) — the output bus is // fixed stereo, so a mode change never renegotiates host I/O. @@ -387,6 +394,10 @@ private: // Both live_ and draining_ observe this same block — a block owned by a snapshot would // leave the drain's still-sounding voices deaf to the knob under them. instrument::engine::LiveParams liveParams_; + // The drawn-contour block, SampleData::liveContours' one target, under liveParams_' ordering + // constraint and for its reason. No automation merge: no contour is a host parameter, so both + // engines read the model/editor publish directly. + instrument::engine::LiveContourBlock liveContours_; // The block the ENGINE reads, and the ONE thing SampleData::live points at. Written only by // the audio thread, which merges liveParams_ with the host's automation points once per block // and republishes ONLY when the RESULT moved — so neither an unchanged model nor a lane @@ -404,10 +415,10 @@ private: // The host lane's state and its release protocol; automation_channel.h owns the mechanism and // this directory's CLAUDE.md the authority model it enforces. AutomationChannel automation_; - // Serializes liveParams_.publish's two writer sites (reloadInstrument, publishLiveParams) - // only — separate from reloadMutex_ so a knob drag's publish never blocks behind a - // reload's WAV decode. The audio thread never takes this; process() only reads via - // LiveParams::read's lock-free seqlock retry. + // Serializes the writer sites of liveParams_ (reloadInstrument, publishLiveParams) and of + // liveContours_ (reloadInstrument, publishLiveContours) only — separate from reloadMutex_ so + // a drag's publish never blocks behind a reload's WAV decode. The audio thread never takes + // this; it only reads via the seqlock's lock-free retry. std::mutex livePublishMutex_; // The rate the loaded capture was decoded/built at, so a live republish resolves the // stored wall-clock seconds to exactly the frames the built SampleData carries. 0 = nothing diff --git a/tests/test_live_contours.cpp b/tests/test_live_contours.cpp new file mode 100644 index 0000000..e0717f6 --- /dev/null +++ b/tests/test_live_contours.cpp @@ -0,0 +1,438 @@ +// Standalone tests for LIVE CONTOUR DELIVERY — a drawn EG edited while notes sound — no VST3, +// no REAPER, no framework. Asserts that each of the three contours (amp, pitch, filter) moves +// the note already sounding and the next one struck, that the swap is absorbed rather than +// stepped, that what start() latched (a Staged EG) stays latched, that an unchanged publish is +// byte-identical, that two engines sharing one block both follow (the drain slot), and that the +// whole audio-side path allocates nothing. This executable replaces the global allocator to +// prove the last one, which is why these tests are not in live_delivery_tests. + +#include "../src/core/instrument/engine/live_contours.h" +#include "../src/core/instrument/engine/voice_engine.h" + +#include +#include +#include +#include +#include +#include +#include + +using namespace reasampler; +using instrument::engine::ContourPoints; +using instrument::engine::CurveDomain; +using instrument::engine::LiveContourBlock; +using instrument::engine::contourPoints; +using instrument::engine::foldContours; + +// --- Allocation counter ----------------------------------------------------------------- +// Counts every global allocation while armed. Replacing the global operator new is the only way +// to observe allocations made inside sampler_core without instrumenting it. + +static std::atomic g_allocations{0}; +static std::atomic g_countAllocations{false}; + +static void* countedAlloc(std::size_t n) { + if (g_countAllocations.load(std::memory_order_relaxed)) { + g_allocations.fetch_add(1, std::memory_order_relaxed); + } + if (void* p = std::malloc(n ? n : 1)) return p; + throw std::bad_alloc(); +} + +void* operator new(std::size_t n) { return countedAlloc(n); } +void* operator new[](std::size_t n) { return countedAlloc(n); } +void operator delete(void* p) noexcept { std::free(p); } +void operator delete[](void* p) noexcept { std::free(p); } +void operator delete(void* p, std::size_t) noexcept { std::free(p); } +void operator delete[](void* p, std::size_t) noexcept { std::free(p); } + +// ------------------------------------------------------------------------------------------ + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +constexpr double kPi = 3.14159265358979323846; +constexpr int kRate = 48000; +constexpr std::size_t kBlock = 256; +constexpr int kRoot = 60; + +// Every frame 1.0, Trigger (a drawn EG is a Trigger fact — resolvePlay's job on the real path), +// played at the root with a flat velocity curve: an output frame IS the amplitude. +static SampleData dcSample(std::size_t frames) { + SampleData s; + s.frames.assign(frames, 1.0f); + s.sampleRate = kRate; + s.rootNote = kRoot; + s.play.playMode = PlayMode::Trigger; + return s; +} + +static VelocityCurve flatAt(double y, CurveDomain domain) { + return VelocityCurve::fromPoints({{0.0, y}, {127.0, y}}, domain); +} + +// Three points rather than two, so a publish that changes the point COUNT is what lands. +static VelocityCurve flatAt3(double y, CurveDomain domain) { + return VelocityCurve::fromPoints({{0.0, y}, {63.0, y}, {127.0, y}}, domain); +} + +static double maxAbsDelta(const std::vector& v, std::size_t from, std::size_t to) { + double worst = 0.0; + for (std::size_t i = from + 1; i < to && i < v.size(); ++i) { + worst = (std::max)(worst, std::fabs(static_cast(v[i]) - v[i - 1])); + } + return worst; +} + +static double peakOf(const std::vector& v, std::size_t from, std::size_t to) { + double peak = 0.0; + for (std::size_t i = from; i < to && i < v.size(); ++i) { + peak = (std::max)(peak, std::fabs(static_cast(v[i]))); + } + return peak; +} + +// One held note through a one-voice engine observing `block`, with `edited`'s contours +// published at the top of block `swapBlock` (-1: never). The initial publish is `sample.play`'s +// own contours, the state a reload leaves. +static std::vector renderHeld(SampleData& sample, LiveContourBlock& block, + const PlayParams* edited, int swapBlock, int blocks) { + sample.liveContours = █ + block.publish(foldContours(sample.play)); + VoiceEngine engine(1, sample); + engine.noteOn(kRoot, 127); + std::vector out; + for (int b = 0; b < blocks; ++b) { + if (edited && b == swapBlock) block.publish(foldContours(*edited)); + engine.render(out, kBlock); + } + return out; +} + +// --- The fold ------------------------------------------------------------------------------ + +static void testTheFoldCarriesEveryPointAndItsHardFlag() { + std::vector pts = {{0.0, 0.1}, {40.0, 0.9}, {90.0, 0.2}, {127.0, 0.7}}; + VelocityCurve c = VelocityCurve::fromPoints(pts, CurveDomain::Unipolar); + c.setHard(2, true); + const ContourPoints folded = contourPoints(c); + CHECK(folded.count == 4); + for (std::size_t i = 0; i < 4; ++i) { + CHECK(folded.points[i].velocity == c.points()[i].velocity); + CHECK(folded.points[i].value == c.points()[i].value); + CHECK(folded.points[i].hard == c.points()[i].hard); + } + CHECK(folded.points[2].hard); + + PlayParams p; + p.ampSpline.contour = flatAt(0.3, CurveDomain::Unipolar); + p.pitchSpline.contour = c; + p.filterSpline.contour = flatAt3(0.6, CurveDomain::Unipolar); + const auto all = foldContours(p); + CHECK(all.amp.count == 2 && all.amp.points[1].value == 0.3); + CHECK(all.pitch.count == 4 && all.pitch.points[1].value == c.points()[1].value); + CHECK(all.filter.count == 3 && all.filter.points[2].value == 0.6); +} + +// --- Amp ----------------------------------------------------------------------------------- + +static void testAHeldNoteFollowsAnAmpContourEditWithoutAStep() { + SampleData s = dcSample(200000); + s.play.ampSpline.mode = EnvMode::Spline; + s.play.ampSpline.contour = flatAt(1.0, CurveDomain::Unipolar); + PlayParams edited = s.play; + edited.ampSpline.contour = flatAt3(0.25, CurveDomain::Unipolar); + + SampleData base = s; + LiveContourBlock blockA, blockB; + const std::vector moved = renderHeld(s, blockA, &edited, 4, 12); + const std::vector still = renderHeld(base, blockB, nullptr, -1, 12); + const std::size_t swapAt = 4 * kBlock; + + CHECK(moved[swapAt - 1] == 1.0f); + // Absorbed: the first frame read through the new contour reproduces the old level exactly... + CHECK(moved[swapAt] == 1.0f); + // ...and glides from there. An unabsorbed swap would step by 0.75 in one frame. + CHECK(maxAbsDelta(moved, swapAt - 1, swapAt + 64) < 0.05); + CHECK(moved[swapAt + 32] < 0.9f); + // The held note lands on the edited contour, where the unedited run stays on the old one. + CHECK(std::fabs(moved.back() - 0.25) < 1e-6); + CHECK(still.back() == 1.0f); +} + +// The processor marshals note-ons BEFORE the block renders, so a publish and a note-on landing +// in one block reach the engine through startVoice first. The voice already sounding must be +// rebound there too: left on the buffer the read just displaced, it would never see this edit. +static void testANoteOnInTheSwapBlockRebindsTheSoundingVoiceAndSnapsTheNewOne() { + SampleData s = dcSample(200000); + s.play.ampSpline.mode = EnvMode::Spline; + s.play.ampSpline.contour = flatAt(1.0, CurveDomain::Unipolar); + LiveContourBlock block; + s.liveContours = █ + block.publish(foldContours(s.play)); + + VoiceEngine engine(2, s); + engine.noteOn(kRoot, 127); + std::vector out; + for (int b = 0; b < 4; ++b) engine.render(out, kBlock); + + PlayParams edited = s.play; + edited.ampSpline.contour = flatAt3(0.25, CurveDomain::Unipolar); + block.publish(foldContours(edited)); + engine.noteOn(kRoot, 127); // the second voice, struck in the swap block + for (int b = 0; b < 8; ++b) engine.render(out, kBlock); + + const std::size_t swapAt = 4 * kBlock; + // The sounding voice's absorbed 1.0 plus the fresh voice's snapped 0.25 — the fresh one + // takes the edit outright even though the snapshot still carries the old contour. + CHECK(std::fabs(out[swapAt] - 1.25) < 1e-6); + // Both land on the edit: 0.25 + 0.25. A sounding voice that missed it would leave 1.25. + CHECK(std::fabs(out.back() - 0.5) < 1e-6); +} + +static void testANoteStruckAfterAnEditPlaysItFromItsFirstFrame() { + SampleData s = dcSample(200000); + s.play.ampSpline.mode = EnvMode::Spline; + s.play.ampSpline.contour = flatAt(1.0, CurveDomain::Unipolar); + LiveContourBlock block; + s.liveContours = █ + block.publish(foldContours(s.play)); + VoiceEngine engine(1, s); + + PlayParams edited = s.play; + edited.ampSpline.contour = flatAt3(0.25, CurveDomain::Unipolar); + block.publish(foldContours(edited)); + engine.noteOn(kRoot, 127); + std::vector out; + engine.render(out, kBlock); + CHECK(out[0] == 0.25f); + + // The snapshot really does still say 1.0: with no block the same engine plays it. + SampleData bare = s; + bare.liveContours = nullptr; + VoiceEngine bareEngine(1, bare); + bareEngine.noteOn(kRoot, 127); + std::vector bareOut; + bareEngine.render(bareOut, kBlock); + CHECK(bareOut[0] == 1.0f); +} + +// --- Pitch --------------------------------------------------------------------------------- + +// A ramp source under Varispeed: an output frame is the read head, so the frame-to-frame step +// IS the pitch ratio. +static void testAHeldNoteFollowsAPitchContourEdit() { + SampleData s; + s.frames.resize(1u << 15); + for (std::size_t i = 0; i < s.frames.size(); ++i) s.frames[i] = static_cast(i); + s.sampleRate = kRate; + s.rootNote = kRoot; + s.play.playMode = PlayMode::Trigger; + s.play.pitchEnv.enabled = true; + s.play.pitchEnv.peakSemitones = 12.0; + s.play.pitchSpline.mode = EnvMode::Spline; + s.play.pitchSpline.contour = flatAt(0.0, CurveDomain::Bipolar); + PlayParams edited = s.play; + edited.pitchSpline.contour = flatAt3(1.0, CurveDomain::Bipolar); // +1 x 12 st = an octave + + SampleData base = s; + LiveContourBlock blockA, blockB; + const std::vector moved = renderHeld(s, blockA, &edited, 4, 12); + const std::vector still = renderHeld(base, blockB, nullptr, -1, 12); + const std::size_t swapAt = 4 * kBlock; + const std::size_t n = moved.size(); + + CHECK(std::fabs((moved[swapAt - 1] - moved[swapAt - 2]) - 1.0) < 1e-3); + CHECK(std::fabs((moved[n - 1] - moved[n - 2]) - 2.0) < 1e-3); + CHECK(std::fabs((still[n - 1] - still[n - 2]) - 1.0) < 1e-3); + // Absorbed here too: the first frame read through the new contour still advances at the old + // ratio, where an unabsorbed swap would already step by 2. + CHECK(std::fabs((moved[swapAt + 1] - moved[swapAt]) - 1.0) < 1e-3); +} + +// --- Filter -------------------------------------------------------------------------------- + +static void testAHeldNoteFollowsAFilterContourEdit() { + SampleData s; + s.frames.resize(100000); + for (std::size_t i = 0; i < s.frames.size(); ++i) { + s.frames[i] = static_cast(std::sin(2.0 * kPi * static_cast(i) / 8.0)); + } + s.sampleRate = kRate; + s.rootNote = kRoot; + s.play.playMode = PlayMode::Trigger; + s.play.filter.enabled = true; + s.play.filter.settings.cutoffNorm = 0.1f; + s.play.filter.settings.morphNorm = 1.0f; // low-pass + s.play.filter.modAmount = 0.8; + s.play.filterSpline.mode = EnvMode::Spline; + s.play.filterSpline.contour = flatAt(0.0, CurveDomain::Unipolar); + PlayParams edited = s.play; + edited.filterSpline.contour = flatAt3(1.0, CurveDomain::Unipolar); // corner 0.1 -> 0.9 + + SampleData base = s; + LiveContourBlock blockA, blockB; + const std::vector moved = renderHeld(s, blockA, &edited, 4, 16); + const std::vector still = renderHeld(base, blockB, nullptr, -1, 16); + const std::size_t tail = moved.size() - 1024; + + const double openedPeak = peakOf(moved, tail, moved.size()); + const double closedPeak = peakOf(still, tail, still.size()); + // A 6 kHz tone through the corner the edit opened, against the unedited note's, which stays + // filtered away (measured ~0.18 vs ~0.0002 — the margins below are loose on purpose). + CHECK(openedPeak > 0.1); + CHECK(openedPeak > 10.0 * closedPeak); +} + +// --- What stays latched, and what must not move -------------------------------------------- + +// Staged|Spline is a note-on fact: an EG that start() left Staged ignores any contour publish. +static void testAStagedEnvelopeIgnoresContourPublishes() { + SampleData s = dcSample(200000); + // A staged shape that is not flat from frame 0, so a leak would show. + s.play.trigAhd.decayFrames = 20000; + s.play.trigAhd.holdFraction = 0.0; + PlayParams hostile = s.play; + hostile.ampSpline.contour = flatAt3(0.0, CurveDomain::Unipolar); + hostile.pitchSpline.contour = flatAt3(1.0, CurveDomain::Bipolar); + hostile.filterSpline.contour = flatAt3(1.0, CurveDomain::Unipolar); + + SampleData base = s; + LiveContourBlock blockA, blockB; + const std::vector published = renderHeld(s, blockA, &hostile, 3, 10); + const std::vector none = renderHeld(base, blockB, nullptr, -1, 10); + CHECK(published == none); + CHECK(none[3 * kBlock + 100] < 1.0f); // the staged decay is really running +} + +// Republishing the contour a voice is already reading rebinds it mid-read; the read must not +// move by a bit. A curved, multi-segment contour with a hard point so the re-located segment +// and its tangents are genuinely exercised. +static void testRepublishingAnUnchangedContourIsByteIdentical() { + SampleData s = dcSample(20000); + s.play.ampSpline.mode = EnvMode::Spline; + VelocityCurve c = VelocityCurve::fromPoints( + {{0.0, 0.0}, {20.0, 1.0}, {60.0, 0.3}, {100.0, 0.9}, {127.0, 0.0}}, + CurveDomain::Unipolar); + c.setHard(2, true); + s.play.ampSpline.contour = c; + const PlayParams same = s.play; + + SampleData base = s; + LiveContourBlock blockA, blockB; + const std::vector republished = renderHeld(s, blockA, &same, 20, 60); + const std::vector none = renderHeld(base, blockB, nullptr, -1, 60); + CHECK(republished == none); + CHECK(peakOf(none, 0, none.size()) > 0.9); // the contour really is being read +} + +// The drain slot: a displaced snapshot and the live one both observe the instance's ONE block, +// so a note ringing out of either follows the edit. +static void testTwoEnginesSharingOneBlockBothFollow() { + SampleData a = dcSample(200000); + a.play.ampSpline.mode = EnvMode::Spline; + a.play.ampSpline.contour = flatAt(1.0, CurveDomain::Unipolar); + SampleData b = a; + LiveContourBlock block; + a.liveContours = █ + b.liveContours = █ + block.publish(foldContours(a.play)); + + VoiceEngine live(1, a), drain(1, b); + live.noteOn(kRoot, 127); + drain.noteOn(kRoot, 127); + std::vector outLive, outDrain; + for (int i = 0; i < 2; ++i) { live.render(outLive, kBlock); drain.render(outDrain, kBlock); } + + PlayParams edited = a.play; + edited.ampSpline.contour = flatAt3(0.25, CurveDomain::Unipolar); + block.publish(foldContours(edited)); + for (int i = 0; i < 8; ++i) { live.render(outLive, kBlock); drain.render(outDrain, kBlock); } + CHECK(std::fabs(outLive.back() - 0.25) < 1e-6); + CHECK(std::fabs(outDrain.back() - 0.25) < 1e-6); +} + +// --- Allocation ---------------------------------------------------------------------------- + +// The counter itself must be live, or the zero below would prove nothing. +static void testTheAllocationCounterSeesAnAllocation() { + g_allocations.store(0); + g_countAllocations.store(true); + std::vector* v = new std::vector(16); + g_countAllocations.store(false); + delete v; + CHECK(g_allocations.load() >= 2); // the vector object and its buffer +} + +// Every audio-side step of a contour swap — the block-boundary read, the rebind of every +// sounding voice on all three EGs, a note struck in the swap block, and the renders around them +// — runs with the counter armed. Only the UI thread's publish is outside it (it allocates +// nothing either, but it is not the audio side). +static void testTheAudioSideOfAContourSwapAllocatesNothing() { + SampleData s = dcSample(200000); + s.play.ampSpline.mode = EnvMode::Spline; + s.play.ampSpline.contour = flatAt(1.0, CurveDomain::Unipolar); + s.play.pitchEnv.enabled = true; + s.play.pitchEnv.peakSemitones = 0.0; // bound, so rebound, but transposing nothing + s.play.pitchSpline.mode = EnvMode::Spline; + s.play.pitchSpline.contour = flatAt(0.0, CurveDomain::Bipolar); + s.play.filter.enabled = true; + s.play.filter.settings.cutoffNorm = 1.0f; + s.play.filter.modAmount = 0.0; + s.play.filterSpline.mode = EnvMode::Spline; + s.play.filterSpline.contour = flatAt(0.0, CurveDomain::Unipolar); + PlayParams edited = s.play; + edited.ampSpline.contour = flatAt3(0.25, CurveDomain::Unipolar); + edited.pitchSpline.contour = flatAt3(0.0, CurveDomain::Bipolar); + edited.filterSpline.contour = flatAt3(0.5, CurveDomain::Unipolar); + + LiveContourBlock block; + s.liveContours = █ + block.publish(foldContours(s.play)); + VoiceEngine engine(4, s); + std::vector buf(kBlock, 0.0f); + const auto renderBlock = [&] { + std::fill(buf.begin(), buf.end(), 0.0f); + engine.render(buf.data(), kBlock); + }; + + g_allocations.store(0); + g_countAllocations.store(true); + engine.noteOn(kRoot, 127); + for (int b = 0; b < 4; ++b) renderBlock(); + g_countAllocations.store(false); + block.publish(foldContours(edited)); // UI thread + g_countAllocations.store(true); + engine.noteOn(kRoot, 127); // startVoice's rebind + snap + renderBlock(); // the block boundary's read finds nothing newer: already taken + g_countAllocations.store(false); + const double afterEdit = buf[kBlock - 1]; + block.publish(foldContours(s.play)); // and back, so the render path does the read itself + g_countAllocations.store(true); + for (int b = 0; b < 8; ++b) renderBlock(); + g_countAllocations.store(false); + + CHECK(g_allocations.load() == 0); + // Not vacuous: both swaps really landed inside the counted window, through a wide-open + // low-pass whose DC gain is unity. Two voices on the edited 0.25 contour by the end of the + // swap block (2.0 had it not landed), then both back on 1.0 through the render path's read. + CHECK(std::fabs(afterEdit - 0.5) < 1e-3); + CHECK(std::fabs(buf[kBlock - 1] - 2.0) < 1e-3); +} + +int main() { + testTheFoldCarriesEveryPointAndItsHardFlag(); + testAHeldNoteFollowsAnAmpContourEditWithoutAStep(); + testANoteOnInTheSwapBlockRebindsTheSoundingVoiceAndSnapsTheNewOne(); + testANoteStruckAfterAnEditPlaysItFromItsFirstFrame(); + testAHeldNoteFollowsAPitchContourEdit(); + testAHeldNoteFollowsAFilterContourEdit(); + testAStagedEnvelopeIgnoresContourPublishes(); + testRepublishingAnUnchangedContourIsByteIdentical(); + testTwoEnginesSharingOneBlockBothFollow(); + testTheAllocationCounterSeesAnAllocation(); + testTheAudioSideOfAContourSwapAllocatesNothing(); + if (g_fail == 0) std::printf("live_contours tests passed\n"); + return g_fail == 0 ? 0 : 1; +} From 5151cb41dcd6216579e82ef41574b9eecb37b903 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:45:33 -0400 Subject: [PATCH 04/12] Docs: add SEPT_VERIFY.md, the DAW checklist for Aug-Sept work --- docs/SEPT_VERIFY.md | 102 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 docs/SEPT_VERIFY.md diff --git a/docs/SEPT_VERIFY.md b/docs/SEPT_VERIFY.md new file mode 100644 index 0000000..127b54e --- /dev/null +++ b/docs/SEPT_VERIFY.md @@ -0,0 +1,102 @@ +# September DAW verification — new work not yet hand-tested + +Checks for work landed or in progress since the last `VERIFICATION.md` pass that no unit test can +close. Build **Release**, install into `UserPlugins/`, restart REAPER. Panel tail toggle = **None**, +project rate 48000, unless a check says otherwise. + +## 1. Instrument control-surface pass (landed Aug 3, 2026) + +- [ ] Play notes and watch the ReaSampler 9000 editor's master meter — it redraws smoothly (~60 fps) +- [ ] Load a heavy project (several other plugins, high track count) and watch the meter again under + that load — still smooth, no visible stutter +- [ ] Click the single-button "Envelope" toggle on a deck — cycles on/off cleanly; confirm its + disabled-state look when the deck it governs isn't applicable +- [ ] Same check for the "Filter" toggle +- [ ] Same check for the "Limiter" toggle +- [ ] Same check for the "Stage"/"Spline" toggle (the envelope mode selector — one button reading + either "Stage" or "Spline") +- [ ] Click anywhere on an envelope deck's control section (not just a knob) — that envelope becomes + selected for editing on the waveform overlay +- [ ] Switch the instrument to Trigger mode — the loop markers and their caption disappear from the + waveform entirely +- [ ] Switch to Gate mode with looping off — the loop markers still show, greyed out, and are still + grabbable +- [ ] Zoom in on a loop marker and drag it near a zero crossing — it snaps only within a small pixel + radius of the crossing, not from anywhere on the waveform +- [ ] Hold Ctrl while dragging a marker near a zero crossing — snapping is disabled for that drag + +## 2. Bank panel additions (Aug 3) + +- [ ] With a track selected and a bank card focused, click "Insert as FX" — ReaSampler 9000 loads on + that track with the focused sample already loaded +- [ ] Open the panel's top-toolbar "⋯" overflow menu — Re-capture and Insert Conform are both there + and both work +- [ ] Drag a bank card over the arrange view — the cursor shows a crosshair cue while over a valid + arrange-insert target + +## 3. Design View FX parking (Aug 21) + +- [ ] Trigger a mode switch that parks/restores FX on a heavy chain, and watch the docked bank + panel while the reclaimer drains: the cursor goes busy over the panel and the footer reads + "parking FX" / "restoring FX" appropriately. If the cursor never changes, note it — REAPER may not + route `WM_SETCURSOR` to the docked panel at all (`src/shell/panel/CLAUDE.md` Gotchas) +- [ ] Native drag-reorder a parked track's FX chain, switch modes, and confirm each plugin's state + restores to the SAME plugin it came from (not swapped by slot) +- [ ] Do the same reorder via an SWS move command +- [ ] Save the project after a reorder, reopen, then switch modes — restore still keys to the right + plugin +- [ ] Reorder a chain holding two copies of the same plugin — each copy's state restores to itself, + not swapped with its twin + (`docs/TODO.md` §"FX-GUID stability for the park's restore is unverified in the DAW") +- [ ] For mode-switching itself (solo-set restore, transport refusal, persistence, undo point count, + etc.) — see `docs/VERIFICATION.md` §"Mode switching"; not repeated here + +## 4. Drag a card into a plugin's editor (in progress, Sept 2026) + +- [ ] Drop a bank card on Serum 2's oscillator in its floating FX editor window — Serum loads the + sample +- [ ] Drop a bank card on Serum 2's oscillator from the FX chain window — same, Serum loads the + sample +- [ ] Drop a bank card on the FX chain list or window chrome (not the oscillator) — ReaSampler 9000 + loads as before, unchanged +- [ ] Drop a bank card on the TCP, MCP, or arrange — unchanged from current behavior +- [ ] Start a drag over a plugin editor, then move back over the arrange mid-drag — the drag reverts + to the panel's own drop behavior (may only visibly update on the next key press or mouse-button + change) +- [ ] Drag a card out to Windows Explorer and back into REAPER — control hands back to the panel's + own drag correctly +- [ ] Drop a card onto an already-open ReaSampler 9000 editor — it shows its "drop on the panel" + banner and does NOT spawn a second instance +- [ ] Drop a card onto ReaEQ's editor — shows a no-drop cursor, nothing loads + +## 5. Live spline-envelope drags (in progress, Sept 2026) + +- [ ] Set the amp envelope to Spline, hold a note, and drag a knot — the sound audibly follows the + shape change during the drag itself +- [ ] Same check for the pitch envelope +- [ ] Same check for the filter envelope +- [ ] After dragging a knot, trigger a new note — it uses the dragged shape +- [ ] Listen closely during a fast drag for any audible glitch beyond what an ordinary knob move + would cause (clicks, zippering) +- [ ] Drag a knot on a held note and listen for an audible pitch/amp/filter glide as it moves, not a + stepped jump +- [ ] Release a dragged knot — commits exactly as before: one undo point, and reopening the project + keeps the dragged shape +- [ ] With a note held, add a new point to the spline — audible immediately +- [ ] With a note held, delete a point from the spline — audible immediately + +## 6. Sample-rate playback (in progress, Sept 2026) + +- [ ] In a 48 kHz project, import a 44.1 kHz file via the Media Explorer into ReaSampler 9000; play + its root note — same pitch and length as the file auditioned in the Media Explorer. Repeat in + Preserve mode. +- [ ] Same with a 96 kHz file in a 44.1 kHz project +- [ ] Envelope attack times, loop (a single-cycle loop holds its pitch; loop crossfade length) and + filter cutoff sound the same as on a same-rate copy of the file +- [ ] Bake a mismatched-rate sample — the baked result has the same pitch and length as what you + heard +- [ ] Change the project sample rate with ReaSampler 9000 loaded (and with it inactive/bypassed, + then re-enabled) — the sample still plays at correct pitch afterwards +- [ ] Existing projects that used mismatched-rate samples WILL sound different now (correct pitch) + — reopen one and confirm it's the expected change +- [ ] Same-rate samples sound exactly as before From 550a0a86e4d38b4846c522c958d116b097f110c5 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:46:10 -0400 Subject: [PATCH 05/12] Docs: record Phase Omega's landed work in COMPLETED.md, trim it from PLAN.md --- docs/COMPLETED.md | 50 +++++ docs/PLAN.md | 545 +++------------------------------------------- 2 files changed, 80 insertions(+), 515 deletions(-) diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index 8d3c3f1..739c617 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -1661,6 +1661,56 @@ v2 blob keeps the behaviour it had rather than losing every FX state. `TrackFX_GetFXGUID` survives a chain reorder while parked — is unverified in the DAW (SWS issue #802 names a specific way it might not hold). +### Phase Ω — Responsiveness, and the control surface's second pass + +Ω-W1's five tracks landed together, merged to `dev` as `1159d36` (2026-08-03): + +- **Ω-W1-T1 `mode-switch-responsiveness`.** Narrowed the mode-switch undo mask off blanket + `UNDO_STATE_ALL`, bracketed the flag-write phase in a `PreventUIRefresh` RAII pair + (`UiRefreshHold`), and replaced the linear GUID scan with a hash map (`TrackByGuid`, an + `unordered_map`). Also shipped a deferred FX-park queue splitting the synchronous mode-switch + apply from per-FX offline/online. A follow-on split of that drain by kind, **Ω-W2-T4** + (`70bd29d`), was **reverted** (`2f96dd3`): FX-park state was left stranded across save/reopen, + so forced synchronous FX-park drains returned — and with them the mode-switch hitch — until a + re-snapshot-after-discard fix lands. +- **Ω-W1-T2 `mode-switch-persist`.** A mode switch now writes only the `view_state` ext-state + key (`saveViewStateOnly`) instead of the full multi-key session save on every switch. +- **Ω-W1-T3 `meter-rate`.** A second timer (`kMeterTimerIntervalMs = 16` ms, ~60 FPS) drains the + master meter independently of the 500 ms bank/bake poll (`kSyncTimerIntervalMs`). +- **Ω-W1-T4 `deck-reflow-focus-toggles`.** Moved Filter Mod onto the Filter Envelope deck, + centred Trigger's reserved knob-cell runs, converted the two-segment toggles (the + "Envelope"/"Filter"/"Limiter" enables and chrome "Loop") to single buttons, and made clicking + anywhere on an envelope deck focus its overlay. +- **Ω-W1-T5 `overlay-mapping`.** Unified the marker/overlay coordinate mapping with the + waveform draw chain around one `frameToX`/`OverlayArea`; Trigger now hides the loop marks and + caption outright, and Gate-with-loop-off draws them grey-disabled and still grabbable. + +Two Ω-W2 tracks landed the same day. **Ω-W2-T2 `content-detect-throttle`** (`84afd1b`) +throttled the panel's new-content enumeration to `kDetectIntervalMs = 500` ms, down from the +~30 Hz timer tick it rode before. **Ω-W2-T5 `snap-radius`** (`aa87d81`) bounded +`nearestZeroCrossing`'s fan-out to `kMarkerGrabWidth` (5 px) and added Ctrl-defeats-snap on a +marker drag; `docs/product/instrument-control-surface.md` §4.1 now documents the gesture. + +**Also landed alongside Ω (2026-08-03), outside the plan:** +- `4b19556` **Insert as FX** — a panel button + bindable action loading ReaSampler 9000 onto + the selected track with the focused capture. +- `098727e` **Panel polish** — a crosshair arrange-drop cue, and Re-capture / Insert Conform + moved to the "⋯" overflow menu. +- `a5febbd` **Drag-out hand-off fix** — the card drag stays internal while the pointer is over + REAPER. + +**Still open, in `docs/PLAN.md`:** Ω-W2-T1 `view-topology-cache` (measurement-gated on +Ω-W1-T1's landed result; never dispatched) and Ω-W2-T3 `omega-verification-sweep` (the DAW +verification record for Ω-W1; not run). + +**Superseded history.** Ω-W1-T1's deferred FX-park queue, a follow-up snapshot guard +(`bd86ede`, 2026-08-05, "refuse to park when the pre-park truth is gone"), and an undo-collapse +fix (`03f7fb5`, 2026-08-06) all landed against the synchronous FX-park approach the Aug 3 +revert left in place. **All of it was superseded on 2026-08-21** by the Design View +mode-switch redesign below, which moved per-FX offline/online off the switch path a second +time — onto a background reclaimer — and collapsed the undo footprint through the new +`src/shell/undo/` module. + ### Design View mode-switch redesign — FX off the switch path, one undo point Four waves, merged to `dev` as `da84012`, that took the per-FX offline/online work diff --git a/docs/PLAN.md b/docs/PLAN.md index f28d9c1..caad96f 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -392,11 +392,19 @@ cross-reference. | **Ω-F5** | **The mode-switch symptom is both entry surfaces**, and it is *"nothing for 2 seconds, then everything switches"* — nothing renders until the whole synchronous body completes | Ω-W1-T1, Ω-W1-T2 | | **Ω-F6** | **ACCEPTED, and it is a spec amendment: separate the FX parking's UI half from its processing half.** Visibility / routing / FX-enable apply **synchronously**; per-FX offline/online **defers to a later idle tick** — *"the switch doesn't happen during playback anyway… the UI is the priority when toggling modes"* | Ω-W1-T1 | -**SUPERSEDED — Ω-F6 and every deferred-park instruction under Ω-W1-T1 below (the new park-queue TU, its `OnTimer` drain, its `target_sources` line).** The deferral cost a SECOND undo point and could not be made to cost one: REAPER's undo surface has no append-to-point and no block held across a return to the message loop, so per-FX offline/online now runs inline in `applyMode`'s own block and the queue is deleted. The hitch is back on the switch's synchronous path, deliberately. The rest of Ω-W1-T1 (the honest undo mask, compare-before-write, `PreventUIRefresh`, the `resolve` hash map) stands as written. Two of that track's acceptance criteria below are superseded with it: "paints its new state before the park work runs" assumed the split that no longer exists — a switch now parks synchronously before anything paints — and "two rapid switches leave every track in the state the SECOND switch specifies" assumed a queue that could apply both — the synchronous latch REFUSES a switch fired while another is already applying, so a rapid second switch is dropped, not applied. - -**Verified by Daniel, not a fork:** the deck knob-spacing defect is in the **Trigger** face. -**Gate is correct and must stay pixel-identical** — an acceptance criterion of Ω-W1-T4, not a -hope. +**Doubly superseded — read this as history, not as current behaviour.** Ω-F6's deferred +FX-park queue (the park-queue TU, its `OnTimer` drain, its `target_sources` line) shipped +with Ω-W1-T1 on 2026-08-03. A follow-on split of that drain by kind, Ω-W2-T4 (`70bd29d`), +was reverted (`2f96dd3`): FX-park state was left stranded across save/reopen, so forced +synchronous FX-park drains returned — per-FX offline/online back to running inline in +`applyMode`'s own block — and with them the mode-switch hitch, until a +re-snapshot-after-discard fix lands. A snapshot guard (`bd86ede`, 2026-08-05) and an +undo-collapse fix (`03f7fb5`, 2026-08-06) then landed +against that synchronous approach. **All of it was superseded again on 2026-08-21** by the +Design View mode-switch redesign (`docs/COMPLETED.md`), which moved per-FX offline/online +off the switch path a second time — this time onto a background reclaimer — and collapsed +the switch's undo footprint through the new `src/shell/undo/` module. Nothing in this +paragraph describes current behaviour; see `docs/COMPLETED.md` for what actually ships. ### Phase-Ω acceptance criteria @@ -433,458 +441,11 @@ These bind every track in this phase, in addition to the plan-wide set above. ### Ω-W1 — The five defects, in parallel -**Depends on:** nothing. **Five tracks, disjoint at the file level.** Items 2, 3 and 4 are ONE -track on purpose: they land in the same deck modules and the same pinned-geometry test files, -so splitting them means re-deriving `test_deck_groups_measured.cpp` three times. Items 5 and 6 -are one track because they overlap inside `paintWaveform`'s mark-draw loop — one deletes and -gates it, the other changes the `mx` it computes. - -| Track | Owns | -|---|---| -| **T1** `mode-switch-responsiveness` | `shell/view/view.{h,cpp}` + one new deferred-park TU + its drain in `app/main.cpp` | -| **T2** `mode-switch-persist` | `shell/actions/design_view_actions.cpp` + `shell/persist/ext_state_io.cpp` — dirty-key gating | -| **T3** `meter-rate` | the editor's timer/paint path + the hero-envelope cache | -| **T4** `deck-reflow-focus-toggles` | Ω.2 + Ω.3 + Ω.4 — the deck's composition, layout, paint, input and pinned tests | -| **T5** `overlay-mapping` | Ω.5 + Ω.6 — the waveform overlay's marks and its one coordinate mapping | - -**Shared files in the wave, named rather than discovered at merge.** All textual adjacency -unless marked otherwise. - -| File | Tracks | Nature | -|---|---|---| -| `src/shell/instrument/reasampler_editor.h` | T3 (meter clock + timer members), T4 (deck focus state), T5 (nothing new expected) | **The one to watch.** Additive member declarations in different sections of a header with ~36 lines of ceiling margin between them. Whichever lands second rebases AND re-checks the line count | -| `src/shell/instrument/editor_paint_waveform.cpp` | T3 (the envelope-cache call site at `paintWaveform`'s bin/`computeEnvelope` block), T5 (the mark, caption and mapping regions below it) | Disjoint regions of one function's body; whichever lands second rebases | -| `src/shell/instrument/editor_controls.cpp` | T4 (`faceLayout`'s `deckDescs` build), T5 (`waveMarksFor` / `grabbableMarks`) | Different functions, no shared state | -| `src/app/CMakeLists.txt` | T1 (one `target_sources` line for the new park-queue TU) | Ω's only CMake edit. **Cross-phase adjacency:** Λ-W2-T2, Λ-W2-T3, Λ-W4-T3 and Λ-W5-T1 all own regions of this file. Ω lands first; Λ rebases | -| `src/app/main.cpp` | T1 (the drain call inside `OnTimer`) | **Cross-phase adjacency:** Λ-W2-T1 owns this file's `REAPERAPI_LoadAPI` failure branch — a different region | - -#### Ω-W1-T1 — `mode-switch-responsiveness` - -**Goal.** A mode switch redraws immediately. The expensive half of the switch still happens; it -just stops being the thing the user waits for. - -**Spec:** Ω.7 and rulings Ω-F5, Ω-F6 above; no product doc. - -**Surface boundary — owns:** `src/shell/view/view.cpp` (the `applyMode` body — the undo block, -the park/restore loops, the parent-visibility loop, `resolve`), `src/shell/view/view.h`, ONE -new TU pair beside them for the deferred queue, one `target_sources` line in -`src/app/CMakeLists.txt`, the drain call in `src/app/main.cpp`'s `OnTimer`, and -`src/shell/view/CLAUDE.md`'s FX-parking caveat. **Does not own:** `core/view` (that is -Ω-W2-T1's), `design_view_actions.cpp` or `ext_state_io.cpp` (T2's), `panel_input.cpp` -(Ω-W2-T2's), or `view_solo.cpp`. - -**The solo surface index is EXONERATED.** It costs ~2T extra API calls, sub-millisecond. Say so -in the brief so it is not re-litigated: the magnitude is two older O(project) costs that are -invisible on a scratch project and quadratic-feeling on a real one. - -**Behavior.** -- **The undo mask (`view.cpp:563`).** `Undo_EndBlock2(proj, undoLabel.c_str(), -1)` — `-1` is - `UNDO_STATE_ALL` (`reaper_plugin.h:1540`), which includes `UNDO_STATE_FX` (`:1542`). REAPER - marshals **every plugin's state chunk in the project** into the undo record, on every switch - **and** every tag/untag reapply. The honest mask is - `UNDO_STATE_TRACKCFG|UNDO_STATE_ITEMS|UNDO_STATE_MISCCFG`, with `UNDO_STATE_FX` OR'd in only - when the park/restore loop actually toggled an offline state this apply. The second - `-1` at `:607` (`mintManagedLanes`) is a separate call site — assess it, do not assume it. -- **Unconditional `TrackFX_SetOffline`.** `parkFxOffline` (`:190-195`) calls it on **every FX of - every parked track**, including tracks already parked and already offline — unloading and - re-instantiating plugins for no state change. `restoreFxOffline` (`:200-204`) writes verbatim - from the snapshot the same way. Both compare before writing. -- **`PreventUIRefresh` is used NOWHERE in `src/`** (verified: zero matches across the tree) - though the SDK exports it (`reaper_plugin_functions.h:5583`). ~10T flag writes land with UI - refresh live. Bracket the write phase, and keep the existing - `TrackList_AdjustWindows(false)` + `UpdateArrange()` as the one deliberate rebuild after the - bracket closes. **Balanced pairing is mandatory** — an early return between the two calls - leaves REAPER's UI frozen. -- **Compare before write.** `applyFlags` (`:182-186`) and the parent-visibility loop - (`:521-531`) write unconditionally, so a reapply that changed nothing still pays every write. -- **`resolve` is a linear scan** (`:122-128`) called once per parked / restored / parent / lane - track — O(T²). `readFolderEntries` (`:103-120`) already builds `handleByGuid` and can hand - back a hash map instead of a vector of pairs. -- **Ω-F6, and it is the headline.** Split the synchronous apply from the deferred park: - visibility, routing and FX-enable apply **inside** `applyMode`; per-FX offline/online is - enqueued and drained on a later idle tick. **The spec must state what the queue does when a - second switch arrives before the first's park has run** — the answer is that a queue entry is - keyed by track GUID and the LATEST intent wins (an enqueued park superseded by a restore - cancels, not stacks), because the two are inverses and replaying both is both slower and - observably wrong. A project close, a track deletion, or a project switch drains the queue - without applying it. -- **Where the queue lives.** `src/shell/view/view.cpp` measures **642 lines** today and is - already over the ~600 ceiling with its seam blocked (`docs/TODO.md`). The queue is therefore - a **new TU beside it**, not more of `view.cpp` — `src/shell/view/` has no `CMakeLists.txt` of - its own, so this costs exactly one `target_sources` line in `src/app/CMakeLists.txt` (where - `view.cpp` and `view_solo.cpp` are already listed). This is the one structural decision in - the track and it moves `view.cpp` toward the ceiling rather than further past it. -- **The invariant amendment.** `src/shell/view/CLAUDE.md`'s "Documented caveat" bullet accepts a - load hitch on the return to the active mode. After this track the hitch is still real but no - longer sits on the switch's synchronous path — the caveat must say so, and must say what the - deferral does NOT change (the plugin still re-instantiates; un-persisted internal state is - still lost). - -**Acceptance criteria.** -- On a project with enough tracks and FX to reproduce it, the mode switch **paints its new - state before the park work runs**, and the wall-clock to first paint is recorded before and - after. `[verify — DAW]`, discharged by Ω-W2-T3. -- A tag/untag reapply on an unchanged project performs **zero** `TrackFX_SetOffline` calls and - **zero** redundant flag writes. -- The undo record for one mode switch no longer carries FX state when no offline state changed; - **one switch is still one Ctrl-Z**, and Ctrl-Z still restores what it restored before. -- The park/restore round trip — the phase's trust anchor — still returns every driven flag to - its captured value, including when the deferral means the park lands on a later tick. -- Two switches in rapid succession leave every track in the state the SECOND switch specifies, - with no track left offline that should be online. -- `PreventUIRefresh` is balanced on every exit path from `applyMode`, including the two early - returns already there. -- `view.cpp` is no longer than it is today. - -**Prerequisites.** None; concurrent with T2–T5. **Discharges:** Ω.7 (the structural half), -Ω-F5, Ω-F6. - -#### Ω-W1-T2 — `mode-switch-persist` - -**Goal.** A mode switch stops rewriting the whole session to record that one key changed. - -**Spec:** Ω.7's lower-priority half; ruling Ω-F5 (both entry surfaces). - -**Surface boundary — owns:** `src/shell/actions/design_view_actions.cpp` (`persistViewState` and -the two callers `doActivateMode` / `doToggleMode`) and `src/shell/persist/ext_state_io.cpp` -(`saveToActiveProject`). **Does not own:** `view.cpp`, `session.cpp`, or any panel TU. - -**Behavior.** -- The panel-click path routes through `Main_OnCommand` → `doActivateMode` - (`panel_input.cpp:330-334`), which calls `persistViewState()` → - `saveToActiveProject()` — a **seven-key** ext-state write (`ext_state_io.cpp:140-190`: banks, - the legacy-key clear, view, tail, owned_files, version stamp, bank generation) plus a - `MarkProjectDirty`, every switch, when only `view_state` changed. Serializing the bank book - and the tracking ledger is the expensive part and neither moved. -- **Gate by dirty key, not by call site.** A per-key dirty flag on the session, or a - `saveViewStateOnly` entry point beside the full save — the track picks, and states why. The - full save must remain the default for every other caller. -- **`saveToActiveProject`'s return contract must not change.** `shell/persist/CLAUDE.md` is - explicit: `false` means *no active/saved project and NOTHING was written*, and it may never - grow an observational third failure mode. A narrowed write returns on the same rule. -- The degraded-ledger suppression (`tracking::ledgerDegraded`) and the version stamp both stay - exactly where they are on the full path. - -**Acceptance criteria.** -- A mode switch on a saved project writes `view_state` and nothing else; a capture, a bank op - and a project save each still write everything they wrote before. -- `MarkProjectDirty` still fires on the narrowed path — the state DID change. -- Both entry surfaces (the footer segment and the bindable action) take the same path. -- The write-count difference is measured and recorded. - -**Prerequisites.** None; concurrent with T1. **Discharges:** Ω.7 (the persist half). - -#### Ω-W1-T3 — `meter-rate` - -**Goal.** The master meter moves at 60 FPS, and a paint costs what one frame should. - -**Spec:** Ω.1; no product doc. `src/shell/instrument/CLAUDE.md` already records that "a -meter-rate timer remains a separate change and is not in" — this is that change. - -**Surface boundary — owns:** `src/shell/instrument/editor_platform.cpp` (the timer set-up and -the `WM_PAINT`/`invalidate` path), `editor_session.cpp` (the meter block currently inside -`onSyncTimer`), `editor_paint.cpp` (the paint entry's bitmap and dirty-rect handling), the -hero-envelope cache's accessor in `editor_paint_waveform.cpp`, and `reasampler_editor.h`'s -meter/timer/cache members. **Does not own:** `meter_ballistics.h`, `master_meter`, the -processor, any deck or overlay module. - -**Ballistics need no rework.** Every constant in `core/instrument/engine/meter_ballistics.h` is -per-second, and elapsed time is measured rather than assumed — the rate is a UI-cadence -problem, not a ballistics one. **Nothing changes on the audio thread.** - -**Behavior.** -- **The cause.** There is exactly ONE timer in the tree: `kSyncTimerIntervalMs = 500` - (`editor_platform.cpp:33`), the bank/bake change-detection poll. The meter's advance + - `invalidate` sit inside `onSyncTimer` (`editor_session.cpp:113-134`), so 2 FPS is the - ceiling, and `WM_TIMER` coalescing under host load takes it below that. -- **The recommended shape: a SECOND timer id on the same child window**, running only the meter - drain, advance and invalidate. **The 500 ms poll body is untouched** — `pollBankSync`, - `bakeAvailable`, `resolveBakeHoldNeeded` and the bake run must not speed up (a bridge read - plus a bank parse each), and the tick-counted UI decays (`kBakeMessageTicks`, - `dropHintTicks_`) keep their meaning for free. **The rejected alternative** is raising the - shared interval and converting those decays to elapsed time: it is one fewer timer and it - silently shortens two user-visible banners by ~15× if the conversion is missed anywhere. -- **`masterBusMeter()` is a CONSUMING drain** (exchange-to-identity) and must keep **exactly - ONE caller**, or the two clocks steal windows from each other. Moving it to the meter timer - keeps that; leaving a copy behind in `onSyncTimer` breaks it. -- **The meter block must stay AHEAD of the in-flight-drag guard.** On its own timer this is - structural rather than positional — state it, because the reason (a drag suppresses the - reload poll but the bus keeps sounding) survives the move. -- **The clock resolution is a real hazard at 16 ms.** Today the elapsed measure is - `GetTickCount64` (`editor_session.cpp:114`), whose granularity is ~15.6 ms — at 60 FPS the - measured delta quantizes to 0 or 16 and a zero-delta advance does nothing but burn a frame. - Move the meter clock to a high-resolution source off the audio thread, and skip the advance - on a zero delta rather than passing it in. -- **Three paint costs, all of which the higher rate exposes:** - 1. `invalidate()` (`editor_platform.cpp:65-66`) dirties the **whole client area** - (`InvalidateRect(hwnd, nullptr, FALSE)`) and `paint` never reads `ps.rcPaint` - (`:149-155`). The meter needs its own rect invalidated and the paint needs to honour the - dirty rect. - 2. `paint` allocates a **full-size `LICE_SysBitmap` per paint** (`editor_paint.cpp:28`). - Hoist it to a member resized on `WM_SIZE`, where `thumbCache_.clear()` already lives. - 3. `paintWaveform` recomputes `computeEnvelope` over the **entire decoded PCM every paint** - (`editor_paint_waveform.cpp:204-212`). The decode is cached (`channelPcmFor` / - `monoPcmFor`); the hero envelope is not. Cache it keyed on sample id + bin count + lane - count, busted by `refreshFromBank` and by a resize — the same discipline `marksCache_` and - `thumbCache_` already use. -- **Target 60 FPS.** 30 is the fallback **only** if the paint-cost work cannot get there, and - choosing it requires the measurement that justifies it. - -**Acceptance criteria.** -- The meter visibly tracks transients at the shipped rate; the measured frame interval is - recorded, with the rate chosen and why. `[verify — DAW]`, discharged by Ω-W2-T3. -- A meter-only frame does **not** repaint the deck, chrome or waveform bands. -- No allocation in the steady-state paint path. -- With no editor open, nothing changed: the meter timer is bound to the child window exactly as - the sync timer is. -- The bake affordance, the Hold applicability, the bake message and the drop-hint banner all - behave exactly as before — same cadence, same durations. -- `masterBusMeter()` has exactly one call site. -- The first meter read is still DISCARDED (session history, not a window) and the clip latch is - still NOT discarded with it. - -**Prerequisites.** None; concurrent with T1, T2, T4, T5. **Discharges:** Ω.1. - -#### Ω-W1-T4 — `deck-reflow-focus-toggles` - -**Goal.** Filter Mod sits with the envelope it modulates, the Trigger face's knobs sit at their -natural pitch, a two-state toggle is one button, and clicking an envelope deck anywhere selects -its overlay. - -**Spec:** Ω.2, Ω.3, Ω.4 and rulings Ω-F3, Ω-F4. Deck composition law: -`src/core/instrument/ui/CLAUDE.md`'s `knob_deck` and `deck_groups` bullets. - -**Surface boundary — owns:** `src/core/instrument/ui/deck_groups.{h,cpp}`, -`knob_deck.{h,cpp}`, `deck_values.cpp` (the toggle commit seam only), -`src/core/instrument/ui/sample_chrome.{h,cpp}`, `src/shell/instrument/editor_paint_deck.cpp`, -`editor_input_deck.cpp`, `editor_paint_chrome.cpp`, `editor_input_chrome.cpp`, -`editor_input.cpp` (the focus-clear arm of the mouse-down dispatch only), `editor_controls.cpp` -(`faceLayout`'s `deckDescs` build only), and `tests/test_deck_groups.cpp`, -`tests/test_deck_groups_measured.cpp`, `tests/test_knob_deck.cpp`. **Does not own:** -`waveform_view`, `envelope_overlay`, `spline_edit`, or any `editor_*_waveform` TU (T5's), and -**no `param/` file whatsoever**. - -**Behavior — Ω.2, the move.** -- `sampleDeckGroups` (`deck_groups.cpp`) is the sole deck-composition authority. Move - `kFilterModAmt` out of FILTER's `cellIds` (`:62-68`) into FILTER ENV's (`:80-89`), **in both - faces**, landing **last in the run** — the `kPitchEnvDepth` precedent (`:50-53`). -- **NO VST3 parameter-id change.** `kFilterModAmt` keeps its frozen id (`kParamFilterModAmount` - = 1240) and its unit; ids are keyed on `DeckParam`, never on deck group. `param_id.h`'s - header comment says the blocks run "per deck group in SIGNAL-FLOW order" (`:34`) — that - becomes untrue for exactly one row. **Annotate it; never renumber.** The frozen-table rule is - the whole reason the annotation is the correct fix. -- **This move costs the filter tie-line, and that cost is stated rather than discovered.** - Today `test_deck_groups_measured.cpp` pins both rows' filter groups to one right edge (x=640 - block-relative) — the only reason the row block was widened 1020→1028. After the move the - Sound row loses 60 px and the Contour row gains 60, and with the two filter groups then equal - in width the tie is **unreachable at any block width** under the space-between law. The - alignment goes; the ruling stands. Re-derive that fixture from the new contract, record the - loss in it, and **do not change `kDeckRowBlockW` or the editor floor to chase it** — revisiting - the floor is a separate proposal, not this track's to make. - -**Behavior — Ω.2, the spacing.** -- The defect is a **layout-algorithm artifact, not a constant**. A group's cell run is a - reserved width, and a `-1` reserve donates its slot to the surviving cells - (`knob_deck.cpp:123-138`), so Trigger's FILTER ENV gets ~100 px cells and AMP ENV ~75 against - the standard 60. Gate is uniform at 60 because it has no reserves. -- **The law: cells stay at `kDeckCellW`; the run is CENTRED in the reserve.** It is the only - candidate satisfying both of Daniel's constraints — group width stays stable across a mode - flip (which is why the reserves exist) and knobs keep their natural pitch. Gate is then - pixel-identical by construction, because with no reserves the centred run and the divided run - are the same run. -- **The two rejected laws, recorded so they are not re-proposed:** *(a) drop the reserves and - let the group narrow in Trigger* — reflows every neighbour on a mode flip, which is the thing - the reserves were introduced to prevent; *(b) keep the division but cap the cell width* — - leaves an asymmetric residue at one end and re-introduces the dead-slot look the division law - replaced. -- A consequence worth stating: once the run is centred, **where a `-1` sits in `cellIds` stops - mattering** — only the count does. The reserves become pure width accounting. - -**Behavior — Ω.3, single buttons.** -- Today every deck toggle is structurally two segments (`DeckToggleDesc` `knob_deck.h:69-72`, - `placeToggle` `knob_deck.cpp:82-94`, `hitTestDeck`'s seg0/seg1 `:259-268`, `drawToggle` - `editor_paint_deck.cpp:117-137`). **Two variants replace it:** - - **Enable** — one button, label names what it controls, **`AccentPrimary` when active and - dim gray when off** (Ω-F3, Ω-F4). Applies to: the envelope enable (`kPitchEnvEnable`, - today `Off|On`) → **"Envelope"**; `kFilterEnable` → **"Filter"**; `kLimiterEnable` → - **"Limiter"** (it currently names nothing it controls); and the chrome row's - `Loop Off|Loop On` → **"Loop"**. - - **Mode selector** — one button whose **label reads the current mode**, with **no dim-gray - state** (Ω-F3). Applies to: the three `Stg|Spl` env-mode toggles → **"Stage"/"Spline"**, - and the chrome row's `Mono|Stereo`. - - **A third visual state survives and must not be collapsed into "off":** a control refused - by the mode (the chrome Loop enable in Trigger, `kFilterLaw` while the filter is off) draws - **Disabled and inert**. Off is live and clickable; Disabled is not. -- **Explicitly NOT converted, and that is a boundary rather than an oversight:** `kPlayMode` - (`Gate|Trigger`), `kPitchEngine` (`Varisp|Presrv`), `kVoiceMode` (`Poly|Mono`), `kFilterLaw` - (`Band|Notch`) and `kMonoTrigger` (`Retrig|Legato`) keep their two-segment form. Daniel named - a set; this track ships that set. The mode-selector variant generalizes to them the day he - asks. -- **Three load-bearing risks the implementation must answer:** - 1. **A single button carries no `segment`**, so the commit must derive the NEXT state from the - current one. `setDeckParam`'s `segment == 1` contract (`deck_values.cpp:135-162`) is the - seam; `nextOverlaySelection` (`deck_groups.cpp:298-302`) is the existing precedent for - computing a next discrete state purely. - 2. **`kLimiterEnable`'s split commit must survive VERBATIM** (`editor_input_deck.cpp:67-79`): - it commits the parameter set and calls `setLimiterEnabled`, which only ARMS the host's - `restartComponent(kLatencyChanged)` for the sync tick to deliver. Nothing on that path may - start calling into the host from inside a mouse handler. - 3. **MASTER has ZERO caption slack** — its caption row (46 + toggle + radio) and its knob row - (`kDeckCellW` + `kDeckColumnGap` + `kMeterColumnW`) both measure 130, which is exactly - `kDeckSpanningW` − 2·`kDeckGroupPadX`. **So the "Limiter" button must be ≤ 64 px** (what - the two 32 px segments occupy today). Wider and `kDeckSpanningW` grows, which pushes - `kEditorMinWidth` into the 82 px budget against `kEditorCeilingWidth`. **Every other group - absorbs a wider button free** — verified: each is knob-row-bound with 50+ px of caption - headroom. -- **`hitTestKnobFace` runs no toggle-precedence pass** (`knob_deck.h:217-224`) and is only - correct while no toggle rect overlaps a knob circle. Any button that grows must be re-checked - against that, or given the same precedence order. - -**Behavior — Ω.4, the deck as the overlay target.** -- Remove the three overlay radios from the env groups' descriptors. **`DeckRadioDesc` itself - must SURVIVE** — MASTER uses a passive one for the GR lamp (`deck_groups.cpp:140`), and the - `passive` skip in `hitTestDeck` (`:255-258`) is what keeps a readout from growing a gesture. -- Clicking anywhere in an envelope deck — panel background, knob, or button — focuses that - envelope's overlay. The focused deck takes an **`AccentPrimary`** border (Ω-F4). Focus clears - when the user clicks a control surface outside it; **the overlay itself is excluded from - stealing or clearing focus**, so editing the thing you selected does not deselect it. -- **Five design constraints, each of which breaks something if missed:** - 1. The focus write must land **before** the `hit.kind` switch in `mouseDownDeck` - (`editor_input_deck.cpp:31-37`) and must **not consume the click**, or every knob grab and - every toggle commit on a focused deck dies. - 2. **`DeckHit` carries no group id** (`knob_deck.h:190-195`). Either the shell scans - `dl.groups` for containment or `DeckHit` gains one — the latter is the smaller lie. - 3. **`hitTestDeck` returns a miss for a click on group padding** (`knob_deck.cpp:252-278`), - which is exactly the "deck panel background" Daniel wants to hit. Containment against - `g.box` is the test, not the hit kind. - 4. **A deck click SETS focus idempotently; it does not toggle to none.** Otherwise every knob - tweak on the focused deck unfocuses it. This retires `nextOverlaySelection`'s - re-click-clears branch (`deck_groups.cpp:301`) and replaces the radio-id map with a - group-id one. - 5. **Refuse focus changes while a drag is in flight**, and keep `overlayEnv_` transient view - state with **no persistence path** — it is what is drawn, not what is played. - -**Acceptance criteria.** -- **The Gate deck face is pixel-identical to today**, group boxes and cell rects both, asserted - in `test_deck_groups_measured.cpp` — except for FILTER and FILTER ENV, whose widths move by - exactly one cell in opposite directions. -- Trigger's FILTER ENV and AMP ENV cells measure `kDeckCellW`, centred in their reserves, and - each group's box width is **identical in both modes**. -- `kFilterModAmt`'s `ParamId` is unchanged; `exposedParams()` returns the same list in the same - order; no `param/` file is edited. -- Every converted control's commit produces the same audible result as the segment it replaced, - including the limiter's latency-restart arm. -- `kEditorMinWidth` and `kEditorCeilingWidth` are unchanged, and the 82 px budget is unspent. -- Clicking a knob, a button and the bare panel of an envelope deck all focus that overlay; the - first two still perform their own action; clicking the overlay does not change focus. -- The deck's pure fixtures cover the new composition, the centred run and the focus map — the - shell layer is not where any of the three is asserted. - -**Prerequisites.** None; concurrent with T1, T2, T3, T5. **Discharges:** Ω.2, Ω.3, Ω.4, Ω-F3, -Ω-F4. - -#### Ω-W1-T5 — `overlay-mapping` - -**Goal.** The loop marks say what the mode means, and every overlay that rides the waveform -lands on the frame it names. - -**Spec:** Ω.5, Ω.6 and rulings Ω-F1, Ω-F2; `docs/product/instrument-control-surface.md` -§6.3(d) and §6.4 **as amended by Ω-F2**. - -**Surface boundary — owns:** `src/core/instrument/ui/waveform_view.{h,cpp}`, -`src/shell/instrument/editor_paint_waveform.cpp` (the mark, caption and overlay regions), -`editor_input_waveform.cpp`, `editor_controls.cpp` (`waveMarksFor` / `grabbableMarks` only), -`src/core/instrument/ui/spline_edit.h`'s `splineOverlayBox` and `envelope_overlay`, and -`tests/test_waveform_view.cpp` + `tests/test_component_geometry.cpp`. **Does not own:** any -deck module or `editor_*_deck` TU (T4's), the envelope-cache region of -`editor_paint_waveform.cpp` (T3's), or — **without an explicit proposal** — -`src/shell/panel/draw_kit.cpp` and `src/core/ui/component_geometry.cpp`. - -**Behavior — Ω.5, the marks.** -- **Trigger ⇒ the loop pair and the crossfade mark are HIDDEN entirely.** Not Disabled, not - dim: absent. **Gate with loop off ⇒ they are still drawn and still grabbable**, styled **grey - disabled** rather than today's dim teal at `kMarkAlphaDisabled` - (`editor_paint_waveform.cpp:51-53, 285-305`). The drag-to-set-loop gesture survives, which is - the whole reason they stay grabbable. -- **The `"LOOP \xe2\x80\x94 GATE ONLY"` caption (`editor_paint_waveform.cpp:254`) is removed - unconditionally** — in Trigger there is no span left to centre it in. -- **The two sibling captions SURVIVE** (`:255`): `DRAG TO SET LOOP` when the pair is parked and - `LOOP OFF` when a span is retained. Their box derives from the marks' span, which still - exists in Gate, and they carry the one thing the grey styling cannot — *which* off state you - are in, and that dragging will fix it. **This is a [propose]-class call made here rather than - left open**; reversing it means deleting information, not styling. -- **The seams.** `waveMarksFor`'s `present[]` (`editor_controls.cpp:145`) is the correct place - to suppress — it also skips label layout. `grabbableMarks` (`:162`) is the separate - grabbability fold and must NOT follow it into hiding for the Gate-off case. -- **Warning to the track:** `resolveWaveformClaim`'s smallest-area arbitration and `capAtPoint`'s - reverse-order resolve (`editor_input_waveform.cpp:65, 89, 111, 212`) are both sensitive to - marks entering and leaving the set — `capAtPoint`'s reverse order exists precisely so a - coincident pair stays separable and the crossfade (the one mark with no column) can never be - shadowed (`core/instrument/CLAUDE.md`, `waveform_view`). Changing which marks are in the set - changes what those two resolve, which is the regression risk in this half of the track. - -**Behavior — Ω.6, the mapping. This is a CLASS defect, not a START-marker instance.** - -Two chains that do not agree, verified this pass: - -| | Marker / overlay chain | Waveform draw chain | -|---|---|---| -| Area | `waveformOverlayArea` returns the **FULL band, no inset** (`waveform_view.cpp:23-25`) | insets **2 px each side**; `waveformColumnCount` = `width − 4` (`component_geometry.cpp:95-98`), columns start at `box.x + 2` (`draw_kit.cpp:292, 311`) | -| Domain | `[0, N]` **closed** (`clampFrame`, `waveform_view.cpp:15-19`) | `[0, N)` **half-open** (`columnMinMax`, `peaks.cpp:69-71`) | -| Rounding | **round-half-up** (`frameToX`, `:51-58`) | **truncation** | -| Phase | a 2 px marker line centred on an integer (`editor_paint_waveform.cpp:298, :301`) | a 1 px column centred at +0.5 | - -Four independent discrepancies. The 2 px inset dominates — frame 0's marker sits 2 px left of -the first column and the error reverses sign toward the end — and the closed domain puts the -last frame one pixel outside the band. **Everything riding the overlay inherits all four:** the -loop span fill, the crossfade wedges, the staged envelope polyline, and the spline contour — -whose `spline_edit.h` guarantee that it uses "the FULL area, no inset, so the drawn contour -stays 1:1 with the sample's time axis" **is currently false by 2 px.** - -- **Mandate ONE mapping both chains read**, so the drift cannot recur. Two routes: - - **Inset the overlay** (recommended) — instrument-only, and the overlay is already the - distinct `OverlayArea` type, so the change has one construction site. - - **De-inset the waveform** — `[propose]`, **requires justification before it is taken**: it - touches the shared `draw_kit`, which also serves the docked bank panel and the browse-card - thumbnails, so its blast radius is three surfaces rather than one. -- **The zero-crossing snap must be preserved EXACTLY *by this track*.** `nearestZeroCrossing` - (`waveform_view.cpp:181-210`) fans out symmetrically, probing `t−d` before `t+d` so an - equidistant tie resolves to the lower frame; it runs on the mono cache at drag time and is - exempt for the crossfade handle. **Behaviour-identical after the change** is an acceptance - criterion, not an assumption. - - > **Amended, ruling R3 (Daniel, 2026-08-03) — read this as scope, not as a verdict on the - > snap.** This clause bound T5 to change the coordinate mapping WITHOUT perturbing the snap, - > and **T5 met it: the fan-out it received is the fan-out it returned, and its tie-rule - > fixtures pass unmodified.** The snap's behaviour was subsequently changed on purpose by a - > DIFFERENT track — **Ω-W2-T5 `snap-radius`**, which bounds the fan-out to - > `kMarkerGrabWidth` — so a later reader finding a bounded snap in the tree is looking at - > Ω-W2-T5's work, not at a T5 violation. **Bounding which frames are REACHABLE does not - > change how a reachable tie RESOLVES**, which is why T5's fixtures survive it untouched. -- **One question the track must ANSWER, not dodge:** what "sample-accurate" means when - `frames < columns` and one frame spans many pixel columns. A marker must land somewhere - inside its own frame's column span; which end, and whether the inverse `xToFrame` - (`:60-70`) still round-trips, are the track's to specify. -- **The two pinned fixtures are re-DERIVED from the new contract, never re-baselined.** - `test_waveform_view.cpp` and `test_component_geometry.cpp` pin the current mapping by - construction; a test updated to match whatever the code now prints has stopped being a test. - -**Acceptance criteria.** -- One mapping function serves both chains; a second one anywhere in the tree fails review. -- Every mark, the loop span fill, both crossfade wedges, the staged polyline and the spline - contour land on the same pixel as the waveform column for the frame they name — at frame 0, - at the last frame, and at an interior frame, asserted in the pure fixtures. -- `frames < columns` and `frames > columns` are both covered. -- The zero-crossing snap returns the identical frame for the identical input, ties included. - **(Scope criterion on THIS track, discharged. Superseded phase-wide by R3 — the snap's - radius bound is Ω-W2-T5's deliberate change, and this track's tie-rule fixtures pass - unmodified under it.)** -- Trigger shows no loop marks and no loop caption; Gate-with-loop-off shows grey marks that - still drag, and dragging one still turns the enable on. -- `docs/product/instrument-control-surface.md` §6.3(d) and §6.4 agree with the shipped - behaviour. - -**Prerequisites.** None; concurrent with T1–T4. **Discharges:** Ω.5, Ω.6, Ω-F1, Ω-F2. +**All five tracks have landed** — T1 `mode-switch-responsiveness`, T2 `mode-switch-persist`, +T3 `meter-rate`, T4 `deck-reflow-focus-toggles`, T5 `overlay-mapping` — merged to `dev` as +`1159d36` (2026-08-03). See `docs/COMPLETED.md` for the full narrative. (T1's deferred +FX-park queue was reverted the same day and its downstream mechanics were superseded again +on 2026-08-21 — see the rulings note above.) --- @@ -894,32 +455,19 @@ stays 1:1 with the sample's time axis" **is currently false by 2 px.** not to be worth doing, and that is a legitimate outcome. T3's deliverable is a **RECORD, not a fix** (the Λ-W3 precedent). -| Track | Owns | -|---|---| -| **T1** `view-topology-cache` | `src/core/view/` — tree / `parentOf` / handle-map memoization | -| **T2** `content-detect-throttle` | `src/shell/panel/panel_input.cpp` — the 30 Hz detector's cadence | -| **T3** `omega-verification-sweep` | `docs/VERIFICATION.md` — the DAW record for all of Ω-W1 | -| **T5** `snap-radius` | `src/core/instrument/ui/waveform_view.{h,cpp}` + the marker-drag input path — **spec'd nowhere; see below** | +| Track | Owns | Status | +|---|---|---| +| **T1** `view-topology-cache` | `src/core/view/` — tree / `parentOf` / handle-map memoization | Open — measurement-gated | +| **T2** `content-detect-throttle` | `src/shell/panel/panel_input.cpp` — the 30 Hz detector's cadence | **Landed**, `84afd1b` (2026-08-03) | +| **T3** `omega-verification-sweep` | `docs/VERIFICATION.md` — the DAW record for all of Ω-W1 | Open — not run | +| **T5** `snap-radius` | `src/core/instrument/ui/waveform_view.{h,cpp}` + the marker-drag input path | **Landed**, `aa87d81` (2026-08-03) | -**Shared files in the wave:** none. T1–T3 touch three disjoint directories, and T5 touches a -fourth (Ω-W1-T5's surface, which lands first). - -**Ω-W2-T5 has no spec section here, on purpose.** It arrived from ruling R3 — Daniel, -2026-08-03, recorded in `docs/product/single-cycle-and-wavetable.md` §7.1 and summarized at -"Flagged for awareness" item 4 above — after this phase was written, and is **committed on -branch `omega-w2-t5-snap-radius` (`b7b7e88`)**: it -bounds `nearestZeroCrossing`'s fan-out to `kMarkerGrabWidth` (5 px) and adds -**Ctrl-defeats-snap** on a marker drag. **Numbered T5 rather than T4 because it inherits -Ω-W1-T5's file surface; there is no Ω-W2-T4.** Two things it needs from other owners: - -- **Flagged for doc-keeper:** its record belongs in `docs/COMPLETED.md`. This plan names it - only so the phase's own superseded criteria can point at what superseded them. -- **Flagged for doc-keeper — a documentation gap, not a defect.** *Ctrl-defeats-snap on a - marker drag* is a new **user-visible gesture** with no home in - `docs/product/instrument-control-surface.md`, and §4.1's exclusion block there — which - excludes waveform markers from the modifier law because *"overloading Shift there would - collide with a shipped gesture"* — now reads as a claim that markers carry no modifier at - all. That block needs amending and the gesture needs documenting. +**T2 and T5 have landed** — see `docs/COMPLETED.md` for the full narrative. T5 arrived from +ruling R3 (Daniel, 2026-08-03, recorded in `docs/product/single-cycle-and-wavetable.md` §7.1) +after this phase was written and inherited Ω-W1-T5's file surface rather than getting its own +spec section here; there is no Ω-W2-T4. Its documentation gap (Ctrl-defeats-snap on a marker +drag, previously unhomed in `docs/product/instrument-control-surface.md`) has since been +closed — that doc's §4.1 now carries the gesture. #### Ω-W2-T1 — `view-topology-cache` *(measurement-gated on Ω-W1-T1's result)* @@ -949,39 +497,6 @@ not take that seam here, and do not grow that file. **Prerequisites.** Ω-W1-T1 landed and measured. **Discharges:** Ω.7's residual, or records it as not worth taking. -#### Ω-W2-T2 — `content-detect-throttle` - -**Goal.** The auto-tag detector stops spending main-thread time 30 times a second on exactly -the projects where the switch is slowest. - -**Spec:** Ω.7's third cost. - -**Surface boundary — owns:** `src/shell/panel/panel_input.cpp` (`detectNewContent` / -`enumerateLiveGuids` and their cadence inside `bankPanelRefresh`). **Does not own:** any other -panel TU, `view.cpp`, or the auto-tag RULES — only how often they are asked. - -**Behavior.** `detectNewContent` (`panel_input.cpp:147`) runs `enumerateLiveGuids` (`:106-135`) -every tick of REAPER's `"timer"` registration (`main.cpp:354`, ~30/sec), regardless of panel -state — O(T + I) REAPER string calls plus roughly 5 allocations per track and 6 per item per -tick (a per-element-type count, not the earlier `~4(T+I)` derivation, which undercounted; the -ratio the throttle buys is unaffected by the correction). Throttle the ENUMERATION, not the -detection semantics: the same GUIDs must still be detected, the first-poll and `reloadPending` -re-baseline guards must still fire before any diff, and the lane-minting pass must still run -only on a tick that tagged. A slower cadence is a latency change to an invisible background -tag, and the track states the chosen interval and why. - -**Acceptance criteria.** -- New tracks and items are still auto-tagged into the active mode, and a project reload still - re-baselines rather than mass-tagging. -- The Ρ-W1-T1 rule survives: an added GUID that already carries a membership record is still - dropped, so a render-in-place result is not re-tagged to Design. -- The per-tick allocation count is derived, not measured: it needs live REAPER-side track/item - counts and a runtime allocation trap neither CTest nor this repo's tooling provides. - `[verify — DAW]`. -- The panel's own repaint cadence is unaffected. - -**Prerequisites.** None beyond Ω-W1. **Discharges:** Ω.7's polling cost. - #### Ω-W2-T3 — `omega-verification-sweep` **Goal.** The DAW-observable half of this phase is recorded rather than assumed. From 0484599408857a60d4105bf3f604d6cc82cf6aca Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:52:28 -0400 Subject: [PATCH 06/12] Fix stuck drag after OS hand-off withdrawal, split panel_drag_os, doc fixes Re-checks the physical button before resuming an internal drag and treats a button-up move mid-drag as a release, closing a stale-payload/silent-drop bug; splits the OS hand-off seam into its own TU for the line ceiling; corrects two docs and skips a redundant module-path lookup. --- src/app/CMakeLists.txt | 1 + src/shell/actions/CLAUDE.md | 2 +- src/shell/actions/drag_out_win.cpp | 4 +- src/shell/actions/drag_out_win.h | 14 +++- src/shell/panel/panel_drag.cpp | 99 ++++--------------------- src/shell/panel/panel_drag_os.cpp | 113 +++++++++++++++++++++++++++++ src/shell/panel/panel_state.h | 21 +++++- src/shell/panel/panel_window.cpp | 2 +- 8 files changed, 165 insertions(+), 91 deletions(-) create mode 100644 src/shell/panel/panel_drag_os.cpp diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index 1a801c0..a295cce 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -27,6 +27,7 @@ add_library(reaper_reasampler MODULE ${REASAMPLER_SRC_DIR}/shell/panel/panel_audition.cpp ${REASAMPLER_SRC_DIR}/shell/panel/panel_bank_ops.cpp ${REASAMPLER_SRC_DIR}/shell/panel/panel_drag.cpp + ${REASAMPLER_SRC_DIR}/shell/panel/panel_drag_os.cpp ${REASAMPLER_SRC_DIR}/shell/panel/footer_input.cpp ${REASAMPLER_SRC_DIR}/shell/panel/panel_input.cpp ${REASAMPLER_SRC_DIR}/shell/panel/panel_layout.cpp diff --git a/src/shell/actions/CLAUDE.md b/src/shell/actions/CLAUDE.md index 561f479..54cec56 100644 --- a/src/shell/actions/CLAUDE.md +++ b/src/shell/actions/CLAUDE.md @@ -57,7 +57,7 @@ is owned by other directories and only skinned here. - `shell/actions` (`action_registry` / `design_view_actions` / `bank_actions` / `prune_action`) — the bindable action families, all routed via the `command_id`/`gaccel`/`hookcommand` contract. `action_registry` owns the shared registration plumbing (interned channel-qualified id strings; register and mirror-unregister present the identical pointer) **and the Q-W6 registration TABLE**: `main.cpp`'s own family (capture scopes, panel toggle, insert, batch, realtime, recapture, version) is one `ActionTableRow` array — suffix, phrase, flat function-pointer handler — that registration, hookcommand dispatch, and the unload mirror-unregister all iterate, so adding an action touches the table only (OCP). Bank mutations flow through the promptless `shell/bank_ops` verbs (`bankOp*` + `persistBankOp`, taking `ReaSamplerSession&`), which the panel menus and `bank_actions` consume as thin UX skins; `src/shell/bank_ops/CLAUDE.md` owns the one-op-one-Ctrl-Z rule those verbs enforce. The prune action (`prune_action`, `BANK_PRUNE_FOLDER`) is **the ONLY file-deletion action in the system**; it opens no undo point (file deletion is not REAPER-undoable). `BANK_PRUNE_FOLDER` halts on `blockedByTracking` and prints each blocker that fired, with recovery instructions. - `package_export_action` — the "export bank as package" skin: survey and report first, confirm what is absent (and, separately, a destination being replaced), pick a destination, write. Every prompt in the flow lives here so `shell/package/export_bank` stays promptless. Read-only against the project — it holds the session by `const&`, so no ext-state write, generation bump or undo point is reachable. Registration rides `main.cpp`'s action table (`EXPORT_BANK_PACKAGE`); the panel's tab menu is the second skin over the same body. -- `drag_out_win` — the host/OS boundary of drag-out: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. Owns the two window tests behind `ui::DropContext`'s hand-off proofs (`core/ui/CLAUDE.md` owns the rule they feed). `pointerOverHostWindow` is process identity on Windows (`WindowFromPoint` + `GetWindowThreadProcessId`), non-null `WindowFromPoint` under SWELL, whose window list is already this-process-only; its shipped exception: a plugin bridged into `reaper_host*.exe` reports off-host though the pointer never left REAPER. `pointerOverPluginEditor` walks from the hit window to its root reading each window's owning process and the module of its CLASS procedure (`GetClassLongPtr(GCLP_WNDPROC)` → `GetModuleHandleEx(FROM_ADDRESS)`), and lets `ui::window_origin` decide; its failure modes are listed at the declaration, and it is always false under SWELL. The OLE drop source re-asks the gesture law on every `QueryContinueDrag` (a function pointer the panel supplies) and withdraws the drag per `ui::decideOsDragTurn`, reporting it as `OsDragEnd::Withdrawn*` so the panel resumes its own gesture. A drop into a plugin writes nothing to the bank and inserts no timeline item — the plugin receives a copy-only path list, exactly as from Explorer. +- `drag_out_win` — the host/OS boundary of drag-out: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. Owns the two window tests behind `ui::DropContext`'s hand-off proofs (`core/ui/CLAUDE.md` owns the rule they feed). `pointerOverHostWindow` is process identity on Windows (`WindowFromPoint` + `GetWindowThreadProcessId`), non-null `WindowFromPoint` under SWELL, whose window list is already this-process-only; its shipped exception: a plugin bridged into `reaper_host*.exe` reports off-host though the pointer never left REAPER. `pointerOverPluginEditor` walks from the hit window to its root reading each window's owning process and the module of its CLASS procedure (`GetClassLongPtr(GCLP_WNDPROC)` → `GetModuleHandleEx(FROM_ADDRESS)`), and lets `ui::window_origin` decide; its failure modes are listed at the declaration, and it is always false under SWELL. The OLE drop source re-asks the gesture law on every `QueryContinueDrag` (a function pointer the panel supplies) and withdraws the drag per `ui::decideOsDragTurn`, reporting it as `OsDragEnd::Withdrawn*` so the panel resumes its own gesture. A drop into a plugin means the extension writes nothing to the bank and inserts no timeline item — the plugin receives a copy-only path list, exactly as from Explorer (the plugin's own reaction to that path list is outside the extension's control). - `instrument_drop_win` — instrument-drop shell: `probeDropTarget` resolves a screen point to a track + a `ReaperSurface` (via the pure `wire::classifyReaperSurface`, whose token rules `core/wire/CLAUDE.md` owns) — it answers only what REAPER named, never whether the pointer is still in REAPER or over a plugin's own editor (so an FX surface reaches the instrument drop only as its chrome; a plugin's editor inside it hands off to the OS) — and the drop half adds a ReaSampler 9000 instance and applies the dragged capture's state via a transient `.vstpreset` + `TrackFX_SetPreset` (the former `TrackFX_SetNamedConfigParm` "vst_chunk" write was silently unappliable for VST3). Exposes `loadInstrumentOntoTrack` (inner half, no own undo block) and `performInstrumentDrop` (wraps in its own undo block). **Never captures, never writes the bank, never inserts a timeline item.** - `insert_fx_action` — the "Insert as FX" verb: a second ENTRY POINT to `instrument_drop_win`'s drop body (the bank panel's Placement button and a bindable action), loading the panel's focused capture onto the first selected track. Owns no insert logic of its own — it resolves the selection, folds `core/ui/insert_fx_enable`, and calls `performInstrumentDrop` (which keeps the one undo block). Refuses visibly on every failing precondition; the panel button additionally paints dead for the two panel-local ones. - `arrange_drop_win` — the drag-out gesture's arrange outcome: `arrangeTimeAtScreenX` (pointer column → time via `GetSet_ArrangeView2`'s one-pixel-span reading — inferred, not SDK-documented) and `performArrangeDrop` (snap the drop time, then one `InsertMedia` per capture on the pointer's track — assumed, not confirmed, to land end-to-end via REAPER's own cursor advance — in ONE undo block, counting only InsertMedia's reported successes, with the caller's track selection and edit cursor restored). The one timeline-placing shell here, per the invariant above; it never captures and never writes the bank. diff --git a/src/shell/actions/drag_out_win.cpp b/src/shell/actions/drag_out_win.cpp index 49a4fe3..e038eec 100644 --- a/src/shell/actions/drag_out_win.cpp +++ b/src/shell/actions/drag_out_win.cpp @@ -356,7 +356,9 @@ ui::WindowFacts windowFacts(HWND hwnd) { } f.procResolved = true; f.hostModule = mod == GetModuleHandleW(nullptr); - f.modulePath = moduleFileName(mod); + // classifyWindowCode returns Host on hostModule before ever reading modulePath — skip the + // (string-building) lookup in that case. + if (!f.hostModule) f.modulePath = moduleFileName(mod); return f; } diff --git a/src/shell/actions/drag_out_win.h b/src/shell/actions/drag_out_win.h index 7129ac7..a85fdbf 100644 --- a/src/shell/actions/drag_out_win.h +++ b/src/shell/actions/drag_out_win.h @@ -64,9 +64,21 @@ bool pointerOverHostWindow(int screenX, int screenY); // - a plugin that draws straight into the parent REAPER hands it, creating no window of its // own, leaves nothing of its code in the chain; // - a plugin binary installed beneath the Windows directory reads as an OS module; -// - a class procedure that resolves to no module (a charset-mismatch handle) is no evidence. +// - a class procedure that resolves to no module (a charset-mismatch handle) is no evidence; +// - an all-#32770 dialog-based editor (the generic Windows dialog class) resolves to no +// recognizable plugin module — falls to REAPER's outcome; +// - a .NET/WPF editor's class procedure is a CLR-thunked address with no resolvable module — +// also falls to REAPER's outcome. +// The opposite direction is unguarded: a third-party hook that replaces REAPER's own chrome +// class procedure via SetClassLongPtr(GCLP_WNDPROC) would make REAPER's own window read as +// Plugin — toward OLE instead of away from it. // [verify — DAW] that REAPER's GetThingFromPoint names an fx_* token over the plugin's own child // window — the law requires it, so an unnamed editor would refuse rather than hand off. +// [verify — DAW] that OLE never walks up to an ancestor drop target (e.g. REAPER's own file +// import) when the plugin editor's window registers no IDropTarget of its own. +// [verify — DAW] that GetThingFromPoint cannot name a REAPER region (e.g. an fx_* token) when a +// foreign window overlaps it at the same screen point — this fn trusts WindowFromPoint's hit +// over whatever GetThingFromPoint separately reports. // SWELL: always false (no plugin-editor hand-off off Windows). Side-effect-free. bool pointerOverPluginEditor(int screenX, int screenY); diff --git a/src/shell/panel/panel_drag.cpp b/src/shell/panel/panel_drag.cpp index a5bb4fe..0b83541 100644 --- a/src/shell/panel/panel_drag.cpp +++ b/src/shell/panel/panel_drag.cpp @@ -148,11 +148,13 @@ void applyDragCursor(CardGesture g) { SetCursor(LoadCursor(nullptr, idc)); } +} // namespace + // The out-of-client half of the same thin lookup: pure class -> pure cue -> stock cursor. The // cue is set on EVERY move, so "will this work" is visible before the button comes up, and a // refusal is a cursor the user can see rather than a release that does nothing. Cursor-only by // design — the docked panel is usually not under the pointer during an out-of-client drag, so -// panel status text would be invisible exactly when it is needed. +// panel status text would be invisible exactly when it is needed. Shared with panel_drag_os.cpp. void applyDropCue(DropCue cue) { const char* idc = nullptr; switch (cue) { @@ -169,15 +171,7 @@ void applyDropCue(DropCue cue) { SetCursor(LoadCursor(nullptr, idc)); } -// One independent evaluation of the drag's target: the resolved class plus the live facts its -// outcome needs. Nothing is remembered between calls (core/ui/CLAUDE.md: "the drag-out law is -// per-move and stateless"). -struct LiveDrop { - DropClass cls = DropClass::None; - MediaTrack* track = nullptr; - int screenX = 0; // the evaluated point in screen coords; meaningful only outside the client -}; - +// Shared with panel_drag_os.cpp (lawHandsOffAtCursor). LiveDrop resolveLiveDrop(int x, int y) { RECT cr{}; GetClientRect(g_panel.hwnd, &cr); @@ -220,80 +214,7 @@ LiveDrop resolveLiveDrop(int x, int y) { return out; } -// Re-asked by the OS drag loop on every turn. The payload fields the law reads survive -// resetDragState, so it still sees the drag the hand-off wound down. With the panel gone -// mid-loop there is no gesture left to give back, so the OS keeps the drag. -bool lawHandsOffAtCursor() { - if (!g_panel.hwnd) return true; - POINT p{}; - GetCursorPos(&p); - ScreenToClient(g_panel.hwnd, &p); - return resolveLiveDrop(p.x, p.y).cls == DropClass::OsHandoff; -} - -// The OS loop gave the gesture back (ui::OsDragTurn::Withdraw*): restore exactly the fields -// resetDragState cleared that the drag still needs, then either keep dragging under our own -// capture or — the release was consumed by the OS loop, so no WM_LBUTTONUP will follow — commit -// at the release point now. No move is replayed here: the next WM_MOUSEMOVE re-resolves the cue, -// and replaying one could re-enter handOffToOs from inside itself. -void resumeInternalDrag(const std::string& primaryId, bool released) { - if (!g_panel.hwnd) return; - g_panel.dragArmed = true; - g_panel.dragging = true; - g_panel.dragPrimaryId = primaryId; - - POINT p{}; - GetCursorPos(&p); - ScreenToClient(g_panel.hwnd, &p); - if (released) { - onLBtnUp(p.x, p.y); - return; - } - // [verify — DAW] that SetCapture re-takes the mouse mid-press after DoDragDrop released it. - SetCapture(g_panel.hwnd); - invalidatePanel(); -} - -// The hand-off: DoDragDrop takes mouse capture and runs its own modal loop, so the internal drag -// must be fully wound down first, and only once the payload is known to be hand-off-able. This -// runs on every qualifying move — do not memoize a failed attempt. The OS loop may hand the -// gesture back (ui::OsDragTurn owns why); a side effect worth keeping is that, on Windows, a -// return trip can no longer drop a CF_HDROP on REAPER's file import or on this panel's own -// WM_DROPFILES ingest — the release there is withdrawn, not dropped. -void handOffToOs() { - // Resolve BEFORE tearing anything down (the resolver reads the live drag payload), then let - // the pure rule couple the two side effects: an unresolvable payload leaves the internal - // drag intact rather than winding it down for a hand-off that never runs — a half-torn-down - // drag reads as "the drag did nothing, try again". canInitiateDragOut extends the same - // coupling to the OS-readiness failures the pure decision cannot see. - const std::vector paths = resolveDragPathsForOs(); - if (!ui::decideOsHandoff(paths).handOffToOs || !canInitiateDragOut(paths)) { - applyDropCue(DropCue::Refuse); - invalidatePanel(); - return; - } - - const std::string primaryId = g_panel.dragPrimaryId; // resetDragState clears it - if (GetCapture() == g_panel.hwnd) ReleaseCapture(); - resetDragState(); - invalidatePanel(); - - // COPY-ONLY; blocking on Windows. - switch (initiateDragOut(g_panel.hwnd, paths, &lawHandsOffAtCursor)) { - case OsDragEnd::Withdrawn: - resumeInternalDrag(primaryId, /*released=*/false); - return; - case OsDragEnd::WithdrawnAtRelease: - resumeInternalDrag(primaryId, /*released=*/true); - return; - case OsDragEnd::Concluded: - break; - } - - // The drag state was reset above, so the WM_LBUTTONUP that ends the modal loop takes - // onLBtnUp's not-dragging branch and never restores the arrow — do it here instead. - SetCursor(LoadCursor(nullptr, IDC_ARROW)); -} +namespace { // Resolves the topmost INTERACTIVE element under client (x, y) for hover feedback, // mirroring handleClick's precedence exactly (so the element that lights on hover is @@ -381,7 +302,15 @@ void maybeShowTooltip() { } } -void onMouseMove(int x, int y) { +void onMouseMove(int x, int y, WPARAM wParam) { + // A move with the button already up while still dragging means the release was missed (the + // OS hand-off's own modal loop can eat a WM_LBUTTONUP, and coalescing can lose one too) — + // treat it as the release now rather than leaving dragging stuck for the next plain click. + if (g_panel.dragging && !(wParam & MK_LBUTTON)) { + onLBtnUp(x, y); + return; + } + // Hover feedback: resolve + repaint-on-change, but NOT during a drag (the drag owns // the visual feedback then — a drop-target highlight, not a hover). if (!g_panel.dragging && !g_panel.dragArmed) updateHover(x, y); diff --git a/src/shell/panel/panel_drag_os.cpp b/src/shell/panel/panel_drag_os.cpp new file mode 100644 index 0000000..278d4c6 --- /dev/null +++ b/src/shell/panel/panel_drag_os.cpp @@ -0,0 +1,113 @@ +// panel_drag_os.cpp — the OS hand-off / resume seam split out of panel_drag.cpp (the ~600-line +// file ceiling): the live re-ask the OS drag loop uses to decide whether to keep the drag, the +// hand-off itself, and resuming the internal drag when the OS loop gives the gesture back. +// +// Compiled into the reaper_reasampler MODULE. No REAPER API functions are called directly here; +// REAPER SDK types arrive via panel_state.h. + +#include +#include + +#include "shell/panel/panel_state.h" + +#include "shell/actions/drag_out_win.h" // OLE / SWELL drag-out initiation seam + +namespace reasampler::panel { + +namespace { + +// The physical left button, honouring a swapped primary button (SM_SWAPBUTTON) — distinct from +// dragging/dragArmed, which only say what OUR state machine last believed. Read fresh rather than +// trusted, because DoDragDrop's own modal loop can consume the WM_LBUTTONUP that would otherwise +// have told us the button came up. +bool leftButtonPhysicallyDown() { + const int vk = GetSystemMetrics(SM_SWAPBUTTON) ? VK_RBUTTON : VK_LBUTTON; + return (GetAsyncKeyState(vk) & 0x8000) != 0; +} + +// Re-asked by the OS drag loop on every turn. The payload fields the law reads survive +// resetDragState, so it still sees the drag the hand-off wound down. With the panel gone +// mid-loop there is no gesture left to give back, so the OS keeps the drag. +bool lawHandsOffAtCursor() { + if (!g_panel.hwnd) return true; + POINT p{}; + GetCursorPos(&p); + ScreenToClient(g_panel.hwnd, &p); + return resolveLiveDrop(p.x, p.y).cls == DropClass::OsHandoff; +} + +// The OS loop gave the gesture back (ui::OsDragTurn::Withdraw*): restore exactly the fields +// resetDragState cleared that the drag still needs, then either keep dragging under our own +// capture or — the release was consumed by the OS loop, so no WM_LBUTTONUP will follow — commit +// at the release point now. No move is replayed here: the next WM_MOUSEMOVE re-resolves the cue, +// and replaying one could re-enter handOffToOs from inside itself. +void resumeInternalDrag(const std::string& primaryId, bool released) { + if (!g_panel.hwnd) return; + g_panel.dragArmed = true; + g_panel.dragging = true; + g_panel.dragPrimaryId = primaryId; + + POINT p{}; + GetCursorPos(&p); + ScreenToClient(g_panel.hwnd, &p); + if (released) { + onLBtnUp(p.x, p.y); + return; + } + // [verify — DAW] that SetCapture re-takes the mouse mid-press after DoDragDrop released it. + SetCapture(g_panel.hwnd); + // The button can have come up between DoDragDrop's own release and this re-capture (the + // Withdrawn turn only promises the button was down AT the last OS-loop turn, not at this + // instant) — without this check dragging stays armed with a stale payload, and the very next + // plain click commits a reorder/replace nobody asked for. + if (!leftButtonPhysicallyDown()) { + onLBtnUp(p.x, p.y); + return; + } + invalidatePanel(); +} + +} // namespace + +// The hand-off: DoDragDrop takes mouse capture and runs its own modal loop, so the internal drag +// must be fully wound down first, and only once the payload is known to be hand-off-able. This +// runs on every qualifying move — do not memoize a failed attempt. The OS loop may hand the +// gesture back (ui::OsDragTurn owns why); a side effect worth keeping is that, on Windows, a +// return trip can no longer drop a CF_HDROP on REAPER's file import or on this panel's own +// WM_DROPFILES ingest — the release there is withdrawn, not dropped. +void handOffToOs() { + // Resolve BEFORE tearing anything down (the resolver reads the live drag payload), then let + // the pure rule couple the two side effects: an unresolvable payload leaves the internal + // drag intact rather than winding it down for a hand-off that never runs — a half-torn-down + // drag reads as "the drag did nothing, try again". canInitiateDragOut extends the same + // coupling to the OS-readiness failures the pure decision cannot see. + const std::vector paths = resolveDragPathsForOs(); + if (!ui::decideOsHandoff(paths).handOffToOs || !canInitiateDragOut(paths)) { + applyDropCue(DropCue::Refuse); + invalidatePanel(); + return; + } + + const std::string primaryId = g_panel.dragPrimaryId; // resetDragState clears it + if (GetCapture() == g_panel.hwnd) ReleaseCapture(); + resetDragState(); + invalidatePanel(); + + // COPY-ONLY; blocking on Windows. + switch (initiateDragOut(g_panel.hwnd, paths, &lawHandsOffAtCursor)) { + case OsDragEnd::Withdrawn: + resumeInternalDrag(primaryId, /*released=*/false); + return; + case OsDragEnd::WithdrawnAtRelease: + resumeInternalDrag(primaryId, /*released=*/true); + return; + case OsDragEnd::Concluded: + break; + } + + // The drag state was reset above, so the WM_LBUTTONUP that ends the modal loop takes + // onLBtnUp's not-dragging branch and never restores the arrow — do it here instead. + SetCursor(LoadCursor(nullptr, IDC_ARROW)); +} + +} // namespace reasampler::panel diff --git a/src/shell/panel/panel_state.h b/src/shell/panel/panel_state.h index 2a77e79..9926bb9 100644 --- a/src/shell/panel/panel_state.h +++ b/src/shell/panel/panel_state.h @@ -536,11 +536,28 @@ void registerAccel(); void unregisterAccel(); // panel_drag.cpp — the card-drag/hover state machine (pure mirror: core/ui/card_drag). -// Per-mouse-move work stays plain free-function calls. -void onMouseMove(int x, int y); +// Per-mouse-move work stays plain free-function calls. wParam carries WM_MOUSEMOVE's own +// button-state flags (MK_LBUTTON etc) so a move with the button already up can be told apart +// from GetAsyncKeyState's live-polled state. +void onMouseMove(int x, int y, WPARAM wParam); void onLBtnUp(int x, int y); void handleRightClick(int x, int y); void resetDragState(); void maybeShowTooltip(); +// One independent evaluation of the drag's target: the resolved class plus the live facts its +// outcome needs. Nothing is remembered between calls (core/ui/CLAUDE.md: "the drag-out law is +// per-move and stateless"). Shared with panel_drag_os.cpp. +struct LiveDrop { + DropClass cls = DropClass::None; + MediaTrack* track = nullptr; + int screenX = 0; // the evaluated point in screen coords; meaningful only outside the client +}; +LiveDrop resolveLiveDrop(int x, int y); +void applyDropCue(DropCue cue); + +// panel_drag_os.cpp — the OS hand-off / resume seam split out of panel_drag.cpp (the ~600-line +// file ceiling). +void handOffToOs(); + } // namespace reasampler::panel diff --git a/src/shell/panel/panel_window.cpp b/src/shell/panel/panel_window.cpp index edb7e3a..d895a4f 100644 --- a/src/shell/panel/panel_window.cpp +++ b/src/shell/panel/panel_window.cpp @@ -152,7 +152,7 @@ WDL_DLGRET dlgProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { return 0; } case WM_MOUSEMOVE: - onMouseMove(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); + onMouseMove(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam), wParam); return 0; case WM_LBUTTONUP: onLBtnUp(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); From fee941a83d06fd481fff169f31c256f2dba4a7fa Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:57:11 -0400 Subject: [PATCH 07/12] Exempt ReaSampler 9000's own editor from the plugin OS drag hand-off A dragged bank card over our own instrument's editor was OLE-dropping like a third-party plugin. Classify our channel's own .vst3 module as Instrument, not Plugin, so it keeps the normal instrument-drop outcome. --- src/core/ui/CLAUDE.md | 6 +++-- src/core/ui/window_origin.cpp | 1 + src/core/ui/window_origin.h | 14 ++++++++---- src/shell/actions/CLAUDE.md | 2 +- src/shell/actions/drag_out_win.cpp | 30 ++++++++++++++++++++++++- src/shell/actions/drag_out_win.h | 4 +++- tests/test_window_origin.cpp | 35 ++++++++++++++++++++++++++++++ 7 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/core/ui/CLAUDE.md b/src/core/ui/CLAUDE.md index 6c2c720..97274db 100644 --- a/src/core/ui/CLAUDE.md +++ b/src/core/ui/CLAUDE.md @@ -98,7 +98,9 @@ L7 sub-pass, 2026-07-27): hand off over any extension window docked in REAPER, so this route also REQUIRES the FX-surface token; within an FX window the proof is what separates the plugin's editor (OS hand-off — the plugin takes the files as it would from Explorer) from REAPER's - chrome (instrument drop, unchanged). + chrome (instrument drop, unchanged). Our own ReaSampler 9000 editor is exempted from + this proof (`WindowCode::Instrument`) — it is FX chrome we own, so a drop on it is the + instrument-drop outcome like any other REAPER chrome, never the OS hand-off. Once handed off, the law keeps running: on Windows the OS drag loop re-asks it every turn (`decideOsDragTurn`) and WITHDRAWS the moment it stops answering `OsHandoff`, @@ -116,7 +118,7 @@ L7 sub-pass, 2026-07-27): - `tab_strip` — REAPER-free scrollable tab-strip layout + hit-test for the named-banks strip. - `prune_button` — pure layout/hit-test for the `bank_panel` footer Prune button. - `drag_out` — the pure drag-out gesture law plus path-list assembly. Owns the `ReaperSurface` vocabulary (TrackPanel / FxSurface / FxEmbed / Arrange / Other — `core/wire/instrument_drop` classifies REAPER's info token INTO it; there is deliberately no off-REAPER member, since no token can carry that fact), `decideDropClass` (surface × single-vs-multi payload → Internal / InstrumentDrop / ArrangeInsert / Refuse / OsHandoff / None), and `cueForDropClass`. `OsHandoff` is reachable ONLY through the two proof members of `DropContext` — `pointerOffHost` (`drag_out_win::pointerOverHostWindow`) where REAPER's hit-test named nothing (`Other`, no track), and `pointerOverPluginEditor` (`drag_out_win::pointerOverPluginEditor`) where it named `FxSurface` — both gated ahead of the surface switch, so no evidence source can give a live drag away to OLE on its own (the invariant above states why each pairing). Also owns `decideOsDragTurn`, the OS drag loop's per-turn Continue / Drop / Cancel / Withdraw / WithdrawAtRelease verdict. **No `DropClass` means "nothing happens"**: a surface with no defined outcome for the payload resolves to `Refuse`, which the shell shows as a cursor, so "no silent no-op release" is a property of the enumeration rather than of any call site. -- `window_origin` — whose code implements a window (Host / System / Plugin / Unknown, from the owning process and the module holding the window CLASS's procedure) and `isPluginEditorChain`, the rule behind `pointerOverPluginEditor`: any Plugin window from the hit window up to its root. Unknown is never proof, and without the OS directory every non-host module degrades to Unknown — both keep the failure direction toward REAPER's own outcome, never toward the modal hand-off. The shell reads the facts; the known misses are listed at `drag_out_win.h`'s `pointerOverPluginEditor`. +- `window_origin` — whose code implements a window (Host / System / Plugin / Instrument / Unknown, from the owning process and the module holding the window CLASS's procedure) and `isPluginEditorChain`, the rule behind `pointerOverPluginEditor`: any Plugin window from the hit window up to its root. Unknown is never proof, and without the OS directory every non-host module degrades to Unknown — both keep the failure direction toward REAPER's own outcome, never toward the modal hand-off. `Instrument` is the one deliberate exemption: our own ReaSampler 9000 module (`WindowFacts::ownModule`, a basename match against this channel's `app_version::vstOutputName() + ".vst3"` — stable and beta each recognise only their own binary) is FX chrome we own, not a hand-off-worthy plugin editor, so it is never `Plugin` and never counts as `isPluginEditorChain` proof — a drag over our own editor stays the instrument-drop outcome. Every other plugin's module is unaffected. The shell reads the facts; the known misses are listed at `drag_out_win.h`'s `pointerOverPluginEditor`. - `theme` — pure palette module: role→color mapping, REAPER-grey neutral ladder + the pastel accent system, the keyboard strip's spectral ramp, WCAG contrast-floor helpers + `compositeOver` (the effective color of a translucent fill, so alpha overlays are testable). Only the ramp's MID stop is its own constant; lo/hi are still aliases of `accent/primary`/`accent/tertiary`, so a categorical accent move CAN still reorder the ramp — `testSpectralRampLuminanceIsMonotonic` is the build-time catch, not the structure. - `component_geometry` — pure button/slider/list-row geometry + hover hit-test helpers. - `action_bar` — pure task-grouped action-bar layout/hit-test: clusters (Capture / Placement / Maintenance / Tagging / Switching). diff --git a/src/core/ui/window_origin.cpp b/src/core/ui/window_origin.cpp index 6248ad8..18348dc 100644 --- a/src/core/ui/window_origin.cpp +++ b/src/core/ui/window_origin.cpp @@ -37,6 +37,7 @@ WindowCode classifyWindowCode(const WindowFacts& facts, const std::string& syste if (facts.otherProcess) return WindowCode::Plugin; if (!facts.procResolved) return WindowCode::Unknown; if (facts.hostModule) return WindowCode::Host; + if (facts.ownModule) return WindowCode::Instrument; if (normalizePath(systemRoot).empty()) return WindowCode::Unknown; return isBeneath(facts.modulePath, systemRoot) ? WindowCode::System : WindowCode::Plugin; } diff --git a/src/core/ui/window_origin.h b/src/core/ui/window_origin.h index cc10c07..de775bd 100644 --- a/src/core/ui/window_origin.h +++ b/src/core/ui/window_origin.h @@ -10,10 +10,11 @@ namespace reasampler::ui { enum class WindowCode { - Host, // the host executable itself — REAPER's own chrome and containers - System, // an OS module (standard controls, dialog class): says nothing about who placed it - Plugin, // any other code — a plugin binary, or a window of another process (bridged plugin) - Unknown, // the procedure resolved to no loaded module — no evidence either way + Host, // the host executable itself — REAPER's own chrome and containers + System, // an OS module (standard controls, dialog class): says nothing about who placed it + Plugin, // any other code — a plugin binary, or a window of another process (bridged plugin) + Instrument, // our own ReaSampler 9000 module: FX chrome, not a hand-off-worthy plugin editor + Unknown, // the procedure resolved to no loaded module — no evidence either way }; // What the shell observed about ONE window. The CLASS procedure, not the instance one, because an @@ -23,6 +24,11 @@ struct WindowFacts { bool otherProcess = false; // the window belongs to a process other than the host's bool procResolved = false; // the class procedure lies inside a loaded module bool hostModule = false; // ...and that module is the host executable + // ...and that module is THIS channel's ReaSampler 9000 .vst3 (basename match, set by the + // shell against app_version::vstOutputName() + ".vst3" — stable and beta each recognise only + // their own binary, never the other channel's). A drag over our own instrument's editor is + // the load-bearing-principle instrument-drop outcome, not the OS hand-off other plugins get. + bool ownModule = false; std::string modulePath; // that module's file path, UTF-8 }; diff --git a/src/shell/actions/CLAUDE.md b/src/shell/actions/CLAUDE.md index 54cec56..1e78de0 100644 --- a/src/shell/actions/CLAUDE.md +++ b/src/shell/actions/CLAUDE.md @@ -57,7 +57,7 @@ is owned by other directories and only skinned here. - `shell/actions` (`action_registry` / `design_view_actions` / `bank_actions` / `prune_action`) — the bindable action families, all routed via the `command_id`/`gaccel`/`hookcommand` contract. `action_registry` owns the shared registration plumbing (interned channel-qualified id strings; register and mirror-unregister present the identical pointer) **and the Q-W6 registration TABLE**: `main.cpp`'s own family (capture scopes, panel toggle, insert, batch, realtime, recapture, version) is one `ActionTableRow` array — suffix, phrase, flat function-pointer handler — that registration, hookcommand dispatch, and the unload mirror-unregister all iterate, so adding an action touches the table only (OCP). Bank mutations flow through the promptless `shell/bank_ops` verbs (`bankOp*` + `persistBankOp`, taking `ReaSamplerSession&`), which the panel menus and `bank_actions` consume as thin UX skins; `src/shell/bank_ops/CLAUDE.md` owns the one-op-one-Ctrl-Z rule those verbs enforce. The prune action (`prune_action`, `BANK_PRUNE_FOLDER`) is **the ONLY file-deletion action in the system**; it opens no undo point (file deletion is not REAPER-undoable). `BANK_PRUNE_FOLDER` halts on `blockedByTracking` and prints each blocker that fired, with recovery instructions. - `package_export_action` — the "export bank as package" skin: survey and report first, confirm what is absent (and, separately, a destination being replaced), pick a destination, write. Every prompt in the flow lives here so `shell/package/export_bank` stays promptless. Read-only against the project — it holds the session by `const&`, so no ext-state write, generation bump or undo point is reachable. Registration rides `main.cpp`'s action table (`EXPORT_BANK_PACKAGE`); the panel's tab menu is the second skin over the same body. -- `drag_out_win` — the host/OS boundary of drag-out: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. Owns the two window tests behind `ui::DropContext`'s hand-off proofs (`core/ui/CLAUDE.md` owns the rule they feed). `pointerOverHostWindow` is process identity on Windows (`WindowFromPoint` + `GetWindowThreadProcessId`), non-null `WindowFromPoint` under SWELL, whose window list is already this-process-only; its shipped exception: a plugin bridged into `reaper_host*.exe` reports off-host though the pointer never left REAPER. `pointerOverPluginEditor` walks from the hit window to its root reading each window's owning process and the module of its CLASS procedure (`GetClassLongPtr(GCLP_WNDPROC)` → `GetModuleHandleEx(FROM_ADDRESS)`), and lets `ui::window_origin` decide; its failure modes are listed at the declaration, and it is always false under SWELL. The OLE drop source re-asks the gesture law on every `QueryContinueDrag` (a function pointer the panel supplies) and withdraws the drag per `ui::decideOsDragTurn`, reporting it as `OsDragEnd::Withdrawn*` so the panel resumes its own gesture. A drop into a plugin means the extension writes nothing to the bank and inserts no timeline item — the plugin receives a copy-only path list, exactly as from Explorer (the plugin's own reaction to that path list is outside the extension's control). +- `drag_out_win` — the host/OS boundary of drag-out: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. Owns the two window tests behind `ui::DropContext`'s hand-off proofs (`core/ui/CLAUDE.md` owns the rule they feed). `pointerOverHostWindow` is process identity on Windows (`WindowFromPoint` + `GetWindowThreadProcessId`), non-null `WindowFromPoint` under SWELL, whose window list is already this-process-only; its shipped exception: a plugin bridged into `reaper_host*.exe` reports off-host though the pointer never left REAPER. `pointerOverPluginEditor` walks from the hit window to its root reading each window's owning process and the module of its CLASS procedure (`GetClassLongPtr(GCLP_WNDPROC)` → `GetModuleHandleEx(FROM_ADDRESS)`), and lets `ui::window_origin` decide; its failure modes are listed at the declaration, and it is always false under SWELL. It also sets `WindowFacts::ownModule` — a basename match of the resolved module against this channel's own `reasampler_9000[_beta].vst3` (`app_version::vstOutputName() + ".vst3"`) — so our own instrument's editor classifies as `Instrument`, not `Plugin`, and a drag over it stays the instrument-drop outcome instead of handing off to the OS; every other plugin's editor is unaffected. The OLE drop source re-asks the gesture law on every `QueryContinueDrag` (a function pointer the panel supplies) and withdraws the drag per `ui::decideOsDragTurn`, reporting it as `OsDragEnd::Withdrawn*` so the panel resumes its own gesture. A drop into a plugin means the extension writes nothing to the bank and inserts no timeline item — the plugin receives a copy-only path list, exactly as from Explorer (the plugin's own reaction to that path list is outside the extension's control). - `instrument_drop_win` — instrument-drop shell: `probeDropTarget` resolves a screen point to a track + a `ReaperSurface` (via the pure `wire::classifyReaperSurface`, whose token rules `core/wire/CLAUDE.md` owns) — it answers only what REAPER named, never whether the pointer is still in REAPER or over a plugin's own editor (so an FX surface reaches the instrument drop only as its chrome; a plugin's editor inside it hands off to the OS) — and the drop half adds a ReaSampler 9000 instance and applies the dragged capture's state via a transient `.vstpreset` + `TrackFX_SetPreset` (the former `TrackFX_SetNamedConfigParm` "vst_chunk" write was silently unappliable for VST3). Exposes `loadInstrumentOntoTrack` (inner half, no own undo block) and `performInstrumentDrop` (wraps in its own undo block). **Never captures, never writes the bank, never inserts a timeline item.** - `insert_fx_action` — the "Insert as FX" verb: a second ENTRY POINT to `instrument_drop_win`'s drop body (the bank panel's Placement button and a bindable action), loading the panel's focused capture onto the first selected track. Owns no insert logic of its own — it resolves the selection, folds `core/ui/insert_fx_enable`, and calls `performInstrumentDrop` (which keeps the one undo block). Refuses visibly on every failing precondition; the panel button additionally paints dead for the two panel-local ones. - `arrange_drop_win` — the drag-out gesture's arrange outcome: `arrangeTimeAtScreenX` (pointer column → time via `GetSet_ArrangeView2`'s one-pixel-span reading — inferred, not SDK-documented) and `performArrangeDrop` (snap the drop time, then one `InsertMedia` per capture on the pointer's track — assumed, not confirmed, to land end-to-end via REAPER's own cursor advance — in ONE undo block, counting only InsertMedia's reported successes, with the caller's track selection and edit cursor restored). The one timeline-placing shell here, per the invariant above; it never captures and never writes the bank. diff --git a/src/shell/actions/drag_out_win.cpp b/src/shell/actions/drag_out_win.cpp index e038eec..19b0539 100644 --- a/src/shell/actions/drag_out_win.cpp +++ b/src/shell/actions/drag_out_win.cpp @@ -14,6 +14,7 @@ #include "core/ui/drag_out.h" // decideOsDragTurn — the OS loop's per-turn verdict #include "core/ui/window_origin.h" // classifyWindowCode / isPluginEditorChain +#include "core/version/app_version.h" // vstOutputName — this channel's own .vst3 basename namespace reasampler { @@ -325,6 +326,30 @@ std::string moduleFileName(HMODULE mod) { } } +// True iff `path`'s file name (the part after the last separator) case-insensitively matches +// `fileName` — Windows paths are case-insensitive, and GetModuleFileName's casing tracks however +// the module was loaded (often not the on-disk casing). +bool sameBaseName(const std::string& path, const std::string& fileName) { + const std::size_t pos = path.find_last_of("\\/"); + const std::string base = pos == std::string::npos ? path : path.substr(pos + 1); + if (base.size() != fileName.size()) return false; + for (std::size_t i = 0; i < base.size(); ++i) { + char a = base[i], b = fileName[i]; + if (a >= 'A' && a <= 'Z') a = static_cast(a - 'A' + 'a'); + if (b >= 'A' && b <= 'Z') b = static_cast(b - 'A' + 'a'); + if (a != b) return false; + } + return true; +} + +// This channel's own ReaSampler 9000 module file name (e.g. "reasampler_9000.vst3", or the +// "_beta" variant) — the same name CMake gives the built .vst3 (OUTPUT_NAME + SUFFIX), so stable +// and beta each recognise only their own binary, never the other channel's. +const std::string& ownVstFileName() { + static const std::string name = version::vstOutputName() + ".vst3"; + return name; +} + // Empty on failure, which window_origin reads as "cannot tell OS code from plugin code". const std::string& systemRoot() { static const std::string root = [] { @@ -358,7 +383,10 @@ ui::WindowFacts windowFacts(HWND hwnd) { f.hostModule = mod == GetModuleHandleW(nullptr); // classifyWindowCode returns Host on hostModule before ever reading modulePath — skip the // (string-building) lookup in that case. - if (!f.hostModule) f.modulePath = moduleFileName(mod); + if (!f.hostModule) { + f.modulePath = moduleFileName(mod); + f.ownModule = sameBaseName(f.modulePath, ownVstFileName()); + } return f; } diff --git a/src/shell/actions/drag_out_win.h b/src/shell/actions/drag_out_win.h index a85fdbf..465ffec 100644 --- a/src/shell/actions/drag_out_win.h +++ b/src/shell/actions/drag_out_win.h @@ -68,7 +68,9 @@ bool pointerOverHostWindow(int screenX, int screenY); // - an all-#32770 dialog-based editor (the generic Windows dialog class) resolves to no // recognizable plugin module — falls to REAPER's outcome; // - a .NET/WPF editor's class procedure is a CLR-thunked address with no resolvable module — -// also falls to REAPER's outcome. +// also falls to REAPER's outcome; +// - our own ReaSampler 9000 module (this channel's reasampler_9000[_beta].vst3, matched by +// basename) also falls to REAPER's outcome, deliberately — see ui::WindowFacts::ownModule. // The opposite direction is unguarded: a third-party hook that replaces REAPER's own chrome // class procedure via SetClassLongPtr(GCLP_WNDPROC) would make REAPER's own window read as // Plugin — toward OLE instead of away from it. diff --git a/tests/test_window_origin.cpp b/tests/test_window_origin.cpp index 4290313..60ced67 100644 --- a/tests/test_window_origin.cpp +++ b/tests/test_window_origin.cpp @@ -23,6 +23,14 @@ static WindowFacts inModule(const std::string& path, bool host = false) { return f; } +static WindowFacts ownModule(const std::string& path) { + WindowFacts f; + f.procResolved = true; + f.modulePath = path; + f.ownModule = true; + return f; +} + // --- classifyWindowCode -------------------------------------------------------- // REAPER's own chrome and containers: the class procedure lives in the executable. @@ -103,6 +111,17 @@ static void testMissingSystemRootNeverYieldsPlugin() { CHECK(classifyWindowCode(inModule("C:\\REAPER\\reaper.exe", true), "") == WindowCode::Host); } +// Our own ReaSampler 9000 module — stable or beta, whichever the shell set ownModule for — is +// Instrument, not Plugin: FX chrome we own, so a drag over it must not hand off to the OS. +static void testOwnModuleIsInstrument() { + CHECK(classifyWindowCode( + ownModule("C:\\Program Files\\Common Files\\VST3\\reasampler_9000.vst3"), kWinDir) == + WindowCode::Instrument); + CHECK(classifyWindowCode( + ownModule("C:\\Program Files\\Common Files\\VST3\\reasampler_9000_beta.vst3"), + kWinDir) == WindowCode::Instrument); +} + // --- isPluginEditorChain ------------------------------------------------------- // Pointer on REAPER's FX-window chrome: a standard control or a REAPER container, under the @@ -134,6 +153,19 @@ static void testBridgedEmbeddedEditorIsPlugin() { WindowCode::System})); } +// A pointer over our own instrument's editor — Instrument code, not Plugin — is not hand-off +// proof: it stays REAPER's FX chrome, unlike a genuine plugin editor (testPluginWindowChainIsPlugin). +static void testOwnInstrumentChainIsNotPlugin() { + CHECK(!isPluginEditorChain({WindowCode::Instrument, WindowCode::Host, WindowCode::System})); +} + +// Another plugin's editor sitting alongside our own instrument in the same chain is still proof — +// only OUR module is exempted, nobody else's. +static void testOtherPluginBesideOwnInstrumentIsStillPlugin() { + CHECK(isPluginEditorChain( + {WindowCode::Plugin, WindowCode::Instrument, WindowCode::Host, WindowCode::System})); +} + // Unknown anywhere, and nothing else, is not proof; an empty chain (no window at all) is not. static void testUnknownAndEmptyChainsAreNotPlugin() { CHECK(!isPluginEditorChain({WindowCode::Unknown, WindowCode::Host, WindowCode::System})); @@ -150,11 +182,14 @@ int main() { testOtherProcessIsPluginRegardlessOfModule(); testUnresolvedProcIsUnknown(); testMissingSystemRootNeverYieldsPlugin(); + testOwnModuleIsInstrument(); testChromeChainIsNotPlugin(); testPluginWindowChainIsPlugin(); testStandardControlInsidePluginIsPlugin(); testBridgedEmbeddedEditorIsPlugin(); + testOwnInstrumentChainIsNotPlugin(); + testOtherPluginBesideOwnInstrumentIsStillPlugin(); testUnknownAndEmptyChainsAreNotPlugin(); if (g_fail == 0) std::printf("All tests passed.\n"); From 4567744b8f040766e07c8397e38c9d90e4090f6b Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 13:59:37 -0400 Subject: [PATCH 08/12] Fix spline-drag rollback, bake contour leak, held-note live miss, amp clamp Editor close mid-drag now rolls back published contours like capture-loss does; bake detaches liveContours so a printed capture can't observe a live drag; a held note now receives a live publish that lands in the same block as a note-on; amp spline read clamped to [0,1]. --- src/core/instrument/bake/bake_render.cpp | 1 + src/core/instrument/engine/velocity_curve.h | 5 ++-- src/core/instrument/engine/voice.cpp | 4 +-- src/core/instrument/engine/voice.h | 6 ++++ src/core/instrument/engine/voice_engine.cpp | 8 +++-- src/shell/instrument/CLAUDE.md | 25 ++++++++++------ src/shell/instrument/editor_platform.cpp | 9 ++++++ src/shell/instrument/reasampler_processor.h | 5 ++-- tests/test_bake_render.cpp | 31 +++++++++++++++++++ tests/test_live_delivery.cpp | 33 +++++++++++++++++++++ 10 files changed, 109 insertions(+), 18 deletions(-) diff --git a/src/core/instrument/bake/bake_render.cpp b/src/core/instrument/bake/bake_render.cpp index d0b1696..cc1bebc 100644 --- a/src/core/instrument/bake/bake_render.cpp +++ b/src/core/instrument/bake/bake_render.cpp @@ -36,6 +36,7 @@ BakeAudio renderBake(SampleData sample, const BakePlan& plan, double masterGainL // depend on what the user happened to be dragging. The dialed values are already in // this SampleData's own play params, which is what the bake is meant to print. sample.live = nullptr; + sample.liveContours = nullptr; const int channels = sample.channelCount(); // The limiter delays its output by its lookahead, so the buffers carry that many extra diff --git a/src/core/instrument/engine/velocity_curve.h b/src/core/instrument/engine/velocity_curve.h index dc1bb75..1f49790 100644 --- a/src/core/instrument/engine/velocity_curve.h +++ b/src/core/instrument/engine/velocity_curve.h @@ -225,8 +225,9 @@ private: // 128-point ceiling. // // Holds a RAW POINTER into the bound point array: the caller guarantees the array outlives the -// cursor, or rebinds it first. The voice binds against its SampleData, which has exactly that -// lifetime, or against its engine's live contour copy, which the engine rebinds before reusing. +// cursor, or rebinds it first. The voice binds against its SampleData (that lifetime), or +// against its engine's live contour copy — the double-buffer contract VoiceEngine::contours_ +// owns (voice_engine.h) is what makes that second binding safe. class SplineCursor { public: // Binds `c` if it has an evaluable segment; a shorter curve leaves the cursor inactive so diff --git a/src/core/instrument/engine/voice.cpp b/src/core/instrument/engine/voice.cpp index ded44cc..335ff13 100644 --- a/src/core/instrument/engine/voice.cpp +++ b/src/core/instrument/engine/voice.cpp @@ -335,8 +335,8 @@ void Voice::applyLive(const instrument::engine::LiveValues& live, bool snap) { namespace { -// One EG's half of Voice::applyContours. `before` is read through the OLD binding, which is -// still intact: the engine only overwrites a buffer no active voice is bound to. +// One EG's half of Voice::applyContours. `before` is read through the OLD binding — still +// intact, by the double-buffer contract VoiceEngine::contours_ owns (voice_engine.h). void rebindContour(SplineCursor& cursor, StepSmoother& smooth, const instrument::engine::ContourPoints& pts, double phase, bool snap) { if (!cursor.active()) return; diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index 6e225c1..435e187 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -302,10 +302,16 @@ private: // Early-free at a genuine permanent terminus (the spline analogue of a staged AHD's // finished()) — onFinalSegment()/segmentEndValue()'s own doc comments own the why. amp = readSpline(ampSplineCur_, ampSplineSmooth_); + // A swap-step absorbed by ampSplineSmooth_ can briefly push the sum outside [0,1] + // (a contour's own points are drawn in-range, but the smoother's addend isn't); the + // filter clamps its own read for the same reason (below). The finished-check reads + // the PRE-clamp value so a genuine zero-terminus isn't masked by the clamp itself. if (amp == 0.0 && ampSplineCur_.onFinalSegment() && ampSplineCur_.segmentEndValue() == 0.0) { amplitudeDone_ = true; } + if (amp < 0.0) amp = 0.0; + else if (amp > 1.0) amp = 1.0; } else if (playMode_ == PlayMode::Gate) { amp = env_.tick(); if (env_.finished()) amplitudeDone_ = true; diff --git a/src/core/instrument/engine/voice_engine.cpp b/src/core/instrument/engine/voice_engine.cpp index eed4e4c..2e9dffa 100644 --- a/src/core/instrument/engine/voice_engine.cpp +++ b/src/core/instrument/engine/voice_engine.cpp @@ -69,9 +69,13 @@ void VoiceEngine::applyContoursToActive() { } void VoiceEngine::startVoice(Voice& voice, int note, int velocity) { - // FIRST, so no sounding voice is left bound to a buffer the next read overwrites. + // FIRST, so no sounding voice is left bound to a buffer the next read overwrites. A held + // note sharing this generation with the note-on must see it too — refreshLive() alone + // would consume the generation without applying it, and since applyLiveToActive() only + // acts on a NEW generation, the held voice would then never see this publish at all + // (not merely late a block), only catching up whenever the next one arrives. applyContoursToActive(); - refreshLive(); + applyLiveToActive(); // THE read of the note-on-latched commit class, and the only one: a published block outranks // the snapshot's own copy (a live edit deliberately leaves that stale), and applyLive below // touches none of these three — so a move reaches the next note and no sounding one. diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index 1e34d86..9b5b8d7 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -91,8 +91,9 @@ else — no parameter set, no generation, no host callback — and its release i host-facing reader needs the model to carry it mid-gesture; it is what keeps a motion event off the host entirely (an unlatched `setInstrumentParams` opens a group edit even when nothing exposed moved). `reloadInstrument` republishes the model's contours, so a release, a click-edit -(add/delete/hard) and a restore all reach the voices through the reload; capture-lost, the one -exit that commits nothing, re-couriers the restored `params_`. +(add/delete/hard) and a restore all reach the voices through the reload; capture-lost and the +editor closing mid-drag (`removedFromParent`) are the two exits that commit nothing — both +re-courier the restored `params_`. **The VST3 parameter surface is a THIRD surface onto the one model, never a second copy.** The pure half — the frozen id table, the exposed set, the plain-value layer, the formatter — is @@ -241,13 +242,19 @@ against a performance budget — they are there because `VoiceEngine::applyLiveT ## Gotchas -- **`reasampler_processor.h` no longer needs a ceiling exception, and the one it had rested on a - false premise.** It was described as ONE class declaration; it also carried two namespace-scope - aggregates (`MasterBusMeter`, `LoadedInstrument`) and the automation lane's own state. Both are - now split out — `processor_snapshot.h` and `automation_channel.h`, on the same grounds - `editor_interaction.h` was split out of `reasampler_editor.h` in this directory: neither is - behaviour. What remains is under the ceiling. Its bulk is the drain-slot proof and the - RT-discipline constraints, which the comment conventions name as keep-worthy. +- **`reasampler_processor.h` carries a small, documented ceiling exception.** It was described as + ONE class declaration; it also carried two namespace-scope aggregates (`MasterBusMeter`, + `LoadedInstrument`) and the automation lane's own state. All three are split out — + `processor_snapshot.h` and `automation_channel.h`, on the same grounds `editor_interaction.h` + was split out of `reasampler_editor.h` in this directory: neither is behaviour. What remains is + the class declaration itself, a handful of lines over the ceiling since the live-contour block + (`liveContours_` + `publishLiveContours`) landed beside `liveParams_` under the same ordering + constraint. No further seam exists that isn't behaviour: every remaining aggregate-shaped chunk + (the preview mailbox, the drain-slot triple) is read and written inline by `process()` on the + audio thread, so splitting it into its own header would either duplicate the RT-discipline + comments at a second site or turn a plain member access into a cross-header one for no gain. + Its bulk is the drain-slot proof and the RT-discipline constraints, which the comment + conventions name as keep-worthy. - **The bake click only ARMS; the editor's sync tick runs it.** Calling `Main_OnCommandEx` inline from `WM_LBUTTONDOWN` would run the extension's whole landing diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index 5631266..b465139 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -162,6 +162,15 @@ void ReaSamplerEditor::removedFromParent() { // an edit forever and every later internal write to that parameter would emit a bare // performEdit. The capture-lost path normally closes it; this does not rely on Windows // delivering WM_CAPTURECHANGED before the window goes away. + if (drag_ == DragKind::kSplineNode) { + // A contour drag reaches the voices without the model (see publishContoursLive) — the + // window closing mid-drag is a second exit alongside WM_CAPTURECHANGED that must roll + // the voices back the same way, or an abandoned drag stays audible after the editor + // that started it is gone. + params_ = dragStartParams_; + publishContoursLive(); + drag_ = DragKind::kNone; + } if (processor_) processor_->endParamGesture(); if (childHwnd_) { KillTimer(childHwnd_, kSyncTimerId); // stop the poll before the window goes away diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index 71361ae..fab1b55 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -394,9 +394,8 @@ private: // Both live_ and draining_ observe this same block — a block owned by a snapshot would // leave the drain's still-sounding voices deaf to the knob under them. instrument::engine::LiveParams liveParams_; - // The drawn-contour block, SampleData::liveContours' one target, under liveParams_' ordering - // constraint and for its reason. No automation merge: no contour is a host parameter, so both - // engines read the model/editor publish directly. + // SampleData::liveContours' one target, under liveParams_' ordering constraint above and for + // its reason (this directory's CLAUDE.md owns why it needs no automation merge). instrument::engine::LiveContourBlock liveContours_; // The block the ENGINE reads, and the ONE thing SampleData::live points at. Written only by // the audio thread, which merges liveParams_ with the host's automation points once per block diff --git a/tests/test_bake_render.cpp b/tests/test_bake_render.cpp index 7a382be..0cff496 100644 --- a/tests/test_bake_render.cpp +++ b/tests/test_bake_render.cpp @@ -11,6 +11,7 @@ #include "../src/core/instrument/bake/bake_render.h" #include "../src/core/instrument/engine/limiter.h" +#include "../src/core/instrument/engine/live_contours.h" #include "../src/core/instrument/engine/live_params.h" #include @@ -338,6 +339,36 @@ int main() { CHECK(sameSamples(audio, reference)); } + // --- The bake detaches liveContours too, not just live ------------------------------- + // A contour drag in flight publishes into liveContours (see reasampler_processor.h); a + // render that observed it would print whatever the user happens to be dragging instead + // of the dialed shape already sitting in play.ampSpline.contour. + { + instrument::engine::LiveContourBlock block; + SampleData s = makeSample(/*stereo=*/false, /*frames=*/1000); + s.play.playMode = PlayMode::Trigger; + s.play.ampSpline.mode = EnvMode::Spline; + s.play.ampSpline.contour = instrument::engine::VelocityCurve::flat(); // dialed: unity + + // Publish a very different contour — ramping down to silence — into the block. + s.liveContours = █ + { + PlayParams dragged = s.play; + dragged.ampSpline.contour = instrument::engine::VelocityCurve::rampDown(); + block.publish(instrument::engine::foldContours(dragged)); + } + + const BakePlan plan = planOf(/*total=*/1000, /*noteOn=*/0, /*noteOff=*/1000); + const BakeAudio audio = renderBake(s, plan, kUnity, kNoLimiter); + + CHECK(s.liveContours == &block); // the caller's own snapshot was not detached + + SampleData detached = s; + detached.liveContours = nullptr; + const BakeAudio reference = renderBake(detached, plan, kUnity, kNoLimiter); + CHECK(sameSamples(audio, reference)); + } + // --- Regression baseline: the neutral render is the source, sample for sample -------- // A Trigger voice at its own root under Varispeed reads at ratio exactly 1 and hits no // filter, so every printed frame equals its source frame PROVIDED the amp curve's gain at diff --git a/tests/test_live_delivery.cpp b/tests/test_live_delivery.cpp index e8b4ac6..d0227b5 100644 --- a/tests/test_live_delivery.cpp +++ b/tests/test_live_delivery.cpp @@ -1167,6 +1167,38 @@ static void testVelocityGainSurvivesAHostilePublishThatReallyLands() { CHECK(proportional); } +// A note-on that lands in the same block boundary as a publish must not let the already- +// sounding voice miss it: startVoice used to call refreshLive() bare, which consumes the +// generation stamp without applying it to the voices already active — a held note that shared +// a block with a note-on then never saw the publish at all (not merely delayed a block). +static void testHeldVoiceReceivesALivePublishThatCoincidesWithANoteOn() { + SampleData s; + s.frames.assign(200000, 1.0f); // DC: the envelope alone shapes the output + s.sampleRate = kRate; + s.rootNote = 60; + s.play.adsr.attackFrames = 0; + s.play.adsr.sustainLevel = 1.0; + s.play.adsr.releaseFrames = 480000; // long enough that release never intrudes + + LiveParams block; + s.live = █ + VoiceEngine engine(2, s); + engine.noteOn(60, 100); // voice A, sustaining at the dialed 1.0 + + LiveValues quieter = foldLive(s.play, s.keyTrack); + quieter.adsr.sustainLevel = 0.2; + block.publish(quieter); + + engine.noteOn(61, 100); // voice B starts in the SAME generation as the publish + + std::vector out; + for (int i = 0; i < 20; ++i) engine.render(out, 512); // let voice A's glide settle + + // Both voices settle at 0.2 — a held voice that missed the publish would still sit at 1.0, + // for a combined 1.2 rather than 0.4. + CHECK(std::fabs(static_cast(out.back()) - 0.4) < 0.01); +} + int main() { testStageDurationChangeHoldsPhase(); testShortenedStageStillLandsContinuously(); @@ -1189,6 +1221,7 @@ int main() { testAPublishedPitchOffsetRefitsThePitchEnvelopeSpan(); testPitchRatioAndVelocityGainStayLatched(); testVelocityGainSurvivesAHostilePublishThatReallyLands(); + testHeldVoiceReceivesALivePublishThatCoincidesWithANoteOn(); if (g_fail == 0) std::printf("live_delivery tests passed\n"); return g_fail == 0 ? 0 : 1; } From 599d525e068cfb8514d5ab6f7674b12cbb678ecf Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 14:04:35 -0400 Subject: [PATCH 09/12] Docs: record live spline-envelope drags; file two follow-ups in TODO.md --- docs/COMPLETED.md | 17 +++++++++++++++++ docs/TODO.md | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index 739c617..3c30c57 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -1776,3 +1776,20 @@ footer readout is the whole indicator. And the readout's fit in the footer's count box is bounded only by a character count, not a real pixel width, since a pure test can't measure the proportional font without pulling shell drawing into `core/ui`; overflow end-ellipsizes rather than breaking layout. + +### Live spline-envelope drags: held and new notes follow a dragged knot + +Dragging a knot on a Spline-mode amp/pitch/filter envelope in ReaSampler 9000's editor +is now audible during the drag, for both held and new notes — releasing still commits +exactly as before, in one undo point. The three contours travel to the audio thread on +their own seqlock-published, double-buffered block (`core/instrument/engine/live_contours.h`, +`seqlock.h`), separate from `LiveValues` so a knob publish, an automation merge or the +live-block compare never carry the ~9 KB contour payload. Landed in the same merge: +closing the editor mid-drag rolls the published contours back to the pre-grab snapshot +(`removedFromParent`); the resample bake detaches live contours before its own render +(`bake_render.cpp`); a pre-existing bug is fixed where a live-parameter publish arriving +in the same block as a note-on failed to reach the already-held, sounding voices — +`startVoice`'s `refreshLive()` consumed the new generation without applying it to active +voices (`VoiceEngine::startVoice` now calls `applyLiveToActive()`); and the amp spline's read is +clamped to `[0,1]` to absorb a swap-step overshoot from the smoother. Not DAW-verified — +checks are tracked in `docs/SEPT_VERIFY.md` §5. diff --git a/docs/TODO.md b/docs/TODO.md index 2ea9c43..f823829 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -906,3 +906,48 @@ them in full. two-instance-of-the-same-plugin case are each observed in a live REAPER session, and either the identity is confirmed to survive all four, or a degradation is found and the keying is amended. + +## Spline-drag abort exits republish the editor's snapshot, not the processor's model + +**Context (what shipped — the live spline-envelope drags track).** Both spline-drag abort +exits — `ReaSamplerEditor::removedFromParent` and the `WM_CAPTURECHANGED` handler in +`src/shell/instrument/editor_platform.cpp` — roll a drag back by restoring +`params_ = dragStartParams_`, the editor's own pre-grab snapshot, then republishing +contours from it. + +**The wart.** `dragStartParams_` is a snapshot taken by the editor at mouse-down. If +`setState` or a bake reload lands mid-drag — either of which can rewrite the processor's +own model out from under the editor — the rollback re-couriers the STALE snapshot over +the model rather than the model's own current value, silently reverting whatever landed +during the drag. + +**Intended fix.** Have the processor republish from its own params on these two abort +paths, rather than the editor courier-ing its own stale pre-grab copy. + +**Priority / risk.** Not yet triaged. + +**Done looks like.** A `setState` or bake reload landing mid-drag survives a subsequent +capture-loss or editor-close abort intact — the abort rolls the drag back without +clobbering the intervening model change. + +## `reasampler_processor.h`'s preview mailbox is a candidate seam for the ceiling exception + +**Context (what shipped — the live spline-envelope drags track).** `reasampler_processor.h` +is a documented ~600-line-ceiling exception (`src/shell/instrument/CLAUDE.md` Gotchas): +after `processor_snapshot.h` and `automation_channel.h` were split out, what remains is +the class declaration itself, plus a handful of lines over the ceiling since the +live-contour block landed beside `liveParams_`. + +**The wart.** No further seam was found that isn't behaviour — except one candidate not +yet taken: the preview mailbox (the packed-sequence post/consume pair) looks like it could +move into its own header-inline `PreviewMailbox` aggregate without adding indirection to +`process()`'s access. + +**Intended fix.** Revisit once the pending sample-rate branch lands — it is expected to +add roughly 10 more lines to the file, which may tip the decision. + +**Priority / risk.** Not yet triaged. + +**Done looks like.** Either the preview mailbox is split into its own header-inline +aggregate with `process()`'s access left zero-cost, or the file is re-affirmed as a +documented exception once the sample-rate branch's size impact is known. From faa513cafbcb61966ef47c9cab27e7e307af86b2 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 14:11:22 -0400 Subject: [PATCH 10/12] Fix SWELL drag guard, channel-fork the VST editor window class, dedupe own-module match Panel drag no longer misfires on SWELL's always-zero wParam; stable/beta VST editors register distinct window classes against their own module instead of colliding on REAPER's; own-module basename matching moves to a tested pure helper. --- src/core/ui/window_origin.cpp | 13 +++++++++ src/core/ui/window_origin.h | 11 ++++--- src/shell/actions/drag_out_win.cpp | 22 ++------------ src/shell/actions/drag_out_win.h | 8 +++-- src/shell/instrument/editor_platform.cpp | 37 ++++++++++++++++++++---- src/shell/panel/CLAUDE.md | 7 +++-- src/shell/panel/panel_drag.cpp | 9 ++++++ src/shell/panel/panel_drag_os.cpp | 6 ++-- src/shell/panel/panel_state.h | 3 +- tests/test_window_origin.cpp | 25 ++++++++++++++++ 10 files changed, 103 insertions(+), 38 deletions(-) diff --git a/src/core/ui/window_origin.cpp b/src/core/ui/window_origin.cpp index 18348dc..c4c8554 100644 --- a/src/core/ui/window_origin.cpp +++ b/src/core/ui/window_origin.cpp @@ -46,4 +46,17 @@ bool isPluginEditorChain(const std::vector& chain) { return std::find(chain.begin(), chain.end(), WindowCode::Plugin) != chain.end(); } +bool isOwnModulePath(const std::string& path, const std::string& ownName) { + const std::size_t pos = path.find_last_of("\\/"); + const std::string base = pos == std::string::npos ? path : path.substr(pos + 1); + if (base.size() != ownName.size()) return false; + for (std::size_t i = 0; i < base.size(); ++i) { + char a = base[i], b = ownName[i]; + if (a >= 'A' && a <= 'Z') a = static_cast(a - 'A' + 'a'); + if (b >= 'A' && b <= 'Z') b = static_cast(b - 'A' + 'a'); + if (a != b) return false; + } + return true; +} + } // namespace reasampler::ui diff --git a/src/core/ui/window_origin.h b/src/core/ui/window_origin.h index de775bd..05afa94 100644 --- a/src/core/ui/window_origin.h +++ b/src/core/ui/window_origin.h @@ -24,10 +24,8 @@ struct WindowFacts { bool otherProcess = false; // the window belongs to a process other than the host's bool procResolved = false; // the class procedure lies inside a loaded module bool hostModule = false; // ...and that module is the host executable - // ...and that module is THIS channel's ReaSampler 9000 .vst3 (basename match, set by the - // shell against app_version::vstOutputName() + ".vst3" — stable and beta each recognise only - // their own binary, never the other channel's). A drag over our own instrument's editor is - // the load-bearing-principle instrument-drop outcome, not the OS hand-off other plugins get. + // ...and that module is THIS channel's ReaSampler 9000 .vst3. Rationale: core/ui/CLAUDE.md's + // window_origin entry (the ownModule/Instrument exemption). bool ownModule = false; std::string modulePath; // that module's file path, UTF-8 }; @@ -43,4 +41,9 @@ WindowCode classifyWindowCode(const WindowFacts& facts, const std::string& syste // proof — the hand-off this gates is modal, so it needs positive evidence. bool isPluginEditorChain(const std::vector& chain); +// True iff `path`'s file name (the part after the last separator, forward or back) matches +// `ownName` case-insensitively — see WindowFacts::ownModule for why this decides Instrument vs +// Plugin. `path` may be a bundle path (only the final component is compared). +bool isOwnModulePath(const std::string& path, const std::string& ownName); + } // namespace reasampler::ui diff --git a/src/shell/actions/drag_out_win.cpp b/src/shell/actions/drag_out_win.cpp index 19b0539..cc07a15 100644 --- a/src/shell/actions/drag_out_win.cpp +++ b/src/shell/actions/drag_out_win.cpp @@ -326,25 +326,9 @@ std::string moduleFileName(HMODULE mod) { } } -// True iff `path`'s file name (the part after the last separator) case-insensitively matches -// `fileName` — Windows paths are case-insensitive, and GetModuleFileName's casing tracks however -// the module was loaded (often not the on-disk casing). -bool sameBaseName(const std::string& path, const std::string& fileName) { - const std::size_t pos = path.find_last_of("\\/"); - const std::string base = pos == std::string::npos ? path : path.substr(pos + 1); - if (base.size() != fileName.size()) return false; - for (std::size_t i = 0; i < base.size(); ++i) { - char a = base[i], b = fileName[i]; - if (a >= 'A' && a <= 'Z') a = static_cast(a - 'A' + 'a'); - if (b >= 'A' && b <= 'Z') b = static_cast(b - 'A' + 'a'); - if (a != b) return false; - } - return true; -} - // This channel's own ReaSampler 9000 module file name (e.g. "reasampler_9000.vst3", or the -// "_beta" variant) — the same name CMake gives the built .vst3 (OUTPUT_NAME + SUFFIX), so stable -// and beta each recognise only their own binary, never the other channel's. +// "_beta" variant) — the same name CMake gives the built .vst3 (OUTPUT_NAME + SUFFIX). Matching +// logic lives in core/ui/window_origin's isOwnModulePath; see there. const std::string& ownVstFileName() { static const std::string name = version::vstOutputName() + ".vst3"; return name; @@ -385,7 +369,7 @@ ui::WindowFacts windowFacts(HWND hwnd) { // (string-building) lookup in that case. if (!f.hostModule) { f.modulePath = moduleFileName(mod); - f.ownModule = sameBaseName(f.modulePath, ownVstFileName()); + f.ownModule = ui::isOwnModulePath(f.modulePath, ownVstFileName()); } return f; } diff --git a/src/shell/actions/drag_out_win.h b/src/shell/actions/drag_out_win.h index 465ffec..5916ee1 100644 --- a/src/shell/actions/drag_out_win.h +++ b/src/shell/actions/drag_out_win.h @@ -68,9 +68,13 @@ bool pointerOverHostWindow(int screenX, int screenY); // - an all-#32770 dialog-based editor (the generic Windows dialog class) resolves to no // recognizable plugin module — falls to REAPER's outcome; // - a .NET/WPF editor's class procedure is a CLR-thunked address with no resolvable module — -// also falls to REAPER's outcome; +// also falls to REAPER's outcome; `[verify]`; // - our own ReaSampler 9000 module (this channel's reasampler_9000[_beta].vst3, matched by -// basename) also falls to REAPER's outcome, deliberately — see ui::WindowFacts::ownModule. +// basename) also falls to REAPER's outcome, deliberately — see ui::WindowFacts::ownModule; +// - ReaSampler 9000 itself run bridged/in a separate process reads as Plugin, same as any other +// bridged plugin — banner (the OS hand-off fires over our own editor), harmless; +// - a renamed ReaSampler 9000 binary (the basename no longer matches ownVstFileName()) misses +// the exemption and reads as Plugin — banner, harmless. // The opposite direction is unguarded: a third-party hook that replaces REAPER's own chrome // class procedure via SetClassLongPtr(GCLP_WNDPROC) would make REAPER's own window read as // Plugin — toward OLE instead of away from it. diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index d329fe5..73080e1 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -13,6 +13,7 @@ #include "shell/instrument/editor_internal.h" // (transitively: lice + the kit, Windows only) #include "shell/instrument/reasampler_processor.h" +#include "core/version/app_version.h" // vstOutputName — channel-forked window class name using namespace Steinberg; @@ -20,7 +21,32 @@ namespace reasampler::vst { #ifdef _WIN32 namespace { -constexpr const wchar_t* kChildClassName = L"ReaSampler9000VstEditor"; + +// Channel-forked so stable and beta never fight over one class registration (a second +// RegisterClassW against an already-taken name silently fails, leaving that channel's editors +// running the FIRST-registered channel's wndProc). vstOutputName() is ASCII (CMake OUTPUT_NAME), +// so a plain widen is safe. +const std::wstring& childClassName() { + static const std::wstring name = [] { + const std::string& base = version::vstOutputName(); + return std::wstring(base.begin(), base.end()) + L"VstEditor"; + }(); + return name; +} + +// The module holding a given code address — used to resolve THIS wndProc's own module rather +// than the parent's hInstance REAPER hands us. Registering against the parent would put the class +// in REAPER's own module, where a second .vst3 (the other channel) sharing that hInstance could +// collide on the (now channel-forked) name, and any lookup-by-class would resolve into the wrong +// binary. Takes the address rather than naming wndProc directly so a free function in this +// anonymous namespace doesn't need access to the private member. +HMODULE moduleHandleFor(void* codeAddress) { + HMODULE mod = nullptr; + GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | + GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + reinterpret_cast(codeAddress), &mod); + return mod; +} // The change-detection poll (WM_TIMER on the child window). A low-frequency UI-thread // timer: responsive enough that a recapture/ingest/assign refreshes within a bounded @@ -105,8 +131,9 @@ void ReaSamplerEditor::attachedToParent() { HWND parent = static_cast(systemWindow); if (!parent) return; - HINSTANCE hInst = - reinterpret_cast(GetWindowLongPtr(parent, GWLP_HINSTANCE)); + // Own module, not the parent's hInstance — see moduleHandleFor() above. + HINSTANCE hInst = reinterpret_cast( + moduleHandleFor(reinterpret_cast(&ReaSamplerEditor::wndProc))); if (!hInst) hInst = GetModuleHandle(nullptr); static bool classRegistered = false; @@ -114,7 +141,7 @@ void ReaSamplerEditor::attachedToParent() { WNDCLASSW wc{}; wc.lpfnWndProc = &ReaSamplerEditor::wndProc; wc.hInstance = hInst; - wc.lpszClassName = kChildClassName; + wc.lpszClassName = childClassName().c_str(); wc.hCursor = LoadCursor(nullptr, IDC_ARROW); // 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 @@ -134,7 +161,7 @@ void ReaSamplerEditor::attachedToParent() { refreshFromBank(); const ViewRect& r = getRect(); - childHwnd_ = CreateWindowExW(0, kChildClassName, L"", WS_CHILD | WS_VISIBLE, 0, 0, + childHwnd_ = CreateWindowExW(0, childClassName().c_str(), L"", WS_CHILD | WS_VISIBLE, 0, 0, r.getWidth(), r.getHeight(), parent, nullptr, hInst, nullptr); if (childHwnd_) { SetWindowLongPtr(childHwnd_, GWLP_USERDATA, reinterpret_cast(this)); diff --git a/src/shell/panel/CLAUDE.md b/src/shell/panel/CLAUDE.md index 2fe6f43..4f2adfd 100644 --- a/src/shell/panel/CLAUDE.md +++ b/src/shell/panel/CLAUDE.md @@ -2,9 +2,9 @@ ## Scope -The REAPER-facing shell for the docked bank panel: the eight `bank_panel` split TUs +The REAPER-facing shell for the docked bank panel: the nine `bank_panel` split TUs (`panel_window` / `panel_layout` / `panel_render` / `panel_input` / `panel_drag` / -`panel_thumbnails` / `panel_audition` / `panel_bank_ops`, sharing state via +`panel_drag_os` / `panel_thumbnails` / `panel_audition` / `panel_bank_ops`, sharing state via `panel_state.h`), plus `draw_kit`, the shared LICE draw shell also consumed by the VST3 editor (`shell/instrument/`). @@ -47,12 +47,13 @@ live in `shell/bank_ops`, a sibling directory, not here. ## Modules -- `bank_panel` (`shell/panel/`: `panel_window` / `panel_layout` / `panel_render` / `panel_input` / `panel_drag` / `panel_thumbnails` / `panel_audition` / `panel_bank_ops`, sharing state via `panel_state.h` — Q-W2 split of the former god-module into eight TUs) — docked LICE-drawn grid with three-zone layout: top toolbar (Capture → Placement, frequent acts only, via `action_bar`, short labels; Maintenance and the rarer Placement/Capture variants sit in the More (⋯) overflow menu via `overflow_menu`), bottom toolbar (four opposite-mode tag buttons + Show Both), and footer (`[Arrange|Design]` toggle, Tail button, Prune via `footer_bar`). Grid renders in sparse slot order with gap cells, drop dispatch, metadata overlay, and selection via `accent/tertiary` purple border. Draws through the L1 kit by palette role; OS drag-out via `drag_out` + `drag_out_win`. `panel_window` owns the SWELL dialog lifecycle + dialog proc + drop-target opt-in; `panel_layout` the toolbar/footer/menu rects + vertical-split geometry (the one geometry source both paint and hit-test read); `panel_render` the WM_PAINT draw; `panel_input` click/wheel/keyboard routing + the new-content auto-tag timer; `panel_drag` the hover + card-drag state machine + drop dispatch; `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 with the VST3 editor) stays a separate TU. +- `bank_panel` (`shell/panel/`: `panel_window` / `panel_layout` / `panel_render` / `panel_input` / `panel_drag` / `panel_drag_os` / `panel_thumbnails` / `panel_audition` / `panel_bank_ops`, sharing state via `panel_state.h` — Q-W2 split of the former god-module into nine TUs) — docked LICE-drawn grid with three-zone layout: top toolbar (Capture → Placement, frequent acts only, via `action_bar`, short labels; Maintenance and the rarer Placement/Capture variants sit in the More (⋯) overflow menu via `overflow_menu`), bottom toolbar (four opposite-mode tag buttons + Show Both), and footer (`[Arrange|Design]` toggle, Tail button, Prune via `footer_bar`). Grid renders in sparse slot order with gap cells, drop dispatch, metadata overlay, and selection via `accent/tertiary` purple border. Draws through the L1 kit by palette role; OS drag-out via `drag_out` + `drag_out_win`. `panel_window` owns the SWELL dialog lifecycle + dialog proc + drop-target opt-in; `panel_layout` the toolbar/footer/menu rects + vertical-split geometry (the one geometry source both paint and hit-test read); `panel_render` the WM_PAINT draw; `panel_input` click/wheel/keyboard routing + the new-content auto-tag timer; `panel_drag` the hover + card-drag state machine + drop dispatch; `panel_drag_os` the OS hand-off/resume seam split out of `panel_drag`; `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 with the VST3 editor) stays a separate TU. - `panel_window` — SWELL dialog lifecycle + dialog proc + drop-target opt-in. The drop splits by extension: a `.rsbank` is a whole bank and routes to the package-import action (one NEW bank each), everything else keeps the audio-ingest route. - `panel_layout` — toolbar/footer/menu rects + vertical-split geometry (the one geometry source both paint and hit-test read). - `panel_render` — the WM_PAINT draw. - `panel_input` — click/wheel/keyboard routing + the new-content auto-tag timer. - `panel_drag` — the hover + card-drag state machine + drop dispatch. + - `panel_drag_os` — the OS hand-off/resume seam: the live re-ask the OS drag loop uses, the hand-off itself, and resuming the internal drag when the OS loop gives the gesture back. - `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. diff --git a/src/shell/panel/panel_drag.cpp b/src/shell/panel/panel_drag.cpp index 0b83541..19a857f 100644 --- a/src/shell/panel/panel_drag.cpp +++ b/src/shell/panel/panel_drag.cpp @@ -306,10 +306,19 @@ void onMouseMove(int x, int y, WPARAM wParam) { // A move with the button already up while still dragging means the release was missed (the // OS hand-off's own modal loop can eat a WM_LBUTTONUP, and coalescing can lose one too) — // treat it as the release now rather than leaving dragging stuck for the next plain click. +#ifdef _WIN32 if (g_panel.dragging && !(wParam & MK_LBUTTON)) { onLBtnUp(x, y); return; } +#else + // SWELL never sets wParam on WM_MOUSEMOVE (swell-types.h ~1315), so the button-up test must + // read the physical key state instead of the message's own (always-zero) wParam. + if (g_panel.dragging && !(GetAsyncKeyState(VK_LBUTTON) & 0x8000)) { + onLBtnUp(x, y); + return; + } +#endif // Hover feedback: resolve + repaint-on-change, but NOT during a drag (the drag owns // the visual feedback then — a drop-target highlight, not a hover). diff --git a/src/shell/panel/panel_drag_os.cpp b/src/shell/panel/panel_drag_os.cpp index 278d4c6..bbc3daa 100644 --- a/src/shell/panel/panel_drag_os.cpp +++ b/src/shell/panel/panel_drag_os.cpp @@ -1,6 +1,6 @@ -// panel_drag_os.cpp — the OS hand-off / resume seam split out of panel_drag.cpp (the ~600-line -// file ceiling): the live re-ask the OS drag loop uses to decide whether to keep the drag, the -// hand-off itself, and resuming the internal drag when the OS loop gives the gesture back. +// panel_drag_os.cpp — the OS hand-off / resume seam split out of panel_drag.cpp: the live +// re-ask the OS drag loop uses to decide whether to keep the drag, the hand-off itself, and +// resuming the internal drag when the OS loop gives the gesture back. // // Compiled into the reaper_reasampler MODULE. No REAPER API functions are called directly here; // REAPER SDK types arrive via panel_state.h. diff --git a/src/shell/panel/panel_state.h b/src/shell/panel/panel_state.h index 9926bb9..3a43271 100644 --- a/src/shell/panel/panel_state.h +++ b/src/shell/panel/panel_state.h @@ -556,8 +556,7 @@ struct LiveDrop { LiveDrop resolveLiveDrop(int x, int y); void applyDropCue(DropCue cue); -// panel_drag_os.cpp — the OS hand-off / resume seam split out of panel_drag.cpp (the ~600-line -// file ceiling). +// panel_drag_os.cpp — the OS hand-off / resume seam split out of panel_drag.cpp. void handOffToOs(); } // namespace reasampler::panel diff --git a/tests/test_window_origin.cpp b/tests/test_window_origin.cpp index 60ced67..0d942ec 100644 --- a/tests/test_window_origin.cpp +++ b/tests/test_window_origin.cpp @@ -173,6 +173,27 @@ static void testUnknownAndEmptyChainsAreNotPlugin() { CHECK(!isPluginEditorChain({})); } +// --- isOwnModulePath ----------------------------------------------------------- + +static void testOwnModulePathCaseFolds() { + CHECK(isOwnModulePath("C:\\VST3\\reasampler_9000.vst3", "reasampler_9000.vst3")); + CHECK(isOwnModulePath("C:\\VST3\\ReaSampler_9000.VST3", "reasampler_9000.vst3")); +} + +static void testOwnModulePathBundlePathForwardAndBackSlash() { + CHECK(isOwnModulePath( + "C:\\VST3\\reasampler_9000.vst3\\Contents\\x86_64-win\\reasampler_9000.vst3", + "reasampler_9000.vst3")); + CHECK(isOwnModulePath( + "C:/VST3/reasampler_9000.vst3/Contents/x86_64-win/reasampler_9000.vst3", + "reasampler_9000.vst3")); +} + +static void testOwnModulePathStableAndBetaDoNotMatchEachOther() { + CHECK(!isOwnModulePath("C:\\VST3\\reasampler_9000_beta.vst3", "reasampler_9000.vst3")); + CHECK(!isOwnModulePath("C:\\VST3\\reasampler_9000.vst3", "reasampler_9000_beta.vst3")); +} + int main() { testHostExecutableIsHost(); testOsModulesAreSystem(); @@ -192,6 +213,10 @@ int main() { testOtherPluginBesideOwnInstrumentIsStillPlugin(); testUnknownAndEmptyChainsAreNotPlugin(); + testOwnModulePathCaseFolds(); + testOwnModulePathBundlePathForwardAndBackSlash(); + testOwnModulePathStableAndBetaDoNotMatchEachOther(); + if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; } From 7e523b050509c1e3fbda1b4cdd5d67fd92ae875a Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 14:15:36 -0400 Subject: [PATCH 11/12] Clean up stale editor-hInstance comments, add isOwnModulePath edge tests Drops a false claim about ReaSampler's own editor's hInstance, corrects the real reason for registering the wndproc class against our own module, and covers isOwnModulePath's separator edge cases. --- src/shell/actions/drag_out_win.cpp | 4 ++-- src/shell/instrument/editor_platform.cpp | 8 ++++---- tests/test_window_origin.cpp | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/shell/actions/drag_out_win.cpp b/src/shell/actions/drag_out_win.cpp index cc07a15..1e5c661 100644 --- a/src/shell/actions/drag_out_win.cpp +++ b/src/shell/actions/drag_out_win.cpp @@ -378,8 +378,8 @@ ui::WindowFacts windowFacts(HWND hwnd) { bool pointerOverPluginEditor(int screenX, int screenY) { // The CLASS procedure's module rather than the window's hInstance: a bare-SDK plugin may - // create its editor with the hInstance of the parent REAPER hands it (ReaSampler 9000's own - // editor does), so hInstance would read that plugin as REAPER. + // create its editor with the hInstance of the parent REAPER hands it, so hInstance would + // read that plugin as REAPER. const POINT pt{screenX, screenY}; const HWND hit = WindowFromPoint(pt); if (!hit) return false; diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index 73080e1..d13dda5 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -36,10 +36,10 @@ const std::wstring& childClassName() { // The module holding a given code address — used to resolve THIS wndProc's own module rather // than the parent's hInstance REAPER hands us. Registering against the parent would put the class -// in REAPER's own module, where a second .vst3 (the other channel) sharing that hInstance could -// collide on the (now channel-forked) name, and any lookup-by-class would resolve into the wrong -// binary. Takes the address rather than naming wndProc directly so a free function in this -// anonymous namespace doesn't need access to the private member. +// in REAPER's own module while the wndproc still points into our DLL — a dangling procedure +// pointer once our DLL unloads but REAPER's class registration outlives it. Takes the address +// rather than naming wndProc directly so a free function in this anonymous namespace doesn't need +// access to the private member. HMODULE moduleHandleFor(void* codeAddress) { HMODULE mod = nullptr; GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | diff --git a/tests/test_window_origin.cpp b/tests/test_window_origin.cpp index 0d942ec..d81fb77 100644 --- a/tests/test_window_origin.cpp +++ b/tests/test_window_origin.cpp @@ -194,6 +194,18 @@ static void testOwnModulePathStableAndBetaDoNotMatchEachOther() { CHECK(!isOwnModulePath("C:\\VST3\\reasampler_9000.vst3", "reasampler_9000_beta.vst3")); } +static void testOwnModulePathBareFilenameWithNoSeparatorMatches() { + CHECK(isOwnModulePath("reasampler_9000.vst3", "reasampler_9000.vst3")); +} + +static void testOwnModulePathEmptyPathDoesNotMatch() { + CHECK(!isOwnModulePath("", "reasampler_9000.vst3")); +} + +static void testOwnModulePathTrailingSeparatorDoesNotMatch() { + CHECK(!isOwnModulePath("C:\\VST3\\reasampler_9000.vst3\\", "reasampler_9000.vst3")); +} + int main() { testHostExecutableIsHost(); testOsModulesAreSystem(); @@ -216,6 +228,9 @@ int main() { testOwnModulePathCaseFolds(); testOwnModulePathBundlePathForwardAndBackSlash(); testOwnModulePathStableAndBetaDoNotMatchEachOther(); + testOwnModulePathBareFilenameWithNoSeparatorMatches(); + testOwnModulePathEmptyPathDoesNotMatch(); + testOwnModulePathTrailingSeparatorDoesNotMatch(); if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; From d22def7c1321de85e2709412ac5040bd72c8a351 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 25 Sep 2026 14:16:59 -0400 Subject: [PATCH 12/12] Docs: record plugin-editor drop; update SEPT_VERIFY plugin-drop checks --- docs/COMPLETED.md | 18 ++++++++++++++++++ docs/SEPT_VERIFY.md | 11 ++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index 3c30c57..5537745 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -1793,3 +1793,21 @@ in the same block as a note-on failed to reach the already-held, sounding voices voices (`VoiceEngine::startVoice` now calls `applyLiveToActive()`); and the amp spline's read is clamped to `[0,1]` to absorb a swap-step overshoot from the smoother. Not DAW-verified — checks are tracked in `docs/SEPT_VERIFY.md` §5. + +### Plugin-editor drop hand-off + +Dragging a bank card onto a third-party plugin's own editor window (e.g. Serum 2's +oscillator) now hands the file to Windows as an ordinary OS file drop, so the plugin +loads it — the same outcome as dragging the file in from Explorer. REAPER's other +surfaces keep their existing outcomes: dropping on the FX chain list or FX window +chrome still loads ReaSampler 9000, and TCP/MCP/arrange are unchanged. ReaSampler +9000's own editor is exempted from the hand-off (`WindowFacts::ownModule`, a basename +match against the channel's own `.vst3`) — a drop on it still loads the instrument. +The OS hand-off is reversible mid-drag: moving back over REAPER (including after +leaving REAPER for Explorer) hands the drag back to the panel's own drop behavior. +Stock REAPER plugins (ReaEQ etc.) now count as plugin editors for this purpose. Landed +in the same merge: stable and beta ReaSampler 9000 editors previously shared one +Windows window class, so side-by-side installs could run the wrong channel's code — +each channel now registers its own class. Mechanism detail: `src/core/ui/CLAUDE.md` +(`window_origin`) and `src/shell/actions/drag_out_win.h`. Not DAW-verified — checks +are tracked in `docs/SEPT_VERIFY.md` §4. diff --git a/docs/SEPT_VERIFY.md b/docs/SEPT_VERIFY.md index 127b54e..806f0ae 100644 --- a/docs/SEPT_VERIFY.md +++ b/docs/SEPT_VERIFY.md @@ -51,7 +51,7 @@ project rate 48000, unless a check says otherwise. - [ ] For mode-switching itself (solo-set restore, transport refusal, persistence, undo point count, etc.) — see `docs/VERIFICATION.md` §"Mode switching"; not repeated here -## 4. Drag a card into a plugin's editor (in progress, Sept 2026) +## 4. Drag a card into a plugin's editor (landed Sept 2026) - [ ] Drop a bank card on Serum 2's oscillator in its floating FX editor window — Serum loads the sample @@ -65,9 +65,14 @@ project rate 48000, unless a check says otherwise. change) - [ ] Drag a card out to Windows Explorer and back into REAPER — control hands back to the panel's own drag correctly -- [ ] Drop a card onto an already-open ReaSampler 9000 editor — it shows its "drop on the panel" - banner and does NOT spawn a second instance +- [ ] Drop a card onto an already-open ReaSampler 9000 editor — it loads ReaSampler 9000 onto the + track (no banner, no OS drop) - [ ] Drop a card onto ReaEQ's editor — shows a no-drop cursor, nothing loads +- [ ] Drop a card onto a plugin editor that doesn't accept files (e.g. a compressor) — note whether + it is refused (no-drop cursor, nothing loads) or lands in REAPER's own file import; either outcome + is accepted, but record which one actually happens +- [ ] Run stable and beta ReaSampler 9000 side by side and open both editors — each works normally, + no crash ## 5. Live spline-envelope drags (in progress, Sept 2026)