fix(window): silence per-frame and setup-path Vulkan validation errors (#153) #154
5 changed files with 265 additions and 4 deletions
fix(window): silence per-frame and setup-path Vulkan validation errors (#153)
Two distinct validation errors the native frame loop emitted, both originating in Crafter.Graphics with no consumer-side influence. Problem 1 — per-frame acquire-barrier access/stage mismatch. The acquire->GENERAL barrier hardcoded dstAccessMask = SHADER_WRITE|TRANSFER_WRITE but used the per-pass stage union as its dst stage mask. For an all-compute frame the union narrows to COMPUTE_SHADER, which does not support TRANSFER_WRITE, so VUID-02820 fired every frame. Derive the access mask from the same stage union via a new SwapchainWriterAccess() helper (mirroring SwapchainStageUnion), and apply it to both the acquire dst and present src masks for symmetry. Problem 2 — mid-session StartInit/FinishInit (and GetCmd/EndCmd) reuse the shared drawCmdBuffers[currentBuffer]. With no steady-state wait-idle the loop's last submission of that buffer may still be in flight when scene setup runs (building map meshes / acceleration structures), so the old code re-began (VUID-00049) and re-submitted (VUID-00071) a pending buffer, and resources freed in the StartInit..FinishInit bracket could still be referenced by it. Drain the queue at the start of StartInit/GetCmd before re-recording; setup is rare, so a wait-idle is fine (FinishInit/EndCmd already wait-idle at the end). Tests: extend SwapchainBarrierScope with SwapchainWriterAccess coverage (pure CPU), and add SetupCmdBufferReuse — a real-frame-loop regression test driving a compute pass plus interleaved mid-session StartInit rounds, asserting the validation layer stays silent. Verified both halves fail (reproducing the exact VUIDs) when their respective fix is reverted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
commit
7316e51dca
|
|
@ -482,12 +482,14 @@ Window::Window(std::uint32_t width, std::uint32_t height) : width(width), height
|
|||
.layerCount = VK_REMAINING_ARRAY_LAYERS,
|
||||
};
|
||||
|
||||
// Transition into VK_IMAGE_LAYOUT_GENERAL before recording passes. image
|
||||
// and oldLayout are patched per frame (oldLayout depends on first use).
|
||||
// Transition into VK_IMAGE_LAYOUT_GENERAL before recording passes. image,
|
||||
// oldLayout (depends on first use) and dstAccessMask (derived from the
|
||||
// per-pass stage union) are patched per frame in Render(); the dstAccessMask
|
||||
// here is just a well-formed default.
|
||||
acquireBarrier = {
|
||||
.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER,
|
||||
.srcAccessMask = 0,
|
||||
.dstAccessMask = VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT,
|
||||
.dstAccessMask = VK_ACCESS_SHADER_WRITE_BIT,
|
||||
.oldLayout = VK_IMAGE_LAYOUT_UNDEFINED,
|
||||
.newLayout = VK_IMAGE_LAYOUT_GENERAL,
|
||||
.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED,
|
||||
|
|
@ -496,7 +498,9 @@ Window::Window(std::uint32_t width, std::uint32_t height) : width(width), height
|
|||
.subresourceRange = range,
|
||||
};
|
||||
|
||||
// Transition back to PRESENT_SRC_KHR after the passes. Only image varies.
|
||||
// Transition back to PRESENT_SRC_KHR after the passes. image and
|
||||
// srcAccessMask (derived from the per-pass stage union) are patched per
|
||||
// frame in Render(); the srcAccessMask here is just a well-formed default.
|
||||
presentBarrier = {
|
||||
.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER,
|
||||
.srcAccessMask = VK_ACCESS_SHADER_WRITE_BIT,
|
||||
|
|
@ -835,6 +839,13 @@ void Window::Render() {
|
|||
// barrier below (passes don't change within a Render()).
|
||||
const VkPipelineStageFlags swapWriterStages = SwapchainStageUnion(passes);
|
||||
|
||||
// dstAccessMask must only contain access flags supported by the dst stage
|
||||
// mask (VUID-vkCmdPipelineBarrier-pImageMemoryBarriers-02820). Derive it
|
||||
// from the same per-pass stage union the barrier uses as its dst stage mask
|
||||
// instead of hardcoding SHADER_WRITE|TRANSFER_WRITE, which would carry
|
||||
// TRANSFER_WRITE into a compute-only frame's COMPUTE_SHADER dst stage.
|
||||
acquireBarrier.dstAccessMask = SwapchainWriterAccess(swapWriterStages);
|
||||
|
||||
// The acquire->GENERAL transition only needs to be visible before the first
|
||||
// pass writes the image, i.e. at the writer stages — not ALL_COMMANDS.
|
||||
vkCmdPipelineBarrier(drawCmdBuffers[currentBuffer], VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, swapWriterStages, 0, 0, nullptr, 0, nullptr, 1, &acquireBarrier);
|
||||
|
|
@ -919,6 +930,12 @@ void Window::Render() {
|
|||
|
||||
presentBarrier.image = images[currentBuffer];
|
||||
|
||||
// Mirror of the acquire barrier: srcAccessMask must be supported by the src
|
||||
// stage mask (swapWriterStages). SHADER_WRITE alone is valid for COMPUTE/RT
|
||||
// and so never fires today, but a transfer-stage writer would need
|
||||
// TRANSFER_WRITE here too — derive both from the same union for symmetry.
|
||||
presentBarrier.srcAccessMask = SwapchainWriterAccess(swapWriterStages);
|
||||
|
||||
// The ->PRESENT_SRC transition only needs to wait on the stages that
|
||||
// actually wrote the image (the per-pass union), not ALL_COMMANDS; present
|
||||
// itself is gated by the render-finished semaphore, so dst stays BOTTOM.
|
||||
|
|
@ -1107,6 +1124,20 @@ void Window::CreateSwapchain()
|
|||
VkCommandBuffer Window::StartInit() {
|
||||
VkCommandBufferBeginInfo cmdBufInfo {};
|
||||
cmdBufInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
|
||||
// StartInit reuses drawCmdBuffers[currentBuffer] — the same buffer the
|
||||
// steady-state Render() loop submits, guarded by waitFences[currentBuffer].
|
||||
// When setup runs mid-session (e.g. building map meshes / acceleration
|
||||
// structures on a scene transition) the loop's last submission of this
|
||||
// buffer may still be in flight: re-beginning it would trip
|
||||
// VUID-vkBeginCommandBuffer-commandBuffer-00049, re-submitting it
|
||||
// VUID-vkQueueSubmit-pCommandBuffers-00071, and any resource the caller
|
||||
// destroys between here and FinishInit could still be referenced by that
|
||||
// pending submission (vkDestroyBuffer-in-use). Drain the queue first so the
|
||||
// buffer is free to re-record and no in-flight frame still references the
|
||||
// resources this setup is about to replace (issue #153). Setup is rare
|
||||
// (not the per-frame path), so a full wait-idle here is fine — FinishInit
|
||||
// already wait-idles at the end.
|
||||
Device::CheckVkResult(vkQueueWaitIdle(Device::queue));
|
||||
Device::CheckVkResult(vkBeginCommandBuffer(drawCmdBuffers[currentBuffer], &cmdBufInfo));
|
||||
|
||||
// The swapchain images are deliberately NOT transitioned here. They are
|
||||
|
|
@ -1132,6 +1163,11 @@ void Window::FinishInit() {
|
|||
VkCommandBuffer Window::GetCmd() {
|
||||
VkCommandBufferBeginInfo cmdBufInfo {};
|
||||
cmdBufInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;
|
||||
// Same lifecycle hazard as StartInit: GetCmd re-begins and EndCmd
|
||||
// re-submits the shared drawCmdBuffers[currentBuffer], so drain any
|
||||
// in-flight submission of it (and free the resources it references) before
|
||||
// re-recording (issue #153).
|
||||
Device::CheckVkResult(vkQueueWaitIdle(Device::queue));
|
||||
Device::CheckVkResult(vkBeginCommandBuffer(drawCmdBuffers[currentBuffer], &cmdBufInfo));
|
||||
|
||||
VkImageSubresourceRange range{};
|
||||
|
|
|
|||
|
|
@ -69,6 +69,26 @@ export namespace Crafter {
|
|||
return stages;
|
||||
}
|
||||
|
||||
// The access mask matching a swapchain-writer stage union, for the
|
||||
// frame-edge barriers (acquire dst / present src). A barrier's access mask
|
||||
// must only contain access flags supported by its accompanying stage mask
|
||||
// (VUID-vkCmdPipelineBarrier-pImageMemoryBarriers-02820): TRANSFER_WRITE is
|
||||
// not a valid access for COMPUTE/RAY_TRACING stages, so hardcoding both
|
||||
// SHADER_WRITE and TRANSFER_WRITE fires the VUID every frame on an
|
||||
// all-compute frame. The swapchain image is written as a storage image by
|
||||
// compute/RT passes (SHADER_WRITE) and only via TRANSFER_WRITE when a
|
||||
// transfer-stage pass is present, so derive the access bits from the same
|
||||
// per-pass stage union the barrier's stage mask uses instead of hardcoding.
|
||||
inline VkAccessFlags SwapchainWriterAccess(VkPipelineStageFlags stages) {
|
||||
VkAccessFlags access = 0;
|
||||
if (stages & (VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT
|
||||
| VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR))
|
||||
access |= VK_ACCESS_SHADER_WRITE_BIT;
|
||||
if (stages & VK_PIPELINE_STAGE_TRANSFER_BIT)
|
||||
access |= VK_ACCESS_TRANSFER_WRITE_BIT;
|
||||
return access;
|
||||
}
|
||||
|
||||
// The inter-pass swapchain barrier (replacing the old queue-wide
|
||||
// VkMemoryBarrier at the #115 location). Scoped to the swapchain image's
|
||||
// single colour subresource so only that image's shader caches round-trip —
|
||||
|
|
|
|||
35
project.cpp
35
project.cpp
|
|
@ -296,6 +296,41 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
|
|||
frameLoopImpls.emplace_back("tests/FrameLoopSync/main");
|
||||
fl.GetInterfacesAndImplementations(ifaces, frameLoopImpls);
|
||||
cfg.tests.push_back(std::move(frameLoopTest));
|
||||
|
||||
// Issue #153: two frame-loop validation errors. (1) The acquire
|
||||
// barrier hardcoded dstAccessMask = SHADER_WRITE|TRANSFER_WRITE but
|
||||
// used the per-pass stage union (COMPUTE_SHADER for an all-compute
|
||||
// frame) as its dst stage, so TRANSFER_WRITE was unsupported and
|
||||
// VUID-02820 fired every frame — FrameLoopSync can't see it because
|
||||
// it runs with no passes (the union falls back to the conservative
|
||||
// writer set, which includes TRANSFER). (2) Mid-session
|
||||
// StartInit/FinishInit reuse the shared draw command buffer while
|
||||
// the loop's last submission of it is still in flight, re-beginning
|
||||
// (00049) and re-submitting (00071) a pending buffer. Drives a real
|
||||
// compute pass through the loop plus interleaved StartInit rounds
|
||||
// and asserts the layer stays silent. Needs the Wayland backend + a
|
||||
// real compositor, so it lives in the !windows block.
|
||||
Test setupReuseTest;
|
||||
Configuration& sr = setupReuseTest.config;
|
||||
sr.path = cfg.path;
|
||||
sr.name = "SetupCmdBufferReuse";
|
||||
sr.outputName = "SetupCmdBufferReuse";
|
||||
sr.type = ConfigurationType::Executable;
|
||||
sr.target = cfg.target;
|
||||
sr.march = cfg.march;
|
||||
sr.mtune = cfg.mtune;
|
||||
sr.debug = cfg.debug;
|
||||
sr.sysroot = cfg.sysroot;
|
||||
sr.dependencies = cfg.dependencies;
|
||||
sr.externalDependencies = cfg.externalDependencies;
|
||||
sr.compileFlags = cfg.compileFlags;
|
||||
sr.linkFlags = cfg.linkFlags;
|
||||
sr.defines = cfg.defines;
|
||||
sr.cFiles = cfg.cFiles;
|
||||
std::vector<fs::path> setupReuseImpls(impls.begin(), impls.end());
|
||||
setupReuseImpls.emplace_back("tests/SetupCmdBufferReuse/main");
|
||||
sr.GetInterfacesAndImplementations(ifaces, setupReuseImpls);
|
||||
cfg.tests.push_back(std::move(setupReuseTest));
|
||||
}
|
||||
|
||||
// Issue #36: BLAS build options. Drives the real hardware AS-build
|
||||
|
|
|
|||
131
tests/SetupCmdBufferReuse/main.cpp
Normal file
131
tests/SetupCmdBufferReuse/main.cpp
Normal file
|
|
@ -0,0 +1,131 @@
|
|||
/*
|
||||
Crafter®.Graphics
|
||||
Copyright (C) 2026 Catcrafts®
|
||||
catcrafts.net
|
||||
|
||||
This library is free software; you can redistribute it and/or
|
||||
modify it under the terms of the GNU Lesser General Public
|
||||
License version 3.0 as published by the Free Software Foundation;
|
||||
|
||||
This library is distributed in the hope that it will be useful,
|
||||
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
||||
Lesser General Public License for more details.
|
||||
|
||||
You should have received a copy of the GNU Lesser General Public
|
||||
License along with this library; if not, write to the Free Software
|
||||
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
|
||||
*/
|
||||
|
||||
// Issue #153: two distinct validation errors the frame loop emitted, both
|
||||
// reproduced here by driving the *real* loop against a live compositor and
|
||||
// asserting the Vulkan validation layer stays silent — same harness idea as
|
||||
// FrameLoopSync (the silent layer is the load-bearing check).
|
||||
//
|
||||
// Problem 1 — per-frame acquire-barrier access/stage mismatch. The
|
||||
// acquire->GENERAL barrier hardcoded dstAccessMask =
|
||||
// SHADER_WRITE | TRANSFER_WRITE but used the per-pass stage union as its dst
|
||||
// stage mask. For an all-compute frame the union narrows to COMPUTE_SHADER,
|
||||
// which does NOT support TRANSFER_WRITE, so VUID-02820 fired every frame.
|
||||
// FrameLoopSync runs with NO passes, where the union falls back to the
|
||||
// conservative writer union (which DOES include TRANSFER) and the VUID never
|
||||
// fires — so it cannot catch this. The key here is a real compute pass that
|
||||
// narrows SwapchainStage() to COMPUTE_SHADER, exactly like UIRenderer.
|
||||
//
|
||||
// Problem 2 — mid-session StartInit/FinishInit reuse the shared
|
||||
// drawCmdBuffers[currentBuffer]. With no steady-state wait-idle the loop's
|
||||
// last submission of that buffer is still in flight when setup runs, so the
|
||||
// old code re-began it (VUID-vkBeginCommandBuffer-commandBuffer-00049) and
|
||||
// re-submitted it (VUID-vkQueueSubmit-pCommandBuffers-00071) while pending,
|
||||
// and any resource freed in the StartInit..FinishInit bracket could still be
|
||||
// referenced by that submission. StartInit/GetCmd now drain the queue first.
|
||||
//
|
||||
// Needs a live Wayland compositor + a Vulkan device at runtime (same as the
|
||||
// windowed examples / FrameLoopSync), so it shares the native build settings
|
||||
// and is Linux-only.
|
||||
|
||||
#include "vulkan/vulkan.h"
|
||||
#include <cstdlib>
|
||||
|
||||
import Crafter.Graphics;
|
||||
import std;
|
||||
|
||||
using namespace Crafter;
|
||||
|
||||
namespace {
|
||||
|
||||
int failures = 0;
|
||||
|
||||
void Check(bool ok, std::string_view what) {
|
||||
std::println("{} {}", ok ? "PASS" : "FAIL", what);
|
||||
if (!ok) ++failures;
|
||||
}
|
||||
|
||||
// A minimal compute pass: records nothing, but narrows its swapchain write
|
||||
// stage to COMPUTE_SHADER like every real menu/game pass. That narrowing is
|
||||
// what shrinks the frame's stage union to COMPUTE_SHADER and so exposes the
|
||||
// problem-1 acquire-barrier access/stage mismatch. Record() needs no work —
|
||||
// the bug is in the frame-edge barriers the loop records around the passes,
|
||||
// not in the pass body.
|
||||
struct ComputePass : RenderPass {
|
||||
void Record(GraphicsCommandBuffer, std::uint32_t, Window&) override {}
|
||||
VkPipelineStageFlags SwapchainStage() const override {
|
||||
return VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT;
|
||||
}
|
||||
};
|
||||
|
||||
} // namespace
|
||||
|
||||
int main() {
|
||||
Device::Initialize();
|
||||
|
||||
Window window(640, 480, "SetupCmdBufferReuse test");
|
||||
|
||||
ComputePass pass;
|
||||
window.passes.push_back(&pass);
|
||||
|
||||
// ── Problem 1: all-compute frames ───────────────────────────────────────
|
||||
// Run enough frames that the acquire barrier is recorded many times with
|
||||
// swapWriterStages == COMPUTE_SHADER. Pre-fix this logged VUID-02820 once
|
||||
// per frame.
|
||||
constexpr int kWarmupFrames = 8;
|
||||
for (int i = 0; i < kWarmupFrames; ++i) window.Render();
|
||||
|
||||
Check(Device::validationErrorCount == 0,
|
||||
std::format("no validation errors over {} all-compute frames "
|
||||
"(acquire-barrier access/stage match) ({} seen)",
|
||||
kWarmupFrames, Device::validationErrorCount));
|
||||
|
||||
// ── Problem 2: mid-session setup while a frame is in flight ──────────────
|
||||
// After Render() returns there is no steady-state wait-idle, so the last
|
||||
// submission of drawCmdBuffers[currentBuffer] is still pending. Calling
|
||||
// StartInit()/FinishInit() now (as a scene transition would, to build map
|
||||
// meshes / acceleration structures) reuses that exact command buffer. The
|
||||
// setup buffer is left empty on purpose: it's the begin/submit lifecycle
|
||||
// around the in-flight buffer that the old code got wrong, not the
|
||||
// contents. Interleave more frames so each StartInit again races a fresh
|
||||
// in-flight submission.
|
||||
constexpr int kSetupRounds = 3;
|
||||
for (int r = 0; r < kSetupRounds; ++r) {
|
||||
VkCommandBuffer cmd = window.StartInit();
|
||||
Check(cmd != VK_NULL_HANDLE, "StartInit returns a usable command buffer");
|
||||
window.FinishInit();
|
||||
for (int f = 0; f < 4; ++f) window.Render();
|
||||
}
|
||||
|
||||
Check(Device::validationErrorCount == 0,
|
||||
std::format("no validation errors across {} mid-session StartInit/"
|
||||
"FinishInit rounds interleaved with rendering ({} seen)",
|
||||
kSetupRounds, Device::validationErrorCount));
|
||||
|
||||
// Drain before teardown — no steady-state wait-idle means GPU work may
|
||||
// still be in flight when the loop exits.
|
||||
Device::CheckVkResult(vkQueueWaitIdle(Device::queue));
|
||||
|
||||
if (failures != 0) {
|
||||
std::println("{} check(s) failed", failures);
|
||||
return EXIT_FAILURE;
|
||||
}
|
||||
std::println("all checks passed");
|
||||
return EXIT_SUCCESS;
|
||||
}
|
||||
|
|
@ -165,6 +165,45 @@ int main() {
|
|||
"inter-pass barrier is scoped to the single swapchain subresource");
|
||||
}
|
||||
|
||||
// ─── SwapchainWriterAccess (issue #153) ─────────────────────────────────
|
||||
// The frame-edge (acquire dst / present src) barriers must set an access
|
||||
// mask supported by their accompanying stage mask
|
||||
// (VUID-vkCmdPipelineBarrier-pImageMemoryBarriers-02820). The old code
|
||||
// hardcoded SHADER_WRITE|TRANSFER_WRITE on a barrier whose stage mask is the
|
||||
// per-pass union, so an all-compute frame carried TRANSFER_WRITE into a
|
||||
// COMPUTE_SHADER dst stage and fired the VUID every frame. The fix derives
|
||||
// the access from the same stage union, so each stage only pulls in the
|
||||
// access flags it actually supports.
|
||||
{
|
||||
// All-compute frame: COMPUTE_SHADER supports SHADER_WRITE only — and
|
||||
// crucially NOT TRANSFER_WRITE (the bit that fired the VUID).
|
||||
VkAccessFlags compute = SwapchainWriterAccess(VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT);
|
||||
Check(compute == VK_ACCESS_SHADER_WRITE_BIT,
|
||||
"compute-only writer access is SHADER_WRITE only");
|
||||
Check((compute & VK_ACCESS_TRANSFER_WRITE_BIT) == 0,
|
||||
"compute-only writer access does NOT include TRANSFER_WRITE (the VUID-02820 trap)");
|
||||
|
||||
// RT frame: RAY_TRACING_SHADER also writes via SHADER_WRITE, no transfer.
|
||||
Check(SwapchainWriterAccess(VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR) == VK_ACCESS_SHADER_WRITE_BIT,
|
||||
"RT-only writer access is SHADER_WRITE only");
|
||||
|
||||
// A transfer-stage writer pulls in TRANSFER_WRITE.
|
||||
Check(SwapchainWriterAccess(VK_PIPELINE_STAGE_TRANSFER_BIT) == VK_ACCESS_TRANSFER_WRITE_BIT,
|
||||
"transfer-only writer access is TRANSFER_WRITE only");
|
||||
|
||||
// The conservative writer union (un-overridden / empty frame) folds in
|
||||
// both — and every bit it sets is supported by some stage in the union.
|
||||
Check(SwapchainWriterAccess(kSwapchainWriterStages)
|
||||
== (VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT),
|
||||
"conservative writer union access is SHADER_WRITE | TRANSFER_WRITE");
|
||||
|
||||
// Mixed compute+RT (no transfer): still SHADER_WRITE only, no transfer.
|
||||
VkAccessFlags mixed = SwapchainWriterAccess(
|
||||
VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT | VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR);
|
||||
Check(mixed == VK_ACCESS_SHADER_WRITE_BIT,
|
||||
"compute+RT writer access is SHADER_WRITE only (no spurious TRANSFER_WRITE)");
|
||||
}
|
||||
|
||||
if (failures != 0) {
|
||||
std::println("{} check(s) failed", failures);
|
||||
return EXIT_FAILURE;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue