One undo-block opener for the whole tree: fourteen sites route through it, and the reclaim span can no longer swallow a user's act

The drain refuses a tick inside a foreign block; applyMode wakes the reclaimer
before it opens the span.
This commit is contained in:
2026-08-07 16:38:11 -04:00
parent b2f6b3f1d0
commit b7b93a7ac3
25 changed files with 561 additions and 313 deletions
+46 -26
View File
@@ -122,9 +122,16 @@ decide membership or mode rules.
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`).
- **NOTHING MAY OPEN AN UNDO BLOCK WHILE THE SPAN IS OPEN — every opener ends it
first, as its first act.** REAPER's blocks are read as a DEPTH COUNTER, not a stack
of identities (`[verify — DAW]`, and the same undocumented nested-block behaviour the
Stated as ONE property over the pure state, which is how it is tested
(`tests/test_fx_reclaim.cpp`, at every terminator rather than incidentally in one
scenario): **`!pending()` implies `!span.open()`.** The timer gates every tick on
`pending()`, so a span left open with the wakeup down has nothing that could ever
close it. That is why `applyMode` raises the wakeup BEFORE it opens the span rather
than on its way out — a throw in between would otherwise leak the block with the
reclaimer asleep — and why the state-load terminator closes BEFORE it abandons.
- **NOTHING MAY OPEN AN UNDO BLOCK WHILE THE SPAN IS OPEN — and no site decides that
for itself any more.** REAPER's blocks are read as a DEPTH COUNTER, not a stack of
identities (`[verify — DAW]`, and the same undocumented nested-block behaviour the
Gotchas below name): `Undo_EndBlock2` at depth 2->1 mints nothing and drops the label
and mask it was handed, and the close that does reach depth 0 mints under whatever
label IT carries. So a block opened over the span leaves each close paired with the
@@ -134,18 +141,31 @@ decide membership or mode rules.
reading: ending the span first yields the switch's point and the opener's as two
correctly-labelled points under the counter reading AND under an identity-pairing one,
which is why it is stated as an unconditional discipline rather than a workaround for
a behaviour nobody has observed. Ending it first mints the switch's point whole, at
depth 0, and leaves the opener's block the only one in play. The
openers, all of them: `design_view_actions`' `ActionUndoBlock` (whose constructor
ends the span, so all four membership actions inherit it), `view_lanes`'
`mintManagedLanes`, and `shell/capture`'s `render_in_place`. The mirror half is
`applyMode`'s: a REAPPLY never closes a span, precisely because it is reachable from
inside one of those blocks — only a real switch supersedes, and a real switch is
always fired at depth 0. RESIDUAL, out of this rule's reach: an extension undo block
ELSEWHERE (a capture, a bank verb, an ingest) that pumps the message loop can re-enter
the timer, and a drain tick that settles in there closes the span at depth 2->1 with
the same result. Closing that needs every extension-owned `Undo_BeginBlock2` to route
through one opener that ends the span — not attempted here.
a behaviour nobody has observed. **The mechanism is `shell/undo`'s `UndoBlock`, which
every extension-owned block in the tree now opens through — its constructor ends the
span when, and only when, it is the OUTERMOST block, so a nested opener cannot close
at depth 2->1 and hijack its enclosing block.** That directory's `CLAUDE.md` owns the
mechanism and the compile-time enforcement behind it; here it closes three things at
once. First, the openers no longer have to be enumerated by hand — the former list
(`ActionUndoBlock`, `mintManagedLanes`, `render_in_place`) was never the whole set and
omitted ten user-fired blocks in `shell/capture`, `shell/bank_ops` and
`shell/actions`, each of which absorbed the user's own act into the mode switch.
Second, `mintManagedLanes` gets the same nesting criterion as everything else: it is
called from inside `doMoveItems`' and `render_in_place`' blocks, where it used to end
the span unconditionally. Third, the RESIDUAL in the other direction is closed too —
a foreign block's message pump can re-enter the timer, and a drain tick in there would
open its span at depth 2; `fxReclaimTick` now refuses outright while `undoBlockOpen()`
and consumes nothing, so it simply drains on the first tick after the block closes.
The mirror half is still `applyMode`'s: a REAPPLY never closes a span, precisely
because it is reachable from inside one of those blocks — only a real switch
supersedes, and a real switch is always fired at depth 0.
ACCEPTED, and not fixed: `mintManagedLanes` opens its block once its plan is
non-empty, but a non-empty plan whose writes are all already satisfied still mints
nothing — so a background detect tick can end a switch's span having competed with it
for nothing. Telling the two apart means running the plan, which is the write. The
cost is bounded at ONE extra undo point (the switch mints early; its remaining parking
mints again under `kReclaimUndoLabel`), the same degradation a transport deferral
already carries, where the alternative destroys the switch's point outright.
- **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`
@@ -234,13 +254,13 @@ applies the resulting lane state to live tracks.
## Modules
- `view` — Design View shell: runs the pure planner over the live folder tree and iterates its park/restore plans, applies managed-lane and parent-visibility writes, and on a REAL SWITCH ends any prior span and opens the ONE undo span all of that rides (`fx_reclaim` is what closes it; a reapply opens none and closes none). Reports what it wrote (`ModeApplyResult`) so a caller holding its own block can discard an empty point. Owns the one discriminator (`target != active`) that separates a real switch from a reapply, and with it both the playback gate (`transportBlocksModeSwitch`) and the solo cache/clear/restore seams. **Never touches master or `B_MUTE`.** FX are NOT written here — see `fx_reclaim`.
- `view` — Design View shell: runs the pure planner over the live folder tree and iterates its park/restore plans, applies managed-lane and parent-visibility writes, and on a REAL SWITCH ends any prior span, raises the reclaimer's wakeup, and opens the ONE undo span all of that rides (`fx_reclaim` is what closes it; a reapply opens none and closes none). Reports what it wrote (`ModeApplyResult`) so a caller holding its own block can discard an empty point. Owns the one discriminator (`target != active`) that separates a real switch from a reapply, and with it both the playback gate (`transportBlocksModeSwitch`) and the solo cache/clear/restore seams. **Never touches master or `B_MUTE`.** FX are NOT written here — see `fx_reclaim`.
- `view_park` — the park surface, FLAG DOMAIN ONLY. `parkTrack` captures the pre-park snapshot (flags AND the FX chain's identity-keyed offline state) when the model holds none, drives the four flags, and EXPOSEs a track that reads park-shaped with no record behind it rather than adopting it; `restoreTrack` writes the flags back from the snapshot and spends its flag half (`releaseSnapshotFlags`). Also `trackFlagParm` (the one `Flag` → REAPER-parameter mapping), `writeIfChanged` (the read-before-write every driven parameter goes through), `fxGuidString` (the one FX-identity read snapshot/reclaim both address FX through), `kViewUndoMask`, and `reportExposedTracks`/`describeExposedTracks` (named tracks, once per changed set per project).
- `view_lanes` — managed fixed lanes: `mintManagedLanes` splits multi-mode content onto per-mode lanes, `applyLaneOps` drives `C_LANEPLAYS` for the active mode, `reconcileManagedLanes` recovers ownership from durable `P_LANENAME` on project open. Manual lanes are never minted, driven, renamed or reassigned.
- `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). It opens no undo block: it runs in the wake of a Ctrl-Z, and its writes are flags only.
- `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 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).
- `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`, never while an extension undo block is open (`fxReclaimTick` refuses the tick outright, consuming nothing), 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
@@ -338,15 +358,15 @@ applies the resulting lane state to live tracks.
into sync.
- **"Nothing nests on the reapply paths" was FALSE, and cost the switch its undo
point.** `render_in_place` and the membership actions each call `applyMode` inside
their own `Undo_BeginBlock2` pair; the reapply opens no block of its own, but a prior
REAL SWITCH's span can still be open — for `kParkHoldMs` plus the whole drain — and
it is a block like any other. Two things nest there, and both are now handled by the
"nothing may open a block while the span is open" invariant: the caller's block over
the span (each caller ends the span first), and the reapply's former unconditional
supersede-close inside the caller's block (only a real switch closes now). What still
nests legitimately: `doMoveItems` and `render_in_place` over `mintManagedLanes`, both
properly paired inside a single outer block, so the SDK's undocumented nested-block
coalescing is load-bearing only for that shape.
their own block; the reapply opens none of its own, but a prior REAL SWITCH's span can
still be open — for `kParkHoldMs` plus the whole drain — and it is a block like any
other. Two things nest there, and both are now handled by the "nothing may open a
block while the span is open" invariant: the caller's block over the span (`UndoBlock`
ends it at the outermost open), and the reapply's former unconditional supersede-close
inside the caller's block (only a real switch closes now). What still nests
legitimately: `doMoveItems` and `render_in_place` over `mintManagedLanes`, all three
`UndoBlock`s, so the SDK's undocumented nested-block coalescing is load-bearing only
for that shape.
- `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays
attached to its plugin across a chain mutation while parked. That is
`[verify — DAW]` (see `fxGuidString` in `view_park.cpp`) and SWS issue #802 is a
+12 -2
View File
@@ -11,6 +11,7 @@
#include <vector>
#include "shell/persist/session.h" // flushViewState — the release's durable half
#include "shell/undo/undo_block.h" // undoBlockOpen — the reverse of the nesting rule
#include "shell/view/view.h" // modeApplyInProgress, transportBlocksModeSwitch
#include "shell/view/view_park.h" // fxGuidString — the one FX-identity read
#include "shell/view/view_tracks.h"
@@ -233,12 +234,21 @@ void invalidateFxReclaim() {
bool fxReclaimDrainInProgress() { return g_draining; }
void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
// Neither guard may close the span: both name a caller that is still on the stack and
// still owns it — the outer drain, or the applyMode that opened it. Both re-tick.
// NO guard here may close the span: each names a caller that is still on the stack and
// still owns it — the outer drain, or the applyMode that opened it. All re-tick.
if (g_draining) return;
// The model is mid-rewrite and its records are being consumed out from under any
// plan derived from it. The apply invalidates on its way out, so nothing is lost.
if (modeApplyInProgress()) return;
// The MIRROR of the rule that no block may open over the span: an extension undo
// block is open — a capture, a bank verb, an ingest — and its own message pump is how
// this tick arrived. openSpan from in there would begin at depth 2, and that block's
// close would then mint under the SPAN's label and destroy the user's own point.
// Refused whole rather than written unspanned, which would instead absorb the FX
// toggles into the user's act. Starves nothing: the wakeup, the derived queue and the
// project pin are all untouched (this returns before beginTick), so the first tick
// after the block closes drains exactly as if this one had never been due.
if (undoBlockOpen()) return;
DrainLatch latch;
// beginTick owns what a pin/project mismatch means. What is local to here: the model
+8 -4
View File
@@ -658,7 +658,9 @@ bool fxReclaimDrainInProgress();
// Drains one tick against `proj` (nullptr = current project), re-deriving the queue from
// the session's view model first if it was invalidated or if the active project moved
// (FxReclaimQueue::beginTick). Returns without touching the project while an applyMode is
// on the stack, while already draining, or when there is no valid active project at all.
// on the stack, while already draining, while any extension undo block is open
// (shell/undo's undoBlockOpen — a refusal that consumes nothing, so the next tick drains),
// or when there is no valid active project at all.
// Takes the SESSION rather than the model alone because completing an online spends the
// park record's FX half — see FxReclaimQueue::run.
void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj);
@@ -667,7 +669,8 @@ void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj);
// ONE Ctrl-Z. Called by applyMode on a REAL SWITCH ONLY — a reapply opens no block at
// all — which then returns with the span still open, and the reclaimer closes it once
// the parking has drained. No-op if a span is already open, so callers END first (see
// applyMode) rather than nesting.
// applyMode) rather than nesting. applyMode also raises the wakeup BEFORE this call: the
// tick it guarantees is the only thing that can ever close the span again.
void fxReclaimBeginUndoSpan(ReaProject* proj);
// Sets what the eventual close names the point. Separate from the begin because the
@@ -677,8 +680,9 @@ 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; 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.
// unload; and shell/undo's UndoBlock, at its OUTERMOST open. A leaked open block would
// swallow everything the user does afterwards, so nothing may return past a terminating
// condition without calling this.
void fxReclaimEndUndoSpan();
// EVERY project-state load, undo-driven or not — Ctrl-Z, Ctrl-Shift-Z, a project open, a
+7
View File
@@ -128,6 +128,13 @@ ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetMod
// CLAUDE.md. Both calls are idempotent; do not reintroduce either unconditionally.
if (realSwitch) {
fxReclaimEndUndoSpan();
// BEFORE the open, never after this function returns. The wakeup is what
// guarantees a tick, and a tick is the only thing that can close this span; a
// throw anywhere below would otherwise leave the block open with the reclaimer
// asleep, swallowing every later edit until a save, a state load, the next switch
// or unload. The wrapper's own invalidate stays where it is — it covers the
// refusal paths above, which never reach this line.
invalidateFxReclaim();
fxReclaimBeginUndoSpan(project);
}
{
+39 -29
View File
@@ -16,15 +16,13 @@
#include "core/view/lane_keys.h"
#include "core/view/view_tree.h"
#include "shell/capture/item_read.h"
#include "shell/view/fx_reclaim.h" // fxReclaimEndUndoSpan — no block opens over the span
#include "shell/undo/undo_block.h" // the ONE undo-block opener
#include "shell/view/view_park.h" // writeIfChanged — the read-before-write every driven parm takes
#define REAPERAPI_MINIMAL
#define REAPERAPI_WANT_GetMediaTrackInfo_Value
#define REAPERAPI_WANT_SetMediaTrackInfo_Value
#define REAPERAPI_WANT_GetSetMediaTrackInfo_String
#define REAPERAPI_WANT_Undo_BeginBlock2
#define REAPERAPI_WANT_Undo_EndBlock2
#define REAPERAPI_WANT_UpdateArrange
#define REAPERAPI_WANT_UpdateTimeline
#define REAPERAPI_WANT_CountTrackMediaItems
@@ -277,21 +275,46 @@ bool mintManagedLanes(ViewModeModel& model, ReaProject* proj) {
const TrackByGuid trackByGuid = indexHandles(handleByGuid);
// No block may open over a still-open reclaim span — mechanism in this directory's
// CLAUDE.md. Placed past the empty-plan return above deliberately: a detect tick
// that mints nothing must not shorten a span it never competes with.
fxReclaimEndUndoSpan();
bool changed = false;
{
// Opening ends any reclaim span — but only when this is the OUTERMOST block, so
// the two callers that already hold one (doMoveItems, render_in_place) are not
// hijacked. UndoBlock owns that criterion; do not re-end the span here.
//
// ACCEPTED, not fixed: the plan being non-empty is not the same as its writes
// being needed, so the already-satisfied case below still ends a switch's span
// having minted nothing of its own. Telling the two apart means running the plan
// first, which is the write. The cost is bounded at ONE extra undo point — the
// switch mints early and its remaining parking mints again under
// kReclaimUndoLabel, the same degradation a transport deferral already carries —
// where the alternative, opening over the span, destroys the switch's point
// outright.
//
// `proj` deliberately NOT pinned the way applyMode pins it: nothing between the
// open and the close pumps the message loop, so no tab switch can land between.
UndoBlock undo(proj);
changed = applyMintPlan(model, plan, trackByGuid);
// `proj` deliberately NOT pinned the way applyMode pins it: nothing between this
// begin and its end pumps the message loop, so no tab switch can land between them.
Undo_BeginBlock2(proj);
const bool changed = applyMintPlan(model, plan, trackByGuid);
if (changed) {
// Reapply the active mode's lane visibility so freshly-minted lanes take
// their play/show state immediately. applyMode is deliberately NOT reused
// here — it would re-park/restore whole tracks and recompute parent
// visibility, which a lane-only mint must not touch.
const TogglePlan togglePlan =
model.planToggle(FolderTree{}, model.activeModeId());
applyLaneOps(trackByGuid, togglePlan.lanes);
UpdateTimeline(); // a split happened this call — refresh is owed
UpdateArrange();
undo.recordPoint("ReaSampler: separate cross-mode content into lanes",
kViewUndoMask);
}
// Otherwise every write was already satisfied: the block's default discard form
// keeps the detect tick from flooding history with empty points.
}
if (!changed) {
// Plan was non-empty but every write was already satisfied — discard the empty
// undo point rather than flooding history every detect tick.
Undo_EndBlock2(proj, "", 0);
// The arrange still needs a redraw: this no-op path is reached when a
// freshly-inserted item already landed on the active mode's playing lane
// (REAPER places new items on the playing lane), so assignItemToLane wrote
@@ -299,21 +322,8 @@ bool mintManagedLanes(ViewModeModel& model, ReaProject* proj) {
// alone (no I_FREEMODE change happened, so UpdateTimeline isn't owed) is a
// repaint, not a mutation — stays outside the undo block.
UpdateArrange();
return false;
}
// Reapply the active mode's lane visibility so freshly-minted lanes take their
// play/show state immediately. applyMode is deliberately NOT reused here — it
// would re-park/restore whole tracks and recompute parent visibility, which a
// lane-only mint must not touch.
const TogglePlan togglePlan = model.planToggle(FolderTree{}, model.activeModeId());
applyLaneOps(trackByGuid, togglePlan.lanes);
UpdateTimeline(); // a split happened this call — refresh is owed
UpdateArrange();
Undo_EndBlock2(proj, "ReaSampler: separate cross-mode content into lanes", kViewUndoMask);
return true;
return changed;
}
void reconcileManagedLanes(ViewModeModel& model, ReaProject* proj) {