diff --git a/docs/plans/2026-07-27-vulkan-campaign.md b/docs/plans/2026-07-27-vulkan-campaign.md index 9216b9f8..fe1fb7f7 100644 --- a/docs/plans/2026-07-27-vulkan-campaign.md +++ b/docs/plans/2026-07-27-vulkan-campaign.md @@ -572,6 +572,7 @@ dialect slice. **V6f closed all three** (the third was the frag's GL-only `sampler2DArray(handle)` construction), so 8/9 pairs now compile. `mesh` is a tenth pair with no consumer at all; see the V6e report. +| **V6g** | The four Vulkan validation defects §5.5.7 and its log left open: the dynamic-descriptor split (an architect decision, §5.5.8 item 1), per-pass depth-format pipeline variants, first-use backbuffer attachment layout transitions, and a backbuffer capture that no longer reads a presented swapchain image. Confined to `Gpu/Vk/`; the GL backend executes not one changed statement. | validation-clean bring-up run (0 errors / 0 warnings over 39,855 frames, against 7 VUIDs + 1 UNASSIGNED at the parent), App tests, GL offline pixel gate 4.08e-05 — its own same-commit control value | | **V7** | GL-versus-Vulkan differential: `tools/run-backend-differential-gate.ps1`, strict paired-PNG compare, divergences fixed in the Vulkan backend only, then lifecycle + R6 soak natively on Vulkan, one validation-layer-clean run, one RenderDoc capture. **Milestone: parity.** | every differential checkpoint passes; both connected routes green on VK | | **V8** | Perf gate on the RX 9070 XT, uncapped, both backends, same route. | §2 acceptance table; parity is the floor | | **V9** | Linux + CI: X11/Wayland surfaces; a `linux-vulkan` job on lavapipe (probe accepts on a real 1.3 software device, a short real render under xvfb, forced-unsupported → exit 4, `.spv` freshness). Physical Linux GPU row deferred post-cutover, as for Slice L. | CI green including the new job | @@ -1116,6 +1117,84 @@ and sky with no scenery or statics, and would be the first real evidence the Vulkan world path works. V6f's shader work is the whole of that path's shader prerequisite. +#### 5.5.8 V6g (2026-07-28): the validation defects are closed, and a fourth was found + +§5.5.7's step 2 — "the three validation defects, since every one of them is on +the path any world frame takes" — is done, and the Vulkan bring-up host now runs +**validation-clean**: zero errors and zero warnings across a 39,855-frame run +with `VK_LAYER_KHRONOS_validation` loaded, against the same run that produced +seven of them at `f8dbe2ee`. + +**1. The dynamic-descriptor limit (`VUID-VkPipelineLayoutCreateInfo-descriptorType-03032` +/ `-pSetLayouts-03040`).** Resolved by decision rather than patch, as §5.5.7 asked. +V6b declared all ten of set 0's bindings `STORAGE_BUFFER_DYNAMIC`; the rule now is +that **a dynamic descriptor is for ring-fed data whose offset moves, and nothing +else**. Instances (0), batches (1), clip slots (3) and instance light sets (5) stay +dynamic; global lights (4), clip regions (2), instance indoor (6), alpha (7), +selection lighting (8) and the GL-only texture table (9) become plain +`STORAGE_BUFFER` carrying their offset in the descriptor. That is **four** dynamic +storage descriptors — not merely under the RX 9070 XT's 8 but exactly Vulkan's +guaranteed minimum, so no conformant device can fail the layout, which is what V9's +lavapipe row and the deferred physical Linux row depend on. The count is asserted +against `maxDescriptorSetStorageBuffersDynamic` in the capability record, so a +device that cannot serve it is rejected at startup under the exit-code-4 contract +instead of failing at `vkCreatePipelineLayout`. Bindings 6–8 are per-instance +arrays grouped with the frame-global tables because their owner writes them whole +once per frame; if the Vulkan world path needs one re-pointed per draw, promoting +it back is one line, with four unused dynamic slots to promote into. + +**2. Depth-off pipelines in depth-carrying passes +(`VUID-vkCmdDraw-dynamicRenderingUnusedAttachments-08914` / `-08917`).** The +backend now builds **two variants of every pipeline** — one declaring the pass's +depth/stencil format, one declaring `UNDEFINED` — and binds whichever matches what +`vkCmdBeginRendering` was actually handed. The same `GpuPipelineDescription` is +legitimately used in both kinds of pass (`ui-text` opens its own depth-less pass; +the world pass it composites over has depth), so the description genuinely cannot +answer the question. **A later slice entitled to change the contract should add a +depth-format field the way V6d added `ColorFormat`**; until then, materialising +both at startup against the persisted cache is the honest expression of the gap and +no frame ever compiles one. + +**3. Missing first-use layout transitions +(`VUID-vkCmdBeginRendering-pRenderingInfo-09588` / `-09590` / `-09592`).** +`vk-backbuffer-depth` and `vk-backbuffer-msaa-color` are created UNDEFINED and were +never moved. Both now get a barrier on every backbuffer pass: from UNDEFINED on the +first use after `Configure`, and from the attachment-optimal layout with a +write-after-write dependency thereafter — the same shape the swapchain image +already had. The dependency matters independently of the layout: two passes in one +frame write both images, and so does the next frame, with no implicit ordering +between render-pass instances. + +**4. The capture path read an image it did not own +(`UNASSIGNED-non-acquired-swapchain-image-used`).** Present in V6f's log and not +called out there. `CaptureBackbuffer` transitioned the **last presented** swapchain +image to `TRANSFER_SRC` and copied out of it; after `vkQueuePresentKHR` that image +belongs to the presentation engine and its contents are not the application's to +read. The pixels were usually right — which is exactly what makes it +unacceptable. **This campaign spent §5.5.1–§5.5.3 discovering what a capture +instrument that is "usually right" costs**, and shipping the same shape on the new +backend would have made every Vulkan PNG, and the V7 differential built on them, +formally undefined. The frame now copies its own output into a host-readable buffer +while it still owns the image, and `CaptureBackbuffer` reads that. Retention is +opt-in (armed when an artifact directory exists) because it costs one full-res +image-to-buffer copy per frame: worth nothing to a player, and the entire +instrument to a gate. + +**Two gaps recorded, not fixed** — both outside this slice's brief, both real: + +- **`UniformSkyParams` (set 1, binding 4) is not in the uniform set layout**, which + declares only bindings 1 and 3, and `VulkanFrameBindings.UniformBindingCount` is + 4, so `SetUniform(4, …)` throws before it can be wrong. The `sky` pair compiles to + SPIR-V declaring that binding, so whoever first draws sky on Vulkan must add it. +- **A binding pointed at two different buffers within one frame silently corrupts + the earlier draws**, on dynamic and plain descriptors alike: `SetStorage` rewrites + the descriptor when the buffer changes, and descriptor contents are read at + execution time, not record time. No consumer does this today. The world path + will: `WbDrawDispatcher` and `EnvCellRenderer` each own their own instance and + batch buffers and both bind bindings 0, 1, 3, 4 and 5 in one frame. **The Vulkan + world arm needs one descriptor set per renderer, or per-renderer sub-ranges of one + buffer, and it needs to know that before it is written.** + ### 5.4 The null-target `BeginPass` divergence (V4c) — must be undone at V6 V4c had to stop GL's `BeginPass` from binding framebuffer 0 when a pass declares diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanBringUpHost.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanBringUpHost.cs index 2969fccb..632745f5 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanBringUpHost.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanBringUpHost.cs @@ -375,7 +375,13 @@ internal sealed unsafe class VulkanBringUpHost : IDisposable _debugNames, new SwapchainBackbuffer(_swapchain!, _presentQueue), ShaderSpirvDirectory(), - _platform.Paths.CacheDirectory); + _platform.Paths.CacheDirectory, + // Slice V6g: a frame can only be read back while it still owns its + // swapchain image, so retention has to be armed before the first + // frame rather than at the moment a screenshot is asked for. Armed + // exactly when an artifact directory exists, which is what a gate + // run has and a player run does not. + retainBackbufferCapture: !string.IsNullOrWhiteSpace(_options.AutomationArtifactDirectory)); // Four samples where the device allows it, so the backbuffer pass really // resolves rather than rendering straight into the swapchain image. diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanCapabilityRecord.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanCapabilityRecord.cs index 6374f8a6..d5e218f6 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanCapabilityRecord.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanCapabilityRecord.cs @@ -157,6 +157,22 @@ internal sealed record VulkanDeviceLimitSupport /// Sets 0, 1 and 2 are all bound simultaneously, so at least 3. public required uint MaxBoundDescriptorSets { get; init; } + /// + /// Must reach . + /// + /// Added at slice V6g. The V6b layout declared all ten of set 0's + /// bindings dynamic and was rejected by this limit on the RX 9070 XT, which + /// allows 8 — silently, at vkCreatePipelineLayout, and only visible + /// with the validation layer loaded (plan §5.5.7 defect 1). Reading the limit + /// into the record means the gate rejects such a device at startup, in the + /// report, under the same exit-code-4 contract as every other requirement, + /// rather than the layout failing at the first pipeline. + /// + public required uint MaxDescriptorSetStorageBuffersDynamic { get; init; } + + /// Must reach the number of dynamic uniform bindings set 1 declares. + public required uint MaxDescriptorSetUniformBuffersDynamic { get; init; } + /// Must reach . public required uint MaxDescriptorSetUpdateAfterBindSampledImages { get; init; } @@ -189,6 +205,10 @@ internal sealed record VulkanDeviceLimitSupport MaxPushConstantsSize = GpuBindingModel.MaxPushConstantBytes, MaxClipDistances = GpuBindingModel.ClipPlanesPerSlot, MaxBoundDescriptorSets = 4, + // Vulkan's guaranteed minimums. That the layout fits inside them is the + // point of slice V6g's split — see VulkanPipelineLayouts. + MaxDescriptorSetStorageBuffersDynamic = 4, + MaxDescriptorSetUniformBuffersDynamic = 8, MaxDescriptorSetUpdateAfterBindSampledImages = GpuBindingModel.TextureTableCapacity, MaxPerStageDescriptorUpdateAfterBindSampledImages = GpuBindingModel.TextureTableCapacity, TimestampComputeAndGraphics = true, @@ -450,6 +470,18 @@ internal static class VulkanCapabilityRequirements $"{VulkanDescriptorSetCount} simultaneously bound descriptor sets are required " + $"(storage, uniform, texture table); this device provides {limits.MaxBoundDescriptorSets}."); } + if (limits.MaxDescriptorSetStorageBuffersDynamic < VulkanPipelineLayouts.DynamicStorageBindingCount) + { + failures.Add( + $"set 0 declares {VulkanPipelineLayouts.DynamicStorageBindingCount} dynamic storage bindings " + + $"(Vulkan guarantees 4); this device provides {limits.MaxDescriptorSetStorageBuffersDynamic}."); + } + if (limits.MaxDescriptorSetUniformBuffersDynamic < VulkanFrameBindings.DynamicUniformBindingCount) + { + failures.Add( + $"set 1 declares {VulkanFrameBindings.DynamicUniformBindingCount} dynamic uniform bindings " + + $"(Vulkan guarantees 8); this device provides {limits.MaxDescriptorSetUniformBuffersDynamic}."); + } if (limits.MaxDescriptorSetUpdateAfterBindSampledImages < GpuBindingModel.TextureTableCapacity) { failures.Add( diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs index 2b1a0ddb..17708b0a 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanFrameBindings.cs @@ -10,11 +10,20 @@ namespace AcDream.App.Rendering.Gpu.Vk; /// and ring allocations mean that range moves every frame. The obvious /// implementation — write a descriptor per bind — would put a /// vkUpdateDescriptorSets in the hot path and reintroduce the exact cost -/// the texture table was designed to remove. So each binding is a +/// the texture table was designed to remove. So each ring-fed binding is a /// *_BUFFER_DYNAMIC descriptor pointing at the whole ring, and the /// per-draw offset travels in vkCmdBindDescriptorSets's dynamic-offset /// array, which is free. /// +/// Not every binding is dynamic. Slice V6g split set 0 by +/// , because ten +/// dynamic storage descriptors exceeded the device limit (plan §5.5.7 defect 1). +/// A plain binding carries its offset in the descriptor itself, so it is +/// rewritten when the range moves rather than when only the buffer changes — and +/// its slot in the dynamic-offset array does not exist. Getting that array's +/// length or ordering wrong is a validation error, so both are derived from the +/// same predicate the layout is built from rather than restated. +/// /// Every binding is always bound, whether a renderer uses it or /// not. Bindings a shader does not declare still need a live descriptor, so /// unused ones point at a shared dummy range. That is what lets there be ONE @@ -39,9 +48,34 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable private bool _disposed; + /// + /// Set 0's dynamic-offset slots, in binding order — the order + /// vkCmdBindDescriptorSets requires. A plain binding has no slot. + /// + private static readonly uint[] DynamicStorageBindings = BuildDynamicStorageBindings(); + + private static uint[] BuildDynamicStorageBindings() + { + var bindings = new List((int)GpuBindingModel.StorageBindingCount); + for (uint binding = 0; binding < GpuBindingModel.StorageBindingCount; binding++) + { + if (VulkanPipelineLayouts.IsDynamicStorageBinding(binding)) + bindings.Add(binding); + } + + return [.. bindings]; + } + /// Bindings 0..3 of set 1; only 1 (SceneLighting) and 3 (terrain tiling) are used. internal const int UniformBindingCount = 4; + /// + /// 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; + /// /// Widest range any single binding may address. Dynamic descriptors take a /// static range at write time and slide it with an offset, so this bounds @@ -62,13 +96,19 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable ArgumentNullException.ThrowIfNull(ring); ArgumentNullException.ThrowIfNull(dummy); - DescriptorPoolSize* sizes = stackalloc DescriptorPoolSize[2]; + DescriptorPoolSize* sizes = stackalloc DescriptorPoolSize[3]; sizes[0] = new DescriptorPoolSize { Type = DescriptorType.StorageBufferDynamic, - DescriptorCount = GpuBindingModel.StorageBindingCount, + DescriptorCount = VulkanPipelineLayouts.DynamicStorageBindingCount, }; sizes[1] = new DescriptorPoolSize + { + Type = DescriptorType.StorageBuffer, + DescriptorCount = + GpuBindingModel.StorageBindingCount - VulkanPipelineLayouts.DynamicStorageBindingCount, + }; + sizes[2] = new DescriptorPoolSize { Type = DescriptorType.UniformBufferDynamic, DescriptorCount = UniformBindingCount, @@ -77,7 +117,7 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable { SType = StructureType.DescriptorPoolCreateInfo, MaxSets = 2, - PoolSizeCount = 2, + PoolSizeCount = 3, PPoolSizes = sizes, }; VulkanInterop.Check( @@ -90,7 +130,12 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable for (uint binding = 0; binding < GpuBindingModel.StorageBindingCount; binding++) { _storageBuffers[binding] = dummy.Handle; - WriteStorage(binding, dummy.Handle, (uint)Math.Min(dummy.SizeBytes, MaxBindingRangeBytes)); + WriteStorage( + binding, + dummy.Handle, + offsetBytes: 0, + (uint)Math.Min(dummy.SizeBytes, MaxBindingRangeBytes), + VulkanPipelineLayouts.IsDynamicStorageBinding(binding)); } // Only the two bindings the layout declares exist; the rest of the @@ -108,14 +153,29 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable internal VulkanGpuBuffer Dummy { get; } - /// Points a storage binding at a range, re-writing the descriptor only when the BUFFER changes. + /// + /// Points a storage binding at a range. + /// + /// A DYNAMIC binding re-writes its descriptor only when the BUFFER + /// changes; the offset rides the bind call. A PLAIN binding has no such + /// channel, so the descriptor itself carries the offset and is re-written + /// when either moves. + /// internal void SetStorage(uint binding, VulkanGpuBuffer buffer, uint offsetBytes, uint sizeBytes) { ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual(binding, GpuBindingModel.StorageBindingCount); - if (_storageBuffers[binding].Handle != buffer.Handle.Handle) + bool dynamic = VulkanPipelineLayouts.IsDynamicStorageBinding(binding); + bool bufferChanged = _storageBuffers[binding].Handle != buffer.Handle.Handle; + bool offsetChanged = _storageOffsets[binding] != offsetBytes; + if (bufferChanged || (!dynamic && offsetChanged)) { _storageBuffers[binding] = buffer.Handle; - WriteStorage(binding, buffer.Handle, ClampRange(buffer, sizeBytes)); + WriteStorage( + binding, + buffer.Handle, + dynamic ? 0 : offsetBytes, + ClampRange(buffer, sizeBytes, dynamic ? 0 : offsetBytes), + dynamic); } _storageOffsets[binding] = offsetBytes; @@ -127,7 +187,7 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable if (_uniformBuffers[binding].Handle != buffer.Handle.Handle) { _uniformBuffers[binding] = buffer.Handle; - WriteUniform(binding, buffer.Handle, Math.Min(ClampRange(buffer, sizeBytes), 65536)); + WriteUniform(binding, buffer.Handle, Math.Min(ClampRange(buffer, sizeBytes, offsetBytes: 0), 65536)); } _uniformOffsets[binding] = offsetBytes; @@ -141,13 +201,14 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable sets[1] = _uniformSet; sets[2] = device.TextureTable.Set; - int dynamicCount = _storageOffsets.Length + 2; + int dynamicCount = DynamicStorageBindings.Length + 2; uint* offsets = stackalloc uint[dynamicCount]; - for (int i = 0; i < _storageOffsets.Length; i++) - offsets[i] = _storageOffsets[i]; - // Dynamic offsets are ordered by set, then by binding number. - offsets[_storageOffsets.Length + 0] = _uniformOffsets[GpuBindingModel.UniformSceneLighting]; - offsets[_storageOffsets.Length + 1] = _uniformOffsets[GpuBindingModel.UniformTerrainTiling]; + // 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] = _storageOffsets[DynamicStorageBindings[i]]; + offsets[DynamicStorageBindings.Length + 0] = _uniformOffsets[GpuBindingModel.UniformSceneLighting]; + offsets[DynamicStorageBindings.Length + 1] = _uniformOffsets[GpuBindingModel.UniformTerrainTiling]; _vk.CmdBindDescriptorSets( commands, @@ -160,9 +221,19 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable offsets); } - private static uint ClampRange(VulkanGpuBuffer buffer, uint requested) + private static uint ClampRange(VulkanGpuBuffer buffer, uint requested, uint offsetBytes) { - uint available = (uint)Math.Min(buffer.SizeBytes, MaxBindingRangeBytes); + long remaining = buffer.SizeBytes - offsetBytes; + if (remaining <= 0) + { + throw new ArgumentOutOfRangeException( + nameof(offsetBytes), + offsetBytes, + $"A storage binding was pointed past the end of its {buffer.SizeBytes}-byte buffer. " + + "A descriptor range of zero is not representable in Vulkan."); + } + + uint available = (uint)Math.Min(remaining, MaxBindingRangeBytes); return requested == 0 ? available : Math.Min(Math.Max(requested, 16), available); } @@ -182,12 +253,17 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable return set; } - private void WriteStorage(uint binding, Silk.NET.Vulkan.Buffer buffer, uint rangeBytes) + private void WriteStorage( + uint binding, + Silk.NET.Vulkan.Buffer buffer, + uint offsetBytes, + uint rangeBytes, + bool dynamic) { var info = new DescriptorBufferInfo { Buffer = buffer, - Offset = 0, + Offset = offsetBytes, Range = rangeBytes, }; var write = new WriteDescriptorSet @@ -196,7 +272,9 @@ internal sealed unsafe class VulkanFrameBindings : IDisposable DstSet = _storageSet, DstBinding = binding, DescriptorCount = 1, - DescriptorType = DescriptorType.StorageBufferDynamic, + DescriptorType = dynamic + ? DescriptorType.StorageBufferDynamic + : DescriptorType.StorageBuffer, PBufferInfo = &info, }; _vk.UpdateDescriptorSets(_device, 1, &write, 0, null); diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.Resources.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.Resources.cs index 3ef4cbb9..753fd2ea 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.Resources.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.Resources.cs @@ -148,6 +148,8 @@ internal sealed unsafe partial class VulkanGpuDevice _bindingDummy?.Dispose(); _bindingDummy = null; + _captureBuffer?.Dispose(); + _captureBuffer = null; _defaultTexture?.Dispose(); _defaultTexture = null; @@ -192,8 +194,11 @@ internal sealed unsafe partial class VulkanGpuDevice /// extent and the requested sample count. Called by the host after a /// swapchain create or recreate, behind a device-idle wait. /// - internal void ConfigureBackbufferAttachments(uint width, uint height, Format colorFormat, int sampleCount) => + internal void ConfigureBackbufferAttachments(uint width, uint height, Format colorFormat, int sampleCount) + { BackbufferAttachments.Configure(width, height, colorFormat, sampleCount); + ConfigureBackbufferCapture(width, height); + } public IGpuTexture CreateTexture(in GpuTextureDescription description) { @@ -406,6 +411,7 @@ internal sealed unsafe partial class VulkanGpuDevice { colorView = attachments.ColorView; resolveView = _backbuffer.ViewAt(imageIndex); + TransitionBackbufferScratchColor(commands, attachments); } else { @@ -413,7 +419,10 @@ internal sealed unsafe partial class VulkanGpuDevice } if (description.Depth is not null && attachments.HasDepth) + { depthView = attachments.DepthView; + TransitionBackbufferDepth(commands, attachments); + } } else { @@ -498,7 +507,8 @@ internal sealed unsafe partial class VulkanGpuDevice _frameBindings[frame.SlotIndex], description, width, - height); + height, + hasDepthAttachment: depthView.Handle != 0); _openPass = encoder; return encoder; } @@ -583,6 +593,60 @@ internal sealed unsafe partial class VulkanGpuDevice _vk.CmdPipelineBarrier2(commands, &dependency); } + /// + /// Campaign V slice V6g: moves the multisampled colour scratch into + /// COLOR_ATTACHMENT_OPTIMAL before the pass that names it there. + /// + /// The first use after (re)creation starts from UNDEFINED — the image + /// genuinely has no contents, and saying so lets the driver skip a + /// decompress. Every later use starts from the layout the previous pass left + /// and needs the barrier for its write-after-write dependency instead: two + /// passes in one frame both write this image, and so does the next frame, + /// with no implicit ordering between render-pass instances. + /// + private void TransitionBackbufferScratchColor( + CommandBuffer commands, + VulkanBackbufferAttachments attachments) + { + bool first = !attachments.ColorLayoutInitialized; + attachments.MarkColorLayoutInitialized(); + TransitionImage( + commands, + attachments.ColorImage, + ImageAspectFlags.ColorBit, + first ? ImageLayout.Undefined : ImageLayout.ColorAttachmentOptimal, + ImageLayout.ColorAttachmentOptimal, + first ? PipelineStageFlags2.TopOfPipeBit : PipelineStageFlags2.ColorAttachmentOutputBit, + first ? AccessFlags2.None : AccessFlags2.ColorAttachmentWriteBit, + PipelineStageFlags2.ColorAttachmentOutputBit, + AccessFlags2.ColorAttachmentWriteBit | AccessFlags2.ColorAttachmentReadBit); + } + + /// + /// The same, for the transient depth/stencil buffer. Both aspects move + /// together because the image carries both and the pass names it as both a + /// depth and a stencil attachment. + /// + private void TransitionBackbufferDepth( + CommandBuffer commands, + VulkanBackbufferAttachments attachments) + { + bool first = !attachments.DepthLayoutInitialized; + attachments.MarkDepthLayoutInitialized(); + const PipelineStageFlags2 DepthStages = + PipelineStageFlags2.EarlyFragmentTestsBit | PipelineStageFlags2.LateFragmentTestsBit; + TransitionImage( + commands, + attachments.DepthImage, + ImageAspectFlags.DepthBit | ImageAspectFlags.StencilBit, + first ? ImageLayout.Undefined : ImageLayout.DepthStencilAttachmentOptimal, + ImageLayout.DepthStencilAttachmentOptimal, + first ? PipelineStageFlags2.TopOfPipeBit : DepthStages, + first ? AccessFlags2.None : AccessFlags2.DepthStencilAttachmentWriteBit, + DepthStages, + AccessFlags2.DepthStencilAttachmentWriteBit | AccessFlags2.DepthStencilAttachmentReadBit); + } + private void TransitionRenderTargetForRendering(CommandBuffer commands, VulkanGpuRenderTarget target) { TransitionImage( @@ -670,12 +734,29 @@ internal sealed unsafe partial class VulkanGpuDevice } /// - /// Reads the presented image back as tightly packed top-left-origin RGBA8. + /// Reads the last presented frame back as tightly packed top-left-origin + /// RGBA8. /// /// The swapchain is B8G8R8A8_UNORM (plan §4.9), so the channels /// are swizzled on the CPU to preserve FrameScreenshotController's /// RGBA byte contract — the same seam every automated screenshot gate already /// uses, so the comparison tooling is unaffected by the backend swap. + /// + /// Campaign V slice V6g: it reads a device-owned copy, not the + /// swapchain image. V6c transitioned the LAST PRESENTED swapchain image + /// to TRANSFER_SRC and copied out of it, which the validation layer + /// rejects as UNASSIGNED-non-acquired-swapchain-image-used: once + /// vkQueuePresentKHR has taken an image, the presentation engine owns + /// it and its contents are not the application's to read. The pixels were + /// usually right, which is precisely what makes it dangerous — this campaign + /// spent three sections (§5.5.1–§5.5.3) discovering how much a capture + /// instrument that is "usually right" can cost. So the frame copies its own + /// output into a host-readable buffer while it still owns the image, and this + /// method reads that. + /// + /// Retention is opt-in and off in production: it costs one full-res + /// image-to-buffer copy per frame, which is worth nothing to a player and is + /// the entire instrument to a gate. /// public byte[] CaptureBackbuffer(int width, int height) { @@ -684,19 +765,98 @@ internal sealed unsafe partial class VulkanGpuDevice ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height); if (_backbuffer is null) throw new InvalidOperationException("This device has no backbuffer to capture."); + if (_captureBuffer is null) + { + throw new InvalidOperationException( + "Backbuffer capture was not retained by this device. Construct it with " + + "retainBackbufferCapture: true — reading the presented swapchain image " + + "instead is a Vulkan usage error (see this method's remarks)."); + } + if (width != _captureWidth || height != _captureHeight) + { + throw new ArgumentException( + $"The retained capture is {_captureWidth}x{_captureHeight}; {width}x{height} was requested. " + + "The capture buffer is sized with the swapchain, so a mismatch means the caller " + + "and the backbuffer disagree about the frame that was just presented."); + } - return CaptureImage(_backbuffer.ImageAt(_lastPresentedImageIndex), (uint)width, (uint)height); + // Everything that could still be writing the buffer is a submitted frame. + VulkanInterop.Check(_vk.DeviceWaitIdle(_device), "vkDeviceWaitIdle (capture)"); + var pixels = new byte[(long)_captureWidth * _captureHeight * 4]; + _captureBuffer.Read(0, pixels); + // ToRgba, NOT ToGlOriginRgba: IGpuDevice.CaptureBackbuffer is documented + // as top-left-origin, and a Vulkan image already is. + return VulkanBackbufferSwizzle.ToRgba(pixels, width, height, width * 4); } - private uint _lastPresentedImageIndex; - private bool _backbufferRenderingReady; - - private byte[] CaptureImage(Image image, uint width, uint height) + /// + /// Records the acquired image's contents into the retained capture buffer, + /// while the frame still owns the image. Returns the layout the image is + /// left in, which the present barrier has to start from. + /// + internal ImageLayout RecordBackbufferCapture(CommandBuffer commands, Image image) { - uint byteCount = width * height * 4; - VulkanInterop.Check(_vk.DeviceWaitIdle(_device), "vkDeviceWaitIdle (capture)"); + if (_captureBuffer is null || _backbuffer is null) + return ImageLayout.ColorAttachmentOptimal; + if (_captureWidth != _backbuffer.Width || _captureHeight != _backbuffer.Height) + return ImageLayout.ColorAttachmentOptimal; - var readback = new VulkanGpuBuffer( + TransitionImage( + commands, + image, + ImageAspectFlags.ColorBit, + ImageLayout.ColorAttachmentOptimal, + ImageLayout.TransferSrcOptimal, + PipelineStageFlags2.ColorAttachmentOutputBit, + AccessFlags2.ColorAttachmentWriteBit, + PipelineStageFlags2.CopyBit, + AccessFlags2.TransferReadBit); + + var region = new BufferImageCopy + { + BufferOffset = 0, + BufferRowLength = 0, + BufferImageHeight = 0, + ImageSubresource = new ImageSubresourceLayers + { + AspectMask = ImageAspectFlags.ColorBit, + MipLevel = 0, + BaseArrayLayer = 0, + LayerCount = 1, + }, + ImageOffset = new Offset3D(0, 0, 0), + ImageExtent = new Extent3D(_captureWidth, _captureHeight, 1), + }; + _vk.CmdCopyImageToBuffer( + commands, + image, + ImageLayout.TransferSrcOptimal, + _captureBuffer.Handle, + 1, + ®ion); + return ImageLayout.TransferSrcOptimal; + } + + /// + /// Sizes the retained capture buffer with the swapchain. Called from + /// , which the host already + /// drives behind a vkDeviceWaitIdle on resize. + /// + private void ConfigureBackbufferCapture(uint width, uint height) + { + if (!_retainBackbufferCapture) + return; + if (_captureBuffer is not null && _captureWidth == width && _captureHeight == height) + return; + + _captureBuffer?.Dispose(); + _captureBuffer = null; + _captureWidth = width; + _captureHeight = height; + if (width == 0 || height == 0) + return; + + _captureBuffer = new VulkanGpuBuffer( _vk, _device, _allocator, @@ -705,116 +865,14 @@ internal sealed unsafe partial class VulkanGpuDevice _debugNames, new GpuBufferDescription( "vk-backbuffer-capture", - byteCount, + width * height * 4, GpuBufferUsage.TransferDestination, GpuMemoryResidency.HostReadable)); - CommandPool pool = default; - try - { - var poolCreate = new CommandPoolCreateInfo - { - SType = StructureType.CommandPoolCreateInfo, - QueueFamilyIndex = _graphicsFamily, - Flags = CommandPoolCreateFlags.TransientBit, - }; - VulkanInterop.Check( - _vk.CreateCommandPool(_device, &poolCreate, null, out pool), - "vkCreateCommandPool (capture)"); - var allocate = new CommandBufferAllocateInfo - { - SType = StructureType.CommandBufferAllocateInfo, - CommandPool = pool, - Level = CommandBufferLevel.Primary, - CommandBufferCount = 1, - }; - VulkanInterop.Check( - _vk.AllocateCommandBuffers(_device, &allocate, out CommandBuffer commands), - "vkAllocateCommandBuffers (capture)"); - - var begin = new CommandBufferBeginInfo - { - SType = StructureType.CommandBufferBeginInfo, - Flags = CommandBufferUsageFlags.OneTimeSubmitBit, - }; - VulkanInterop.Check(_vk.BeginCommandBuffer(commands, &begin), "vkBeginCommandBuffer (capture)"); - - TransitionImage( - commands, - image, - ImageAspectFlags.ColorBit, - ImageLayout.PresentSrcKhr, - ImageLayout.TransferSrcOptimal, - PipelineStageFlags2.AllCommandsBit, - AccessFlags2.None, - PipelineStageFlags2.CopyBit, - AccessFlags2.TransferReadBit); - - var region = new BufferImageCopy - { - BufferOffset = 0, - BufferRowLength = 0, - BufferImageHeight = 0, - ImageSubresource = new ImageSubresourceLayers - { - AspectMask = ImageAspectFlags.ColorBit, - MipLevel = 0, - BaseArrayLayer = 0, - LayerCount = 1, - }, - ImageOffset = new Offset3D(0, 0, 0), - ImageExtent = new Extent3D(width, height, 1), - }; - _vk.CmdCopyImageToBuffer( - commands, - image, - ImageLayout.TransferSrcOptimal, - readback.Handle, - 1, - ®ion); - - TransitionImage( - commands, - image, - ImageAspectFlags.ColorBit, - ImageLayout.TransferSrcOptimal, - ImageLayout.PresentSrcKhr, - PipelineStageFlags2.CopyBit, - AccessFlags2.TransferReadBit, - PipelineStageFlags2.AllCommandsBit, - AccessFlags2.None); - - VulkanInterop.Check(_vk.EndCommandBuffer(commands), "vkEndCommandBuffer (capture)"); - - var commandSubmit = new CommandBufferSubmitInfo - { - SType = StructureType.CommandBufferSubmitInfo, - CommandBuffer = commands, - }; - var submit = new SubmitInfo2 - { - SType = StructureType.SubmitInfo2, - CommandBufferInfoCount = 1, - PCommandBufferInfos = &commandSubmit, - }; - VulkanInterop.Check(_vk.QueueSubmit2(_graphicsQueue, 1, &submit, default), "vkQueueSubmit2 (capture)"); - VulkanInterop.Check(_vk.QueueWaitIdle(_graphicsQueue), "vkQueueWaitIdle (capture)"); - - var pixels = new byte[byteCount]; - readback.Read(0, pixels); - // ToRgba, NOT ToGlOriginRgba: IGpuDevice.CaptureBackbuffer is - // documented as top-left-origin, and a Vulkan image already is. - // (VulkanSwapchain.CaptureImage feeds FrameScreenshotController - // instead, which flips again on the way to the PNG, so THAT path - // flips here to cancel. Two consumers, two conventions, one - // difference — worth stating because a single wrong choice produces - // a perfectly plausible upside-down screenshot.) - return VulkanBackbufferSwizzle.ToRgba(pixels, (int)width, (int)height, (int)width * 4); - } - finally - { - if (pool.Handle != 0) - _vk.DestroyCommandPool(_device, pool, null); - readback.Dispose(); - } } + + private readonly bool _retainBackbufferCapture; + private VulkanGpuBuffer? _captureBuffer; + private uint _captureWidth; + private uint _captureHeight; + private bool _backbufferRenderingReady; } diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.cs index 5d392341..bb6b732f 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuDevice.cs @@ -117,8 +117,13 @@ internal sealed unsafe partial class VulkanGpuDevice : IGpuDevice string? shaderSpirvDirectory = null, string? pipelineCacheDirectory = null, int ringCapacityBytesPerSlot = DefaultRingCapacityBytesPerSlot, - int framesInFlight = VulkanFrameFlightController.DefaultFramesInFlight) + int framesInFlight = VulkanFrameFlightController.DefaultFramesInFlight, + bool retainBackbufferCapture = false) { + // Slice V6g. Off by default because it costs a full-resolution + // image-to-buffer copy per frame; on for the automated gates, which is + // the only legal way to read a frame back — see CaptureBackbuffer. + _retainBackbufferCapture = retainBackbufferCapture; _vk = vk ?? throw new ArgumentNullException(nameof(vk)); ArgumentNullException.ThrowIfNull(features); ArgumentNullException.ThrowIfNull(limits); @@ -406,7 +411,12 @@ internal sealed unsafe partial class VulkanGpuDevice : IGpuDevice if (_acquiredImageIndex is { } imageIndex && _backbuffer is not null) { - TransitionBackbufferForPresent(commands, _backbuffer.ImageAt(imageIndex)); + Image presentable = _backbuffer.ImageAt(imageIndex); + // Slice V6g: the ONLY point at which this frame's output may legally + // be read is here, while the image is still acquired. After the + // present below it belongs to the presentation engine. + ImageLayout current = RecordBackbufferCapture(commands, presentable); + TransitionBackbufferForPresent(commands, presentable, current); } VulkanInterop.Check(_vk.EndCommandBuffer(commands), "vkEndCommandBuffer (frame)"); @@ -461,7 +471,6 @@ internal sealed unsafe partial class VulkanGpuDevice : IGpuDevice if (_acquiredImageIndex is { } toPresent && _backbuffer is not null) { - _lastPresentedImageIndex = toPresent; PresentSucceeded = _backbuffer.Present(toPresent); _acquiredImageIndex = null; } @@ -470,16 +479,21 @@ internal sealed unsafe partial class VulkanGpuDevice : IGpuDevice /// False after a present that reported the swapchain should be rebuilt. internal bool PresentSucceeded { get; private set; } = true; - private void TransitionBackbufferForPresent(CommandBuffer commands, Image image) + private void TransitionBackbufferForPresent(CommandBuffer commands, Image image, ImageLayout currentLayout) { + bool captured = currentLayout == ImageLayout.TransferSrcOptimal; var barrier = new ImageMemoryBarrier2 { SType = StructureType.ImageMemoryBarrier2, - SrcStageMask = PipelineStageFlags2.ColorAttachmentOutputBit, - SrcAccessMask = AccessFlags2.ColorAttachmentWriteBit, + SrcStageMask = captured + ? PipelineStageFlags2.CopyBit + : PipelineStageFlags2.ColorAttachmentOutputBit, + SrcAccessMask = captured + ? AccessFlags2.TransferReadBit + : AccessFlags2.ColorAttachmentWriteBit, DstStageMask = PipelineStageFlags2.BottomOfPipeBit, DstAccessMask = AccessFlags2.None, - OldLayout = ImageLayout.ColorAttachmentOptimal, + OldLayout = currentLayout, NewLayout = ImageLayout.PresentSrcKhr, SrcQueueFamilyIndex = Silk.NET.Vulkan.Vk.QueueFamilyIgnored, DstQueueFamilyIndex = Silk.NET.Vulkan.Vk.QueueFamilyIgnored, diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPassEncoder.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPassEncoder.cs index 8a041beb..48cec39e 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPassEncoder.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPassEncoder.cs @@ -29,6 +29,7 @@ internal sealed unsafe class VulkanGpuPassEncoder : IGpuPassEncoder private readonly CommandBuffer _commands; private readonly VulkanFrameBindings _bindings; private readonly uint _attachmentHeight; + private readonly bool _hasDepthAttachment; private VulkanGpuPipeline? _pipeline; private bool _closed; @@ -40,13 +41,19 @@ internal sealed unsafe class VulkanGpuPassEncoder : IGpuPassEncoder VulkanFrameBindings bindings, GpuPassDescription pass, uint attachmentWidth, - uint attachmentHeight) + uint attachmentHeight, + bool hasDepthAttachment) { _device = device; _frame = frame; _commands = commands; _bindings = bindings; _attachmentHeight = attachmentHeight; + // Slice V6g: what vkCmdBeginRendering was actually handed, not what the + // description asked for. A backbuffer pass that requests depth before + // the attachments exist gets none, and the pipeline variant has to agree + // with the command buffer rather than with the intent. + _hasDepthAttachment = hasDepthAttachment; Pass = pass; // A pass always starts with the whole attachment drawable. GL's @@ -68,7 +75,10 @@ internal sealed unsafe class VulkanGpuPassEncoder : IGpuPassEncoder throw new ArgumentException("The Vulkan backend can only bind a Vulkan pipeline.", nameof(pipeline)); _pipeline = vulkanPipeline; - _device.Api.CmdBindPipeline(_commands, PipelineBindPoint.Graphics, vulkanPipeline.Handle); + _device.Api.CmdBindPipeline( + _commands, + PipelineBindPoint.Graphics, + vulkanPipeline.HandleFor(_hasDepthAttachment)); // Every pipeline shares one layout, so the descriptor sets and push // constants bound earlier in the pass survive this call. That is the diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPipeline.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPipeline.cs index f2ed1bf3..7eee3dce 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPipeline.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuPipeline.cs @@ -17,12 +17,34 @@ namespace AcDream.App.Rendering.Gpu.Vk; /// core in 1.3. That is what lets a pass be described by /// alone rather than by an object that has to be /// created, cached and matched. +/// +/// Two variants, selected at bind time (slice V6g). Dynamic +/// rendering bakes the depth/stencil attachment FORMAT into the pipeline, and it +/// must equal the format of the pass the pipeline draws in — UNDEFINED +/// when the pass has no depth attachment, the real format when it has one. V6c +/// declared the format only when the pipeline itself tested or wrote depth, +/// which made every depth-off pipeline malformed the moment it drew inside a +/// depth-carrying pass. That is not an edge case: debug lines, the retained UI +/// and the sky are all depth-off and all draw inside the main pass, and plan +/// §5.5.7 recorded it firing as +/// VUID-vkCmdDraw-dynamicRenderingUnusedAttachments-08914/-08917. +/// +/// The same is legitimately used in +/// both kinds of pass — ui-text opens a depth-less pass of its own, and +/// the world pass it composites over has depth — so the description cannot +/// answer the question and the backend builds both. The contract could grow a +/// depth-format field the way it grew +/// at V6d; until a slice is entitled to change the contract, materialising both +/// is the honest expression of the gap. Both are built at startup against the +/// persisted cache, so no frame ever compiles one. /// internal sealed unsafe class VulkanGpuPipeline : IGpuPipeline { private readonly Silk.NET.Vulkan.Vk _vk; private readonly Device _device; private readonly IGpuResourceRetirementQueue _retirement; + private readonly Pipeline _withDepthAttachment; + private readonly Pipeline _withoutDepthAttachment; private bool _disposed; internal VulkanGpuPipeline( @@ -181,12 +203,8 @@ internal sealed unsafe class VulkanGpuPipeline : IGpuPipeline SType = StructureType.PipelineRenderingCreateInfo, ColorAttachmentCount = 1, PColorAttachmentFormats = &color, - DepthAttachmentFormat = description.Depth.Test || description.Depth.Write - ? depthStencilFormat - : Format.Undefined, - StencilAttachmentFormat = description.Depth.Test || description.Depth.Write - ? depthStencilFormat - : Format.Undefined, + DepthAttachmentFormat = depthStencilFormat, + StencilAttachmentFormat = depthStencilFormat, }; var create = new GraphicsPipelineCreateInfo @@ -210,10 +228,26 @@ internal sealed unsafe class VulkanGpuPipeline : IGpuPipeline }; VulkanInterop.Check( - _vk.CreateGraphicsPipelines(_device, cache, 1, &create, null, out Pipeline pipeline), - $"vkCreateGraphicsPipelines ('{description.Name}')"); - Handle = pipeline; - debugNames.NamePipeline(pipeline, description.Name); + _vk.CreateGraphicsPipelines(_device, cache, 1, &create, null, out Pipeline withDepth), + $"vkCreateGraphicsPipelines ('{description.Name}', depth attachment)"); + _withDepthAttachment = withDepth; + debugNames.NamePipeline(withDepth, description.Name); + + rendering.DepthAttachmentFormat = Format.Undefined; + rendering.StencilAttachmentFormat = Format.Undefined; + try + { + VulkanInterop.Check( + _vk.CreateGraphicsPipelines(_device, cache, 1, &create, null, out Pipeline withoutDepth), + $"vkCreateGraphicsPipelines ('{description.Name}', no depth attachment)"); + _withoutDepthAttachment = withoutDepth; + debugNames.NamePipeline(withoutDepth, $"{description.Name}-nodepth"); + } + catch + { + _vk.DestroyPipeline(_device, withDepth, null); + throw; + } } finally { @@ -223,15 +257,28 @@ internal sealed unsafe class VulkanGpuPipeline : IGpuPipeline public GpuPipelineDescription Description { get; } - internal Pipeline Handle { get; } + /// + /// The variant whose declared depth/stencil format matches the open pass. + /// Binding the wrong one is undefined behaviour that only a validation layer + /// reports, which is why the caller is never allowed to guess: the value + /// comes from whether vkCmdBeginRendering was handed a depth image + /// view, not from what the pass description asked for. + /// + internal Pipeline HandleFor(bool passHasDepthAttachment) => + passHasDepthAttachment ? _withDepthAttachment : _withoutDepthAttachment; public void Dispose() { if (_disposed) return; _disposed = true; - Pipeline handle = Handle; - _retirement.Retire(() => _vk.DestroyPipeline(_device, handle, null)); + Pipeline withDepth = _withDepthAttachment; + Pipeline withoutDepth = _withoutDepthAttachment; + _retirement.Retire(() => + { + _vk.DestroyPipeline(_device, withDepth, null); + _vk.DestroyPipeline(_device, withoutDepth, null); + }); } } diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuRenderTarget.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuRenderTarget.cs index 68842377..60f22c15 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuRenderTarget.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanGpuRenderTarget.cs @@ -164,6 +164,35 @@ internal sealed unsafe class VulkanBackbufferAttachments : IDisposable internal bool HasDepth => _depthView.Handle != 0; + /// + /// Whether each attachment has been moved out of VK_IMAGE_LAYOUT_UNDEFINED + /// since it was last (re)created. + /// + /// Campaign V slice V6g. Both images are created with an UNDEFINED + /// initial layout and both are named in vkCmdBeginRendering as + /// COLOR_ATTACHMENT_OPTIMAL / DEPTH_STENCIL_ATTACHMENT_OPTIMAL, + /// and nothing ever moved them — plan §5.5.7 defect 3, reported as + /// VUID-vkCmdBeginRendering-pRenderingInfo-09588/-09590/-09592. The + /// swapchain image had a transition from the start; these two were simply + /// missed, because the swapchain's is acquired per frame and these are not. + /// + /// + /// The flag is per (re)creation rather than per frame: after a pass + /// ends, dynamic rendering leaves the image in the layout the pass declared, + /// so only the first use after starts from UNDEFINED. + /// Every later use still needs a barrier — for the write-after-write + /// dependency between frames, not for the layout — which is why the caller + /// issues one either way and only the old layout and source masks differ. + /// + /// + internal bool ColorLayoutInitialized { get; private set; } + + internal bool DepthLayoutInitialized { get; private set; } + + internal void MarkColorLayoutInitialized() => ColorLayoutInitialized = true; + + internal void MarkDepthLayoutInitialized() => DepthLayoutInitialized = true; + /// /// Rebuilds both attachments for a new extent, format or sample count. /// A no-op when nothing changed, so the host may call it every frame. @@ -280,6 +309,10 @@ internal sealed unsafe class VulkanBackbufferAttachments : IDisposable _depthView = default; _depthImage = default; _depthAllocation = default; + // A recreated image is a new image in UNDEFINED layout, whatever the old + // one had reached. + ColorLayoutInitialized = false; + DepthLayoutInitialized = false; } public void Dispose() diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanInterop.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanInterop.cs index df359f5d..1f74bf7b 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanInterop.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanInterop.cs @@ -362,6 +362,8 @@ internal static unsafe class VulkanPhysicalDeviceInspector MaxPushConstantsSize = limits.MaxPushConstantsSize, MaxClipDistances = limits.MaxClipDistances, MaxBoundDescriptorSets = limits.MaxBoundDescriptorSets, + MaxDescriptorSetStorageBuffersDynamic = limits.MaxDescriptorSetStorageBuffersDynamic, + MaxDescriptorSetUniformBuffersDynamic = limits.MaxDescriptorSetUniformBuffersDynamic, MaxDescriptorSetUpdateAfterBindSampledImages = indexing.MaxDescriptorSetUpdateAfterBindSampledImages, MaxPerStageDescriptorUpdateAfterBindSampledImages = diff --git a/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs b/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs index 8bc862c8..a4ac876f 100644 --- a/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs +++ b/src/AcDream.App/Rendering/Gpu/Vk/VulkanPipelineLayouts.cs @@ -82,19 +82,78 @@ internal static unsafe class VulkanPipelineLayouts } /// - /// Set 0 — the ten storage bindings pins. + /// Campaign V slice V6g: which of the ten storage bindings gets a DYNAMIC + /// descriptor, and why not all of them. /// - /// DYNAMIC storage buffers, because the RHI contract lets a renderer - /// bind an arbitrary range per draw and ring allocations move that range - /// every frame. A non-dynamic descriptor would have to be rewritten each - /// time, putting a vkUpdateDescriptorSets in the hot path — exactly the cost - /// the texture table was designed to remove. The dynamic offset travels in - /// vkCmdBindDescriptorSets instead, which is free. + /// V6b declared all ten STORAGE_BUFFER_DYNAMIC, on the reasoning + /// that the contract lets a renderer bind an arbitrary range per draw. That + /// met a real device limit the first time a validation layer looked at it: + /// maxDescriptorSetStorageBuffersDynamic is 8 on the RX 9070 XT and + /// only 4 at Vulkan's guaranteed minimum, so ten was never portable — + /// see plan §5.5.7 defect 1. /// - /// Ten dynamic storage descriptors is above Vulkan`s guaranteed - /// minimum of four, so this is a real requirement rather than a free choice. - /// It is asserted at layout creation, which fails loudly at startup on a - /// device that cannot serve it rather than at the first draw. + /// The rule. A dynamic descriptor buys exactly one thing: the + /// ability to address the SAME buffer at a DIFFERENT offset without a + /// descriptor write. That is the shape of a per-frame ring allocation, so + /// the bindings a renderer feeds from the ring stay dynamic and the offset + /// travels in vkCmdBindDescriptorSets for free. Bindings that point at + /// a long-lived, renderer-owned buffer written whole and bound once per pass + /// buy nothing from it, and each one costs a scarce device resource. + /// + /// Four dynamic descriptors is not merely under the RX 9070 XT's 8 — it + /// is exactly Vulkan's guaranteed minimum, so no device that can run acdream + /// at all can fail this layout. That matters for slice V9's lavapipe row and + /// for whatever Linux driver the deferred physical row eventually uses. + /// + /// Binding 9 is the clearest case. The texture table is the + /// GL-only uvec2 handle-buffer emulation; the Vulkan backend binds set + /// 2 instead and never touches binding 9 at all, so a dynamic descriptor for + /// it would be a device resource spent on a binding that is provably never + /// bound. + /// + /// What to do if V4c disagrees. Bindings 6, 7 and 8 are + /// per-instance arrays grouped here with the frame-global tables because + /// their owner writes them whole once per frame. If the Vulkan world path + /// turns out to re-point one of them at a moving ring offset per draw, + /// promoting it back is one line here plus one in + /// — and there are four unused dynamic + /// slots to promote into before the guaranteed minimum is exceeded. + /// + internal static bool IsDynamicStorageBinding(uint binding) => binding switch + { + // Per-frame ring uploads: the instance transform array, the per-draw + // batch table, and the two arrays the world dispatcher chunks alongside + // instances. + GpuBindingModel.StorageInstances => true, + GpuBindingModel.StorageBatches => true, + GpuBindingModel.StorageClipSlots => true, + GpuBindingModel.StorageInstanceLightSets => true, + _ => false, + }; + + /// + /// How many of set 0's bindings are dynamic. Asserted against + /// maxDescriptorSetStorageBuffersDynamic by the capability gate, so a + /// device that cannot serve the layout is rejected at startup with the + /// exit-code-4 contract rather than at vkCreatePipelineLayout. + /// + internal static uint DynamicStorageBindingCount { get; } = CountDynamicStorageBindings(); + + private static uint CountDynamicStorageBindings() + { + uint count = 0; + for (uint binding = 0; binding < GpuBindingModel.StorageBindingCount; binding++) + { + if (IsDynamicStorageBinding(binding)) + count++; + } + + return count; + } + + /// + /// Set 0 — the ten storage bindings pins, split + /// between dynamic and plain by . /// internal static DescriptorSetLayout CreateStorageSetLayout(Silk.NET.Vulkan.Vk vk, Device device) { @@ -105,7 +164,9 @@ internal static unsafe class VulkanPipelineLayouts bindings[i] = new DescriptorSetLayoutBinding { Binding = (uint)i, - DescriptorType = DescriptorType.StorageBufferDynamic, + DescriptorType = IsDynamicStorageBinding((uint)i) + ? DescriptorType.StorageBufferDynamic + : DescriptorType.StorageBuffer, DescriptorCount = 1, StageFlags = ShaderStageFlags.VertexBit | ShaderStageFlags.FragmentBit, }; diff --git a/tests/AcDream.App.Tests/Rendering/Gpu/Vk/VulkanCapabilityGateTests.cs b/tests/AcDream.App.Tests/Rendering/Gpu/Vk/VulkanCapabilityGateTests.cs index e209563c..e6ca461b 100644 --- a/tests/AcDream.App.Tests/Rendering/Gpu/Vk/VulkanCapabilityGateTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Gpu/Vk/VulkanCapabilityGateTests.cs @@ -193,6 +193,64 @@ public sealed class VulkanCapabilityGateTests StringComparison.Ordinal)); } + /// + /// Campaign V slice V6g, plan §5.5.7 defect 1. The V6b layout declared all + /// ten of set 0's bindings STORAGE_BUFFER_DYNAMIC and was rejected by + /// the RX 9070 XT's limit of 8 — silently, and only visible under the + /// validation layer. The split has to stay inside Vulkan's GUARANTEED + /// minimum, not merely inside one device's: at four, no conformant + /// implementation can fail the layout, which is what slice V9's lavapipe row + /// and the deferred physical Linux row depend on. + /// + [Fact] + public void TheDynamicStorageBindingSplitFitsVulkansGuaranteedMinimum() + { + const uint VulkanGuaranteedMinimum = 4; + Assert.True( + VulkanPipelineLayouts.DynamicStorageBindingCount <= VulkanGuaranteedMinimum, + $"set 0 declares {VulkanPipelineLayouts.DynamicStorageBindingCount} dynamic storage " + + $"bindings; Vulkan only guarantees {VulkanGuaranteedMinimum}."); + + // The bindings that stay dynamic are the ones a renderer feeds from the + // per-frame ring; the rest point at long-lived buffers bound once per + // pass and buy nothing from a dynamic offset. + Assert.True(VulkanPipelineLayouts.IsDynamicStorageBinding(GpuBindingModel.StorageInstances)); + Assert.True(VulkanPipelineLayouts.IsDynamicStorageBinding(GpuBindingModel.StorageBatches)); + Assert.False(VulkanPipelineLayouts.IsDynamicStorageBinding(GpuBindingModel.StorageGlobalLights)); + Assert.False(VulkanPipelineLayouts.IsDynamicStorageBinding(GpuBindingModel.StorageClipRegions)); + + // Binding 9 is the GL-only uvec2 handle-table emulation. The Vulkan + // backend binds set 2 instead and never touches it, so spending a scarce + // dynamic descriptor on it would be spending one on a binding that is + // provably never bound. + Assert.False(VulkanPipelineLayouts.IsDynamicStorageBinding(GpuBindingModel.StorageTextureTable)); + } + + [Fact] + public void ADeviceWithTooFewDynamicBufferDescriptorsIsRejected() + { + VulkanCapabilityRecord storage = SupportedRecord( + limits: VulkanDeviceLimitSupport.Complete with + { + MaxDescriptorSetStorageBuffersDynamic = + VulkanPipelineLayouts.DynamicStorageBindingCount - 1, + }); + VulkanCapabilityRecord uniform = SupportedRecord( + limits: VulkanDeviceLimitSupport.Complete with + { + MaxDescriptorSetUniformBuffersDynamic = + VulkanFrameBindings.DynamicUniformBindingCount - 1, + }); + + Assert.Contains( + storage.SupportFailures, + failure => failure.Contains("dynamic storage bindings", StringComparison.Ordinal)); + Assert.Contains( + uniform.SupportFailures, + failure => failure.Contains("dynamic uniform bindings", StringComparison.Ordinal)); + Assert.Empty(SupportedRecord().SupportFailures); + } + [Fact] public void ATextureTableSmallerThanTheCapacityIsRejectedPerSetAndPerStage() {