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); + } }