From fef595be9196ff3f825232f0cf56feb645688acb Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Mon, 27 Jul 2026 13:26:10 -0400 Subject: [PATCH 1/2] fix(drop): recognize TCP/MCP FX-button hotspot for instrument drop (S-VIEW-BUG-1) The FX-hotspot classifier matched only fx_ (the FX-chain/floating windows), so a drop on a track TCP FX button (info tcp*) never armed the drop and the capture fell through to arrange as audio. Widen to fx_/tcp/mcp in a pure, unit-tested predicate; shell calls it. --- src/instrument_drop.cpp | 8 +++++++ src/instrument_drop.h | 20 ++++++++++++++++ src/instrument_drop_win.cpp | 19 ++++----------- tests/test_instrument_drop.cpp | 42 ++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/instrument_drop.cpp b/src/instrument_drop.cpp index bc28657..b24d42a 100644 --- a/src/instrument_drop.cpp +++ b/src/instrument_drop.cpp @@ -39,6 +39,14 @@ std::string buildInstrumentDropChunk(const std::string& sampleId) { return encodeBase64(instrumentDropStateBytes(sampleId)); } +bool infoNamesFxHotspot(const std::string& info) { + // See the header contract. The FX-chain / floating-FX windows report "fx_*"; the track / + // mixer panel that carries the FX button reports "tcp*" / "mcp*". Prefix-match all three — + // the TCP/MCP surfaces are the fix for S-VIEW-BUG-1 (the "fx_"-only predicate missed them). + auto startsWith = [&info](const char* p) { return info.rfind(p, 0) == 0; }; + return startsWith("fx_") || startsWith("tcp") || startsWith("mcp"); +} + std::string encodeBase64(const std::vector& bytes) { std::string out; out.reserve(((bytes.size() + 2) / 3) * 4); diff --git a/src/instrument_drop.h b/src/instrument_drop.h index 3fbddbf..fb23852 100644 --- a/src/instrument_drop.h +++ b/src/instrument_drop.h @@ -49,6 +49,26 @@ namespace reasampler { // Deterministic: the same sampleId always yields the same blob (base64 of the same bytes). std::string buildInstrumentDropChunk(const std::string& sampleId); +// -- FX-drop-target classification (S-VIEW-BUG-1) ------------------------------ +// +// Pure classifier for GetThingFromPoint's info string: is the point over a surface where an +// instrument drop should instantiate ReaSampler 9000 on the resolved track? This is string +// logic (no REAPER types), so it lives here and is unit-tested outside the DAW — the shell +// (instrument_drop_win) only supplies the info bytes GetThingFromPoint filled. +// +// THE BUG (S-VIEW-BUG-1): the original shell predicate matched ONLY "fx_" — but per the SDK +// (reaper_plugin_functions.h §GetThingFromPoint) "fx_chain"/"fx_N" are the FX-CHAIN and +// FLOATING-FX windows; a hit on the TRACK PANEL (where the TCP/MCP FX button actually lives — +// the intuitive "drop onto the track's FX chain" target) reports a string that BEGINS WITH +// "tcp" or "mcp" (e.g. "tcp.fx"). So dropping on the TCP FX button never armed the drop; the +// gesture fell through and the file dropped to arrange as audio. The drop target is therefore +// EITHER of the SDK's two documented FX-bearing surfaces: +// * the FX chain / floating FX window -> info begins with "fx_" ("fx_chain", "fx_0", ...) +// * the track/mixer panel that hosts the FX button -> info begins with "tcp" or "mcp" +// A single-capture drag released over any of these, on a resolved track, is an instrument drop. +// Any other info ("arrange", "spacer_0", "", ...) is not an FX hotspot. +bool infoNamesFxHotspot(const std::string& info); + // The raw (pre-base64) component-state bytes — exposed so the round-trip test can decode them // back through the instrument's OWN reader (sample_map::deserializeComponentState) and assert // the capture is selected, proving buildInstrumentDropChunk feeds the instrument exactly what diff --git a/src/instrument_drop_win.cpp b/src/instrument_drop_win.cpp index a0245c1..73d1e78 100644 --- a/src/instrument_drop_win.cpp +++ b/src/instrument_drop_win.cpp @@ -5,10 +5,10 @@ #include "instrument_drop_win.h" -#include #include -#include "app_version.h" // vstPluginName() — the CHANNEL-correct FX name (stable/beta pairing) +#include "app_version.h" // vstPluginName() — the CHANNEL-correct FX name (stable/beta pairing) +#include "instrument_drop.h" // infoNamesFxHotspot — the PURE, unit-tested hotspot classifier #include "reaper_plugin.h" @@ -23,19 +23,6 @@ namespace reasampler { -namespace { - -// GetThingFromPoint's info string prefixes (verified against reaper_plugin_functions.h: -// "Updates infoOut with information such as 'arrange', 'fx_chain', 'fx_0' ... If a track -// panel is hit, string will begin with 'tcp' or 'mcp' or 'tcp.mute' etc"). The FX region -// reports "fx_chain" (the FX list area) or "fx_N" (a specific FX button). We treat either -// as the FX hotspot — the S17 drop target. -bool infoNamesFxHotspot(const char* info) { - return std::strncmp(info, "fx_", 3) == 0; -} - -} // namespace - FxDropTarget resolveFxDropTarget(int screenX, int screenY) { FxDropTarget out; char info[256] = {0}; @@ -46,6 +33,8 @@ FxDropTarget resolveFxDropTarget(int screenX, int screenY) { MediaTrack* track = GetThingFromPoint(screenX, screenY, info, sizeof(info)); out.track = track; out.overReaperUi = (track != nullptr) || (info[0] != '\0'); + // S-VIEW-BUG-1: the hotspot is either the FX chain/floating window ("fx_*") OR the track/ + // mixer panel that hosts the FX button ("tcp*"/"mcp*"). The pure classifier owns the rule. out.overFxHotspot = (track != nullptr) && infoNamesFxHotspot(info); return out; } diff --git a/tests/test_instrument_drop.cpp b/tests/test_instrument_drop.cpp index 79bbcab..7487ea8 100644 --- a/tests/test_instrument_drop.cpp +++ b/tests/test_instrument_drop.cpp @@ -105,6 +105,44 @@ static void testBase64DecodeRejectsMalformed() { CHECK(decodeBase64("Zg==Zg==").empty()); // interior padding (pad before the final quad) } +// --- FX-hotspot classification (S-VIEW-BUG-1) --------------------------------- +// +// THE BUG: dropping a capture onto a track's FX button (in the TCP) never armed the +// instrument drop, because the old predicate matched only "fx_" — which the SDK reserves for +// the FX CHAIN / FLOATING-FX windows. A track-panel hit (where the FX button lives) reports a +// string beginning with "tcp"/"mcp". These tests fix the classifier at the boundary the shell +// consumes: each would FAIL under the old "fx_"-only rule for the tcp/mcp cases. + +// The regression case: the TCP FX-button surface reports "tcp*" and MUST classify as an FX +// hotspot. Under the pre-fix "fx_"-only predicate these all returned false — the exact miss +// that produced the "drops as audio to arrange" symptom. +static void testTcpMcpPanelIsHotspot() { + CHECK(infoNamesFxHotspot("tcp")); // bare track control panel + CHECK(infoNamesFxHotspot("tcp.fx")); // the TCP FX-button WALTER element + CHECK(infoNamesFxHotspot("tcp.mute")); // any tcp.* sub-element resolves to the track + CHECK(infoNamesFxHotspot("mcp")); // mixer control panel + CHECK(infoNamesFxHotspot("mcp.fx")); // the MCP FX area +} + +// The FX chain / floating-FX windows (the surfaces the ORIGINAL predicate matched) still +// classify as hotspots — the fix widens the rule, it 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 +} + +// Non-FX surfaces are NOT hotspots — a drop here is not an instrument drop (it would fall +// through to the OS drag / no-op). Guards against over-broad matching (e.g. "spacer_0" must +// not match despite living near the tracks; "arrange" is the audio-import surface). +static void testNonFxSurfacesAreNotHotspot() { + 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 +} + int main() { testBlobRoundTripsThroughInstrumentReader(); testGuidLikeIdRoundTrips(); @@ -114,6 +152,10 @@ int main() { testBase64RoundTripAllBytes(); testBase64DecodeRejectsMalformed(); + testTcpMcpPanelIsHotspot(); + testFxWindowStillHotspot(); + testNonFxSurfacesAreNotHotspot(); + if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; } From 5870b18362ffa6d83bb9d0d52a2a3ba0d4c739b9 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Mon, 27 Jul 2026 13:34:40 -0400 Subject: [PATCH 2/2] fix(instrument_drop): narrow FX hotspot to tcp.fx/mcp.fx, not bare tcp/mcp (S-VIEW-BUG-1) --- src/instrument_drop.cpp | 12 +++++++---- src/instrument_drop.h | 19 +++++++++-------- src/instrument_drop_win.h | 16 +++++++------- tests/test_instrument_drop.cpp | 38 ++++++++++++++++++++-------------- 4 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/instrument_drop.cpp b/src/instrument_drop.cpp index b24d42a..6007620 100644 --- a/src/instrument_drop.cpp +++ b/src/instrument_drop.cpp @@ -40,11 +40,15 @@ std::string buildInstrumentDropChunk(const std::string& sampleId) { } bool infoNamesFxHotspot(const std::string& info) { - // See the header contract. The FX-chain / floating-FX windows report "fx_*"; the track / - // mixer panel that carries the FX button reports "tcp*" / "mcp*". Prefix-match all three — - // the TCP/MCP surfaces are the fix for S-VIEW-BUG-1 (the "fx_"-only predicate missed them). + // See the header contract. Two documented FX-bearing surfaces (SDK §GetThingFromPoint): + // "fx_chain" / "fx_N" — the FX-chain and floating-FX windows. + // "tcp.fx" / "mcp.fx" — the TCP / MCP FX button specifically. + // Bare "tcp" / "mcp" and any other "tcp.*" / "mcp.*" sub-element (e.g. "tcp.mute", + // "tcp.vol") are track-panel hits that are NOT on the FX button — those must not trigger + // an instrument drop. Exact-string match for the two .fx tokens; prefix-match for fx_. + if (info == "tcp.fx" || info == "mcp.fx") return true; auto startsWith = [&info](const char* p) { return info.rfind(p, 0) == 0; }; - return startsWith("fx_") || startsWith("tcp") || startsWith("mcp"); + return startsWith("fx_"); } std::string encodeBase64(const std::vector& bytes) { diff --git a/src/instrument_drop.h b/src/instrument_drop.h index fb23852..b3c2a60 100644 --- a/src/instrument_drop.h +++ b/src/instrument_drop.h @@ -58,15 +58,16 @@ std::string buildInstrumentDropChunk(const std::string& sampleId); // // THE BUG (S-VIEW-BUG-1): the original shell predicate matched ONLY "fx_" — but per the SDK // (reaper_plugin_functions.h §GetThingFromPoint) "fx_chain"/"fx_N" are the FX-CHAIN and -// FLOATING-FX windows; a hit on the TRACK PANEL (where the TCP/MCP FX button actually lives — -// the intuitive "drop onto the track's FX chain" target) reports a string that BEGINS WITH -// "tcp" or "mcp" (e.g. "tcp.fx"). So dropping on the TCP FX button never armed the drop; the -// gesture fell through and the file dropped to arrange as audio. The drop target is therefore -// EITHER of the SDK's two documented FX-bearing surfaces: -// * the FX chain / floating FX window -> info begins with "fx_" ("fx_chain", "fx_0", ...) -// * the track/mixer panel that hosts the FX button -> info begins with "tcp" or "mcp" -// A single-capture drag released over any of these, on a resolved track, is an instrument drop. -// Any other info ("arrange", "spacer_0", "", ...) is not an FX hotspot. +// FLOATING-FX windows; a hit on the TCP/MCP FX button (where the intuitive "drop onto the +// track's FX chain" gesture lands) reports a string in the "tcp.*"/"mcp.*" family — +// specifically "tcp.fx" / "mcp.fx". So dropping on the TCP FX button never armed the drop; +// the gesture fell through and the file dropped to arrange as audio. +// +// The fix: two documented FX-bearing surfaces are hotspots (SDK §GetThingFromPoint): +// * "fx_chain" / "fx_N" — the FX-chain and floating-FX windows (prefix "fx_") +// * "tcp.fx" / "mcp.fx" — the TCP / MCP FX button (exact token) +// Bare "tcp"/"mcp" and any other "tcp.*"/"mcp.*" sub-element (e.g. "tcp.mute") are track-panel +// hits on non-FX surfaces — they must NOT trigger an instrument drop. Only the FX button does. bool infoNamesFxHotspot(const std::string& info); // The raw (pre-base64) component-state bytes — exposed so the round-trip test can decode them diff --git a/src/instrument_drop_win.h b/src/instrument_drop_win.h index fba0cce..c8675b3 100644 --- a/src/instrument_drop_win.h +++ b/src/instrument_drop_win.h @@ -33,14 +33,14 @@ struct FxDropTarget { }; // Hit-test a screen point (REAPER screen coords) to an FX drop target. Wraps -// GetThingFromPoint, whose info string tells us what was hit ("tcp"/"mcp" for a track panel, -// "fx_chain"/"fx_N" for the FX area/button). `overReaperUi` is the shell-supplied predicate -// the pure drag_out::decideGesture consumes (true when the point is over REAPER's own UI — -// i.e. GetThingFromPoint returned a track OR a recognizable non-track thing, false when the -// pointer has left REAPER entirely). `overFxHotspot` is true when the info string names the -// FX region specifically — the S17 "FX-button hotspot vs. whole TCP" question is resolved to -// the FX hotspot (the discoverable, unambiguous target), decided here from the SDK's own -// hit-test string rather than a home-grown geometry guess. +// GetThingFromPoint, whose info string tells us what was hit ("tcp.fx"/"mcp.fx" for the TCP/MCP +// FX button; "fx_chain"/"fx_N" for the FX-chain and floating-FX windows; bare "tcp"/"mcp" or +// other "tcp.*"/"mcp.*" tokens for non-FX track-panel regions). `overReaperUi` is the +// shell-supplied predicate the pure drag_out::decideGesture consumes (true when the point is +// over REAPER's own UI — i.e. GetThingFromPoint returned a track OR a recognizable non-track +// thing, false when the pointer has left REAPER entirely). `overFxHotspot` is true only when +// the info string names a genuine FX-bearing surface ("tcp.fx", "mcp.fx", or "fx_*") — +// decided by instrument_drop::infoNamesFxHotspot from the SDK's own hit-test string. FxDropTarget resolveFxDropTarget(int screenX, int screenY); // Perform the drop on `track`: add a fresh ReaSampler 9000 instance and inject `chunkBase64` diff --git a/tests/test_instrument_drop.cpp b/tests/test_instrument_drop.cpp index 7487ea8..6c28c48 100644 --- a/tests/test_instrument_drop.cpp +++ b/tests/test_instrument_drop.cpp @@ -109,23 +109,25 @@ static void testBase64DecodeRejectsMalformed() { // // THE BUG: dropping a capture onto a track's FX button (in the TCP) never armed the // instrument drop, because the old predicate matched only "fx_" — which the SDK reserves for -// the FX CHAIN / FLOATING-FX windows. A track-panel hit (where the FX button lives) reports a -// string beginning with "tcp"/"mcp". These tests fix the classifier at the boundary the shell -// consumes: each would FAIL under the old "fx_"-only rule for the tcp/mcp cases. +// the FX CHAIN / FLOATING-FX windows. A track-panel FX-button hit reports "tcp.fx"/"mcp.fx" +// (SDK §GetThingFromPoint: "string will begin with 'tcp' or 'mcp' or 'tcp.mute' etc"). +// +// THE FIX: narrow to two documented FX-bearing surfaces: +// * "tcp.fx" / "mcp.fx" — the TCP/MCP FX button (exact token, NOT bare tcp/mcp) +// * "fx_*" — the FX-chain / floating-FX windows (prefix, as before) +// Bare "tcp"/"mcp" and any other "tcp.*"/"mcp.*" sub-element (e.g. "tcp.mute", "tcp.vol") +// are non-FX track-panel regions — an instrument drop must NOT fire there. -// The regression case: the TCP FX-button surface reports "tcp*" and MUST classify as an FX -// hotspot. Under the pre-fix "fx_"-only predicate these all returned false — the exact miss -// that produced the "drops as audio to arrange" symptom. -static void testTcpMcpPanelIsHotspot() { - CHECK(infoNamesFxHotspot("tcp")); // bare track control panel - CHECK(infoNamesFxHotspot("tcp.fx")); // the TCP FX-button WALTER element - CHECK(infoNamesFxHotspot("tcp.mute")); // any tcp.* sub-element resolves to the track - CHECK(infoNamesFxHotspot("mcp")); // mixer control panel - CHECK(infoNamesFxHotspot("mcp.fx")); // the MCP FX area +// The TCP/MCP FX button specifically — the surface that arms an instrument drop (S-VIEW-BUG-1 +// fix). Under the old "fx_"-only predicate these returned false — the exact miss that produced +// the "drops as audio to arrange" symptom on the FX button. +static void testTcpMcpFxButtonIsHotspot() { + CHECK(infoNamesFxHotspot("tcp.fx")); // TCP FX button (SDK token) + CHECK(infoNamesFxHotspot("mcp.fx")); // MCP FX area (SDK token) } // The FX chain / floating-FX windows (the surfaces the ORIGINAL predicate matched) still -// classify as hotspots — the fix widens the rule, it does not regress the "fx_" surface. +// 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 @@ -133,9 +135,13 @@ static void testFxWindowStillHotspot() { } // Non-FX surfaces are NOT hotspots — a drop here is not an instrument drop (it would fall -// through to the OS drag / no-op). Guards against over-broad matching (e.g. "spacer_0" must -// not match despite living near the tracks; "arrange" is the audio-import surface). +// 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("arrange")); CHECK(!infoNamesFxHotspot("spacer_0")); CHECK(!infoNamesFxHotspot("")); // pointer over nothing REAPER classifies @@ -152,7 +158,7 @@ int main() { testBase64RoundTripAllBytes(); testBase64DecodeRejectsMalformed(); - testTcpMcpPanelIsHotspot(); + testTcpMcpFxButtonIsHotspot(); testFxWindowStillHotspot(); testNonFxSurfacesAreNotHotspot();