From ec8e5d5ef17870165e8e8ac6a02f29a56ac76060 Mon Sep 17 00:00:00 2001 From: squidbus <175574877+squidbus@users.noreply.github.com> Date: Thu, 12 Dec 2024 10:45:18 -0800 Subject: [PATCH] renderer_vulkan: Fix some color attachment indexing issues. (#1755) --- .../renderer_vulkan/vk_graphics_pipeline.cpp | 8 ++--- .../renderer_vulkan/vk_graphics_pipeline.h | 1 + .../renderer_vulkan/vk_pipeline_cache.cpp | 34 +++++++++++++------ .../renderer_vulkan/vk_platform.cpp | 1 + .../renderer_vulkan/vk_rasterizer.cpp | 1 + 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp index 814657e5..79553757 100644 --- a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp +++ b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp @@ -229,17 +229,15 @@ GraphicsPipeline::GraphicsPipeline(const Instance& instance_, Scheduler& schedul }); } - const auto it = std::ranges::find(key.color_formats, vk::Format::eUndefined); - const u32 num_color_formats = std::distance(key.color_formats.begin(), it); const vk::PipelineRenderingCreateInfoKHR pipeline_rendering_ci = { - .colorAttachmentCount = num_color_formats, + .colorAttachmentCount = key.num_color_attachments, .pColorAttachmentFormats = key.color_formats.data(), .depthAttachmentFormat = key.depth_format, .stencilAttachmentFormat = key.stencil_format, }; std::array attachments; - for (u32 i = 0; i < num_color_formats; i++) { + for (u32 i = 0; i < key.num_color_attachments; i++) { const auto& control = key.blend_controls[i]; const auto src_color = LiverpoolToVK::BlendFactor(control.color_src_factor); const auto dst_color = LiverpoolToVK::BlendFactor(control.color_dst_factor); @@ -292,7 +290,7 @@ GraphicsPipeline::GraphicsPipeline(const Instance& instance_, Scheduler& schedul const vk::PipelineColorBlendStateCreateInfo color_blending = { .logicOpEnable = false, .logicOp = vk::LogicOp::eCopy, - .attachmentCount = num_color_formats, + .attachmentCount = key.num_color_attachments, .pAttachments = attachments.data(), .blendConstants = std::array{1.0f, 1.0f, 1.0f, 1.0f}, }; diff --git a/src/video_core/renderer_vulkan/vk_graphics_pipeline.h b/src/video_core/renderer_vulkan/vk_graphics_pipeline.h index 2834fceb..703a0680 100644 --- a/src/video_core/renderer_vulkan/vk_graphics_pipeline.h +++ b/src/video_core/renderer_vulkan/vk_graphics_pipeline.h @@ -29,6 +29,7 @@ using Liverpool = AmdGpu::Liverpool; struct GraphicsPipelineKey { std::array stage_hashes; + u32 num_color_attachments; std::array color_formats; std::array color_num_formats; std::array mrt_swizzles; diff --git a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp index b9f318f7..0fa77e19 100644 --- a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp @@ -268,6 +268,7 @@ bool PipelineCache::RefreshGraphicsKey() { // `RenderingInfo` is assumed to be initialized with a contiguous array of valid color // attachments. This might be not a case as HW color buffers can be bound in an arbitrary // order. We need to do some arrays compaction at this stage + key.num_color_attachments = 0; key.color_formats.fill(vk::Format::eUndefined); key.color_num_formats.fill(AmdGpu::NumberFormat::Unorm); key.blend_controls.fill({}); @@ -277,11 +278,19 @@ bool PipelineCache::RefreshGraphicsKey() { // First pass of bindings check to idenitfy formats and swizzles and pass them to rhe shader // recompiler. - for (auto cb = 0u, remapped_cb = 0u; cb < Liverpool::NumColorBuffers; ++cb) { + for (auto cb = 0u; cb < Liverpool::NumColorBuffers; ++cb) { auto const& col_buf = regs.color_buffers[cb]; - if (skip_cb_binding || !col_buf || !regs.color_target_mask.GetMask(cb)) { + if (skip_cb_binding || !col_buf) { + // No attachment bound and no incremented index. continue; } + + const auto remapped_cb = key.num_color_attachments++; + if (!regs.color_target_mask.GetMask(cb)) { + // Bound to null handle, skip over this attachment index. + continue; + } + const auto base_format = LiverpoolToVK::SurfaceFormat(col_buf.info.format, col_buf.NumFormat()); key.color_formats[remapped_cb] = @@ -290,8 +299,6 @@ bool PipelineCache::RefreshGraphicsKey() { if (base_format == key.color_formats[remapped_cb]) { key.mrt_swizzles[remapped_cb] = col_buf.info.comp_swap.Value(); } - - ++remapped_cb; } fetch_shader = std::nullopt; @@ -385,10 +392,18 @@ bool PipelineCache::RefreshGraphicsKey() { // Second pass to fill remain CB pipeline key data for (auto cb = 0u, remapped_cb = 0u; cb < Liverpool::NumColorBuffers; ++cb) { auto const& col_buf = regs.color_buffers[cb]; - if (skip_cb_binding || !col_buf || !regs.color_target_mask.GetMask(cb) || - (key.mrt_mask & (1u << cb)) == 0) { - key.color_formats[cb] = vk::Format::eUndefined; - key.mrt_swizzles[cb] = Liverpool::ColorBuffer::SwapMode::Standard; + if (skip_cb_binding || !col_buf) { + // No attachment bound and no incremented index. + continue; + } + + if (!regs.color_target_mask.GetMask(cb) || (key.mrt_mask & (1u << cb)) == 0) { + // Attachment is masked out by either color_target_mask or shader mrt_mask. In the case + // of the latter we need to change format to undefined, and either way we need to + // increment the index for the null attachment binding. + key.color_formats[remapped_cb] = vk::Format::eUndefined; + key.mrt_swizzles[remapped_cb] = Liverpool::ColorBuffer::SwapMode::Standard; + ++remapped_cb; continue; } @@ -397,10 +412,9 @@ bool PipelineCache::RefreshGraphicsKey() { !col_buf.info.blend_bypass); key.write_masks[remapped_cb] = vk::ColorComponentFlags{regs.color_target_mask.GetMask(cb)}; key.cb_shader_mask.SetMask(remapped_cb, regs.color_shader_mask.GetMask(cb)); + ++remapped_cb; num_samples = std::max(num_samples, 1u << col_buf.attrib.num_samples_log2); - - ++remapped_cb; } // It seems that the number of samples > 1 set in the AA config doesn't mean we're always diff --git a/src/video_core/renderer_vulkan/vk_platform.cpp b/src/video_core/renderer_vulkan/vk_platform.cpp index 2e717397..f5e51361 100644 --- a/src/video_core/renderer_vulkan/vk_platform.cpp +++ b/src/video_core/renderer_vulkan/vk_platform.cpp @@ -44,6 +44,7 @@ static VKAPI_ATTR VkBool32 VKAPI_CALL DebugUtilsCallback( case 0xc81ad50e: case 0xb7c39078: case 0x32868fde: // vkCreateBufferView(): pCreateInfo->range does not equal VK_WHOLE_SIZE + case 0x1012616b: // `VK_FORMAT_UNDEFINED` does not match fragment shader output type return VK_FALSE; default: break; diff --git a/src/video_core/renderer_vulkan/vk_rasterizer.cpp b/src/video_core/renderer_vulkan/vk_rasterizer.cpp index bfcdc953..9abf1b52 100644 --- a/src/video_core/renderer_vulkan/vk_rasterizer.cpp +++ b/src/video_core/renderer_vulkan/vk_rasterizer.cpp @@ -100,6 +100,7 @@ RenderState Rasterizer::PrepareRenderState(u32 mrt_mask) { // an unnecessary transition and may result in state conflict if the resource is already // bound for reading. if ((mrt_mask & (1 << col_buf_id)) == 0) { + state.color_attachments[state.num_color_attachments++].imageView = VK_NULL_HANDLE; continue; }