view: assert the FX-park coalescing delay's debounce; correct four overclaiming doc/comment claims
Extracts parkReadyAt/parkIsReady as a tested pure fold per PLAN.md's phase criterion; the rest is wording fixes — hitch bound, hazard width, forced-drain scope, progressive CPU reclaim.
This commit is contained in:
+26
-11
@@ -57,17 +57,32 @@ decide membership or mode rules.
|
||||
`[verify — DAW]`. **The drain is SPLIT BY INTENT KIND, because only one kind is
|
||||
unsafe to persist over.** A pending RESTORE is: flags restored, snapshot already
|
||||
dropped, FX still offline — a save inside that window records offline FX beside a
|
||||
model that can no longer replan them, unrecoverable on reopen. So any path that
|
||||
serializes the view model drains every restore synchronously first
|
||||
(`persistViewState`, `render_in_place`), and an action-driven switch still pays
|
||||
the RELOAD hitch before it returns — its repaint and undo block have both closed
|
||||
by then, which is what the deferral was for. Any new caller that reapplies a mode
|
||||
and then persists inherits this obligation. A pending PARK is safe to save over:
|
||||
flags parked, snapshot stored, FX still online, so a reopen replans the park and
|
||||
the drain offlines it — it converges. Parks therefore go lazy, ONE FX per idle
|
||||
tick behind a one-second coalescing delay, so the unload half never blocks the
|
||||
UI at all. The delay is also the width of the live-enumeration hazard at
|
||||
`tickDeferredFxParks` — do not lengthen it casually.
|
||||
model that can no longer replan them, unrecoverable on reopen. `persistViewState`
|
||||
and `render_in_place` are the only two paths that force the restore drain before
|
||||
they serialize; roughly ten other `saveToActiveProject` callers (`ingest.cpp`,
|
||||
`bank_ops.cpp`, `bake_land.cpp`, `capture_batch.cpp`, `capture_orchestrator.cpp`,
|
||||
`realtime_lifecycle.cpp`, `panel_input.cpp`) persist without forcing it first — a
|
||||
pre-existing, roughly one-tick-wide gap (the idle tick drains restores too),
|
||||
not known to be exploitable in practice. A new caller that reapplies a mode and
|
||||
then persists inherits the forced-drain obligation; one that merely persists does
|
||||
not. `drainDeferredFxRestores` is also a silent no-op while a drain is already in
|
||||
flight (`g_draining`), so a restore forced from inside another drain's
|
||||
message-pump window can itself be skipped — same order of window, same
|
||||
pre-existing status. An action-driven switch still pays the RELOAD hitch before
|
||||
it returns — its repaint and undo block have both closed by then, which is what
|
||||
the deferral was for. A pending PARK is safe to save over: flags parked, snapshot
|
||||
stored, FX still online, so a reopen replans the park and the drain offlines it —
|
||||
it converges. Parks therefore go lazy, ONE FX per idle tick behind a one-second
|
||||
coalescing delay, which bounds each tick's hitch to one FX's unload rather than
|
||||
eliminating it — see `view_fx_park.h` for why per-track chunking would not help.
|
||||
CPU reclaim is therefore PROGRESSIVE, completing over FX-count ticks rather than
|
||||
in one step: a user watching the CPU meter after a switch sees a ramp, not a
|
||||
drop — in tension with `design-view.md`'s full-CPU-park rationale ("a heavy FX
|
||||
bench you don't want taxing the CPU while you arrange"). The delay is only ONE term
|
||||
of the live-enumeration hazard's width: `firstOnlineFx` re-enumerates the chain
|
||||
every tick, not just the first, so the actual exposure is delay + (FX count ×
|
||||
tick interval), and for a large chain the second term dominates — see
|
||||
`tickDeferredFxParks`. Do not lengthen the delay casually.
|
||||
`[verify — DAW]` whether the switch's `UpdateArrange` paints before the first
|
||||
drain tick: nothing pumps the message loop between them, so "the switch paints
|
||||
first" is an assumption, not an SDK guarantee.
|
||||
|
||||
@@ -33,14 +33,19 @@ namespace {
|
||||
// swallow a rapid A→B→A flip whole — that flip's restore then cancels the park
|
||||
// outright and the plugins never move at all — and short enough that the window
|
||||
// in which a user could add an FX to the track the switch just hid, and so miss
|
||||
// the pre-park snapshot, stays implausible. Seconds, not minutes: this delay is
|
||||
// the width of the hazard documented at tickDeferredFxParks.
|
||||
// the pre-park snapshot, stays implausible. Seconds, not minutes; this is ONE
|
||||
// term of the hazard's width — see tickDeferredFxParks for the other.
|
||||
constexpr double kParkCoalesceSeconds = 1.0;
|
||||
|
||||
FxParkQueue g_queue;
|
||||
ReaProject* g_owner = nullptr; // the project the pending intents were enqueued against
|
||||
bool g_draining = false;
|
||||
double g_parkReadyAt = 0.0; // time_precise() stamp; re-armed by every enqueued park
|
||||
// The gate is GLOBAL, not per-track: a park enqueued for track Z re-arms this
|
||||
// same stamp even though it does nothing to an already-partial park on track A.
|
||||
// Benign — that track's FX are already I_FXEN=0/B_MAINSEND=0 from the
|
||||
// synchronous half, so only its CPU reclaim (not correctness) is delayed — but
|
||||
// this is not a per-intent debounce.
|
||||
double g_parkReadyAt = 0.0; // parkReadyAt() stamp; re-armed by every enqueued park
|
||||
|
||||
// Makes "one drain at a time" explicit rather than implied by the call sites.
|
||||
// RAII because an apply can throw and a stuck flag would silence the queue for
|
||||
@@ -81,10 +86,11 @@ int firstOnlineFx(MediaTrack* tr) {
|
||||
return -1;
|
||||
}
|
||||
|
||||
// One track by GUID. The park tick resolves ONE track and does so afresh every
|
||||
// tick — a whole-project map would be rebuilt per tick for a single lookup, and
|
||||
// caching the handle across ticks is exactly the dangle ValidatePtr2 is here to
|
||||
// prevent.
|
||||
// One track by GUID, resolved afresh every tick — caching the handle across
|
||||
// ticks is exactly the dangle ValidatePtr2 is here to prevent. Still O(tracks),
|
||||
// still one guidString() per track per tick; a per-tick unordered_map keyed by
|
||||
// GUID would cost the same enumeration to build for a single lookup, so the only
|
||||
// thing avoided by NOT building one is that allocation.
|
||||
MediaTrack* findTrack(ReaProject* proj, const std::string& guid) {
|
||||
const int count = CountTracks(proj);
|
||||
for (int i = 0; i < count; ++i) {
|
||||
@@ -169,7 +175,7 @@ std::vector<FxOfflineOp> deferFxPark(ReaProject* proj, const std::string& guid)
|
||||
adoptOwner(proj);
|
||||
// Re-armed, not set once: while switches keep arriving the park keeps
|
||||
// waiting, which is the whole point of coalescing them.
|
||||
g_parkReadyAt = time_precise() + kParkCoalesceSeconds;
|
||||
g_parkReadyAt = parkReadyAt(time_precise(), kParkCoalesceSeconds);
|
||||
return g_queue.park(guid);
|
||||
}
|
||||
|
||||
@@ -239,7 +245,7 @@ void drainDeferredFxRestores() {
|
||||
|
||||
void tickDeferredFxParks() {
|
||||
if (!drainGateOpen()) return;
|
||||
if (time_precise() < g_parkReadyAt) return; // still coalescing
|
||||
if (!parkIsReady(time_precise(), g_parkReadyAt)) return; // still coalescing
|
||||
|
||||
const std::string guid = g_queue.nextParkGuid();
|
||||
if (guid.empty()) return; // restores only — not this half's work
|
||||
|
||||
@@ -16,6 +16,15 @@ class ReaProject;
|
||||
|
||||
namespace reasampler {
|
||||
|
||||
// The coalescing delay's gate, pure so the debounce can be asserted without a
|
||||
// DAW clock (test_view_fx_park.cpp). parkReadyAt is called on EVERY enqueue,
|
||||
// not just the first — that re-arming from each call's own `now` is what makes
|
||||
// it a debounce rather than a one-shot timer, and is what lets a rapid A→B→A
|
||||
// flip (three enqueues inside one delay window) cost a single wait measured
|
||||
// from the last flip rather than three, or one anchored to the first.
|
||||
inline double parkReadyAt(double now, double coalesceSeconds) { return now + coalesceSeconds; }
|
||||
inline bool parkIsReady(double now, double readyAt) { return now >= readyAt; }
|
||||
|
||||
// The chain as it stands now: identity by current slot. Snapshot, park and
|
||||
// restore all address FX through this one plain 0..TrackFX_GetCount-1
|
||||
// enumeration — never the 0x1000000/0x2000000 input-FX or container forms — so
|
||||
@@ -27,14 +36,11 @@ std::vector<std::string> liveFxGuids(MediaTrack* tr);
|
||||
struct FxParkIntent {
|
||||
std::string guid;
|
||||
bool park = false;
|
||||
// A park write is landing or has landed on this track, so the live chain
|
||||
// matches NEITHER endpoint. Parks apply one FX per idle tick, so this is a
|
||||
// state a park genuinely sits in — and while it holds, the cancel rule
|
||||
// below is unsafe in both directions: only the restore's own ops can put
|
||||
// back what a half-finished park offlined, and a park arriving on that
|
||||
// restore must RESUME rather than annihilate or it strands every FX the
|
||||
// first pass had not reached. Restores carry no such state: they are
|
||||
// detached whole and applied in one drain.
|
||||
// Set once a park write has landed on this track — the live chain then
|
||||
// matches NEITHER endpoint, so the cancel-on-inverse rule below is unsafe
|
||||
// until the intent resumes/supersedes instead of annihilating. Restores
|
||||
// carry no such state: they are detached whole and applied in one drain.
|
||||
// Full contract: src/shell/view/CLAUDE.md's `view_fx_park` entry.
|
||||
bool partial = false;
|
||||
std::vector<FxOfflineOp> restoreOps;
|
||||
};
|
||||
@@ -158,22 +164,16 @@ struct PreParkFx {
|
||||
|
||||
// The FX half a fresh pre-park snapshot must carry when the park CANCELLED a
|
||||
// pending restore: those ops are the only surviving record of the pre-park
|
||||
// state, because the chain still reads the parked values until that restore
|
||||
// drains — and it never will, the cancel dropped it. Empty in (nothing was
|
||||
// cancelled) means the caller reads the live chain instead. The restore's own
|
||||
// keying travels with it so a slot-keyed snapshot lifted from a legacy
|
||||
// view_state does not silently become an identity-keyed one with no identities.
|
||||
//
|
||||
// FAITHFUL TO THE SNAPSHOT, NOT THE CHAIN. The ops describe the chain as it was
|
||||
// at the ORIGINAL park; applyPark enumerates it again at drain time. So an FX
|
||||
// added while the track was parked (a floating FX-chain window, ReaScript) is
|
||||
// absent from this reconstruction yet IS offlined by the cancelling park's
|
||||
// drain — and so never comes back online. Rare, and recoverable by hand in the
|
||||
// FX chain, but specific to the deferral.
|
||||
//
|
||||
// `offline` is already boolean by the time it arrives: makeRestorePlan narrowed
|
||||
// FxOfflineState's defensive int to FxOfflineOp's bool, so this widening back to
|
||||
// int restores the type, not lost information.
|
||||
// state, because the chain still reads the parked values and the cancel means
|
||||
// no drain will ever put them back. Empty (nothing cancelled) means the caller
|
||||
// reads the live chain instead; the restore's own keying travels with it so a
|
||||
// slot-keyed snapshot does not silently become identity-keyed with no
|
||||
// identities. The ops describe the chain as of the ORIGINAL park, not the live
|
||||
// chain applyPark re-enumerates at drain time — so an FX added while parked (a
|
||||
// floating FX-chain window, ReaScript) is absent here yet still offlined by the
|
||||
// cancelling park's drain, and never comes back online. Rare and hand-recoverable.
|
||||
// `offline` widens back from FxOfflineOp's bool to FxOfflineState's defensive int
|
||||
// — restoring the type, not adding information.
|
||||
inline PreParkFx preParkFxFromCancelledRestore(const std::vector<FxOfflineOp>& cancelled) {
|
||||
PreParkFx out;
|
||||
if (cancelled.empty()) return out;
|
||||
@@ -202,33 +202,27 @@ void deferFxRestore(ReaProject* proj, const std::string& guid, std::vector<FxOff
|
||||
// idle, and both early-out re-entrantly: one drain at a time, the outer one owns
|
||||
// the queue.
|
||||
//
|
||||
// RESTORES, applied whole, synchronously. Called on the idle tick AND before the
|
||||
// view model is serialized (persistViewState, render_in_place). The second call
|
||||
// is not an optimization: between a restore's synchronous flag writes and its
|
||||
// drain the FX are still offline while the model has already dropped the
|
||||
// snapshot that would replan them, so a save inside that window — deterministic
|
||||
// under a custom action chain like "toggle mode; save project" — records offline
|
||||
// FX beside a snapshot-free model, and nothing on reopen brings them back
|
||||
// online. The cost is that an action-driven switch pays the RELOAD hitch before
|
||||
// it returns; its repaint and undo block have both closed by then, which is what
|
||||
// the deferral was for.
|
||||
// RESTORES, applied whole, synchronously. Called on the idle tick and forced by
|
||||
// a subset of the paths that serialize the view model. WHICH paths force it, and
|
||||
// why forcing is required, is src/shell/view/CLAUDE.md's Documented-caveat entry
|
||||
// — kept there only, not restated here, so the two cannot drift apart.
|
||||
void drainDeferredFxRestores();
|
||||
|
||||
// PARKS, one FX per idle tick behind a short coalescing delay. Nothing waits on
|
||||
// a park, and unlike a restore it is safe to persist over: the flags are already
|
||||
// parked and the snapshot already stored, so a .rpp saved mid-window reopens with
|
||||
// the FX online, applyMode replans a park for that inactive leaf, and the drain
|
||||
// offlines it. It converges on its own, which is why this half does not force.
|
||||
// PARKS, one FX per idle tick behind a short coalescing delay (parkReadyAt /
|
||||
// parkIsReady above). Unlike a restore it is safe to persist over — it
|
||||
// converges on its own; see CLAUDE.md for why, and for the CPU-reclaim-is-
|
||||
// progressive consequence.
|
||||
//
|
||||
// One FX, not one track: a single convolution reverb or loaded sampler is the
|
||||
// unit of cost, so per-track chunking would not bound the hitch.
|
||||
// unit of cost, so per-track chunking would not bound the hitch — only the
|
||||
// per-FX split does, and only to ONE FX's unload per tick, not to zero.
|
||||
//
|
||||
// The delay's cost is the width of a documented hazard, not a new one. applyPark
|
||||
// enumerates the chain LIVE at drain time while the pre-park snapshot was taken
|
||||
// at switch time, so an FX added inside the window is offlined carrying no
|
||||
// snapshot entry — resolveFxRestore then has no op for it and it stays offline.
|
||||
// Same failure as the cancelled-restore case below, over a longer window; keep
|
||||
// the delay short.
|
||||
// The live-enumeration hazard's width is delay + (FX count × tick interval), NOT
|
||||
// the delay alone: firstOnlineFx re-enumerates the chain LIVE every tick, not
|
||||
// just the first, so an FX added at any point before the park retires is
|
||||
// offlined carrying no snapshot entry — resolveFxRestore then has no op for it
|
||||
// and it stays offline. Same failure as the cancelled-restore case above, over a
|
||||
// longer window. Do not lengthen the delay casually; it is only one term.
|
||||
void tickDeferredFxParks();
|
||||
|
||||
// Drops every pending intent without applying it. Called when the model the
|
||||
|
||||
Reference in New Issue
Block a user