From cd1cdee0e50bb5e783e6c9884295f11c9f91e9e1 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 29 Aug 2026 16:51:32 +0200 Subject: [PATCH] fix(render) #443: private viewports take the ring transform path The paperdoll was visible only in portal space. Root cause: the classic WbDrawDispatcher.Draw path appended its transforms into the SHARED world transform frame (WorldTransformFrameArena.Append) with a non-zero base instance, but the default mesh shaders index every parallel per-instance array - clip slots, light sets, indoor, OPACITY, selection lighting, detail category - zero-based; only the packed world submission's shader convention subtracts the shared-arena prefix. With a world frame active the doll drew all instances at per-instance opacity 0 into a cleared target: counted draws, blank pixels, deterministic. Portal space worked because no world transform frame is active there, so the same code took the ring path with base 0. The private viewports are the only production consumers of the classic path, hiding the defect everywhere else. Fix: WbDrawDispatcher.NextClassicDrawIsPrivatePass - the private viewport renderer marks its draw and WriteWorldTransformSection routes private passes onto the plain ring path unconditionally (self-contained render state: the private pass owns its own camera, lighting, and target, and must not depend on the world frame's pose address space). Also landed, each independently justified: - Per-GPU-flight-slot private targets (PrivateViewportFlightTargets), restoring the pre-f6fe0f2a design: that revert's claim that frame submission order protects the single target's write->sample transition is not guaranteed across Vulkan command buffers. Per-slot completed scenes fix the cleared-sibling-after-reveal wart the old attempt had. - Paperdoll resource preparation moved to the frame resource phase (IPrivateEntityViewportResourcePreparation) before world draws consume the bounded composite-upload budget. - The presenter redresses on every dirty edge (an appearance-equal clone can pin retired readiness across generations; the renderer's two-phase promote keeps the last completed image visible during replacement), publishes only non-zero handles, and clears the viewport exactly once at the explicit character-session boundary. Verified live on the clean build: doll visible in the NORMAL world, visible through portal space, and still visible after arrival - the exact reported repro cycle. 26 paperdoll/private-viewport/preparation tests plus 60 renderer-suite tests pass; owner visual gate pending. Co-Authored-By: Claude Fable 5 --- docs/ISSUES.md | 24 +- .../Composition/FrameRootComposition.cs | 3 +- .../Rendering/PaperdollFramePresenter.cs | 124 +++---- .../PrivateEntityViewportRenderer.cs | 310 ++++++++++++------ .../Rendering/PrivatePresentationRenderer.cs | 13 + .../RenderFramePreparationController.cs | 11 +- .../Rendering/Wb/WbDrawDispatcher.Rhi.cs | 22 +- .../Rendering/PaperdollFramePresenterTests.cs | 76 ++++- .../PrivateViewportFlightTargetsTests.cs | 65 ++++ 9 files changed, 439 insertions(+), 209 deletions(-) create mode 100644 tests/AcDream.App.Tests/Rendering/PrivateViewportFlightTargetsTests.cs diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 7744316e..0c0dab2f 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -431,7 +431,29 @@ authoritative 8→8 reconciliation. Owner check is in ## #443 — Examination/paperdoll private viewport: doll appears only after a delay on first open (was: "renders nothing") -**Status:** CLOSED 2026-08-28 — owner-directed ledger cleanup. +**Status:** FIXED 2026-08-29 — root cause found; owner visual gate pending. + +**2026-08-29 root cause (the "only visible in portal space" recurrence):** +the classic `WbDrawDispatcher.Draw` path appended its transforms into the +SHARED world transform frame with a non-zero base instance, but the default +mesh shaders index every parallel per-instance array (clip slots, light +sets, indoor, OPACITY, selection lighting, detail category) zero-based — +only the packed world submission's shader convention subtracts the +shared-arena prefix. The doll therefore drew all of its instances with +per-instance opacity 0 into a cleared target whenever a world transform +frame was active: counted draws, blank pixels, deterministic. Portal space +worked because the arena is inactive there (ring path, base 0). The +private viewports are the only production consumers of the classic path, +which is why nothing else ever showed the defect. Fix: +`WbDrawDispatcher.NextClassicDrawIsPrivatePass` — private passes always +take the ring transform path (self-contained render state). The same round +also restored per-flight-slot private targets (the f6fe0f2a single-slot +revert relied on cross-command-buffer ordering Vulkan does not guarantee), +moved paperdoll resource preparation into the frame's resource phase ahead +of world composite-budget consumption, made the presenter redress on every +dirty edge, and stopped transient zero handles from erasing a completed +image (session reset is the one explicit clear). Verified live: doll +visible in the normal world, through portal space, and after arrival. **Previous status:** FIXED / CONNECTED LIVE RE-GATE PASSED 2026-08-26 — awaiting owner acceptance. Reopened after the owner again observed a missing paperdoll that diff --git a/src/AcDream.App/Composition/FrameRootComposition.cs b/src/AcDream.App/Composition/FrameRootComposition.cs index cf0bf2d1..dc94659c 100644 --- a/src/AcDream.App/Composition/FrameRootComposition.cs +++ b/src/AcDream.App/Composition/FrameRootComposition.cs @@ -762,7 +762,8 @@ internal sealed class FrameRootCompositionPhase var framePreparation = new RenderFramePreparationController( renderFrameResources, devTools: null, - renderWeatherFrame); + renderWeatherFrame, + live.PaperdollPresenter); IRenderFramePostDiagnosticsPhase postDiagnostics = renderSceneShadowComparison is not null && lifecycleAutomation is not null diff --git a/src/AcDream.App/Rendering/PaperdollFramePresenter.cs b/src/AcDream.App/Rendering/PaperdollFramePresenter.cs index 815a794c..b19eba1d 100644 --- a/src/AcDream.App/Rendering/PaperdollFramePresenter.cs +++ b/src/AcDream.App/Rendering/PaperdollFramePresenter.cs @@ -24,6 +24,8 @@ internal interface IPaperdollFrameView bool TryGetVisibleSize(out int width, out int height); void SetTextureHandle(uint textureHandle); + + void ClearTextureHandle(); } internal interface IPaperdollInventoryVisibility @@ -51,7 +53,9 @@ internal interface IPaperdollPoseApplicator /// presentation edge. The renderer remains a borrowed resource disposed by /// the existing window shutdown transaction. /// -internal sealed class PaperdollFramePresenter : IPrivateEntityViewportFrame +internal sealed class PaperdollFramePresenter : + IPrivateEntityViewportFrame, + IPrivateEntityViewportResourcePreparation { private readonly IPaperdollDollRenderer _renderer; private readonly IPaperdollFrameView _view; @@ -73,22 +77,30 @@ internal sealed class PaperdollFramePresenter : IPrivateEntityViewportFrame public void MarkDirty() => _dirty = true; - public void Render() + /// + /// Pre-world resource phase: rebuild/redress the private clone and advance + /// its mesh/composite readiness BEFORE world draws can consume the + /// bounded per-frame composite-upload budget. In a dense scene the late + /// presentation phase never wins that budget, which is why the doll was + /// visible only while portal space quiesced the world (#443). + /// + public void PrepareResources() { if (_dirty) { if (_factory.TryBuild(out WorldEntity? doll)) { - // Same-generation CreateObject refreshes can repeat the exact - // player ObjDesc at a portal boundary. Retail redresses its - // private inventory object in place; releasing and reacquiring - // an identical synthetic owner briefly blanks the viewport and - // churns its texture composites. - if (!HasEquivalentAppearance(_doll, doll)) - { - _renderer.SetDoll(doll); - _doll = doll; - } + // Redress every accepted live-player refresh, including an + // appearance-equivalent ObjDesc after portal/relogin. The + // private clone belongs to the current presentation + // generation; retaining the old object merely because its + // pixels compare equal can leave it attached to retired + // mesh/composite readiness that never completes again. + // PrivateEntityViewportRenderer promotes replacements in two + // phases, so the last completed target remains visible until + // this fresh clone is completely drawable. + _renderer.SetDoll(doll); + _doll = doll; _dirty = false; } else @@ -96,16 +108,25 @@ internal sealed class PaperdollFramePresenter : IPrivateEntityViewportFrame // gmPaperDollUI::RedressCreature @ 0x004A3BC0 leaves its // private m_pInventoryObject intact when the SmartBox player // is temporarily unavailable. Keep the successful doll and - // retry this dirty redress on the next visible frame. + // retry this dirty redress on the next frame. } } _renderer.Prepare(); + } + public void Render() + { if (!_view.TryGetVisibleSize(out int width, out int height)) return; - _view.SetTextureHandle(_renderer.Render(width, height)); + // Zero is a transient not-ready result, not a request to erase a + // previously completed paperdoll. Session reset clears explicitly in + // ResetSession; ordinary mesh/composite upload latency keeps the last + // good image instead of intermittently blanking the viewport. + uint textureHandle = _renderer.Render(width, height); + if (textureHandle != 0u) + _view.SetTextureHandle(textureHandle); } /// @@ -115,80 +136,10 @@ internal sealed class PaperdollFramePresenter : IPrivateEntityViewportFrame public void ResetSession() { _renderer.SetDoll(null); + _view.ClearTextureHandle(); _doll = null; _dirty = true; } - - private static bool HasEquivalentAppearance( - WorldEntity? current, - WorldEntity? candidate) - { - if (current is null || candidate is null) - return ReferenceEquals(current, candidate); - if (current.SourceGfxObjOrSetupId != candidate.SourceGfxObjOrSetupId - || current.Scale != candidate.Scale - || current.HiddenPartsMask != candidate.HiddenPartsMask - || current.MeshRefs.Count != candidate.MeshRefs.Count - || current.PartOverrides.Count != candidate.PartOverrides.Count) - { - return false; - } - - for (int i = 0; i < current.MeshRefs.Count; i++) - { - MeshRef left = current.MeshRefs[i]; - MeshRef right = candidate.MeshRefs[i]; - if (left.GfxObjId != right.GfxObjId - || left.PartTransform != right.PartTransform - || !DictionaryEquals( - left.SurfaceOverrides, - right.SurfaceOverrides)) - { - return false; - } - } - - for (int i = 0; i < current.PartOverrides.Count; i++) - { - if (current.PartOverrides[i] != candidate.PartOverrides[i]) - return false; - } - - PaletteOverride? leftPalette = current.PaletteOverride; - PaletteOverride? rightPalette = candidate.PaletteOverride; - if (leftPalette is null || rightPalette is null) - return leftPalette is null && rightPalette is null; - if (leftPalette.BasePaletteId != rightPalette.BasePaletteId - || leftPalette.SubPalettes.Count != rightPalette.SubPalettes.Count) - { - return false; - } - for (int i = 0; i < leftPalette.SubPalettes.Count; i++) - { - if (leftPalette.SubPalettes[i] != rightPalette.SubPalettes[i]) - return false; - } - return true; - } - - private static bool DictionaryEquals( - IReadOnlyDictionary? left, - IReadOnlyDictionary? right) - { - if (left is null || right is null) - return left is null && right is null; - if (left.Count != right.Count) - return false; - foreach ((uint key, uint value) in left) - { - if (!right.TryGetValue(key, out uint rightValue) - || rightValue != value) - { - return false; - } - } - return true; - } } /// Retained-UI visibility and texture publication for the doll view. @@ -226,6 +177,9 @@ internal sealed class RetailPaperdollFrameView : IPaperdollFrameView /// public void SetTextureHandle(uint textureHandle) => _viewport.TextureSlot = UiTextureTableHandle.ToSlot(textureHandle); + + public void ClearTextureHandle() => + _viewport.TextureSlot = GpuTextureSlot.Unassigned; } /// Narrow visibility adapter for the paperdoll's inventory host. diff --git a/src/AcDream.App/Rendering/PrivateEntityViewportRenderer.cs b/src/AcDream.App/Rendering/PrivateEntityViewportRenderer.cs index 84961aab..c395af3f 100644 --- a/src/AcDream.App/Rendering/PrivateEntityViewportRenderer.cs +++ b/src/AcDream.App/Rendering/PrivateEntityViewportRenderer.cs @@ -20,8 +20,8 @@ internal interface IPrivateEntityViewportCamera : ICamera /// /// Shared render-to-texture implementation for the private 3-D creature /// viewports used by paperdoll and examination UI. Each instance owns one -/// , one synthetic render identity, and one -/// balanced texture-owner lease. +/// per encountered GPU flight slot, one +/// synthetic render identity, and one balanced texture-owner lease. /// /// Campaign V slice V6k (V4g's first half). The target used to be a /// hand-rolled FBO, colour texture and depth renderbuffer, and the resulting GL @@ -65,7 +65,6 @@ internal sealed class PrivateEntityViewportRenderer : { private const uint PrivateLandblockId = 0u; - private readonly IGpuDevice _device; private readonly ICurrentGpuFrameSource _frames; /// @@ -94,17 +93,12 @@ internal sealed class PrivateEntityViewportRenderer : /// feature does not exist for them, not just "unused". private readonly EntitySlot? _backdropSlot; - // One stable sampled texture-table slot is part of the retained viewport's - // presentation contract. Rotating the slot with the Vulkan flight index - // made the UI sample a freshly-created/cleared sibling after world reveal. - // The frame submission order already protects this target's write -> sample - // transition; keep its identity stable until resize or disposal. - private IGpuRenderTarget? _target; - private IGpuSampler? _sampler; - private GpuTextureSlot _slot = GpuTextureSlot.Unassigned; - private int _fbW; - private int _fbH; - private bool _hasRenderedScene; + // A target written by frame N cannot also be sampled by an unretired frame + // N-1. Vulkan permits those command buffers to overlap, so one shared image + // is a cross-frame write/read race. The current frame slot selects one + // bounded target + texture-table handle; the retained UI samples that exact + // handle later in the same command buffer. + private readonly PrivateViewportFlightTargets _flightTargets; public PrivateEntityViewportRenderer( IWorldPassScope scope, @@ -127,7 +121,7 @@ internal sealed class PrivateEntityViewportRenderer : _scope = scope ?? throw new ArgumentNullException( nameof(scope), "The viewport must publish a world pass scope to draw into."); - _device = device ?? throw new ArgumentNullException(nameof(device)); + ArgumentNullException.ThrowIfNull(device); _frames = frames ?? throw new ArgumentNullException(nameof(frames)); _dispatcher = dispatcher ?? throw new ArgumentNullException(nameof(dispatcher)); _lightUbo = lightUbo ?? throw new ArgumentNullException(nameof(lightUbo)); @@ -137,6 +131,9 @@ internal sealed class PrivateEntityViewportRenderer : _diagnosticName = string.IsNullOrWhiteSpace(diagnosticName) ? "creature viewport" : diagnosticName; + _flightTargets = new PrivateViewportFlightTargets( + device, + _diagnosticName); IEntityTextureLifetime textureLifetimeChecked = textureLifetime ?? throw new ArgumentNullException(nameof(textureLifetime)); @@ -173,7 +170,17 @@ internal sealed class PrivateEntityViewportRenderer : /// public bool TextureIsBottomUp => false; - public void SetEntity(WorldEntity? entity) => _mainSlot.Set(entity); + public void SetEntity(WorldEntity? entity) + { + _mainSlot.Set(entity); + if (entity is null) + { + // A character-session reset explicitly invalidates the sampled + // scenes. Do not let a replacement that is still uploading expose + // the previous character through any flight target. + _flightTargets.InvalidateCompletedScenes(); + } + } /// /// Advances the private entity's mesh and texture-composite readiness @@ -226,51 +233,50 @@ internal sealed class PrivateEntityViewportRenderer : /// public uint Render(int width, int height) { + if (width <= 0 || height <= 0) + return 0u; + + IGpuFrame frame = _frames.CurrentFrame + ?? throw new InvalidOperationException( + $"The {_diagnosticName} requires an open IGpuFrame (see GpuDeviceFrameLifetime)."); + int frameSlot = frame.SlotIndex; + // #443: acquiring a synthetic mesh reference only schedules CPU // preparation/GPU upload; it does not make the mesh drawable. Keep the - // last completed private scene intact until every drawable mesh in the - // replacement has crossed that upload barrier. On first open there is - // no completed scene, so return zero and let the authored panel art - // show through instead of publishing a freshly-cleared black target. + // current flight slot's last completed private scene intact until every + // drawable mesh in the replacement has crossed that upload barrier. + // On first open there is no completed scene, so return zero and let the + // authored panel art show through instead of publishing a cleared target. bool mainReady = _mainSlot.PrepareForDraw(); bool backdropReady = _backdropSlot?.PrepareForDraw() ?? true; if (!mainReady || !backdropReady) { return _mainSlot.Entity is not null - && _hasRenderedScene - && _slot.IsAssigned - ? UiTextureTableHandle.FromSlot(_slot) + ? _flightTargets.CompletedHandle(frameSlot) : 0u; } WorldEntity? entity = _mainSlot.Entity; - if (entity is null || entity.MeshRefs.Count == 0 || width <= 0 || height <= 0) + if (entity is null || entity.MeshRefs.Count == 0) return 0u; IReadOnlyList drawEntities = BuildDrawEntities( _backdropSlot?.Entity, entity); if (!_dispatcher.PreparePrivateEntityResources(drawEntities)) - { - return _hasRenderedScene && _slot.IsAssigned - ? UiTextureTableHandle.FromSlot(_slot) - : 0u; - } + return _flightTargets.CompletedHandle(frameSlot); - EnsureRenderTarget(width, height); - if (_target is null) + PrivateViewportFlightTargets.TargetSlot? targetSlot = + _flightTargets.Ensure(frameSlot, width, height); + if (targetSlot is null) return 0u; _camera.Aspect = width / (float)height; - IGpuFrame frame = _frames.CurrentFrame - ?? throw new InvalidOperationException( - $"The {_diagnosticName} requires an open IGpuFrame (see GpuDeviceFrameLifetime)."); - using IGpuPassEncoder encoder = frame.BeginPass(new GpuPassDescription { Name = _diagnosticName, Color = new GpuColorAttachment( - Target: _target, + Target: targetSlot.Target, Load: GpuLoadOp.Clear, Store: GpuStoreOp.Store, ClearColor: Vector4.Zero), @@ -304,6 +310,12 @@ internal sealed class PrivateEntityViewportRenderer : null), }; + // #443: a private pass must not append its transforms into the shared + // world transform frame — the default mesh shaders index parallel + // per-instance arrays zero-based, so a non-zero arena base zeroes the + // doll's per-instance opacity and the target stays blank whenever a + // world frame is active. See NextClassicDrawIsPrivatePass. + _dispatcher.NextClassicDrawIsPrivatePass = true; _dispatcher.Draw( _camera, entries, @@ -311,8 +323,8 @@ internal sealed class PrivateEntityViewportRenderer : neverCullLandblockId: PrivateLandblockId, visibleCellIds: null, animatedEntityIds: _animatedIds); - _hasRenderedScene = true; - return UiTextureTableHandle.FromSlot(_slot); + targetSlot.HasRenderedScene = true; + return UiTextureTableHandle.FromSlot(targetSlot.TextureSlot); } /// @@ -363,69 +375,6 @@ internal sealed class PrivateEntityViewportRenderer : }); } - private void EnsureRenderTarget(int width, int height) - { - if (_target is not null && width == _fbW && height == _fbH) - return; - ReleaseRenderTarget(); - - IGpuRenderTarget target; - try - { - target = _device.CreateRenderTarget(new GpuRenderTargetDescription( - _diagnosticName, - width, - height, - GpuTextureFormat.Rgba8UnormRenderTarget, - // Depth24Stencil8, as the hand-rolled renderbuffer was: nothing - // samples it, and the stencil aspect keeps the attachment shape - // the depth/stencil renderers already expect. - GpuTextureFormat.Depth24Stencil8, - SampleCount: 1)); - } - catch (Exception failure) - { - Console.WriteLine( - $"[{_diagnosticName}] render target unavailable ({width}x{height}): {failure.Message}"); - return; - } - - try - { - // The retained UI blits this attachment as an ordinary table entry. - // Linear/clamped is the filtering the hand-rolled colour texture set - // on itself before the §7.1 seam registered it. - _sampler = _device.CreateSampler(GpuSamplerDescription.WorldClamp); - _slot = _device.RegisterTexture(target.ColorTexture, _sampler); - } - catch - { - target.Dispose(); - _sampler = null; - _slot = GpuTextureSlot.Unassigned; - throw; - } - - _target = target; - _fbW = width; - _fbH = height; - } - - private void ReleaseRenderTarget() - { - if (_slot.IsAssigned) - { - _device.ReleaseTextureSlot(_slot); - _slot = GpuTextureSlot.Unassigned; - } - _sampler = null; - _target?.Dispose(); - _target = null; - _fbW = 0; - _fbH = 0; - _hasRenderedScene = false; - } - public void Dispose() { List? failures = null; @@ -447,7 +396,7 @@ internal sealed class PrivateEntityViewportRenderer : } try { - ReleaseRenderTarget(); + _flightTargets.Dispose(); } catch (Exception error) { @@ -462,6 +411,163 @@ internal sealed class PrivateEntityViewportRenderer : } } + /// + /// Bounded render-target ownership keyed by . + /// A frame slot is reopened only after its previous submission retires, so + /// the target selected here can be written and sampled within that frame + /// without racing a different in-flight command buffer. + /// + internal sealed class PrivateViewportFlightTargets : IDisposable + { + internal sealed class TargetSlot( + IGpuRenderTarget target, + GpuTextureSlot textureSlot) + { + internal IGpuRenderTarget Target { get; } = target; + internal GpuTextureSlot TextureSlot { get; } = textureSlot; + internal bool HasRenderedScene { get; set; } + } + + private readonly IGpuDevice _device; + private readonly string _diagnosticName; + private readonly List _slots = []; + private int _width; + private int _height; + private bool _disposed; + + internal PrivateViewportFlightTargets( + IGpuDevice device, + string diagnosticName) + { + _device = device ?? throw new ArgumentNullException(nameof(device)); + _diagnosticName = string.IsNullOrWhiteSpace(diagnosticName) + ? "creature viewport" + : diagnosticName; + } + + internal int AllocatedSlotCount => + _slots.Count(static slot => slot is not null); + + internal TargetSlot? Ensure(int frameSlot, int width, int height) + { + ObjectDisposedException.ThrowIf(_disposed, this); + ArgumentOutOfRangeException.ThrowIfNegative(frameSlot); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height); + + if (_width != 0 && (_width != width || _height != height)) + ReleaseAll(); + + while (_slots.Count <= frameSlot) + _slots.Add(null); + if (_slots[frameSlot] is { } existing) + return existing; + + IGpuRenderTarget target; + try + { + target = _device.CreateRenderTarget( + new GpuRenderTargetDescription( + $"{_diagnosticName}-flight-{frameSlot}", + width, + height, + GpuTextureFormat.Rgba8UnormRenderTarget, + // Depth24Stencil8, as the original private viewport + // renderbuffer was. Nothing samples this attachment. + GpuTextureFormat.Depth24Stencil8, + SampleCount: 1)); + } + catch (Exception failure) + { + Console.WriteLine( + $"[{_diagnosticName}] render target unavailable " + + $"({width}x{height}, flight {frameSlot}): {failure.Message}"); + return null; + } + + try + { + // The device de-duplicates immutable samplers. Retained UI + // blits this target through its ordinary texture-table entry. + IGpuSampler sampler = _device.CreateSampler( + GpuSamplerDescription.WorldClamp); + GpuTextureSlot textureSlot = _device.RegisterTexture( + target.ColorTexture, + sampler); + var created = new TargetSlot(target, textureSlot); + _slots[frameSlot] = created; + _width = width; + _height = height; + return created; + } + catch + { + target.Dispose(); + throw; + } + } + + internal uint CompletedHandle(int frameSlot) + { + if ((uint)frameSlot >= (uint)_slots.Count + || _slots[frameSlot] is not { HasRenderedScene: true } slot) + { + return 0u; + } + + return UiTextureTableHandle.FromSlot(slot.TextureSlot); + } + + internal void InvalidateCompletedScenes() + { + for (int i = 0; i < _slots.Count; i++) + { + if (_slots[i] is { } slot) + slot.HasRenderedScene = false; + } + } + + private void ReleaseAll() + { + List? failures = null; + for (int i = 0; i < _slots.Count; i++) + { + TargetSlot? slot = _slots[i]; + if (slot is null) + continue; + try + { + _device.ReleaseTextureSlot(slot.TextureSlot); + } + catch (Exception error) + { + (failures ??= []).Add(error); + } + try + { + slot.Target.Dispose(); + } + catch (Exception error) + { + (failures ??= []).Add(error); + } + } + _slots.Clear(); + _width = 0; + _height = 0; + if (failures is { Count: > 0 }) + throw new AggregateException(failures); + } + + public void Dispose() + { + if (_disposed) + return; + _disposed = true; + ReleaseAll(); + } + } + /// /// One private entity's mesh-reference/texture-owner lifetime, independent /// of every other slot on the renderer. Publication is two-phase: a candidate owns its mesh diff --git a/src/AcDream.App/Rendering/PrivatePresentationRenderer.cs b/src/AcDream.App/Rendering/PrivatePresentationRenderer.cs index 1a3d5db8..fa59bf91 100644 --- a/src/AcDream.App/Rendering/PrivatePresentationRenderer.cs +++ b/src/AcDream.App/Rendering/PrivatePresentationRenderer.cs @@ -16,6 +16,19 @@ internal interface IPrivateEntityViewportFrame void Render(); } +/// +/// Optional pre-world resource preparation for a private viewport whose +/// visibility must not compete with world composite uploads. The composite +/// upload budget opens with the frame's resource phase; a dense world can +/// consume all of it every frame, so a viewport that only prepares during +/// late presentation can starve indefinitely (#443's paperdoll: visible in +/// portal space — where the world is quiesced — and nowhere busy). +/// +internal interface IPrivateEntityViewportResourcePreparation +{ + void PrepareResources(); +} + internal interface IRetainedGameplayUiFrame { void Render(double deltaSeconds, int width, int height); diff --git a/src/AcDream.App/Rendering/RenderFramePreparationController.cs b/src/AcDream.App/Rendering/RenderFramePreparationController.cs index 82306d67..820ddaa1 100644 --- a/src/AcDream.App/Rendering/RenderFramePreparationController.cs +++ b/src/AcDream.App/Rendering/RenderFramePreparationController.cs @@ -29,20 +29,29 @@ internal sealed class RenderFramePreparationController : IRenderFrameResourcePha private readonly IRenderFrameResourcePhase _resources; private readonly IDevToolsFrameLifecycle? _devTools; private readonly IRenderWeatherFramePhase _weather; + private readonly IPrivateEntityViewportResourcePreparation? _privateViewports; public RenderFramePreparationController( IRenderFrameResourcePhase resources, IDevToolsFrameLifecycle? devTools, - IRenderWeatherFramePhase weather) + IRenderWeatherFramePhase weather, + IPrivateEntityViewportResourcePreparation? privateViewports = null) { _resources = resources ?? throw new ArgumentNullException(nameof(resources)); _devTools = devTools; _weather = weather ?? throw new ArgumentNullException(nameof(weather)); + _privateViewports = privateViewports; } public void Prepare(RenderFrameInput input) { _resources.Prepare(input); + // The composite upload budget opens with the resource phase. Give the + // paperdoll's private object its prewarm slot before the world can + // consume the complete per-frame budget (#443 — the doll rendered + // only while portal space quiesced the world); presentation samples + // the result later, after the world pass has closed. + _privateViewports?.PrepareResources(); _devTools?.BeginFrame((float)input.DeltaSeconds); _weather.Tick(input.DeltaSeconds); } diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.Rhi.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.Rhi.cs index 30b22946..2d9fac4b 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.Rhi.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.Rhi.cs @@ -924,6 +924,24 @@ public sealed unsafe partial class WbDrawDispatcher return new RhiSection(allocation.Buffer, allocation.OffsetBytes, (uint)byteCount); } + /// + /// #443 — the next classic draw is a PRIVATE pass (paperdoll, appraisal, + /// chargen preview) and must take the plain ring transform path with + /// firstInstance = 0, never an append into the shared world + /// transform frame. The default mesh shaders index every parallel + /// per-instance array (clip slots, light sets, indoor, opacity, selection, + /// detail category) zero-based — only the packed world submission's + /// shader convention subtracts a shared-arena prefix — so an + /// arena-appended classic draw with a non-zero base reads zeroed + /// per-instance data (opacity 0 ⇒ an invisible doll whenever a world + /// frame is active; portal space worked only because the arena was + /// inactive there). The private pass owns its own camera, lighting, and + /// target; per the self-contained-render-state rule it must not depend on + /// the world frame's pose address space at all. Consumed and cleared by + /// the next . + /// + internal bool NextClassicDrawIsPrivatePass; + private RhiSection WriteWorldTransformSection( IGpuFrame frame, ReadOnlySpan matrixFloats, @@ -939,7 +957,9 @@ public sealed unsafe partial class WbDrawDispatcher ObserveOrdinaryTransformDemand( frame.Serial, checked((uint)(matrixFloats.Length / 16))); - if (!_worldTransformFrames.IsActive) + bool privatePass = NextClassicDrawIsPrivatePass; + NextClassicDrawIsPrivatePass = false; + if (privatePass || !_worldTransformFrames.IsActive) { firstInstance = 0; return WriteRingSection(frame, matrixFloats); diff --git a/tests/AcDream.App.Tests/Rendering/PaperdollFramePresenterTests.cs b/tests/AcDream.App.Tests/Rendering/PaperdollFramePresenterTests.cs index 41f47c47..5b4ee653 100644 --- a/tests/AcDream.App.Tests/Rendering/PaperdollFramePresenterTests.cs +++ b/tests/AcDream.App.Tests/Rendering/PaperdollFramePresenterTests.cs @@ -8,6 +8,14 @@ namespace AcDream.App.Tests.Rendering; public sealed class PaperdollFramePresenterTests { + /// One frame in production order: the pre-world resource phase + /// (build/redress + prewarm) then the late presentation phase. + private static void Frame(PaperdollFramePresenter presenter) + { + presenter.PrepareResources(); + presenter.Render(); + } + [Fact] public void HiddenView_BuildsAndPrewarmsWithoutRendering() { @@ -16,7 +24,7 @@ public sealed class PaperdollFramePresenterTests var factory = new RecordingFactory { Doll = CreateDoll() }; var presenter = new PaperdollFramePresenter(renderer, view, factory); - presenter.Render(); + Frame(presenter); Assert.False(presenter.IsDirty); Assert.Equal(1, factory.BuildCount); @@ -34,7 +42,7 @@ public sealed class PaperdollFramePresenterTests var factory = new RecordingFactory { Doll = doll }; var presenter = new PaperdollFramePresenter(renderer, view, factory); - presenter.Render(); + Frame(presenter); Assert.False(presenter.IsDirty); Assert.Equal(1, factory.BuildCount); @@ -54,20 +62,25 @@ public sealed class PaperdollFramePresenterTests }; var presenter = new PaperdollFramePresenter(renderer, view, factory); - presenter.Render(); - presenter.Render(); + Frame(presenter); + Frame(presenter); presenter.MarkDirty(); - presenter.Render(); + Frame(presenter); Assert.Equal(2, factory.BuildCount); - Assert.Single(renderer.Dolls); + Assert.Equal(2, renderer.Dolls.Count); Assert.Equal(3, renderer.RenderCount); Assert.False(presenter.IsDirty); } [Fact] - public void EquivalentPortalRefresh_KeepsPrivateDollAndTextureOwner() + public void PortalRefresh_RedressesEvenAnEquivalentAppearance() { + // #443: the private clone belongs to the current presentation + // generation. Retaining an old clone because its pixels compare equal + // can pin retired mesh/composite readiness that never completes + // again; the renderer's two-phase promote keeps the last completed + // image visible while the fresh clone becomes drawable. WorldEntity first = CreateDoll(); WorldEntity repeated = CreateDoll(); var renderer = new RecordingRenderer(); @@ -75,13 +88,13 @@ public sealed class PaperdollFramePresenterTests var factory = new RecordingFactory { Doll = first }; var presenter = new PaperdollFramePresenter(renderer, view, factory); - presenter.Render(); + Frame(presenter); factory.Doll = repeated; presenter.MarkDirty(); - presenter.Render(); + Frame(presenter); Assert.Equal(2, factory.BuildCount); - Assert.Equal([first], renderer.Dolls); + Assert.Equal([first, repeated], renderer.Dolls); Assert.Equal(2, renderer.RenderCount); Assert.False(presenter.IsDirty); } @@ -96,14 +109,34 @@ public sealed class PaperdollFramePresenterTests var factory = new RecordingFactory { Doll = first }; var presenter = new PaperdollFramePresenter(renderer, view, factory); - presenter.Render(); + Frame(presenter); factory.Doll = changed; presenter.MarkDirty(); - presenter.Render(); + Frame(presenter); Assert.Equal([first, changed], renderer.Dolls); } + [Fact] + public void TransientZeroRender_NeverErasesThePublishedTexture() + { + // #443: zero is a not-ready result (mesh/composite upload latency), + // not a request to blank a completed paperdoll. + var renderer = new RecordingRenderer { TextureHandle = 91u }; + var view = new RecordingView(); + var factory = new RecordingFactory { Doll = CreateDoll() }; + var presenter = new PaperdollFramePresenter(renderer, view, factory); + + Frame(presenter); + renderer.TextureHandle = 0u; + Frame(presenter); + renderer.TextureHandle = 91u; + Frame(presenter); + + Assert.Equal([91u, 91u], view.TextureHandles); + Assert.Equal(0, view.ClearCount); + } + private static WorldEntity CreateDoll(float scale = 1f) => new() { Id = 42u, @@ -123,11 +156,11 @@ public sealed class PaperdollFramePresenterTests var factory = new RecordingFactory { Doll = firstDoll }; var presenter = new PaperdollFramePresenter(renderer, view, factory); - presenter.Render(); + Frame(presenter); factory.CanBuild = false; presenter.MarkDirty(); - presenter.Render(); - presenter.Render(); + Frame(presenter); + Frame(presenter); Assert.True(presenter.IsDirty); Assert.Equal(3, factory.BuildCount); @@ -146,14 +179,17 @@ public sealed class PaperdollFramePresenterTests var factory = new RecordingFactory { Doll = firstDoll }; var presenter = new PaperdollFramePresenter(renderer, view, factory); - presenter.Render(); + Frame(presenter); presenter.ResetSession(); factory.Doll = secondDoll; - presenter.Render(); + Frame(presenter); Assert.False(presenter.IsDirty); Assert.Equal(2, factory.BuildCount); Assert.Equal([firstDoll, null, secondDoll], renderer.Dolls); + // The session boundary is the ONE explicit viewport clear (#443); + // an old character must not linger while the next one uploads. + Assert.Equal(1, view.ClearCount); } [Fact] @@ -252,7 +288,7 @@ public sealed class PaperdollFramePresenterTests private sealed class RecordingRenderer : IPaperdollDollRenderer { - public uint TextureHandle { get; init; } + public uint TextureHandle { get; set; } public List Dolls { get; } = []; public List<(int Width, int Height)> RenderSizes { get; } = []; public int RenderCount => RenderSizes.Count; @@ -285,6 +321,10 @@ public sealed class PaperdollFramePresenterTests public void SetTextureHandle(uint textureHandle) => TextureHandles.Add(textureHandle); + + public int ClearCount { get; private set; } + + public void ClearTextureHandle() => ClearCount++; } private sealed class RecordingFactory : IPaperdollDollFactory diff --git a/tests/AcDream.App.Tests/Rendering/PrivateViewportFlightTargetsTests.cs b/tests/AcDream.App.Tests/Rendering/PrivateViewportFlightTargetsTests.cs new file mode 100644 index 00000000..ac2d7bf5 --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/PrivateViewportFlightTargetsTests.cs @@ -0,0 +1,65 @@ +using AcDream.App.Rendering; +using AcDream.App.Rendering.Gpu; +using AcDream.App.Tests.Rendering.Gpu; + +namespace AcDream.App.Tests.Rendering; + +public sealed class PrivateViewportFlightTargetsTests +{ + [Fact] + public void FlightSlotsOwnDistinctTargetsAndPublishOnlyCompletedScenes() + { + using var device = new RecordingGpuDevice(); + using var targets = + new PrivateEntityViewportRenderer.PrivateViewportFlightTargets( + device, + "paperdoll"); + + var first = Assert.IsType< + PrivateEntityViewportRenderer.PrivateViewportFlightTargets.TargetSlot>( + targets.Ensure(0, 120, 180)); + var second = Assert.IsType< + PrivateEntityViewportRenderer.PrivateViewportFlightTargets.TargetSlot>( + targets.Ensure(1, 120, 180)); + + Assert.NotSame(first.Target, second.Target); + Assert.NotEqual(first.TextureSlot, second.TextureSlot); + Assert.Equal(2, targets.AllocatedSlotCount); + Assert.Equal(0u, targets.CompletedHandle(0)); + Assert.Equal(0u, targets.CompletedHandle(1)); + + first.HasRenderedScene = true; + + Assert.NotEqual(0u, targets.CompletedHandle(0)); + Assert.Equal(0u, targets.CompletedHandle(1)); + Assert.Same(first, targets.Ensure(0, 120, 180)); + + targets.InvalidateCompletedScenes(); + + Assert.Equal(0u, targets.CompletedHandle(0)); + Assert.Equal(2, targets.AllocatedSlotCount); + } + + [Fact] + public void ResizeRetiresEveryFlightTargetBeforeCreatingTheNewExtent() + { + using var device = new RecordingGpuDevice(); + using var targets = + new PrivateEntityViewportRenderer.PrivateViewportFlightTargets( + device, + "paperdoll"); + var first = targets.Ensure(0, 120, 180)!; + var second = targets.Ensure(1, 120, 180)!; + var firstTarget = Assert.IsType(first.Target); + var secondTarget = Assert.IsType(second.Target); + + var resized = targets.Ensure(1, 160, 220)!; + + Assert.True(firstTarget.IsDisposed); + Assert.True(secondTarget.IsDisposed); + Assert.Equal(1, targets.AllocatedSlotCount); + Assert.Equal(160, resized.Target.Description.Width); + Assert.Equal(220, resized.Target.Description.Height); + Assert.Equal(3, device.CreatedRenderTargets.Count); + } +}