From 2339fe199f3b08faa33b15048aa7158a2e0eff00 Mon Sep 17 00:00:00 2001 From: ReinUsesLisp Date: Wed, 1 Apr 2020 21:37:32 -0300 Subject: [PATCH] shader_decompiler: Remove FragCoord.w hack and change IPA implementation Credits go to gdkchan and Ryujinx. The pull request used for this can be found here: https://github.com/Ryujinx/Ryujinx/pull/1082 yuzu was already using the header for interpolation, but it was missing the FragCoord.w multiplication described in the linked pull request. This commit finally removes the FragCoord.w == 1.0f hack from the shader decompiler. While we are at it, this commit renames some enumerations to match Nvidia's documentation (linked below) and fixes component declaration order in the shader program header (z and w were swapped). https://github.com/NVIDIA/open-gpu-doc/blob/master/Shader-Program-Header/Shader-Program-Header.html --- src/video_core/engines/shader_header.h | 55 ++++++++++--------- .../renderer_opengl/gl_shader_decompiler.cpp | 34 ++++++------ .../renderer_vulkan/vk_shader_decompiler.cpp | 17 +++--- src/video_core/shader/decode/other.cpp | 36 +++++++----- 4 files changed, 74 insertions(+), 68 deletions(-) diff --git a/src/video_core/engines/shader_header.h b/src/video_core/engines/shader_header.h index bc80661d89..72e2a33d5c 100644 --- a/src/video_core/engines/shader_header.h +++ b/src/video_core/engines/shader_header.h @@ -4,6 +4,9 @@ #pragma once +#include +#include + #include "common/bit_field.h" #include "common/common_funcs.h" #include "common/common_types.h" @@ -16,7 +19,7 @@ enum class OutputTopology : u32 { TriangleStrip = 7, }; -enum class AttributeUse : u8 { +enum class PixelImap : u8 { Unused = 0, Constant = 1, Perspective = 2, @@ -24,7 +27,7 @@ enum class AttributeUse : u8 { }; // Documentation in: -// http://download.nvidia.com/open-gpu-doc/Shader-Program-Header/1/Shader-Program-Header.html#ImapTexture +// http://download.nvidia.com/open-gpu-doc/Shader-Program-Header/1/Shader-Program-Header.html struct Header { union { BitField<0, 5, u32> sph_type; @@ -59,8 +62,8 @@ struct Header { union { BitField<0, 12, u32> max_output_vertices; BitField<12, 8, u32> store_req_start; // NOTE: not used by geometry shaders. - BitField<24, 4, u32> reserved; - BitField<12, 8, u32> store_req_end; // NOTE: not used by geometry shaders. + BitField<20, 4, u32> reserved; + BitField<24, 8, u32> store_req_end; // NOTE: not used by geometry shaders. } common4{}; union { @@ -93,17 +96,20 @@ struct Header { struct { INSERT_UNION_PADDING_BYTES(3); // ImapSystemValuesA INSERT_UNION_PADDING_BYTES(1); // ImapSystemValuesB + union { - BitField<0, 2, AttributeUse> x; - BitField<2, 2, AttributeUse> y; - BitField<4, 2, AttributeUse> w; - BitField<6, 2, AttributeUse> z; + BitField<0, 2, PixelImap> x; + BitField<2, 2, PixelImap> y; + BitField<4, 2, PixelImap> z; + BitField<6, 2, PixelImap> w; u8 raw; } imap_generic_vector[32]; + INSERT_UNION_PADDING_BYTES(2); // ImapColor INSERT_UNION_PADDING_BYTES(2); // ImapSystemValuesC INSERT_UNION_PADDING_BYTES(10); // ImapFixedFncTexture[10] INSERT_UNION_PADDING_BYTES(2); // ImapReserved + struct { u32 target; union { @@ -112,31 +118,30 @@ struct Header { BitField<2, 30, u32> reserved; }; } omap; + bool IsColorComponentOutputEnabled(u32 render_target, u32 component) const { const u32 bit = render_target * 4 + component; return omap.target & (1 << bit); } - AttributeUse GetAttributeIndexUse(u32 attribute, u32 index) const { - return static_cast( - (imap_generic_vector[attribute].raw >> (index * 2)) & 0x03); - } - AttributeUse GetAttributeUse(u32 attribute) const { - AttributeUse result = AttributeUse::Unused; - for (u32 i = 0; i < 4; i++) { - const auto index = GetAttributeIndexUse(attribute, i); - if (index == AttributeUse::Unused) { + + PixelImap GetPixelImap(u32 attribute) const { + const auto get_index = [this, attribute](u32 index) { + return static_cast( + (imap_generic_vector[attribute].raw >> (index * 2)) & 3); + }; + + std::optional result; + for (u32 component = 0; component < 4; ++component) { + const PixelImap index = get_index(component); + if (index == PixelImap::Unused) { continue; } - if (result == AttributeUse::Unused || result == index) { - result = index; - continue; - } - LOG_CRITICAL(HW_GPU, "Generic Attribute Conflict in Interpolation Mode"); - if (index == AttributeUse::Perspective) { - result = index; + if (result && result != index) { + LOG_CRITICAL(HW_GPU, "Generic attribute conflict in interpolation mode"); } + result = index; } - return result; + return result.value_or(PixelImap::Unused); } } ps; diff --git a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp index c7d24cf141..160ae43405 100644 --- a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp +++ b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp @@ -31,11 +31,11 @@ namespace { using Tegra::Engines::ShaderType; using Tegra::Shader::Attribute; -using Tegra::Shader::AttributeUse; using Tegra::Shader::Header; using Tegra::Shader::IpaInterpMode; using Tegra::Shader::IpaMode; using Tegra::Shader::IpaSampleMode; +using Tegra::Shader::PixelImap; using Tegra::Shader::Register; using VideoCommon::Shader::BuildTransformFeedback; using VideoCommon::Shader::Registry; @@ -702,20 +702,19 @@ private: code.AddNewLine(); } - std::string GetInputFlags(AttributeUse attribute) { + const char* GetInputFlags(PixelImap attribute) { switch (attribute) { - case AttributeUse::Perspective: - // Default, Smooth - return {}; - case AttributeUse::Constant: - return "flat "; - case AttributeUse::ScreenLinear: - return "noperspective "; - default: - case AttributeUse::Unused: - UNIMPLEMENTED_MSG("Unknown attribute usage index={}", static_cast(attribute)); - return {}; + case PixelImap::Perspective: + return "smooth"; + case PixelImap::Constant: + return "flat"; + case PixelImap::ScreenLinear: + return "noperspective"; + case PixelImap::Unused: + break; } + UNIMPLEMENTED_MSG("Unknown attribute usage index={}", static_cast(attribute)); + return {}; } void DeclareInputAttributes() { @@ -749,8 +748,8 @@ private: std::string suffix; if (stage == ShaderType::Fragment) { - const auto input_mode{header.ps.GetAttributeUse(location)}; - if (skip_unused && input_mode == AttributeUse::Unused) { + const auto input_mode{header.ps.GetPixelImap(location)}; + if (input_mode == PixelImap::Unused) { return; } suffix = GetInputFlags(input_mode); @@ -927,7 +926,7 @@ private: const u32 address{generic_base + index * generic_stride + element * element_stride}; const bool declared = stage != ShaderType::Fragment || - header.ps.GetAttributeUse(index) != AttributeUse::Unused; + header.ps.GetPixelImap(index) != PixelImap::Unused; const std::string value = declared ? ReadAttribute(attribute, element).AsFloat() : "0.0f"; code.AddLine("case 0x{:X}U: return {};", address, value); @@ -1142,8 +1141,7 @@ private: GetSwizzle(element)), Type::Float}; case ShaderType::Fragment: - return {element == 3 ? "1.0f" : ("gl_FragCoord"s + GetSwizzle(element)), - Type::Float}; + return {"gl_FragCoord"s + GetSwizzle(element), Type::Float}; default: UNREACHABLE(); } diff --git a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp index d67f08cf9c..b9f9e2714e 100644 --- a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp +++ b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp @@ -35,7 +35,7 @@ namespace { using Sirit::Id; using Tegra::Engines::ShaderType; using Tegra::Shader::Attribute; -using Tegra::Shader::AttributeUse; +using Tegra::Shader::PixelImap; using Tegra::Shader::Register; using namespace VideoCommon::Shader; @@ -752,16 +752,16 @@ private: if (stage != ShaderType::Fragment) { continue; } - switch (header.ps.GetAttributeUse(location)) { - case AttributeUse::Constant: + switch (header.ps.GetPixelImap(location)) { + case PixelImap::Constant: Decorate(id, spv::Decoration::Flat); break; - case AttributeUse::ScreenLinear: - Decorate(id, spv::Decoration::NoPerspective); - break; - case AttributeUse::Perspective: + case PixelImap::Perspective: // Default break; + case PixelImap::ScreenLinear: + Decorate(id, spv::Decoration::NoPerspective); + break; default: UNREACHABLE_MSG("Unused attribute being fetched"); } @@ -1145,9 +1145,6 @@ private: switch (attribute) { case Attribute::Index::Position: { if (stage == ShaderType::Fragment) { - if (element == 3) { - return {Constant(t_float, 1.0f), Type::Float}; - } return {OpLoad(t_float, AccessElement(t_in_float, frag_coord, element)), Type::Float}; } diff --git a/src/video_core/shader/decode/other.cpp b/src/video_core/shader/decode/other.cpp index 4944e9d69f..e6edec459b 100644 --- a/src/video_core/shader/decode/other.cpp +++ b/src/video_core/shader/decode/other.cpp @@ -11,12 +11,17 @@ namespace VideoCommon::Shader { +using std::move; using Tegra::Shader::ConditionCode; using Tegra::Shader::Instruction; +using Tegra::Shader::IpaInterpMode; using Tegra::Shader::OpCode; +using Tegra::Shader::PixelImap; using Tegra::Shader::Register; using Tegra::Shader::SystemVariable; +using Index = Tegra::Shader::Attribute::Index; + u32 ShaderIR::DecodeOther(NodeBlock& bb, u32 pc) { const Instruction instr = {program_code[pc]}; const auto opcode = OpCode::Decode(instr); @@ -213,27 +218,28 @@ u32 ShaderIR::DecodeOther(NodeBlock& bb, u32 pc) { } case OpCode::Id::IPA: { const bool is_physical = instr.ipa.idx && instr.gpr8.Value() != 0xff; - const auto attribute = instr.attribute.fmt28; - const Tegra::Shader::IpaMode input_mode{instr.ipa.interp_mode.Value(), - instr.ipa.sample_mode.Value()}; + const Index index = attribute.index; Node value = is_physical ? GetPhysicalInputAttribute(instr.gpr8) - : GetInputAttribute(attribute.index, attribute.element); - const Tegra::Shader::Attribute::Index index = attribute.index.Value(); - const bool is_generic = index >= Tegra::Shader::Attribute::Index::Attribute_0 && - index <= Tegra::Shader::Attribute::Index::Attribute_31; - if (is_generic || is_physical) { - // TODO(Blinkhawk): There are cases where a perspective attribute use PASS. - // In theory by setting them as perspective, OpenGL does the perspective correction. - // A way must figured to reverse the last step of it. - if (input_mode.interpolation_mode == Tegra::Shader::IpaInterpMode::Multiply) { - value = Operation(OperationCode::FMul, PRECISE, value, GetRegister(instr.gpr20)); + : GetInputAttribute(index, attribute.element); + + // Code taken from Ryujinx. + if (index >= Index::Attribute_0 && index <= Index::Attribute_31) { + const u32 location = static_cast(index) - static_cast(Index::Attribute_0); + if (header.ps.GetPixelImap(location) == PixelImap::Perspective) { + Node position_w = GetInputAttribute(Index::Position, 3); + value = Operation(OperationCode::FMul, move(value), move(position_w)); } } - value = GetSaturatedFloat(value, instr.ipa.saturate); - SetRegister(bb, instr.gpr0, value); + if (instr.ipa.interp_mode == IpaInterpMode::Multiply) { + value = Operation(OperationCode::FMul, move(value), GetRegister(instr.gpr20)); + } + + value = GetSaturatedFloat(move(value), instr.ipa.saturate); + + SetRegister(bb, instr.gpr0, move(value)); break; } case OpCode::Id::OUT_R: {