diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index aa776a8..e282e8e 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -41,6 +41,7 @@ add_library(reaper_reasampler MODULE ${REASAMPLER_SRC_DIR}/shell/actions/bank_actions.cpp ${REASAMPLER_SRC_DIR}/shell/actions/prune_action.cpp ${REASAMPLER_SRC_DIR}/shell/actions/ingest.cpp + ${REASAMPLER_SRC_DIR}/shell/actions/arrange_drop_win.cpp ${REASAMPLER_SRC_DIR}/shell/actions/drag_out_win.cpp ${REASAMPLER_SRC_DIR}/shell/actions/instrument_drop_win.cpp ${REASAMPLER_SRC_DIR}/shell/persist/usage_scan.cpp diff --git a/src/core/ui/CLAUDE.md b/src/core/ui/CLAUDE.md index e28b7f9..de448ba 100644 --- a/src/core/ui/CLAUDE.md +++ b/src/core/ui/CLAUDE.md @@ -76,14 +76,20 @@ L7 sub-pass, 2026-07-27): geometry modules; the kit's *draw* half is shell, its *geometry* half is pure, even where a WDL piece is reused. Look-and-feel work never touches capture, placement, or bank data ownership. -- **L7 drag-gesture precedence is a pure decision helper.** The rule — leave - client rect → OS drag-out; else drop on a tab/other bank → move/copy; else - same-bank grid → reorder-to-slot (empty slot = place, occupied + no modifier = - insert-before-and-shift, occupied + Alt = replace) — is "encoded in a pure - decision helper (mirror `drag_out::decideGesture`)"; the shell only reads live - pointer/focus/client-rect/modifier state and calls it, then maps the resolved - gesture to a cursor via `SetCursor`. No cue or precedence logic belongs in the - shell. +- **Drag-gesture precedence is a pure decision helper, on both sides of the client + rect.** Inside: drop on a tab/other bank → move/copy; else same-bank grid → + reorder-to-slot (empty slot = place, occupied + no modifier = + insert-before-and-shift, occupied + Alt = replace). Outside: `decideDropClass` + resolves the surface under the cursor. The shell only reads live + pointer/focus/client-rect/modifier state and calls these, then maps the resolved + cue to a cursor via `SetCursor`. No cue or precedence logic belongs in the shell. +- **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. A first-move class lock and a drag-lifetime "cannot hand off" latch both + existed here and were removed — do not reintroduce either. ## Modules @@ -91,7 +97,7 @@ 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` — pure OS drag-out module: gesture-boundary decision and path-list assembly. The `InstrumentDrop` gesture signals that the shell should execute an instrument-drop rather than a file-copy drag. +- `drag_out` — the pure drag-out gesture law plus path-list assembly. Owns the `ReaperSurface` vocabulary (OffReaper / TrackPanel / FxSurface / FxEmbed / Arrange / Other — `core/wire/instrument_drop` classifies REAPER's info token INTO it), `decideDropClass` (surface × single-vs-multi payload → Internal / InstrumentDrop / ArrangeInsert / Refuse / OsHandoff / None), and `cueForDropClass`. **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. - `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). @@ -119,9 +125,12 @@ L7 sub-pass, 2026-07-27): worked example. A prior revision wrote the threshold ~25% low and let 15px semibold clear a floor it was not entitled to. - `card_drag`'s precedence order must stay a pure decision helper mirroring - `drag_out::decideGesture` — don't let a shell reimplement gesture precedence + `drag_out::decideDropClass` — don't let a shell reimplement gesture precedence ad hoc; the cursor-cue mapping in the shell must stay a thin lookup over the pure result. +- `drag_out` is deliberately **dependency-free**, and `instrument_drop` links it + rather than the reverse. Inverting that edge would drag the instrument's state + serializer into `card_drag` and into every `drag_out` consumer. - `rect`'s prior role names survive only as `using` aliases at their old call sites — changing `rect.h` itself ripples across every directory that aliases it (e.g. `editor_geometry::Rect`); check all alias sites, not just this one. diff --git a/src/core/ui/drag_out.cpp b/src/core/ui/drag_out.cpp index d4d79df..a082960 100644 --- a/src/core/ui/drag_out.cpp +++ b/src/core/ui/drag_out.cpp @@ -15,14 +15,45 @@ bool insideClient(int px, int py, const PanelClientRect& c) { } // namespace -DragGesture decideGesture(int px, int py, const PanelClientRect& client, - const DragState& state) { - if (!state.dragging || !state.hasArmedSamples) return DragGesture::None; - if (insideClient(px, py, client)) return DragGesture::Internal; - // Outside the client: a single-capture drag still over REAPER's own UI is an instrument - // drop; anything else (multi-capture, or pointer off REAPER entirely) is an OS drag-out. - if (state.singleCapture && state.overReaperUi) return DragGesture::InstrumentDrop; - return DragGesture::OsDrag; +DropClass decideDropClass(int px, int py, const PanelClientRect& client, + const DropContext& ctx) { + if (!ctx.drag.dragging || !ctx.drag.hasArmedSamples) return DropClass::None; + if (insideClient(px, py, client)) return DropClass::Internal; + + switch (ctx.surface) { + case ReaperSurface::OffReaper: + return DropClass::OsHandoff; + + case ReaperSurface::TrackPanel: + case ReaperSurface::FxSurface: + // One instance holds one capture, so a multi payload names no instrument to build — + // it refuses with a cue rather than falling through to some other surface's outcome. + return (ctx.singlePayload && ctx.haveTrack) ? DropClass::InstrumentDrop + : DropClass::Refuse; + + case ReaperSurface::Arrange: + // GetThingFromPoint may return a null track with a valid info string (the SDK says + // so): over the arrange that is the region below the last track, which names no lane + // to place on. Refuse rather than guess a track. + return ctx.haveTrack ? DropClass::ArrangeInsert : DropClass::Refuse; + + case ReaperSurface::FxEmbed: + case ReaperSurface::Other: + return DropClass::Refuse; + } + return DropClass::Refuse; // an unclassifiable surface still refuses visibly, never silently +} + +DropCue cueForDropClass(DropClass cls) { + switch (cls) { + case DropClass::Internal: return DropCue::Internal; + case DropClass::InstrumentDrop: return DropCue::Instrument; + case DropClass::ArrangeInsert: return DropCue::ArrangeInsert; + case DropClass::Refuse: return DropCue::Refuse; + case DropClass::OsHandoff: return DropCue::OsOwned; + case DropClass::None: return DropCue::None; + } + return DropCue::None; } PathList assemblePathList(const std::vector& resolved) { diff --git a/src/core/ui/drag_out.h b/src/core/ui/drag_out.h index 02b0e24..afb2ab7 100644 --- a/src/core/ui/drag_out.h +++ b/src/core/ui/drag_out.h @@ -1,15 +1,15 @@ #pragma once #include "core/ui/rect.h" -// drag_out — decision logic behind the bank_panel's native OS drag-out. OLE/SWELL initiation and -// the panel gesture hook stay in the shell (drag_out_win.* + shell/panel/panel_drag.cpp). +// drag_out — the pure drag-out gesture law plus the OS hand-off's path-list assembly. REAPER +// hit-testing, cursor setting, and outcome execution stay in the shell (panel_drag.cpp + +// instrument_drop_win / arrange_drop_win / drag_out_win). // -// Gesture boundary: the panel's own internal drag (press a selected cell, drop onto a pool/bank -// region or tab) lives entirely inside the panel client rect. The moment the pointer LEAVES that -// rect while a drag is armed with samples, the gesture becomes OS-bound — dragged out to another -// window/Explorer/DAW. A single-capture drag that leaves the rect but is still over REAPER's own -// UI is instead an InstrumentDrop (heading for a track's FX button); do not regress this boundary. +// 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, and every REAPER-internal target executes natively on release. +// Do not reintroduce a first-move class lock or a drag-lifetime "blocked" latch. // -// Path-list assembly: turns armed sample ids into the absolute path list the OS drop carries +// Path-list assembly: turns armed sample ids into the absolute path list an OS drop carries // (Windows CF_HDROP / macOS file-list pasteboard) — set algebra only; the shell resolves each id // to its on-disk bank file. No temp files; copy-only is enforced at the OS layer (drag_out_win). @@ -18,34 +18,61 @@ namespace reasampler::ui { -// --- Gesture boundary --------------------------------------------------------- +// --- Gesture law -------------------------------------------------------------- // The panel's client rect, own client coords, top-left origin. Half-open: [x, x+width) x // [y, y+height). using PanelClientRect = Rect; -// Live drag state reduced to what the boundary decision needs. Pre-threshold "armed but not yet -// dragging" is not a drag for this decision. +// Live drag state reduced to what every gesture decision needs. Pre-threshold "armed but not +// yet dragging" is not a drag. Shared with card_drag's in-grid precedence decision. struct DragState { bool dragging = false; // threshold crossed; a drag is in progress bool hasArmedSamples = false; // payload holds >= 1 sample id - bool singleCapture = false; // payload holds EXACTLY one sample (arms InstrumentDrop) - bool overReaperUi = false; // pointer is over REAPER's own UI (shell-supplied) }; -// What the shell should do with the drag given the current pointer position. -enum class DragGesture { - None, // no drag under way, or an empty payload - Internal, // dragging inside the panel — bank-to-bank move/copy - InstrumentDrop, // single-capture drag left the panel but is over REAPER's UI — shell - // hover-tracks the TCP FX button; on release adds a preloaded instance - OsDrag, // dragging with samples, pointer left REAPER entirely — hand to the OS +// What REAPER reports under the pointer, reduced to the surfaces the law distinguishes. +// Produced from GetThingFromPoint's info token by wire::classifyReaperSurface. +enum class ReaperSurface { + OffReaper, // not over REAPER at all — the one irreversible exit + TrackPanel, // TCP/MCP, ANY sub-element: the WHOLE panel is the instrument hotspot + FxSurface, // fx_* — the FX chain and floating-FX windows + FxEmbed, // tcp.fxembed / mcp.fxembed — an instance already draws there + Arrange, // the timeline + Other, // ruler, transport, spacers, docker chrome, unknown future tokens }; -// Decides the gesture for a drag at pointer (px, py) over `client`, given `state`. Position-only -// + state-only (no hidden state), so re-entry back inside always returns Internal. -DragGesture decideGesture(int px, int py, const PanelClientRect& client, - const DragState& state); +// The resolved target class. Every value is a DEFINED outcome the shell executes or visibly +// refuses — there is deliberately no "nothing happens" member, which is what makes "no silent +// no-op release" a property of the enumeration rather than of any one call site. +enum class DropClass { + None, // no drag under way, or an empty payload — nothing to resolve + Internal, // inside the panel client — the bank-to-bank drag, unchanged + 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 +}; + +// Everything the law reads. Nothing here is remembered between evaluations: two evaluations at +// the same point with the same payload resolve identically, whatever happened in between. +struct DropContext { + DragState drag; + bool singlePayload = false; // payload holds EXACTLY one capture (arms InstrumentDrop) + ReaperSurface surface = ReaperSurface::OffReaper; // only read outside the client rect + bool haveTrack = false; // GetThingFromPoint returned a non-null MediaTrack* +}; + +// Resolves the class for a drag at pointer (px, py) over `client`. Position-and-context only, +// so re-entry into the client always returns Internal and a surface transition always reverses. +DropClass decideDropClass(int px, int py, const PanelClientRect& client, const DropContext& ctx); + +// The pointer cue for a class, so "will this work" is visible BEFORE release. Internal defers +// to card_drag's own cue; OsOwned means the OS drag loop draws the cursor and we must not fight +// it. +enum class DropCue { None, Internal, Instrument, ArrangeInsert, Refuse, OsOwned }; + +DropCue cueForDropClass(DropClass cls); // --- Path-list assembly ------------------------------------------------------- diff --git a/src/core/wire/CLAUDE.md b/src/core/wire/CLAUDE.md index e7efe59..97f74f7 100644 --- a/src/core/wire/CLAUDE.md +++ b/src/core/wire/CLAUDE.md @@ -81,7 +81,7 @@ This directory owns two cross-artifact contracts specifically: - `reasampler_uid.h` — SDK-free header owning the FOREVER-FROZEN VST3 class-UID integer macros (stable + beta pairs, `REASAMPLER_PROC_UID_*` / `REASAMPLER_PROC_UID_BETA_*`) and the `REASAMPLER_ACTIVE_UID_*` channel-selector macros. Split out of `reasampler_vst.h` so the pure extension side (`instrument_drop`) can derive the `.vstpreset` class-ID hex string without pulling in the VST3 SDK. Both `reasampler_vst.h` (runtime `FUID`) and `instrument_drop` (preset hex string) source from this single header — the binary identity and the preset-file identity cannot diverge. - `assignment_request` — pure ingest-assign wire: typed request record carrying the drop payload from the `ingest` shell through to the VST3 bridge. - `bake_wire` — the resample bake's request/outcome pair on ONE per-instance key (`rsbake_`): the instrument writes a `BakeRequest`, invokes the extension's action synchronously, and reads the extension's `BakeOutcome` back over the same key inside that one call. Not a handshake — a call and a return, and it must not grow a claim protocol. Also the ONE home of the bake action's command-id suffix and of the leading underscore `NamedCommandLookup` needs but `rec->Register("command_id", …)` does not, so both artifacts name one action. `BakeStatus` values are WIRE INTEGERS: never renumber, only append, and an unrecognized value decodes as `Failed` rather than as the numeric default `Ok`. -- `instrument_drop` — pure FX-drop payload builder: constructs a Steinberg-format `.vstpreset` image (channel-active class ID + the instrument's own component state, capture pre-selected) the shell applies via `TrackFX_SetPreset`; owns the `infoNamesFxHotspot` prefix classifier for `GetThingFromPoint` tokens. All-or-nothing contract — caller rolls back via `TrackFX_Delete` on any failure. +- `instrument_drop` — pure FX-drop payload builder: constructs a Steinberg-format `.vstpreset` image (channel-active class ID + the instrument's own component state, capture pre-selected) the shell applies via `TrackFX_SetPreset`; owns `classifyReaperSurface`, the prefix classifier mapping a `GetThingFromPoint` (info token, track-present) pair onto `core/ui/drag_out`'s `ReaperSurface`. Classifier ordering is load-bearing: the embed strip is matched before the `tcp`/`mcp` panel family, which now claims the WHOLE track panel rather than just its FX sub-elements. All-or-nothing contract — caller rolls back via `TrackFX_Delete` on any failure. - `sample_usage` — instance-usage wire: `UsageRecord`, `planUsagePublish` (fresh/heal/clean-replace/union/remint publish plan), `foldUsageRecords`/`usageHeldPaths` (liveness fold — protect-all when records exist but no instance is live; abort→protect-all on unreadable record; `counted` carries key-attributed live records), `identityMatches` (ReaSampler 9000 FX identity). REAPER-free, unit-tested. The mirror of `assignment_request` on the instrument→extension direction: the wire format and the two safety-critical decisions (what to write on publish, which records count at prune time) are pure so they are provable without a DAW. It lives here because it is a *wire format* with an instrument-side writer; the fold's output is consumed by `core/tracking`'s authority, which owns every consumer-facing decision built on it. ## Gotchas diff --git a/src/core/wire/CMakeLists.txt b/src/core/wire/CMakeLists.txt index c87e282..bb1964a 100644 --- a/src/core/wire/CMakeLists.txt +++ b/src/core/wire/CMakeLists.txt @@ -18,6 +18,9 @@ reasampler_test(sample_usage LINK sample_usage prune_reconcile) # parallel byte writer, so the cross-artifact contract cannot drift — hence the link to # component_state_io, which stays engine-free. The class-ID string derives from the frozen # UID macros, channel-selected via the generated version header, hence its include dir. -reasampler_pure_library(instrument_drop SOURCES instrument_drop.cpp LINK PUBLIC component_state_io) +# drag_out is the dependency-free owner of the ReaperSurface vocabulary this module classifies +# INTO; the edge points this way so drag_out (and card_drag through it) stays free of the +# serializer. +reasampler_pure_library(instrument_drop SOURCES instrument_drop.cpp LINK PUBLIC component_state_io drag_out) target_include_directories(instrument_drop PUBLIC ${PROJECT_BINARY_DIR}/generated) reasampler_test(instrument_drop LINK instrument_drop) diff --git a/src/core/wire/instrument_drop.cpp b/src/core/wire/instrument_drop.cpp index 9c3cfda..73f1a79 100644 --- a/src/core/wire/instrument_drop.cpp +++ b/src/core/wire/instrument_drop.cpp @@ -87,11 +87,16 @@ DropOutcome decideDropOutcome(const DropAttempt& attempt) { return out; } -bool infoNamesFxHotspot(const std::string& info) { - // See the header contract for the prefix rule and the embed-strip exclusion. +ui::ReaperSurface classifyReaperSurface(const std::string& info, bool haveTrack) { + // See the header contract for the prefix rule and the ordering it depends on. auto startsWith = [&info](const char* p) { return info.rfind(p, 0) == 0; }; - if (startsWith("tcp.fxembed") || startsWith("mcp.fxembed")) return false; - return startsWith("fx_") || startsWith("tcp.fx") || startsWith("mcp.fx"); + + if (info.empty()) return haveTrack ? ui::ReaperSurface::Other : ui::ReaperSurface::OffReaper; + if (startsWith("tcp.fxembed") || startsWith("mcp.fxembed")) return ui::ReaperSurface::FxEmbed; + if (startsWith("fx_")) return ui::ReaperSurface::FxSurface; + if (startsWith("tcp") || startsWith("mcp")) return ui::ReaperSurface::TrackPanel; + if (startsWith("arrange")) return ui::ReaperSurface::Arrange; + return ui::ReaperSurface::Other; } } // namespace reasampler::wire diff --git a/src/core/wire/instrument_drop.h b/src/core/wire/instrument_drop.h index 27b2296..845512c 100644 --- a/src/core/wire/instrument_drop.h +++ b/src/core/wire/instrument_drop.h @@ -1,7 +1,8 @@ #pragma once // instrument_drop — pure payload-construction core of drop-and-load: dropping a -// bank capture onto a track's FX surface instantiates ReaSampler 9000 on that -// track already playing that capture. No REAPER/SWELL/VST3 SDK/vendor includes +// 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 // (+ the pure sample_map it reuses and the SDK-free UID macros in // reasampler_uid.h); unit-tested outside the DAW. // @@ -25,6 +26,8 @@ #include #include +#include "core/ui/drag_out.h" // ReaperSurface — the surface vocabulary the gesture law reads + namespace reasampler::wire { // The 32-char uppercase-hex class-ID string of this build's channel-active @@ -52,16 +55,23 @@ std::vector buildVstPresetBytes(const std::string& classIdHex32, // preset. Deterministic. std::vector buildInstrumentDropPreset(const std::string& sampleId); -// Pure classifier for GetThingFromPoint's info string: is the point over a -// surface where an instrument drop should instantiate ReaSampler 9000? The -// SDK warns future versions may append information, so the rule is -// PREFIX-based: "fx_" (FX-chain/floating windows) or "tcp.fx"/"mcp.fx" (the -// TCP/MCP FX button + sibling elements) EXCEPT "tcp.fxembed"/"mcp.fxembed" — -// the embed-strip surface where an instance already draws; dropping there -// must not add a second instance. Bare "tcp"/"mcp" and non-FX sub-elements -// are not hotspots. The exact live token is DAW-only — confirm via +// Pure classifier for GetThingFromPoint's (info string, track-was-returned) pair +// into the surfaces the drag-out gesture law distinguishes. The SDK warns future +// versions may append information, so every rule is PREFIX-based: +// "tcp.fxembed*" / "mcp.fxembed*" -> FxEmbed. Checked FIRST: it is the surface +// an existing instance already draws on, and a drop must not stack a second. +// "fx_*" -> FxSurface (FX chain + floating FX windows). +// "tcp*" / "mcp*" -> TrackPanel — the WHOLE panel, every +// sub-element, not just the FX button. A TCP too narrow to draw that button +// still means "sampler on this track", and the old glyph-only rule is exactly +// why such a drop landed nowhere. +// "arrange*" -> Arrange. +// "" with no track -> OffReaper (the pointer left REAPER). +// anything else, incl. "" WITH a track -> Other. Over REAPER on a surface we +// cannot name: refuse visibly, never guess an outcome. +// The exact live token is DAW-only — confirm via // reaper.GetThingFromPoint(reaper.GetMousePosition()) in ReaScript if unsure. -bool infoNamesFxHotspot(const std::string& info); +ui::ReaperSurface classifyReaperSurface(const std::string& info, bool haveTrack); // The raw component-state bytes the preset carries, exposed so the round-trip // test can decode them back through sample_map::deserializeComponentState and diff --git a/src/shell/actions/CLAUDE.md b/src/shell/actions/CLAUDE.md index 77c1a2d..59a674a 100644 --- a/src/shell/actions/CLAUDE.md +++ b/src/shell/actions/CLAUDE.md @@ -4,8 +4,9 @@ The bindable action families routed through REAPER's `command_id`/`gaccel`/ `hookcommand` contract (Design View toggle actions, bank actions, the prune -action, and the shared registration plumbing/table), plus the OS drag-out and -FX-drop shells, plus the extension-side ingest-through-the-bank shell. This is +action, and the shared registration plumbing/table), plus the three drag-out +outcome shells (OS hand-off, instrument drop, arrange drop), plus the +extension-side ingest-through-the-bank shell. This is where user-facing REAPER actions and OS-level drag/drop live; the underlying mutation logic (bank verbs, prune's orphan computation, view-mode reconciliation) is owned by other directories and only skinned here. @@ -15,6 +16,10 @@ is owned by other directories and only skinned here. - **Ingest is an extension act; the instrument is a read-only bank consumer.** Any instrument code path that captures, imports, inserts a timeline item, or writes back into the bank is a bug — the instrument reads and plays only. +- **`arrange_drop_win` is the only timeline-placing shell in this directory**, and + it places because the USER dragged a card onto the arrange. Root `CLAUDE.md`'s + capture/placement separation forbids a CAPTURE placing an item; a deliberate drop + is placement on demand. No other module here may grow an `InsertMedia` call. - **Ingest NEVER inserts a timeline item.** Arrange capture→bank→assign reuses the existing capture add-path and assigns the resulting `Sample` id to the target instance; it never places anything on the timeline — capture/placement @@ -33,7 +38,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. **Every bank index verb wraps its mutation in a batched REAPER undo point (`Undo_BeginBlock2`/`EndBlock2`, `UNDO_STATE_MISCCFG`) so one bank operation is one Ctrl-Z.** 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. - `drag_out_win` — OS drag-out shell: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. -- `instrument_drop_win` — FX-button drop shell: resolves a screen point to a track + FX-surface hotspot, then 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.** +- `instrument_drop_win` — instrument-drop shell: `probeDropTarget` resolves a screen point to a track + a `ReaperSurface` (via the pure `classifyReaperSurface`, which owns every token rule), 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.** +- `arrange_drop_win` — the drag-out gesture's arrange outcome: `arrangeTimeAtScreenX` (pointer column → time via `GetSet_ArrangeView2` over a one-pixel span) and `performArrangeDrop` (snap the drop time, then one `InsertMedia` per capture on the pointer's track, laid out sequentially, in ONE undo block, with the caller's track selection and edit cursor restored). This is the one shell in this directory that DOES place timeline items — and it is legitimate because the user dragged them there, the same class of act as `RunInsertSelected`. It never captures and never writes the bank. - `ingest` — ingest-through-the-bank shell on the EXTENSION side: three surfaces — (1) arrange capture→bank→assign (bindable action), (2) Media-Explorer import→bank→instrument on the selected track, (3) file drop onto the bank panel→bank only. Only surface (1) writes the `assignment_request` ext-state wire. **ingest NEVER inserts a timeline item.** ## Gotchas diff --git a/src/shell/actions/arrange_drop_win.cpp b/src/shell/actions/arrange_drop_win.cpp new file mode 100644 index 0000000..b6c6cc9 --- /dev/null +++ b/src/shell/actions/arrange_drop_win.cpp @@ -0,0 +1,105 @@ +// arrange_drop_win.cpp — see arrange_drop_win.h. main.cpp owns the API pointers; this TU gets +// them extern via the WANT list. +// +// Runtime assumptions, all DAW-verifiable and none confirmed by the SDK header: +// A. InsertMedia base mode 0 targets the sole selected track and inserts at the edit cursor; +// SetOnlyTrackSelected isolates that track first (the same pair insert.cpp relies on). +// B. InsertMedia advances the edit cursor past the media it added. That advance IS the +// multi-file layout: the cursor is deliberately not reset between files, so N captures +// land end to end. If REAPER does not advance it, they stack at one position instead — +// visible and one Ctrl-Z away, never silent. +// C. SnapToGrid honors the project's snap-enabled toggle. The header documents no +// snap-enabled query for the arrange, so a drop taken with snapping OFF is the test that +// settles it. + +#include "shell/actions/arrange_drop_win.h" + +#include +#include + +#include "core/capture/insert_plan.h" // computeInsertMode — the ONE InsertMedia bitfield owner + +#include "reaper_plugin.h" + +#define REAPERAPI_MINIMAL +#define REAPERAPI_WANT_CountSelectedTracks +#define REAPERAPI_WANT_GetCursorPosition +#define REAPERAPI_WANT_GetSelectedTrack +#define REAPERAPI_WANT_GetSet_ArrangeView2 +#define REAPERAPI_WANT_InsertMedia +#define REAPERAPI_WANT_SetEditCurPos +#define REAPERAPI_WANT_SetOnlyTrackSelected +#define REAPERAPI_WANT_SetTrackSelected +#define REAPERAPI_WANT_SnapToGrid +#define REAPERAPI_WANT_Undo_BeginBlock2 +#define REAPERAPI_WANT_Undo_EndBlock2 +#include "reaper_plugin_functions.h" + +namespace reasampler { + +using capture::computeInsertMode; +using capture::InsertOptions; + +namespace { + +// Selection snapshot/restore, so a drop leaves the user's track selection exactly as it found +// it. Mirrors insert.cpp's pair; that TU belongs to the capture pillar and this one to the drag +// chain, so they are kept separate rather than sharing a helper across the seam. +std::vector snapshotSelectedTracks() { + const int n = CountSelectedTracks(nullptr); // nullptr = active project + std::vector tracks; + tracks.reserve(static_cast(n)); + for (int i = 0; i < n; ++i) tracks.push_back(GetSelectedTrack(nullptr, i)); + return tracks; +} + +void restoreSelectedTracks(const std::vector& tracks) { + if (tracks.empty()) return; // nothing was selected; leave whatever the drop selected + SetOnlyTrackSelected(tracks[0]); + for (std::size_t i = 1; i < tracks.size(); ++i) SetTrackSelected(tracks[i], true); +} + +} // namespace + +double arrangeTimeAtScreenX(int screenX) { + double start = 0.0, end = 0.0; + // isSet=false reads the arrange times spanned by [screen_x_start, screen_x_end); a + // one-pixel span therefore reads the time at that column. (The all-zero span is the SDK's + // documented "whole view" form, which is why the span here must not be empty.) + GetSet_ArrangeView2(nullptr, false, screenX, screenX + 1, &start, &end); + return start < 0.0 ? 0.0 : start; +} + +int performArrangeDrop(MediaTrack* track, double time, + const std::vector& absolutePaths) { + if (!track || absolutePaths.empty()) return 0; + + const std::vector priorSelection = snapshotSelectedTracks(); + const double priorCursor = GetCursorPosition(); + const int mode = computeInsertMode(InsertOptions{}); // current track, native length, no stretch + + // One undo block around the whole drop (every item plus the selection/cursor restore) so a + // single Ctrl-Z returns the project to exactly its pre-drop state. + Undo_BeginBlock2(nullptr); + + SetOnlyTrackSelected(track); + SetEditCurPos(SnapToGrid(nullptr, time), /*moveview=*/false, /*seekplay=*/false); + + int inserted = 0; + for (const std::string& path : absolutePaths) { + // No cursor reset between files — see assumption B in the file header. + InsertMedia(path.c_str(), mode); + ++inserted; + } + + restoreSelectedTracks(priorSelection); + SetEditCurPos(priorCursor, /*moveview=*/false, /*seekplay=*/false); + + // extraflags -1 = UNDO_STATE_ALL, matching the insert action's own block. + const std::string label = "ReaSampler: drop " + std::to_string(inserted) + + (inserted == 1 ? " capture" : " captures") + " onto arrange"; + Undo_EndBlock2(nullptr, label.c_str(), -1); + return inserted; +} + +} // namespace reasampler diff --git a/src/shell/actions/arrange_drop_win.h b/src/shell/actions/arrange_drop_win.h new file mode 100644 index 0000000..9cfd544 --- /dev/null +++ b/src/shell/actions/arrange_drop_win.h @@ -0,0 +1,30 @@ +#pragma once +// arrange_drop_win — the arrange outcome of the panel's drag-out gesture: places the dragged +// bank captures on the timeline at the track and time under the pointer. +// +// LOAD-BEARING: this is USER-INITIATED PLACEMENT, the same class of act as RunInsertSelected. +// Root CLAUDE.md's "capture and placement are separate acts" forbids a CAPTURE placing an item; +// a deliberate drop onto the timeline is placement on demand, and the user chose the spot. +// Nothing here captures or writes the bank. + +#include +#include + +// Declared as a class (matching reaper_plugin.h) so the mangled name agrees, without pulling +// in the SDK. +class MediaTrack; + +namespace reasampler { + +// The arrange time under a screen X, via REAPER's own pixel-column -> time mapping. Times left +// of project start clamp to 0. +double arrangeTimeAtScreenX(int screenX); + +// Places every path in `absolutePaths` on `track`, the first at `time` and each subsequent one +// after the previous (REAPER's own multi-file drop shape), inside ONE undo block. The drop time +// honors the project's snap setting. The caller's track selection and edit-cursor position are +// restored before returning. Returns the number of files handed to REAPER. +int performArrangeDrop(MediaTrack* track, double time, + const std::vector& absolutePaths); + +} // namespace reasampler diff --git a/src/shell/actions/instrument_drop_win.cpp b/src/shell/actions/instrument_drop_win.cpp index 7b3887f..44e3649 100644 --- a/src/shell/actions/instrument_drop_win.cpp +++ b/src/shell/actions/instrument_drop_win.cpp @@ -12,7 +12,7 @@ #include #include "core/version/app_version.h" // vstPluginName() — the CHANNEL-correct FX name (stable/beta pairing) -#include "core/wire/instrument_drop.h" // infoNamesFxHotspot — the PURE, unit-tested hotspot classifier +#include "core/wire/instrument_drop.h" // classifyReaperSurface — the PURE, unit-tested classifier #include "reaper_plugin.h" @@ -33,7 +33,7 @@ using version::vstPluginName; using wire::decideDropOutcome; using wire::DropAttempt; using wire::DropOutcome; -using wire::infoNamesFxHotspot; +using wire::classifyReaperSurface; namespace { @@ -76,17 +76,14 @@ std::filesystem::path writeTempPreset(const std::vector& bytes) { } // namespace -FxDropTarget resolveFxDropTarget(int screenX, int screenY) { - FxDropTarget out; +DropProbe probeDropTarget(int screenX, int screenY) { + DropProbe out; char info[256] = {0}; - // A non-empty info OR a non-null track means the point is over REAPER's own UI; - // a null track with empty info means the pointer has left REAPER entirely. - MediaTrack* track = GetThingFromPoint(screenX, screenY, info, sizeof(info)); - out.track = track; - out.overReaperUi = (track != nullptr) || (info[0] != '\0'); - // The hotspot is either the FX chain/floating window ("fx_*") OR the FX-button family of - // the track/mixer panel ("tcp.fx*"/"mcp.fx*"). The pure classifier owns the rule. - out.overFxHotspot = (track != nullptr) && infoNamesFxHotspot(info); + // GetThingFromPoint may return a null track together with a valid info string (its own + // doc-comment says so), so the track and the surface are two independent facts and both + // are reported. The pure classifier owns every token rule. + out.track = GetThingFromPoint(screenX, screenY, info, sizeof(info)); + out.surface = classifyReaperSurface(info, out.track != nullptr); return out; } diff --git a/src/shell/actions/instrument_drop_win.h b/src/shell/actions/instrument_drop_win.h index 5e4e675..8fc762d 100644 --- a/src/shell/actions/instrument_drop_win.h +++ b/src/shell/actions/instrument_drop_win.h @@ -9,33 +9,33 @@ // // LOAD-BEARING: an EXPLICIT user placement-of-the-player gesture — adds a READER of // the bank on a track, pointed at an already-captured sample. NEVER captures, NEVER -// writes the bank, NEVER inserts a timeline item. The only writes are a new FX -// instance + its component state, both wrapped in one undo block (one Ctrl-Z), plus -// a transient .vstpreset deleted before returning. +// writes the bank, NEVER inserts a timeline item (the drag's arrange outcome is a +// separate shell, arrange_drop_win). The only writes are a new FX instance + its +// component state, both wrapped in one undo block (one Ctrl-Z), plus a transient +// .vstpreset deleted before returning. #include #include +#include "core/ui/drag_out.h" // ReaperSurface + // Declared as a class (matching reaper_plugin.h) so the mangled name agrees, without // pulling in the SDK. class MediaTrack; namespace reasampler { -struct FxDropTarget { +// One evaluation of what sits under a screen point. Carries no verdict — the pure law +// (ui::decideDropClass) turns this plus the payload size into an outcome. +struct DropProbe { MediaTrack* track = nullptr; // the track under the pointer (null if none / not a track) - bool overReaperUi = false; // the point is over REAPER's own window/UI at all - bool overFxHotspot = false; // specifically over this track's FX button/chain surface - - bool valid() const { return track != nullptr && overFxHotspot; } + ui::ReaperSurface surface = ui::ReaperSurface::OffReaper; }; -// Wraps GetThingFromPoint, whose info string tells us what was hit ("tcp.fx*"/ -// "mcp.fx*" for the TCP/MCP FX button family; "fx_chain"/"fx_N" for the FX-chain and -// floating windows). `overReaperUi` is true when the point is over REAPER's own UI -// at all; `overFxHotspot` is true only for a genuine FX-bearing surface (decided by -// the pure instrument_drop::infoNamesFxHotspot). -FxDropTarget resolveFxDropTarget(int screenX, int screenY); +// Wraps GetThingFromPoint and hands its (info string, track) pair to the pure +// wire::classifyReaperSurface. Cheap enough to run on every mouse-move, but the panel +// evaluates it only OUTSIDE its own client rect — the internal drag never pays for it. +DropProbe probeDropTarget(int screenX, int screenY); // Adds a fresh ReaSampler 9000 instance to `track` and applies `presetBytes` as its // component state. Wraps add + apply in one REAPER undo block. All-or-nothing: if diff --git a/src/shell/panel/panel_drag.cpp b/src/shell/panel/panel_drag.cpp index aa5ffb5..ef2742c 100644 --- a/src/shell/panel/panel_drag.cpp +++ b/src/shell/panel/panel_drag.cpp @@ -1,10 +1,10 @@ // panel_drag.cpp — the card-drag / hover state machine seam of the docked bank panel: // WM_MOUSEMOVE (hover + tooltip timing + the live drag), drop-target/gesture // classification, cursor cues, button-up drop dispatch, and right-click menu routing. -// Its PURE mirror is core/ui/card_drag (gesture precedence + slot hit-test), with -// core/ui/drag_out owning the OS-drag boundary decision — this shell supplies only the -// live rects, modifier state, and side effects. Per-mouse-move work stays plain -// free-function calls — no interface, no virtual dispatch. +// Its PURE mirrors are core/ui/card_drag (in-grid precedence + slot hit-test) and +// core/ui/drag_out (the out-of-client gesture law) — this shell supplies only the live +// rects, modifier state, and side effects. Per-mouse-move work stays plain free-function +// calls — no interface, no virtual dispatch. // // Compiled into the reaper_reasampler MODULE. No REAPER API functions are called // directly here; REAPER SDK types arrive via panel_state.h. @@ -16,8 +16,9 @@ #include "shell/panel/panel_state.h" #include "shell/bank_ops/bank_ops.h" // persistBankOp — shared undo-block wrapper +#include "shell/actions/arrange_drop_win.h" // arrangeTimeAtScreenX / performArrangeDrop #include "shell/actions/drag_out_win.h" // OLE / SWELL drag-out initiation seam -#include "shell/actions/instrument_drop_win.h" // resolveFxDropTarget / performInstrumentDrop +#include "shell/actions/instrument_drop_win.h" // probeDropTarget / performInstrumentDrop namespace reasampler::panel { @@ -147,6 +148,88 @@ void applyDragCursor(CardGesture g) { SetCursor(LoadCursor(nullptr, idc)); } +// 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. +void applyDropCue(DropCue cue) { + const char* idc = nullptr; + switch (cue) { + case DropCue::Instrument: idc = IDC_HAND; break; + case DropCue::ArrangeInsert: idc = IDC_IBEAM; break; // an insertion point on a timeline + case DropCue::Refuse: idc = IDC_NO; break; + case DropCue::OsOwned: return; // the OS drag loop draws its own copy cursor + case DropCue::Internal: return; // applyDragCursor owns the in-client cue + case DropCue::None: return; + } + 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 — that is the whole point (a transition +// reverses, and an unresolvable evaluation cannot poison a later one). +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) { + RECT cr{}; + GetClientRect(g_panel.hwnd, &cr); + const PanelClientRect client{cr.left, cr.top, cr.right - cr.left, cr.bottom - cr.top}; + + DropContext ctx; + ctx.drag = DragState{/*dragging=*/true, /*hasArmedSamples=*/!g_panel.dragSampleIds.empty()}; + ctx.singlePayload = g_panel.dragSampleIds.size() == 1; + + LiveDrop out; + // The SDK hit-test is evaluated ONLY outside the client rect, so the common internal-drag + // path costs nothing. Unlike before, it runs for multi payloads too — still per-mouse-move + // cold, and it is what gives a multi drag a defined outcome on every surface. + if (!(x >= cr.left && x < cr.right && y >= cr.top && y < cr.bottom)) { + POINT sp{x, y}; + ClientToScreen(g_panel.hwnd, &sp); + const DropProbe probe = probeDropTarget(sp.x, sp.y); + ctx.surface = probe.surface; + ctx.haveTrack = probe.track != nullptr; + out.track = probe.track; + out.screenX = sp.x; + } + out.cls = decideDropClass(x, y, client, ctx); + 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 left REAPER, dragging back INTO a REAPER window +// 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. +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; + } + + if (GetCapture() == g_panel.hwnd) ReleaseCapture(); + resetDragState(); + invalidatePanel(); + + initiateDragOut(g_panel.hwnd, paths); // COPY-ONLY; blocking on Windows +} + // 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 // the one a click would hit). Returns HoverKind::None for the grid / dead space (the @@ -259,80 +342,26 @@ void onMouseMove(int x, int y) { } } if (g_panel.dragging) { - // Inside the client rect it stays the internal bank-to-bank drag. Once it LEAVES, - // drag_out::decideGesture splits three ways: single-capture over REAPER's OWN UI -> - // InstrumentDrop; multi-capture or fully outside REAPER -> OsDrag; inside -> Internal. - RECT cr{}; - GetClientRect(g_panel.hwnd, &cr); - const PanelClientRect client{cr.left, cr.top, cr.right - cr.left, cr.bottom - cr.top}; - const bool inside = (x >= cr.left && x < cr.right && y >= cr.top && y < cr.bottom); + // The class is re-resolved from scratch on every move — no first-move lock, nothing + // latched — so a transition in either direction always reverses, and drag speed cannot + // change where the gesture ends up. + const LiveDrop live = resolveLiveDrop(x, y); - DragState st{/*dragging=*/true, /*hasArmedSamples=*/!g_panel.dragSampleIds.empty()}; - st.singleCapture = (g_panel.dragSampleIds.size() == 1); - - // Only resolved OUTSIDE the client rect and for a single-capture payload, so the SDK - // hit-test costs nothing on the common internal-drag path. - FxDropTarget fx; - if (!inside && st.singleCapture) { - POINT sp{x, y}; - ClientToScreen(g_panel.hwnd, &sp); - fx = resolveFxDropTarget(sp.x, sp.y); - st.overReaperUi = fx.overReaperUi; - } - - const DragGesture gesture = decideGesture(x, y, client, st); - - if (gesture == DragGesture::InstrumentDrop) { - // Track the FX hotspot for the release. Unlike OsDrag this does NOT hand off to - // a modal OS loop, so the internal-drag capture stays alive; clear any bank - // drop-target highlight so the panel doesn't paint that cue too. - g_panel.instrumentDropTrack = fx.valid() ? fx.track : nullptr; + if (live.cls != DropClass::Internal) { + // Anything but the in-grid drag (including an empty payload, which resolves to + // None): clear the bank drop-target highlight so the panel does not paint a cue for + // a drop that is not going there, then show whatever cue this class carries. g_panel.dropKind = DropKind::None; g_panel.dropBankId.clear(); + if (live.cls == DropClass::OsHandoff) { + handOffToOs(); + return; + } + applyDropCue(cueForDropClass(live.cls)); invalidatePanel(); return; } - // Left InstrumentDrop territory (back inside, or over a non-FX area): drop the FX target. - g_panel.instrumentDropTrack = nullptr; - - if (gesture == DragGesture::OsDrag) { - if (g_panel.dragOsHandoffBlocked) { - // Already known un-hand-off-able for this gesture (empty/unresolvable payload, - // or the OS wasn't ready) — skip the fs::exists work and the readiness probe on - // every move; keep the drag alive with no drop-target highlight. - g_panel.dropKind = DropKind::None; - g_panel.dropBankId.clear(); - invalidatePanel(); - return; - } - - // Resolve the payload to existing on-disk paths BEFORE tearing down internal - // drag state (the resolver reads dragSourceBankId / dragSampleIds), then let the - // pure rule couple the two side effects: an unresolvable payload must leave the - // internal drag intact rather than wind it down for a hand-off that never runs — - // a half-torn-down drag reads to the user as "the drag did nothing, try again". - // canInitiateDragOut extends the same coupling to the OS-readiness failure modes - // (OLE unavailable, HDROP build failure) that the pure decision cannot see. - const std::vector paths = resolveDragPathsForOs(); - const ui::OsHandoff handoff = ui::decideOsHandoff(paths); - if (!handoff.handOffToOs || !canInitiateDragOut(paths)) { - g_panel.dragOsHandoffBlocked = true; - g_panel.dropKind = DropKind::None; - g_panel.dropBankId.clear(); - invalidatePanel(); - return; - } - - // DoDragDrop runs its own modal loop and takes over mouse capture, so the internal - // drag must be fully wound down first. - if (GetCapture() == g_panel.hwnd) ReleaseCapture(); - resetDragState(); - invalidatePanel(); - - initiateDragOut(g_panel.hwnd, paths); // COPY-ONLY; blocking on Windows - return; - } // Inside the client: classify the in-grid gesture (reorder/replace vs move/copy) and // reflect it as a cursor cue. updateDropTarget first so dropKind/dropBankId are // current for classifyCardDrag's same-vs-other-bank decision. @@ -369,6 +398,39 @@ void doReplaceDrop(const std::string& newId, const std::string& oldId, invalidatePanel(); } +// The in-client release: re-resolve the in-grid gesture at the drop point (modifiers may have +// changed since the last move) and commit it. Bank-to-bank semantics are unchanged. +void commitInternalDrop(int x, int y) { + RECT cr{}; + GetClientRect(g_panel.hwnd, &cr); + + updateDropTarget(x, y); + classifyCardDrag(x, y); + const CardGesture g = g_panel.cardGesture; + + if (g == CardGesture::Reorder) { + doReorderDrop(g_panel.dragPrimaryId, g_panel.dragSourceBankId, g_panel.dragTargetSlot); + } else if (g == CardGesture::Replace) { + // Replace targets the OCCUPANT of the target slot with the single grabbed card. + const bool isBanks = g_panel.dragSourceRegion == Region::Banks; + const int w = cr.right - cr.left, h = cr.bottom - cr.top; + const RECT region = isBanks ? banksRegionRect(w, h) : poolRegionRect(w, h); + const RegionDisplay disp = regionDisplay(region, isBanks, g_panel.dragSourceRegion); + const std::string occupant = disp.idAtSlot(g_panel.dragTargetSlot); + // Replace only makes sense for a single grabbed card over a DIFFERENT occupant. + if (!occupant.empty() && occupant != g_panel.dragPrimaryId) + doReplaceDrop(g_panel.dragPrimaryId, occupant, g_panel.dragSourceBankId); + } else if (g == CardGesture::Move || g == CardGesture::Copy) { + const std::string destId = dropTargetBankId(); + if (!destId.empty() && destId != g_panel.dragSourceBankId && + !g_panel.dragSampleIds.empty()) { + transferSamples(g_panel.dragSampleIds, g_panel.dragSourceBankId, destId, + /*copy=*/g == CardGesture::Copy); + } + } + // CardGesture::None -> a release over dead space or the source-bank gap: no bank change. +} + } // namespace // Clears all drag-state fields to their resting values. Called from every exit path @@ -382,75 +444,50 @@ void resetDragState() { g_panel.cardGesture = CardGesture::None; g_panel.dragTargetSlot = -1; g_panel.dragPrimaryId.clear(); - g_panel.instrumentDropTrack = nullptr; - g_panel.dragOsHandoffBlocked = false; } -// Commits (or abandons) a drag on button-up. The resolved pure CardGesture decides: -// * Reorder / Replace -> in-grid, within the source bank; one Ctrl-Z each. -// * Move / Copy -> the cross-bank transfer (Ctrl = copy). -// * None -> a drop over dead space / the source-bank gap = no-op. -// OsDragOut is never seen here: the pointer-left-client handoff happens live in onMouseMove. +// Commits (or abandons) a drag on button-up, over the class resolved AT THE RELEASE POINT. +// The switch is deliberately exhaustive with no default: every DropClass either performs its +// outcome or is an explicit, already-cued refusal, so a new class cannot be added without +// answering "what does releasing here do?". void onLBtnUp(int x, int y) { if (g_panel.dragging) { - // Drop-and-load: a release over a valid FX hotspot instantiates a ReaSampler 9000 on - // that track preloaded with the dragged capture — NOT a bank move, NOT an OS drag, - // NEVER a timeline insert. Takes priority over the in-grid / cross-bank drop. - // Single-capture only, so dragSampleIds.front() is the capture. - // - // Re-resolve at the RELEASE point rather than trusting only the hover-tracked target: - // WM_MOUSEMOVE is coalesced, so a fast drag onto a dense surface (an FX chain row, a - // container) can release over a hotspot no processed move ever reported. Gated on - // !inside exactly like onMouseMove's live resolve — GetThingFromPoint can return a - // track+FX hit at a release point that is still inside the panel's own client rect, and - // an in-grid release must always go through the reorder/replace/move/copy path below, - // never be reinterpreted as an FX add. Strictly additive otherwise — a release-point - // miss falls back to the tracked target, so the hover-then-release-on-the-FX-button - // path is untouched. - const bool singleCapture = g_panel.dragSampleIds.size() == 1; - MediaTrack* dropTrack = g_panel.instrumentDropTrack; - RECT cr{}; - GetClientRect(g_panel.hwnd, &cr); - const bool inside = (x >= cr.left && x < cr.right && y >= cr.top && y < cr.bottom); - if (!inside && singleCapture) { - POINT sp{x, y}; - ClientToScreen(g_panel.hwnd, &sp); - const FxDropTarget fx = resolveFxDropTarget(sp.x, sp.y); - if (fx.valid()) dropTrack = fx.track; - } - if (!inside && dropTrack && singleCapture) { - const std::string sampleId = g_panel.dragSampleIds.front(); - performInstrumentDrop(dropTrack, buildInstrumentDropPreset(sampleId)); - // Read-only over the bank + arrange: the only mutations are the new FX instance + - // its state (both undoable in performInstrumentDrop). - } else { - updateDropTarget(x, y); - classifyCardDrag(x, y); // re-resolve at the drop point (modifiers may have changed) - const CardGesture g = g_panel.cardGesture; + // Resolve at the release point, not from anything the moves remembered: WM_MOUSEMOVE is + // coalesced, so the last processed move can sit well away from where the button actually + // came up, and the release point is the user's stated target. + const LiveDrop live = resolveLiveDrop(x, y); - if (g == CardGesture::Reorder) { - doReorderDrop(g_panel.dragPrimaryId, g_panel.dragSourceBankId, - g_panel.dragTargetSlot); - } else if (g == CardGesture::Replace) { - // Replace targets the OCCUPANT of the target slot with the single grabbed card. - const bool isBanks = g_panel.dragSourceRegion == Region::Banks; - const int w = cr.right - cr.left, h = cr.bottom - cr.top; - const RECT region = isBanks ? banksRegionRect(w, h) : poolRegionRect(w, h); - const RegionDisplay disp = regionDisplay(region, isBanks, g_panel.dragSourceRegion); - const std::string occupant = disp.idAtSlot(g_panel.dragTargetSlot); - // Replace only makes sense for a single grabbed card over a DIFFERENT occupant. - if (!occupant.empty() && occupant != g_panel.dragPrimaryId) - doReplaceDrop(g_panel.dragPrimaryId, occupant, g_panel.dragSourceBankId); - } else if (g == CardGesture::Move || g == CardGesture::Copy) { - const std::string destId = dropTargetBankId(); - if (!destId.empty() && destId != g_panel.dragSourceBankId && - !g_panel.dragSampleIds.empty()) { - transferSamples(g_panel.dragSampleIds, g_panel.dragSourceBankId, destId, - /*copy=*/g == CardGesture::Copy); - } + switch (live.cls) { + case DropClass::InstrumentDrop: { + // Adds a ReaSampler 9000 on that track preloaded with the capture — NOT a bank + // move, NOT an OS drag, NEVER a timeline insert. singlePayload is what armed + // this class, so front() IS the capture. + const std::string sampleId = g_panel.dragSampleIds.front(); + performInstrumentDrop(live.track, buildInstrumentDropPreset(sampleId)); + break; } + + case DropClass::ArrangeInsert: + // The one branch here that places timeline items, and legitimately so — see + // arrange_drop_win.h for why a deliberate drop is not a capture auto-insert. + performArrangeDrop(live.track, arrangeTimeAtScreenX(live.screenX), + resolveDragPathsForOs()); + break; + + case DropClass::Internal: + commitInternalDrop(x, y); + break; + + case DropClass::Refuse: + case DropClass::OsHandoff: + case DropClass::None: + // Nothing to perform, and nothing silent about it: the refuse cursor has been + // showing since the move that resolved this class. An OsHandoff reaching release + // means the payload never resolved — a live hand-off consumes the drag inside + // DoDragDrop's modal loop and never returns here. + break; } - // CardGesture::None -> no-op drop (dead space, or same-bank gap resolved to None). + SetCursor(LoadCursor(nullptr, IDC_ARROW)); // restore the arrow on drop if (GetCapture() == g_panel.hwnd) ReleaseCapture(); } else if (g_panel.dragArmed) { diff --git a/src/shell/panel/panel_state.h b/src/shell/panel/panel_state.h index 2187c2f..61c90d5 100644 --- a/src/shell/panel/panel_state.h +++ b/src/shell/panel/panel_state.h @@ -71,9 +71,11 @@ using ui::CardGesture; using ui::CellRect; using ui::ClusterSpec; using ui::CursorCue; -using ui::DragGesture; using ui::DragModifiers; using ui::DragState; +using ui::DropClass; +using ui::DropContext; +using ui::DropCue; using ui::DropRegion; using ui::FooterBarLayout; using ui::FooterBarSpec; @@ -118,9 +120,10 @@ using ui::computeSlotRectsForDrop; using ui::computeTabRects; using ui::computeTabStripLayout; using ui::computeTooltip; +using ui::cueForDropClass; using ui::cursorForGesture; using ui::decideCardGesture; -using ui::decideGesture; +using ui::decideDropClass; using ui::formatBarsBeats; using ui::formatSecondsMs; using ui::hitTestActionBar; @@ -317,14 +320,10 @@ struct PanelState { CardGesture cardGesture = CardGesture::None; int dragTargetSlot = -1; - // While a single-capture drag is over REAPER's own UI, heading for a track's TCP FX - // button: on release this adds a ReaSampler 9000 preloaded with the capture. Null - // when the pointer is not over an FX button. - MediaTrack* instrumentDropTrack = nullptr; - - // Latched once an OsDrag hand-off resolves "cannot hand off" for this gesture — skips - // re-running resolveDragPathsForOs (fs::exists per sample) each move. Cleared by resetDragState. - bool dragOsHandoffBlocked = false; + // NO out-of-client drop-target state is kept here on purpose. The drag-out class and its + // track are re-resolved from the live pointer on every move and again at the release point, + // which is what makes a class transition reversible and one unresolvable evaluation + // harmless to the next. Do not reintroduce a remembered target or a "blocked" latch. // Authoritative tail setting lives in ReaSamplerSession, not here; panel reads it for // drawing and mutates via footer click / scroll-wheel. bankPanelTailSetting is the diff --git a/tests/test_drag_out.cpp b/tests/test_drag_out.cpp index 1a15a7b..bc855c2 100644 --- a/tests/test_drag_out.cpp +++ b/tests/test_drag_out.cpp @@ -1,13 +1,14 @@ -// Standalone tests for reasampler::drag_out — no REAPER, no test framework. Same fast loop -// as the sibling pure tests (mode_switch et al.): assert the gesture- -// boundary decision and the path-list assembly directly. +// Standalone tests for reasampler::ui::drag_out — no REAPER, no test framework. Same fast loop +// as the sibling pure tests: assert the gesture law and the path-list assembly directly. // -// Covers (M11 drag-out brief §test cases): -// * Gesture boundary: inside-panel drag stays Internal; leaving the client area with -// armed samples -> OsDrag; no armed samples (or not dragging) -> None; half-open edge -// behavior; re-entry back inside returns to Internal (position-only decision). -// * Path-list assembly: single, multi, dedupe (cross-bank copy case), skip-missing, -// skip-unresolved, empty selection, order preservation, mixed tallies. +// Covers: +// * The full class matrix: every ReaperSurface x {single, multi} x {track, no track}, inside +// and outside the client rect, plus the half-open edge and a non-zero panel origin. +// * 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. +// * Exhaustiveness: no surface resolves to a do-nothing class, and every class has a cue. +// * Path-list assembly + OS hand-off ordering (unchanged from before this law). #include "../src/core/ui/drag_out.h" @@ -22,115 +23,253 @@ static int g_fail = 0; #define CHECK(cond) do { if(!(cond)) { \ std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) -// --- Gesture boundary --------------------------------------------------------- +// --- Fixtures ----------------------------------------------------------------- static const PanelClientRect kPanel{0, 0, 400, 300}; -// A drag with samples, pointer well inside the client rect -> the existing internal drag -// (invariant #4: inside-panel drag stays internal, unchanged). -static void testInsidePanelStaysInternal() { - DragState s{/*dragging=*/true, /*hasArmedSamples=*/true}; - CHECK(decideGesture(200, 150, kPanel, s) == DragGesture::Internal); - CHECK(decideGesture(0, 0, kPanel, s) == DragGesture::Internal); // top-left corner - CHECK(decideGesture(399, 299, kPanel, s) == DragGesture::Internal); // last inside px +// A live single-card drag over `surface`, with a track resolved unless stated otherwise. +static DropContext ctx(ReaperSurface surface, bool single = true, bool haveTrack = true) { + DropContext c; + c.drag = DragState{/*dragging=*/true, /*hasArmedSamples=*/true}; + c.singlePayload = single; + c.surface = surface; + c.haveTrack = haveTrack; + return c; } -// A drag with samples whose pointer has left the client rect (any edge) -> OS drag. -static void testLeavingClientAreaIsOsDrag() { - DragState s{/*dragging=*/true, /*hasArmedSamples=*/true}; - CHECK(decideGesture(-1, 150, kPanel, s) == DragGesture::OsDrag); // left of panel - CHECK(decideGesture(400, 150, kPanel, s) == DragGesture::OsDrag); // right edge (x+w) - CHECK(decideGesture(200, -5, kPanel, s) == DragGesture::OsDrag); // above - CHECK(decideGesture(200, 300, kPanel, s) == DragGesture::OsDrag); // below (y+h) - CHECK(decideGesture(1000, 1000, kPanel, s) == DragGesture::OsDrag);// far outside +// Every surface the law enumerates, so the matrix tests iterate rather than list. +static const ReaperSurface kAllSurfaces[] = { + ReaperSurface::OffReaper, ReaperSurface::TrackPanel, ReaperSurface::FxSurface, + ReaperSurface::FxEmbed, ReaperSurface::Arrange, ReaperSurface::Other, +}; + +// A point comfortably outside the panel client rect. +static const int kOutX = 500, kOutY = 150; + +// --- Matrix: inside the client ------------------------------------------------- + +// Inside the client the drag is the bank-to-bank drag, whatever REAPER reports underneath and +// whatever the payload size — the internal path must never be reinterpreted as an FX add or a +// timeline insert. This is the bank-to-bank regression floor. +static void testInsideClientIsAlwaysInternal() { + for (ReaperSurface s : kAllSurfaces) { + for (bool single : {true, false}) { + 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); + } + } } -// The half-open boundary: x+width and y+height are OUTSIDE (OsDrag), the pixel just inside -// is Internal — matches the panel's other hit-tests so the edge is claimed consistently. +// The half-open boundary: x+width and y+height are OUTSIDE, the pixel just inside is Internal — +// matches the panel's other hit-tests so the edge is claimed consistently. static void testBoundaryHalfOpen() { - DragState s{true, true}; - CHECK(decideGesture(399, 150, kPanel, s) == DragGesture::Internal); - CHECK(decideGesture(400, 150, kPanel, s) == DragGesture::OsDrag); - CHECK(decideGesture(200, 299, kPanel, s) == DragGesture::Internal); - CHECK(decideGesture(200, 300, kPanel, s) == DragGesture::OsDrag); + const DropContext off = ctx(ReaperSurface::OffReaper); + CHECK(decideDropClass(399, 150, kPanel, off) == DropClass::Internal); + CHECK(decideDropClass(400, 150, kPanel, off) == DropClass::OsHandoff); + CHECK(decideDropClass(200, 299, kPanel, off) == DropClass::Internal); + CHECK(decideDropClass(200, 300, kPanel, off) == DropClass::OsHandoff); } -// No armed samples -> None regardless of position (an empty-payload drag never goes to the -// OS). Not dragging -> None even with samples (the shell asks only mid-drag, but the guard -// is explicit). -static void testNoDragOrNoSamplesIsNone() { - CHECK(decideGesture(1000, 1000, kPanel, DragState{true, false}) == DragGesture::None); - CHECK(decideGesture(200, 150, kPanel, DragState{true, false}) == DragGesture::None); - CHECK(decideGesture(1000, 1000, kPanel, DragState{false, true}) == DragGesture::None); - CHECK(decideGesture(200, 150, kPanel, DragState{false, false}) == DragGesture::None); -} - -// Re-entry: the decision is position-only, so a pointer that left (OsDrag) and came back -// inside reads Internal again. (The shell, having handed off to the modal OS loop, simply -// stops asking — but the pure function must not be stateful.) -static void testReentryReturnsInternal() { - DragState s{true, true}; - CHECK(decideGesture(500, 150, kPanel, s) == DragGesture::OsDrag); // left - CHECK(decideGesture(200, 150, kPanel, s) == DragGesture::Internal); // re-entered -} - -// A non-zero panel origin (the client rect need not sit at 0,0) — the boundary tracks the -// rect, not the absolute axes. +// A non-zero panel origin — the boundary tracks the rect, not the absolute axes. static void testOffsetPanelRect() { - PanelClientRect p{50, 20, 100, 80}; // spans x[50,150) y[20,100) - DragState s{true, true}; - CHECK(decideGesture(100, 60, p, s) == DragGesture::Internal); - CHECK(decideGesture(49, 60, p, s) == DragGesture::OsDrag); // just left of origin - CHECK(decideGesture(150, 60, p, s) == DragGesture::OsDrag); // x+width - CHECK(decideGesture(100, 19, p, s) == DragGesture::OsDrag); // just above origin + const PanelClientRect p{50, 20, 100, 80}; // spans x[50,150) y[20,100) + const DropContext off = ctx(ReaperSurface::OffReaper); + CHECK(decideDropClass(100, 60, p, off) == DropClass::Internal); + CHECK(decideDropClass(49, 60, p, off) == DropClass::OsHandoff); + CHECK(decideDropClass(150, 60, p, off) == DropClass::OsHandoff); + CHECK(decideDropClass(100, 19, p, off) == DropClass::OsHandoff); } -// --- S17 InstrumentDrop gesture (single-capture over REAPER UI) --------------- -// -// M11 REGRESSION GUARD (load-bearing): every M11 case above uses DragState{true, true}, -// which leaves singleCapture=overReaperUi=false — so an M11-era payload outside the client -// rect still decides OsDrag exactly as before. The tests above ARE the M11 non-regression -// proof; these add the new middle case. +// --- Matrix: outside the client, single card ----------------------------------- -// A SINGLE-capture drag that has left the panel but is still over REAPER's own UI is an -// instrument drop (heading for a track's FX button), NOT an OS drag. -static void testSingleCaptureOverReaperUiIsInstrumentDrop() { - DragState s{/*dragging=*/true, /*hasArmedSamples=*/true, - /*singleCapture=*/true, /*overReaperUi=*/true}; - CHECK(decideGesture(500, 150, kPanel, s) == DragGesture::InstrumentDrop); // right of panel - CHECK(decideGesture(-5, 150, kPanel, s) == DragGesture::InstrumentDrop); // left of panel - CHECK(decideGesture(200, 400, kPanel, s) == DragGesture::InstrumentDrop); // below +// A single card over a track's panel loads the instrument — the WHOLE panel, which is the +// root-cause fix: a TCP too narrow to draw the FX button used to yield a cue-less no-op. +static void testSingleOverTrackPanelIsInstrumentDrop() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::TrackPanel)) == + DropClass::InstrumentDrop); + CHECK(decideDropClass(-5, kOutY, kPanel, ctx(ReaperSurface::TrackPanel)) == + DropClass::InstrumentDrop); + CHECK(decideDropClass(200, 400, kPanel, ctx(ReaperSurface::TrackPanel)) == + DropClass::InstrumentDrop); } -// InstrumentDrop is an OUTSIDE-only refinement: the same single-capture state INSIDE the -// client rect is still the unchanged Internal bank-to-bank drag (invariant #4). -static void testSingleCaptureInsidePanelStaysInternal() { - DragState s{true, true, /*singleCapture=*/true, /*overReaperUi=*/true}; - CHECK(decideGesture(200, 150, kPanel, s) == DragGesture::Internal); +// The FX chain / floating-FX windows keep their existing outcome. +static void testSingleOverFxSurfaceIsInstrumentDrop() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxSurface)) == + DropClass::InstrumentDrop); } -// A single-capture drag that has left REAPER ENTIRELY (overReaperUi=false) falls through to -// OsDrag — the M11 OS drag-out to Explorer/another DAW, unchanged. This is the boundary -// refinement's other half: leaving the client rect no longer immediately means OS-bound. -static void testSingleCaptureOffReaperIsOsDrag() { - DragState s{true, true, /*singleCapture=*/true, /*overReaperUi=*/false}; - CHECK(decideGesture(500, 150, kPanel, s) == DragGesture::OsDrag); +// The embed strip is where an instance ALREADY draws: dropping there must not stack a second, +// so it refuses rather than instantiating. +static void testSingleOverFxEmbedRefuses() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxEmbed)) == + DropClass::Refuse); } -// A MULTI-capture drag over REAPER's UI is REJECTED for InstrumentDrop (the S17 open-question -// lean): it is NOT a single instrument placement, so it falls through to OsDrag even while -// over REAPER's UI — the multi-file drag-out is the natural gesture for a multi payload. -static void testMultiCaptureOverReaperUiIsOsDrag() { - DragState s{true, true, /*singleCapture=*/false, /*overReaperUi=*/true}; - CHECK(decideGesture(500, 150, kPanel, s) == DragGesture::OsDrag); +// THE HEADLINE DEFECT: a single card over the arrange places a timeline item. It used to lock +// to InstrumentDrop on the first move outside the client and then release into nothing. +static void testSingleOverArrangeIsArrangeInsert() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::Arrange)) == + DropClass::ArrangeInsert); } -// Not-dragging / no-armed-samples still short-circuits to None regardless of the S17 fields. -static void testS17FieldsIgnoredWhenNotDragging() { - CHECK(decideGesture(500, 150, kPanel, - DragState{false, true, true, true}) == DragGesture::None); - CHECK(decideGesture(500, 150, kPanel, - DragState{true, false, true, true}) == DragGesture::None); +// Ruler / transport / docker chrome / any token REAPER adds later: a defined refusal. +static void testSingleOverOtherReaperUiRefuses() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::Other)) == DropClass::Refuse); +} + +// Off REAPER entirely -> the OS drag-out, the one irreversible transition. +static void testSingleOffReaperIsOsHandoff() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::OffReaper)) == + DropClass::OsHandoff); +} + +// --- Matrix: outside the client, multi card ------------------------------------ + +// A multi payload names no single instrument, so both instrument surfaces refuse — a DEFINED +// outcome with a cue, where the old law silently took the OS path from these surfaces. +static void testMultiOverInstrumentSurfacesRefuses() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::TrackPanel, false)) == + DropClass::Refuse); + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxSurface, false)) == + DropClass::Refuse); + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::FxEmbed, false)) == + DropClass::Refuse); +} + +// Multi over the arrange still places — one item per capture; the shell lays them out. +static void testMultiOverArrangeIsArrangeInsert() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::Arrange, false)) == + DropClass::ArrangeInsert); +} + +// Multi off REAPER is the classic multi-file drag-out, unchanged. +static void testMultiOffReaperIsOsHandoff() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::OffReaper, false)) == + DropClass::OsHandoff); +} + +static void testMultiOverOtherReaperUiRefuses() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::Other, false)) == + DropClass::Refuse); +} + +// --- The documented null-track / non-empty-info case --------------------------- + +// GetThingFromPoint "may return NULL with valid info string to indicate non-track thing". Both +// outcomes that need a track therefore refuse instead of dereferencing nothing: over the arrange +// that is the empty region below the last track, and over a track panel it is a surface we +// cannot attribute. +static void testNullTrackWithSurfaceRefuses() { + for (ReaperSurface s : {ReaperSurface::TrackPanel, ReaperSurface::FxSurface, + ReaperSurface::Arrange}) { + for (bool single : {true, false}) { + CHECK(decideDropClass(kOutX, kOutY, kPanel, + ctx(s, single, /*haveTrack=*/false)) == DropClass::Refuse); + } + } +} + +// A track under the pointer never turns OffReaper into a REAPER-internal outcome: OffReaper is +// the shell's "the info string was empty and there was no track" verdict, and the law trusts it. +static void testOffReaperIgnoresTrackFlag() { + CHECK(decideDropClass(kOutX, kOutY, kPanel, + ctx(ReaperSurface::OffReaper, true, false)) == DropClass::OsHandoff); +} + +// --- Not-a-drag ---------------------------------------------------------------- + +// No armed samples, or not dragging -> None regardless of position or surface. +static void testNoDragOrNoSamplesIsNone() { + for (ReaperSurface s : kAllSurfaces) { + DropContext c = 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); + + c.drag = DragState{/*dragging=*/false, /*hasArmedSamples=*/true}; + CHECK(decideDropClass(kOutX, kOutY, kPanel, c) == DropClass::None); + CHECK(decideDropClass(200, 150, kPanel, c) == DropClass::None); + } +} + +// --- Reversibility, speed-independence, exhaustiveness ------------------------- + +// The transition sequence from the acceptance criteria: client -> arrange -> FX window -> +// arrange -> client. Every step resolves on its own terms, and the return leg reproduces the +// outgoing leg exactly — the instrument drop is still available after crossing the arrange, and +// re-entering the client resumes the internal drag. +static void testClassTransitionsAreReversible() { + const DropContext arrange = ctx(ReaperSurface::Arrange); + const DropContext fx = ctx(ReaperSurface::FxSurface); + const DropContext inside = ctx(ReaperSurface::Other); // surface is ignored inside + + CHECK(decideDropClass(200, 150, kPanel, inside) == DropClass::Internal); + CHECK(decideDropClass(kOutX, kOutY, kPanel, arrange) == DropClass::ArrangeInsert); + CHECK(decideDropClass(kOutX, kOutY, kPanel, fx) == DropClass::InstrumentDrop); + CHECK(decideDropClass(kOutX, kOutY, kPanel, arrange) == DropClass::ArrangeInsert); + CHECK(decideDropClass(200, 150, kPanel, inside) == DropClass::Internal); +} + +// One unresolvable evaluation (a surface with no track, which refuses) followed by a resolvable +// one: the second resolves exactly as it would have on its own. This is the property the retired +// drag-lifetime "blocked" latch violated. +static void testUnresolvableEvaluationDoesNotAffectTheNext() { + const DropContext trackless = ctx(ReaperSurface::Arrange, true, /*haveTrack=*/false); + const DropContext resolvable = ctx(ReaperSurface::Arrange); + const DropClass standalone = decideDropClass(kOutX, kOutY, kPanel, resolvable); + + CHECK(decideDropClass(kOutX, kOutY, kPanel, trackless) == DropClass::Refuse); + CHECK(decideDropClass(kOutX, kOutY, kPanel, resolvable) == standalone); + CHECK(standalone == DropClass::ArrangeInsert); + + // And in the other order — the refusal is equally uninfluenced by what preceded it. + CHECK(decideDropClass(kOutX, kOutY, kPanel, trackless) == DropClass::Refuse); +} + +// Drag speed only changes WHICH intermediate points get evaluated. Since the class is a function +// of the current point and context alone, a "fast flick" (one evaluation at the release point) +// and a "slow drag" (many evaluations ending at the same point) agree at that point — with the +// intermediate surfaces deliberately chosen to disagree with the destination. +static void testDragSpeedCannotChangeTheOutcome() { + const DropContext destination = ctx(ReaperSurface::TrackPanel); + const DropClass flick = decideDropClass(kOutX, kOutY, kPanel, destination); + + // The slow path crosses everything else first. + for (ReaperSurface s : kAllSurfaces) { + (void)decideDropClass(kOutX - 10, kOutY, kPanel, ctx(s)); + (void)decideDropClass(200, 150, kPanel, ctx(s)); // and back through the client + } + CHECK(decideDropClass(kOutX, kOutY, kPanel, destination) == flick); + CHECK(flick == DropClass::InstrumentDrop); +} + +// EXHAUSTIVENESS (acceptance criterion 7, structural rather than spot-checked): for a live drag +// outside the client, no surface x payload x track combination resolves to None — i.e. there is +// no cell whose release does nothing without having said so. None is reachable only from a dead +// drag, which is asserted separately above. +static void testNoLiveOutsideCombinationResolvesToNone() { + for (ReaperSurface s : kAllSurfaces) { + for (bool single : {true, false}) { + for (bool haveTrack : {true, false}) { + const DropClass c = decideDropClass(kOutX, kOutY, kPanel, ctx(s, single, haveTrack)); + CHECK(c != DropClass::None); + CHECK(c != DropClass::Internal); + } + } + } +} + +// Every class carries a cue, and only the two the shell deliberately does not draw map to a +// no-cursor cue — so "will this work" is answerable before release for every resolvable class. +static void testEveryClassHasACue() { + CHECK(cueForDropClass(DropClass::Internal) == DropCue::Internal); + CHECK(cueForDropClass(DropClass::InstrumentDrop) == DropCue::Instrument); + CHECK(cueForDropClass(DropClass::ArrangeInsert) == DropCue::ArrangeInsert); + CHECK(cueForDropClass(DropClass::Refuse) == DropCue::Refuse); + CHECK(cueForDropClass(DropClass::OsHandoff) == DropCue::OsOwned); + CHECK(cueForDropClass(DropClass::None) == DropCue::None); } // --- Path-list assembly ------------------------------------------------------- @@ -158,7 +297,7 @@ static void testMultiPreservesOrder() { } // Two index entries resolving to the SAME file (the cross-bank copy case — one file, two -// entries) yield ONE CF_HDROP path; the extra is counted, first occurrence wins. +// entries) yield ONE path; the extra is counted, first occurrence wins. static void testDedupeSamePath() { PathList l = assemblePathList({ok("x.wav"), ok("y.wav"), ok("x.wav")}); CHECK(l.paths.size() == 2); @@ -167,8 +306,7 @@ static void testDedupeSamePath() { CHECK(l.skippedDuplicate == 1); } -// A stale index entry (file gone from disk) is skipped — never a dangling path on the OS -// clipboard. +// A stale index entry (file gone from disk) is skipped — never a dangling path handed onward. static void testSkipMissing() { PathList l = assemblePathList({ok("a.wav"), missing("gone.wav"), ok("b.wav")}); CHECK(l.paths.size() == 2); @@ -227,9 +365,8 @@ static void testMixedTallies() { // (release capture, clear drag state) BEFORE checking whether the payload had resolved to any // on-disk file. For an unresolvable payload, initiateDragOut is never reached — no OS drop // happens at all — but the teardown ran anyway, so the drag just silently stopped mid-gesture -// with no highlight and no drop. The user has to press and start an entirely new drag; retrying -// the SAME stale selection resolves to the same empty payload and fails identically. These pin -// the fix: the two side effects (teardown, hand-off) are one decision. +// with no highlight and no drop. These pin the fix: the two side effects (teardown, hand-off) +// are one decision. // Nothing draggable -> hand off nothing AND keep the internal drag alive. static void testEmptyPathsHandsOffNothingAndKeepsInternalDrag() { @@ -254,18 +391,31 @@ static void testAllSkippedSelectionKeepsInternalDrag() { } int main() { - testInsidePanelStaysInternal(); - testLeavingClientAreaIsOsDrag(); + testInsideClientIsAlwaysInternal(); testBoundaryHalfOpen(); - testNoDragOrNoSamplesIsNone(); - testReentryReturnsInternal(); testOffsetPanelRect(); - testSingleCaptureOverReaperUiIsInstrumentDrop(); - testSingleCaptureInsidePanelStaysInternal(); - testSingleCaptureOffReaperIsOsDrag(); - testMultiCaptureOverReaperUiIsOsDrag(); - testS17FieldsIgnoredWhenNotDragging(); + testSingleOverTrackPanelIsInstrumentDrop(); + testSingleOverFxSurfaceIsInstrumentDrop(); + testSingleOverFxEmbedRefuses(); + testSingleOverArrangeIsArrangeInsert(); + testSingleOverOtherReaperUiRefuses(); + testSingleOffReaperIsOsHandoff(); + + testMultiOverInstrumentSurfacesRefuses(); + testMultiOverArrangeIsArrangeInsert(); + testMultiOffReaperIsOsHandoff(); + testMultiOverOtherReaperUiRefuses(); + + testNullTrackWithSurfaceRefuses(); + testOffReaperIgnoresTrackFlag(); + testNoDragOrNoSamplesIsNone(); + + testClassTransitionsAreReversible(); + testUnresolvableEvaluationDoesNotAffectTheNext(); + testDragSpeedCannotChangeTheOutcome(); + testNoLiveOutsideCombinationResolvesToNone(); + testEveryClassHasACue(); testSinglePath(); testMultiPreservesOrder(); diff --git a/tests/test_instrument_drop.cpp b/tests/test_instrument_drop.cpp index 0b0ec1a..157327d 100644 --- a/tests/test_instrument_drop.cpp +++ b/tests/test_instrument_drop.cpp @@ -177,68 +177,98 @@ static void testBadClassIdRejected() { CHECK(!buildVstPresetBytes(std::string(32, 'A'), state).empty()); } -// --- FX-hotspot classification (S-VIEW-BUG-1 / S-GA-DropFX) ------------------- +// --- Surface classification --------------------------------------------------- // -// THE RULE (prefix-based — see instrument_drop.h): "fx_*" names the FX-chain / floating-FX -// windows; "tcp.fx*" / "mcp.fx*" name the TCP/MCP FX button and its sibling FX sub-elements. -// The SDK warns GetThingFromPoint "may append additional information", so exact-token -// matching (the previous, DAW-falsified predicate) is wrong; the prefix family is the -// documented-adjacent surface. Bare "tcp"/"mcp" and non-FX sub-elements are NOT hotspots. +// THE RULE (prefix-based — see instrument_drop.h): the SDK warns GetThingFromPoint "may append +// additional information", so exact-token matching (a previous, DAW-falsified predicate) is +// wrong. Ordering is load-bearing: the embed strip is matched BEFORE the track panel, and the +// track panel now claims the WHOLE "tcp*"/"mcp*" family rather than just its FX sub-elements. -// The TCP/MCP FX-button family arms an instrument drop — including sibling FX sub-elements -// and tokens with appended information. -static void testTcpMcpFxFamilyIsHotspot() { - CHECK(infoNamesFxHotspot("tcp.fx")); // TCP FX button (WALTER element name) - CHECK(infoNamesFxHotspot("mcp.fx")); // MCP FX button - CHECK(infoNamesFxHotspot("tcp.fxbyp")); // FX bypass — sibling FX element - CHECK(infoNamesFxHotspot("tcp.fxparm")); // FX param knob area — sibling FX element - CHECK(infoNamesFxHotspot("mcp.fxlist")); // MCP FX insert list - CHECK(infoNamesFxHotspot("tcp.fx.1")); // appended info (SDK: "may append...") - CHECK(infoNamesFxHotspot("tcp.fx extra")); // appended info, arbitrary form +using ui::ReaperSurface; + +static ReaperSurface onTrack(const std::string& info) { + return classifyReaperSurface(info, /*haveTrack=*/true); } -// The FX chain / floating-FX windows (the surfaces the ORIGINAL predicate matched) still -// classify as hotspots — the fix does not regress the "fx_" surface. -static void testFxWindowStillHotspot() { - CHECK(infoNamesFxHotspot("fx_chain")); // FX chain window - CHECK(infoNamesFxHotspot("fx_0")); // first FX, floating - CHECK(infoNamesFxHotspot("fx_12")); // arbitrary floating-FX index +// The FX chain / floating-FX windows. +static void testFxWindowIsFxSurface() { + CHECK(onTrack("fx_chain") == ReaperSurface::FxSurface); + CHECK(onTrack("fx_0") == ReaperSurface::FxSurface); + CHECK(onTrack("fx_12") == ReaperSurface::FxSurface); } -// Non-FX surfaces are NOT hotspots — a drop here is not an instrument drop (it would fall -// through to the OS drag / no-op). This includes the bare TCP/MCP tokens and all non-FX -// "tcp.*"/"mcp.*" sub-elements (e.g. mute button, volume fader, track name, meter). -static void testNonFxSurfacesAreNotHotspot() { - CHECK(!infoNamesFxHotspot("tcp")); // bare track control panel — NOT an FX hotspot - CHECK(!infoNamesFxHotspot("mcp")); // bare mixer control panel — NOT an FX hotspot - CHECK(!infoNamesFxHotspot("tcp.mute")); // mute button — track panel, not FX - CHECK(!infoNamesFxHotspot("tcp.vol")); // volume fader — track panel, not FX - CHECK(!infoNamesFxHotspot("tcp.f")); // truncated non-FX token — prefix must be whole - CHECK(!infoNamesFxHotspot("arrange")); - CHECK(!infoNamesFxHotspot("spacer_0")); - CHECK(!infoNamesFxHotspot("")); // pointer over nothing REAPER classifies - CHECK(!infoNamesFxHotspot("trans")); // transport - CHECK(!infoNamesFxHotspot("envcp")); // envelope control panel — a track thing, not FX +// The FX-button family within the TCP/MCP — the surface the glyph-only rule used to be limited +// to, still an instrument surface (as TrackPanel, which resolves identically). +static void testTcpMcpFxFamilyIsTrackPanel() { + CHECK(onTrack("tcp.fx") == ReaperSurface::TrackPanel); + CHECK(onTrack("mcp.fx") == ReaperSurface::TrackPanel); + CHECK(onTrack("tcp.fxbyp") == ReaperSurface::TrackPanel); + CHECK(onTrack("tcp.fxparm") == ReaperSurface::TrackPanel); + CHECK(onTrack("mcp.fxlist") == ReaperSurface::TrackPanel); + CHECK(onTrack("tcp.fx.1") == ReaperSurface::TrackPanel); // appended info + CHECK(onTrack("tcp.fx extra") == ReaperSurface::TrackPanel); // appended info, arbitrary form } -// The embed-strip sub-element is NOT a hotspot. "tcp.fxembed" / "mcp.fxembed" is the surface -// where a ReaSampler 9000 instance draws inline in the TCP/MCP via IReaperUIEmbedInterface. -// Dropping a card there must NOT add a SECOND instance on top of the existing embed — the -// drop should be ignored (no instrument drop), even though the token starts with "tcp.fx". -// This documents the explicit exclusion in infoNamesFxHotspot and would catch a regression if -// the exclude guard were accidentally removed. -static void testEmbedStripIsNotHotspot() { - CHECK(!infoNamesFxHotspot("tcp.fxembed")); // TCP embed strip — existing instance's surface - CHECK(!infoNamesFxHotspot("mcp.fxembed")); // MCP embed strip — existing instance's surface - // With hypothetically appended info (SDK "may append") — still excluded. - CHECK(!infoNamesFxHotspot("tcp.fxembed.1")); - CHECK(!infoNamesFxHotspot("mcp.fxembed extra")); +// THE ROOT-CAUSE FIX: the bare panel token and every non-FX sub-element are now the instrument +// hotspot too. A TCP too narrow to draw the FX button reports "tcp", which under the old +// glyph-only rule produced a cue-less no-op. +static void testWholeTrackPanelIsTheHotspot() { + CHECK(onTrack("tcp") == ReaperSurface::TrackPanel); // bare track control panel + CHECK(onTrack("mcp") == ReaperSurface::TrackPanel); // bare mixer control panel + CHECK(onTrack("tcp.mute") == ReaperSurface::TrackPanel); // mute button + CHECK(onTrack("tcp.vol") == ReaperSurface::TrackPanel); // volume fader + CHECK(onTrack("tcp.meter") == ReaperSurface::TrackPanel); // meter + CHECK(onTrack("tcp.f") == ReaperSurface::TrackPanel); // truncated token — still the panel +} + +// The embed strip is where a ReaSampler 9000 instance already draws inline via +// IReaperUIEmbedInterface. It must NOT resolve to a hotspot — a drop there would stack a second +// instance on the first. Matched before the "tcp"/"mcp" rule, so widening the panel hotspot +// cannot swallow it; do not reorder these two checks in the classifier. +static void testEmbedStripIsItsOwnSurface() { + CHECK(onTrack("tcp.fxembed") == ReaperSurface::FxEmbed); + CHECK(onTrack("mcp.fxembed") == ReaperSurface::FxEmbed); + CHECK(onTrack("tcp.fxembed.1") == ReaperSurface::FxEmbed); + CHECK(onTrack("mcp.fxembed extra") == ReaperSurface::FxEmbed); +} + +// The arrange, including a token with appended information. +static void testArrangeIsArrange() { + CHECK(onTrack("arrange") == ReaperSurface::Arrange); + CHECK(onTrack("arrange extra") == ReaperSurface::Arrange); +} + +// Anything else REAPER names is Other — a defined refusal, never a guessed outcome. Includes +// tokens REAPER may add in future versions. +static void testUnnamedReaperSurfacesAreOther() { + CHECK(onTrack("spacer_0") == ReaperSurface::Other); + CHECK(onTrack("trans") == ReaperSurface::Other); + CHECK(onTrack("envcp") == ReaperSurface::Other); + CHECK(onTrack("ruler") == ReaperSurface::Other); + CHECK(onTrack("something_reaper_adds_in_2030") == ReaperSurface::Other); +} + +// The empty info string splits on whether a track came back with it. No track means the pointer +// has left REAPER (the OS hand-off's trigger); a track with no info means we are over REAPER on +// a surface we cannot name, which must refuse rather than be treated as off-REAPER. +static void testEmptyInfoSplitsOnTrackPresence() { + CHECK(classifyReaperSurface("", /*haveTrack=*/false) == ReaperSurface::OffReaper); + CHECK(classifyReaperSurface("", /*haveTrack=*/true) == ReaperSurface::Other); +} + +// The SDK's documented null-track-with-valid-info case: the surface is read from the string +// alone, so the classifier reports it faithfully and the gesture law decides what a missing +// track means for that surface. +static void testNullTrackStillClassifiesTheSurface() { + CHECK(classifyReaperSurface("arrange", false) == ReaperSurface::Arrange); + CHECK(classifyReaperSurface("tcp", false) == ReaperSurface::TrackPanel); + CHECK(classifyReaperSurface("fx_chain", false) == ReaperSurface::FxSurface); } // Do not reintroduce a per-surface "capture carries" loop test: buildInstrumentDropPreset takes -// only sampleId (proven by testPresetRoundTripsThroughInstrumentReader), and per-surface hotspot -// coverage already exists (testTcpMcpFxFamilyIsHotspot / testFxWindowStillHotspot) — a loop with -// an identical body per surface string can't distinguish them. +// only sampleId (proven by testPresetRoundTripsThroughInstrumentReader), and per-surface +// coverage already exists above — a loop with an identical body per surface string can't +// distinguish them. // --- All-or-nothing rollback -------------------------------------------------- @@ -288,10 +318,14 @@ int main() { testDeterministic(); testBadClassIdRejected(); - testTcpMcpFxFamilyIsHotspot(); - testFxWindowStillHotspot(); - testNonFxSurfacesAreNotHotspot(); - testEmbedStripIsNotHotspot(); + testFxWindowIsFxSurface(); + testTcpMcpFxFamilyIsTrackPanel(); + testWholeTrackPanelIsTheHotspot(); + testEmbedStripIsItsOwnSurface(); + testArrangeIsArrange(); + testUnnamedReaperSurfacesAreOther(); + testEmptyInfoSplitsOnTrackPresence(); + testNullTrackStillClassifiesTheSurface(); testAddFailureLeavesNothingToRollBack(); testPresetFailureRollsBackTheCreatedIndex();