Design View: drop the undo blocks around writes that mint nothing; the membership actions own theirs

Flag writes are silent from a timer, so a block around a reapply or the reconcile
repair could only manufacture a point. The real switch's span is unchanged.
This commit is contained in:
2026-08-07 14:41:17 -04:00
parent 67d9ec3da9
commit 39529a5ac4
8 changed files with 141 additions and 144 deletions
+52 -16
View File
@@ -27,6 +27,7 @@
#include "shell/panel/panel_window.h" // bankPanelInvalidate — footer toggle repaint #include "shell/panel/panel_window.h" // bankPanelInvalidate — footer toggle repaint
#include "shell/view/view.h" // applyMode (D2 shell) #include "shell/view/view.h" // applyMode (D2 shell)
#include "shell/view/view_lanes.h" // mintManagedLanes #include "shell/view/view_lanes.h" // mintManagedLanes
#include "shell/view/view_park.h" // kViewUndoMask
#define REAPERAPI_MINIMAL #define REAPERAPI_MINIMAL
#define REAPERAPI_WANT_CountSelectedTracks #define REAPERAPI_WANT_CountSelectedTracks
@@ -201,40 +202,76 @@ void doActivateMode(const std::string& modeId) {
bankPanelInvalidate(); // repaint the footer [Arrange|Design] toggle immediately bankPanelInvalidate(); // repaint the footer [Arrange|Design] toggle immediately
} }
// The block each membership-edit action below owns. The reapply they drive opens none
// of its own — nothing it writes mints a point unaided — so a user-fired tag stays one
// Ctrl-Z only because the ACTION spans it, and the label names the act the user
// 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.
class ActionUndoBlock {
public:
ActionUndoBlock(std::string label, int mask) : label_(std::move(label)), mask_(mask) {
Undo_BeginBlock2(nullptr);
}
~ActionUndoBlock() { Undo_EndBlock2(nullptr, label_.c_str(), mask_); }
ActionUndoBlock(const ActionUndoBlock&) = delete;
ActionUndoBlock& operator=(const ActionUndoBlock&) = delete;
private:
std::string label_;
int mask_;
};
// tag() replaces any prior single-mode membership (a leaf lives in one mode; the // tag() replaces any prior single-mode membership (a leaf lives in one mode; the
// cross-mode case is show-both). // cross-mode case is show-both).
void doTag(const std::string& modeId) { void doTag(const std::string& modeId) {
for (const std::string& g : selectedTrackGuids()) const std::vector<std::string> guids = selectedTrackGuids();
g_session->view().membership().tag(g, modeId); if (guids.empty()) return; // nothing selected — no act, no undo point (cf. doMoveItems)
{
ActionUndoBlock undo("ReaSampler: tag selected tracks -> " + modeId, kViewUndoMask);
for (const std::string& g : guids) g_session->view().membership().tag(g, modeId);
reapplyActiveMode(); reapplyActiveMode();
}
persistViewState(PersistScope::Full); persistViewState(PersistScope::Full);
} }
// Shared body behind "Untag selected" and "Tag -> Arrange" — Arrange is the absence // Shared body behind "Untag selected" and "Tag -> Arrange" — Arrange is the absence
// of a tag, so the two actions are the same act. // of a tag, so the two actions are the same act.
void doUntag() { void doUntag() {
for (const std::string& g : selectedTrackGuids()) const std::vector<std::string> guids = selectedTrackGuids();
g_session->view().membership().untag(g); if (guids.empty()) return;
{
ActionUndoBlock undo("ReaSampler: untag selected tracks", kViewUndoMask);
for (const std::string& g : guids) g_session->view().membership().untag(g);
reapplyActiveMode(); reapplyActiveMode();
}
persistViewState(PersistScope::Full); persistViewState(PersistScope::Full);
} }
// Flips each track's pin independently — the honest semantics of a toggle on a // Flips each track's pin independently — the honest semantics of a toggle on a
// multi-selection (a mixed selection converges toward uniform only if it already was). // multi-selection (a mixed selection converges toward uniform only if it already was).
void doShowBoth() { void doShowBoth() {
const std::vector<std::string> guids = selectedTrackGuids();
if (guids.empty()) return;
{
ActionUndoBlock undo("ReaSampler: toggle show-both on selected tracks", kViewUndoMask);
MembershipIndex& m = g_session->view().membership(); MembershipIndex& m = g_session->view().membership();
for (const std::string& g : selectedTrackGuids()) for (const std::string& g : guids) m.setShowBoth(g, !m.isShowBoth(g));
m.setShowBoth(g, !m.isShowBoth(g));
reapplyActiveMode(); reapplyActiveMode();
}
persistViewState(PersistScope::Full); persistViewState(PersistScope::Full);
} }
// Retag the current ITEM selection to `targetMode` (empty => untag -> Arrange // Retag the current ITEM selection to `targetMode` (empty => untag -> Arrange
// default). planItemRetag decides which items to retag (manual-lane items are // default). planItemRetag decides which items to retag (manual-lane items are
// EXEMPT), upholding the managed-lanes-only invariant. Wrapped in ONE Undo block. // EXEMPT), upholding the managed-lanes-only invariant.
// //
// UNDO/SAVE ordering: persistViewState may pop a Save-As dialog, which must NOT sit // The mask stays UNDO_STATE_ALL rather than the view mask the three track actions
// inside the Undo block, so we close the block first, then persist. // close on: this one can mint lanes, and the fixed-lane properties are only ASSUMED to
// ride UNDO_STATE_ITEMS (see kViewUndoMask).
void doMoveItems(const std::string& targetMode) { void doMoveItems(const std::string& targetMode) {
const std::vector<RetagItem> selected = selectedRetagItems(); const std::vector<RetagItem> selected = selectedRetagItems();
const std::vector<ItemRetagOp> ops = planItemRetag(selected, targetMode); const std::vector<ItemRetagOp> ops = planItemRetag(selected, targetMode);
@@ -242,7 +279,11 @@ void doMoveItems(const std::string& targetMode) {
MembershipIndex& membership = g_session->view().membership(); MembershipIndex& membership = g_session->view().membership();
Undo_BeginBlock2(nullptr); {
ActionUndoBlock undo(targetMode.empty()
? std::string("ReaSampler: untag selected items")
: "ReaSampler: move selected items -> " + targetMode,
-1);
for (const ItemRetagOp& op : ops) { for (const ItemRetagOp& op : ops) {
if (op.untag) membership.untag(op.guid); if (op.untag) membership.untag(op.guid);
else membership.tag(op.guid, op.modeId); else membership.tag(op.guid, op.modeId);
@@ -251,12 +292,7 @@ void doMoveItems(const std::string& targetMode) {
// track whose items now span modes, then reassert active-mode lane visibility. // track whose items now span modes, then reassert active-mode lane visibility.
mintManagedLanes(g_session->view(), nullptr); mintManagedLanes(g_session->view(), nullptr);
reapplyActiveMode(); reapplyActiveMode();
}
const std::string label =
targetMode.empty()
? std::string("ReaSampler: untag selected items")
: std::string("ReaSampler: move selected items -> ") + targetMode;
Undo_EndBlock2(nullptr, label.c_str(), -1);
persistViewState(PersistScope::Full); persistViewState(PersistScope::Full);
} }
+40 -33
View File
@@ -64,22 +64,30 @@ decide membership or mode rules.
offline/online runs on the background reclaimer (`fx_reclaim`), in ticks offline/online runs on the background reclaimer (`fx_reclaim`), in ticks
bounded by `kOnlineBudgetMs`/`kOfflineBudgetMs`, off `applyMode`'s block bounded by `kOnlineBudgetMs`/`kOfflineBudgetMs`, off `applyMode`'s block
entirely (measured: `TrackFX_SetOffline` was 1897ms of a 1942ms switch). entirely (measured: `TrackFX_SetOffline` was 1897ms of a 1942ms switch).
- **`TrackFX_SetOffline` is the ONLY write in this feature that mints an undo point,
and a block around anything else can only manufacture one.** DAW-MEASURED, and the
premise every decision below rests on: ten writes of the four driven flags
(`B_SHOWINTCP`, `B_SHOWINMIXER`, `B_MAINSEND`, `I_FXEN`) from a timer callback, with
no block and no undo call, via `SetMediaTrackInfo_Value` and again via
`GetSetMediaTrackInfo`, added ZERO entries to the Undo History. Ten
`TrackFX_SetOffline` calls from a timer minted TEN points unblocked, TEN under a
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.
- **ONE mode switch is ONE undo point — the background FX parking included.** The - **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 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 minutes-of-plugin-loads later all roll back in a single Ctrl-Z, because they share
ONE undo block: `applyMode` opens it (`fx_reclaim`'s `UndoSpan`) and a REAL switch ONE undo block: `applyMode` opens it on a REAL SWITCH ONLY (`fx_reclaim`'s
RETURNS WITH IT OPEN, for the reclaimer to close once the drain has converged. A `UndoSpan`) and RETURNS WITH IT OPEN, for the reclaimer to close once the drain has
reapply closes its own before returning. This does not move an FX write back onto converged. This does not move an FX write back onto the switch — the writes still
the switch — the writes still run on the timer, off the 1897ms path. run on the timer, off the 1897ms path. Whether a block held from an ACTION into a
DAW-MEASURED, and the reason the design is shaped this way: block WIDTH is the later timer callback holds is `[verify — DAW]`; if it does not, the switch and its
entire effect. For ten `TrackFX_SetOffline` calls from a TIMER — unblocked, or parking land as two points, which is the intended degradation.
under a block per write, by every dispatch route — REAPER minted TEN points; under
ONE block spanning all ten it minted ONE. 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. Whether one
held from an ACTION into a later timer callback holds is the same property but is
`[verify — DAW]`; if it does not, the switch and its parking land as two points,
which is the intended degradation.
- **No path may return past a terminating condition with the span open.** A leaked - **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 block swallows every edit the user makes afterwards, indefinitely, so every
terminating path routes through ONE idempotent close (`fxReclaimEndUndoSpan`): the terminating path routes through ONE idempotent close (`fxReclaimEndUndoSpan`): the
@@ -129,16 +137,13 @@ decide membership or mode rules.
rather than resuming on the next tick. (That a revert leaves the view model stale at 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 all is the identity poll's defect, not the reclaimer's — this invariant only keeps the
reclaimer from acting on it.) reclaimer from acting on it.)
- **A reapply that wrote nothing leaves no undo point** — and a reapply never holds - **A reapply opens no undo block at all, so it can leave no point** — whether it
the span open, so a project open, a membership edit and `render_in_place`'s inner wrote or not, and whoever drove it: a project open, a membership edit,
reapply all keep the block shape they had. The load-tick reapply `render_in_place`'s inner reapply. Nothing a reapply writes mints unaided (see the
re-plans a park for every inactive leaf, and over a project saved fully parked measurement above), so a block there could only manufacture the Ctrl-Z opening a
it finds every flag and every FX already where the plan wants them. The block project must not cost. A caller whose own act needs to stay undoable spans it
then closes on the `("", 0)` discard form (`view.h`'s `applyMintsUndoPoint`; itself. A REAL switch still always mints, written state or not: it is an explicitly
`mintManagedLanes`' no-op path uses the same idiom) — opening a project must fired action and has to stay undoable even when the plan found nothing to write.
not cost the user a Ctrl-Z. A REAL switch 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 rapid A→B→A flip moves zero plugins.** The reclaimer only starts a park's - **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` OFFLINE work once a track has read continuously parked for `kParkHoldMs`
(3000ms). A flip inside that window flips the flags twice but the debounce (3000ms). A flip inside that window flips the flags twice but the debounce
@@ -181,10 +186,10 @@ applies the resulting lane state to live tracks.
## Modules ## 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 OPENS the ONE undo span all of that rides (`applyMintsUndoPoint` decides whether it leaves a point; on a real switch `fx_reclaim` is what closes it). 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 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_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_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_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). - `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_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`. - `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`, 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).
@@ -231,9 +236,10 @@ applies the resulting lane state to live tracks.
unsaved project — land inside the block after all. That is a consequence of the unsaved project — land inside the block after all. That is a consequence of the
span, not a decision made here; it does not resolve the redo hypothesis below, span, not a decision made here; it does not resolve the redo hypothesis below,
which turns on capture-time semantics the SDK does not document. `[verify — DAW]` which turns on capture-time semantics the SDK does not document. `[verify — DAW]`
that a Save-As dialog opening under an open undo block is harmless. The rest of that a Save-As dialog opening under an open undo block is harmless. On the reapply
this gotcha describes the reapply path, where nothing changed. The UNDO direction path the write stays OUTSIDE every block, as it always did: the reapply opens none,
still comes and a membership action's own `ActionUndoBlock` closes before `persistViewState` for
exactly this modal-Save-As reason. The UNDO direction still comes
out right — the point closed over the ext state as it stood BEFORE the switch, out right — the point closed over the ext state as it stood BEFORE the switch,
which is what a Ctrl-Z should restore — but by position, not by design. The which is what a Ctrl-Z should restore — but by position, not by design. The
REDO direction is what this leaves open, and the cost is worse than the REDO direction is what this leaves open, and the cost is worse than the
@@ -282,11 +288,12 @@ applies the resulting lane state to live tracks.
refusal leaves the membership tags it already wrote in place, so the next 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 reapply — not a user re-fire of render-in-place — is what brings the tracks
into sync. into sync.
- `render_in_place` still calls `applyMode` inside its own `Undo_BeginBlock2` - `render_in_place` calls `applyMode` inside its own `Undo_BeginBlock2` pair, and
pair for the reapply — one point rather than two for the flags and lane state that block is now the only one in play on that path — the reapply opens none of
— but with no FX writes left inside `applyMode`'s block there is nothing left its own, so nothing nests and the SDK's undocumented nested-block coalescing
to coalesce beyond that; the SDK's nested-block coalescing itself is still stopped being load-bearing here. The same holds for the membership actions'
`[verify — DAW]`. `ActionUndoBlock`; `doMoveItems` is the one that still nests, over
`mintManagedLanes`.
- `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays - `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays
attached to its plugin across a chain mutation while parked. That is 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 `[verify — DAW]` (see `fxGuidString` in `view_park.cpp`) and SWS issue #802 is a
+6 -6
View File
@@ -586,7 +586,7 @@ public:
return true; return true;
} }
// A mode switch decides its label only after its writes (applyMintsUndoPoint). // A mode switch names its point only after its writes, so the open cannot carry it.
void relabel(std::string label, int mask) { void relabel(std::string label, int mask) {
if (!open_) return; if (!open_) return;
label_ = std::move(label); label_ = std::move(label);
@@ -664,14 +664,14 @@ bool fxReclaimDrainInProgress();
void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj); void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj);
// Opens the undo span a mode switch and its background FX parking share, so the two are // Opens the undo span a mode switch and its background FX parking share, so the two are
// ONE Ctrl-Z. Called by applyMode in place of its own Undo_BeginBlock2; a REAL switch // ONE Ctrl-Z. Called by applyMode on a REAL SWITCH ONLY — a reapply opens no block at
// then returns with the span still open and the reclaimer closes it once the parking has // all — which then returns with the span still open, and the reclaimer closes it once
// drained. No-op if a span is already open, so callers END first (see applyMode) rather // the parking has drained. No-op if a span is already open, so callers END first (see
// than nesting. // applyMode) rather than nesting.
void fxReclaimBeginUndoSpan(ReaProject* proj); void fxReclaimBeginUndoSpan(ReaProject* proj);
// Sets what the eventual close names the point. Separate from the begin because the // Sets what the eventual close names the point. Separate from the begin because the
// switch's own label is only decided after its writes (applyMintsUndoPoint). // switch's own label is only decided after its writes.
void fxReclaimLabelUndoSpan(const char* label, int mask); void fxReclaimLabelUndoSpan(const char* label, int mask);
// The ONE close, idempotent: every terminating path routes through it — the drain // The ONE close, idempotent: every terminating path routes through it — the drain
+16 -18
View File
@@ -2,8 +2,8 @@
// reaper_plugin_functions.h without REAPERAPI_IMPLEMENT (main.cpp owns that). // reaper_plugin_functions.h without REAPERAPI_IMPLEMENT (main.cpp owns that).
// Tree arithmetic lives in view_tree (pure), one track's park/restore in view_park // Tree arithmetic lives in view_tree (pure), one track's park/restore in view_park
// and the lane writes in view_lanes; this file owns the plan iteration, parent // and the lane writes in view_lanes; this file owns the plan iteration, parent
// visibility, and opening the one undo span they all ride (fx_reclaim's UndoSpan, // visibility, and opening the undo span a REAL switch rides (fx_reclaim's UndoSpan,
// which a real switch hands to the reclaimer still open). // which it hands to the reclaimer still open).
#include "shell/view/view.h" #include "shell/view/view.h"
@@ -115,14 +115,17 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro
(targetMode ? targetMode->displayName : targetModeId) + " view"; (targetMode ? targetMode->displayName : targetModeId) + " view";
bool laneModeChanged = false; bool laneModeChanged = false;
bool wrote = false; // anything at all this apply changed — see applyMintsUndoPoint
std::vector<std::string> exposedNames; // named while handles are fresh, reported after the writes std::vector<std::string> 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 // 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 // supersedes it, so that parking lands as its own point rather than nesting under
// ours. Both calls are idempotent — the end reaches nothing when none is open. // ours. Idempotent — it reaches nothing when none is open.
fxReclaimEndUndoSpan(); fxReclaimEndUndoSpan();
fxReclaimBeginUndoSpan(project);
// 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);
{ {
UiRefreshHold uiHold; // every write below lands with the TCP/MCP frozen UiRefreshHold uiHold; // every write below lands with the TCP/MCP frozen
@@ -141,7 +144,6 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro
MediaTrack* tr = resolve(trackByGuid, guid); MediaTrack* tr = resolve(trackByGuid, guid);
if (!tr) continue; // stale/deleted GUID — prune if (!tr) continue; // stale/deleted GUID — prune
const TrackParkResult r = parkTrack(model, guid, tr, tp.flags); const TrackParkResult r = parkTrack(model, guid, tr, tp.flags);
if (r.wrote) wrote = true;
// Named here, not after the block: TrackList_AdjustWindows below can // Named here, not after the block: TrackList_AdjustWindows below can
// invalidate the handle. // invalidate the handle.
if (r.exposed) exposedNames.push_back(trackDisplayName(tr)); if (r.exposed) exposedNames.push_back(trackDisplayName(tr));
@@ -152,14 +154,13 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro
const std::string& guid = tp.flags.front().guid; const std::string& guid = tp.flags.front().guid;
MediaTrack* tr = resolve(trackByGuid, guid); MediaTrack* tr = resolve(trackByGuid, guid);
if (!tr) continue; // stale/deleted GUID — prune if (!tr) continue; // stale/deleted GUID — prune
if (restoreTrack(model, guid, tr, tp.flags)) wrote = true; restoreTrack(model, guid, tr, tp.flags);
} }
// MANAGED LANES: the active mode's lane plays+shows, every other managed lane // 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. // is silenced+hidden. Empty on a D1-only project — byte-identical there.
const LaneApplyResult lanes = applyLaneOps(trackByGuid, plan.lanes); const LaneApplyResult lanes = applyLaneOps(trackByGuid, plan.lanes);
laneModeChanged = lanes.freeMode; laneModeChanged = lanes.freeMode;
if (lanes.wrote) wrote = true;
// PARENT VISIBILITY (never parked): recomputed every toggle, never recorded. // PARENT VISIBILITY (never parked): recomputed every toggle, never recorded.
// Only the two visibility flags — never mainSend/FX on a parent. // Only the two visibility flags — never mainSend/FX on a parent.
@@ -169,8 +170,8 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro
MediaTrack* tr = resolve(trackByGuid, node.guid); MediaTrack* tr = resolve(trackByGuid, node.guid);
if (!tr) continue; // stale GUID — prune if (!tr) continue; // stale GUID — prune
const double show = visible.count(node.guid) ? 1.0 : 0.0; const double show = visible.count(node.guid) ? 1.0 : 0.0;
if (writeIfChanged(tr, "B_SHOWINTCP", show)) wrote = true; writeIfChanged(tr, "B_SHOWINTCP", show);
if (writeIfChanged(tr, "B_SHOWINMIXER", show)) wrote = true; writeIfChanged(tr, "B_SHOWINMIXER", show);
} }
model.setActiveMode(targetModeId); model.setActiveMode(targetModeId);
@@ -197,14 +198,11 @@ bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaPro
// this apply (SDK requirement for I_FREEMODE changes). // this apply (SDK requirement for I_FREEMODE changes).
if (laneModeChanged) UpdateTimeline(); if (laneModeChanged) UpdateTimeline();
const bool mint = applyMintsUndoPoint(realSwitch, wrote); // A real switch mints whether or not the plan found anything to write — it is an
fxReclaimLabelUndoSpan(mint ? undoLabel.c_str() : "", mint ? kViewUndoMask : 0); // explicitly fired action and has to stay undoable — and RETURNS WITH THE SPAN STILL
// OPEN: the reclaimer closes it once the parking this apply is about to invalidate
// A REAL switch returns with the span STILL OPEN: the reclaimer closes it once the // has drained, so the switch and its FX writes are ONE Ctrl-Z.
// parking this apply is about to invalidate has drained, so the switch and its FX if (realSwitch) fxReclaimLabelUndoSpan(undoLabel.c_str(), kViewUndoMask);
// writes are ONE Ctrl-Z. Everything else closes here as it always did — a reapply is
// not a switch, and the load tick must not leave a block open across a project open.
if (!realSwitch) fxReclaimEndUndoSpan();
// Neither of the two below is a project write — a console message and an in-memory // Neither of the two below is a project write — a console message and an in-memory
// fingerprint — so whether the span is still open over them is immaterial. // fingerprint — so whether the span is still open over them is immaterial.
+7 -19
View File
@@ -1,9 +1,9 @@
#pragma once #pragma once
// REAPER-facing shell of Design View: reads the live folder tree, runs // REAPER-facing shell of Design View: reads the live folder tree, runs
// ViewModeModel's pure planner, and applies the resulting flag / lane / parent // ViewModeModel's pure planner, and applies the resulting flag / lane / parent
// writes under ONE undo block. See src/shell/view/CLAUDE.md for the enforced // writes — under ONE undo block on a real switch, under none at all on a reapply.
// invariants (never touch master/mute, solo cached per mode and never lost, // See src/shell/view/CLAUDE.md for the enforced invariants (never touch
// record-based restore). // master/mute, solo cached per mode and never lost, record-based restore).
#include <string> #include <string>
@@ -22,20 +22,6 @@ namespace reasampler {
// `nullptr` meaning the current project here is inferred by analogy, not confirmed. // `nullptr` meaning the current project here is inferred by analogy, not confirmed.
bool transportBlocksModeSwitch(ReaProject* proj); bool transportBlocksModeSwitch(ReaProject* proj);
// Does one applyMode leave an undo point behind?
//
// A REAPPLY that wrote nothing must not: the load-tick reapply over a project saved
// fully parked re-plans a park for every inactive leaf and finds every flag already
// where it wants them, so a point there would mean opening a project costs an undo
// step. `Undo_EndBlock2(proj, "", 0)` is the discard form (mintManagedLanes' no-op
// path and the reconcile pass use the same idiom).
//
// A real switch mints regardless, fail-safe: it is an explicitly fired action and has
// to stay undoable even in the degenerate case where the plan found nothing to write.
inline bool applyMintsUndoPoint(bool realSwitch, bool wroteAnything) {
return realSwitch || wroteAnything;
}
// True while an applyMode call is on the stack. applyMode fails closed when // True while an applyMode call is on the stack. applyMode fails closed when
// re-entered, and this lets a caller with no fallback test BEFORE it spends a // re-entered, and this lets a caller with no fallback test BEFORE it spends a
// one-shot signal on a call that would be refused — see main.cpp's load glue. // one-shot signal on a call that would be refused — see main.cpp's load glue.
@@ -45,7 +31,8 @@ bool modeApplyInProgress();
// the outgoing mode's solo state and replays the incoming mode's, runs planToggle, // 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 // applies the park/restore flag writes plus managed-lane and parent-visibility
// writes, then sets the active mode. ONE switch is ONE undo point; everything it // writes, then sets the active mode. ONE switch is ONE undo point; everything it
// moved rolls back in a single Ctrl-Z. // moved rolls back in a single Ctrl-Z. A REAPPLY leaves no point at all — it opens no
// block, because none of what it writes mints one on its own.
// //
// FX are NOT written here. Offlining a parked chain is the reclaimer's job, off this // FX are NOT written here. Offlining a parked chain is the reclaimer's job, off this
// path: TrackFX_SetOffline was 1897ms of a measured 1942ms switch, against 0.02ms for // path: TrackFX_SetOffline was 1897ms of a measured 1942ms switch, against 0.02ms for
@@ -53,7 +40,8 @@ bool modeApplyInProgress();
// //
// A REAL switch therefore RETURNS WITH ITS UNDO BLOCK STILL OPEN (fx_reclaim's // 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 // 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 closes its own. // 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).
// //
// Returns false (no mutation) if `targetModeId` isn't registered, if this is a real // 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 // switch (target != active) while the transport is running, if it is re-entered
+4 -4
View File
@@ -14,8 +14,6 @@
#define REAPERAPI_MINIMAL #define REAPERAPI_MINIMAL
#define REAPERAPI_WANT_EnumProjects #define REAPERAPI_WANT_EnumProjects
#define REAPERAPI_WANT_TrackList_AdjustWindows #define REAPERAPI_WANT_TrackList_AdjustWindows
#define REAPERAPI_WANT_Undo_BeginBlock2
#define REAPERAPI_WANT_Undo_EndBlock2
#define REAPERAPI_WANT_UpdateArrange #define REAPERAPI_WANT_UpdateArrange
#define REAPERAPI_WANT_ValidatePtr2 #define REAPERAPI_WANT_ValidatePtr2
#include "reaper_plugin_functions.h" #include "reaper_plugin_functions.h"
@@ -58,8 +56,11 @@ void reconcileAppliedFlags(ViewModeModel& model, ReaProject* proj) {
readFolderEntries(project, handleByGuid); readFolderEntries(project, handleByGuid);
const TrackByGuid trackByGuid = indexHandles(handleByGuid); const TrackByGuid trackByGuid = indexHandles(handleByGuid);
// NO undo block, deliberately: this runs in the wake of a Ctrl-Z, and the flag
// writes below mint nothing on their own — a block could only manufacture the point
// that would become the thing the user's next Ctrl-Z undoes. Do not reintroduce one
// (see this directory's CLAUDE.md).
bool wrote = false; bool wrote = false;
Undo_BeginBlock2(project);
{ {
UiRefreshHold uiHold; UiRefreshHold uiHold;
for (const TrackFlagOp& op : result.ops) { for (const TrackFlagOp& op : result.ops) {
@@ -69,7 +70,6 @@ void reconcileAppliedFlags(ViewModeModel& model, ReaProject* proj) {
wrote = true; wrote = true;
} }
} }
Undo_EndBlock2(project, "", 0); // discard form — see the header
if (wrote) { if (wrote) {
TrackList_AdjustWindows(false); // tracks reappearing owe the full relayout TrackList_AdjustWindows(false); // tracks reappearing owe the full relayout
+3 -3
View File
@@ -85,9 +85,9 @@ inline ProjectReconcileResult planReconcilePerProject(
// once an undo has rolled the model back underneath it. // once an undo has rolled the model back underneath it.
void noteAppliedState(ReaProject* proj, const ViewModeModel& model); void noteAppliedState(ReaProject* proj, const ViewModeModel& model);
// Runs planReconcile over the reloaded `model` and writes its ops. The undo block it // Runs planReconcile over the reloaded `model` and writes its ops. It opens NO undo
// opens is deliberately DISCARDED — this runs in the wake of a Ctrl-Z, and a repair // block — this runs in the wake of a Ctrl-Z, and a repair that minted a point would
// that minted a point would become the thing the user's next Ctrl-Z undoes. // become the thing the user's next Ctrl-Z undoes.
void reconcileAppliedFlags(ViewModeModel& model, ReaProject* proj); void reconcileAppliedFlags(ViewModeModel& model, ReaProject* proj);
} // namespace reasampler } // namespace reasampler
+2 -34
View File
@@ -2,12 +2,10 @@
// test framework. // test framework.
// //
// The properties under test: the four flags a park drives address exactly the four // The properties under test: the four flags a park drives address exactly the four
// REAPER parameters the tool owns (never mute, never solo), and one mode apply leaves // REAPER parameters the tool owns (never mute, never solo), the re-park union, the
// at most ONE undo point — none at all when a reapply found everything already where // park-shaped chain read, and the EXPOSE report's wording.
// it wanted it.
#include "../src/shell/view/view_park.h" #include "../src/shell/view/view_park.h"
#include "../src/shell/view/view.h" // applyMintsUndoPoint — the apply's undo-point fold
#include "core/view/view_mode_model.h" // makeParkPlan — the park flags' only producer #include "core/view/view_mode_model.h" // makeParkPlan — the park flags' only producer
@@ -65,32 +63,6 @@ static void testAParkPlanDrivesEveryDrivenFlagToZero() {
CHECK(parms.size() == 4); CHECK(parms.size() == 4);
} }
// -- the apply's one undo point ----------------------------------------------
//
// These pin applyMintsUndoPoint's fold only. The `wroteAnything` verdict it consumes
// — writeIfChanged / parkTrack / restoreTrack, all running against live REAPER calls
// — cannot be pinned here; docs/VERIFICATION.md §"Mode switching" is the sole cover
// for that plumbing being wired correctly.
static void testARealSwitchAlwaysLeavesAPointEvenWithNothingToWrite() {
// An explicitly fired action stays undoable even when the plan found nothing to
// write — the discard form is for reapplies only.
CHECK(applyMintsUndoPoint(/*realSwitch=*/true, /*wroteAnything=*/false));
CHECK(applyMintsUndoPoint(/*realSwitch=*/true, /*wroteAnything=*/true));
}
static void testAReapplyThatWroteSomethingLeavesAPoint() {
// A tag/untag reapply that actually parked a track: real project state moved.
CHECK(applyMintsUndoPoint(/*realSwitch=*/false, /*wroteAnything=*/true));
}
static void testAReapplyThatWroteNothingLeavesNoPointAtAll() {
// The project-load reapply over a project saved fully parked: every flag already
// sits where the plan wants it. Opening a project must not cost the user an undo
// step.
CHECK(!applyMintsUndoPoint(/*realSwitch=*/false, /*wroteAnything=*/false));
}
// -- the re-park union: a plugin added between a restore and the next park -------- // -- the re-park union: a plugin added between a restore and the next park --------
// //
// Sequence under test: park -> restore -> user adds plugin P -> re-park. Without the // Sequence under test: park -> restore -> user adds plugin P -> re-park. Without the
@@ -216,10 +188,6 @@ int main() {
testNoDrivenFlagCanEverAddressMuteOrSolo(); testNoDrivenFlagCanEverAddressMuteOrSolo();
testAParkPlanDrivesEveryDrivenFlagToZero(); testAParkPlanDrivesEveryDrivenFlagToZero();
testARealSwitchAlwaysLeavesAPointEvenWithNothingToWrite();
testAReapplyThatWroteSomethingLeavesAPoint();
testAReapplyThatWroteNothingLeavesNoPointAtAll();
testAPluginAddedWhileRestoredJoinsTheUnionRatherThanBeingDropped(); testAPluginAddedWhileRestoredJoinsTheUnionRatherThanBeingDropped();
testAPluginRemovedWhileRestoredStaysInTheUnionForResolveFxRestoreToDrop(); testAPluginRemovedWhileRestoredStaysInTheUnionForResolveFxRestoreToDrop();