Make an FX reclaim step one whole track's chain: drop the resume cursor, yield only between tracks
This commit is contained in:
@@ -7,32 +7,9 @@ namespace reasampler::view {
|
||||
|
||||
namespace {
|
||||
|
||||
// Everything up to and including the resume point is already done. An empty
|
||||
// `resumeAfterFxGuid` is ambiguous by itself — "nothing done yet" and "the last
|
||||
// completed op had no identity" look identical — so an empty guid falls back to
|
||||
// matching by slot instead; `resumeAfterSlot < 0` is what actually means
|
||||
// "nothing done yet". Not found ⇒ nothing dropped: the resume point left the
|
||||
// chain between ticks, and re-walking a compare-before-write list beats
|
||||
// trusting a cursor that no longer points at anything.
|
||||
std::vector<FxOfflineOp> remainingAfter(const std::vector<FxOfflineOp>& ops,
|
||||
const std::string& resumeAfterFxGuid,
|
||||
int resumeAfterSlot) {
|
||||
if (resumeAfterFxGuid.empty() && resumeAfterSlot < 0) return ops;
|
||||
for (std::size_t i = 0; i < ops.size(); ++i) {
|
||||
const bool isResumePoint = resumeAfterFxGuid.empty()
|
||||
? ops[i].slot == resumeAfterSlot
|
||||
: ops[i].fxGuid == resumeAfterFxGuid;
|
||||
if (isResumePoint)
|
||||
return std::vector<FxOfflineOp>(ops.begin() + static_cast<std::ptrdiff_t>(i) + 1,
|
||||
ops.end());
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
// Slot keying, deliberately: the live chain IS the target set, so its positions
|
||||
// are current by construction. Identity keying would drop an FX REAPER reports no
|
||||
// GUID for, and reclaim has to reach those too — the fxGuid rides along only so
|
||||
// the resume point has something to match.
|
||||
// GUID for, and reclaim has to reach those too.
|
||||
std::vector<FxOfflineOp> offlineTargets(const std::string& trackGuid,
|
||||
const std::vector<std::string>& liveFxGuids) {
|
||||
std::vector<FxOfflineOp> ops;
|
||||
@@ -70,14 +47,13 @@ std::vector<FxTrackWork> desiredFxState(const ViewModeModel& model,
|
||||
if (liveTrackGuids.count(guid) == 0) continue; // dead track — nothing to write
|
||||
|
||||
if (!model.leafBelongsToMode(guid, targetMode)) {
|
||||
offline.push_back(FxTrackWork{guid, FxWorkKind::Offline, {}, {}});
|
||||
offline.push_back(FxTrackWork{guid, FxWorkKind::Offline, {}});
|
||||
continue;
|
||||
}
|
||||
// Active with the FX half already released: the chain is back online and
|
||||
// the record is only waiting on its flags. Nothing owed either way.
|
||||
if (!snap.fxHeld) continue;
|
||||
online.push_back(
|
||||
FxTrackWork{guid, FxWorkKind::Online, fxRestoreTargets(model, guid), {}});
|
||||
online.push_back(FxTrackWork{guid, FxWorkKind::Online, fxRestoreTargets(model, guid)});
|
||||
}
|
||||
|
||||
online.reserve(online.size() + offline.size());
|
||||
@@ -87,14 +63,9 @@ std::vector<FxTrackWork> desiredFxState(const ViewModeModel& model,
|
||||
|
||||
FxRestoreResolution resolveFxWork(const FxTrackWork& work,
|
||||
const std::vector<std::string>& liveFxGuids) {
|
||||
if (work.kind == FxWorkKind::Offline) {
|
||||
return resolveFxRestore(
|
||||
remainingAfter(offlineTargets(work.trackGuid, liveFxGuids), work.resumeAfterFxGuid,
|
||||
work.resumeAfterSlot),
|
||||
liveFxGuids);
|
||||
}
|
||||
return resolveFxRestore(
|
||||
remainingAfter(work.targets, work.resumeAfterFxGuid, work.resumeAfterSlot), liveFxGuids);
|
||||
if (work.kind == FxWorkKind::Offline)
|
||||
return resolveFxRestore(offlineTargets(work.trackGuid, liveFxGuids), liveFxGuids);
|
||||
return resolveFxRestore(work.targets, liveFxGuids);
|
||||
}
|
||||
|
||||
} // namespace reasampler::view
|
||||
|
||||
+10
-26
@@ -19,9 +19,13 @@ enum class FxWorkKind {
|
||||
Offline, // reclaim: every live FX offline
|
||||
};
|
||||
|
||||
// One track's whole FX move. Track-granular so a drain may stop between tracks,
|
||||
// and resumable WITHIN a track by FX identity: a slot cursor would silently skip
|
||||
// or repeat a plugin if the chain were edited between two ticks.
|
||||
// One track's whole FX move, and the ATOMIC unit of work: a drain may stop
|
||||
// between items, never inside one, so an item is either untouched or complete
|
||||
// and no partial state survives a tick boundary. Do not reintroduce a
|
||||
// within-track resume cursor — carrying one across ticks meant reconciling it
|
||||
// against a chain the user may have edited in between, and the case an FX with
|
||||
// no reported identity forced (a cursor that reads as "nothing done yet" and
|
||||
// re-walks the item forever) was a live non-termination hazard.
|
||||
struct FxTrackWork {
|
||||
std::string trackGuid;
|
||||
FxWorkKind kind = FxWorkKind::Offline;
|
||||
@@ -32,28 +36,8 @@ struct FxTrackWork {
|
||||
// read it, so resolveFxWork synthesises the targets from what it is handed.
|
||||
std::vector<FxOfflineOp> targets;
|
||||
|
||||
// Identity of the last FX this item's drain completed; empty means EITHER
|
||||
// nothing has completed yet OR the completed FX itself carried no identity
|
||||
// (REAPER reports no GUID for it, or it came from a Slot-keyed record) — see
|
||||
// resumeAfterSlot below, which disambiguates the two. This is load-bearing
|
||||
// for forward progress, not an optimisation: without the disambiguation, an
|
||||
// unidentified FX would look like "none yet" and the drain would restart at
|
||||
// the top of the list every tick, forever. Compare-before-write makes every
|
||||
// write idempotent, so an identity that has since left the chain safely
|
||||
// restarts the item rather than stranding it.
|
||||
std::string resumeAfterFxGuid;
|
||||
|
||||
// Position of the last completed op, consulted only when resumeAfterFxGuid
|
||||
// is empty, to tell "resumed past an unidentified FX at this slot" apart
|
||||
// from "nothing done yet" (-1). Every FxOfflineOp carries its slot
|
||||
// regardless of keying, so this is available for both Online and Offline
|
||||
// work.
|
||||
int resumeAfterSlot = -1;
|
||||
|
||||
bool operator==(const FxTrackWork& o) const {
|
||||
return trackGuid == o.trackGuid && kind == o.kind && targets == o.targets &&
|
||||
resumeAfterFxGuid == o.resumeAfterFxGuid &&
|
||||
resumeAfterSlot == o.resumeAfterSlot;
|
||||
return trackGuid == o.trackGuid && kind == o.kind && targets == o.targets;
|
||||
}
|
||||
};
|
||||
|
||||
@@ -82,8 +66,8 @@ std::vector<FxOfflineOp> fxRestoreTargets(const ViewModeModel& model,
|
||||
|
||||
// Resolves one work item against the chain as it stands, where `liveFxGuids[i]`
|
||||
// is the identity of the FX at slot `i` right now (empty when REAPER reported
|
||||
// none). Honors the resume point, then matches by identity: an identity that is
|
||||
// not live is DROPPED, never re-pointed at a slot.
|
||||
// none) — the item's WHOLE move, matched by identity: an identity that is not
|
||||
// live is DROPPED, never re-pointed at a slot.
|
||||
FxRestoreResolution resolveFxWork(const FxTrackWork& work,
|
||||
const std::vector<std::string>& liveFxGuids);
|
||||
|
||||
|
||||
@@ -38,7 +38,8 @@ ParkClock g_clock;
|
||||
|
||||
// TrackFX_SetOffline is ASSUMED to pump the message loop, so a plugin load can re-enter
|
||||
// the timer and land back here mid-drain. Fail closed: a re-entered drain would walk the
|
||||
// same items the outer one is mutating and double-write from a stale resume cursor.
|
||||
// same queue the outer one holds a reference into, re-running the very item that outer
|
||||
// step is still writing.
|
||||
bool g_draining = false;
|
||||
struct DrainLatch {
|
||||
DrainLatch() { g_draining = true; }
|
||||
@@ -99,9 +100,12 @@ struct ReclaimHost {
|
||||
if (TrackFX_GetOffline(tr, fxIndex) == offline) return false; // already there
|
||||
|
||||
// DAW-VERIFIED: the same ten TrackFX_SetOffline calls mint ten undo points
|
||||
// unblocked and ZERO inside this pair. One block per write, not per tick: the
|
||||
// write itself is what pumps, so the block cannot avoid spanning a pump — it can
|
||||
// only span as little of one as possible.
|
||||
// unblocked and ZERO inside this pair. One block per WRITE even though a step is
|
||||
// now one whole track: the write itself is what pumps, so the block cannot avoid
|
||||
// spanning a pump — it can only span as little of one as possible. Widening it to
|
||||
// the track would multiply that exposure by the chain length while suppressing no
|
||||
// extra undo point (the discard form mints none either way), and a user action
|
||||
// landing inside a discarded block can lose its own Ctrl-Z.
|
||||
Undo_BeginBlock2(project);
|
||||
TrackFX_SetOffline(tr, fxIndex, offline);
|
||||
Undo_EndBlock2(project, "", 0);
|
||||
|
||||
+29
-59
@@ -25,15 +25,17 @@ namespace reasampler {
|
||||
inline constexpr double kParkHoldMs = 3000.0;
|
||||
|
||||
// Per-tick budgets, set against the measured per-plugin costs (Phase Plant 36ms
|
||||
// offline / 105ms online; MTurboReverb 42ms / 240ms). A step is one plugin and cannot
|
||||
// be split, so a budget decides only how many plugins a tick moves.
|
||||
// offline / 105ms online; MTurboReverb 42ms / 240ms). A step is one whole track's
|
||||
// chain and cannot be split, so a budget decides only how many TRACKS a tick moves --
|
||||
// and a single track with a long chain overruns it by as much as that chain costs.
|
||||
//
|
||||
// ONLINE 120ms: onlining restores state the user owns and is waiting on, so a tick
|
||||
// buys at least one typical load and two cheap ones. OFFLINE 30ms: under one typical
|
||||
// unload, so opportunistic reclaim moves exactly ONE plugin per tick and yields --
|
||||
// unload, so opportunistic reclaim moves exactly ONE track per tick and yields --
|
||||
// background work must not make the UI feel worse than the switch already did. The
|
||||
// budget is checked BEFORE each step and never mid-step, so a plugin dearer than the
|
||||
// whole budget still runs; that is what makes forward progress unconditional.
|
||||
// budget is checked only AFTER a step, never before the first and never mid-step, so
|
||||
// a track dearer than the whole budget still runs; that is what makes forward
|
||||
// progress unconditional.
|
||||
inline constexpr double kOnlineBudgetMs = 120.0;
|
||||
inline constexpr double kOfflineBudgetMs = 30.0;
|
||||
|
||||
@@ -43,9 +45,9 @@ inline double tickBudgetMs(view::FxWorkKind kind) {
|
||||
|
||||
// --- the queue: a cache of intent, never state --------------------------------
|
||||
|
||||
// One derived work item plus this loop's own completion mark. The work — including its
|
||||
// resume cursor — is core/view/fx_state's; nothing here is persisted, and the whole
|
||||
// queue is discarded and re-derived on any model change.
|
||||
// One derived work item plus this loop's own completion mark. The work is
|
||||
// core/view/fx_state's; nothing here is persisted, and the whole queue is discarded
|
||||
// and re-derived on any model change.
|
||||
struct ReclaimItem {
|
||||
view::FxTrackWork work;
|
||||
bool done = false;
|
||||
@@ -109,31 +111,6 @@ inline int nextEligible(const std::vector<ReclaimItem>& items, const ParkClock&
|
||||
return -1;
|
||||
}
|
||||
|
||||
// --- the resume cursor --------------------------------------------------------
|
||||
|
||||
// A Slot-keyed ONLINE record (lifted from a project saved before FX identity existed)
|
||||
// carries no identity in its targets, so an identity cursor would match nothing there
|
||||
// and restart the item on every yield. Every other shape matches on the live identity,
|
||||
// which is what survives a chain reorder between two ticks.
|
||||
inline bool resumeBySlotOnly(const view::FxTrackWork& work) {
|
||||
return work.kind == view::FxWorkKind::Online && !work.targets.empty() &&
|
||||
work.targets.front().keying == FxKeying::Slot;
|
||||
}
|
||||
|
||||
inline std::string slotIdentity(const std::vector<std::string>& liveFxGuids, int slot) {
|
||||
const auto i = static_cast<std::size_t>(slot);
|
||||
return slot >= 0 && i < liveFxGuids.size() ? liveFxGuids[i] : std::string{};
|
||||
}
|
||||
|
||||
// Records the step just completed. The SLOT is written unconditionally because an FX
|
||||
// REAPER reports no GUID for has nothing else to carry the cursor — see FxTrackWork's
|
||||
// resumeAfterSlot for what the resolve does with the pair.
|
||||
inline void markResumePoint(view::FxTrackWork& work, const std::vector<std::string>& liveFxGuids,
|
||||
int slot) {
|
||||
work.resumeAfterFxGuid = resumeBySlotOnly(work) ? std::string{} : slotIdentity(liveFxGuids, slot);
|
||||
work.resumeAfterSlot = slot;
|
||||
}
|
||||
|
||||
// --- the drain ----------------------------------------------------------------
|
||||
|
||||
// What one tick did. `settled` means every item is complete and the caller may go idle
|
||||
@@ -144,8 +121,8 @@ struct ReclaimTickResult {
|
||||
bool settled = false;
|
||||
};
|
||||
|
||||
// One tick of the convergence loop. Templated on `Host` so the ordering, budget, resume
|
||||
// and release bookkeeping are all exercised without REAPER (tests/test_fx_reclaim.cpp);
|
||||
// One tick of the convergence loop. Templated on `Host` so the ordering, budget and
|
||||
// release bookkeeping are all exercised without REAPER (tests/test_fx_reclaim.cpp);
|
||||
// fx_reclaim.cpp holds the one live implementation. Host must provide:
|
||||
//
|
||||
// double nowMs() monotonic milliseconds
|
||||
@@ -178,33 +155,26 @@ ReclaimTickResult drainTick(std::vector<ReclaimItem>& items, ParkClock& clock, H
|
||||
// apart deliberately, and a tick that started on one must not spend the other's.
|
||||
if (item.work.kind != tickKind) break;
|
||||
|
||||
// A track gone from the project writes nothing and releases nothing — the next
|
||||
// apply's reconcile drops the whole record for a GUID that left it.
|
||||
std::vector<std::string> liveFxGuids;
|
||||
if (!host.liveChain(item.work.trackGuid, liveFxGuids)) {
|
||||
// Track gone. Nothing to write, and no record to release either — the next
|
||||
// apply's reconcile drops the whole record for a GUID that left the project.
|
||||
item.done = true;
|
||||
continue;
|
||||
if (host.liveChain(item.work.trackGuid, liveFxGuids)) {
|
||||
// Held in a named local: through C++20 a range-for does not extend the
|
||||
// lifetime of a temporary reached through a member access, so iterating the
|
||||
// resolution's `.writes` directly off the call would dangle.
|
||||
const FxRestoreResolution resolved = view::resolveFxWork(item.work, liveFxGuids);
|
||||
for (const FxOfflineWrite& w : resolved.writes)
|
||||
if (host.writeOffline(item.work.trackGuid, w.fxIndex, w.offline)) ++result.writes;
|
||||
|
||||
// The record's FX half is released HERE and only here: the chain has now
|
||||
// been driven to every state the record captured, so the record is spent.
|
||||
// Releasing it any earlier destroys the only account of which plugins the
|
||||
// USER had offline before the park, and the next online blanket-enables them.
|
||||
if (item.work.kind == view::FxWorkKind::Online) host.releaseFx(item.work.trackGuid);
|
||||
}
|
||||
item.done = true; // both arms: a step is atomic, so no item is half-moved
|
||||
|
||||
// Held in a named local: through C++20 a range-for does not extend the lifetime
|
||||
// of a temporary reached through a member access, so iterating the resolution's
|
||||
// `.writes` directly off the call would dangle.
|
||||
const FxRestoreResolution resolved = view::resolveFxWork(item.work, liveFxGuids);
|
||||
|
||||
bool yielded = false;
|
||||
for (const FxOfflineWrite& w : resolved.writes) {
|
||||
if (host.nowMs() - start >= budget) { yielded = true; break; }
|
||||
if (host.writeOffline(item.work.trackGuid, w.fxIndex, w.offline)) ++result.writes;
|
||||
markResumePoint(item.work, liveFxGuids, w.fxIndex);
|
||||
}
|
||||
if (yielded) return result; // the resume cursor picks this item up next tick
|
||||
|
||||
item.done = true;
|
||||
// The record's FX half is released HERE and only here: the chain has now been
|
||||
// driven to every state the record captured, so the record is spent. Releasing
|
||||
// it any earlier destroys the only account of which plugins the USER had
|
||||
// offline before the park, and the next online blanket-enables them.
|
||||
if (item.work.kind == view::FxWorkKind::Online) host.releaseFx(item.work.trackGuid);
|
||||
if (host.nowMs() - start >= budget) break; // between tracks only
|
||||
}
|
||||
|
||||
result.settled = queueSettled(items);
|
||||
|
||||
Reference in New Issue
Block a user