daniel/reasampler
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge drag-out hand-off fix: keep the drag internal while the pointer is over REAPER

Browse Source
This commit is contained in:
Daniel Harvey
2026-08-03 16:45:14 -04:00
parent aa87d81f1f b71a05fbef
commit a5febbd6aa
14 changed files with 271 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_drag_out.cpp
+125 -32
View File
@@ -2,8 +2,9 @@
// as the sibling pure tests: assert the gesture law and the path-list assembly directly.
//
// 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.
// * The full class matrix: every ReaperSurface x {single, multi} x {track, no track} x
// {over a host window, off it}, inside and outside the client rect, plus the half-open edge
// and a non-zero panel origin.
// * 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.
@@ -28,7 +29,8 @@ static int g_fail = 0;
static const PanelClientRect kPanel{0, 0, 400, 300};
// A live single-card drag over `surface`, with a track resolved unless stated otherwise.
// A live single-card drag over `surface`, still inside REAPER, 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};
@@ -38,13 +40,23 @@ static DropContext ctx(ReaperSurface surface, bool single = true, bool haveTrack
return c;
}
// The same drag with the shell's positive off-REAPER proof set. `surface` is whatever the SDK
// hit-test reported at that point — it still matters: the hand-off needs REAPER to have named
// nothing too, so the defaults here (Other, no track) are the only combination that hands off.
static DropContext offHost(ReaperSurface surface = ReaperSurface::Other, bool single = true,
bool haveTrack = false) {
DropContext c = ctx(surface, single, haveTrack);
c.pointerOffHost = true;
return c;
}
// 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,
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
// Pins kAllSurfaces against ReaperSurface::Count so a 6th 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).
@@ -66,6 +78,10 @@ static void testInsideClientIsAlwaysInternal() {
CHECK(decideDropClass(200, 150, kPanel, ctx(s, single)) == DropClass::Internal);
CHECK(decideDropClass(0, 0, kPanel, ctx(s, single)) == DropClass::Internal);
CHECK(decideDropClass(399, 299, kPanel, ctx(s, single)) == DropClass::Internal);
// Even a stale off-host proof cannot reinterpret a point inside our own client rect:
// the inside-client answer is resolved ahead of the hand-off gate, so re-entering the
// panel always resumes the bank-to-bank gesture.
CHECK(decideDropClass(200, 150, kPanel, offHost(s, single)) == DropClass::Internal);
}
}
}
@@ -73,7 +89,7 @@ static void testInsideClientIsAlwaysInternal() {
// 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() {
const DropContext off = ctx(ReaperSurface::OffReaper);
const DropContext off = offHost();
CHECK(decideDropClass(399, 150, kPanel, off) == DropClass::Internal);
CHECK(decideDropClass(400, 150, kPanel, off) == DropClass::OsHandoff);
CHECK(decideDropClass(200, 299, kPanel, off) == DropClass::Internal);
@@ -83,7 +99,7 @@ static void testBoundaryHalfOpen() {
// A non-zero panel origin — the boundary tracks the rect, not the absolute axes.
static void testOffsetPanelRect() {
const PanelClientRect p{50, 20, 100, 80}; // spans x[50,150) y[20,100)
const DropContext off = ctx(ReaperSurface::OffReaper);
const DropContext off = offHost();
CHECK(decideDropClass(100, 60, p, off) == DropClass::Internal);
CHECK(decideDropClass(49, 60, p, off) == DropClass::OsHandoff);
CHECK(decideDropClass(150, 60, p, off) == DropClass::OsHandoff);
@@ -129,9 +145,8 @@ static void testSingleOverOtherReaperUiRefuses() {
}
// Off REAPER entirely -> the OS drag-out, the one irreversible transition.
static void testSingleOffReaperIsOsHandoff() {
CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::OffReaper)) ==
DropClass::OsHandoff);
static void testSingleOffHostIsOsHandoff() {
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost()) == DropClass::OsHandoff);
}
// --- Matrix: outside the client, multi card ------------------------------------
@@ -154,8 +169,8 @@ static void testMultiOverArrangeIsArrangeInsert() {
}
// Multi off REAPER is the classic multi-file drag-out, unchanged.
static void testMultiOffReaperIsOsHandoff() {
CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(ReaperSurface::OffReaper, false)) ==
static void testMultiOffHostIsOsHandoff() {
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost(ReaperSurface::Other, false)) ==
DropClass::OsHandoff);
}
@@ -180,11 +195,73 @@ static void testNullTrackWithSurfaceRefuses() {
}
}
// 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);
// --- The hand-off gate ---------------------------------------------------------
// THE REGRESSION FLOOR. No reading of REAPER's hit-test can reach the modal OLE loop while the
// pointer is over a host window — asserted over EVERY surface the probe can report, both payload
// sizes and both track verdicts, not just the toolbar/transport cell that was reported. The
// defect was structural (an unnamed token read as "the user left REAPER"), so the guarantee has
// to be structural too.
static void testNoInReaperCombinationCanHandOff() {
for (ReaperSurface s : kAllSurfaces) {
for (bool single : {true, false}) {
for (bool haveTrack : {true, false}) {
CHECK(decideDropClass(kOutX, kOutY, kPanel, ctx(s, single, haveTrack)) !=
DropClass::OsHandoff);
}
}
}
}
// The converse, stated as the gate's exact shape: with the ownership proof set, the hand-off
// happens EXACTLY on the cells where REAPER's hit-test also named nothing — no surface, no track.
// Both directions in one loop, so neither half can be weakened without a failure: a gate that
// dropped the token condition fails on the named cells, one that over-tightened fails on Other.
static void testOffHostHandsOffOnlyWhereReaperNamedNothing() {
for (ReaperSurface s : kAllSurfaces) {
for (bool single : {true, false}) {
for (bool haveTrack : {true, false}) {
const bool namedNothing = (s == ReaperSurface::Other) && !haveTrack;
const DropClass c =
decideDropClass(kOutX, kOutY, kPanel, offHost(s, single, haveTrack));
CHECK((c == DropClass::OsHandoff) == namedNothing);
}
}
}
}
// The shipped exception the token condition exists for: REAPER runs a bridged plugin's UI in
// reaper_host*.exe, so the window-ownership test reports off-host over a floating bridged FX
// editor even though the pointer never left REAPER. There the recognised token vetoes the
// hand-off and the surface's own REAPER-internal outcome stands — identical to the native case.
static void testOffHostOverANamedSurfaceKeepsTheReaperOutcome() {
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost(ReaperSurface::FxSurface, true, true)) ==
DropClass::InstrumentDrop);
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost(ReaperSurface::TrackPanel, true, true)) ==
DropClass::InstrumentDrop);
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost(ReaperSurface::Arrange, true, true)) ==
DropClass::ArrangeInsert);
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost(ReaperSurface::FxEmbed, true, true)) ==
DropClass::Refuse);
// And not just for those four spot values: over every named cell the ownership proof changes
// nothing at all, which is what "a veto can only move the answer toward staying in REAPER"
// means operationally.
for (ReaperSurface s : kAllSurfaces) {
if (s == ReaperSurface::Other) continue; // the one surface the gate can hand off from
for (bool single : {true, false}) {
for (bool haveTrack : {true, false}) {
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost(s, single, haveTrack)) ==
decideDropClass(kOutX, kOutY, kPanel, ctx(s, single, haveTrack)));
}
}
}
// Same equality on the remaining named cell: Other WITH a track is a surface REAPER did
// attribute, so it refuses off-host exactly as it does on-host.
for (bool single : {true, false}) {
CHECK(decideDropClass(kOutX, kOutY, kPanel,
offHost(ReaperSurface::Other, single, true)) == DropClass::Refuse);
}
}
// --- Not-a-drag ----------------------------------------------------------------
@@ -192,14 +269,16 @@ static void testOffReaperIgnoresTrackFlag() {
// 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);
for (bool off : {false, true}) {
DropContext c = off ? offHost(s) : ctx(s);
c.drag = DragState{/*dragging=*/true, /*hasArmedSamples=*/false};
CHECK(decideDropClass(kOutX, kOutY, kPanel, c) == DropClass::None);
CHECK(decideDropClass(200, 150, kPanel, c) == DropClass::None);
c.drag = DragState{/*dragging=*/false, /*hasArmedSamples=*/true};
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);
}
}
}
@@ -219,6 +298,13 @@ static void testClassTransitionsAreReversible() {
CHECK(decideDropClass(kOutX, kOutY, kPanel, fx) == DropClass::InstrumentDrop);
CHECK(decideDropClass(kOutX, kOutY, kPanel, arrange) == DropClass::ArrangeInsert);
CHECK(decideDropClass(200, 150, kPanel, inside) == DropClass::Internal);
// The hand-off leg is irreversible only because the SHELL goes modal on it. The law itself
// stays stateless across it: evaluating an off-host context leaves the next in-REAPER
// evaluation exactly where it was.
CHECK(decideDropClass(kOutX, kOutY, kPanel, offHost()) == DropClass::OsHandoff);
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
@@ -245,9 +331,10 @@ static void testDragSpeedCannotChangeTheOutcome() {
const DropContext destination = ctx(ReaperSurface::TrackPanel);
const DropClass flick = decideDropClass(kOutX, kOutY, kPanel, destination);
// The slow path crosses everything else first.
// The slow path crosses everything else first, off-host legs included.
for (ReaperSurface s : kAllSurfaces) {
(void)decideDropClass(kOutX - 10, kOutY, kPanel, ctx(s));
(void)decideDropClass(kOutX - 10, kOutY, kPanel, offHost(s));
(void)decideDropClass(200, 150, kPanel, ctx(s)); // and back through the client
}
CHECK(decideDropClass(kOutX, kOutY, kPanel, destination) == flick);
@@ -262,9 +349,13 @@ 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);
for (bool off : {false, true}) {
const DropClass c = decideDropClass(
kOutX, kOutY, kPanel,
off ? offHost(s, single, haveTrack) : ctx(s, single, haveTrack));
CHECK(c != DropClass::None);
CHECK(c != DropClass::Internal);
}
}
}
}
@@ -409,15 +500,17 @@ int main() {
testSingleOverFxEmbedRefuses();
testSingleOverArrangeIsArrangeInsert();
testSingleOverOtherReaperUiRefuses();
testSingleOffReaperIsOsHandoff();
testSingleOffHostIsOsHandoff();
testMultiOverInstrumentSurfacesRefuses();
testMultiOverArrangeIsArrangeInsert();
testMultiOffReaperIsOsHandoff();
testMultiOffHostIsOsHandoff();
testMultiOverOtherReaperUiRefuses();
testNullTrackWithSurfaceRefuses();
testOffReaperIgnoresTrackFlag();
testNoInReaperCombinationCanHandOff();
testOffHostHandsOffOnlyWhereReaperNamedNothing();
testOffHostOverANamedSurfaceKeepsTheReaperOutcome();
testNoDragOrNoSamplesIsNone();
testClassTransitionsAreReversible();
tests/test_instrument_drop.cpp
+37 -45
View File
@@ -186,39 +186,39 @@ static void testBadClassIdRejected() {
using ui::ReaperSurface;
static ReaperSurface onTrack(const std::string& info) {
return classifyReaperSurface(info, /*haveTrack=*/true);
static ReaperSurface surfaceFor(const std::string& info) {
return classifyReaperSurface(info);
}
// 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);
CHECK(surfaceFor("fx_chain") == ReaperSurface::FxSurface);
CHECK(surfaceFor("fx_0") == ReaperSurface::FxSurface);
CHECK(surfaceFor("fx_12") == ReaperSurface::FxSurface);
}
// 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
CHECK(surfaceFor("tcp.fx") == ReaperSurface::TrackPanel);
CHECK(surfaceFor("mcp.fx") == ReaperSurface::TrackPanel);
CHECK(surfaceFor("tcp.fxbyp") == ReaperSurface::TrackPanel);
CHECK(surfaceFor("tcp.fxparm") == ReaperSurface::TrackPanel);
CHECK(surfaceFor("mcp.fxlist") == ReaperSurface::TrackPanel);
CHECK(surfaceFor("tcp.fx.1") == ReaperSurface::TrackPanel); // appended info
CHECK(surfaceFor("tcp.fx extra") == ReaperSurface::TrackPanel); // appended info, arbitrary form
}
// 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
CHECK(surfaceFor("tcp") == ReaperSurface::TrackPanel); // bare track control panel
CHECK(surfaceFor("mcp") == ReaperSurface::TrackPanel); // bare mixer control panel
CHECK(surfaceFor("tcp.mute") == ReaperSurface::TrackPanel); // mute button
CHECK(surfaceFor("tcp.vol") == ReaperSurface::TrackPanel); // volume fader
CHECK(surfaceFor("tcp.meter") == ReaperSurface::TrackPanel); // meter
CHECK(surfaceFor("tcp.f") == ReaperSurface::TrackPanel); // truncated token — still the panel
}
// The embed strip is where a ReaSampler 9000 instance already draws inline via
@@ -226,44 +226,37 @@ static void testWholeTrackPanelIsTheHotspot() {
// 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);
CHECK(surfaceFor("tcp.fxembed") == ReaperSurface::FxEmbed);
CHECK(surfaceFor("mcp.fxembed") == ReaperSurface::FxEmbed);
CHECK(surfaceFor("tcp.fxembed.1") == ReaperSurface::FxEmbed);
CHECK(surfaceFor("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);
CHECK(surfaceFor("arrange") == ReaperSurface::Arrange);
CHECK(surfaceFor("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);
CHECK(surfaceFor("spacer_0") == ReaperSurface::Other);
CHECK(surfaceFor("trans") == ReaperSurface::Other);
CHECK(surfaceFor("envcp") == ReaperSurface::Other);
CHECK(surfaceFor("ruler") == ReaperSurface::Other);
CHECK(surfaceFor("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);
// An empty info string is Other, NOT an off-REAPER verdict — ui::DropContext::pointerOffHost
// owns why.
static void testEmptyInfoIsOtherNotOffReaper() {
CHECK(surfaceFor("") == 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);
}
// There is deliberately no second empty-info case: track presence is not an input to the
// classifier, and "no classifier output alone can trigger a hand-off" is structural (no such
// member exists) rather than something a per-token loop could falsify.
// Do not reintroduce a per-surface "capture carries" loop test: buildInstrumentDropPreset takes
// only sampleId (proven by testPresetRoundTripsThroughInstrumentReader), and per-surface
@@ -324,8 +317,7 @@ int main() {
testEmbedStripIsItsOwnSurface();
testArrangeIsArrange();
testUnnamedReaperSurfacesAreOther();
testEmptyInfoSplitsOnTrackPresence();
testNullTrackStillClassifiesTheSurface();
testEmptyInfoIsOtherNotOffReaper();
testAddFailureLeavesNothingToRollBack();
testPresetFailureRollsBackTheCreatedIndex();