From 7e4cfb37c307155b74c3d03ab3596b0952b98f96 Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 22 Jul 2026 04:18:31 +0200 Subject: [PATCH] refactor(render): establish frame orchestration contract --- ...ndow-slice-7-render-frame-orchestration.md | 18 +- .../Rendering/GpuFrameFlightController.cs | 5 +- .../Rendering/RenderFrameOrchestrator.cs | 131 +++++++ .../Rendering/RenderFrameOrchestratorTests.cs | 340 ++++++++++++++++++ 4 files changed, 489 insertions(+), 5 deletions(-) create mode 100644 src/AcDream.App/Rendering/RenderFrameOrchestrator.cs create mode 100644 tests/AcDream.App.Tests/Rendering/RenderFrameOrchestratorTests.cs diff --git a/docs/plans/2026-07-22-gamewindow-slice-7-render-frame-orchestration.md b/docs/plans/2026-07-22-gamewindow-slice-7-render-frame-orchestration.md index 135cf817..0a1e41dd 100644 --- a/docs/plans/2026-07-22-gamewindow-slice-7-render-frame-orchestration.md +++ b/docs/plans/2026-07-22-gamewindow-slice-7-render-frame-orchestration.md @@ -14,7 +14,7 @@ port, or resource-lifetime redesign. ## Progress ledger -- [ ] A — freeze the complete render graph, correct the architecture SSOT, and +- [x] A — freeze the complete render graph, correct the architecture SSOT, and introduce data-only frame contracts plus deterministic order/failure tests. - [ ] B — extract paperdoll, panel-layout/devtools, and render-diagnostics leaf owners while `GameWindow` still orders the frame. @@ -319,14 +319,24 @@ never choose a draw branch or mutate canonical world ownership. - correct the stale render-pipeline and per-frame architecture sections; - add `RenderFrameInput`, phase interfaces, and a recording orchestrator; -- pin normal order, portal replacement, login wait, indoor/outdoor/null-root - branches, and screenshot placement; camera is an initialized runtime - invariant, so the slice does not invent a new graceful missing-camera path; +- pin the outer resource/world/private/diagnostic order across normal, + portal-replacement, login-wait, and screenshot outcome shapes; defer the + concrete indoor/outdoor/null-root branch tests to the world builder and scene + owner in D/F; camera is an initialized runtime invariant, so the slice does + not invent a new graceful missing-camera path; - pin the exact GPU begin/end exception matrix; - add source guards against `GameWindow` back-references, mega contexts, and a new PView delegate bag; - capture the current connected framebuffer/resource baseline. +Completed 2026-07-22. `RenderFrameOrchestrator` now owns the inert five-owner +outer transaction contract; `GpuFrameFlightController` implements its narrow +lifetime seam. Twenty-six focused render/GPU tests pin value-only contracts, +required-owner construction, exact phase prefixes, per-phase input propagation, +and every begin/render/close failure combination. Release build and the full +suite pass (7,199 passed / 5 skipped after adding 17 tests). All three +corrected-diff reviews are clean; production `GameWindow.OnRender` is unchanged. + ### B — leaf presentation and diagnostics - extract paperdoll refresh/pose/dirty/FBO ownership; diff --git a/src/AcDream.App/Rendering/GpuFrameFlightController.cs b/src/AcDream.App/Rendering/GpuFrameFlightController.cs index 3090eb21..d2342d7d 100644 --- a/src/AcDream.App/Rendering/GpuFrameFlightController.cs +++ b/src/AcDream.App/Rendering/GpuFrameFlightController.cs @@ -214,7 +214,10 @@ internal sealed class ImmediateGpuResourceRetirementQueue : IGpuResourceRetireme } } -internal sealed class GpuFrameFlightController : IGpuResourceRetirementQueue, IDisposable +internal sealed class GpuFrameFlightController : + IGpuResourceRetirementQueue, + IRenderFrameLifetime, + IDisposable { internal const int DefaultMaximumFramesInFlight = 3; private const ulong WaitSliceNanoseconds = 1_000_000; diff --git a/src/AcDream.App/Rendering/RenderFrameOrchestrator.cs b/src/AcDream.App/Rendering/RenderFrameOrchestrator.cs new file mode 100644 index 00000000..6aa79fb9 --- /dev/null +++ b/src/AcDream.App/Rendering/RenderFrameOrchestrator.cs @@ -0,0 +1,131 @@ +namespace AcDream.App.Rendering; + +/// +/// Host-owned values for one render callback. Render phases borrow this value +/// for the duration of only. +/// +internal readonly record struct RenderFrameInput( + double DeltaSeconds, + int ViewportWidth, + int ViewportHeight); + +/// +/// World facts published after the normal-world phase has either drawn or +/// intentionally skipped the viewport. +/// +internal readonly record struct WorldRenderFrameOutcome( + int VisibleLandblocks, + int TotalLandblocks, + bool NormalWorldDrawn); + +/// +/// Private-presentation facts published after portal, paperdoll, retained UI, +/// developer UI, and screenshot work has completed. +/// +internal readonly record struct PrivatePresentationFrameOutcome( + bool PortalViewportDrawn, + bool ScreenshotCaptured); + +/// The immutable result observed by post-screenshot diagnostics. +internal readonly record struct RenderFrameOutcome( + WorldRenderFrameOutcome World, + PrivatePresentationFrameOutcome Presentation); + +internal interface IRenderFrameLifetime +{ + void BeginFrame(); + + void EndFrame(); +} + +internal interface IRenderFrameResourcePhase +{ + void Prepare(RenderFrameInput input); +} + +internal interface IWorldSceneFramePhase +{ + WorldRenderFrameOutcome Render(RenderFrameInput input); +} + +internal interface IPrivatePresentationFramePhase +{ + PrivatePresentationFrameOutcome Render( + RenderFrameInput input, + WorldRenderFrameOutcome world); +} + +internal interface IRenderFrameDiagnosticsPhase +{ + void Publish(RenderFrameInput input, RenderFrameOutcome outcome); +} + +/// +/// Owns the accepted outer render transaction. Focused phase owners retain the +/// detailed PView, private-viewport, UI, and diagnostic order inside their +/// typed boundaries. +/// +/// +/// intentionally runs outside +/// the protected render body. Once begin succeeds, close is attempted exactly +/// once. This preserves the production GPU-flight contract: a render failure +/// and close failure are reported together, while either failure alone +/// propagates directly. +/// +internal sealed class RenderFrameOrchestrator +{ + private readonly IRenderFrameLifetime _lifetime; + private readonly IRenderFrameResourcePhase _resources; + private readonly IWorldSceneFramePhase _world; + private readonly IPrivatePresentationFramePhase _presentation; + private readonly IRenderFrameDiagnosticsPhase _diagnostics; + + public RenderFrameOrchestrator( + IRenderFrameLifetime lifetime, + IRenderFrameResourcePhase resources, + IWorldSceneFramePhase world, + IPrivatePresentationFramePhase presentation, + IRenderFrameDiagnosticsPhase diagnostics) + { + _lifetime = lifetime ?? throw new ArgumentNullException(nameof(lifetime)); + _resources = resources ?? throw new ArgumentNullException(nameof(resources)); + _world = world ?? throw new ArgumentNullException(nameof(world)); + _presentation = presentation ?? throw new ArgumentNullException(nameof(presentation)); + _diagnostics = diagnostics ?? throw new ArgumentNullException(nameof(diagnostics)); + } + + public RenderFrameOutcome Render(RenderFrameInput input) + { + _lifetime.BeginFrame(); + Exception? renderFailure = null; + try + { + _resources.Prepare(input); + WorldRenderFrameOutcome world = _world.Render(input); + PrivatePresentationFrameOutcome presentation = + _presentation.Render(input, world); + var outcome = new RenderFrameOutcome(world, presentation); + _diagnostics.Publish(input, outcome); + return outcome; + } + catch (Exception error) + { + renderFailure = error; + throw; + } + finally + { + try + { + _lifetime.EndFrame(); + } + catch (Exception closeFailure) when (renderFailure is not null) + { + throw new AggregateException( + "Rendering failed and the in-flight GPU frame could not be closed.", + renderFailure, + closeFailure); + } + } + } +} diff --git a/tests/AcDream.App.Tests/Rendering/RenderFrameOrchestratorTests.cs b/tests/AcDream.App.Tests/Rendering/RenderFrameOrchestratorTests.cs new file mode 100644 index 00000000..b132cadb --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/RenderFrameOrchestratorTests.cs @@ -0,0 +1,340 @@ +using System.Reflection; +using AcDream.App.Rendering; + +namespace AcDream.App.Tests.Rendering; + +public sealed class RenderFrameOrchestratorTests +{ + private static readonly RenderFrameInput Input = new( + DeltaSeconds: 1.0 / 60.0, + ViewportWidth: 1920, + ViewportHeight: 1080); + + [Theory] + [InlineData(12, 40, true, false, false)] + [InlineData(0, 0, false, false, false)] + [InlineData(0, 0, false, true, false)] + [InlineData(4, 17, true, false, true)] + public void Render_PreservesOuterOrderAcrossWorldAndPresentationOutcomes( + int visibleLandblocks, + int totalLandblocks, + bool normalWorldDrawn, + bool portalViewportDrawn, + bool screenshotCaptured) + { + var calls = new List(); + var phases = new RecordingPhases(calls) + { + World = new WorldRenderFrameOutcome( + visibleLandblocks, + totalLandblocks, + normalWorldDrawn), + Presentation = new PrivatePresentationFrameOutcome( + portalViewportDrawn, + screenshotCaptured), + }; + var orchestrator = Create(phases); + + RenderFrameOutcome outcome = orchestrator.Render(Input); + + Assert.Equal( + ["gpu-begin", "resources", "world", "presentation", "diagnostics", "gpu-end"], + calls); + Assert.Equal(phases.World, outcome.World); + Assert.Equal(phases.Presentation, outcome.Presentation); + Assert.Equal( + [ + ("resources", Input), + ("world", Input), + ("presentation", Input), + ("diagnostics", Input), + ], + phases.ObservedInputs); + Assert.Equal(phases.World, phases.ObservedWorld); + Assert.Equal(outcome, phases.ObservedOutcome); + } + + [Fact] + public void BeginFailure_DoesNotAttemptAnyPhaseOrClose() + { + var calls = new List(); + var expected = new InvalidOperationException("begin"); + var phases = new RecordingPhases(calls) + { + FailurePoint = "gpu-begin", + Failure = expected, + }; + + InvalidOperationException actual = Assert.Throws( + () => Create(phases).Render(Input)); + + Assert.Same(expected, actual); + Assert.Equal(["gpu-begin"], calls); + } + + [Theory] + [InlineData("resources")] + [InlineData("world")] + [InlineData("presentation")] + [InlineData("diagnostics")] + public void RenderFailure_ClosesExactlyOnceAndPropagates(string failurePoint) + { + var calls = new List(); + var expected = new InvalidOperationException(failurePoint); + var phases = new RecordingPhases(calls) + { + FailurePoint = failurePoint, + Failure = expected, + }; + + InvalidOperationException actual = Assert.Throws( + () => Create(phases).Render(Input)); + + Assert.Same(expected, actual); + Assert.Equal(ExpectedFailureCalls(failurePoint), calls); + Assert.Equal( + ExpectedPhaseInputs(failurePoint), + phases.ObservedInputs); + } + + [Fact] + public void CloseOnlyFailure_PropagatesDirectly() + { + var calls = new List(); + var expected = new InvalidOperationException("close"); + var phases = new RecordingPhases(calls) + { + CloseFailure = expected, + }; + + InvalidOperationException actual = Assert.Throws( + () => Create(phases).Render(Input)); + + Assert.Same(expected, actual); + Assert.Equal( + ["gpu-begin", "resources", "world", "presentation", "diagnostics", "gpu-end"], + calls); + } + + [Theory] + [InlineData("resources")] + [InlineData("world")] + [InlineData("presentation")] + [InlineData("diagnostics")] + public void RenderAndCloseFailure_AreAggregatedInCausalOrder(string failurePoint) + { + var calls = new List(); + var renderFailure = new InvalidOperationException("render"); + var closeFailure = new InvalidOperationException("close"); + var phases = new RecordingPhases(calls) + { + FailurePoint = failurePoint, + Failure = renderFailure, + CloseFailure = closeFailure, + }; + + AggregateException actual = Assert.Throws( + () => Create(phases).Render(Input)); + + Assert.Equal( + "Rendering failed and the in-flight GPU frame could not be closed.", + actual.Message.Split(" (")[0]); + Assert.Equal(2, actual.InnerExceptions.Count); + Assert.Same(renderFailure, actual.InnerExceptions[0]); + Assert.Same(closeFailure, actual.InnerExceptions[1]); + Assert.Equal(ExpectedFailureCalls(failurePoint), calls); + Assert.Equal( + ExpectedPhaseInputs(failurePoint), + phases.ObservedInputs); + } + + [Fact] + public void Constructor_RejectsEveryMissingRequiredOwner() + { + var phases = new RecordingPhases([]); + + Assert.Throws(() => new RenderFrameOrchestrator( + null!, phases, phases, phases, phases)); + Assert.Throws(() => new RenderFrameOrchestrator( + phases, null!, phases, phases, phases)); + Assert.Throws(() => new RenderFrameOrchestrator( + phases, phases, null!, phases, phases)); + Assert.Throws(() => new RenderFrameOrchestrator( + phases, phases, phases, null!, phases)); + Assert.Throws(() => new RenderFrameOrchestrator( + phases, phases, phases, phases, null!)); + } + + [Fact] + public void Orchestrator_UsesOnlyTheExplicitTypedOwnerGraph() + { + Type[] expectedFieldTypes = + [ + typeof(IRenderFrameLifetime), + typeof(IRenderFrameResourcePhase), + typeof(IWorldSceneFramePhase), + typeof(IPrivatePresentationFramePhase), + typeof(IRenderFrameDiagnosticsPhase), + ]; + FieldInfo[] fields = typeof(RenderFrameOrchestrator).GetFields( + BindingFlags.Instance | BindingFlags.NonPublic); + + Assert.Equal( + expectedFieldTypes.OrderBy(type => type.FullName), + fields.Select(field => field.FieldType).OrderBy(type => type.FullName)); + Assert.All(fields, field => + { + Assert.NotEqual(typeof(GameWindow), field.FieldType); + Assert.False(typeof(Delegate).IsAssignableFrom(field.FieldType)); + }); + Assert.All( + expectedFieldTypes, + contract => Assert.False(contract.IsAssignableFrom(typeof(GameWindow)))); + foreach (Type contract in expectedFieldTypes) + { + foreach (MethodInfo method in contract.GetMethods()) + { + Assert.True( + method.ReturnType == typeof(void) || method.ReturnType.IsValueType, + $"{contract.Name}.{method.Name} returned owner-like type " + + method.ReturnType.FullName); + Assert.All( + method.GetParameters(), + parameter => Assert.True( + parameter.ParameterType.IsValueType, + $"{contract.Name}.{method.Name} accepted owner-like type " + + parameter.ParameterType.FullName)); + } + } + Assert.True(typeof(IRenderFrameLifetime).IsAssignableFrom( + typeof(GpuFrameFlightController))); + } + + [Fact] + public void FrameContracts_AreDataOnlyValuesWithoutOwnerOrDelegateReferences() + { + Type[] contracts = + [ + typeof(RenderFrameInput), + typeof(WorldRenderFrameOutcome), + typeof(PrivatePresentationFrameOutcome), + typeof(RenderFrameOutcome), + ]; + + foreach (Type contract in contracts) + { + Assert.True(contract.IsValueType); + Assert.All( + contract.GetFields( + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic), + field => + { + Assert.True( + field.FieldType.IsValueType, + $"{contract.Name}.{field.Name} retained owner-like type " + + field.FieldType.FullName); + }); + } + } + + private static string[] ExpectedFailureCalls(string failurePoint) => failurePoint switch + { + "resources" => ["gpu-begin", "resources", "gpu-end"], + "world" => ["gpu-begin", "resources", "world", "gpu-end"], + "presentation" => + ["gpu-begin", "resources", "world", "presentation", "gpu-end"], + "diagnostics" => + [ + "gpu-begin", "resources", "world", "presentation", "diagnostics", + "gpu-end", + ], + _ => throw new ArgumentOutOfRangeException(nameof(failurePoint)), + }; + + private static (string Phase, RenderFrameInput Input)[] ExpectedPhaseInputs( + string failurePoint) + { + string[] phases = failurePoint switch + { + "resources" => ["resources"], + "world" => ["resources", "world"], + "presentation" => ["resources", "world", "presentation"], + "diagnostics" => ["resources", "world", "presentation", "diagnostics"], + _ => throw new ArgumentOutOfRangeException(nameof(failurePoint)), + }; + return phases.Select(phase => (phase, Input)).ToArray(); + } + + private static RenderFrameOrchestrator Create(RecordingPhases phases) => + new(phases, phases, phases, phases, phases); + + private sealed class RecordingPhases : + IRenderFrameLifetime, + IRenderFrameResourcePhase, + IWorldSceneFramePhase, + IPrivatePresentationFramePhase, + IRenderFrameDiagnosticsPhase + { + private readonly List _calls; + + public RecordingPhases(List calls) + { + _calls = calls; + } + + public string? FailurePoint { get; init; } + public Exception? Failure { get; init; } + public Exception? CloseFailure { get; init; } + public WorldRenderFrameOutcome World { get; init; } = new(7, 19, true); + public PrivatePresentationFrameOutcome Presentation { get; init; } = new(false, false); + public List<(string Phase, RenderFrameInput Input)> ObservedInputs { get; } = []; + public WorldRenderFrameOutcome ObservedWorld { get; private set; } + public RenderFrameOutcome ObservedOutcome { get; private set; } + + public void BeginFrame() => Record("gpu-begin"); + + public void EndFrame() + { + _calls.Add("gpu-end"); + if (CloseFailure is not null) + throw CloseFailure; + } + + public void Prepare(RenderFrameInput input) + { + ObservedInputs.Add(("resources", input)); + Record("resources"); + } + + public WorldRenderFrameOutcome Render(RenderFrameInput input) + { + ObservedInputs.Add(("world", input)); + Record("world"); + return World; + } + + public PrivatePresentationFrameOutcome Render( + RenderFrameInput input, + WorldRenderFrameOutcome world) + { + ObservedInputs.Add(("presentation", input)); + ObservedWorld = world; + Record("presentation"); + return Presentation; + } + + public void Publish(RenderFrameInput input, RenderFrameOutcome outcome) + { + ObservedInputs.Add(("diagnostics", input)); + ObservedOutcome = outcome; + Record("diagnostics"); + } + + private void Record(string call) + { + _calls.Add(call); + if (FailurePoint == call && Failure is not null) + throw Failure; + } + } +}