From 17b00a3d054b434f067860317041755cce19ac3a Mon Sep 17 00:00:00 2001 From: Bill Currie Date: Wed, 24 Jan 2024 17:35:28 +0900 Subject: [PATCH] [vulkan] Enable synchronization validation And clean up the resulting errors. While some were tricky, there weren't all that many: just some attachment issues and the multi-stage image copy for scraps. Fixing scraps required a barrier between copies. It might be overkill, but a transfer_dst to transfer_dst image barrier worked. Fixing attachments was a bit trickier: - depth needed early and late fragment tests to be treated as one stage - all attachments that were read later needed storeOp = none (using the extension) - and then finalLayout needed to be correct to avoid ghost transitions - as well, for some reason the deffered gbuffer subpass needed a depth dependency on the translucent pass even though neither one writes to the depth attachment (possibly a validation bug, needs more investigation). --- include/QF/Vulkan/barrier.h | 1 + libs/video/renderer/vulkan/barrier.c | 13 +++++++ libs/video/renderer/vulkan/instance.c | 1 + libs/video/renderer/vulkan/render.c | 2 + libs/video/renderer/vulkan/rp_main_def.plist | 37 ++++++++----------- libs/video/renderer/vulkan/rp_main_fwd.plist | 28 +++++++------- libs/video/renderer/vulkan/scrap.c | 13 ++++++- .../video/renderer/vulkan/vulkan_vid_common.c | 1 + 8 files changed, 58 insertions(+), 38 deletions(-) diff --git a/include/QF/Vulkan/barrier.h b/include/QF/Vulkan/barrier.h index 9880a56f0..0163a5d89 100644 --- a/include/QF/Vulkan/barrier.h +++ b/include/QF/Vulkan/barrier.h @@ -23,6 +23,7 @@ enum { qfv_LT_Undefined_to_TransferDst, qfv_LT_Undefined_to_General, qfv_LT_Undefined_to_ShaderReadOnly, + qfv_LT_TransferDst_to_TransferDst, qfv_LT_TransferDst_to_TransferSrc, qfv_LT_TransferDst_to_General, qfv_LT_TransferDst_to_ShaderReadOnly, diff --git a/libs/video/renderer/vulkan/barrier.c b/libs/video/renderer/vulkan/barrier.c index 20ea50195..a34d5033c 100644 --- a/libs/video/renderer/vulkan/barrier.c +++ b/libs/video/renderer/vulkan/barrier.c @@ -70,6 +70,19 @@ const qfv_imagebarrier_t imageBarriers[] = { { VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1 } }, }, + [qfv_LT_TransferDst_to_TransferDst] = { + .srcStages = VK_PIPELINE_STAGE_TRANSFER_BIT, + .dstStages = VK_PIPELINE_STAGE_TRANSFER_BIT, + .barrier = { + VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER, 0, + VK_ACCESS_TRANSFER_WRITE_BIT, + VK_ACCESS_TRANSFER_WRITE_BIT, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + VK_QUEUE_FAMILY_IGNORED, VK_QUEUE_FAMILY_IGNORED, 0, + { VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1 } + }, + }, [qfv_LT_TransferDst_to_TransferSrc] = { .srcStages = VK_PIPELINE_STAGE_TRANSFER_BIT, .dstStages = VK_PIPELINE_STAGE_TRANSFER_BIT, diff --git a/libs/video/renderer/vulkan/instance.c b/libs/video/renderer/vulkan/instance.c index 594915f6a..83cb2577c 100644 --- a/libs/video/renderer/vulkan/instance.c +++ b/libs/video/renderer/vulkan/instance.c @@ -217,6 +217,7 @@ QFV_CreateInstance (vulkan_ctx_t *ctx, }; VkValidationFeatureEnableEXT valfeat_enable[] = { // VK_VALIDATION_FEATURE_ENABLE_BEST_PRACTICES_EXT, + VK_VALIDATION_FEATURE_ENABLE_SYNCHRONIZATION_VALIDATION_EXT, }; #define valfeat_count sizeof(valfeat_enable)/sizeof(valfeat_enable[0]) VkValidationFeaturesEXT validation_features= { diff --git a/libs/video/renderer/vulkan/render.c b/libs/video/renderer/vulkan/render.c index a6b0833f6..d9eae2a19 100644 --- a/libs/video/renderer/vulkan/render.c +++ b/libs/video/renderer/vulkan/render.c @@ -191,6 +191,8 @@ QFV_RunRenderPassCmd (VkCommandBuffer cmd, vulkan_ctx_t *ctx, //the attachments won't be transitioned correctly. //However, only if not the last (or only) subpass. if (i < rp->subpass_count - 1) { + //auto np = &rp->subpasses[i + 1]; + //printf ("%s -> %s\n", sp->label.name, np->label.name); dfunc->vkCmdNextSubpass (cmd, rp->subpassContents); } } diff --git a/libs/video/renderer/vulkan/rp_main_def.plist b/libs/video/renderer/vulkan/rp_main_def.plist index 1575c8d1c..6c332722c 100644 --- a/libs/video/renderer/vulkan/rp_main_def.plist +++ b/libs/video/renderer/vulkan/rp_main_def.plist @@ -27,11 +27,11 @@ properties = { }; depth_dependency = { src = { - stage = late_fragment_tests; + stage = early_fragment_tests|late_fragment_tests; access = depth_stencil_attachment_write; }; dst = { - stage = fragment_shader|early_fragment_tests; + stage = fragment_shader|early_fragment_tests|late_fragment_tests; access = input_attachment_read|depth_stencil_attachment_read; }; flags = by_region; @@ -83,13 +83,19 @@ properties = { attachment_base = { samples = 1; loadOp = dont_care; - storeOp = dont_care; + storeOp = none_ext; stencilLoadOp = dont_care; - stencilStoreOp = dont_care; + stencilStoreOp = none_ext; initialLayout = undefined; - finalLayout = color_attachment_optimal; + finalLayout = shader_read_only_optimal; clearValue = { color = "[0, 0, 0, 1]"; }; }; + depth_base = { + @inherit = $attachment_base; + loadOp = clear; + finalLayout = depth_stencil_read_only_optimal; + clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; + }; no_cull = { depthClampEnable = false; @@ -1324,11 +1330,8 @@ renderpasses = { layers = 1; attachments = { depth = { - @inherit = $attachment_base; + @inherit = $depth_base; format = $images.depth.format; - loadOp = clear; - finalLayout = depth_stencil_attachment_optimal; - clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; view = occlusion_depth; }; }; @@ -1426,11 +1429,8 @@ renderpasses = { layers = 1; attachments = { depth = { - @inherit = $attachment_base; + @inherit = $depth_base; format = $images.cube_depth.format; - loadOp = clear; - finalLayout = depth_stencil_attachment_optimal; - clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; view = occlusion_cube_depth; }; }; @@ -1444,11 +1444,8 @@ renderpasses = { layers = 1; attachments = { depth = { - @inherit = $attachment_base; + @inherit = $depth_base; format = $images.depth.format; - loadOp = clear; - finalLayout = depth_stencil_attachment_optimal; - clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; view = depth; }; color = { @@ -1704,6 +1701,7 @@ renderpasses = { color = "[ 0.3, 0.7, 0.3, 1]"; dependencies = { depth = $depth_dependency; + translucent = $depth_dependency;//FIXME why? }; attachments = { color = { @@ -2051,11 +2049,8 @@ renderpasses = { layers = 1; attachments = { depth = { - @inherit = $attachment_base; + @inherit = $depth_base; format = $images.cube_depth.format; - loadOp = clear; - finalLayout = depth_stencil_attachment_optimal; - clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; view = cube_depth; }; color = { diff --git a/libs/video/renderer/vulkan/rp_main_fwd.plist b/libs/video/renderer/vulkan/rp_main_fwd.plist index 766bfd2aa..61f946916 100644 --- a/libs/video/renderer/vulkan/rp_main_fwd.plist +++ b/libs/video/renderer/vulkan/rp_main_fwd.plist @@ -27,11 +27,11 @@ properties = { }; depth_dependency = { src = { - stage = late_fragment_tests; + stage = early_fragment_tests|late_fragment_tests; access = depth_stencil_attachment_write; }; dst = { - stage = fragment_shader|early_fragment_tests; + stage = fragment_shader|early_fragment_tests|late_fragment_tests; access = input_attachment_read|depth_stencil_attachment_read; }; flags = by_region; @@ -84,13 +84,19 @@ properties = { attachment_base = { samples = 1; loadOp = dont_care; - storeOp = dont_care; + storeOp = none_ext; stencilLoadOp = dont_care; - stencilStoreOp = dont_care; + stencilStoreOp = none_ext; initialLayout = undefined; - finalLayout = color_attachment_optimal; + finalLayout = shader_read_only_optimal; clearValue = { color = "[0, 0, 0, 1]"; }; }; + depth_base = { + @inherit = $attachment_base; + loadOp = clear; + finalLayout = depth_stencil_read_only_optimal; + clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; + }; no_cull = { depthClampEnable = false; @@ -1062,19 +1068,14 @@ renderpasses = { layers = 1; attachments = { depth = { - @inherit = $attachment_base; + @inherit = $depth_base; format = $images.depth.format; - loadOp = clear; - finalLayout = depth_stencil_attachment_optimal; - clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; view = depth; }; color = { @inherit = $attachment_base; format = $images.color.format; loadOp = clear; - storeOp = store; - finalLayout = $render_output.finalLayout; view = color; }; output = { @@ -1299,11 +1300,8 @@ renderpasses = { layers = 1; attachments = { depth = { - @inherit = $attachment_base; + @inherit = $depth_base; format = $images.cube_depth.format; - loadOp = clear; - finalLayout = depth_stencil_attachment_optimal; - clearValue = { depthStencil = { depth = 0; stencil = 0; }; }; view = cube_depth; }; color = { diff --git a/libs/video/renderer/vulkan/scrap.c b/libs/video/renderer/vulkan/scrap.c index e130fcbab..ebe360ee9 100644 --- a/libs/video/renderer/vulkan/scrap.c +++ b/libs/video/renderer/vulkan/scrap.c @@ -311,11 +311,14 @@ QFV_ScrapFlush (scrap_t *scrap) 0, 0, 0, 0, 0, 1, &ib.barrier); - size_t offset = packet->offset, size; + auto sb = imageBarriers[qfv_LT_TransferDst_to_TransferDst]; + sb.barrier.image = scrap->image; + + size_t offset = packet->offset; vrect_t *batch = scrap->batch; while (scrap->batch_count) { for (i = 0; i < scrap->batch_count && i < 128; i++) { - size = batch->width * batch->height * scrap->bpp; + size_t size = batch->width * batch->height * scrap->bpp; copy->a[i].bufferOffset = offset; copy->a[i].imageOffset.x = batch->x; @@ -330,6 +333,12 @@ QFV_ScrapFlush (scrap_t *scrap) VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, i, copy->a); scrap->batch_count -= i; + if (scrap->batch_count) { + dfunc->vkCmdPipelineBarrier (packet->cmd, + sb.srcStages, sb.dstStages, + 0, 0, 0, 0, 0, + 1, &sb.barrier); + } } ib = imageBarriers[qfv_LT_TransferDst_to_ShaderReadOnly]; diff --git a/libs/video/renderer/vulkan/vulkan_vid_common.c b/libs/video/renderer/vulkan/vulkan_vid_common.c index d2d50bc27..6bd31aea3 100644 --- a/libs/video/renderer/vulkan/vulkan_vid_common.c +++ b/libs/video/renderer/vulkan/vulkan_vid_common.c @@ -97,6 +97,7 @@ static const char *instance_extensions[] = { static const char *device_extensions[] = { VK_KHR_SWAPCHAIN_EXTENSION_NAME, + VK_EXT_LOAD_STORE_OP_NONE_EXTENSION_NAME, #ifdef TRACY_ENABLE VK_EXT_CALIBRATED_TIMESTAMPS_EXTENSION_NAME, #endif