From fac0940711238e4382ec6ba0b9b6483dfe7d34fb Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 28 Jul 2026 10:11:00 +0200 Subject: [PATCH] feat(render): Campaign V slice V6f-2 - terrain's tiling table becomes a buffer terrain_modern.frag declared `uniform float uTexTiling[36]` - the per-layer tiling factors retail passes to TexMerge::CopyAndTile / TexMerge::Merge, one per terrain atlas layer. Vulkan GLSL has no default uniform block, so a loose array is unspellable there, and 144 bytes of payload cannot ride the pinned 96-byte push-constant block. GpuBindingModel reserved UniformTerrainTiling (binding 3) for exactly this at slice V4d. The array now lives in that block. The ELEMENT TYPE is deliberately unchanged. std140 pads every array element out to 16 bytes, so the block is 576 bytes rather than 144, and packing four values per vec4 would be tighter - but it would also rewrite the accessor and every use site, and this commit's whole value is that its pixel gate measures the move to a uniform buffer and nothing else. `uTexTiling[int(layer)]` reads exactly as it did. That padding is the hazard the change introduces, so it is pinned twice. The CPU writer walks TerrainTextureTilingTable.UniformElementStrideBytes and zero-fills the dead words rather than blitting 36 packed floats, and a new test asserts the stride is 16, the block is 576, and the two are consistent with LayerCapacity. A tightly-packed writer would not crash or even look obviously wrong: the shader would read layer 0's factor for layers 0-3, layer 4's for 4-7, and in a scene where most layers tile at 1 the error stays invisible until a layer that does not appears. Nothing else in the suite could see that. The buffer is allocated once in the constructor, through the same TrackedGlResource + ResourceCleanupGroup rollback path every other terrain buffer uses, written on the first bound draw - preserving the upload-once property the linked program's uniform had for free - and released through the dispose ledger. It is REBOUND every draw rather than once: GL's uniform-buffer binding points are global and shared with SceneLighting at 1 and the sky's params at 4, so a renderer running between two terrain draws can take binding 3 out from under us. Self-contained render state, per the standing rule. Gates. Release build clean. App tests 4,073 passed / 3 skipped - the baseline 4,072 plus the new layout test. Offline pixel gate against 5e13b45f: 21 differing pixels of 563,200 compared (fraction 3.73e-05), inside the documented 15-23 pixel band and ~27x under the 0.001 threshold. This gate is a real test of the layout rather than a formality: terrain blending, road overlays and the water edge are most of the captured frame, and every one of those samples goes through terrainTiling(), so a stride mismatch would have shown as a wholesale retexture rather than as noise. The gate run's client log has zero exceptions and an empty stderr. Manifest regenerated in the same commit. terrain_modern's remaining Vulkan error moved from `'uTexTiling' : undeclared identifier` to the frag's direct `sampler2DArray(...)` construction, which is the same dialect migration V6e ran for mesh_modern and which lands next. The pair count is unchanged at 7/9. No divergence-register row: the tiling values, their source and their use are unchanged, and no retail-facing behaviour moves. Co-Authored-By: Claude Fable 5 --- .../Shaders/spv/shaders.manifest.json | 4 +- .../Rendering/Shaders/terrain_modern.frag | 19 ++++- .../Rendering/TerrainModernRenderer.cs | 82 +++++++++++++++++-- .../Rendering/TerrainTextureTilingTable.cs | 19 +++++ .../TerrainTextureTilingTableTests.cs | 38 ++++++++- 5 files changed, 150 insertions(+), 12 deletions(-) diff --git a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json index 0bd707cb..15d29592 100644 --- a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json +++ b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json @@ -110,9 +110,9 @@ }, { "stage": "frag", - "sourceSha256": "6003b81df6da6cbea7f00310bd956348bc7b2525345dd490b0b6a3b6428340d9", + "sourceSha256": "dc4d38b5eb356629c83681fbc6a1d5d867792e99a8c4e659031f390333edb7b8", "compiled": false, - "message": "terrain_modern.frag:110: error: \u0027uTexTiling\u0027 : undeclared identifier" + "message": "terrain_modern.frag:150: error: \u0027sampler2DArray\u0027 : sampler-constructor requires the extension GL_ARB_bindless_texture enabled" } ] }, diff --git a/src/AcDream.App/Rendering/Shaders/terrain_modern.frag b/src/AcDream.App/Rendering/Shaders/terrain_modern.frag index d1ceb477..2e527308 100644 --- a/src/AcDream.App/Rendering/Shaders/terrain_modern.frag +++ b/src/AcDream.App/Rendering/Shaders/terrain_modern.frag @@ -35,10 +35,27 @@ out vec4 fragColor; // push-constant plumbing yet, so these stay plain uniforms for now. uniform uint uTextureIndexA; uniform uint uTextureIndexB; -uniform float uTexTiling[36]; #define uTerrain sampler2DArray(ACDREAM_TEXTURE_HANDLE(uTextureIndexA)) #define uAlpha sampler2DArray(ACDREAM_TEXTURE_HANDLE(uTextureIndexB)) +// Campaign V slice V6f-2: the 36 per-layer tiling factors moved out of a loose +// `uniform float uTexTiling[36]` and into the uniform buffer GpuBindingModel +// reserved binding 3 for. Vulkan GLSL has no default uniform block, so the loose +// array was unspellable there, and at 144 bytes of payload it cannot ride the +// 96-byte push-constant block either. A uniform buffer is the only legal home, +// and the same declaration is legal in both dialects. +// +// The ELEMENT TYPE is deliberately unchanged. std140 pads every array element to +// 16 bytes, so the block is 576 bytes rather than 144, and packing four floats +// per vec4 would be tighter — but it would also change every use site below. +// Keeping `float uTexTiling[36]` means `uTexTiling[int(layer)]` reads exactly as +// it did before, so this commit's pixel gate is measuring the move to a uniform +// buffer and nothing else. TerrainTextureTilingTable.UniformBufferBytes and the +// stride beside it are the CPU half of this layout. +layout(std140, ACDREAM_UBO_SET binding = 3) uniform TerrainTiling { + float uTexTiling[36]; +}; + struct Light { vec4 posAndKind; vec4 dirAndRange; diff --git a/src/AcDream.App/Rendering/TerrainModernRenderer.cs b/src/AcDream.App/Rendering/TerrainModernRenderer.cs index 1308248f..28d1482f 100644 --- a/src/AcDream.App/Rendering/TerrainModernRenderer.cs +++ b/src/AcDream.App/Rendering/TerrainModernRenderer.cs @@ -87,9 +87,15 @@ public sealed unsafe class TerrainModernRenderer : IDisposable // uniform locations (matrix uniforms are set by name via Shader.SetMatrix4). private int _uTextureIndexALoc; private int _uTextureIndexBLoc; - private int _uTexTilingLoc; private bool _textureTilingUploaded; + // Campaign V slice V6f-2: the 36 per-layer tiling factors used to be a loose + // `uniform float uTexTiling[36]`, which Vulkan GLSL cannot declare at all. + // They now live in the uniform buffer GpuBindingModel reserved + // UniformTerrainTiling (binding 3) for — see terrain_modern.frag for the + // 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 @@ -151,13 +157,33 @@ public sealed unsafe class TerrainModernRenderer : IDisposable _uTextureIndexALoc = _gl.GetUniformLocation(_shader.Program, "uTextureIndexA"); _uTextureIndexBLoc = _gl.GetUniformLocation(_shader.Program, "uTextureIndexB"); - _uTexTilingLoc = _gl.GetUniformLocation(_shader.Program, "uTexTiling[0]"); - if (_uTexTilingLoc < 0) - throw new InvalidOperationException("terrain_modern.frag is missing the required uTexTiling uniform."); var constructionResources = new ResourceCleanupGroup(); try { + // Campaign V slice V6f-2: the tiling UBO. Fixed size — the table is + // 36 immutable floats packed four to a vec4 — so it is allocated + // once here and written once on the first bound draw, which is the + // same cadence the glUniform1fv it replaces already had. + _tilingUbo = TrackedGlResource.CreateBuffer( + _gl, + "creating terrain tiling UBO"); + RetryableGpuResourceRelease tilingUboRelease = + TrackedGlResource.CreateRetryableBufferDeletion( + _gl, + _tilingUbo, + TerrainTextureTilingTable.UniformBufferBytes, + "rolling back terrain tiling UBO"); + constructionResources.Add("terrain tiling UBO", tilingUboRelease.Run); + TrackedGlResource.AllocateBufferStorage( + _gl, + BufferTargetARB.UniformBuffer, + _tilingUbo, + 0, + TerrainTextureTilingTable.UniformBufferBytes, + 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( @@ -501,6 +527,15 @@ public sealed unsafe class TerrainModernRenderer : IDisposable // unified camera matrix everywhere, so no separate viewpoint divergence can occur. _shader.Use(); UploadTextureTilingOnce(); + // Campaign V slice V6f-2: bind the tiling UBO every draw, not once. GL's + // uniform-buffer binding points are global and shared with the sky's + // params block and the SceneLighting block, so a renderer that runs + // between two terrain draws can take binding 3 out from under us. + // Self-contained state, per feedback_render_self_contained_gl_state. + _gl.BindBufferBase( + BufferTargetARB.UniformBuffer, + Gpu.GpuBindingModel.UniformTerrainTiling, + _tilingUbo); // Campaign V slice V6f-1: one uViewProjection, matching the field // GpuPushConstants already carries, instead of the separate uView and // uProjection the shader used to combine per vertex. viewProjection is @@ -620,6 +655,16 @@ public sealed unsafe class TerrainModernRenderer : IDisposable "deleting terrain fallback clip UBO"); releases.Add(("fallback-clip-ubo", release.Run)); } + if (_tilingUbo != 0) + { + RetryableGpuResourceRelease release = + TrackedGlResource.CreateRetryableBufferDeletion( + _gl, + _tilingUbo, + TerrainTextureTilingTable.UniformBufferBytes, + "deleting terrain tiling UBO"); + releases.Add(("tiling-ubo", release.Run)); + } if (_textureTableSsbo != 0) { RetryableGpuResourceRelease release = @@ -687,11 +732,32 @@ public sealed unsafe class TerrainModernRenderer : IDisposable $"expected {TerrainTextureTilingTable.LayerCapacity}."); } - Span values = stackalloc float[TerrainTextureTilingTable.LayerCapacity]; - for (int i = 0; i < values.Length; i++) - values[i] = _atlas.TilingByLayer[i]; + // Campaign V slice V6f-2: one whole-buffer write into the binding=3 + // uniform buffer, replacing the glUniform1fv into the loose array. The + // block is std140, so each value sits at a 16-byte stride with three + // dead words after it; the span is cleared first so those words are + // zero rather than whatever the stack held. + Span block = stackalloc byte[TerrainTextureTilingTable.UniformBufferBytes]; + block.Clear(); + for (int i = 0; i < TerrainTextureTilingTable.LayerCapacity; i++) + { + BitConverter.TryWriteBytes( + block[(i * TerrainTextureTilingTable.UniformElementStrideBytes)..], + _atlas.TilingByLayer[i]); + } + + fixed (byte* p = block) + { + TrackedGlResource.UpdateBufferSubData( + _gl, + BufferTargetARB.UniformBuffer, + _tilingUbo, + 0, + TerrainTextureTilingTable.UniformBufferBytes, + p, + "uploading terrain tiling UBO"); + } - _gl.Uniform1(_uTexTilingLoc, values); _textureTilingUploaded = true; } diff --git a/src/AcDream.App/Rendering/TerrainTextureTilingTable.cs b/src/AcDream.App/Rendering/TerrainTextureTilingTable.cs index a5686057..d457306f 100644 --- a/src/AcDream.App/Rendering/TerrainTextureTilingTable.cs +++ b/src/AcDream.App/Rendering/TerrainTextureTilingTable.cs @@ -15,6 +15,25 @@ internal static class TerrainTextureTilingTable // capacity before the shader table is built. internal const int LayerCapacity = 36; + /// + /// std140 stride of one float in the shader's uTexTiling array. + /// std140 pads every array element out to 16 bytes, which is why the block is + /// 576 bytes rather than the 144 the payload occupies. Packing four values + /// per vec4 would be tighter but would change the shader's use site; + /// keeping the element type means uTexTiling[int(layer)] reads exactly + /// as it did when the array was a loose uniform. + /// + internal const int UniformElementStrideBytes = 16; + + /// + /// Size of the TerrainTiling uniform buffer + /// (GpuBindingModel.UniformTerrainTiling, binding 3) that has carried + /// this table since slice V6f-2. Asserted against the stride above rather + /// than written as a literal, because the two disagreeing is a mismatch only + /// a wrongly-tiled terrain texture would reveal. + /// + internal const int UniformBufferBytes = LayerCapacity * UniformElementStrideBytes; + internal static float[] Build(IEnumerable<(uint Layer, uint RepeatCount)> entries) { ArgumentNullException.ThrowIfNull(entries); diff --git a/tests/AcDream.App.Tests/Rendering/TerrainTextureTilingTableTests.cs b/tests/AcDream.App.Tests/Rendering/TerrainTextureTilingTableTests.cs index e2bec44f..d5193b1c 100644 --- a/tests/AcDream.App.Tests/Rendering/TerrainTextureTilingTableTests.cs +++ b/tests/AcDream.App.Tests/Rendering/TerrainTextureTilingTableTests.cs @@ -55,7 +55,14 @@ public sealed class TerrainTextureTilingTableTests "terrain_modern.frag"); string shader = File.ReadAllText(shaderPath); - Assert.Contains("uniform float uTexTiling[36];", shader); + // Campaign V slice V6f-2 moved the table out of a loose + // `uniform float uTexTiling[36]` and into a std140 block at + // GpuBindingModel.UniformTerrainTiling. The element type and count are + // what every sample site below depends on, so both are still pinned — + // and the binding number is now pinned too, because the shader and + // GpuBindingModel have to agree. + Assert.Contains("binding = 3) uniform TerrainTiling {", shader); + Assert.Contains("float uTexTiling[36];", shader); Assert.Contains("baseUV * terrainTiling(pOverlay0.z)", shader); Assert.Contains("baseUV * terrainTiling(pOverlay1.z)", shader); Assert.Contains("baseUV * terrainTiling(pOverlay2.z)", shader); @@ -63,4 +70,33 @@ public sealed class TerrainTextureTilingTableTests Assert.Contains("vBaseUV * terrainTiling(vBaseTexIdx)", shader); Assert.DoesNotContain("const float TILE", shader); } + + /// + /// Campaign V slice V6f-2: the CPU writer and the std140 block must agree on + /// where each value lands. + /// + /// This is the specific hazard the move to a uniform buffer + /// introduced. std140 pads every element of a scalar array to 16 bytes, so a + /// writer that packs 36 floats tightly produces a buffer in which the shader + /// reads layer 0's value for layers 0-3, layer 4's for 4-7, and so on. That + /// renders — it just tiles the wrong textures, in a scene where nine of ten + /// layers use a tiling factor of 1 and the difference is invisible until the + /// tenth appears. Nothing else in the suite would catch it, so the two + /// constants are pinned to the layout rule directly. + /// + [Fact] + public void UniformBufferMatchesTheStd140LayoutTheShaderDeclares() + { + Assert.Equal(16, TerrainTextureTilingTable.UniformElementStrideBytes); + Assert.Equal(576, TerrainTextureTilingTable.UniformBufferBytes); + Assert.Equal( + TerrainTextureTilingTable.LayerCapacity + * TerrainTextureTilingTable.UniformElementStrideBytes, + TerrainTextureTilingTable.UniformBufferBytes); + + // A std140 float array element is padded, never packed. If this ever + // equals sizeof(float) the writer above has been "simplified" into the + // bug this test exists for. + Assert.NotEqual(sizeof(float), TerrainTextureTilingTable.UniformElementStrideBytes); + } }