Gate the FX reclaimer on the transport, both directions; move the release's persist off the drain and onto the project save
The drain no longer dirties a project on its own initiative — a spent park record rides the user's next save via OnSaveExtensionConfig.
This commit is contained in:
+182
-40
@@ -2,10 +2,11 @@
|
||||
// loop is templated on its host precisely so the properties that make it safe to run in
|
||||
// the background are assertable here: the debounce, the per-tick budget's between-track
|
||||
// yield, the queue being a cache of intent that a model change discards, the user's own
|
||||
// offline plugins surviving an online, the release reaching the persisted model, and —
|
||||
// via a host that invalidates from inside a write — that a re-entered invalidation
|
||||
// reaches nothing the drain is using. The queue owns the project pin and the timer's
|
||||
// wakeup for the same reason: a compare-only pointer makes the tab switch assertable.
|
||||
// offline plugins surviving an online, the transport gate, the release reaching disk on
|
||||
// the user's save rather than on the drain, and — via a host that invalidates from inside
|
||||
// a write — that a re-entered invalidation reaches nothing the drain is using. The queue
|
||||
// owns the project pin and the timer's wakeup for the same reason: a compare-only pointer
|
||||
// makes the tab switch assertable.
|
||||
|
||||
#include "../src/shell/view/fx_reclaim.h"
|
||||
|
||||
@@ -28,6 +29,11 @@ namespace {
|
||||
|
||||
// -- a fake project ------------------------------------------------------------
|
||||
|
||||
// Two project handles. The queue and the flush treat a project pointer as COMPARE-ONLY
|
||||
// and never dereference it, so a fabricated address is a faithful stand-in for REAPER's.
|
||||
ReaProject* const kProjA = reinterpret_cast<ReaProject*>(static_cast<std::uintptr_t>(0x1));
|
||||
ReaProject* const kProjB = reinterpret_cast<ReaProject*>(static_cast<std::uintptr_t>(0x2));
|
||||
|
||||
// One live FX: its identity (empty when REAPER reports none) and whether it is offline.
|
||||
struct FakeFx {
|
||||
std::string guid;
|
||||
@@ -51,7 +57,10 @@ struct FakeHost {
|
||||
int invalidateAfterWrite = -1;
|
||||
int itemsSeenMidDrain = -1; // queue size as the re-entrant caller finds it
|
||||
|
||||
bool rolling = false; // the transport gate's input
|
||||
|
||||
double nowMs() const { return clock; }
|
||||
bool transportRolling() const { return rolling; }
|
||||
|
||||
bool liveChain(const std::string& guid, std::vector<std::string>& out) {
|
||||
auto it = chains.find(guid);
|
||||
@@ -80,21 +89,18 @@ struct FakeHost {
|
||||
}
|
||||
|
||||
// `model` is the live host's session model and `persisted` stands in for view_state
|
||||
// on disk — the blob a reopen or an undo/redo loadFromProject rebuilds from. Set them
|
||||
// and the release actually lands, so a persist ordered BEFORE it would serialize a
|
||||
// record that is still held and fail the reload assertions.
|
||||
ViewModeModel* model = nullptr;
|
||||
// on disk — the blob a reopen or an undo/redo loadFromProject rebuilds from. The
|
||||
// drain writes the model and NEVER `persisted`: only saveProject() below does, which
|
||||
// is what makes "the spend rides the user's save" assertable.
|
||||
ViewModeModel* model = nullptr;
|
||||
ReleaseFlush* flush = nullptr;
|
||||
ReaProject* project = kProjA;
|
||||
std::string persisted;
|
||||
int persists = 0;
|
||||
|
||||
void releaseFx(const std::string& guid) {
|
||||
released.insert(guid);
|
||||
if (model) model->releaseSnapshotFx(guid);
|
||||
}
|
||||
|
||||
void persistModel() {
|
||||
++persists;
|
||||
if (model) persisted = model->serialize();
|
||||
if (flush) flush->arm(project);
|
||||
}
|
||||
|
||||
bool offlineAt(const std::string& guid, std::size_t slot) const {
|
||||
@@ -103,6 +109,14 @@ struct FakeHost {
|
||||
}
|
||||
};
|
||||
|
||||
// What the project-save hook does (fxReclaimFlushRelease): write the in-memory model into
|
||||
// this project's view state, but only when it is the project holding the release.
|
||||
void saveProject(FakeHost& host, ReleaseFlush& flush, ReaProject* active) {
|
||||
if (!flush.pendingFor(active)) return;
|
||||
host.persisted = host.model->serialize();
|
||||
flush.clear();
|
||||
}
|
||||
|
||||
// -- model helpers -------------------------------------------------------------
|
||||
|
||||
TrackSnapshot recordWithFx(std::vector<FxOfflineState> fx) {
|
||||
@@ -178,11 +192,6 @@ FxReclaimQueue queueOf(const ViewModeModel& vm, const std::set<std::string>& liv
|
||||
return q;
|
||||
}
|
||||
|
||||
// Two project handles. The queue treats a project pointer as COMPARE-ONLY and never
|
||||
// dereferences it, so a fabricated address is a faithful stand-in for REAPER's.
|
||||
ReaProject* const kProjA = reinterpret_cast<ReaProject*>(static_cast<std::uintptr_t>(0x1));
|
||||
ReaProject* const kProjB = reinterpret_cast<ReaProject*>(static_cast<std::uintptr_t>(0x2));
|
||||
|
||||
} // namespace
|
||||
|
||||
// -- 1. debounce ---------------------------------------------------------------
|
||||
@@ -505,6 +514,8 @@ static void testAnInvalidationDuringAnOnlineDropsThatTicksRelease() {
|
||||
vm.releaseSnapshotFlags("{T}");
|
||||
|
||||
FakeHost host;
|
||||
ReleaseFlush flush;
|
||||
host.flush = &flush;
|
||||
host.chains["{T}"] = offlineChain({"{FX-A}", "{FX-B}"});
|
||||
|
||||
FxReclaimQueue q = queueOf(vm, {"{T}"});
|
||||
@@ -514,9 +525,9 @@ static void testAnInvalidationDuringAnOnlineDropsThatTicksRelease() {
|
||||
|
||||
const ReclaimRunResult r = q.run(host);
|
||||
CHECK(r.aborted);
|
||||
CHECK(r.writes == 2); // the item still completed — a step is atomic
|
||||
CHECK(host.released.empty()); // but its record was NOT spent
|
||||
CHECK(host.persists == 0); // and nothing durable claims it was
|
||||
CHECK(r.writes == 2); // the item still completed — a step is atomic
|
||||
CHECK(host.released.empty()); // but its record was NOT spent
|
||||
CHECK(!flush.pendingFor(kProjA)); // and no save is owed one that never happened
|
||||
|
||||
// Re-owed, not lost: the re-derived item finds the chain already where the record
|
||||
// wants it, writes nothing more, and releases then.
|
||||
@@ -525,45 +536,97 @@ static void testAnInvalidationDuringAnOnlineDropsThatTicksRelease() {
|
||||
CHECK(q.run(host).settled);
|
||||
CHECK(host.writeLog.size() == 2);
|
||||
CHECK(host.released.count("{T}") == 1);
|
||||
CHECK(host.persists == 1);
|
||||
CHECK(flush.pendingFor(kProjA));
|
||||
}
|
||||
|
||||
// -- 6. the release has to outlive the session ------------------------------------
|
||||
|
||||
static void testACompletedOnlineIsPersistedSoAReloadPlansNoWork() {
|
||||
static void testACompletedOnlineReachesDiskOnTheSaveAndNotOnTheDrain() {
|
||||
ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
|
||||
vm.setActiveMode(kDesignModeId);
|
||||
vm.releaseSnapshotFlags("{T}");
|
||||
|
||||
// The bug this closes, stated as the baseline: the model as it stands BEFORE the
|
||||
// The bug the flush closes, stated as the baseline: the model as it stands BEFORE the
|
||||
// release re-derives the same Online work when reloaded, and re-driving it would
|
||||
// revert whatever per-FX offline the user set after the reclaim had finished.
|
||||
const std::optional<ViewModeModel> stale = ViewModeModel::deserialize(vm.serialize());
|
||||
CHECK(stale.has_value());
|
||||
CHECK(derive(*stale, {"{T}"}).size() == 1);
|
||||
|
||||
FakeHost host;
|
||||
host.model = &vm;
|
||||
host.persisted = vm.serialize(); // the project as it sits on disk, record unspent
|
||||
host.chains["{T}"] = offlineChain({"{FX-A}", "{FX-B}"});
|
||||
ReleaseFlush flush;
|
||||
host.flush = &flush;
|
||||
|
||||
FxReclaimQueue q = queueOf(vm, {"{T}"});
|
||||
drainToSettled(q, host);
|
||||
CHECK(host.released.count("{T}") == 1);
|
||||
CHECK(host.persists == 1); // once per tick that released, not once per release
|
||||
|
||||
const std::optional<ViewModeModel> reloaded = ViewModeModel::deserialize(host.persisted);
|
||||
CHECK(reloaded.has_value());
|
||||
CHECK(reloaded->snapshot("{T}") == nullptr); // both halves spent ⇒ record erased
|
||||
CHECK(derive(*reloaded, {"{T}"}).empty());
|
||||
// The drain wrote NOTHING durable. That is the whole change: a background tick that
|
||||
// persisted would also have dirtied the project, asking the user to save one they
|
||||
// had only opened.
|
||||
const std::optional<ViewModeModel> beforeSave = ViewModeModel::deserialize(host.persisted);
|
||||
CHECK(beforeSave.has_value());
|
||||
CHECK(beforeSave->snapshot("{T}") != nullptr);
|
||||
CHECK(derive(*beforeSave, {"{T}"}).size() == 1); // still stale on disk
|
||||
CHECK(flush.pendingFor(kProjA)); // ...and the save knows it is owed
|
||||
|
||||
// The user's next save is what spends it — and afterwards a reload plans no work,
|
||||
// so nothing re-drives the chain.
|
||||
saveProject(host, flush, kProjA);
|
||||
CHECK(!flush.pendingFor(kProjA));
|
||||
const std::optional<ViewModeModel> afterSave = ViewModeModel::deserialize(host.persisted);
|
||||
CHECK(afterSave.has_value());
|
||||
CHECK(afterSave->snapshot("{T}") == nullptr); // both halves spent ⇒ record erased
|
||||
CHECK(derive(*afterSave, {"{T}"}).empty());
|
||||
}
|
||||
|
||||
static void testATickThatSpendsNoRecordPersistsNothing() {
|
||||
// OFFLINE work spends no record, so there is nothing to make durable — and a
|
||||
// background reclaim must not dirty the project for work it can simply redo.
|
||||
static void testAReleaseKilledBeforeTheFlushIsSimplyReEarned() {
|
||||
// The exposure the save-time flush opens: between the in-memory release and the next
|
||||
// save, the record on disk is still unspent. Quitting there must cost a redo of the
|
||||
// reclaim, never a repair.
|
||||
ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
|
||||
vm.setActiveMode(kDesignModeId);
|
||||
vm.releaseSnapshotFlags("{T}");
|
||||
|
||||
FakeHost host;
|
||||
host.model = &vm;
|
||||
host.persisted = vm.serialize();
|
||||
host.chains["{T}"] = offlineChain({"{FX-A}", "{FX-B}"});
|
||||
ReleaseFlush flush;
|
||||
host.flush = &flush;
|
||||
|
||||
FxReclaimQueue q = queueOf(vm, {"{T}"});
|
||||
drainToSettled(q, host);
|
||||
CHECK(host.writeLog.size() == 2); // the chain really was driven online
|
||||
CHECK(flush.pendingFor(kProjA));
|
||||
|
||||
// Killed here: no save, so the model in memory goes and the disk record survives.
|
||||
const std::optional<ViewModeModel> reopened = ViewModeModel::deserialize(host.persisted);
|
||||
CHECK(reopened.has_value());
|
||||
ViewModeModel reloaded = *reopened;
|
||||
|
||||
FakeHost after;
|
||||
after.chains = host.chains; // the DAW state outlives the reload: the chain is online
|
||||
after.model = &reloaded;
|
||||
ReleaseFlush flushAfter;
|
||||
after.flush = &flushAfter;
|
||||
|
||||
FxReclaimQueue q2 = queueOf(reloaded, {"{T}"});
|
||||
CHECK(q2.items().size() == 1); // the surviving record re-derives the same Online work
|
||||
CHECK(drainToSettled(q2, after) == 1);
|
||||
CHECK(after.writeLog.empty()); // compare-before-write: nothing left to move
|
||||
CHECK(after.released.count("{T}") == 1);
|
||||
CHECK(flushAfter.pendingFor(kProjA)); // and the spend is owed to the next save again
|
||||
}
|
||||
|
||||
static void testATickThatSpendsNoRecordOwesNoSaveAnything() {
|
||||
// OFFLINE work spends no record, so there is nothing to make durable — and the
|
||||
// reclaimer must never give the user's next save work it did not earn.
|
||||
ViewModeModel vm = parkedDesignLeaf({"{FX-A}"});
|
||||
FakeHost host;
|
||||
host.model = &vm;
|
||||
host.chains["{T}"] = onlineChain({"{FX-A}"});
|
||||
ReleaseFlush flush;
|
||||
host.flush = &flush;
|
||||
|
||||
FxReclaimQueue q = queueOf(vm, {"{T}"});
|
||||
q.run(host); // observes the hold at t=0
|
||||
@@ -572,7 +635,8 @@ static void testATickThatSpendsNoRecordPersistsNothing() {
|
||||
|
||||
CHECK(host.writeLog.size() == 1); // it did move the plugin
|
||||
CHECK(host.released.empty());
|
||||
CHECK(host.persists == 0);
|
||||
CHECK(!flush.pendingFor(kProjA));
|
||||
saveProject(host, flush, kProjA);
|
||||
CHECK(host.persisted.empty());
|
||||
}
|
||||
|
||||
@@ -772,6 +836,79 @@ static void testATickThatLostNothingNeitherPrintsNorDisturbsTheDropMemo() {
|
||||
CHECK(!shouldReportDrops(/*sameOwnerAsLast=*/true, lost, lost));
|
||||
}
|
||||
|
||||
// -- 9. the transport gate ------------------------------------------------------------
|
||||
|
||||
static void testARollingTransportMovesNoPluginsAndKeepsTheHold() {
|
||||
ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
|
||||
FakeHost host;
|
||||
host.chains["{T}"] = onlineChain({"{FX-A}", "{FX-B}"});
|
||||
|
||||
FxReclaimQueue q = queueOf(vm, {"{T}"});
|
||||
CHECK(!q.run(host).settled); // the hold starts at t=0
|
||||
|
||||
host.rolling = true;
|
||||
host.clock += kParkHoldMs; // matured — the debounce is no longer what holds it back
|
||||
const ReclaimRunResult gated = q.run(host);
|
||||
CHECK(gated.writes == 0);
|
||||
CHECK(host.writeLog.empty());
|
||||
CHECK(!gated.aborted);
|
||||
CHECK(!gated.settled); // the work is still owed, so the shell must not go idle
|
||||
CHECK(!q.stale()); // and the queue it will resume from survives untouched
|
||||
|
||||
// The hold kept its origin across the gated tick: the drain fires on the FIRST tick
|
||||
// after the transport stops, with no fresh 3s wait.
|
||||
host.rolling = false;
|
||||
CHECK(q.run(host).settled);
|
||||
CHECK(host.writeLog.size() == 2);
|
||||
}
|
||||
|
||||
static void testTheGateStopsTheOnlineDirectionToo() {
|
||||
// The ruling is symmetric because both directions load or unload plugins: onlining
|
||||
// re-instantiates the very chain the user is listening through.
|
||||
ViewModeModel vm = parkedDesignLeaf({"{FX-A}", "{FX-B}"});
|
||||
vm.setActiveMode(kDesignModeId);
|
||||
vm.releaseSnapshotFlags("{T}");
|
||||
|
||||
FakeHost host;
|
||||
host.model = &vm;
|
||||
host.chains["{T}"] = offlineChain({"{FX-A}", "{FX-B}"});
|
||||
ReleaseFlush flush;
|
||||
host.flush = &flush;
|
||||
host.rolling = true;
|
||||
|
||||
FxReclaimQueue q = queueOf(vm, {"{T}"});
|
||||
CHECK(q.items().size() == 1 && q.items()[0].work.kind == FxWorkKind::Online);
|
||||
const ReclaimRunResult gated = q.run(host);
|
||||
CHECK(gated.writes == 0);
|
||||
CHECK(!gated.settled);
|
||||
CHECK(host.released.empty()); // no record spent while the transport rolls
|
||||
CHECK(!flush.pendingFor(kProjA));
|
||||
CHECK(host.offlineAt("{T}", 0)); // the chain is untouched, still parked
|
||||
|
||||
// The contrast, so the assertions above are about the gate and not about an online
|
||||
// queue that cannot drain: stop the transport and the same tick completes.
|
||||
host.rolling = false;
|
||||
CHECK(q.run(host).settled);
|
||||
CHECK(host.writeLog.size() == 2);
|
||||
CHECK(host.released.count("{T}") == 1);
|
||||
}
|
||||
|
||||
static void testAGatedTickStillGoesIdleWithNothingLeftToConverge() {
|
||||
// A settled queue must not spin the timer for the length of a playback pass: the
|
||||
// gate's answer is "no writes", not "never finished".
|
||||
ViewModeModel vm = parkedDesignLeaf({"{FX-A}"});
|
||||
FakeHost host;
|
||||
host.chains["{T}"] = onlineChain({"{FX-A}"});
|
||||
|
||||
FxReclaimQueue q = queueOf(vm, {"{T}"});
|
||||
q.run(host);
|
||||
host.clock += kParkHoldMs;
|
||||
CHECK(q.run(host).settled);
|
||||
|
||||
host.rolling = true;
|
||||
CHECK(q.run(host).settled);
|
||||
}
|
||||
|
||||
int main() {
|
||||
testFastFlipMovesZeroPluginsButAHeldParkStillReclaims();
|
||||
|
||||
@@ -790,8 +927,9 @@ int main() {
|
||||
testAnInvalidationDuringTheLastItemLeavesTheTickUnsettled();
|
||||
testAnInvalidationDuringAnOnlineDropsThatTicksRelease();
|
||||
|
||||
testACompletedOnlineIsPersistedSoAReloadPlansNoWork();
|
||||
testATickThatSpendsNoRecordPersistsNothing();
|
||||
testACompletedOnlineReachesDiskOnTheSaveAndNotOnTheDrain();
|
||||
testAReleaseKilledBeforeTheFlushIsSimplyReEarned();
|
||||
testATickThatSpendsNoRecordOwesNoSaveAnything();
|
||||
|
||||
testADropIsReportedOnceAcrossRepeatedRederivations();
|
||||
testAnOfflineTickReportsNoDrops();
|
||||
@@ -801,6 +939,10 @@ int main() {
|
||||
testOnlyANoProjectTickClearsTheWakeup();
|
||||
testATickThatLostNothingNeitherPrintsNorDisturbsTheDropMemo();
|
||||
|
||||
testARollingTransportMovesNoPluginsAndKeepsTheHold();
|
||||
testTheGateStopsTheOnlineDirectionToo();
|
||||
testAGatedTickStillGoesIdleWithNothingLeftToConverge();
|
||||
|
||||
if (g_fail == 0) std::printf("All tests passed.\n");
|
||||
return g_fail ? 1 : 0;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user