Remove the temporary undo-span tracing now that the mid-drain Ctrl-Z fix has landed

Deletes the ring-buffer tracer and its diagnostic action; reverts the span
entry points to their pre-tracing no-parameter signatures.
This commit is contained in:
2026-08-09 13:53:56 -04:00
parent f6f09d983c
commit 258b1b6b85
10 changed files with 22 additions and 246 deletions
-1
View File
@@ -40,7 +40,6 @@ add_library(reaper_reasampler MODULE
${REASAMPLER_SRC_DIR}/shell/capture/insert.cpp
${REASAMPLER_SRC_DIR}/shell/undo/undo_block.cpp
${REASAMPLER_SRC_DIR}/shell/undo/undo_intercept.cpp
${REASAMPLER_SRC_DIR}/shell/undo/undo_trace.cpp
${REASAMPLER_SRC_DIR}/shell/view/view.cpp
${REASAMPLER_SRC_DIR}/shell/view/fx_reclaim.cpp
${REASAMPLER_SRC_DIR}/shell/view/view_lanes.cpp
+5 -41
View File
@@ -39,7 +39,6 @@
#include "shell/panel/panel_window.h" // panel lifecycle (init/toggle/open-query/shutdown)
#include "shell/persist/session.h" // ReaSamplerSession
#include "shell/undo/undo_intercept.h" // REAPER's own Undo/Redo, caught at dispatch
#include "shell/undo/undo_trace.h" // TEMPORARY — see undo_trace.h; revert together
#include "shell/view/fx_reclaim.h" // the FX reclaimer's tick
#include "shell/view/view.h" // applyMode
#include "shell/view/view_lanes.h" // reconcileManagedLanes
@@ -109,10 +108,6 @@ static void RunShowVersion(int) {
// the console window).
ShowConsoleMsg(("ReaSampler " + reasampler::version::appVersion() + "\n").c_str());
}
// TEMPORARY: drains the undo-trace ring buffer to the console in one call. See
// undo_trace.h; revert together with the rest of the undo-span tracing pass.
static void RunDumpUndoTrace(int) { reasampler::undoTraceDump(); }
// ONE row per bindable action this TU owns: FOREVER-STABLE id suffix, Actions-list
// phrase, handler, per-row arg. Registration, hookcommand dispatch, and the unload
// mirror-unregister all iterate this data. The capture scope rows come first,
@@ -174,9 +169,6 @@ static std::vector<reasampler::ActionTableRow> buildMainActionTable() {
rows.push_back({"IMPORT_BANK_PACKAGE", "import bank package (.rsbank)",
&RunImportBankPackage});
rows.push_back({"SHOW_VERSION", "show version", &RunShowVersion});
// TEMPORARY: see undo_trace.h; revert together with the rest of the undo-span
// tracing pass.
rows.push_back({"DUMP_UNDO_TRACE", "DIAG: dump undo trace", &RunDumpUndoTrace});
return rows;
}
@@ -191,17 +183,6 @@ static void OnTimer()
// the idle fast-path is a SINGLE POINTER TEST — drive only when a capture is live.
if (capture::g_rtCapture) capture::DriveRealtimeCapture(g_session);
// TEMPORARY: captured in a local (rather than calling GetCurrentProjectInLoadSave()
// twice) so the trace can report it without a second call, with no change to the
// call below. See undo_trace.h. Traced only on CHANGE — every tick reads this
// regardless of state, and the transition is the whole point of the line.
ReaProject* const loadSaveProj = GetCurrentProjectInLoadSave();
static ReaProject* s_tracedLoadSaveProj = nullptr;
if (loadSaveProj != s_tracedLoadSaveProj) {
REASAMPLER_TRACE("[Timer] loadSaveProj=%p", (void*)loadSaveProj);
s_tracedLoadSaveProj = loadSaveProj;
}
// A project load or save is IN FLIGHT, so this tick is running inside its own message
// pump — a restore brings plugins online and a plugin load pumps. Everything below
// would then act on a half-restored project: poll() reads the pre-load <EXTSTATE> that
@@ -211,10 +192,7 @@ static void OnTimer()
// the first tick after the load runs as if this one had never been due. Self-lowering
// (reaper_plugin_functions.h:1584-1587), and placed after the realtime drive so that
// fast path keeps its single-pointer-test shape. See src/shell/view/CLAUDE.md.
if (loadSaveProj) {
REASAMPLER_TRACE("[Timer] STANDOFF guard=loadSaveProj");
return;
}
if (GetCurrentProjectInLoadSave()) return;
// poll()'s undo/redo-reload and project-switch Load branches call
// loadFromProject, replacing g_session.view() wholesale — deferred here while
@@ -224,13 +202,8 @@ static void OnTimer()
// cheap to skip for one tick and resumes correctly, unlatched, on the next.
// The reclaim drain is latched for the same reason and is the same window: it
// holds that view model for the length of a plugin unload, which pumps.
if (!reasampler::modeApplyInProgress() && !reasampler::fxReclaimDrainInProgress()) {
if (!reasampler::modeApplyInProgress() && !reasampler::fxReclaimDrainInProgress())
g_session.poll();
} else {
REASAMPLER_TRACE("[Timer] SKIP-POLL modeApply=%d drain=%d",
reasampler::modeApplyInProgress() ? 1 : 0,
reasampler::fxReclaimDrainInProgress() ? 1 : 0);
}
// persist stays MODEL-ONLY (loads the saved view model but does not apply
// visibility, to avoid coupling persist to the view shell); poll() raises a
@@ -243,15 +216,8 @@ static void OnTimer()
// signal is one-shot, a re-entered applyMode fails closed, and a reapply dropped here
// is the saved mode never applied at all. Holding the whole block back one tick also
// keeps the panel re-arm ahead of the reapply.
// TEMPORARY: split into locals + if/else-if so the skip can be traced without
// calling consumeLoadSignal() (which has a side effect) a second time — same
// short-circuit semantics as the original single `if`. See undo_trace.h.
const bool loadGateModeApply = reasampler::modeApplyInProgress();
const bool loadGateDraining = reasampler::fxReclaimDrainInProgress();
if (loadGateModeApply || loadGateDraining) {
REASAMPLER_TRACE("[Timer] SKIP-LOAD modeApply=%d drain=%d", loadGateModeApply ? 1 : 0,
loadGateDraining ? 1 : 0);
} else if (g_session.consumeLoadSignal()) {
if (!reasampler::modeApplyInProgress() && !reasampler::fxReclaimDrainInProgress() &&
g_session.consumeLoadSignal()) {
reasampler::bankPanelNotifyProjectLoaded();
// Reconcile lane ownership against the live project's lanes (P_LANENAME,
// the cross-session source of truth) BEFORE reapplying visibility. Never
@@ -291,8 +257,6 @@ static void OnTimer()
// forked-sibling re-GUID branch requesting a reload here would bypass.
static void OnBeginLoadProjectState(bool isUndo, project_config_extension_t* /*reg*/)
{
REASAMPLER_TRACE("[Load] BeginLoadProjectState isUndo=%d", isUndo ? 1 : 0);
// UNBRANCHED, and the reason the terminator is a hook at all: this is REAPER's one
// entry for a state load — "project load/undo", plus a new project
// (reaper_plugin.h:1203-1205) — and the last moment before the restore. Why that
@@ -390,7 +354,7 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT(
// Same reason, for the reclaimer's undo span: a block left open here has no
// owner left to close it and would swallow every edit the user makes after.
reasampler::fxReclaimEndUndoSpan("extension unload");
reasampler::fxReclaimEndUndoSpan();
g_rec->Register("-timer", (void*)&OnTimer);
g_rec->Register("-projectconfig", (void*)&g_projectConfig);
+1 -5
View File
@@ -5,7 +5,6 @@
#include <utility>
#include "shell/undo/undo_trace.h" // TEMPORARY — see undo_trace.h; revert together
#include "shell/view/fx_reclaim.h" // fxReclaimEndUndoSpan — the span this must not nest over
#define REAPERAPI_MINIMAL
@@ -41,13 +40,12 @@ UndoBlock::UndoBlock(ReaProject* proj) : project_(proj) {
// outermost. Roll the depth back on any throw so the type degrades to "this one
// block failed to open," not "every block after it is now poisoned."
try {
if (g_depth++ == 0) fxReclaimEndUndoSpan("UndoBlock ctor");
if (g_depth++ == 0) fxReclaimEndUndoSpan();
Undo_BeginBlock2(proj);
} catch (...) {
--g_depth;
throw;
}
REASAMPLER_TRACE("[UndoBlock] CTOR proj=%s depth=%d", proj ? "real" : "active", g_depth);
}
UndoBlock::~UndoBlock() {
@@ -65,8 +63,6 @@ UndoBlock::~UndoBlock() {
if (!project_ || ValidatePtr2(nullptr, project_, "ReaProject*"))
Undo_EndBlock2(project_, label_.c_str(), mask_);
--g_depth;
REASAMPLER_TRACE("[UndoBlock] DTOR label=\"%s\" mask=%d depth=%d",
label_.empty() ? "(discard)" : label_.c_str(), mask_, g_depth);
}
void UndoBlock::recordPoint(std::string label, int mask) {
+1 -6
View File
@@ -3,20 +3,15 @@
#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");
fxReclaimEndUndoSpan();
}
} // namespace reasampler
-94
View File
@@ -1,94 +0,0 @@
// See undo_trace.h. Compiled into the reaper_reasampler module; includes
// reaper_plugin_functions.h without REAPERAPI_IMPLEMENT (main.cpp owns that). Isolated
// in its own TU so every call site needs only the trace-only header, not the REAPER
// ShowConsoleMsg want-define.
#include "shell/undo/undo_trace.h"
#if REASAMPLER_UNDO_TRACE
#include <chrono>
#include <cstdarg>
#include <cstdio>
#include <string>
#define REAPERAPI_MINIMAL
#define REAPERAPI_WANT_ShowConsoleMsg
#include "reaper_plugin_functions.h"
namespace reasampler {
namespace {
// A few thousand lines comfortably outlasts a full mode switch, its background park
// drain, and a mid-drain undo — the exact span the dump has to still hold whole when
// fired well after the fact. Static storage: one allocation for the process, none per
// line.
constexpr std::size_t kCapacity = 4096;
constexpr std::size_t kLineChars = 288;
struct Line {
char text[kLineChars];
};
Line g_lines[kCapacity];
std::size_t g_next = 0; // next write slot, wraps
unsigned long long g_totalWritten = 0; // lines written since the last dump/clear
unsigned long long g_seq = 0;
double startMs() {
static const double t0 = std::chrono::duration<double, std::milli>(
std::chrono::steady_clock::now().time_since_epoch())
.count();
return t0;
}
double nowMs() {
return std::chrono::duration<double, std::milli>(
std::chrono::steady_clock::now().time_since_epoch())
.count() -
startMs();
}
} // namespace
void undoTrace(const char* fmt, ...) {
char body[224];
va_list args;
va_start(args, fmt);
std::vsnprintf(body, sizeof(body), fmt, args);
va_end(args);
Line& line = g_lines[g_next];
std::snprintf(line.text, sizeof(line.text), "#%llu %.0fms %s\n", ++g_seq, nowMs(), body);
g_next = (g_next + 1) % kCapacity;
++g_totalWritten;
}
void undoTraceDump() {
const bool overflowed = g_totalWritten > kCapacity;
const std::size_t count =
overflowed ? kCapacity : static_cast<std::size_t>(g_totalWritten);
const std::size_t oldest = overflowed ? g_next : 0;
std::string out;
out.reserve(count * 96 + 128);
if (overflowed) {
char marker[128];
std::snprintf(marker, sizeof(marker),
"[UT OVERFLOW: %llu lines dropped, buffer holds %zu]\n",
g_totalWritten - kCapacity, kCapacity);
out += marker;
}
for (std::size_t i = 0; i < count; ++i) out += g_lines[(oldest + i) % kCapacity].text;
if (out.empty()) out = "[UT] buffer empty\n";
ShowConsoleMsg(out.c_str());
g_next = 0;
g_totalWritten = 0;
}
} // namespace reasampler
#endif // REASAMPLER_UNDO_TRACE
-41
View File
@@ -1,41 +0,0 @@
#pragma once
// TEMPORARY diagnostic tracing for the Design-View undo-span bug (many-undo-entries-on-
// Ctrl-Z-mid-drain). NOT for shipping. ONE switch below gates every trace line in the
// tree; flip REASAMPLER_UNDO_TRACE to 0, or delete this file pair plus the call sites
// that include it, to revert.
//
// Lines land in an in-memory ring buffer, never straight to the console — a tick-rate
// trace point that called ShowConsoleMsg directly scrolled the console faster than
// Daniel could copy the interesting span. "DIAG: dump undo trace"
// (shell/actions/action_registry, DUMP_UNDO_TRACE) is the one bindable action that
// drains the buffer to the console.
#define REASAMPLER_UNDO_TRACE 1
#if REASAMPLER_UNDO_TRACE
namespace reasampler {
// Appends one line to the ring buffer: "#<seq> <ms>ms <fmt...>\n". printf-style;
// sequence number and a monotonic millisecond timestamp are stamped here so lines sort
// and date themselves however the buffer is later drained. No ShowConsoleMsg call here
// — that is what kept this off the reentrancy risk a tick-rate console write posed.
// Keep each `fmt` short — this has to read at a glance once dumped.
void undoTrace(const char* fmt, ...);
// Dumps the whole buffer to the console in ONE ShowConsoleMsg call, oldest line
// first, then clears it. If the buffer wrapped since the last dump, the dump is
// prefixed with a marker naming how many lines were lost rather than dropping them
// silently.
void undoTraceDump();
} // namespace reasampler
#define REASAMPLER_TRACE(...) ::reasampler::undoTrace(__VA_ARGS__)
#else
#define REASAMPLER_TRACE(...) \
do { \
} while (0)
#endif
-9
View File
@@ -375,15 +375,6 @@ 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
+13 -43
View File
@@ -12,7 +12,6 @@
#include "shell/persist/session.h" // flushViewState — the release's durable half
#include "shell/undo/undo_block.h" // undoBlockOpen — the reverse of the nesting rule
#include "shell/undo/undo_trace.h" // TEMPORARY — see undo_trace.h; revert together
#include "shell/view/view.h" // modeApplyInProgress, transportBlocksModeSwitch
#include "shell/view/view_park.h" // fxGuidString — the one FX-identity read
#include "shell/view/view_tracks.h"
@@ -47,17 +46,11 @@ UndoSpan g_span;
// unconditionally; the close reads the span's values before clearing it, because the
// clear has to precede the SDK call — nothing re-entered through Undo_EndBlock2 may
// find a span it could close a second time.
//
// `caller`/`reason` are TEMPORARY tracing plumbing only (see shell/undo/undo_trace.h) —
// they select nothing and gate nothing.
void openSpan(ReaProject* proj, const char* label, int mask, const char* caller) {
if (g_span.begin(proj, label, mask)) {
Undo_BeginBlock2(proj);
REASAMPLER_TRACE("[UndoSpan] OPEN caller=%s label=\"%s\" mask=%d", caller, label, mask);
}
void openSpan(ReaProject* proj, const char* label, int mask) {
if (g_span.begin(proj, label, mask)) Undo_BeginBlock2(proj);
}
void closeSpan(const char* reason) {
void closeSpan() {
if (!g_span.open()) return;
ReaProject* const proj = g_span.project();
const std::string label = g_span.label();
@@ -66,8 +59,6 @@ void closeSpan(const char* reason) {
// The span outlives its opener, so by here the project may be the one that just
// closed — its undo history went with it and there is no block left to end.
if (ValidatePtr2(nullptr, proj, "ReaProject*")) Undo_EndBlock2(proj, label.c_str(), mask);
REASAMPLER_TRACE("[UndoSpan] CLOSE reason=%s label=\"%s\" mask=%d",
reason, label.empty() ? "(discard)" : label.c_str(), mask);
}
// Monotonic and sub-millisecond: the drain's budget discriminates at 30ms, which
@@ -180,12 +171,9 @@ struct ReclaimHost {
// no-op under a switch's span, which is why the FX bit is added separately —
// here, at the first real write, so a switch whose parking never matured closes
// on the flags-and-items mask it opened with and serializes no FX state.
const bool spanWasOpenBeforeWrite = g_span.open();
openSpan(project, kReclaimUndoLabel, UNDO_STATE_FX, "drain");
openSpan(project, kReclaimUndoLabel, UNDO_STATE_FX);
g_span.widen(UNDO_STATE_FX);
TrackFX_SetOffline(tr, fxIndex, offline);
REASAMPLER_TRACE("[writeOffline] track=%s fx=%d offline=%d spanWasOpen=%d",
guid.c_str(), fxIndex, offline ? 1 : 0, spanWasOpenBeforeWrite ? 1 : 0);
return true;
}
@@ -253,16 +241,10 @@ bool fxReclaimDrainInProgress() { return g_draining; }
void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
// NO guard here may close the span: each names a caller that is still on the stack and
// still owns it — the outer drain, or the applyMode that opened it. All re-tick.
if (g_draining) {
REASAMPLER_TRACE("[Tick] STANDOFF guard=g_draining");
return;
}
if (g_draining) return;
// The model is mid-rewrite and its records are being consumed out from under any
// plan derived from it. The apply invalidates on its way out, so nothing is lost.
if (modeApplyInProgress()) {
REASAMPLER_TRACE("[Tick] STANDOFF guard=modeApplyInProgress");
return;
}
if (modeApplyInProgress()) return;
// The MIRROR of the rule that no block may open over the span: an extension undo
// block is open — a capture, a bank verb, an ingest — and its own message pump is how
// this tick arrived. openSpan from in there would begin at depth 2, and that block's
@@ -274,10 +256,7 @@ void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
// claim is this function's own control-flow order — this TU is REAPER-linked, off the
// pure fx_reclaim_tests target, so unlike the queue/span state machine above it, the
// ordering is DAW-verified, not unit-tested.
if (undoBlockOpen()) {
REASAMPLER_TRACE("[Tick] STANDOFF guard=undoBlockOpen");
return;
}
if (undoBlockOpen()) return;
DrainLatch latch;
// beginTick owns what a pin/project mismatch means. What is local to here: the model
@@ -291,8 +270,7 @@ void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
// rather than the queue's pin, which moves independently: a tab switch, a closed
// project, or no active project at all ends it where it stands rather than carrying
// it into another project's history — and the no-project arm below returns.
if (g_span.open() && g_span.project() != project)
closeSpan(project ? "project retarget" : "no active project");
if (g_span.open() && g_span.project() != project) closeSpan();
if (!g_queue.beginTick(project)) return;
// DERIVED, never persisted: recomputed from view state every time the queue was
@@ -306,26 +284,19 @@ void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
ReclaimHost host{project, &session};
const ReclaimRunResult run = g_queue.run(host);
reportDrops(project, run.drops);
if (spanEndsAfter(run)) {
// Priority order for the trace label only, when more than one is true at once —
// does not affect which branch runs; spanEndsAfter already decided that.
const char* reason = run.aborted ? "drain aborted"
: run.transportDeferred ? "transport-deferred"
: "drain settled";
closeSpan(reason);
}
if (spanEndsAfter(run)) closeSpan();
if (!run.aborted && run.settled) g_queue.goIdle();
}
void fxReclaimBeginUndoSpan(ReaProject* proj) {
openSpan(proj, "", 0, "action");
openSpan(proj, "", 0);
}
void fxReclaimLabelUndoSpan(const char* label, int mask) {
g_span.relabel(label ? label : "", mask);
}
void fxReclaimEndUndoSpan(const char* reason) { closeSpan(reason); }
void fxReclaimEndUndoSpan() { closeSpan(); }
void fxReclaimAbandonForStateLoad() {
// Close FIRST and unconditionally. The abandon below stops the drain and puts the loop
@@ -335,8 +306,7 @@ void fxReclaimAbandonForStateLoad() {
// confirmed case of that shape on the save side) — the span can never be open at depth
// > 0 in the first place: src/shell/view/CLAUDE.md's "NOTHING MAY OPEN AN UNDO BLOCK
// WHILE THE SPAN IS OPEN" invariant. closeSpan() just finds nothing to close there.
REASAMPLER_TRACE("[abandon] spanOpen=%d", g_span.open() ? 1 : 0);
closeSpan("state load");
closeSpan();
g_queue.abandon();
}
@@ -352,7 +322,7 @@ void fxReclaimFlushRelease(ReaSamplerSession& session) {
// absorb, and the arm is not what decides whether one is open. Same "depth > 0 never
// holds an open span" invariant as fxReclaimAbandonForStateLoad above makes this safe
// even when the save itself is issued from inside an extension UndoBlock.
closeSpan("project save");
closeSpan();
if (!g_releaseFlush.pendingFor(saving)) return;
// Promptless, unlike design_view_actions' persistViewState, which can open a modal
// Save-As. [verify — DAW] whether this write reaches the <EXTSTATE> block of the
+1 -5
View File
@@ -699,11 +699,7 @@ void fxReclaimLabelUndoSpan(const char* label, int mask);
// 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
// of the undo-span tracing pass.
void fxReclaimEndUndoSpan(const char* reason);
void fxReclaimEndUndoSpan();
// EVERY project-state load, undo-driven or not — Ctrl-Z, Ctrl-Shift-Z, a project open, a
// File > Revert, a new project. Ends the span, stops any drain on the stack, drops the
+1 -1
View File
@@ -126,7 +126,7 @@ ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetMod
// 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("next apply supersedes");
fxReclaimEndUndoSpan();
// BEFORE the open, never after this function returns. The wakeup is what
// guarantees a tick, and a tick is the only thing that can close this span; a
// throw anywhere below would otherwise leave the block open with the reclaimer