From dfd5d4477f7296cb99d2db197e894abf2ff5aa32 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 23 Jul 2026 05:50:19 -0400 Subject: [PATCH] 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. --- src/view.cpp | 21 ++++- tests/test_view_mode_model.cpp | 143 +++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 2 deletions(-) diff --git a/src/view.cpp b/src/view.cpp index 04a68fe..2c6af50 100644 --- a/src/view.cpp +++ b/src/view.cpp @@ -158,7 +158,15 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject MediaTrack* tr = resolve(handleByGuid, guid); 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); parkFxOffline(tr); } @@ -191,6 +199,15 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject 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); // 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); UpdateArrange(); - Undo_EndBlock2(proj, "ReaSampler: apply Design View mode", -1); + Undo_EndBlock2(proj, undoLabel.c_str(), -1); return true; } diff --git a/tests/test_view_mode_model.cpp b/tests/test_view_mode_model.cpp index 8630d9f..0b3b2be 100644 --- a/tests/test_view_mode_model.cpp +++ b/tests/test_view_mode_model.cpp @@ -11,6 +11,9 @@ // 5. Unknown/stale GUID tolerated (ignore-and-prune, no crash). // 6. JSON round-trip lossless: modes + membership + show-both + snapshots + active. // 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" @@ -493,6 +496,145 @@ static void testNextModeIdCycles() { 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; + +// 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() { testNModeRegistryAndMembership(); testParentDerivationMultiMode(); @@ -505,6 +647,7 @@ int main() { testPlanToggleParkHasEmptyFxOffline(); testUntaggedLeavesManagedByModeSystem(); testTaggedLeafBehaviorUnchangedWithUntagged(); + testNestedToggleSnapshotSurvivesRepark(); testNextModeIdCycles(); if (g_fail == 0) std::printf("All tests passed.\n");