fix(instrument_drop_win): UTF-8 temp path, PID collision guard, embed-strip excluded from drop hotspot

This commit is contained in:
2026-07-28 06:37:42 -04:00
parent b4dd6dc9f1
commit 66f2e402b0
4 changed files with 78 additions and 25 deletions
+9 -3
View File
@@ -92,10 +92,16 @@ std::vector<std::uint8_t> buildInstrumentDropPreset(const std::string& sampleId)
bool infoNamesFxHotspot(const std::string& info) { bool infoNamesFxHotspot(const std::string& info) {
// See the header contract. Prefix rule (S-GA-DropFX): "fx_" names the FX-chain / // See the header contract. Prefix rule (S-GA-DropFX): "fx_" names the FX-chain /
// floating-FX windows; "tcp.fx" / "mcp.fx" prefixes name the TCP/MCP FX button and its // floating-FX windows; "tcp.fx" / "mcp.fx" prefixes name the TCP/MCP FX button and its
// sibling FX sub-elements (fxbyp/fxparm/fxembed/fxlist...), tolerant of the // sibling FX sub-elements (fxbyp/fxparm/fxlist...), tolerant of the SDK-documented "may
// SDK-documented "may append additional information". Bare "tcp"/"mcp" and non-FX // append additional information". Bare "tcp"/"mcp" and non-FX sub-elements ("tcp.mute",
// sub-elements ("tcp.mute", "tcp.vol") must NOT trigger an instrument drop. // "tcp.vol") must NOT trigger an instrument drop.
//
// EXCLUDE the embed-strip sub-element ("tcp.fxembed" / "mcp.fxembed"): that is the
// surface where a ReaSampler 9000 embed strip draws inside the TCP/MCP. Dropping a card
// there must NOT add a SECOND instance — the surface is the existing instance's own UI,
// not an FX-chain drop target. It starts with "tcp.fx" so it must be explicitly excluded.
auto startsWith = [&info](const char* p) { return info.rfind(p, 0) == 0; }; 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"); return startsWith("fx_") || startsWith("tcp.fx") || startsWith("mcp.fx");
} }
+6 -1
View File
@@ -94,7 +94,12 @@ std::vector<std::uint8_t> buildInstrumentDropPreset(const std::string& sampleId)
// "mcp.fx". So the hotspot rule is PREFIX-based (S-GA-DropFX: the earlier exact-token match // "mcp.fx". So the hotspot rule is PREFIX-based (S-GA-DropFX: the earlier exact-token match
// on "tcp.fx"/"mcp.fx" was too strict for appended info and sibling FX elements): // on "tcp.fx"/"mcp.fx" was too strict for appended info and sibling FX elements):
// * "fx_" prefix — the FX-chain and floating-FX windows // * "fx_" prefix — the FX-chain and floating-FX windows
// * "tcp.fx" prefix / "mcp.fx" prefix — the TCP/MCP FX button + sibling FX sub-elements // * "tcp.fx" / "mcp.fx" prefix — the TCP/MCP FX button + sibling FX sub-elements
// EXCEPT "tcp.fxembed" / "mcp.fxembed" — the embed-strip surface where a ReaSampler 9000
// instance draws inside the TCP/MCP. Dropping onto the existing instance's own UI must NOT
// add a second instance; the embed surface is explicitly excluded even though it starts
// with "tcp.fx". All other "tcp.fx*" / "mcp.fx*" tokens (fxbyp, fxparm, fxlist, ...) are
// hotspots — they are FX-chain controls, not a running instance's own surface.
// Bare "tcp"/"mcp" and non-FX sub-elements (e.g. "tcp.mute", "tcp.vol") are NOT hotspots. // Bare "tcp"/"mcp" and non-FX sub-elements (e.g. "tcp.mute", "tcp.vol") are NOT hotspots.
// The exact live token over the FX button remains a DAW-only fact — confirm in REAPER (a // The exact live token over the FX button remains a DAW-only fact — confirm in REAPER (a
// deferred ReaScript around reaper.GetThingFromPoint(reaper.GetMousePosition()) prints it). // deferred ReaScript around reaper.GetThingFromPoint(reaper.GetMousePosition()) prints it).
+36 -9
View File
@@ -31,17 +31,36 @@ namespace reasampler {
namespace { namespace {
// Write `bytes` to a fresh uniquely-named .vstpreset in the OS temp dir and return its // Write `bytes` to a fresh uniquely-named .vstpreset in the OS temp dir and return its path;
// absolute path; empty string on any failure. The .vstpreset extension is load-bearing — // returns an empty path on any failure. The .vstpreset extension is load-bearing —
// TrackFX_SetPreset's full-path form is documented for .vstpreset files (VST3). The file is // TrackFX_SetPreset's full-path form is documented for .vstpreset files (VST3). The file is
// transient: the caller deletes it right after the SetPreset call. // transient: the caller deletes it right after the SetPreset call.
std::string writeTempPreset(const std::vector<std::uint8_t>& bytes) { //
// The temp filename embeds the process ID so two concurrent REAPER instances (e.g. stable +
// beta) cannot collide in the shared OS temp dir, and one instance's cleanup cannot
// accidentally delete another's in-flight file.
//
// Non-throwing: every std::filesystem call uses the error_code overload. The whole body is
// wrapped in try/catch to guarantee no exception crosses the REAPER C callback boundary
// (the same discipline persist.cpp uses — see its non-throwing scanPruneOrphans comment).
//
// Returns the path object (not a narrow string) so the caller can:
// (a) pass path.u8string() to TrackFX_SetPreset — UTF-8 on MSVC, not ACP-converted,
// so a temp dir with accented or CJK user-name bytes is handled correctly;
// (b) delete via the retained path object — not via re-parsing the narrow string —
// so the cleanup cannot leak if the conversion above were to round-trip incorrectly.
std::filesystem::path writeTempPreset(const std::vector<std::uint8_t>& bytes) {
try {
static std::atomic<unsigned> counter{0}; static std::atomic<unsigned> counter{0};
std::error_code ec; std::error_code ec;
const std::filesystem::path dir = std::filesystem::temp_directory_path(ec); const std::filesystem::path dir = std::filesystem::temp_directory_path(ec);
if (ec) return {}; if (ec) return {};
const std::filesystem::path path = // PID in the name keeps files from distinct REAPER instances distinct in the shared
dir / ("reasampler_drop_" + std::to_string(counter.fetch_add(1)) + ".vstpreset"); // temp dir — prevents cross-instance collisions and spurious post-apply deletions.
const std::string name =
"reasampler_drop_" + std::to_string(GetCurrentProcessId()) +
"_" + std::to_string(counter.fetch_add(1)) + ".vstpreset";
const std::filesystem::path path = dir / name;
std::ofstream out(path, std::ios::binary | std::ios::trunc); std::ofstream out(path, std::ios::binary | std::ios::trunc);
if (!out) return {}; if (!out) return {};
out.write(reinterpret_cast<const char*>(bytes.data()), out.write(reinterpret_cast<const char*>(bytes.data()),
@@ -51,7 +70,10 @@ std::string writeTempPreset(const std::vector<std::uint8_t>& bytes) {
std::filesystem::remove(path, ec); std::filesystem::remove(path, ec);
return {}; return {};
} }
return path.string(); return path;
} catch (...) {
return {};
}
} }
} // namespace } // namespace
@@ -77,7 +99,7 @@ bool loadInstrumentOntoTrack(MediaTrack* track, const std::vector<std::uint8_t>&
// Materialize the .vstpreset FIRST so an I/O failure leaves the track untouched (no FX // Materialize the .vstpreset FIRST so an I/O failure leaves the track untouched (no FX
// added yet — nothing to roll back). // added yet — nothing to roll back).
const std::string presetPath = writeTempPreset(presetBytes); const std::filesystem::path presetPath = writeTempPreset(presetBytes);
if (presetPath.empty()) return false; if (presetPath.empty()) return false;
// The CHANNEL-correct FX name: "VST3:ReaSampler 9000" on stable, "VST3:ReaSampler 9000 // The CHANNEL-correct FX name: "VST3:ReaSampler 9000" on stable, "VST3:ReaSampler 9000
@@ -100,9 +122,14 @@ bool loadInstrumentOntoTrack(MediaTrack* track, const std::vector<std::uint8_t>&
// serializer produced (instrument_drop::buildInstrumentDropPreset -> // serializer produced (instrument_drop::buildInstrumentDropPreset ->
// sample_map::serializeComponentState). Unlike the former "vst_chunk" named-config-parm // sample_map::serializeComponentState). Unlike the former "vst_chunk" named-config-parm
// write, a failure here is REPORTED (false), not silently ignored. // write, a failure here is REPORTED (false), not silently ignored.
if (ok) ok = TrackFX_SetPreset(track, fxIndex, presetPath.c_str()); //
// u8string() gives UTF-8 bytes on MSVC (not ACP-converted), so a temp dir under an
// accented or CJK user-name is handled correctly by REAPER's path APIs.
if (ok) ok = TrackFX_SetPreset(track, fxIndex, presetPath.u8string().c_str());
// The preset file is transient regardless of outcome; best-effort cleanup (temp dir). // The preset file is transient regardless of outcome; delete via the retained path object
// (not a re-parsed narrow string) so cleanup cannot leak even if the UTF-8 conversion
// round-trip were incorrect.
std::error_code ec; std::error_code ec;
std::filesystem::remove(presetPath, ec); std::filesystem::remove(presetPath, ec);
+15
View File
@@ -218,6 +218,20 @@ static void testNonFxSurfacesAreNotHotspot() {
CHECK(!infoNamesFxHotspot("envcp")); // envelope control panel — a track thing, not FX CHECK(!infoNamesFxHotspot("envcp")); // envelope control panel — a track thing, not FX
} }
// 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"));
}
int main() { int main() {
testClassIdHexPinnedPerChannel(); testClassIdHexPinnedPerChannel();
testPresetRoundTripsThroughInstrumentReader(); testPresetRoundTripsThroughInstrumentReader();
@@ -230,6 +244,7 @@ int main() {
testTcpMcpFxFamilyIsHotspot(); testTcpMcpFxFamilyIsHotspot();
testFxWindowStillHotspot(); testFxWindowStillHotspot();
testNonFxSurfacesAreNotHotspot(); testNonFxSurfacesAreNotHotspot();
testEmbedStripIsNotHotspot();
if (g_fail == 0) std::printf("All tests passed.\n"); if (g_fail == 0) std::printf("All tests passed.\n");
return g_fail ? 1 : 0; return g_fail ? 1 : 0;