renderer_vulkan: Fix some color attachment indexing issues. (#1755)

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<vk::PipelineColorBlendAttachmentState, Liverpool::NumColorBuffers> 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},
     };

src/video_core/renderer_vulkan/vk_graphics_pipeline.h

@@ -29,6 +29,7 @@ using Liverpool = AmdGpu::Liverpool;
 
 struct GraphicsPipelineKey {
     std::array<size_t, MaxShaderStages> stage_hashes;
+    u32 num_color_attachments;
     std::array<vk::Format, Liverpool::NumColorBuffers> color_formats;
     std::array<AmdGpu::NumberFormat, Liverpool::NumColorBuffers> color_num_formats;
     std::array<Liverpool::ColorBuffer::SwapMode, Liverpool::NumColorBuffers> mrt_swizzles;

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

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;

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;
         }