diff --git a/docs/ISSUES.md b/docs/ISSUES.md index e9ad4b4b..6ab2b246 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -97,6 +97,63 @@ Copy this block when adding a new issue: --- +## #251 — glClientWaitSync returned 0 and crashed the render loop, once in nine connected runs + +**Status:** OPEN +**Severity:** MEDIUM (one observed occurrence; kills the process when it fires) +**Filed:** 2026-07-28 +**Component:** rendering / GL frame-flight fences + +**Description:** During the Campaign V slice V4t connected gate, one run died +with an unhandled `InvalidOperationException` in `OnRender`: + +``` +OpenGL returned unexpected fence wait status NoError (0x0). + at GpuFrameFlightController.RetireFence(Int32 slot) + at RenderFrameOrchestrator.Render(RenderFrameInput input) + at GameWindow.OnRender(Double deltaSeconds) +``` + +`glClientWaitSync` is specified to return `ALREADY_SIGNALED`, +`TIMEOUT_EXPIRED`, `CONDITION_SATISFIED` or `WAIT_FAILED`. It returned 0, which +is none of those, so `SilkGpuFenceApi.Wait`'s exhaustive switch threw — the +switch is correct and the throw is the right behaviour; the anomaly is the +driver's return value. The run had already reached `world-visible` and +`complete` and had logged a graceful logout; the crash landed while it was +still rendering, before the probe's screenshot, so the gate recorded +`NO-CAPTURE`. Shutdown then reported +`status=AbandonedIncomplete, blocked=submitted GPU work` — the same fence, hit +a second time from `WaitForSubmittedWork` — followed by Silk.NET's +"You cannot call `Reset` inside of the render loop!" from the native fallback. +Both are consequences, not separate defects. + +**Root cause / status:** Unknown, and NOT attributed to V4t. It occurred once +in nine connected runs on 2026-07-28: once in three at the V4t-1 tree, then +zero in three more at that same tree and zero in three interleaved runs at +`cb2a70b8`. The V4t-1 diff creates, deletes and waits on no fence and adds no +retirement registration that executes on that path. A sync object whose handle +stops being valid mid-session, on a clean `glGetError`, is the same +below-the-API failure family the campaign documented four instances of on this +exact driver (AMD 26.6.4, RX 9070 XT) in plan §5.5.1–§5.5.3 — a deadlocking +`glGetQueryObject` read, a never-executed `GL_QUERY_BUFFER` write, a +multisampled `glReadPixels`, and a capture that could not see the presented +surface. That is a hypothesis, not a finding: nothing here rules out a real +double-delete or a lifetime bug in our own fence bookkeeping. + +**Files:** + +- `src/AcDream.App/Rendering/GpuFrameFlightController.cs:274` `RetireFence` +- `src/AcDream.App/Rendering/GpuFrameFlightController.cs:474` `SilkGpuFenceApi.Wait` +- `src/AcDream.App/Rendering/GameWindowLifetime.cs:419` shutdown's `frame flight drain` + +**Acceptance:** Either a reproduction that pins the invalidation to our own +bookkeeping and a fix for it, or — if the driver is confirmed — a decision +recorded here about whether a 0 return should be treated as `WAIT_FAILED` and +retried rather than thrown. Do not silently widen the switch to swallow it: an +unexpected status is exactly the signal §5.5 spent three days wishing it had. + +--- + ## #250 — Zero-allocation tests fail intermittently, roughly 1 run in 3 **Status:** OPEN diff --git a/src/AcDream.App/Composition/WorldRenderComposition.cs b/src/AcDream.App/Composition/WorldRenderComposition.cs index b493e11b..e5a1f74c 100644 --- a/src/AcDream.App/Composition/WorldRenderComposition.cs +++ b/src/AcDream.App/Composition/WorldRenderComposition.cs @@ -116,6 +116,7 @@ internal interface IWorldRenderCompositionFactory BindlessSupport bindless, Shader shader, TerrainAtlas atlas, + AcDream.App.Rendering.Gpu.IGpuDevice gpuDevice, IGpuResourceRetirementQueue retirement); /// /// The built terrain atlas, or null on a backend that has none. The atlas is @@ -265,8 +266,19 @@ internal sealed class RetailWorldRenderCompositionFactory BindlessSupport bindless, Shader shader, TerrainAtlas atlas, + AcDream.App.Rendering.Gpu.IGpuDevice gpuDevice, IGpuResourceRetirementQueue retirement) => - new(gl, bindless, shader, atlas, retirement); + // Campaign V slice V4t: terrain's atlas slots live in the device's one + // texture table. This renderer only exists on GL — the `gl is not null` + // gate at its call site is the same one — so the backend cast is a + // statement of that fact rather than a narrowing. + new( + gl, + bindless, + shader, + atlas, + (AcDream.App.Rendering.Gpu.Gl.GlGpuDevice)gpuDevice, + retirement); public WorldTerrainBuildContext CreateTerrainBuildContext( uint initialCenterLandblockId, @@ -570,6 +582,7 @@ internal sealed class WorldRenderCompositionPhase bindless!, terrainShader!, terrainAtlas!, + _dependencies.GpuDevice, _dependencies.ResourceRetirement), _publication.PublishTerrain, WorldRenderCompositionPoint.TerrainPublished); diff --git a/src/AcDream.App/Rendering/Gpu/Gl/GlGpuDevice.cs b/src/AcDream.App/Rendering/Gpu/Gl/GlGpuDevice.cs index 6213d73e..ae25fd5c 100644 --- a/src/AcDream.App/Rendering/Gpu/Gl/GlGpuDevice.cs +++ b/src/AcDream.App/Rendering/Gpu/Gl/GlGpuDevice.cs @@ -313,6 +313,17 @@ internal sealed class GlGpuDevice : IGpuDevice $"ring slot {slotIndex}"); } + FlushTextureTable(); + } + + /// + /// Drains every dirty run of the texture table into its GL buffer. Called by + /// for RHI draws, and directly by the still-raw-GL + /// world renderers before their own draws — see the V4t remarks on + /// . + /// + internal void FlushTextureTable() + { while (_textureTableDirtySlots.TryTakeNextRun(out uint firstSlot, out uint slotCount)) { ReadOnlySpan bytes = MemoryMarshal.AsBytes( @@ -438,6 +449,69 @@ internal sealed class GlGpuDevice : IGpuDevice return _externalColorTextureGlNamesBySlot.TryGetValue(slot.Index, out glTextureName); } + // ── V4t transitional seam: the world texture stack's entry to this table ── + // + // Campaign V slice V4t moves the world's CPU data model off the raw 64-bit + // ARB_bindless_texture handle and onto GpuTextureSlot, but §5.5.6 closed the + // GL re-land of V4c/V4d, so WbDrawDispatcher, EnvCellRenderer, + // TerrainModernRenderer and ParticleRenderer still submit through raw GL and + // cannot reach FlushBeforeDraw. They therefore call FlushTextureTable and + // bind TextureTableGlName at GpuBindingModel.StorageTextureTable themselves, + // immediately before their own draws — the same shape their retired private + // GlBindlessHandleTable had, against the one table that is now the device's. + // + // Residency ownership does NOT move. Each world texture is created, made + // resident and destroyed by its own cache (TerrainAtlas, + // CompositeTextureArrayCache, StandaloneBindlessTextureCache, + // ManagedGLTextureArray); this device only owns the table entry, keyed 1:1 by + // the caller's already-resident handle. That is what separates this from + // RegisterTexture, which owns the residency it creates. + // + // Deleted with the raw-GL world path when the Vulkan world arm lands, at + // which point every one of those caches registers through RegisterTexture. + private readonly Dictionary _worldTextureSlotsByHandle = new(); + + /// + /// Interns an already-resident world texture handle into the device's table + /// and returns its slot. Idempotent: the same handle always resolves to the + /// same slot until retires it, which + /// is what lets a cache call this per draw rather than tracking the slot. + /// + internal GpuTextureSlot RegisterWorldTextureHandle(ulong residentHandle) + { + ThrowIfDisposed(); + if (residentHandle == 0) + return GpuTextureSlot.Unassigned; + if (_worldTextureSlotsByHandle.TryGetValue(residentHandle, out GpuTextureSlot existing)) + return existing; + + uint slotIndex = _textureSlotAllocator.Allocate(); + WriteHandle(slotIndex, residentHandle); + var slot = new GpuTextureSlot(slotIndex); + _worldTextureSlotsByHandle.Add(residentHandle, slot); + return slot; + } + + /// + /// Retires the table entry for a world handle the caller is about to make + /// non-resident. The slot itself returns to the free list only once the + /// retirement queue confirms no submitted frame can still read it, exactly + /// as for . A handle that was never + /// registered is a no-op, so a cache may call this unconditionally on its + /// teardown path. + /// + internal void ReleaseWorldTextureHandle(ulong residentHandle) + { + if (residentHandle == 0) + return; + if (!_worldTextureSlotsByHandle.Remove(residentHandle, out GpuTextureSlot slot)) + return; + ReleaseTextureSlot(slot); + } + + /// Live world-handle registrations. Diagnostics and tests only. + internal int WorldTextureSlotCount => _worldTextureSlotsByHandle.Count; + internal void ApplyRenderState(GlRenderStateSnapshot desired) { GlRenderStateChanges changes = _renderState.Apply(desired); diff --git a/src/AcDream.App/Rendering/TerrainAtlas.cs b/src/AcDream.App/Rendering/TerrainAtlas.cs index e4c7ad59..aff1a281 100644 --- a/src/AcDream.App/Rendering/TerrainAtlas.cs +++ b/src/AcDream.App/Rendering/TerrainAtlas.cs @@ -1,3 +1,5 @@ +using AcDream.App.Rendering.Gpu; +using AcDream.App.Rendering.Gpu.Gl; using AcDream.Core.Textures; using DatReaderWriter; using AcDream.Content; @@ -66,20 +68,49 @@ public sealed unsafe class TerrainAtlas : IDisposable private readonly RestoredTextureBindingMutation _anisotropyBindingMutation = new(); private ResourceShutdownTransaction? _shutdown; + private ulong _registeredTerrainHandle; + private ulong _registeredAlphaHandle; + private GpuTextureSlot _terrainSlot = GpuTextureSlot.Unassigned; + private GpuTextureSlot _alphaSlot = GpuTextureSlot.Unassigned; + /// - /// Get 64-bit bindless handles for the terrain + alpha texture arrays. - /// Throws if the atlas was constructed - /// without a instance. Handles are generated - /// lazily on first call and cached for the atlas's lifetime; both textures - /// are made resident. + /// Campaign V slice V4t: the device texture-table slots for the terrain and + /// alpha arrays — the backend-neutral replacement for the raw 64-bit + /// ARB_bindless_texture handles this used to return. Residency still + /// belongs to this atlas (acquired lazily here, released by + /// ); the device owns only the two table entries. + /// + /// Throws if the atlas was + /// constructed without a instance. + /// + /// makes both textures non-resident and + /// re-acquires them, which yields new handles. Re-registering is therefore + /// conditional on the handle actually having changed, and the superseded + /// table entry is retired in the same step — otherwise a quality-preset + /// change would leak a slot holding a non-resident handle. Registration is + /// idempotent, so the common per-draw call is two dictionary lookups. /// - public (ulong terrain, ulong alpha) GetBindlessHandles() + internal (GpuTextureSlot Terrain, GpuTextureSlot Alpha) GetTextureSlots(GlGpuDevice device) { + ArgumentNullException.ThrowIfNull(device); if (_bindless is null) throw new InvalidOperationException( - "TerrainAtlas was constructed without BindlessSupport; cannot return bindless handles."); + "TerrainAtlas was constructed without BindlessSupport; cannot return texture slots."); + (ulong terrain, ulong alpha) = _bindlessHandles!.Acquire(); - return (terrain, alpha); + if (terrain != _registeredTerrainHandle) + { + device.ReleaseWorldTextureHandle(_registeredTerrainHandle); + _terrainSlot = device.RegisterWorldTextureHandle(terrain); + _registeredTerrainHandle = terrain; + } + if (alpha != _registeredAlphaHandle) + { + device.ReleaseWorldTextureHandle(_registeredAlphaHandle); + _alphaSlot = device.RegisterWorldTextureHandle(alpha); + _registeredAlphaHandle = alpha; + } + return (_terrainSlot, _alphaSlot); } private TerrainAtlas( diff --git a/src/AcDream.App/Rendering/TerrainModernRenderer.cs b/src/AcDream.App/Rendering/TerrainModernRenderer.cs index 28d1482f..4d710233 100644 --- a/src/AcDream.App/Rendering/TerrainModernRenderer.cs +++ b/src/AcDream.App/Rendering/TerrainModernRenderer.cs @@ -1,5 +1,6 @@ using System.Numerics; using AcDream.App.Rendering.Gpu; +using AcDream.App.Rendering.Gpu.Gl; using AcDream.App.Rendering.Wb; using AcDream.Core.Terrain; using Silk.NET.OpenGL; @@ -96,13 +97,13 @@ public sealed unsafe class TerrainModernRenderer : IDisposable // std140 packing and why it is vec4[9] rather than float[36]. private uint _tilingUbo; - // GL-only emulation of the eventual Vulkan global texture descriptor array - // (binding=9, GpuBindingModel.StorageTextureTable). Owns its own table — - // see GlBindlessHandleTable's doc comment and the campaign doc's §5.2 for - // why terrain doesn't share WbDrawDispatcher's/EnvCellRenderer's tables. - private readonly GlBindlessHandleTable _textureTable = new(); - private uint _textureTableSsbo; - private int _textureTableSsboCapacityBytes; + // Campaign V slice V4t: the interim per-renderer GlBindlessHandleTable is + // retired. TerrainAtlas hands out GpuTextureSlots from the device's one + // table and this renderer flushes and binds that table at + // GpuBindingModel.StorageTextureTable itself, because it still submits + // through raw GL and so never reaches GlGpuDevice.FlushBeforeDraw. Null on + // a backend with no GL device, where this renderer is never constructed. + private readonly GlGpuDevice? _gpuDevice; // Reusable per-frame buffers. private readonly List _visibleSlots = new(); @@ -122,17 +123,19 @@ public sealed unsafe class TerrainModernRenderer : IDisposable public void BeginVisibilityFrame() => _visibleCellIds.Clear(); - public TerrainModernRenderer( + internal TerrainModernRenderer( GL gl, BindlessSupport bindless, Shader shader, TerrainAtlas atlas, + GlGpuDevice? gpuDevice, int initialSlotCapacity = 64) : this( gl, bindless, shader, atlas, + gpuDevice, ImmediateGpuResourceRetirementQueue.Instance, initialSlotCapacity) { @@ -143,6 +146,7 @@ public sealed unsafe class TerrainModernRenderer : IDisposable BindlessSupport bindless, Shader shader, TerrainAtlas atlas, + GlGpuDevice? gpuDevice, IGpuResourceRetirementQueue resourceRetirement, int initialSlotCapacity = 64) { @@ -150,6 +154,7 @@ public sealed unsafe class TerrainModernRenderer : IDisposable _bindless = bindless; _shader = shader; _atlas = atlas; + _gpuDevice = gpuDevice; ArgumentNullException.ThrowIfNull(resourceRetirement); _retirementLedger = new GpuRetirementLedger(resourceRetirement); _alloc = new GpuRetiredTerrainSlotAllocator(initialSlotCapacity, resourceRetirement); @@ -184,21 +189,6 @@ public sealed unsafe class TerrainModernRenderer : IDisposable BufferUsageARB.StaticDraw, "allocating terrain tiling UBO"); - // Campaign V slice V2b: binding=9 texture-table SSBO (GL-only - // emulation of the eventual Vulkan descriptor array). - _textureTableSsbo = TrackedGlResource.CreateBuffer( - _gl, - "creating terrain texture-table SSBO"); - RetryableGpuResourceRelease textureTableRelease = - TrackedGlResource.CreateRetryableBufferDeletion( - _gl, - _textureTableSsbo, - () => _textureTableSsboCapacityBytes, - "rolling back terrain texture-table SSBO"); - constructionResources.Add( - "terrain texture-table SSBO", - textureTableRelease.Run); - _globalVao = TrackedGlResource.CreateVertexArray( _gl, "creating terrain global VAO"); @@ -542,16 +532,16 @@ public sealed unsafe class TerrainModernRenderer : IDisposable // the same product the visibility pass above already computed. _shader.SetMatrix4("uViewProjection", viewProjection); - var (terrainHandle, alphaHandle) = _atlas.GetBindlessHandles(); - // Campaign V slice V2b: pass each handle's binding=9 table slot + // Campaign V slice V2b: pass each texture's binding=9 table slot // instead of the raw uvec2 handle. GLSL reconstructs // sampler2DArray(ACDREAM_TEXTURE_HANDLE(uTextureIndexA)) at the use - // site — see terrain_modern.frag. - uint terrainSlot = _textureTable.GetOrAdd(terrainHandle); - uint alphaSlot = _textureTable.GetOrAdd(alphaHandle); + // site — see terrain_modern.frag. Slice V4t: the slots come from the + // device's one table rather than a table private to this renderer. + (GpuTextureSlot terrainSlot, GpuTextureSlot alphaSlot) = + _atlas.GetTextureSlots(GpuDevice); FlushAndBindTextureTable(); - _gl.ProgramUniform1(_shader.Program, _uTextureIndexALoc, terrainSlot); - _gl.ProgramUniform1(_shader.Program, _uTextureIndexBLoc, alphaSlot); + _gl.ProgramUniform1(_shader.Program, _uTextureIndexALoc, terrainSlot.Index); + _gl.ProgramUniform1(_shader.Program, _uTextureIndexBLoc, alphaSlot.Index); // Phase U.3: bind the terrain clip UBO (binding=2). Shared ClipFrame UBO // when wired, else the no-clip fallback (count 0 = ungated terrain). @@ -665,16 +655,6 @@ public sealed unsafe class TerrainModernRenderer : IDisposable "deleting terrain tiling UBO"); releases.Add(("tiling-ubo", release.Run)); } - if (_textureTableSsbo != 0) - { - RetryableGpuResourceRelease release = - TrackedGlResource.CreateRetryableBufferDeletion( - _gl, - _textureTableSsbo, - _textureTableSsboCapacityBytes, - "deleting terrain texture-table SSBO"); - releases.Add(("texture-table", release.Run)); - } _disposeResources = new RetryableResourceReleaseLedger(releases); } @@ -696,8 +676,6 @@ public sealed unsafe class TerrainModernRenderer : IDisposable _indirectCapacity = 0; _dynamicFrameStarted = false; _fallbackClipUbo = 0; - _textureTableSsbo = 0; - _textureTableSsboCapacityBytes = 0; _disposeResources = null; _disposed = true; @@ -762,43 +740,34 @@ public sealed unsafe class TerrainModernRenderer : IDisposable } /// - /// Campaign V slice V2b: uploads 's handles to - /// when a new one was registered since the - /// last flush, then (re)binds it at - /// . Terrain registers at - /// most two handles per draw (the terrain and alpha atlases), so this is - /// dirty only on the atlas's first draw and stays clean afterward. + /// The GL device whose texture table this renderer samples through. + /// Campaign V slice V4t: a terrain renderer without one could not resolve a + /// single texture, so the failure names the composition that built it rather + /// than dereferencing null mid-draw. /// - private unsafe void FlushAndBindTextureTable() + private GlGpuDevice GpuDevice => _gpuDevice ?? throw new InvalidOperationException( + "TerrainModernRenderer was constructed without a GL GPU device: its texture " + + "slots come from that device's table (Campaign V slice V4t)."); + + /// + /// Campaign V slice V4t: drains the device texture table's dirty runs and + /// (re)binds it at . + /// Terrain registers at most two slots (the terrain and alpha atlases), so + /// the table is dirty only on the atlas's first draw — but the bind is + /// unconditional, because GL storage-buffer binding points are global and + /// another renderer's binding 9 sits there between two terrain draws. + /// Deleted with the raw-GL world path when the Vulkan world arm lands and + /// this renderer's draws go through the encoder, which binds the same table + /// on every pipeline bind. + /// + private void FlushAndBindTextureTable() { - if (_textureTable.Dirty) - { - ReadOnlySpan handles = _textureTable.Handles; - int byteCount = handles.Length * sizeof(ulong); - fixed (ulong* p = handles) - { - if (_textureTableSsboCapacityBytes < byteCount) - { - int grown = DynamicBufferCapacity.Grow(_textureTableSsboCapacityBytes, byteCount); - TrackedGlResource.AllocateBufferStorage( - _gl, - GLEnum.ShaderStorageBuffer, - _textureTableSsbo, - _textureTableSsboCapacityBytes, - grown, - GLEnum.DynamicDraw, - "growing terrain texture-table SSBO"); - _textureTableSsboCapacityBytes = grown; - } - _gl.BindBuffer(GLEnum.ShaderStorageBuffer, _textureTableSsbo); - _gl.BufferSubData(GLEnum.ShaderStorageBuffer, 0, (nuint)byteCount, p); - } - _textureTable.MarkFlushed(); - } + GlGpuDevice device = GpuDevice; + device.FlushTextureTable(); _gl.BindBufferBase( GLEnum.ShaderStorageBuffer, GpuBindingModel.StorageTextureTable, - _textureTableSsbo); + device.TextureTableGlName); } /// diff --git a/tests/AcDream.App.Tests/Composition/WorldRenderCompositionTests.cs b/tests/AcDream.App.Tests/Composition/WorldRenderCompositionTests.cs index 4b76c611..956db65a 100644 --- a/tests/AcDream.App.Tests/Composition/WorldRenderCompositionTests.cs +++ b/tests/AcDream.App.Tests/Composition/WorldRenderCompositionTests.cs @@ -271,6 +271,7 @@ public sealed class WorldRenderCompositionTests BindlessSupport bindless, Shader shader, TerrainAtlas atlas, + IGpuDevice gpuDevice, IGpuResourceRetirementQueue retirement) => Resource("terrain");