diff --git a/src/app/main.cpp b/src/app/main.cpp index 567c02c..0ae42b1 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -241,21 +241,19 @@ static void OnTimer() // TIMING: BeginLoadProjectState fires BEFORE any state restore, so reading // GetProjExtState here would return the PRE-undo value. Instead we raise a one-shot // reload request that OnTimer's poll() drains on the NEXT tick, once REAPER has -// finished restoring the block. A normal project open also fires this -// (isUndo=false); ignored here so a normal open flows solely through the timer's -// identity-transition Load path (no double load). +// finished restoring the block. Only the RELOAD REQUEST is undo-only: a +// normal open's reload belongs to the timer's identity-transition Load path, whose +// forked-sibling re-GUID branch requesting a reload here would bypass. static void OnBeginLoadProjectState(bool isUndo, project_config_extension_t* /*reg*/) { + // UNBRANCHED, and the reason the terminator is a hook at all: this is REAPER's one + // entry for a state load — "project load/undo", plus a new project + // (reaper_plugin.h:1203-1205) — and the last moment before the restore. Why that + // matters, and why isUndo does not narrow it: fxReclaimAbandonForStateLoad. + reasampler::fxReclaimAbandonForStateLoad(); + if (isUndo) - { - // BEFORE the reload request, and the whole reason this is a hook rather than - // something the reload's own reapply does on arrival: the reclaimer's undo block - // is open for at least kParkHoldMs after every real mode switch, and a switch is - // exactly what this undo is rolling back. Closing it here is the last moment it - // still closes over the state it was opened on. - reasampler::fxReclaimAbandonForStateLoad(); g_session.requestReload(); - } } // Intentional no-op: ReaSampler stores state via project EXT STATE, not this diff --git a/src/shell/view/CLAUDE.md b/src/shell/view/CLAUDE.md index c6bb253..5135ef8 100644 --- a/src/shell/view/CLAUDE.md +++ b/src/shell/view/CLAUDE.md @@ -84,25 +84,35 @@ decide membership or mode rules. block swallows every edit the user makes afterwards, indefinitely, so every terminating path routes through ONE idempotent close (`fxReclaimEndUndoSpan`): the drain settling, aborting on a generation bump, or deferring on the transport; the - project retargeting or closing; the next apply superseding it; a USER-DRIVEN STATE + project retargeting or closing; the next apply superseding it; ANY PROJECT-STATE LOAD; a project save; extension unload. The span may stay open only across a wait it can BOUND — the 3s debounce, which is the dead time between the switch and its first FX write, and the only reason the span exists. A TRANSPORT deferral is not bounded (it lasts as long as the playback pass), so it ends the span and the parking that follows mints its own point (`spanEndsAfter`). -- **A Ctrl-Z terminates the span itself, at the hook, before REAPER restores - anything.** `fxReclaimAbandonForStateLoad` runs from `BeginLoadProjectState` +- **EVERY project-state load terminates the span itself, at the hook, before REAPER + restores anything — `isUndo` does not narrow it.** + `fxReclaimAbandonForStateLoad` runs UNBRANCHED from `BeginLoadProjectState` (`main.cpp`) — the only point at which the block still closes over the state it was - OPENED on. The span is open for at least `kParkHoldMs` after every real switch and a - switch is exactly what a user undoes, so letting the close happen where it used to — - as a side effect of the reapply the reloaded model triggers, a full timer tick later - — closed the pre-undo block over POST-undo state, minting a spurious point and - destroying the redo branch. The same call stops any drain on the stack where it + OPENED on, and the only entry REAPER gives for a state load at all, so undo, redo, a + project open, a `File > Revert` and a new project (`reaper_plugin.h:1203-1205`) all + reach the one terminator. The span is open for at least `kParkHoldMs` after every + real switch and a switch is exactly what a user undoes, so letting the close happen + where it used to — as a side effect of the reapply the reloaded model triggers, a + full timer tick later — closed the pre-undo block over POST-undo state, minting a + spurious point and destroying the redo branch. A NON-undo load left to the next + tick's project-retarget close is the identical defect one branch away: the same block + closes over the same post-load state, a tick late. `isUndo` decides one thing only, + in `main.cpp`: whether the SESSION owes a deferred ext-state reload (a normal open's + belongs to the identity poll's Load branch, whose forked-sibling re-GUID a reload + request here would bypass). The same call stops any drain on the stack where it stands (`FxReclaimQueue::abandon`, refused at both host entry points), which is stronger than an invalidation: an invalidation still finishes the track it is on, - because a step is atomic. Daniel's ruling, and the accepted cost: undoing a switch - mid-drain leaves a PARTIAL park — some FX offlined, some not — covered by the one - point the terminator closed. + because a step is atomic. A load that keeps the SAME `ReaProject` — a revert — is + what makes that stop load-bearing rather than tidy, for the reason stated at + `abandon`. Daniel's ruling, and the accepted cost: undoing a switch mid-drain leaves + a PARTIAL park — some FX offlined, some not — covered by the one point the + terminator closed. - **A reapply that wrote nothing leaves no undo point** — and a reapply never holds the span open, so a project open, a membership edit and `render_in_place`'s inner reapply all keep the block shape they had. The load-tick reapply @@ -161,7 +171,7 @@ applies the resulting lane state to live tracks. - `view_reconcile` — the repair pass for a project state REAPER reloaded under us: `noteAppliedState` records what an apply left driven; `reconcileAppliedFlags` diffs that fingerprint against the reloaded model and hands back any track the reload no longer records but still reads driven — the fix for `UNDO_STATE_TRACKCFG` not carrying `B_SHOWINTCP`/`B_SHOWINMIXER` (DAW-confirmed). - `view_tracks` — the scaffolding every write pass sets up: `TrackHandles`/`TrackByGuid`, `readFolderEntries`/`indexHandles`, and `UiRefreshHold` (the `PreventUIRefresh` RAII pair every write phase runs under). - `view_solo` — the `I_SOLO` read/write pair behind the per-mode solo surface, plus `clearTrackSolos`/`restoreTrackSolos`, the outgoing-clear and incoming-replay entry points `view` drives them through. Holds no policy: what to cache, clear, or replay is `core/view/solo_cache`. -- `fx_reclaim` — the FX convergence loop, off the switch's synchronous path (`TrackFX_SetOffline` was 1897ms of a measured 1942ms switch). `drainTick` moves one track's FX per step against `core/view/fx_state`'s derived work list (Online before Offline; an Offline item waits out `kParkHoldMs` so a fast A→B→A flip moves nothing), budgeted per tick (`kOnlineBudgetMs`/`kOfflineBudgetMs`) and gated off the transport for the Offline direction only. `FxReclaimQueue` owns the derived queue's lifetime across re-entrancy (a plugin unload is assumed to pump). `UndoSpan` owns the ONE undo block the whole drain writes under — opened by `applyMode` on a real switch, else lazily by the first `writeOffline`, and closed through the single idempotent `fxReclaimEndUndoSpan` that every terminating path routes through (`spanEndsAfter` decides which ticks terminate; `fxReclaimAbandonForStateLoad` is the undo/redo terminator, which also stops the drain). `ReleaseFlush`/`fxReclaimFlushRelease` make a completed record's spend durable on the project's next save, never on the drain's own initiative. `fxReclaimActivity` reports WHICH DIRECTION the drain is moving for an indicator to paint — Idle whenever nothing may move, so a debounced or transport-held offline item never names a direction nothing is travelling in; undirected `fxReclaimPending` stays the timer's one idle test and must not drive a UI (it is true throughout the debounce, when only the OPPOSITE direction is eligible). +- `fx_reclaim` — the FX convergence loop, off the switch's synchronous path (`TrackFX_SetOffline` was 1897ms of a measured 1942ms switch). `drainTick` moves one track's FX per step against `core/view/fx_state`'s derived work list (Online before Offline; an Offline item waits out `kParkHoldMs` so a fast A→B→A flip moves nothing), budgeted per tick (`kOnlineBudgetMs`/`kOfflineBudgetMs`) and gated off the transport for the Offline direction only. `FxReclaimQueue` owns the derived queue's lifetime across re-entrancy (a plugin unload is assumed to pump). `UndoSpan` owns the ONE undo block the whole drain writes under — opened by `applyMode` on a real switch, else lazily by the first `writeOffline`, and closed through the single idempotent `fxReclaimEndUndoSpan` that every terminating path routes through (`spanEndsAfter` decides which ticks terminate; `fxReclaimAbandonForStateLoad` is the state-load terminator, which also stops the drain). `ReleaseFlush`/`fxReclaimFlushRelease` make a completed record's spend durable on the project's next save, never on the drain's own initiative. `fxReclaimActivity` reports WHICH DIRECTION the drain is moving for an indicator to paint — Idle whenever nothing may move, so a debounced or transport-held offline item never names a direction nothing is travelling in; undirected `fxReclaimPending` stays the timer's one idle test and must not drive a UI (it is true throughout the debounce, when only the OPPOSITE direction is eligible). ## Gotchas diff --git a/src/shell/view/fx_reclaim.h b/src/shell/view/fx_reclaim.h index 9d402a1..7162df7 100644 --- a/src/shell/view/fx_reclaim.h +++ b/src/shell/view/fx_reclaim.h @@ -360,12 +360,15 @@ public: pending_ = true; } - // A project-state load the USER drove (undo/redo). The whole project is about to be - // replaced, so this is STRONGER than an invalidation: the queue's work is discarded as - // usual, AND any drain on the stack — reachable only through a write's own message - // pump — stops where it stands instead of finishing the track it is on. Accepted, and - // the reason a step's atomicity yields here: a park half-landed this way stays - // half-landed, covered by the one point the terminator closed. + // A project-state load — undo, redo, an open, a revert, a new project. The whole + // project is about to be replaced, so this is STRONGER than an invalidation: the + // queue's work is discarded as usual, AND any drain on the stack — reachable only + // through a write's own message pump — stops where it stands instead of finishing the + // track it is on. Accepted, and the reason a step's atomicity yields here: a park + // half-landed this way stays half-landed, covered by the one point the terminator + // closed. A load that keeps the SAME ReaProject (a revert) is what makes the stop + // load-bearing rather than tidy: the drain's pin and cached track handle both still + // validate afterwards, so nothing else would keep it out of the reloaded project. void abandon() { invalidate(); abandoned_ = true; @@ -657,20 +660,23 @@ void fxReclaimLabelUndoSpan(const char* label, int mask); // The ONE close, idempotent: every terminating path routes through it — the drain // settling, aborting, or deferring on the transport; the project retargeting or closing; -// the next apply superseding it; a user-driven state load; a project save; extension +// the next apply superseding it; ANY project-state load; a project save; extension // unload. A leaked open block would swallow everything the user does afterwards, so // nothing may return past a terminating condition without calling this. void fxReclaimEndUndoSpan(); -// A project-state load the USER drove — Ctrl-Z or Ctrl-Shift-Z. Ends the span and stops -// any drain on the stack (FxReclaimQueue::abandon). +// 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 and stops any drain on the stack +// (FxReclaimQueue::abandon). // // 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 // state it was opened on. The span is open for at least kParkHoldMs after every real // switch and a switch is exactly what a user undoes, so closing it as a side effect of -// the reapply the load triggers — several calls later, over the restored project — would -// mint the point over post-undo state and destroy the redo branch. +// the load — several calls later, over the restored project — would mint the point over +// post-load state and, for an undo, destroy the redo branch. That defect does not turn on +// isUndo: a non-undo load left to the next tick's project-retarget close lands the same +// point over the same wrong state, one hook branch away, so this hook does not branch. void fxReclaimAbandonForStateLoad(); // Writes an in-memory release into the SAVING project's view state (identified via