diff --git a/src/app/main.cpp b/src/app/main.cpp index 0ae42b1..ef62f3a 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -183,6 +183,17 @@ static void OnTimer() // the idle fast-path is a SINGLE POINTER TEST — drive only when a capture is live. if (capture::g_rtCapture) capture::DriveRealtimeCapture(g_session); + // A project load or save is IN FLIGHT, so this tick is running inside its own message + // pump — a restore brings plugins online and a plugin load pumps. Everything below + // would then act on a half-restored project: poll() reads the pre-load that + // OnBeginLoadProjectState defers past precisely because it is wrong, and the reclaim + // tick writes FX into a project REAPER is still rewriting. The extension's other + // re-entrancy guards cover OUR pumps; this one covers REAPER's. Consumes nothing, so + // the first tick after the load runs as if this one had never been due. Self-lowering + // (reaper_plugin_functions.h:1584-1587), and placed after the realtime drive so that + // fast path keeps its single-pointer-test shape. See src/shell/view/CLAUDE.md. + if (GetCurrentProjectInLoadSave()) return; + // poll()'s undo/redo-reload and project-switch Load branches call // loadFromProject, replacing g_session.view() wholesale — deferred here while // an applyMode is on the stack (reachable if a pumped message loop re-enters diff --git a/src/shell/view/CLAUDE.md b/src/shell/view/CLAUDE.md index 1f7f005..79b86ae 100644 --- a/src/shell/view/CLAUDE.md +++ b/src/shell/view/CLAUDE.md @@ -108,9 +108,12 @@ decide membership or mode rules. ONE undo block: `applyMode` opens it on a REAL SWITCH ONLY (`fx_reclaim`'s `UndoSpan`) and RETURNS WITH IT OPEN, for the reclaimer to close once the drain has converged. This does not move an FX write back onto the switch — the writes still - run on the timer, off the 1897ms path. Whether a block held from an ACTION into a - later timer callback holds is `[verify — DAW]`; if it does not, the switch and its - parking land as two points, which is the intended degradation. + run on the timer, off the 1897ms path. **DAW-CONFIRMED, no longer inferred:** a block + opened in the `applyMode` ACTION and closed several timer callbacks later by the drain + holds — switch, then an undisturbed parking drained to completion, then one Ctrl-Z gave + exactly ONE undo entry and returned to the previous mode. (The two-point degradation + this once hedged against never materialised; the degradations that DO occur are the + transport deferral and `mintManagedLanes`' early close, both named below.) - **No path may return past a terminating condition with the span open.** A leaked block swallows every edit the user makes afterwards, indefinitely, so every terminating path routes through ONE idempotent close (`fxReclaimEndUndoSpan`): the @@ -225,7 +228,30 @@ decide membership or mode rules. OFFLINE work once a track has read continuously parked for `kParkHoldMs` (3000ms). A flip inside that window flips the flags twice but the debounce never matures, so the reclaimer finds every chain already where the record - wants it and writes nothing. + wants it and writes nothing. **Undoing a switch mid-park is that same flip**, so a + state load DROPS the holds where an invalidation keeps them (`FxReclaimQueue::abandon`, + and again at the end of `run` for an abandon that arrived through a write's own pump, + which found the clock moved into the drain's local). Keeping them fired the post-load + re-park on the FIRST tick after the load, with zero debounce, in the wake of REAPER's + own restore; re-earning them is what makes the undo cost nothing and keeps the writes + clear of the load. +- **NOTHING THE EXTENSION DOES PER TICK RUNS WHILE REAPER IS IN A PROJECT LOAD OR SAVE.** + `BeginLoadProjectState` fires before REAPER restores anything, and the restore itself + brings plugins online — a plugin load pumps the message loop, which is how a timer tick + lands INSIDE the load. `main.cpp`'s `OnTimer` therefore returns immediately while + `GetCurrentProjectInLoadSave()` names a project + (`reaper_plugin_functions.h:1584-1587`), self-lowering, with no latch to leak. Without + it, that tick's `poll()` reads the half-restored `` — the very pre-load value + the hook defers past because it is wrong — the reapply plans against it, and the reclaim + tick writes FX into a project REAPER is still rewriting; each such round is its own + settle-and-close, so a re-convergence that owes ONE point costs one per round. That last + step is reachable by inspection, not measured: the DAW report it answers is one undo + entry per FX after a Ctrl-Z mid-drain, and whether this is the whole of that cause is + `[verify — DAW]`. The extension's other re-entrancy guards (`modeApplyInProgress`, + `fxReclaimDrainInProgress`) cover OUR pumps only; this is the one that covers REAPER's. + It consumes nothing — reload request, load signal, wakeup, derived queue and pin all + stand. The realtime-capture drive deliberately sits AHEAD of it, keeping its + single-pointer-test idle path and its behaviour across a load unchanged. - **Show-both semantics:** a per-track "pin visible across modes" flag re-enables processing whenever shown. A show-both leaf appears in every mode's visible set and is never parked — its driven flags stay at snapshot/restored values, FX diff --git a/src/shell/view/fx_reclaim.h b/src/shell/view/fx_reclaim.h index bff05c7..e131fea 100644 --- a/src/shell/view/fx_reclaim.h +++ b/src/shell/view/fx_reclaim.h @@ -386,8 +386,18 @@ public: abandoned_ = true; discard(); pending_ = false; - // The pin STAYS — it is what the next tick's retarget tests the debounce clock - // against, and that clock outlives this exactly as it outlives an invalidation. + // The HOLDS GO, where an invalidation keeps them — the one place the two differ on + // the clock, and for the same reason they differ on the wakeup. A hold records "this + // track has read continuously parked since T", an observation about the project this + // load is replacing; after the load the chains may sit anywhere (an undo of a park + // restores them online) and no such continuity is in evidence. Keeping it fired the + // post-load re-park on the FIRST tick after the load, with zero debounce, right in + // the wake of REAPER's restore. Undoing a switch mid-park is precisely the A->B->A + // flip kParkHoldMs exists to make free: re-earning the hold is what makes it free + // again, and what keeps the re-convergence's writes clear of the load. + clock_.forget(); + // The pin STAYS — beginTick compares it to decide a retarget, and losing it there + // would read as "no project" rather than "same project, nothing derived yet". } // Read by the shell's host on every step; false again from the next run(), because the @@ -477,6 +487,11 @@ public: // The hold is keyed on the track and outlives any re-derivation, so it comes // back whether or not the drain's own work survives. clock_.swap(clock); + // ...but not across a state load. An abandon reaching us through a write's own + // message pump found the clock already moved out into the drain's local, so its + // forget cleared an empty member; redo it now the holds are back. `abandoned_` is + // cleared at this call's entry, so it can only mean an abandon landed inside it. + if (abandoned_) clock_.forget(); // Assembled field by field rather than positionally: the aggregate is read by // the undo span's close decision now, and a silently mis-slotted bool there @@ -686,8 +701,9 @@ void fxReclaimLabelUndoSpan(const char* label, int mask); void fxReclaimEndUndoSpan(); // EVERY project-state load, undo-driven or not — Ctrl-Z, Ctrl-Shift-Z, a project open, a -// File > Revert, a new project. Ends the span, stops any drain on the stack, and leaves the -// loop ASLEEP until something that re-read the model wakes it (FxReclaimQueue::abandon). +// File > Revert, a new project. Ends the span, stops any drain on the stack, drops the +// debounce holds, and leaves the loop ASLEEP until something that re-read the model wakes +// it — FxReclaimQueue::abandon owns all four and the reasoning behind each. // // Called from the projectconfig BeginLoadProjectState hook, which fires BEFORE REAPER // restores anything: that is the ONLY point at which the block can still close over the diff --git a/tests/test_fx_reclaim.cpp b/tests/test_fx_reclaim.cpp index b37c5a1..685078f 100644 --- a/tests/test_fx_reclaim.cpp +++ b/tests/test_fx_reclaim.cpp @@ -1272,14 +1272,75 @@ static void testAStateLoadStopsTheDrainWhereItStandsRatherThanFinishingTheTrack( CHECK(q.stale()); CHECK(!q.pending()); - // ...and the stop belonged to that tick alone. The reloaded project's own drain writes. + // ...and the stop belonged to that tick alone. The reloaded project's own drain writes — + // but not before its holds are earned again: the abandon dropped them (it reached the + // clock through the drain's own local, see FxReclaimQueue::run), so the re-park starts + // its 3s from the first post-load tick rather than inheriting the pre-load observation. host.abandonAfterWrite = -1; q.adopt(derive(vm, {"{T1}", "{T2}"})); + CHECK(q.run(host).writes == 0); // first post-load tick: the holds start here + CHECK(!host.offlineAt("{T1}", 1)); + host.clock += kParkHoldMs; CHECK(q.run(host).writes == 2); // {T1}'s remaining slot, and {T2}'s whole chain CHECK(host.offlineAt("{T1}", 1)); CHECK(host.offlineAt("{T2}", 0)); } +static void testTheWholePostLoadReconvergenceIsOneSpanHoweverFarTheParkingGot() { + // The acceptance property behind a Ctrl-Z landing anywhere in the drain: whatever + // fraction of the parking had completed, everything the reloaded project owes rides + // ONE span and costs ONE point. Two things carry it — the abandon dropping the holds, + // so nothing writes on the tick that re-derives (the load's own wake), and the lazy + // open being the drain's, so the many ticks that follow share one block. + ViewModeModel vm = parkedDesignLeaves({{"{T1}", {"{FX-A}", "{FX-B}"}}, + {"{T2}", {"{FX-C}"}}, + {"{T3}", {"{FX-D}"}}}); + FakeHost host; + host.chains["{T1}"] = onlineChain({"{FX-A}", "{FX-B}"}); + host.chains["{T2}"] = onlineChain({"{FX-C}"}); + host.chains["{T3}"] = onlineChain({"{FX-D}"}); + host.stepCostMs = 40.0; // over the offline budget: one track per tick, many ticks + + UndoSpan span; // the switch's, still open when the undo arrives + CHECK(span.begin(kProjA, "ReaSampler: activate Design view", 9)); + host.span = &span; + + FxReclaimQueue q = queueOf(vm, {"{T1}", "{T2}", "{T3}"}); + q.run(host); + host.clock += kParkHoldMs; + host.reentrant = &q; + host.abandonAfterWrite = 1; // Ctrl-Z one write in — the parking is part-done + q.run(host); + CHECK(!span.open()); // the terminator closed the switch's point over pre-load state + host.reentrant = nullptr; + host.abandonAfterWrite = -1; + + // The reloaded model's reapply raises the wakeup and the queue re-derives. + q.invalidate(); + q.adopt(derive(vm, {"{T1}", "{T2}", "{T3}"})); + + int closes = 0; + auto tick = [&]() { + const ReclaimRunResult r = q.run(host); + if (spanEndsAfter(r) && span.end()) ++closes; + return r; + }; + + CHECK(!tick().settled); // the holds start here, so this tick writes nothing at all + CHECK(host.spanOpens == 0); + CHECK(!span.open()); + CHECK(closes == 0); + + host.clock += kParkHoldMs; + int ticks = 0; + while (!tick().settled && ++ticks < 20) {} + CHECK(ticks >= 2); // it really did take several ticks to converge... + CHECK(host.spanOpens == 1); // ...under ONE span, opened by the drain's first write + CHECK(closes == 1); // ...closed exactly once, on the tick that settled + CHECK(!span.open()); + CHECK(host.offlineAt("{T1}", 1) && host.offlineAt("{T2}", 0) && host.offlineAt("{T3}", 0)); +} + static void testARevertBetweenTicksLeavesTheLoopAsleepOnAStaleModel() { // File > Revert is the load that re-reads NOTHING: it keeps the same ReaProject, so the // session's identity poll answers NoOp, no reload follows and therefore no reapply @@ -1576,6 +1637,7 @@ int main() { testADrainNoSwitchOpenedMintsOnePointOfItsOwn(); testAStateLoadStopsTheDrainWhereItStandsRatherThanFinishingTheTrack(); + testTheWholePostLoadReconvergenceIsOneSpanHoweverFarTheParkingGot(); testARevertBetweenTicksLeavesTheLoopAsleepOnAStaleModel(); testTheDirectionReadsOnlineWhileTheParksOwnOfflineWorkWaitsOutItsHold();