diff --git a/src/core/view/fx_state.cpp b/src/core/view/fx_state.cpp index 00e4073..e28b202 100644 --- a/src/core/view/fx_state.cpp +++ b/src/core/view/fx_state.cpp @@ -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 remainingAfter(const std::vector& 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(ops.begin() + static_cast(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 offlineTargets(const std::string& trackGuid, const std::vector& liveFxGuids) { std::vector ops; @@ -70,14 +47,13 @@ std::vector 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 desiredFxState(const ViewModeModel& model, FxRestoreResolution resolveFxWork(const FxTrackWork& work, const std::vector& 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 diff --git a/src/core/view/fx_state.h b/src/core/view/fx_state.h index c74e242..b804ef1 100644 --- a/src/core/view/fx_state.h +++ b/src/core/view/fx_state.h @@ -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 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 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& liveFxGuids); diff --git a/src/shell/view/fx_reclaim.cpp b/src/shell/view/fx_reclaim.cpp index 1563118..3360ae3 100644 --- a/src/shell/view/fx_reclaim.cpp +++ b/src/shell/view/fx_reclaim.cpp @@ -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); diff --git a/src/shell/view/fx_reclaim.h b/src/shell/view/fx_reclaim.h index 92dc8e2..2e5cfa3 100644 --- a/src/shell/view/fx_reclaim.h +++ b/src/shell/view/fx_reclaim.h @@ -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& 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& liveFxGuids, int slot) { - const auto i = static_cast(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& 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& 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 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); diff --git a/tests/test_fx_reclaim.cpp b/tests/test_fx_reclaim.cpp index bb91e62..6577d02 100644 --- a/tests/test_fx_reclaim.cpp +++ b/tests/test_fx_reclaim.cpp @@ -1,8 +1,8 @@ // Standalone tests for the FX reclaimer's drain — no REAPER, no test framework. The // loop is templated on its host precisely so the four properties that make it safe to // run in the background are assertable here: the debounce, the per-tick budget's -// yield-and-resume, the queue being a cache of intent that a model change discards, and -// the user's own offline plugins surviving an online. +// between-track yield, the queue being a cache of intent that a model change discards, +// and the user's own offline plugins surviving an online. #include "../src/shell/view/fx_reclaim.h" @@ -95,16 +95,23 @@ std::vector offlineChain(const std::vector& guids) { return chain; } -// A Design leaf the tool has parked: record held, both halves, nothing user-offlined. -ViewModeModel parkedDesignLeaf(const std::vector& fxGuids) { +// Design leaves the tool has parked: record held, both halves, nothing user-offlined. +// Active mode stays Arrange, so every one of them is parked. +ViewModeModel parkedDesignLeaves(const std::map>& tracks) { ViewModeModel vm; - vm.membership().tag("{T}", kDesignModeId); // active mode is Arrange ⇒ {T} is parked - std::vector fx; - for (const std::string& g : fxGuids) fx.push_back(FxOfflineState{g, 0}); - vm.storeSnapshot("{T}", recordWithFx(fx)); + for (const auto& [guid, fxGuids] : tracks) { + vm.membership().tag(guid, kDesignModeId); + std::vector fx; + for (const std::string& g : fxGuids) fx.push_back(FxOfflineState{g, 0}); + vm.storeSnapshot(guid, recordWithFx(fx)); + } return vm; } +ViewModeModel parkedDesignLeaf(const std::vector& fxGuids) { + return parkedDesignLeaves({{"{T}", fxGuids}}); +} + // Puts the queue's offline items past their debounce the way the live loop does: the // hold starts at the FIRST observation, so advancing the clock before that one just // moves the origin and matures nothing. @@ -168,48 +175,72 @@ static void testFastFlipMovesZeroPluginsButAHeldParkStillReclaims() { CHECK(host2.released.empty()); // an OFFLINE never releases — the record is still owed } -// -- 2. budget: yield and resume ------------------------------------------------ +// -- 2. budget: the step is a whole track ---------------------------------------- -static void testOfflineDrainYieldsOnBudgetAndResumesWhereItStopped() { - ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}", "{FX-C}", "{FX-D}"}); +static void testAWholeChainMovesInOneStepAndTheYieldFallsBetweenTracks() { + // {T1}'s chain costs 4x the offline budget on its own. The budget cannot split it — + // a step is one track — so the tick overruns to finish it and yields only once the + // NEXT track would start. Slot 1 carries no identity, which the drain no longer has + // to reason about at all now that nothing resumes mid-chain. + ViewModeModel vm = parkedDesignLeaves({{"{T1}", {"{FX-A}", "", "{FX-C}", "{FX-D}"}}, + {"{T2}", {"{FX-E}"}}, + {"{T3}", {"{FX-F}"}}}); FakeHost host; - host.chains["{T}"] = onlineChain({"{FX-A}", "{FX-B}", "{FX-C}", "{FX-D}"}); - host.stepCostMs = 50.0; // dearer than kOfflineBudgetMs ⇒ exactly one plugin per tick + host.chains["{T1}"] = onlineChain({"{FX-A}", "", "{FX-C}", "{FX-D}"}); + host.chains["{T2}"] = onlineChain({"{FX-E}"}); + host.chains["{T3}"] = onlineChain({"{FX-F}"}); + host.stepCostMs = 50.0; // dearer than kOfflineBudgetMs ⇒ one TRACK per tick ParkClock clock; - std::vector items = derive(vm, {"{T}"}); + std::vector items = derive(vm, {"{T1}", "{T2}", "{T3}"}); + CHECK(items.size() == 3); matureHold(items, clock, host); - CHECK(drainTick(items, clock, host).writes == 1); - CHECK(items[0].work.resumeAfterSlot == 0); - CHECK(items[0].work.resumeAfterFxGuid == "{FX-A}"); - CHECK(!items[0].done); - CHECK(drainTick(items, clock, host).writes == 1); - CHECK(items[0].work.resumeAfterSlot == 1); + CHECK(drainTick(items, clock, host).writes == 4); // all of {T1}, 200ms over a 30ms budget + CHECK(items[0].done && !items[1].done && !items[2].done); + const std::vector firstTick{"{T1}:0=off", "{T1}:1=off", "{T1}:2=off", + "{T1}:3=off"}; + CHECK(host.writeLog == firstTick); - // Slots 2 and 3, one per tick; the tick that writes the last one also exhausts the - // item's write list, so it settles in the same pass rather than costing a spare tick. + // {T2} then {T3}, one track per tick; the tick that finishes the last one exhausts + // the queue in the same pass rather than costing a spare tick. CHECK(drainToSettled(items, clock, host) == 2); - - // Each slot moved exactly once and in order — no slot re-walked across a yield. - const std::vector expected{"{T}:0=off", "{T}:1=off", "{T}:2=off", "{T}:3=off"}; + std::vector expected = firstTick; + expected.push_back("{T2}:0=off"); + expected.push_back("{T3}:0=off"); CHECK(host.writeLog == expected); } static void testOnlineDrainGetsTheLargerBudget() { - // Same per-plugin cost, same chain length: the only difference is the direction, and + // Same per-plugin cost, same track count, one plugin each so neither direction is + // distorted by an overrunning chain: the only difference is the direction, and // onlining is the one the user is waiting on. - ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}", "{FX-C}", "{FX-D}"}); - vm.setActiveMode(kDesignModeId); - vm.releaseSnapshotFlags("{T}"); - FakeHost host; - host.chains["{T}"] = offlineChain({"{FX-A}", "{FX-B}", "{FX-C}", "{FX-D}"}); - host.stepCostMs = 50.0; - ParkClock clock; + const std::map> tracks{ + {"{T1}", {"{FX-1}"}}, {"{T2}", {"{FX-2}"}}, {"{T3}", {"{FX-3}"}}, {"{T4}", {"{FX-4}"}}}; - std::vector items = derive(vm, {"{T}"}); - CHECK(items.size() == 1 && items[0].work.kind == FxWorkKind::Online); - CHECK(drainTick(items, clock, host).writes == 3); // 120ms budget vs 30ms + ViewModeModel back = parkedDesignLeaves(tracks); + back.setActiveMode(kDesignModeId); + FakeHost onlineHost; + for (const auto& [guid, fx] : tracks) { + back.releaseSnapshotFlags(guid); + onlineHost.chains[guid] = offlineChain(fx); + } + onlineHost.stepCostMs = 50.0; + ParkClock onlineClock; + + std::vector onlineItems = derive(back, {"{T1}", "{T2}", "{T3}", "{T4}"}); + CHECK(onlineItems.size() == 4 && onlineItems[0].work.kind == FxWorkKind::Online); + CHECK(drainTick(onlineItems, onlineClock, onlineHost).writes == 3); // 120ms buys three + + ViewModeModel parked = parkedDesignLeaves(tracks); + FakeHost offlineHost; + for (const auto& [guid, fx] : tracks) offlineHost.chains[guid] = onlineChain(fx); + offlineHost.stepCostMs = 50.0; + ParkClock offlineClock; + + std::vector offlineItems = derive(parked, {"{T1}", "{T2}", "{T3}", "{T4}"}); + matureHold(offlineItems, offlineClock, offlineHost); + CHECK(drainTick(offlineItems, offlineClock, offlineHost).writes == 1); // 30ms buys one } static void testATickStartedOnOnlineWorkDoesNotSpendIntoOfflineWork() { @@ -254,21 +285,19 @@ static void testModelChangeDiscardsDrainProgressButNotTheParkHold() { std::vector items = derive(vm, {"{T}", "{U}"}); drainTick(items, clock, host); // observes both parked at t=0 host.clock = kParkHoldMs + 100.0; - drainTick(items, clock, host); // one plugin moves; {T} now carries a cursor - CHECK(items[0].work.resumeAfterSlot >= 0); + drainTick(items, clock, host); // {T}'s whole chain moves, then the tick yields + CHECK(items[0].done && !items[1].done); // The model changes: {T} is retagged into the active mode and restored, while {U} // stays exactly as parked as it was. The queue is discarded whole — direction, - // cursors, completion marks and all — and re-derived. + // completion marks and all — and re-derived. vm.membership().tag("{T}", kArrangeModeId); vm.releaseSnapshotFlags("{T}"); items = derive(vm, {"{T}", "{U}"}); CHECK(items.size() == 2); CHECK(items[0].work.kind == FxWorkKind::Online); CHECK(items[0].work.trackGuid == "{T}"); - CHECK(items[0].work.resumeAfterSlot == -1); // no drain progress survives a re-derive - CHECK(items[0].work.resumeAfterFxGuid.empty()); - CHECK(!items[0].done); + CHECK(!items[0].done && !items[1].done); // no drain progress survives a re-derive // ...but {U}'s hold does survive it. {U} never stopped being parked, and restarting // its 3s on every unrelated model change would defer its reclaim indefinitely. @@ -313,34 +342,11 @@ static void testAPluginTheUserOfflinedIsNeverOnlinedByTheDrain() { CHECK(host.released.count("{T}") == 1); } -// -- forward progress: the cursor cases that decide termination ----------------- +// -- forward progress ----------------------------------------------------------- -static void testResumeCursorAdvancesPastAnFxWithNoIdentity() { - // The case Wave 1's slot cursor exists for. Slot 1 is an FX REAPER reports no GUID - // for; with the identity alone the cursor would read as "nothing done yet" and the - // item would re-walk from slot 0 on every tick. - ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}", "{FX-C}"}); - FakeHost host; - host.chains["{T}"] = onlineChain({"{FX-A}", "", "{FX-C}"}); - host.stepCostMs = 50.0; // one plugin per tick on the offline budget - ParkClock clock; - - std::vector items = derive(vm, {"{T}"}); - matureHold(items, clock, host); - drainTick(items, clock, host); - drainTick(items, clock, host); - CHECK(items[0].work.resumeAfterFxGuid.empty()); // slot 1 has no identity to record - CHECK(items[0].work.resumeAfterSlot == 1); // ...so the slot is what carries it - - CHECK(drainToSettled(items, clock, host) == 1); // slot 2, and the item is exhausted - const std::vector expected{"{T}:0=off", "{T}:1=off", "{T}:2=off"}; - CHECK(host.writeLog == expected); -} - -static void testSlotKeyedOnlineRecordCarriesItsCursorAsASlot() { +static void testSlotKeyedOnlineRecordRestoresByPosition() { // A record lifted from a project saved before FX identity existed: its targets hold - // no identity, so a cursor taken from the LIVE identity would match none of them and - // the item would re-walk its whole chain after every yield. + // no identity at all, so position is the only thing that can address them. ViewModeModel vm; vm.membership().tag("{T}", kDesignModeId); vm.setActiveMode(kDesignModeId); @@ -351,16 +357,14 @@ static void testSlotKeyedOnlineRecordCarriesItsCursorAsASlot() { FakeHost host; host.chains["{T}"] = offlineChain({"{FX-X}", "{FX-Y}", "{FX-Z}"}); - host.stepCostMs = kOnlineBudgetMs + 10.0; // one plugin per tick, then yield + // Dearer per plugin than the whole online budget: the step still runs the chain to + // the end, because the budget cannot split one track's move. + host.stepCostMs = kOnlineBudgetMs + 10.0; ParkClock clock; std::vector items = derive(vm, {"{T}"}); - CHECK(items.size() == 1 && resumeBySlotOnly(items[0].work)); - drainTick(items, clock, host); - CHECK(items[0].work.resumeAfterFxGuid.empty()); // NOT "{FX-X}", which matches no target - CHECK(items[0].work.resumeAfterSlot == 0); - - drainToSettled(items, clock, host); + CHECK(items.size() == 1); + CHECK(drainToSettled(items, clock, host) == 1); const std::vector expected{"{T}:0=on", "{T}:2=on"}; CHECK(host.writeLog == expected); // slot 1 stays offline, as the record captured CHECK(host.offlineAt("{T}", 1)); @@ -381,7 +385,7 @@ static void testATrackThatLeftTheProjectCompletesRatherThanStalls() { int main() { testFastFlipMovesZeroPluginsButAHeldParkStillReclaims(); - testOfflineDrainYieldsOnBudgetAndResumesWhereItStopped(); + testAWholeChainMovesInOneStepAndTheYieldFallsBetweenTracks(); testOnlineDrainGetsTheLargerBudget(); testATickStartedOnOnlineWorkDoesNotSpendIntoOfflineWork(); @@ -389,8 +393,7 @@ int main() { testAPluginTheUserOfflinedIsNeverOnlinedByTheDrain(); - testResumeCursorAdvancesPastAnFxWithNoIdentity(); - testSlotKeyedOnlineRecordCarriesItsCursorAsASlot(); + testSlotKeyedOnlineRecordRestoresByPosition(); testATrackThatLeftTheProjectCompletesRatherThanStalls(); if (g_fail == 0) std::printf("All tests passed.\n"); diff --git a/tests/test_fx_state.cpp b/tests/test_fx_state.cpp index c20dadc..0ce5b1b 100644 --- a/tests/test_fx_state.cpp +++ b/tests/test_fx_state.cpp @@ -240,74 +240,6 @@ static void testSlotKeyedRecordStillRestoresByPosition() { CHECK(writeAt(res, 2, false)); } -// -- the resume point --------------------------------------------------------- - -static void testResumePointSkipsWhatTheDrainAlreadyDid() { - ViewModeModel vm = parkedWithThreeFx(); - CHECK(vm.setActiveMode(kDesignModeId)); - std::vector work = desiredFxState(vm, kDesignModeId, {"{T}"}); - CHECK(work.size() == 1); - if (work.empty()) return; - - work[0].resumeAfterFxGuid = "{FX-A}"; - const FxRestoreResolution res = resolveFxWork(work[0], {"{FX-A}", "{FX-B}", "{FX-C}"}); - CHECK(res.writes.size() == 2); - CHECK(!anyWriteAt(res, 0)); // A is done - CHECK(writeAt(res, 1, true)); - CHECK(writeAt(res, 2, false)); - - // Resuming after the LAST identity leaves nothing owed — the item is finished. - work[0].resumeAfterFxGuid = "{FX-C}"; - CHECK(resolveFxWork(work[0], {"{FX-A}", "{FX-B}", "{FX-C}"}).writes.empty()); -} - -static void testResumePointThatLeftTheChainRestartsRatherThanStrands() { - ViewModeModel vm = parkedWithThreeFx(); - CHECK(vm.setActiveMode(kDesignModeId)); - std::vector work = desiredFxState(vm, kDesignModeId, {"{T}"}); - CHECK(work.size() == 1); - if (work.empty()) return; - - // The drain's cursor named a plugin the user has since deleted. Every write is - // compare-before-write, so re-walking the item is cheap and correct; honoring a - // cursor that matches nothing would strand the rest of the chain. - work[0].resumeAfterFxGuid = "{FX-GONE}"; - const FxRestoreResolution res = resolveFxWork(work[0], {"{FX-A}", "{FX-B}", "{FX-C}"}); - CHECK(res.writes.size() == 3); - CHECK(writeAt(res, 1, true)); - - // Same rule on the offline side, where the cursor keys into the live chain. - ViewModeModel parked = parkedWithThreeFx(); // Arrange active ⇒ {T} parks - std::vector offlineWork = desiredFxState(parked, kArrangeModeId, {"{T}"}); - CHECK(offlineWork.size() == 1); - if (offlineWork.empty()) return; - offlineWork[0].resumeAfterFxGuid = "{FX-A}"; - CHECK(resolveFxWork(offlineWork[0], {"{FX-A}", "{FX-B}", "{FX-C}"}).writes.size() == 2); - offlineWork[0].resumeAfterFxGuid = "{FX-GONE}"; - CHECK(resolveFxWork(offlineWork[0], {"{FX-A}", "{FX-B}", "{FX-C}"}).writes.size() == 3); -} - -static void testResumeAfterUnidentifiedFxMakesForwardProgress() { - // The drain completed slot 1, an FX REAPER reports no GUID for. Before the - // slot-cursor fix, an empty resumeAfterFxGuid was indistinguishable from - // "nothing done yet" and this item would restart at slot 0 every tick, - // forever — the non-terminating drain this test pins against. - ViewModeModel parked = parkedWithThreeFx(); // Arrange active ⇒ {T} parks (Offline, slot-keyed) - std::vector work = desiredFxState(parked, kArrangeModeId, {"{T}"}); - CHECK(work.size() == 1); - if (work.empty()) return; - - work[0].resumeAfterFxGuid = ""; - work[0].resumeAfterSlot = 1; - const FxRestoreResolution res = - resolveFxWork(work[0], {"{FX-A}", "", "{FX-B}", "{FX-C}"}); - CHECK(res.writes.size() == 2); // slots 2 and 3 only — 0 and 1 already done - CHECK(!anyWriteAt(res, 0)); - CHECK(!anyWriteAt(res, 1)); - CHECK(anyWriteAt(res, 2)); - CHECK(anyWriteAt(res, 3)); -} - int main() { testActiveTrackIsOwedOnlineInactiveIsOwedOffline(); testOnlineWorkSortsBeforeOfflineWork(); @@ -321,10 +253,6 @@ int main() { testAddedFxIsLeftAlone(); testSlotKeyedRecordStillRestoresByPosition(); - testResumePointSkipsWhatTheDrainAlreadyDid(); - testResumePointThatLeftTheChainRestartsRatherThanStrands(); - testResumeAfterUnidentifiedFxMakesForwardProgress(); - if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; }