From 1690f1adba2c6971256d939f260d865689f517ce Mon Sep 17 00:00:00 2001 From: ReinUsesLisp Date: Fri, 24 Jan 2020 01:15:50 -0300 Subject: [PATCH] vk_shader_decompiler: Disable default values on unwritten render targets Some games like The Legend of Zelda: Breath of the Wild assign render targets without writing them from the fragment shader. This generates Vulkan validation errors, so silence these I previously introduced a commit to set "vec4(0, 0, 0, 1)" for these attachments. The problem is that this is not what games expect. This commit reverts that change. --- .../renderer_vulkan/vk_pipeline_cache.cpp | 3 -- .../renderer_vulkan/vk_shader_decompiler.cpp | 29 ++++++++++--------- .../renderer_vulkan/vk_shader_decompiler.h | 3 -- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp index 48e23d4cd..7ddf7d3ee 100644 --- a/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_pipeline_cache.cpp @@ -325,9 +325,6 @@ VKPipelineCache::DecompileShaders(const GraphicsPipelineCacheKey& key) { specialization.tessellation.primitive = fixed_state.tessellation.primitive; specialization.tessellation.spacing = fixed_state.tessellation.spacing; specialization.tessellation.clockwise = fixed_state.tessellation.clockwise; - for (const auto& rt : key.renderpass_params.color_attachments) { - specialization.enabled_rendertargets.set(rt.index); - } SPIRVProgram program; std::vector bindings; diff --git a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp index dd6d2ef03..b53078721 100644 --- a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp +++ b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp @@ -542,11 +542,10 @@ private: return; } - for (u32 rt = 0; rt < static_cast(frag_colors.size()); ++rt) { - if (!specialization.enabled_rendertargets[rt]) { + for (u32 rt = 0; rt < static_cast(std::size(frag_colors)); ++rt) { + if (!IsRenderTargetEnabled(rt)) { continue; } - const Id id = AddGlobalVariable(OpVariable(t_out_float4, spv::StorageClass::Output)); Name(id, fmt::format("frag_color{}", rt)); Decorate(id, spv::Decoration::Location, rt); @@ -852,6 +851,15 @@ private: return binding; } + bool IsRenderTargetEnabled(u32 rt) const { + for (u32 component = 0; component < 4; ++component) { + if (header.ps.IsColorComponentOutputEnabled(rt, component)) { + return true; + } + } + return false; + } + bool IsInputAttributeArray() const { return stage == ShaderType::TesselationControl || stage == ShaderType::TesselationEval || stage == ShaderType::Geometry; @@ -1889,19 +1897,14 @@ private: // rendertargets/components are skipped in the register assignment. u32 current_reg = 0; for (u32 rt = 0; rt < Maxwell::NumRenderTargets; ++rt) { - if (!specialization.enabled_rendertargets[rt]) { - // Skip rendertargets that are not enabled - continue; - } // TODO(Subv): Figure out how dual-source blending is configured in the Switch. for (u32 component = 0; component < 4; ++component) { - const Id pointer = AccessElement(t_out_float, frag_colors.at(rt), component); - if (header.ps.IsColorComponentOutputEnabled(rt, component)) { - OpStore(pointer, SafeGetRegister(current_reg)); - ++current_reg; - } else { - OpStore(pointer, component == 3 ? v_float_one : v_float_zero); + if (!header.ps.IsColorComponentOutputEnabled(rt, component)) { + continue; } + const Id pointer = AccessElement(t_out_float, frag_colors[rt], component); + OpStore(pointer, SafeGetRegister(current_reg)); + ++current_reg; } } if (header.ps.omap.depth) { diff --git a/src/video_core/renderer_vulkan/vk_shader_decompiler.h b/src/video_core/renderer_vulkan/vk_shader_decompiler.h index 10794be1c..f5dc14d9e 100644 --- a/src/video_core/renderer_vulkan/vk_shader_decompiler.h +++ b/src/video_core/renderer_vulkan/vk_shader_decompiler.h @@ -102,9 +102,6 @@ struct Specialization final { Maxwell::TessellationSpacing spacing{}; bool clockwise{}; } tessellation; - - // Fragment specific - std::bitset<8> enabled_rendertargets; }; // Old gcc versions don't consider this trivially copyable. // static_assert(std::is_trivially_copyable_v);