diff --git a/src/shell/actions/design_view_actions.cpp b/src/shell/actions/design_view_actions.cpp index c8176eb..2147866 100644 --- a/src/shell/actions/design_view_actions.cpp +++ b/src/shell/actions/design_view_actions.cpp @@ -25,6 +25,7 @@ #include "shell/capture/item_read.h" // shared MediaItem* -> GUID + fixed-lane-name reads (D2 W3-B) #include "shell/capture/track_guid.h" // shared MediaTrack* -> canonical GUID key #include "shell/panel/panel_window.h" // bankPanelInvalidate — footer toggle repaint +#include "shell/view/fx_reclaim.h" // fxReclaimEndUndoSpan — no block opens over the span #include "shell/view/view.h" // applyMode (D2 shell) #include "shell/view/view_lanes.h" // mintManagedLanes #include "shell/view/view_park.h" // kViewUndoMask @@ -105,12 +106,16 @@ std::vector selectedTrackGuids() { } // Reapplies the CURRENT active mode so a membership mutation takes visible effect -// immediately (park/unpark/re-derive parents). The return is deliberately discarded: a -// refused reapply leaves the membership edit standing and unapplied, and the user's next -// edit or mode switch reapplies it. What must NOT be dropped with it is the reclaimer's -// queue, which is why applyMode invalidates on its refusal path too (view.cpp). -void reapplyActiveMode() { - applyMode(g_session->view(), g_session->view().activeModeId(), nullptr); +// immediately (park/unpark/re-derive parents), answering whether it wrote any project +// state — the caller's block mints only if something did. `applied` is deliberately +// discarded: a refused reapply leaves the membership edit standing and unapplied, and +// the user's next edit or mode switch reapplies it (a refusal writes nothing, so it +// reports false here and the block discards). What must NOT be dropped with it is the +// reclaimer's queue, which is why applyMode invalidates on its refusal path too +// (view.cpp). +bool reapplyActiveMode() { + return applyMode(g_session->view(), g_session->view().activeModeId(), nullptr) + .wroteProjectState; } constexpr int kFreeModeFixedLanes = 2; // I_FREEMODE value; mirrors the shell's constant @@ -189,15 +194,21 @@ void doToggleMode() { const std::string target = nextModeId(g_session->view().modes(), g_session->view().activeModeId()); if (target.empty()) return; // no modes to cycle to (degenerate) - if (!applyMode(g_session->view(), target, nullptr)) { reportModeSwitchRefused(); return; } + if (!applyMode(g_session->view(), target, nullptr).applied) { + reportModeSwitchRefused(); + return; + } persistViewState(PersistScope::ViewOnly); bankPanelInvalidate(); // repaint the footer [Arrange|Design] toggle immediately } -// Direct jump to a named mode. applyMode is a no-op (returns false, no mutation) if +// Direct jump to a named mode. applyMode is a no-op (`applied` false, no mutation) if // the id is unregistered or the transport is running, so both fail safe. void doActivateMode(const std::string& modeId) { - if (!applyMode(g_session->view(), modeId, nullptr)) { reportModeSwitchRefused(); return; } + if (!applyMode(g_session->view(), modeId, nullptr).applied) { + reportModeSwitchRefused(); + return; + } persistViewState(PersistScope::ViewOnly); bankPanelInvalidate(); // repaint the footer [Arrange|Design] toggle immediately } @@ -208,18 +219,34 @@ void doActivateMode(const std::string& modeId) { // actually fired rather than the reapply it happened to trigger. Every one of them // closes the block before persistViewState, which can pop a modal Save-As. Stack RAII // so a throw between the two calls cannot leave the block open in the user's project. +// +// ENDING THE RECLAIM SPAN IS PART OF OPENING, and every action here inherits it from +// this one constructor. A mode switch fired seconds ago has left the span open; a block +// opened over it makes each close pair with the other's open, collapsing the switch +// into this tag — mechanism and full rule in src/shell/view/CLAUDE.md. +// +// `markWrote` decides whether this block mints at all: re-tagging tracks that already +// sit in that mode writes nothing to the project, and the ("", 0) discard form is real +// from a hand-fired action (DAW-measured, same doc). The membership move itself is +// never in the point either way — persistViewState runs after the close. class ActionUndoBlock { public: ActionUndoBlock(std::string label, int mask) : label_(std::move(label)), mask_(mask) { + fxReclaimEndUndoSpan(); Undo_BeginBlock2(nullptr); } - ~ActionUndoBlock() { Undo_EndBlock2(nullptr, label_.c_str(), mask_); } + ~ActionUndoBlock() { + if (wrote_) Undo_EndBlock2(nullptr, label_.c_str(), mask_); + else Undo_EndBlock2(nullptr, "", 0); + } + void markWrote() { wrote_ = true; } ActionUndoBlock(const ActionUndoBlock&) = delete; ActionUndoBlock& operator=(const ActionUndoBlock&) = delete; private: std::string label_; int mask_; + bool wrote_ = false; }; // tag() replaces any prior single-mode membership (a leaf lives in one mode; the @@ -231,7 +258,7 @@ void doTag(const std::string& modeId) { { ActionUndoBlock undo("ReaSampler: tag selected tracks -> " + modeId, kViewUndoMask); for (const std::string& g : guids) g_session->view().membership().tag(g, modeId); - reapplyActiveMode(); + if (reapplyActiveMode()) undo.markWrote(); } persistViewState(PersistScope::Full); } @@ -245,7 +272,7 @@ void doUntag() { { ActionUndoBlock undo("ReaSampler: untag selected tracks", kViewUndoMask); for (const std::string& g : guids) g_session->view().membership().untag(g); - reapplyActiveMode(); + if (reapplyActiveMode()) undo.markWrote(); } persistViewState(PersistScope::Full); } @@ -260,7 +287,7 @@ void doShowBoth() { ActionUndoBlock undo("ReaSampler: toggle show-both on selected tracks", kViewUndoMask); MembershipIndex& m = g_session->view().membership(); for (const std::string& g : guids) m.setShowBoth(g, !m.isShowBoth(g)); - reapplyActiveMode(); + if (reapplyActiveMode()) undo.markWrote(); } persistViewState(PersistScope::Full); } @@ -290,8 +317,11 @@ void doMoveItems(const std::string& targetMode) { } // Re-drive the same minting/apply path auto-tag uses: mint/split lanes for any // track whose items now span modes, then reassert active-mode lane visibility. - mintManagedLanes(g_session->view(), nullptr); - reapplyActiveMode(); + // Either half writing is enough to mint; a retag that split no lanes and drove + // no flag leaves nothing for a point to hold. + const bool minted = mintManagedLanes(g_session->view(), nullptr); + const bool reapplied = reapplyActiveMode(); + if (minted || reapplied) undo.markWrote(); } persistViewState(PersistScope::Full); diff --git a/src/shell/capture/render_in_place.cpp b/src/shell/capture/render_in_place.cpp index dfaf22c..cab8935 100644 --- a/src/shell/capture/render_in_place.cpp +++ b/src/shell/capture/render_in_place.cpp @@ -21,6 +21,7 @@ #include "shell/capture/track_guid.h" // guidString #include "shell/panel/panel_input.h" // bankPanelTailSetting #include "shell/persist/session.h" +#include "shell/view/fx_reclaim.h" // fxReclaimEndUndoSpan #include "shell/view/view.h" // applyMode #include "shell/view/view_lanes.h" // mintManagedLanes @@ -133,6 +134,11 @@ void RunRenderTrackInPlace(ReaSamplerSession& session) { // visible — never to choose a tag. Both tags below are absolute. const std::string activeMode = session.view().activeModeId(); + // A mode switch fired seconds ago has left the FX reclaimer's undo span open, and + // no block may open over it — mechanism in src/shell/view/CLAUDE.md. Ending it here + // mints the switch's point, whole, before this block exists; idempotent otherwise. + fxReclaimEndUndoSpan(); + Undo_BeginBlock2(nullptr); // flags = 0, never 1: flags&1 adds default envelopes/FX, and a default chain diff --git a/src/shell/view/CLAUDE.md b/src/shell/view/CLAUDE.md index 03d8c10..2c5abdb 100644 --- a/src/shell/view/CLAUDE.md +++ b/src/shell/view/CLAUDE.md @@ -74,11 +74,34 @@ decide membership or mode rules. block per write, and ONE under a single block spanning all ten — block WIDTH is the entire effect there. The `("", 0)` discard form does NOT discard outside an action, it coalesces; it discards only from a hand-fired action. A block opened on one timer - callback and closed on a LATER one holds. So: a path that writes only flags opens NO - block (a repair or a reapply must not cost the user a Ctrl-Z), and a path whose act - IS the user's opens one to make that act undoable at all — which is why the three - membership-edit actions own their own (`design_view_actions`' `ActionUndoBlock`) - rather than leaning on a reapply that now mints nothing. + callback and closed on a LATER one holds. + **The four flags above are the whole of what was measured.** The lane parameters this + directory also writes with no block behind them — `C_LANEPLAYS`, `I_FREEMODE`, + `C_LANESCOLLAPSED`, `C_LANESETTINGS` (`view_lanes`' `applyLaneOps`) — are assumed + silent BY INFERENCE from the four, never observed: `[verify — DAW]`. If they turn out + to mint unaided, a reapply that repairs a lane leaves a stray point the criterion + below did not intend; that is the failure mode to look for. +- **ONE CRITERION decides who opens an undo block: the path whose own act has to + survive as a point, and no other.** Two things make an act the path's own — it was + explicitly fired by the user (a real mode switch, the membership actions, + `render_in_place`), or it makes a STRUCTURAL project change no re-derivation + reproduces (`view_lanes`' `mintManagedLanes`, which creates lanes and re-homes items; + the panel's detect tick fires it, so it is nobody's action, and it opens a block + anyway because nothing else would ever put those lanes back). Everything else + RE-ASSERTS driven state the view model recomputes from membership on every apply — + the park/restore flags, parent visibility, and `applyLaneOps`' `C_LANEPLAYS` plus its + `I_FREEMODE` re-assert on a track that already owns managed lanes — and opens none, + because a block there could only manufacture the point a project open or a membership + edit must not cost. This is why the same `I_FREEMODE` write is blocked at one site and + not the other: minting SPLITS a track, `applyLaneOps` only re-states which lane plays. + Second clause, binding every site that does open one: **a block that wrote nothing + closes on the discard form** (`Undo_EndBlock2(proj, "", 0)` — real from a hand-fired + action, per the measurement above), so re-tagging tracks already in that mode leaves + no empty point. The one exception is a REAL SWITCH, which mints written state or not: + it is an explicitly fired action and has to stay undoable even where the plan found + nothing to write. "Wrote" means a REAPER write throughout — model moves persist + through `persistViewState`, outside every block, so no point could carry them + (`ModeApplyResult`, `view.h`). - **ONE mode switch is ONE undo point — the background FX parking included.** The driven flags, the lane state, AND the per-FX offline/online the reclaimer writes minutes-of-plugin-loads later all roll back in a single Ctrl-Z, because they share @@ -92,12 +115,37 @@ 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; ANY PROJECT-STATE - LOAD; a project save; extension unload. The span may stay open only across a wait it + project retargeting or closing; the next REAL SWITCH superseding it; ANY PROJECT-STATE + LOAD; a project save; extension unload; and any path about to open a block of its own + (next invariant). 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`). +- **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 + 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 + other's open — the switch's point gone, its parking relabelled as the intruder's act, + and the `UNDO_STATE_FX` the drain widened the span to (`UndoSpan::widen`) dropped, + which is the exact loss the widen exists to prevent. The RULE does not rest on that + 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. - **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` @@ -137,13 +185,13 @@ decide membership or mode rules. rather than resuming on the next tick. (That a revert leaves the view model stale at all is the identity poll's defect, not the reclaimer's — this invariant only keeps the reclaimer from acting on it.) -- **A reapply opens no undo block at all, so it can leave no point** — whether it - wrote or not, and whoever drove it: a project open, a membership edit, - `render_in_place`'s inner reapply. Nothing a reapply writes mints unaided (see the - measurement above), so a block there could only manufacture the Ctrl-Z opening a - project must not cost. A caller whose own act needs to stay undoable spans it - itself. A REAL switch still always mints, written state or not: it is an explicitly - fired action and has to stay undoable even when the plan found nothing to write. +- **A reapply opens no undo block and CLOSES NO SPAN, so it can leave no point** — + whether it wrote or not, and whoever drove it: a project open, a membership edit, + `render_in_place`'s inner reapply. The open half is the criterion above (a reapply + re-asserts derived state and is nobody's act); the close half is the nesting rule + above (a reapply runs inside its driver's block, so a supersede-close from there + would land at depth 2->1 and hijack it). It reports what it wrote instead, and the + driver holding the block decides — `ModeApplyResult::wroteProjectState`. - **A rapid A→B→A flip moves zero plugins.** The reclaimer only starts a park's OFFLINE work once a track has read continuously parked for `kParkHoldMs` (3000ms). A flip inside that window flips the flags twice but the debounce @@ -186,7 +234,7 @@ 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 opens the ONE undo span all of that rides (`fx_reclaim` is what closes it; a reapply opens none). 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 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. @@ -288,12 +336,17 @@ applies the resulting lane state to live tracks. refusal leaves the membership tags it already wrote in place, so the next reapply — not a user re-fire of render-in-place — is what brings the tracks into sync. -- `render_in_place` calls `applyMode` inside its own `Undo_BeginBlock2` pair, and - that block is now the only one in play on that path — the reapply opens none of - its own, so nothing nests and the SDK's undocumented nested-block coalescing - stopped being load-bearing here. The same holds for the membership actions' - `ActionUndoBlock`; `doMoveItems` is the one that still nests, over - `mintManagedLanes`. +- **"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. - `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 diff --git a/src/shell/view/view.cpp b/src/shell/view/view.cpp index 7143d62..b9ddb6f 100644 --- a/src/shell/view/view.cpp +++ b/src/shell/view/view.cpp @@ -63,19 +63,20 @@ bool modeApplyInProgress() { return g_applying; } namespace { -bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj) { - if (g_applying) return false; // see ApplyLatch; same fail-closed shape as the guards below +ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetModeId, + ReaProject* proj) { + if (g_applying) return {}; // see ApplyLatch; same fail-closed shape as the guards below ApplyLatch latch; if (!model.modes().contains(targetModeId)) { - return false; // reject before touching the project — no partial apply + return {}; // reject before touching the project — no partial apply } // Pinned ONCE: resolving "current project" independently at the begin and the end // would let a tab switch mid-apply open the block on one project and close it on // another. ReaProject* const project = proj ? proj : EnumProjects(-1, nullptr, 0); - if (!ValidatePtr2(nullptr, project, "ReaProject*")) return false; + if (!ValidatePtr2(nullptr, project, "ReaProject*")) return {}; // The discriminator behind both halves of the contract in view.h. A gate placed // unconditionally here would break tagging and the project-load reapply during @@ -85,7 +86,7 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro const bool realSwitch = targetModeId != outgoingModeId; if (realSwitch && transportBlocksModeSwitch(project)) { - return false; // same fail-closed shape as the mode-exists guard above + return {}; // same fail-closed shape as the mode-exists guard above } TrackHandles handleByGuid; @@ -115,17 +116,20 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro (targetMode ? targetMode->displayName : targetModeId) + " view"; bool laneModeChanged = false; + bool wrote = false; // REAPER writes only — see ModeApplyResult std::vector exposedNames; // named while handles are fresh, reported after the writes - // A prior switch's span, if the reclaimer has not closed it yet: this apply - // supersedes it, so that parking lands as its own point rather than nesting under - // ours. Idempotent — it reaches nothing when none is open. - fxReclaimEndUndoSpan(); - - // ONLY a real switch opens one. Nothing a reapply writes mints on its own, so a - // block around it could only manufacture the undo point a project open must not - // cost — do not reintroduce one here (see this directory's CLAUDE.md). - if (realSwitch) fxReclaimBeginUndoSpan(project); + // ONLY A REAL SWITCH, both halves — end a prior switch's still-open span, then open + // this one's. A REAPPLY must do neither: it opens none because nothing it writes + // mints unaided, and it CLOSES none because it is reachable from inside a caller's + // own block (a membership action, render_in_place), where a close would land at + // depth 2->1 and hijack the switch's point instead of minting it. A caller that + // opens a block ends the span itself, first, at depth 0 — see this directory's + // CLAUDE.md. Both calls are idempotent; do not reintroduce either unconditionally. + if (realSwitch) { + fxReclaimEndUndoSpan(); + fxReclaimBeginUndoSpan(project); + } { UiRefreshHold uiHold; // every write below lands with the TCP/MCP frozen @@ -144,6 +148,7 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro MediaTrack* tr = resolve(trackByGuid, guid); if (!tr) continue; // stale/deleted GUID — prune const TrackParkResult r = parkTrack(model, guid, tr, tp.flags); + if (r.wrote) wrote = true; // Named here, not after the block: TrackList_AdjustWindows below can // invalidate the handle. if (r.exposed) exposedNames.push_back(trackDisplayName(tr)); @@ -154,13 +159,14 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro const std::string& guid = tp.flags.front().guid; MediaTrack* tr = resolve(trackByGuid, guid); if (!tr) continue; // stale/deleted GUID — prune - restoreTrack(model, guid, tr, tp.flags); + if (restoreTrack(model, guid, tr, tp.flags)) wrote = true; } // MANAGED LANES: the active mode's lane plays+shows, every other managed lane // is silenced+hidden. Empty on a D1-only project — byte-identical there. const LaneApplyResult lanes = applyLaneOps(trackByGuid, plan.lanes); laneModeChanged = lanes.freeMode; + if (lanes.wrote) wrote = true; // PARENT VISIBILITY (never parked): recomputed every toggle, never recorded. // Only the two visibility flags — never mainSend/FX on a parent. @@ -170,8 +176,8 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro MediaTrack* tr = resolve(trackByGuid, node.guid); if (!tr) continue; // stale GUID — prune const double show = visible.count(node.guid) ? 1.0 : 0.0; - writeIfChanged(tr, "B_SHOWINTCP", show); - writeIfChanged(tr, "B_SHOWINMIXER", show); + if (writeIfChanged(tr, "B_SHOWINTCP", show)) wrote = true; + if (writeIfChanged(tr, "B_SHOWINMIXER", show)) wrote = true; } model.setActiveMode(targetModeId); @@ -212,13 +218,14 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro // an undo rolling the model back underneath it. In memory, never on disk. noteAppliedState(project, model); - return true; + return ModeApplyResult{true, wrote}; } } // namespace -bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj) { - const bool applied = applyModeImpl(model, targetModeId, proj); +ModeApplyResult applyMode(ViewModeModel& model, const std::string& targetModeId, + ReaProject* proj) { + const ModeApplyResult result = applyModeImpl(model, targetModeId, proj); // Every membership, snapshot and active-mode mutation in the system reaches the // project through this call, so this is the whole invalidation surface: the @@ -231,7 +238,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject // else would drop a queue that now converges toward the mode the user just left. // The cost when the refusal followed no mutation is one re-derivation on the timer. invalidateFxReclaim(); - return applied; + return result; } } // namespace reasampler diff --git a/src/shell/view/view.h b/src/shell/view/view.h index 8c3c973..3df9b7b 100644 --- a/src/shell/view/view.h +++ b/src/shell/view/view.h @@ -27,6 +27,21 @@ bool transportBlocksModeSwitch(ReaProject* proj); // one-shot signal on a call that would be refused — see main.cpp's load glue. bool modeApplyInProgress(); +// What one applyMode did. +// +// `wroteProjectState` counts REAPER writes ONLY — a driven flag, a lane parameter, +// a parent's visibility. Model moves (a snapshot stored, a record's flag half spent, +// the active mode set) are deliberately excluded: they reach disk through +// persistViewState, which every caller runs OUTSIDE its undo block, so no point can +// carry them and counting them would only mint an empty one. It is the answer a +// caller holding its own block needs for the discard decision (design_view_actions' +// ActionUndoBlock); the solo pair cannot affect it, since only a real switch drives +// solo and a real switch mints regardless. +struct ModeApplyResult { + bool applied = false; + bool wroteProjectState = false; +}; + // Captures each about-to-park track's prior flag values into `model`, caches/clears // the outgoing mode's solo state and replays the incoming mode's, runs planToggle, // applies the park/restore flag writes plus managed-lane and parent-visibility @@ -41,16 +56,20 @@ bool modeApplyInProgress(); // A REAL switch therefore RETURNS WITH ITS UNDO BLOCK STILL OPEN (fx_reclaim's // UndoSpan): the reclaimer closes it once the parking has drained, which is what keeps // the switch and its FX writes ONE point rather than two. A reapply opens none, so a -// caller that wants its own act undoable opens its own (design_view_actions does). +// caller that wants its own act undoable opens its own (design_view_actions does) — +// and, because REAPER's blocks are a depth counter, ENDS THE SPAN BEFORE IT OPENS +// THAT BLOCK. A reapply never closes a span, precisely because it can be running +// inside such a caller's block. // -// Returns false (no mutation) if `targetModeId` isn't registered, if this is a real -// switch (target != active) while the transport is running, if it is re-entered +// `applied` is false (no mutation) if `targetModeId` isn't registered, if this is a +// real switch (target != active) while the transport is running, if it is re-entered // (modeApplyInProgress), or if the project does not validate. A reapply (target == // active) is never gated and never touches solo. `proj` == nullptr means the current // project, resolved ONCE and used for every call this apply makes. // // Either way — applied or refused — this drops the FX reclaimer's derived queue, which // is why callers may safely discard the return after mutating the model. -bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj); +ModeApplyResult applyMode(ViewModeModel& model, const std::string& targetModeId, + ReaProject* proj); } // namespace reasampler diff --git a/src/shell/view/view_lanes.cpp b/src/shell/view/view_lanes.cpp index 087589c..982acce 100644 --- a/src/shell/view/view_lanes.cpp +++ b/src/shell/view/view_lanes.cpp @@ -16,6 +16,7 @@ #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/view/view_park.h" // writeIfChanged — the read-before-write every driven parm takes #define REAPERAPI_MINIMAL @@ -276,6 +277,11 @@ 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(); + // `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); diff --git a/src/shell/view/view_lanes.h b/src/shell/view/view_lanes.h index 6fd1311..b8adcc6 100644 --- a/src/shell/view/view_lanes.h +++ b/src/shell/view/view_lanes.h @@ -15,8 +15,8 @@ class ReaProject; namespace reasampler { // `freeMode` alone decides whether UpdateTimeline is owed (SDK: required after an -// I_FREEMODE change); `wrote` is any project write at all, which is what the apply's -// undo-point decision reads. +// I_FREEMODE change); `wrote` is a REAPER write, the reading ModeApplyResult +// (view.h) carries through to the caller's undo-point decision. struct LaneApplyResult { bool freeMode = false; bool wrote = false; @@ -24,14 +24,22 @@ struct LaneApplyResult { // Reconciles each op's durable laneKey to the track's current ordinal (a // stale/renamed/deleted key is pruned, never mis-driven) and drives C_LANEPLAYS. +// +// Opens NO undo block, where mintManagedLanes below opens one over the same +// parameters — the one criterion this directory's CLAUDE.md states, applied: this +// re-asserts derived state the view model recomputes from membership on every apply, +// so nothing here needs to survive as a point of its own; minting creates lanes and +// re-homes items, which no re-derivation reproduces. LaneApplyResult applyLaneOps(const TrackByGuid& handleByGuid, const std::vector& lanes); // Splits any track visible in more than one mode while carrying its own media into // fixed lanes (one managed lane per involved mode), assigns items, and records -// ownership in `model`. Wraps the mutation in one Undo block when non-empty. Returns -// true if any lane was minted (repaint hint). `proj` == nullptr means the current -// project. +// ownership in `model`. Wraps the mutation in one Undo block when non-empty — see +// applyLaneOps above for why this site opens one and that one does not — and ENDS THE +// RECLAIM SPAN first, since REAPER's blocks are a depth counter and this is reachable +// from a timer tick with a prior switch's span still open. Returns true if any lane +// was minted (repaint hint). `proj` == nullptr means the current project. bool mintManagedLanes(ViewModeModel& model, ReaProject* proj); // Recovers managed-lane ownership from durable P_LANENAME on project open — diff --git a/src/shell/view/view_park.cpp b/src/shell/view/view_park.cpp index 58f4785..a65da1b 100644 --- a/src/shell/view/view_park.cpp +++ b/src/shell/view/view_park.cpp @@ -144,7 +144,6 @@ TrackParkResult parkTrack(ViewModeModel& model, const std::string& guid, MediaTr snap.fxKeying = prior->fxKeying; } model.storeSnapshot(guid, snap); - result.wrote = true; } if (applyFlags(tr, flags)) result.wrote = true; @@ -153,9 +152,9 @@ TrackParkResult parkTrack(ViewModeModel& model, const std::string& guid, MediaTr bool restoreTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, const std::vector& flags) { - applyFlags(tr, flags); + const bool wrote = applyFlags(tr, flags); model.releaseSnapshotFlags(guid); - return true; + return wrote; } std::string trackDisplayName(MediaTrack* tr) { diff --git a/src/shell/view/view_park.h b/src/shell/view/view_park.h index 5150db2..fa639f6 100644 --- a/src/shell/view/view_park.h +++ b/src/shell/view/view_park.h @@ -93,11 +93,16 @@ extern const int kViewUndoMask; // Returns whether it wrote. bool writeIfChanged(MediaTrack* tr, const char* parm, double value); -// What one parkTrack call did. `wrote` is any project write at all — a flag write -// or a record write — and feeds the apply's undo-point fold. `exposed` is true iff -// this call handed the track back visible instead of parking or adopting it; the -// caller (view.cpp) collects exposed tracks across the whole apply for the one -// batched console report. +// What one parkTrack call did. `wrote` is a REAPER write — a driven flag moved — +// and nothing else: storing the snapshot is a MODEL move, and the model reaches disk +// outside every undo block, so counting it would mint a point carrying none of it +// (ModeApplyResult, view.h). The two readings part company on exactly one path — a +// RE-PARK whose restore had already handed the flags back at park-shaped values, which +// stores a fresh snapshot and writes no flag — and the REAPER reading is the one that +// wants: nothing in the project moved, so nothing is there for a point to hold. +// `exposed` is true iff this call handed the track back visible instead of parking or +// adopting it; the caller (view.cpp) collects exposed tracks across the whole apply for +// the one batched console report. struct TrackParkResult { bool wrote = false; bool exposed = false; @@ -170,8 +175,9 @@ inline bool shouldReport(bool sameOwnerAsLast, const std::vector& l void reportExposedTracks(ReaProject* proj, const std::vector& trackNames); // Restores one returning leaf from its record and spends the record's flag half. -// Always true: the record is consumed here, so this apply changed state and is not -// repeatable, which is exactly what its one reader (the undo-point decision) asks. +// Returns whether a FLAG actually moved — the same REAPER-writes-only reading +// TrackParkResult::wrote carries, and for the same reason: spending the record is a +// model move no undo point can hold. bool restoreTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, const std::vector& flags);