diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 90c2fb0f..e7758a94 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -24,6 +24,44 @@ What does NOT go here: - Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending. - Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed. +## #425 — Options Apply "Atmospheric rendering" fell back to the default path and stayed locked out: Low's 64 MiB resident budget did not scale with resolution + +**Status:** ✅ FIXED 2026-08-23 (found at the owner's VM3/VM6 gate launch). +**Component:** rendering / render packs (Campaign AR budget contract + activation memo) + +**Symptom:** live Holtburg, 2560×1440 fullscreen; Options → Atmospheric rendering → +Apply → the panel snapped back to "acdream default". Log +(`artifacts/owner-gate/launch2.log`): first Apply `Render pack preset 'low' +needs 67368164 resident GPU bytes after materializing its scene-dependent +shadow command buffers; the active pack budget is 67108864 bytes`, then every +later Apply `This pack selection already failed for the current registration +and will not be retried.` + +**Two root causes:** (1) a preset's `MaxResidentGpuBytes` (Low 64 / Medium 128 / +High 256 MiB) was applied as an absolute ceiling at any resolution, but the +pack's resident set is dominated by screen-sized images (12 B/px HDR+depth plus +ray/bloom intermediates): Low's targets are 25 MB at 1080p and 44 MB at 1440p, +and live Holtburg's shadow command buffers took the total to 64.25 MiB — 0.4 % +over. Medium would have failed the same way at 4K. The ceilings were only ever +validated at 1080p. (2) The controller's failure memo keyed the user's explicit +Apply and automatic re-activation identically, so one failed attempt locked +that selection out until restart even after the cause (resolution, scene) +changed. + +**Fix:** `RenderPackResidentBudget.Effective` — the declared figure is the 1080p +ceiling, scaled by the viewport's pixel-count ratio (never below 1) and still +capped by the hardware's `MaxPackResidentBytes`; used by both pack graphs and +mirrored in `tools/run-atmospheric-performance-matrix.ps1` so the tool judges +1440p/4K rows by the same rule. `RenderPackController.Request(selection, +explicitUserChoice: true)` clears the memo for that selection; the settings +binding passes it for every display edge (the user's Apply, including a +resolution change); startup keeps the memo. Tests: +`RenderPackResidentBudgetTests`, `An_explicit_user_request_retries_a_selection_that_failed_earlier`, +the extended `Selection_binding_activates_only_at_boundary_and_persists_failed_fallback`, +and the matrix contract test. Not a workaround: the ceiling now means what the +budget table implied ("at 1080p"), and a deliberate user action is allowed to +try again. + ## #424 — Client crashed on alt-tab out of exclusive fullscreen: zero-area frame reached `RenderPackActivationExtent.Validate` **Status:** ✅ FIXED 2026-08-23 (same session it was found — the owner's VM6/VM3 gate launch). diff --git a/docs/plans/2026-08-21-atmospheric-rendering.md b/docs/plans/2026-08-21-atmospheric-rendering.md index a2c0f8b9..e8ac1a0b 100644 --- a/docs/plans/2026-08-21-atmospheric-rendering.md +++ b/docs/plans/2026-08-21-atmospheric-rendering.md @@ -739,7 +739,7 @@ CPU-submission-bound. Fullscreen work may occupy currently idle GPU time, but it is not treated as free. Shadow cascades must protect the CPU submission path. -| Preset | Incremental GPU p50 / p99 at 1080p | Incremental render-CPU p50 / p99 | Pack-owned resident GPU memory | +| Preset | Incremental GPU p50 / p99 at 1080p | Incremental render-CPU p50 / p99 | Pack-owned resident GPU memory **at 1080p** (scales with pixel count — `RenderPackResidentBudget.Effective`, #425) | |---|---:|---:|---:| | Low | ≤ 2.0 / 3.0 ms | ≤ 0.15 / 0.50 ms | ≤ 64 MiB | | Medium | ≤ 3.25 / 4.50 ms | ≤ 0.25 / 0.75 ms | ≤ 128 MiB | diff --git a/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs b/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs index a7ada34d..1cadfb34 100644 --- a/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs +++ b/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs @@ -692,8 +692,10 @@ internal sealed class AtmosphericPostProcessGraph : } TargetSet? previous = _targets; _targets = candidate; - _residentGpuBudgetBytes = Math.Min( + _residentGpuBudgetBytes = RenderPackResidentBudget.Effective( Preset.MaxResidentGpuBytes, + width, + height, capabilities.MaxPackResidentBytes); ResourceGeneration = checked(ResourceGeneration + 1); _cpuStageProfiler?.Reset(); diff --git a/src/AcDream.App/Rendering/Packs/DeclaredFullscreenRenderPackGraph.cs b/src/AcDream.App/Rendering/Packs/DeclaredFullscreenRenderPackGraph.cs index 248cf90e..0bf5fafc 100644 --- a/src/AcDream.App/Rendering/Packs/DeclaredFullscreenRenderPackGraph.cs +++ b/src/AcDream.App/Rendering/Packs/DeclaredFullscreenRenderPackGraph.cs @@ -268,8 +268,10 @@ internal class DeclaredFullscreenRenderPackGraph : TargetSet? prior = _targets; _targets = candidate; _resourceBudget = budget; - _residentGpuBudgetBytes = Math.Min( + _residentGpuBudgetBytes = RenderPackResidentBudget.Effective( Preset.MaxResidentGpuBytes, + width, + height, capabilities.MaxPackResidentBytes); _resourceGeneration = checked(_resourceGeneration + 1); _renderedFrame = false; diff --git a/src/AcDream.App/Rendering/Packs/RenderPackController.cs b/src/AcDream.App/Rendering/Packs/RenderPackController.cs index 55e7fc2e..0b764e8d 100644 --- a/src/AcDream.App/Rendering/Packs/RenderPackController.cs +++ b/src/AcDream.App/Rendering/Packs/RenderPackController.cs @@ -272,10 +272,25 @@ internal sealed class RenderPackController : }; } - internal void Request(RenderPackSelectionSettings? selection) + /// + /// True when the request is a deliberate user action (the Options panel's + /// Apply, which also fires for a resolution change). Such a request gets a + /// fresh activation attempt even if the same selection failed earlier in + /// this registration: the conditions that failed it — resolution, scene, + /// resident budget — may have changed, and the user asked. Automatic + /// re-activation (startup, catalog change) keeps the failure memo, which + /// exists to stop a known-bad selection from failing every frame. + /// (#425, Campaign VM VM7 owner gate: a Low preset that failed its budget + /// once locked the user out of the pack until restart.) + /// + internal void Request( + RenderPackSelectionSettings? selection, + bool explicitUserChoice = false) { ObjectDisposedException.ThrowIf(_disposed, this); RenderPackSelectionSettings normalized = Normalize(selection); + if (explicitUserChoice) + _failedSelections.Remove(normalized); if (normalized == _snapshot.Selection && _pending is null) return; diff --git a/src/AcDream.App/Rendering/Packs/RenderPackResidentBudget.cs b/src/AcDream.App/Rendering/Packs/RenderPackResidentBudget.cs new file mode 100644 index 00000000..8536eb90 --- /dev/null +++ b/src/AcDream.App/Rendering/Packs/RenderPackResidentBudget.cs @@ -0,0 +1,55 @@ +namespace AcDream.App.Rendering.Packs; + +/// +/// The one rule for a pack preset's effective resident-GPU ceiling. +/// +/// +/// +/// A preset's MaxResidentGpuBytes is the 1080p figure of the +/// Campaign AR budget table (Low 64 / Medium 128 / High 256 MiB). The pack's +/// resident set is dominated by screen-sized images — the HDR world colour + +/// depth targets alone are 12 bytes per pixel, plus the ray/bloom +/// intermediates — so a ceiling that does not scale with pixel count refuses +/// the pack the moment the user picks a larger mode: at 2560×1440 the Low +/// preset's screen targets are 44 MB of a 64 MiB budget, and live Holtburg's +/// scene-dependent shadow command buffers pushed the total to 64.25 MiB. +/// That refusal is what the owner hit at the Campaign VM VM7 gate +/// (2026-08-23, #425): Apply → "needs 67368164 resident GPU bytes; the +/// active pack budget is 67108864" → persisted back to the default path. +/// +/// +/// The effective ceiling is therefore the declared 1080p figure scaled by +/// the pixel-count ratio (never below 1, so smaller modes keep the declared +/// ceiling), and still capped by the hardware's MaxPackResidentBytes. +/// The parts of the resident set that do NOT scale with the screen (shadow +/// depth maps, shadow command/transform buffers) are over-allowed by the same +/// factor; that errs on the side of honouring the user's explicit resolution +/// choice while keeping the bound proportional to what they asked the GPU to +/// draw. tools/run-atmospheric-performance-matrix.ps1 judges its +/// resident column with this same rule so the tool and the runtime cannot +/// disagree. +/// +/// +internal static class RenderPackResidentBudget +{ + internal const long ReferencePixels = 1920L * 1080L; + + internal static long Effective( + long declaredBytesAt1080p, + int viewportWidth, + int viewportHeight, + long hardwareCapBytes) + { + ArgumentOutOfRangeException.ThrowIfNegative(declaredBytesAt1080p); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(viewportWidth); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(viewportHeight); + ArgumentOutOfRangeException.ThrowIfNegative(hardwareCapBytes); + + long pixels = checked((long)viewportWidth * viewportHeight); + long scaled = pixels <= ReferencePixels + ? declaredBytesAt1080p + : checked((long)Math.Ceiling( + (double)declaredBytesAt1080p * pixels / ReferencePixels)); + return Math.Min(scaled, hardwareCapBytes); + } +} diff --git a/src/AcDream.App/Rendering/Packs/RenderPackSelectionBinding.cs b/src/AcDream.App/Rendering/Packs/RenderPackSelectionBinding.cs index 23bc4537..68e038a1 100644 --- a/src/AcDream.App/Rendering/Packs/RenderPackSelectionBinding.cs +++ b/src/AcDream.App/Rendering/Packs/RenderPackSelectionBinding.cs @@ -79,7 +79,11 @@ internal sealed class RenderPackSelectionBinding : IDisposable private void OnDisplayChanged(DisplaySettings display) { + // A display edge is the user's Apply (pack, preset or resolution) — + // an explicit choice, so a selection that failed earlier gets one + // fresh attempt (#425). The constructor's startup request above is + // automatic and keeps the memo. if (!_disposed && !_suppressDisplayEdge) - _controller.Request(display.RenderPack); + _controller.Request(display.RenderPack, explicitUserChoice: true); } } diff --git a/tests/AcDream.App.Tests/Diagnostics/AtmosphericPerformanceMatrixContractTests.cs b/tests/AcDream.App.Tests/Diagnostics/AtmosphericPerformanceMatrixContractTests.cs index 1b6afd0c..0be66111 100644 --- a/tests/AcDream.App.Tests/Diagnostics/AtmosphericPerformanceMatrixContractTests.cs +++ b/tests/AcDream.App.Tests/Diagnostics/AtmosphericPerformanceMatrixContractTests.cs @@ -162,10 +162,20 @@ public sealed class AtmosphericPerformanceMatrixContractTests "$cpuP99 -gt $budget.IncrementalCpuMillisecondsP99", source, StringComparison.Ordinal); + // #425: the declared resident ceiling is the 1080p figure and scales + // with the row's pixel count — the same rule as + // RenderPackResidentBudget.Effective, so the tool and the runtime + // cannot disagree about a 1440p/4K row. Assert.Contains( - "$residentGpuBytes -gt $budget.ResidentGpuBytes", + "$residentGpuBytes -gt $residentCeiling", source, StringComparison.Ordinal); + Assert.Contains("$referencePixels = 1920L * 1080L", source, StringComparison.Ordinal); + Assert.Contains( + "[Math]::Ceiling([double]$budget.ResidentGpuBytes * $rowPixels / $referencePixels)", + source, + StringComparison.Ordinal); + Assert.Contains("$rowPixels -le $referencePixels", source, StringComparison.Ordinal); Assert.Contains( "$gpuP50 -gt $budget.InclusiveGpuMillisecondsP50At1080p", source, diff --git a/tests/AcDream.App.Tests/Rendering/Packs/RenderPackControllerTests.cs b/tests/AcDream.App.Tests/Rendering/Packs/RenderPackControllerTests.cs index 98f0ea56..a36bdfd8 100644 --- a/tests/AcDream.App.Tests/Rendering/Packs/RenderPackControllerTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Packs/RenderPackControllerTests.cs @@ -291,6 +291,47 @@ public sealed class RenderPackControllerTests Assert.Equal(1, assets.OpenCount); } + [Fact] + public void An_explicit_user_request_retries_a_selection_that_failed_earlier() + { + // #425 (Campaign VM VM7 owner gate): a preset that failed its resident + // budget once locked the user out of the pack until restart, because + // the Options panel's Apply went through the same memo as automatic + // re-activation. An explicit choice is a fresh attempt — here the + // second attempt opens the assets again and fails on its own merits + // (still invalid SPIR-V), not with the "will not be retried" memo. + RenderPackDescriptor descriptor = Descriptor() with + { + Passes = + [ + new RenderPassDeclaration( + "tone-map", + RenderPassHook.ToneMap, + "shaders/fullscreen.vert.spv", + "shaders/tone-map.frag.spv", + [RenderSemanticInput.WorldColor], + [], + []), + ], + }; + var assets = new StubAssets([1, 2, 3, 4]); + using var registry = new BufferedRenderPackRegistry(); + using IDisposable registration = registry.Register(descriptor, assets); + var factory = new StubFactory(); + using var controller = Controller(registry, factory); + + controller.Request(Selection()); + RenderPackActivationSnapshot first = controller.ApplyAtFrameBoundary(Extent); + controller.Request(Selection(), explicitUserChoice: true); + RenderPackActivationSnapshot second = controller.ApplyAtFrameBoundary(Extent); + + Assert.Equal(RenderPackActivationState.FailedToRetail, first.State); + Assert.Equal(RenderPackActivationState.FailedToRetail, second.State); + Assert.Contains("not valid SPIR-V", second.Reason, StringComparison.Ordinal); + Assert.DoesNotContain("will not be retried", second.Reason, StringComparison.Ordinal); + Assert.Equal(2, assets.OpenCount); + } + [Fact] public void Arbitrary_plugin_asset_exception_is_contained_as_a_retail_fallback() { @@ -682,6 +723,20 @@ public sealed class RenderPackControllerTests Assert.True(settings.Display.RenderPack.IsRetail); Assert.Contains("pipeline rejected", controller.Snapshot.Reason, StringComparison.Ordinal); Assert.Null(controller.ActiveRuntime); + + // #425: the user's next Apply of the SAME selection is an explicit + // choice — with the failure cause gone it activates instead of being + // refused with the "will not be retried" memo. + factory.Failure = null; + settings.SaveDisplay(settings.Display with + { + RenderPack = Selection() with { PresetId = "medium" }, + }); + RenderPackActivationSnapshot retried = binding.ApplyAtFrameBoundary(Extent); + + Assert.NotEqual(RenderPackActivationState.FailedToRetail, retried.State); + Assert.DoesNotContain("will not be retried", retried.Reason ?? string.Empty, StringComparison.Ordinal); + Assert.NotNull(controller.ActiveRuntime); } [Fact] diff --git a/tests/AcDream.App.Tests/Rendering/Packs/RenderPackResidentBudgetTests.cs b/tests/AcDream.App.Tests/Rendering/Packs/RenderPackResidentBudgetTests.cs new file mode 100644 index 00000000..20f1937a --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/Packs/RenderPackResidentBudgetTests.cs @@ -0,0 +1,51 @@ +using AcDream.App.Rendering.Packs; + +namespace AcDream.App.Tests.Rendering.Packs; + +public sealed class RenderPackResidentBudgetTests +{ + private const long LowAt1080p = 64L * 1024 * 1024; + private const long Unlimited = long.MaxValue; + + [Theory] + [InlineData(1920, 1080, LowAt1080p)] + [InlineData(1280, 720, LowAt1080p)] // smaller than the reference keeps the declared ceiling + [InlineData(1600, 900, LowAt1080p)] + public void AtOrBelowTheReferenceResolutionTheDeclaredCeilingApplies(int width, int height, long expected) + { + Assert.Equal(expected, RenderPackResidentBudget.Effective(LowAt1080p, width, height, Unlimited)); + } + + [Fact] + public void At1440pTheCeilingScalesByPixelCount() + { + // 2560x1440 / 1920x1080 = 16/9 ≈ 1.777…; #425's live Holtburg Low + // preset needed 67,368,164 bytes, which the scaled ceiling admits. + long effective = RenderPackResidentBudget.Effective(LowAt1080p, 2560, 1440, Unlimited); + Assert.Equal((long)Math.Ceiling(LowAt1080p * 16.0 / 9.0), effective); + Assert.True(effective >= 67_368_164L); + } + + [Fact] + public void At4KTheCeilingIsFourTimesTheDeclaredFigure() + { + Assert.Equal(4L * LowAt1080p, RenderPackResidentBudget.Effective(LowAt1080p, 3840, 2160, Unlimited)); + } + + [Fact] + public void TheHardwareCapStillWins() + { + Assert.Equal( + 100_000_000L, + RenderPackResidentBudget.Effective(LowAt1080p, 3840, 2160, hardwareCapBytes: 100_000_000L)); + } + + [Theory] + [InlineData(0, 1080)] + [InlineData(1920, 0)] + public void AZeroExtentIsRejected(int width, int height) + { + Assert.Throws( + () => RenderPackResidentBudget.Effective(LowAt1080p, width, height, Unlimited)); + } +} diff --git a/tools/run-atmospheric-performance-matrix.ps1 b/tools/run-atmospheric-performance-matrix.ps1 index 873a058e..1ed00ca3 100644 --- a/tools/run-atmospheric-performance-matrix.ps1 +++ b/tools/run-atmospheric-performance-matrix.ps1 @@ -527,9 +527,19 @@ foreach ($pacing in $pacingModes) { "incremental CPU p99 $(Format-Invariant $cpuP99) ms exceeds " + "$(Format-Invariant $budget.IncrementalCpuMillisecondsP99) ms") } - if ($residentGpuBytes -gt $budget.ResidentGpuBytes) { + # RenderPackResidentBudget.Effective: the declared ceiling is the + # 1080p figure; it scales with the row's pixel count (never below 1). + $rowPixels = [long]$dimensions[0] * [long]$dimensions[1] + $referencePixels = 1920L * 1080L + $residentCeiling = if ($rowPixels -le $referencePixels) { + [long]$budget.ResidentGpuBytes + } else { + [long][Math]::Ceiling([double]$budget.ResidentGpuBytes * $rowPixels / $referencePixels) + } + if ($residentGpuBytes -gt $residentCeiling) { Add-Failure $rowFailures ( - "resident GPU bytes $residentGpuBytes exceed $($budget.ResidentGpuBytes)") + "resident GPU bytes $residentGpuBytes exceed $residentCeiling " + + "(declared $($budget.ResidentGpuBytes) at 1080p, scaled to ${resolution})") } if ($gpuBudgetApplies -and $gpuP50 -gt $budget.InclusiveGpuMillisecondsP50At1080p) {