From a5c8df7a37cfe49928c071c49c262fcd43edd98f Mon Sep 17 00:00:00 2001 From: Triang3l Date: Sat, 25 Jun 2022 20:57:44 +0300 Subject: [PATCH] [Vulkan] Remove UB-based independent blend logic On Vulkan, unlike Direct3D, not writing to a color target in the fragment shader produces an undefined result. --- src/xenia/gpu/spirv_shader_translator.cc | 21 +++++++-------- src/xenia/gpu/spirv_shader_translator.h | 5 ---- src/xenia/gpu/vulkan/vulkan_pipeline_cache.cc | 27 ------------------- 3 files changed, 9 insertions(+), 44 deletions(-) diff --git a/src/xenia/gpu/spirv_shader_translator.cc b/src/xenia/gpu/spirv_shader_translator.cc index b260f78fa..146af6823 100644 --- a/src/xenia/gpu/spirv_shader_translator.cc +++ b/src/xenia/gpu/spirv_shader_translator.cc @@ -1270,19 +1270,16 @@ void SpirvShaderTranslator::StartFragmentShaderBeforeMain() { "xe_out_fragment_data_2", "xe_out_fragment_data_3", }; - uint32_t fragment_data_outputs_written = - current_shader().writes_color_targets() & - ~GetSpirvShaderModification().pixel.color_outputs_disabled; - for (uint32_t i = 0; i < xenos::kMaxColorRenderTargets; ++i) { - if (!(fragment_data_outputs_written & (uint32_t(1) << i))) { - continue; - } - spv::Id output_fragment_data_rt = - builder_->createVariable(spv::NoPrecision, spv::StorageClassOutput, - type_float4_, kFragmentDataNames[i]); - output_fragment_data_[i] = output_fragment_data_rt; + uint32_t color_targets_remaining = current_shader().writes_color_targets(); + uint32_t color_target_index; + while (xe::bit_scan_forward(color_targets_remaining, &color_target_index)) { + color_targets_remaining &= ~(UINT32_C(1) << color_target_index); + spv::Id output_fragment_data_rt = builder_->createVariable( + spv::NoPrecision, spv::StorageClassOutput, type_float4_, + kFragmentDataNames[color_target_index]); + output_fragment_data_[color_target_index] = output_fragment_data_rt; builder_->addDecoration(output_fragment_data_rt, spv::DecorationLocation, - int(i)); + int(color_target_index)); // Make invariant as pixel shaders may be used for various precise // computations. builder_->addDecoration(output_fragment_data_rt, spv::DecorationInvariant); diff --git a/src/xenia/gpu/spirv_shader_translator.h b/src/xenia/gpu/spirv_shader_translator.h index aa0265afb..76caab044 100644 --- a/src/xenia/gpu/spirv_shader_translator.h +++ b/src/xenia/gpu/spirv_shader_translator.h @@ -46,11 +46,6 @@ class SpirvShaderTranslator : public ShaderTranslator { struct PixelShaderModification { // Dynamically indexable register count from SQ_PROGRAM_CNTL. uint32_t dynamic_addressable_register_count : 8; - // Color outputs removed from the shader to implement a zero color write - // mask when independent blending (and thus independent write masks) is - // not supported without switching to a render pass with some attachments - // actually excluded. - uint32_t color_outputs_disabled : 4; } pixel; uint64_t value = 0; diff --git a/src/xenia/gpu/vulkan/vulkan_pipeline_cache.cc b/src/xenia/gpu/vulkan/vulkan_pipeline_cache.cc index 574e1ba36..5e9fec78d 100644 --- a/src/xenia/gpu/vulkan/vulkan_pipeline_cache.cc +++ b/src/xenia/gpu/vulkan/vulkan_pipeline_cache.cc @@ -141,33 +141,6 @@ VulkanPipelineCache::GetCurrentPixelShaderModification( shader.GetDynamicAddressableRegisterCount( sq_program_cntl.ps_num_reg))); - const ui::vulkan::VulkanProvider& provider = - command_processor_.GetVulkanProvider(); - const VkPhysicalDeviceFeatures& device_features = provider.device_features(); - if (!device_features.independentBlend) { - // Since without independent blending, the write mask is common for all - // attachments, but the render pass may still include the attachments from - // previous draws (to prevent excessive render pass changes potentially - // doing stores and loads), disable writing to render targets with a - // completely empty write mask by removing the output from the shader. - // Only explicitly excluding render targets that the shader actually writes - // to, for better pipeline storage compatibility between devices with and - // without independent blending (so in the usual situation - the shader - // doesn't write to any render targets disabled via the color mask - no - // explicit disabling of shader outputs will be needed, and the disabled - // output mask will be 0). - uint32_t color_targets_remaining = shader.writes_color_targets(); - uint32_t color_target_index; - while (xe::bit_scan_forward(color_targets_remaining, &color_target_index)) { - color_targets_remaining &= ~(uint32_t(1) << color_target_index); - if (!(normalized_color_mask & - (uint32_t(0b1111) << (4 * color_target_index)))) { - modification.pixel.color_outputs_disabled |= uint32_t(1) - << color_target_index; - } - } - } - return modification; }