diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index 7c82568..6b6722d 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -42,6 +42,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 f11d14a..876550d 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..1a0440c 100644 --- a/src/core/ui/drag_out.cpp +++ b/src/core/ui/drag_out.cpp @@ -15,14 +15,49 @@ 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; +bool needsSurfaceProbe(int px, int py, const PanelClientRect& client) { + return !insideClient(px, py, client); +} + +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..4c3336f 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,69 @@ 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 + Count, // sentinel, NOT a real surface — tests/test_drag_out.cpp's kAllSurfaces is + // pinned against this via static_assert so a 7th surface can't silently skip + // the exhaustiveness matrix }; -// 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); + +// True outside the panel client rect — the same half-open test decideDropClass gates the SDK +// hit-test on. Exported so the shell reads this one pure predicate instead of reimplementing the +// inside-client math inline (core/ui/CLAUDE.md forbids hit-test geometry living in shell code). +bool needsSurfaceProbe(int px, int py, const PanelClientRect& client); + +// 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 42ce78f..e5443df 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 `wire::classifyReaperSurface`, whose token rules `core/wire/CLAUDE.md` owns), 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`'s one-pixel-span reading — inferred, not SDK-documented) and `performArrangeDrop` (snap the drop time, then one `InsertMedia` per capture on the pointer's track — assumed, not confirmed, to land end-to-end via REAPER's own cursor advance — in ONE undo block, counting only InsertMedia's reported successes, with the caller's track selection and edit cursor restored). The one timeline-placing shell here, per the invariant above; it never captures and never writes the bank. - `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.** Surface (2)'s action is the one in this directory published into a NON-main action section (Media Explorer) as well as Main — two ids, one handler, two dispatch hooks; see root `CLAUDE.md` §"REAPER extension contract" for the mechanism. ## 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..8886413 --- /dev/null +++ b/src/shell/actions/arrange_drop_win.cpp @@ -0,0 +1,129 @@ +// 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. insert.cpp does NOT share this assumption: it +// resets the cursor before every track's insert specifically to stay independent of +// cursor-advance behavior (insert.cpp:18-20, "this doesn't matter either way"). This call +// site is the first in the tree to depend on it. +// 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. +// D. GetSet_ArrangeView2's one-pixel span [screenX, screenX+1) reads the time at that column. +// The header documents only the all-zero span (screen_x_start==screen_x_end==0) as the +// "whole view" special case; the per-column reading for any other span is inferred, not +// documented. +// E. InsertMedia's int return isn't SDK-documented; treated conservatively as 0=failure, +// nonzero=success — performArrangeDrop counts only the latter. + +#include "shell/actions/arrange_drop_win.h" + +#include +#include +#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; kept separate here because insert.cpp sits in the capture +// pillar, outside this track's surface fence — not a ruling that the two should never share a +// helper, just not this track's call to make. +// +// CountSelectedTracks/GetSelectedTrack both skip the master track (SDK header), so a user with +// the master selected loses that selection across the drop — pre-existing behavior inherited +// from insert.cpp's identical pair, not fixed here. +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 with a one-pixel span [screenX, screenX+1) is assumed to read the time at + // that column — inferred, not documented (assumption D in the file header). The SDK's ONLY + // documented special form is screen_x_start==screen_x_end==0 (both zero) for "the whole + // arrange view's start/end time"; a zero-width span at a nonzero column (e.g. screenX, + // screenX) is NOT that special case, so the +1 here is precautionary rather than required. + 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's + // return isn't SDK-documented (assumption E); treated conservatively as 0=failure, so a + // REAPER-side refusal is reflected in the count and in the undo label, not silent. + if (InsertMedia(path.c_str(), mode) != 0) ++inserted; + } + + // A fixed stack buffer, not std::string concatenation: the prior shape built the label with + // std::to_string + `+` between the restore below and Undo_EndBlock2, so a bad_alloc there + // would leave an unbalanced undo block open. snprintf here removes the allocation outright. + char label[64]; + std::snprintf(label, sizeof(label), "ReaSampler: drop %d %s onto arrange", inserted, + inserted == 1 ? "capture" : "captures"); + + restoreSelectedTracks(priorSelection); + SetEditCurPos(priorCursor, /*moveview=*/false, /*seekplay=*/false); + + // extraflags -1 = UNDO_STATE_ALL, matching the insert action's own block. + Undo_EndBlock2(nullptr, label, -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..1094525 --- /dev/null +++ b/src/shell/actions/arrange_drop_win.h @@ -0,0 +1,34 @@ +#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 GetSet_ArrangeView2's one-pixel-column reading — +// inferred behavior, not SDK-documented (see the .cpp's assumption D). Times left of project +// start clamp to 0. +double arrangeTimeAtScreenX(int screenX); + +// Places every path in `absolutePaths` on `track`, inside ONE undo block. The first lands at +// `time`; whether REAPER's own cursor advance lands the rest end-to-end, or stacks them at one +// position instead, is assumption B in the .cpp (visible, one Ctrl-Z away, either way). The +// drop time is assumed to honor the project's snap setting (assumption C). The caller's track +// selection and edit-cursor position are restored before returning. Returns the number of files +// InsertMedia reported inserting successfully — its return isn't SDK-documented; treated as +// 0=failure (assumption E in the .cpp). +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..6d62efd 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,102 @@ 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; // reached once per move resolving to OsHandoff, + // right before handOffToOs is attempted; the OS + // drag loop (once it actually starts) draws its + // own copy cursor, so leave the cursor alone here + 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 (core/ui/CLAUDE.md: "the drag-out law is +// per-move and stateless"). +struct LiveDrop { + DropClass cls = DropClass::None; + MediaTrack* track = nullptr; + int screenX = 0; // the evaluated point in screen coords; meaningful only outside the client +}; + +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 (needsSurfaceProbe — exported + // by the pure law so this gate can't drift from decideDropClass's own inside-client check), + // 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 (needsSurfaceProbe(x, y, client)) { + 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); + + // ArrangeInsert has no defined outcome once every armed sample is stale/missing — the same + // gate handOffToOs applies via decideOsHandoff before an OS hand-off, so acceptance + // criterion 7 (no silent no-op release) holds on this cell too, at both a cueing move and + // the release itself (both call this function). Cheap: a few fs::exists checks, only + // reached outside the client — already a cold path. + if (out.cls == DropClass::ArrangeInsert && resolveDragPathsForOs().empty()) { + out.cls = DropClass::Refuse; + } + 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 +356,25 @@ 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); + // Re-resolved from scratch every move — core/ui/CLAUDE.md, "the drag-out law is + // per-move and stateless." + 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(); + applyDropCue(cueForDropClass(live.cls)); // OsHandoff -> OsOwned, a documented no-op cue + if (live.cls == DropClass::OsHandoff) { + handOffToOs(); + return; + } 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 +411,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 +457,54 @@ 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. Every +// DropClass either performs its outcome or is an explicit, already-cued refusal (core/ui/ +// CLAUDE.md: "no DropClass means nothing happens"). The switch below has no default so each case +// is spelled out by hand — but this build sets no warning flags (root CLAUDE.md), so a missing +// case is NOT a compile error here; exhaustiveness is a review discipline, not a compiler +// guarantee. 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; + // Resolved at the release point, not from what the (coalesced) moves last recorded — + // core/ui/CLAUDE.md, "the drag-out law is per-move and stateless." + 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. OsHandoff reaching release + // usually means a live hand-off consumed the drag inside DoDragDrop's modal loop + // and this call never ran — but WM_MOUSEMOVE coalescing can still deliver a + // WM_LBUTTONUP with no intervening processed move (or right after a + // canInitiateDragOut refusal), so this case CAN be reached with a stale + // OsHandoff/Refuse class; the no-op here is correct either way. + 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 a16eb6c..8f9ef02 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; @@ -132,6 +135,7 @@ using ui::hitTestTabStrip; using ui::menuButtonReserve; using ui::modeSegmentEnabled; using ui::navigate; +using ui::needsSurfaceProbe; using ui::roleColor; using ui::stripActionPrefix; using ui::tagButtonEnabled; @@ -324,14 +328,9 @@ 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 (core/ui/CLAUDE.md: "the + // drag-out law is per-move and stateless"). 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..5cac9fb 100644 --- a/tests/test_drag_out.cpp +++ b/tests/test_drag_out.cpp @@ -1,16 +1,18 @@ -// 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" +#include #include #include #include @@ -22,115 +24,261 @@ 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, +}; + +// Pins kAllSurfaces against ReaperSurface::Count so a 7th surface added to the enum without a +// matching entry here fails the BUILD, not just a silently-incomplete matrix — the compiler +// alone does not enforce this (no -Wswitch/-Wall or /W4 anywhere in the build; see +// panel_drag.cpp's onLBtnUp for the same caveat on DropClass). +static_assert(sizeof(kAllSurfaces) / sizeof(kAllSurfaces[0]) == + static_cast(ReaperSurface::Count), + "kAllSurfaces must list exactly the surfaces below ReaperSurface::Count"); + +// 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 +306,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 +315,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 +374,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 +400,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();