fix(view): snapshot-once park guard + correct per-mode undo label
Only capture a track's pre-park snapshot when none exists, so a re-park while already parked no longer overwrites it with hidden-state zeros (leaves vanished after toggling twice). Undo label now names the actual target mode.
This commit is contained in:
+19
-2
@@ -158,7 +158,15 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
|
|||||||
MediaTrack* tr = resolve(handleByGuid, guid);
|
MediaTrack* tr = resolve(handleByGuid, guid);
|
||||||
if (!tr) continue; // stale GUID — prune
|
if (!tr) continue; // stale GUID — prune
|
||||||
|
|
||||||
model.storeSnapshot(guid, snapshotTrack(tr));
|
// Snapshot ONCE, at the first park. If a snapshot already exists the track is
|
||||||
|
// still parked from a prior apply, and its live flags are the PARKED (hidden)
|
||||||
|
// values — recapturing here would overwrite the true pre-park state with zeros,
|
||||||
|
// so a later restore would restore the track to hidden and it would vanish for
|
||||||
|
// good. Re-applying the park flags to an already-parked track is idempotent and
|
||||||
|
// fine; only the snapshot must not be recaptured. Restore clears the snapshot,
|
||||||
|
// so the next genuine park recaptures fresh state.
|
||||||
|
if (model.snapshot(guid) == nullptr)
|
||||||
|
model.storeSnapshot(guid, snapshotTrack(tr));
|
||||||
applyFlags(tr, tp.flags);
|
applyFlags(tr, tp.flags);
|
||||||
parkFxOffline(tr);
|
parkFxOffline(tr);
|
||||||
}
|
}
|
||||||
@@ -191,6 +199,15 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
|
|||||||
SetMediaTrackInfo_Value(tr, "B_SHOWINMIXER", show);
|
SetMediaTrackInfo_Value(tr, "B_SHOWINMIXER", show);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Build the undo label from the ACTUAL target mode's display name, so activating
|
||||||
|
// Arrange doesn't leave an "activate Design view" undo point (and vice versa).
|
||||||
|
// The target is guaranteed registered (checked at entry), so query() is non-null;
|
||||||
|
// fall back to the id defensively if that ever changes.
|
||||||
|
const Mode* targetMode = model.modes().query(targetModeId);
|
||||||
|
const std::string undoLabel =
|
||||||
|
"ReaSampler: activate " +
|
||||||
|
(targetMode ? targetMode->displayName : targetModeId) + " view";
|
||||||
|
|
||||||
model.setActiveMode(targetModeId);
|
model.setActiveMode(targetModeId);
|
||||||
|
|
||||||
// Force REAPER to rebuild the TCP + MCP so visibility/park changes appear now,
|
// Force REAPER to rebuild the TCP + MCP so visibility/park changes appear now,
|
||||||
@@ -201,7 +218,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
|
|||||||
TrackList_AdjustWindows(false);
|
TrackList_AdjustWindows(false);
|
||||||
UpdateArrange();
|
UpdateArrange();
|
||||||
|
|
||||||
Undo_EndBlock2(proj, "ReaSampler: apply Design View mode", -1);
|
Undo_EndBlock2(proj, undoLabel.c_str(), -1);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -11,6 +11,9 @@
|
|||||||
// 5. Unknown/stale GUID tolerated (ignore-and-prune, no crash).
|
// 5. Unknown/stale GUID tolerated (ignore-and-prune, no crash).
|
||||||
// 6. JSON round-trip lossless: modes + membership + show-both + snapshots + active.
|
// 6. JSON round-trip lossless: modes + membership + show-both + snapshots + active.
|
||||||
// 7. planToggle park path: fxOffline is empty (shell-expands-FX contract).
|
// 7. planToggle park path: fxOffline is empty (shell-expands-FX contract).
|
||||||
|
// 9. Nested-folder toggle: the snapshot store/clear lifecycle survives a re-park
|
||||||
|
// (park-while-parked) so untagged leaves return to visible after toggling back;
|
||||||
|
// guards the in-DAW "all leaves hidden after toggling twice" regression.
|
||||||
|
|
||||||
#include "../src/view_mode_model.h"
|
#include "../src/view_mode_model.h"
|
||||||
|
|
||||||
@@ -493,6 +496,145 @@ static void testNextModeIdCycles() {
|
|||||||
CHECK(nextModeId(empty, kArrangeModeId).empty());
|
CHECK(nextModeId(empty, kArrangeModeId).empty());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// -- 9. Nested-folder toggle: snapshot lifecycle survives a re-park -----------
|
||||||
|
//
|
||||||
|
// Regression for the in-DAW bug: a nested structure (root parent > intermediate
|
||||||
|
// parent > leaves), ONE leaf tagged Design, toggled twice, hid ALL leaves for good.
|
||||||
|
//
|
||||||
|
// Root cause: the D2 shell's park loop stored a fresh snapshot on EVERY park. If a
|
||||||
|
// track is parked again while already parked — which happens on any redundant
|
||||||
|
// same-mode re-apply (a re-activate of the current mode, the segmented switch, a
|
||||||
|
// tag/untag reapply) — the second snapshot captures the track's already-HIDDEN
|
||||||
|
// flags, so a later restore returns it to hidden and the leaf vanishes permanently.
|
||||||
|
//
|
||||||
|
// The pure model can't run the REAPER shell, but the corruption is entirely in the
|
||||||
|
// snapshot store/clear lifecycle, which is model state. This harness mirrors
|
||||||
|
// applyMode's park/restore loops faithfully: it maintains a per-track live "visible"
|
||||||
|
// flag (proxy for B_SHOWINTCP), and for each toggle it runs planToggle, then for
|
||||||
|
// each park op it (a) snapshots the live flag BEFORE parking — GUARDED to only
|
||||||
|
// capture when no snapshot exists yet, exactly like the fixed shell — and (b) hides
|
||||||
|
// the track; for each restore op it writes the snapshot's flag back and clears the
|
||||||
|
// snapshot. Asserting the live flags after the sequence proves the fix; a parallel
|
||||||
|
// UNGUARDED run reproduces the original corruption.
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
|
||||||
|
// A tiny stand-in for the shell's live REAPER flag reads/writes: guid -> visible.
|
||||||
|
using LiveFlags = std::map<std::string, int>;
|
||||||
|
|
||||||
|
// Runs one applyMode-equivalent toggle against `vm` + `live`. `guard` selects the
|
||||||
|
// fixed (snapshot-once) behavior vs. the original buggy (snapshot-every-park) one.
|
||||||
|
// Returns nothing; mutates `vm` snapshots/active mode and `live` flags in place,
|
||||||
|
// exactly mirroring view.cpp's park then restore then setActiveMode ordering.
|
||||||
|
void simulateApplyMode(ViewModeModel& vm, LiveFlags& live, const FolderTree& tree,
|
||||||
|
const std::string& target, bool guard) {
|
||||||
|
TogglePlan plan = vm.planToggle(tree, target);
|
||||||
|
|
||||||
|
// PARK: snapshot-before-hide (guarded or not), then hide.
|
||||||
|
for (const auto& tp : plan.park) {
|
||||||
|
if (tp.flags.empty()) continue;
|
||||||
|
const std::string& guid = tp.flags.front().guid;
|
||||||
|
if (!guard || vm.snapshot(guid) == nullptr) {
|
||||||
|
TrackSnapshot snap;
|
||||||
|
snap.showInTcp = live[guid]; // capture the LIVE visible flag
|
||||||
|
vm.storeSnapshot(guid, snap);
|
||||||
|
}
|
||||||
|
live[guid] = 0; // park hides it
|
||||||
|
}
|
||||||
|
|
||||||
|
// RESTORE: write snapshot flag back, then drop the snapshot.
|
||||||
|
for (const auto& tp : plan.restore) {
|
||||||
|
if (tp.flags.empty()) continue;
|
||||||
|
const std::string& guid = tp.flags.front().guid;
|
||||||
|
if (const TrackSnapshot* snap = vm.snapshot(guid))
|
||||||
|
live[guid] = snap->showInTcp; // restore the captured visible flag
|
||||||
|
vm.clearSnapshot(guid);
|
||||||
|
}
|
||||||
|
|
||||||
|
vm.setActiveMode(target);
|
||||||
|
}
|
||||||
|
|
||||||
|
// The nested tree Daniel reported: root parent {R} > intermediate parent {I} >
|
||||||
|
// leaves {L1} (tagged Design) and {L2}, {L3} (untagged => Arrange).
|
||||||
|
FolderTree nestedTree() {
|
||||||
|
FolderTree t;
|
||||||
|
t.nodes.push_back(FolderNode{"{R}", "", /*isParent=*/true});
|
||||||
|
t.nodes.push_back(FolderNode{"{I}", "{R}", /*isParent=*/true});
|
||||||
|
t.nodes.push_back(FolderNode{"{L1}", "{I}", false});
|
||||||
|
t.nodes.push_back(FolderNode{"{L2}", "{I}", false});
|
||||||
|
t.nodes.push_back(FolderNode{"{L3}", "{I}", false});
|
||||||
|
return t;
|
||||||
|
}
|
||||||
|
|
||||||
|
} // namespace
|
||||||
|
|
||||||
|
static void testNestedToggleSnapshotSurvivesRepark() {
|
||||||
|
const FolderTree tree = nestedTree();
|
||||||
|
|
||||||
|
// Structural preconditions: buildFolderTree-shaped 3-level tree is classified
|
||||||
|
// correctly and the intermediate node is a parent (never parked), and its
|
||||||
|
// visibility derives from its descendant leaves.
|
||||||
|
{
|
||||||
|
ViewModeModel probe;
|
||||||
|
probe.membership().tag("{L1}", kDesignModeId);
|
||||||
|
// {I} and {R} are parents => never parked, in either mode.
|
||||||
|
auto pd = probe.planToggle(tree, kDesignModeId);
|
||||||
|
auto pa = probe.planToggle(tree, kArrangeModeId);
|
||||||
|
CHECK(!parkTargets(pd, "{I}") && !parkTargets(pd, "{R}"));
|
||||||
|
CHECK(!parkTargets(pa, "{I}") && !parkTargets(pa, "{R}"));
|
||||||
|
// Design: {L1} (its only Design leaf) is visible => {I} and {R} derive visible.
|
||||||
|
auto vd = probe.visibleTracks(tree, kDesignModeId);
|
||||||
|
CHECK(visibleHas(vd, "{I}") && visibleHas(vd, "{R}"));
|
||||||
|
// Arrange: {L2}/{L3} are Arrange leaves => {I} and {R} still derive visible.
|
||||||
|
auto va = probe.visibleTracks(tree, kArrangeModeId);
|
||||||
|
CHECK(visibleHas(va, "{I}") && visibleHas(va, "{R}"));
|
||||||
|
}
|
||||||
|
|
||||||
|
// GUARDED (fixed shell): drive the reported sequence and assert every leaf is
|
||||||
|
// returned to its correct visibility. Include a redundant same-mode re-apply
|
||||||
|
// (the real trigger) between toggles to force a park-while-parked.
|
||||||
|
{
|
||||||
|
ViewModeModel vm;
|
||||||
|
vm.membership().tag("{L1}", kDesignModeId);
|
||||||
|
LiveFlags live{{"{L1}", 1}, {"{L2}", 1}, {"{L3}", 1}}; // all visible at start
|
||||||
|
|
||||||
|
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true);
|
||||||
|
// In Design: {L2}/{L3} parked (hidden), {L1} visible.
|
||||||
|
CHECK(live["{L1}"] == 1 && live["{L2}"] == 0 && live["{L3}"] == 0);
|
||||||
|
|
||||||
|
// Redundant re-apply of the CURRENT mode (segmented switch / re-activate).
|
||||||
|
// With the guard this must NOT recapture the now-hidden snapshots.
|
||||||
|
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true);
|
||||||
|
CHECK(live["{L1}"] == 1 && live["{L2}"] == 0 && live["{L3}"] == 0);
|
||||||
|
|
||||||
|
simulateApplyMode(vm, live, tree, kArrangeModeId, /*guard=*/true);
|
||||||
|
// Back in Arrange: {L2}/{L3} RESTORED to visible; {L1} parked.
|
||||||
|
CHECK(live["{L2}"] == 1 && live["{L3}"] == 1 && live["{L1}"] == 0);
|
||||||
|
|
||||||
|
// A second full round to match "toggle twice" exactly.
|
||||||
|
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true);
|
||||||
|
CHECK(live["{L1}"] == 1 && live["{L2}"] == 0 && live["{L3}"] == 0);
|
||||||
|
simulateApplyMode(vm, live, tree, kArrangeModeId, /*guard=*/true);
|
||||||
|
CHECK(live["{L2}"] == 1 && live["{L3}"] == 1 && live["{L1}"] == 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
// UNGUARDED (original shell): the same sequence corrupts — the redundant re-apply
|
||||||
|
// recaptures {L2}/{L3}'s hidden flags, so toggling back to Arrange restores them
|
||||||
|
// to HIDDEN and they never return. This pins the exact regression the guard fixes.
|
||||||
|
{
|
||||||
|
ViewModeModel vm;
|
||||||
|
vm.membership().tag("{L1}", kDesignModeId);
|
||||||
|
LiveFlags live{{"{L1}", 1}, {"{L2}", 1}, {"{L3}", 1}};
|
||||||
|
|
||||||
|
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/false);
|
||||||
|
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/false); // re-park corrupts
|
||||||
|
simulateApplyMode(vm, live, tree, kArrangeModeId, /*guard=*/false);
|
||||||
|
|
||||||
|
// The bug: Arrange leaves stay hidden after returning to Arrange.
|
||||||
|
CHECK(live["{L2}"] == 0 && live["{L3}"] == 0);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
int main() {
|
int main() {
|
||||||
testNModeRegistryAndMembership();
|
testNModeRegistryAndMembership();
|
||||||
testParentDerivationMultiMode();
|
testParentDerivationMultiMode();
|
||||||
@@ -505,6 +647,7 @@ int main() {
|
|||||||
testPlanToggleParkHasEmptyFxOffline();
|
testPlanToggleParkHasEmptyFxOffline();
|
||||||
testUntaggedLeavesManagedByModeSystem();
|
testUntaggedLeavesManagedByModeSystem();
|
||||||
testTaggedLeafBehaviorUnchangedWithUntagged();
|
testTaggedLeafBehaviorUnchangedWithUntagged();
|
||||||
|
testNestedToggleSnapshotSurvivesRepark();
|
||||||
testNextModeIdCycles();
|
testNextModeIdCycles();
|
||||||
|
|
||||||
if (g_fail == 0) std::printf("All tests passed.\n");
|
if (g_fail == 0) std::printf("All tests passed.\n");
|
||||||
|
|||||||
Reference in New Issue
Block a user