From 47ccadf726b44cd01b0276debb073e988257fe49 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Wed, 29 Jul 2026 21:15:00 -0400 Subject: [PATCH] Restore review-flagged comment content cut too aggressively in comment reduction --- src/core/capture/render_settings.h | 8 +++++--- src/core/view/view_mode_model.cpp | 3 +++ src/core/wire/sample_usage.cpp | 3 +++ src/core/wire/sample_usage.h | 4 ++++ src/shell/capture/capture.cpp | 14 ++++++++------ src/shell/capture/capture_orchestrator.cpp | 2 +- src/shell/persist/prune_fs.cpp | 2 +- src/shell/persist/session.cpp | 8 +++++--- src/shell/persist/usage_scan.cpp | 7 ++++--- src/shell/view/view.cpp | 3 ++- 10 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/core/capture/render_settings.h b/src/core/capture/render_settings.h index a80e9eb..211a11d 100644 --- a/src/core/capture/render_settings.h +++ b/src/core/capture/render_settings.h @@ -128,9 +128,11 @@ RangeSource inferRangeSource(bool hasRazorArea); // track's OWN FX, (b) every ancestor (parent/folder) track's FX, (c) the // master FX. The caller (FxBypassGuard, shell) walks the ancestor chain via // GetParentTrack, clears I_FXEN on each flagged track (RAII restore), and also -// neutralizes D_VOL/D_PAN/D_WIDTH/D_PANLAW to unity/center on the same set — -// I_FXEN alone doesn't touch a track's volume/pan. This plan selects the set; -// the guard applies both the FX bypass and the neutralize. +// neutralizes D_VOL/D_PAN/D_WIDTH/D_PANLAW/I_PANMODE to unity/center on the +// same set (I_PANMODE is load-bearing: in pan mode 6, D_PAN/D_WIDTH are +// ignored entirely, so forcing it is what makes the other neutralizations +// take effect) — I_FXEN alone doesn't touch a track's volume/pan. This plan +// selects the set; the guard applies both the FX bypass and the neutralize. struct FxBypassPlan { bool bypassSelfFx = false; // the captured track's own FX bool bypassAncestorFx = false; // every ancestor (parent/folder) track's FX diff --git a/src/core/view/view_mode_model.cpp b/src/core/view/view_mode_model.cpp index fd6ab92..0a6da20 100644 --- a/src/core/view/view_mode_model.cpp +++ b/src/core/view/view_mode_model.cpp @@ -283,6 +283,9 @@ std::set ViewModeModel::visibleTracks(const FolderTree& tree, std::map parentOf; for (const auto& node : tree.nodes) parentOf[node.guid] = node.parentGuid; + // Seed from the full pass-1 set (not just leaves) so a parent already visible by + // its own membership still propagates visibility up its remaining ancestors; a + // snapshot copy so mid-loop insertions into `visible` are never re-walked. const std::vector seeds(visible.begin(), visible.end()); for (const auto& node : seeds) { auto it = parentOf.find(node); diff --git a/src/core/wire/sample_usage.cpp b/src/core/wire/sample_usage.cpp index 3257924..471ac56 100644 --- a/src/core/wire/sample_usage.cpp +++ b/src/core/wire/sample_usage.cpp @@ -65,6 +65,9 @@ UsagePublishPlan planUsagePublish(const std::optional& existing, plan.wire = encodeUsageRecord(cleanMine); if (!existing || existing->empty()) { + // NOTE (>16 MB gap): readReasamplerExtState returning nullopt for a value + // larger than 16 MB is indistinguishable from "absent" here, so that narrow + // case also takes this fresh-write branch rather than remint. return plan; // fresh key — write mine } diff --git a/src/core/wire/sample_usage.h b/src/core/wire/sample_usage.h index a42e6f5..8ff2e09 100644 --- a/src/core/wire/sample_usage.h +++ b/src/core/wire/sample_usage.h @@ -28,6 +28,10 @@ // never degrade toward delete); // * unreadable record -> ABORT the prune entirely (a record we cannot read // may protect anything; halting deletes nothing). +// Residual: readReasamplerExtState returning nullopt +// for a >16 MB value is indistinguishable from +// "absent" at the publish site, so that narrow case +// takes the fresh-write branch, not remint. // // Liveness is decided extension-side at read time, not by teardown clearing // (REAPER destroys the plugin instance when an FX goes offline, including diff --git a/src/shell/capture/capture.cpp b/src/shell/capture/capture.cpp index ec6ed70..687e9ca 100644 --- a/src/shell/capture/capture.cpp +++ b/src/shell/capture/capture.cpp @@ -50,9 +50,10 @@ namespace { // The no-dialog render is the built-in action "File: Render project, using the // most recent render settings" — command id 42230. Stock main action id, not in -// reaper_plugin_functions.h, confirmed against a running REAPER. Renders -// headlessly using whatever RENDER_* settings are currently on the project — -// why we set them all explicitly first. +// reaper_plugin_functions.h, so it cannot be verified against the SDK header +// (offline capture shipping as the default is the empirical evidence it holds). +// Renders headlessly using whatever RENDER_* settings are currently on the +// project — why we set them all explicitly first. constexpr int kActionRenderUsingMostRecentSettings = 42230; // RENDER_BOUNDSFLAG 0 = custom time bounds (we set STARTPOS/ENDPOS ourselves @@ -275,9 +276,10 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { // the .rpp location, and is never empty even when unsaved (the original bug — // captures landed in REAPER's default media location instead of by the .rpp). // - // Flow: read .rpp path; if empty, Main_SaveProject(proj, true) prompts - // Save-As and blocks until dismissed; re-read; if still empty (cancelled), - // refuse with NoProject and write nothing. + // Flow: read .rpp path; if empty, Main_SaveProject(proj, true) — true = + // forceSaveAsIn — prompts Save-As and (per SDK header) blocks until dismissed, + // though that blocking behaviour cannot be verified from the header itself; + // re-read; if still empty (cancelled), refuse with NoProject and write nothing. auto readRppPath = [&]() -> std::string { std::vector buf(4096, '\0'); EnumProjects(-1, buf.data(), static_cast(buf.size())); diff --git a/src/shell/capture/capture_orchestrator.cpp b/src/shell/capture/capture_orchestrator.cpp index 124034f..524e34e 100644 --- a/src/shell/capture/capture_orchestrator.cpp +++ b/src/shell/capture/capture_orchestrator.cpp @@ -450,7 +450,7 @@ void RunCancelRealtime(ReaSamplerSession& session) // via InsertMedia, undo-wrapped. `conform` selects the explicit opt-in tempo-match // variant (never silent — it fires only from the distinct "conform" action). This // is the INTENDED placement path: it adds items to the arrange on purpose -// (CONTEXT.md §load-bearing principle) and runs only from a user-invoked action. +// (CLAUDE.md §The load-bearing principle) and runs only from a user-invoked action. void RunInsertSelected(ReaSamplerSession& session, bool conform) { InsertRequest req; diff --git a/src/shell/persist/prune_fs.cpp b/src/shell/persist/prune_fs.cpp index 783164b..104e180 100644 --- a/src/shell/persist/prune_fs.cpp +++ b/src/shell/persist/prune_fs.cpp @@ -178,7 +178,7 @@ bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash, outUsedTrash = true; return true; // deleted this call -> reclaimed } - // Distinguish "already absent" (nonzero return on some REAPER versions + // Distinguish "already absent" (nonzero return on some Windows/shell versions // for a vanished file) from a real failure so the caller can tally separately. std::error_code ec; if (!fs::exists(absPath, ec)) { diff --git a/src/shell/persist/session.cpp b/src/shell/persist/session.cpp index 0d9bca7..e1fb10e 100644 --- a/src/shell/persist/session.cpp +++ b/src/shell/persist/session.cpp @@ -7,9 +7,11 @@ // // classifyProjectTransition (pure, capture_paths) takes a `sameProjectObject` // bool so the decision stays REAPER-free and testable; poll() executes the -// verdict. REAPER exposes no stable per-project GUID, so we mint one (genGuid/ -// guidToString) under kProjExtGuidKey; Save-As copies the whole .rpp including -// our ext state, so the new project initially shares the old GUID, and poll() +// verdict. REAPER exposes no stable per-project GUID (GetSetProjectInfo_String +// has no PROJECT_GUID desc; GetProjectStateChangeCount is a session-local +// counter, not cross-open identity), so we mint one (genGuid/guidToString) +// under kProjExtGuidKey; Save-As copies the whole .rpp including our ext +// state, so the new project initially shares the old GUID, and poll() // re-GUIDs it after relocating (or on the forked-sibling Load branch). // // Division of labour for undo/redo: the identity-transition poll (this file) diff --git a/src/shell/persist/usage_scan.cpp b/src/shell/persist/usage_scan.cpp index cbed7f7..7d721fb 100644 --- a/src/shell/persist/usage_scan.cpp +++ b/src/shell/persist/usage_scan.cpp @@ -105,9 +105,10 @@ bool fxSubtreeHasInstance(const FxParmGetter& parm, int fxId, return false; } -// Real-world FX containers are typically 2-4 levels deep; 32 is unreachable -// in practice while remaining finite. The truncation->protect-all guard above -// is the primary protection even at this depth. +// Raised from 8 to 32 (defense in depth against truncation). Real-world FX +// containers are typically 2-4 levels deep; 32 is unreachable in practice +// while remaining finite. The truncation->protect-all guard above is the +// primary protection even at this depth. constexpr int kMaxContainerDepth = 32; std::string trackFxParm(MediaTrack* tr, int fxId, const char* parm) { diff --git a/src/shell/view/view.cpp b/src/shell/view/view.cpp index b4af6d2..9bb8ed1 100644 --- a/src/shell/view/view.cpp +++ b/src/shell/view/view.cpp @@ -148,7 +148,8 @@ void parkFxOffline(MediaTrack* tr) { } } -// Restores per-FX offline from the snapshot, bounds-checked against the live +// Restores per-FX offline from the snapshot verbatim — each slot back to its +// captured value, never a blanket "online" — bounds-checked against the live // FX count (prune-safe if the chain changed while parked). // // HAZARD (open, tracked in docs/TODO.md): this remaps by slot INDEX, not