diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs index 12638d35..f1f6fbea 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs @@ -75,15 +75,22 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable return [.. bindings]; } - /// Bindings 0..3 of set 1; only 1 (SceneLighting) and 3 (terrain tiling) are used. - internal const int UniformBindingCount = 4; + /// + /// Bindings 0..4 of set 1. Slice V6i-2 raised this from 4 when the layout + /// gained binding 4 (sky params); binding 0 remains unused and is counted + /// only so the bookkeeping arrays stay index-aligned with the binding number. + /// Which of them the layout DECLARES is + /// . + /// + internal const int UniformBindingCount = 5; /// /// How many of set 1's bindings the layout actually declares, all dynamic. /// Asserted against maxDescriptorSetUniformBuffersDynamic by the /// capability gate; Vulkan guarantees 8, so this is comfortable. /// - internal const uint DynamicUniformBindingCount = 2; + internal static uint DynamicUniformBindingCount { get; } = + (uint)VulkanPipelineLayouts.DeclaredUniformBindings.Length; /// /// Widest range any single binding may address. Dynamic descriptors take a @@ -179,14 +186,15 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable sets[1] = _sets[slot].Uniform; sets[2] = device.TextureTable.Set; - int dynamicCount = DynamicStorageBindings.Length + 2; + uint[] declaredUniforms = VulkanPipelineLayouts.DeclaredUniformBindings; + int dynamicCount = DynamicStorageBindings.Length + declaredUniforms.Length; uint* offsets = stackalloc uint[dynamicCount]; // Dynamic offsets are ordered by set, then by binding number, and only // the DYNAMIC descriptors have a slot at all. for (int i = 0; i < DynamicStorageBindings.Length; i++) offsets[i] = _arena.StorageOffset(DynamicStorageBindings[i]); - offsets[DynamicStorageBindings.Length + 0] = _arena.UniformOffset(GpuBindingModel.UniformSceneLighting); - offsets[DynamicStorageBindings.Length + 1] = _arena.UniformOffset(GpuBindingModel.UniformTerrainTiling); + for (int i = 0; i < declaredUniforms.Length; i++) + offsets[DynamicStorageBindings.Length + i] = _arena.UniformOffset(declaredUniforms[i]); _vk.CmdBindDescriptorSets( commands, @@ -212,18 +220,16 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable VulkanPipelineLayouts.IsDynamicStorageBinding(binding)); } - // Only the two bindings the layout declares exist; the rest of the array - // is bookkeeping so the offsets stay index-aligned. - WriteUniform( - pair.Uniform, - GpuBindingModel.UniformSceneLighting, - new Silk.NET.Vulkan.Buffer(_arena.UniformBuffer(GpuBindingModel.UniformSceneLighting)), - _arena.UniformRange(GpuBindingModel.UniformSceneLighting)); - WriteUniform( - pair.Uniform, - GpuBindingModel.UniformTerrainTiling, - new Silk.NET.Vulkan.Buffer(_arena.UniformBuffer(GpuBindingModel.UniformTerrainTiling)), - _arena.UniformRange(GpuBindingModel.UniformTerrainTiling)); + // Only the bindings the layout declares exist; the rest of the array is + // bookkeeping so the offsets stay index-aligned with the binding number. + foreach (uint binding in VulkanPipelineLayouts.DeclaredUniformBindings) + { + WriteUniform( + pair.Uniform, + binding, + new Silk.NET.Vulkan.Buffer(_arena.UniformBuffer(binding)), + _arena.UniformRange(binding)); + } } private (DescriptorSet Storage, DescriptorSet Uniform) AllocatePair() diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs index a4ac876f..40c0c62c 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs @@ -184,29 +184,93 @@ internal static unsafe class VulkanPipelineLayouts return layout; } - /// Set 1 — the SceneLighting and terrain-tiling uniform blocks. + /// + /// The terrain screen-space clip block's uniform binding. + /// + /// does not name this number — it only + /// records, twice, that "binding 2 is taken by the terrain clip block" while + /// explaining why terrain tiling is 3 and sky params are 4. The number itself + /// has been pinned by terrain_modern.vert and sky.vert since + /// Phase U.3. Restating it here rather than promoting it into the frozen + /// binding model keeps slice V6i-2 out of the pinned contract; a later slice + /// entitled to change §3.3 should move it. + /// + internal const uint UniformTerrainClip = 2; + + /// + /// Which of set 1's bindings the layout declares — the uniform-side twin of + /// , and for the same reason: the + /// layout, the descriptor writes and vkCmdBindDescriptorSets's + /// dynamic-offset array must agree on both membership and ORDER, and getting + /// either wrong is a validation error. Deriving all three from one predicate + /// is what stops them drifting. + /// + /// Binding 0 is unused — + /// is 1 so that GL's separate UBO/SSBO namespaces keep their existing numbers. + /// It is still counted by + /// so the bookkeeping arrays stay index-aligned with the binding number. + /// + /// Slice V6i-2 added 2 (terrain clip) and 4 (sky params). Both blocks + /// have compiled to SPIR-V declaring set 1 for some time — plan §5.5.8 + /// recorded the sky gap and §5.5.12 measured the terrain one — but neither + /// binding existed in the layout, so a pipeline built for either shader was + /// malformed. Nothing caught it because no terrain or sky pipeline has ever + /// been created on Vulkan. + /// + internal static bool IsDeclaredUniformBinding(uint binding) => binding switch + { + GpuBindingModel.UniformSceneLighting => true, + UniformTerrainClip => true, + GpuBindingModel.UniformTerrainTiling => true, + GpuBindingModel.UniformSkyParams => true, + _ => false, + }; + + /// + /// Set 1's declared bindings in ascending order — the order + /// vkCmdBindDescriptorSets requires its dynamic offsets in. + /// + internal static uint[] DeclaredUniformBindings { get; } = BuildDeclaredUniformBindings(); + + private static uint[] BuildDeclaredUniformBindings() + { + var bindings = new List(VulkanFrameBindings.UniformBindingCount); + for (uint binding = 0; binding < VulkanFrameBindings.UniformBindingCount; binding++) + { + if (IsDeclaredUniformBinding(binding)) + bindings.Add(binding); + } + + return [.. bindings]; + } + + /// + /// Set 1 — the SceneLighting, terrain-clip, terrain-tiling and sky-params + /// uniform blocks. All four are dynamic: each is fed from the per-frame ring, + /// so its offset moves every frame and a dynamic descriptor is exactly what + /// spares the write. Four is half Vulkan's guaranteed + /// maxDescriptorSetUniformBuffersDynamic of 8, and the capability gate + /// asserts it. + /// internal static DescriptorSetLayout CreateUniformSetLayout(Silk.NET.Vulkan.Vk vk, Device device) { - DescriptorSetLayoutBinding* bindings = stackalloc DescriptorSetLayoutBinding[2]; - bindings[0] = new DescriptorSetLayoutBinding + uint[] declared = DeclaredUniformBindings; + DescriptorSetLayoutBinding* bindings = stackalloc DescriptorSetLayoutBinding[declared.Length]; + for (int i = 0; i < declared.Length; i++) { - Binding = GpuBindingModel.UniformSceneLighting, - DescriptorType = DescriptorType.UniformBufferDynamic, - DescriptorCount = 1, - StageFlags = ShaderStageFlags.VertexBit | ShaderStageFlags.FragmentBit, - }; - bindings[1] = new DescriptorSetLayoutBinding - { - Binding = GpuBindingModel.UniformTerrainTiling, - DescriptorType = DescriptorType.UniformBufferDynamic, - DescriptorCount = 1, - StageFlags = ShaderStageFlags.VertexBit | ShaderStageFlags.FragmentBit, - }; + bindings[i] = new DescriptorSetLayoutBinding + { + Binding = declared[i], + DescriptorType = DescriptorType.UniformBufferDynamic, + DescriptorCount = 1, + StageFlags = ShaderStageFlags.VertexBit | ShaderStageFlags.FragmentBit, + }; + } var create = new DescriptorSetLayoutCreateInfo { SType = StructureType.DescriptorSetLayoutCreateInfo, - BindingCount = 2, + BindingCount = (uint)declared.Length, PBindings = bindings, }; VulkanInterop.Check( diff --git a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json index f6a65f17..6ca80364 100644 --- a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json +++ b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json @@ -105,7 +105,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "336880b293e95c9ba13dce7a616a15e941191afec8bdaabe04a4993182eb6811", + "sourceSha256": "a3f8592d482793622f7a69c08f8ba828a4e071f2fb5e1763f865be84797bceb7", "compiled": true }, { diff --git a/src/AcDream.App/Rendering/Shaders/spv/terrain_modern.vert.spv b/src/AcDream.App/Rendering/Shaders/spv/terrain_modern.vert.spv index 6c4f766e..bbf61fad 100644 Binary files a/src/AcDream.App/Rendering/Shaders/spv/terrain_modern.vert.spv and b/src/AcDream.App/Rendering/Shaders/spv/terrain_modern.vert.spv differ diff --git a/src/AcDream.App/Rendering/Shaders/terrain_modern.vert b/src/AcDream.App/Rendering/Shaders/terrain_modern.vert index 254b948e..dbf24f40 100644 --- a/src/AcDream.App/Rendering/Shaders/terrain_modern.vert +++ b/src/AcDream.App/Rendering/Shaders/terrain_modern.vert @@ -47,7 +47,13 @@ layout(std140, ACDREAM_UBO_SET binding = 1) uniform SceneLighting { // UBO, leaving binding=2 free. uTerrainClipCount == 0 (the U.3 default) ungates // terrain entirely (the second loop sets all 8 distances to +1.0). Uploaded by // ClipFrame.UploadShared each frame; TerrainModernRenderer binds it before draw. -layout(std140, binding = 2) uniform TerrainClip { +// +// Campaign V slice V6i-2: ACDREAM_UBO_SET is what puts this in set 1 under the +// Vulkan dialect and expands to nothing under GL. Omitting it left the block at +// set 0 binding 2, which the storage layout declares as a STORAGE buffer — see +// plan §5.5.12 finding 2, measured on the committed SPIR-V rather than inferred. +// sky.vert declares the SAME block correctly and is the precedent. +layout(std140, ACDREAM_UBO_SET binding = 2) uniform TerrainClip { int uTerrainClipCount; vec4 uTerrainClipPlanes[8]; }; diff --git a/tests/AcDream.App.Tests/Rendering/Gpu/Vk/VulkanShaderDescriptorContractTests.cs b/tests/AcDream.App.Tests/Rendering/Gpu/Vk/VulkanShaderDescriptorContractTests.cs new file mode 100644 index 00000000..53d3c5c0 --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/Gpu/Vk/VulkanShaderDescriptorContractTests.cs @@ -0,0 +1,223 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using AcDream.App.Rendering.Gpu; +using AcDream.App.Rendering.Gpu.Vk; + +namespace AcDream.App.Tests.Rendering.Gpu.Vk; + +/// +/// Campaign V slice V6i-2: every compiled SPIR-V module's descriptor decorations +/// must match the descriptor set layouts the backend creates. +/// +/// Why this test exists. Slice V6i measured that +/// terrain_modern.vert's TerrainClip block was declared +/// layout(std140, binding = 2) with no ACDREAM_UBO_SET, so under +/// the Vulkan dialect it landed in set 0 binding 2 — which set 0's layout +/// declares as a STORAGE buffer. A terrain pipeline built against the shared +/// layout was therefore malformed. Nothing caught it: GL expands the macro to +/// nothing and keeps its UBO and SSBO namespaces separate, the shader compiled +/// cleanly for both backends, and no terrain pipeline had ever been created on +/// Vulkan. Plan §5.5.8 recorded a second instance of the same class — sky's +/// UniformSkyParams at binding 4 was absent from the layout — found the +/// same way, by hand, months apart. +/// +/// A one-word omission in a shader that cannot fail to compile, cannot +/// fail on the shipping backend, and is only wrong on a backend nothing has run +/// yet, is exactly the shape of defect that needs a test rather than an audit. +/// Reading the committed .spv is what makes the assertion about what the +/// GPU will actually be handed, not about what the GLSL looks like. +/// +public sealed class VulkanShaderDescriptorContractTests +{ + private static string RepositoryRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "AcDream.slnx"))) + directory = directory.Parent; + return directory?.FullName + ?? throw new InvalidOperationException("Could not locate the repository root from the test binary."); + } + + private static string SpirvDirectory() => + Path.Combine(RepositoryRoot(), "src", "AcDream.App", "Rendering", "Shaders", "spv"); + + /// One interface variable's set, binding and storage class. + private readonly record struct ShaderResource( + string Module, + uint Id, + SpirvStorageClass StorageClass, + uint? Set, + uint? Binding); + + private enum SpirvStorageClass : uint + { + UniformConstant = 0, + Uniform = 2, + StorageBuffer = 12, + } + + /// + /// The smallest SPIR-V reader that can answer this question: walk the + /// instruction stream collecting OpDecorate DescriptorSet/Binding and + /// the storage class of every OpVariable. No type or name recovery is + /// needed, so no dependency is either. + /// + private static IReadOnlyList ReadResources(string path) + { + byte[] bytes = File.ReadAllBytes(path); + Assert.True(bytes.Length >= 20 && bytes.Length % 4 == 0, $"{path} is not a SPIR-V module."); + var words = new uint[bytes.Length / 4]; + Buffer.BlockCopy(bytes, 0, words, 0, bytes.Length); + Assert.Equal(0x07230203u, words[0]); + + const uint OpDecorate = 71; + const uint OpVariable = 59; + const uint DecorationBinding = 33; + const uint DecorationDescriptorSet = 34; + + var sets = new Dictionary(); + var bindings = new Dictionary(); + var variables = new List<(uint Id, SpirvStorageClass StorageClass)>(); + + int index = 5; + while (index < words.Length) + { + uint header = words[index]; + int wordCount = (int)(header >> 16); + uint opcode = header & 0xFFFF; + Assert.True(wordCount > 0, $"{path} has a zero-length instruction at word {index}."); + + if (opcode == OpDecorate && wordCount >= 4) + { + uint target = words[index + 1]; + uint decoration = words[index + 2]; + if (decoration == DecorationDescriptorSet) + sets[target] = words[index + 3]; + else if (decoration == DecorationBinding) + bindings[target] = words[index + 3]; + } + else if (opcode == OpVariable && wordCount >= 4) + { + variables.Add((words[index + 2], (SpirvStorageClass)words[index + 3])); + } + + index += wordCount; + } + + string module = Path.GetFileName(path); + return + [ + .. variables.Select(v => new ShaderResource( + module, + v.Id, + v.StorageClass, + sets.TryGetValue(v.Id, out uint s) ? s : null, + bindings.TryGetValue(v.Id, out uint b) ? b : null)), + ]; + } + + private static IReadOnlyList AllResources() => + [ + .. Directory + .EnumerateFiles(SpirvDirectory(), "*.spv") + .OrderBy(path => path, StringComparer.Ordinal) + .SelectMany(ReadResources), + ]; + + /// + /// The regression itself, named. TerrainClip is the only uniform block + /// terrain_modern.vert declares besides SceneLighting, so + /// asserting the module's uniform bindings as a set pins it exactly. + /// + [Fact] + public void TerrainVertexShaderDeclaresItsClipBlockInTheUniformSet() + { + uint[] uniformBindings = + [ + .. ReadResources(Path.Combine(SpirvDirectory(), "terrain_modern.vert.spv")) + .Where(r => r.StorageClass == SpirvStorageClass.Uniform) + .Select(r => + { + Assert.Equal(GpuBindingModel.UniformSet, r.Set); + return r.Binding!.Value; + }) + .Order(), + ]; + + Assert.Equal( + [GpuBindingModel.UniformSceneLighting, VulkanPipelineLayouts.UniformTerrainClip], + uniformBindings); + } + + /// + /// The general rule the specific case is an instance of: a uniform block may + /// only live at a binding + /// actually declares, in the set it declares them in. + /// + [Fact] + public void EveryUniformBlockLandsAtADeclaredUniformBinding() + { + string[] violations = + [ + .. AllResources() + .Where(r => r.StorageClass == SpirvStorageClass.Uniform) + .Where(r => + r.Set != GpuBindingModel.UniformSet + || r.Binding is null + || !VulkanPipelineLayouts.IsDeclaredUniformBinding(r.Binding.Value)) + .Select(r => + $"{r.Module}: uniform block %{r.Id} is at set {r.Set?.ToString() ?? "(none)"} " + + $"binding {r.Binding?.ToString() ?? "(none)"}; set 1 declares " + + $"[{string.Join(", ", VulkanPipelineLayouts.DeclaredUniformBindings)}]."), + ]; + + Assert.Empty(violations); + } + + /// + /// The other half of the same contract: set 0 is storage buffers only, and + /// within the range pins. A + /// uniform block straying into set 0 fails this as well as the test above, + /// which is the point — the two layouts must partition the bindings. + /// + [Fact] + public void EveryStorageBlockLandsInTheStorageSetWithinItsDeclaredRange() + { + string[] violations = + [ + .. AllResources() + .Where(r => r.StorageClass == SpirvStorageClass.StorageBuffer) + .Where(r => r.Set != 0 || r.Binding is null || r.Binding >= GpuBindingModel.StorageBindingCount) + .Select(r => + $"{r.Module}: storage block %{r.Id} is at set {r.Set?.ToString() ?? "(none)"} " + + $"binding {r.Binding?.ToString() ?? "(none)"}; set 0 declares bindings " + + $"0..{GpuBindingModel.StorageBindingCount - 1}."), + ]; + + Assert.Empty(violations); + } + + /// + /// The sampled-texture table is one variable-count descriptor array at set 2 + /// binding 0. Any other combined-image-sampler would need a layout that does + /// not exist. + /// + [Fact] + public void EverySampledTextureIsTheSharedTable() + { + string[] violations = + [ + .. AllResources() + .Where(r => r.StorageClass == SpirvStorageClass.UniformConstant && r.Set is not null) + .Where(r => r.Set != GpuBindingModel.TextureTableSet || r.Binding != GpuBindingModel.TextureTableBinding) + .Select(r => + $"{r.Module}: sampled resource %{r.Id} is at set {r.Set} binding " + + $"{r.Binding?.ToString() ?? "(none)"}; the only declared table is set " + + $"{GpuBindingModel.TextureTableSet} binding {GpuBindingModel.TextureTableBinding}."), + ]; + + Assert.Empty(violations); + } +}