Close the FX reclaim span when REAPER's own Undo/Redo is dispatched, so the switch's point mints first
Both command hooks intercept Edit: Undo (40029) and Edit: Redo (40030) and consume nothing. Undo routes that never dispatch an action remain uncovered.
This commit is contained in:
@@ -2,12 +2,13 @@
|
||||
|
||||
## Scope
|
||||
|
||||
One type, `UndoBlock`, and one query, `undoBlockOpen()`. Every extension-owned undo
|
||||
One type, `UndoBlock`, and one query, `undoBlockOpen()`, plus the interception that ends
|
||||
the FX reclaim span when REAPER's own Undo/Redo is dispatched. Every extension-owned undo
|
||||
block in the tree is opened through this type; nothing here decides what a point is
|
||||
called or whether one is worth minting — each call site still owns its own label,
|
||||
mask, and discard-vs-mint condition. The FX reclaim span is NOT this directory's:
|
||||
`shell/view/fx_reclaim` owns it, and this directory only guarantees no block ever
|
||||
nests over it.
|
||||
`shell/view/fx_reclaim` owns it, and this directory only guarantees that neither a block
|
||||
nor REAPER's own undo ever runs nested over it.
|
||||
|
||||
## Invariants
|
||||
|
||||
@@ -48,6 +49,11 @@ nests over it.
|
||||
|
||||
## Modules
|
||||
|
||||
- `undo_intercept` — the ids of REAPER's own `Edit: Undo`/`Edit: Redo`, the main-section
|
||||
scoping of that pair, and `endReclaimSpanForUndoCommand`, which `app/main.cpp` calls
|
||||
from BOTH command hooks before anything else claims the id. Consumes nothing. Why
|
||||
dispatch and not the state-load hook, which ids are covered, and which undo routes are
|
||||
NOT: `src/shell/view/CLAUDE.md`, which owns the span.
|
||||
- `undo_block` — `UndoBlock` (RAII open/close plus `recordPoint`) and
|
||||
`undoBlockOpen()`. The latter is read by `fxReclaimTick`, which refuses a tick
|
||||
outright while any block is open: the drain's lazy `openSpan` would otherwise
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
// See undo_intercept.h. Touches no REAPER API of its own: the ids arrive from main.cpp's
|
||||
// two command hooks, and the only thing done with one is the span's existing close.
|
||||
|
||||
#include "shell/undo/undo_intercept.h"
|
||||
|
||||
#include "shell/undo/undo_trace.h" // TEMPORARY — see undo_trace.h; revert together
|
||||
#include "shell/view/fx_reclaim.h" // fxReclaimEndUndoSpan — the ONE idempotent close
|
||||
|
||||
namespace reasampler {
|
||||
|
||||
void endReclaimSpanForUndoCommand(int command, int sectionId) {
|
||||
if (!isMainUndoRedoCommand(command, sectionId)) return;
|
||||
// Traced whether or not a span is open, and before the close: the dump has to show the
|
||||
// hook SEEING the command, which is the half that was missing from the trace of the
|
||||
// undo that ran under an open block.
|
||||
REASAMPLER_TRACE("[UndoIntercept] cmd=%d section=%d", command, sectionId);
|
||||
// Safe at any depth: a span is never open at depth > 0 (src/shell/view/CLAUDE.md), so
|
||||
// a hook re-entered from inside an extension block finds nothing to close.
|
||||
fxReclaimEndUndoSpan(command == kEditUndoCommand ? "undo action" : "redo action");
|
||||
}
|
||||
|
||||
} // namespace reasampler
|
||||
@@ -0,0 +1,30 @@
|
||||
#pragma once
|
||||
// REAPER's own Undo/Redo, caught at action dispatch. The reclaim span has to close there
|
||||
// — minting the mode switch's point — BEFORE REAPER runs the undo: while the span is open
|
||||
// the switch has no point yet, so Ctrl-Z undoes whatever preceded it, with REAPER's undo
|
||||
// machinery running nested inside our still-open block.
|
||||
|
||||
namespace reasampler {
|
||||
|
||||
// Main-section "Edit: Undo" / "Edit: Redo". Taken from the vendored SDK's own reference
|
||||
// plugins, which send exactly these for undo/redo
|
||||
// (vendor/reaper-sdk/reaper-plugins/reaper_csurf/csurf.h:312-313, used by csurf_01X,
|
||||
// csurf_alphatrack, csurf_faderport, csurf_mcu) — not read off the action list by eye.
|
||||
inline constexpr int kEditUndoCommand = 40029;
|
||||
inline constexpr int kEditRedoCommand = 40030;
|
||||
|
||||
// Command ids are per SECTION (reaper_plugin.h's custom_action_register_t: 0/100 =
|
||||
// main/main alt, 32060-32063 = the MIDI and media-explorer sections), so the same integer
|
||||
// arriving under another section is a different action entirely and must not be
|
||||
// intercepted. What that leaves uncovered is enumerated in src/shell/view/CLAUDE.md.
|
||||
inline bool isMainUndoRedoCommand(int command, int sectionId) {
|
||||
if (sectionId != 0 && sectionId != 100) return false;
|
||||
return command == kEditUndoCommand || command == kEditRedoCommand;
|
||||
}
|
||||
|
||||
// Ends the reclaim span when `command` is one of the two above, and does nothing
|
||||
// otherwise. CONSUMES NOTHING: the caller must still return false from its hook and let
|
||||
// REAPER run the undo, which then executes at depth 0 against a properly formed point.
|
||||
void endReclaimSpanForUndoCommand(int command, int sectionId);
|
||||
|
||||
} // namespace reasampler
|
||||
@@ -118,12 +118,21 @@ decide membership or mode rules.
|
||||
exactly ONE undo entry and returned to the previous mode. (The two-point degradation
|
||||
this once hedged against never materialised; the degradations that DO occur are the
|
||||
transport deferral and `mintManagedLanes`' early close, both named below.)
|
||||
**Opening the span any LATER than the switch is rejected**, and the ~3s of open block
|
||||
before the drain's first FX write is not exposure buying nothing. The four driven flags
|
||||
mint nothing unaided (measured above), so the block's CLOSE is the only thing that ever
|
||||
makes the switch undoable; open the span at the drain's first write instead and the whole
|
||||
park hold carries no point for a Ctrl-Z to reach — which is precisely the bug the dispatch
|
||||
interception below fixes. The dead time is where the switch's point sits waiting to be
|
||||
minted, which is exactly what the state-load terminator and the dispatch interception
|
||||
reach into that window to do.
|
||||
- **No path may return past a terminating condition with the span open.** A leaked
|
||||
block swallows every edit the user makes afterwards, indefinitely, so every
|
||||
terminating path routes through ONE idempotent close (`fxReclaimEndUndoSpan`): the
|
||||
drain settling, aborting on a generation bump, or deferring on the transport; the
|
||||
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
|
||||
LOAD; REAPER's OWN Undo/Redo caught at action dispatch; 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
|
||||
@@ -205,6 +214,35 @@ decide membership or mode rules.
|
||||
`abandon`. Daniel's ruling, and the accepted cost: undoing a switch mid-drain leaves
|
||||
a PARTIAL park — some FX offlined, some not — covered by the one point the
|
||||
terminator closed.
|
||||
- **REAPER's own Undo/Redo is intercepted one step EARLIER than that, at action dispatch —
|
||||
the load hook is too late for an undo.** DAW-OBSERVED, and the whole reason this exists:
|
||||
switch to Design view, press Ctrl-Z inside the park hold, and the user gets one undo point
|
||||
per FX. While the span is open the switch HAS NO POINT YET, so Ctrl-Z does not undo the
|
||||
switch — it undoes whatever preceded it, with REAPER's undo machinery running nested
|
||||
inside our still-open block, and the drain then parks against a model that was undone
|
||||
underneath it. So `main.cpp`'s two command hooks end the span BEFORE REAPER runs the
|
||||
action (`shell/undo`'s `endReclaimSpanForUndoCommand`): the switch's point mints at that
|
||||
instant and the undo then executes at depth 0 against a properly formed point. The
|
||||
interception CONSUMES NOTHING — the hook returns false and REAPER proceeds normally.
|
||||
The ids are `40029`/`40030`, taken from Cockos' own reference plugins in the vendored SDK
|
||||
(`vendor/reaper-sdk/reaper-plugins/reaper_csurf/csurf.h:312-313`, sent as `WM_COMMAND` by
|
||||
four of them) rather than read off the action list by eye; the pair and its section
|
||||
scoping are pinned by `tests/test_undo_intercept.cpp`. BOTH hooks call it, because
|
||||
`hookcommand2` is not the non-main sections' hook: it runs prior to every action triggered
|
||||
by a KEY or MIDI event in ANY section, main included (`reaper_plugin.h:214`), and a
|
||||
Ctrl-Z is a key event. Which of the two fires first is undocumented and immaterial — the
|
||||
close is idempotent.
|
||||
**What this does NOT cover — every one of these still lands inside an open span, exactly
|
||||
as before:** an undo fired in a NON-MAIN section (the MIDI editor's own Ctrl-Z is a
|
||||
project undo; its command id lives in that section's own id space and is not verifiable
|
||||
from the SDK, so nothing is guessed here) — plausible in real use; the Undo History
|
||||
window's list, which drives the undo directly rather than dispatching `40029` — plausible,
|
||||
and the likeliest remaining route; and a ReaScript or another extension calling
|
||||
`Undo_DoUndo2`/`Undo_DoRedo2` (`reaper_plugin_functions.h:7777-7790`) — plausible wherever
|
||||
the user runs scripts. What IS covered beyond the keystroke: the Edit menu, the toolbar
|
||||
button, a control surface (the csurf plugins above), an OSC/MIDI binding, and — by
|
||||
inference from `reaper_plugin.h:206`'s recursion warning rather than by measurement — a
|
||||
script's `Main_OnCommand(40029)`.
|
||||
- **A state load puts the reclaimer to SLEEP; only a re-read of the model wakes it.**
|
||||
`abandon` clears the wakeup where `invalidate` raises it, because the two say
|
||||
different things: an invalidation says the MODEL moved and owes a re-derivation, a
|
||||
@@ -337,6 +375,21 @@ applies the resulting lane state to live tracks.
|
||||
point — and, since the terminator above, whatever the drain had NOT yet written when
|
||||
the undo arrived, because the span closes at that instant. Whether that reads as
|
||||
correct in the DAW is `[verify — DAW]`, `docs/VERIFICATION.md` §"Mode switching".
|
||||
- **The trace of the failing mid-hold Ctrl-Z showed NO `BeginLoadProjectState` line at
|
||||
all**, which the state-load terminator says should fire for an undo
|
||||
(`reaper_plugin.h:1203-1205`), and the dump had not wrapped. Unexplained: either REAPER
|
||||
declined the undo outright while our block was open, or it performed one without a state
|
||||
load. The dispatch interception does not rest on which — it mints the point before REAPER
|
||||
is asked, either way — but the next dump discriminates, and the expected order is
|
||||
`[UndoIntercept]`, `[UndoSpan] CLOSE reason=undo action`, then `[Load]
|
||||
BeginLoadProjectState isUndo=1`. A dump still missing the load line means the diagnosis
|
||||
needs revisiting, not that the fix regressed.
|
||||
- **A NATIVE REAPER action fired during the span is absorbed into the switch's point.**
|
||||
The nesting rule and `UndoBlock` cover every EXTENSION-owned block; REAPER's own actions
|
||||
cannot be routed through it, so under the depth-counter reading their block closes at
|
||||
depth 2->1, minting nothing and folding their state into ours. Not fixed here and not
|
||||
measured — it rests on the same `[verify — DAW]` counter reading as the nesting rule, and
|
||||
the window is the park hold plus the drain.
|
||||
- **The `view_state` write is NOT inside `applyMode`'s block — but on a REAL switch
|
||||
it is now inside the SPAN.** `persistViewState` (`design_view_actions.cpp`) runs
|
||||
after `applyMode` has returned, and a real switch returns with the span open, so
|
||||
|
||||
@@ -694,10 +694,11 @@ 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; 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.
|
||||
// the next apply superseding it; ANY project-state load; REAPER's own Undo/Redo at action
|
||||
// dispatch (shell/undo's endReclaimSpanForUndoCommand, which mints the switch's point
|
||||
// before REAPER runs the undo); a project save; extension 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.
|
||||
//
|
||||
// `reason` is TEMPORARY tracing plumbing (see shell/undo/undo_trace.h) naming which
|
||||
// terminating path called — it has no effect on behavior. Revert together with the rest
|
||||
|
||||
Reference in New Issue
Block a user