From 2519d14e36bf47f4ef6172928db13c5f7fae14e3 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 9 Aug 2021 13:50:07 -0700 Subject: [PATCH 1/3] UberShaderVertex: Fix Tony Hawk Pro Skater 4 Fixes https://bugs.dolphin-emu.org/issues/12620 The changed code did not match the corresponding code in VertexShaderGen. Some parts of the sky have 2 color channels in each vertex, while others only have 1, despite only color channel 0 being used and XFMEM_SETNUMCHAN being set to 1 for both of them. The old code (from #4601) caused channel 0 to be set to channel 1 if the vertex contained both color channels but the number of channels was set to 1, which is wrong. --- Source/Core/VideoCommon/UberShaderVertex.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/VideoCommon/UberShaderVertex.cpp b/Source/Core/VideoCommon/UberShaderVertex.cpp index 5b02b1f989..ceafb09219 100644 --- a/Source/Core/VideoCommon/UberShaderVertex.cpp +++ b/Source/Core/VideoCommon/UberShaderVertex.cpp @@ -218,14 +218,14 @@ ShaderCode GenVertexShader(APIType api_type, const ShaderHostConfig& host_config " if ((components & {}u) != 0u)\n" " o.colors_0 = rawcolor0;\n" " else\n" - " o.colors_1 = float4(1.0, 1.0, 1.0, 1.0);\n" + " o.colors_0 = float4(1.0, 1.0, 1.0, 1.0);\n" "}}\n", VB_HAS_COL0); out.Write("if (xfmem_numColorChans < 2u) {{\n" " if ((components & {}u) != 0u)\n" - " o.colors_0 = rawcolor1;\n" + " o.colors_1 = rawcolor1;\n" " else\n" - " o.colors_1 = float4(1.0, 1.0, 1.0, 1.0);\n" + " o.colors_1 = o.colors_0;\n" "}}\n", VB_HAS_COL1); From 06579e4d53844e5a45ae18f8eda6c4ee69ebad2c Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 9 Aug 2021 14:17:11 -0700 Subject: [PATCH 2/3] VertexShaderGen: Simplify color channel logic --- Source/Core/VideoCommon/VertexShaderGen.cpp | 45 +++++++++++---------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/Source/Core/VideoCommon/VertexShaderGen.cpp b/Source/Core/VideoCommon/VertexShaderGen.cpp index 4e4121b564..8f8133de63 100644 --- a/Source/Core/VideoCommon/VertexShaderGen.cpp +++ b/Source/Core/VideoCommon/VertexShaderGen.cpp @@ -441,19 +441,38 @@ ShaderCode GenerateVertexShaderCode(APIType api_type, const ShaderHostConfig& ho out.Write("}}\n"); } + // The number of colors available to TEV is determined by numColorChans. + // We have to provide the fields to match the interface, so set to zero if it's not enabled. + + // When per-pixel lighting is enabled, the vertex colors are passed through unmodified so we can + // evaluate the lighting in the same manner in the pixel shader. if (uid_data->numColorChans == 0) { if ((uid_data->components & VB_HAS_COL0) != 0) - out.Write("o.colors_0 = rawcolor0;\n"); + { + if (per_pixel_lighting) + out.Write("o.colors_0 = vertex_color_0;\n"); + else + out.Write("o.colors_0 = rawcolor0;\n"); + } else - out.Write("o.colors_0 = float4(1.0, 1.0, 1.0, 1.0);\n"); + { + out.Write("o.colors_0 = float4(0.0, 0.0, 0.0, 0.0);\n"); + } } - if (uid_data->numColorChans < 2) + if (uid_data->numColorChans <= 1) { if ((uid_data->components & VB_HAS_COL1) != 0) - out.Write("o.colors_1 = rawcolor1;\n"); + { + if (per_pixel_lighting) + out.Write("o.colors_1 = vertex_color_1;\n"); + else + out.Write("o.colors_1 = rawcolor1;\n"); + } else - out.Write("o.colors_1 = o.colors_0;\n"); + { + out.Write("o.colors_1 = float4(0.0, 0.0, 0.0, 0.0);\n"); + } } // clipPos/w needs to be done in pixel shader, not here @@ -464,22 +483,6 @@ ShaderCode GenerateVertexShaderCode(APIType api_type, const ShaderHostConfig& ho { out.Write("o.Normal = _norm0;\n" "o.WorldPos = pos.xyz;\n"); - - // Pass through the vertex colors unmodified so we can evaluate the lighting in the same manner. - if ((uid_data->components & VB_HAS_COL0) != 0) - out.Write("o.colors_0 = vertex_color_0;\n"); - - if ((uid_data->components & VB_HAS_COL1) != 0) - out.Write("o.colors_1 = vertex_color_1;\n"); - } - else - { - // The number of colors available to TEV is determined by numColorChans. - // We have to provide the fields to match the interface, so set to zero if it's not enabled. - if (uid_data->numColorChans == 0) - out.Write("o.colors_0 = float4(0.0, 0.0, 0.0, 0.0);\n"); - if (uid_data->numColorChans <= 1) - out.Write("o.colors_1 = float4(0.0, 0.0, 0.0, 0.0);\n"); } // If we can disable the incorrect depth clipping planes using depth clamping, then we can do From c3dec343918ff44e90886daf71a6bf0c49033de1 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 9 Aug 2021 14:17:26 -0700 Subject: [PATCH 3/3] UberShaderVertex: Simplify color channel logic --- Source/Core/VideoCommon/UberShaderVertex.cpp | 51 +++++++++----------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/Source/Core/VideoCommon/UberShaderVertex.cpp b/Source/Core/VideoCommon/UberShaderVertex.cpp index ceafb09219..bb8732546a 100644 --- a/Source/Core/VideoCommon/UberShaderVertex.cpp +++ b/Source/Core/VideoCommon/UberShaderVertex.cpp @@ -214,20 +214,36 @@ ShaderCode GenVertexShader(APIType api_type, const ShaderHostConfig& host_config if (num_texgen > 0) GenVertexShaderTexGens(api_type, num_texgen, out); + out.Write("// The number of colors available to TEV is determined by numColorChans.\n" + "// We have to provide the fields to match the interface, so set to zero\n" + "// if it's not enabled.\n"); + + const char* color_prefix; + if (per_pixel_lighting) + { + out.Write("\n// Since per-pixel lighting is enabled, the vertex colors are passed through\n" + "// unmodified so we can evaluate the lighting in the pixel shader.\n"); + color_prefix = "vertex_color_"; + } + else + { + color_prefix = "rawcolor"; + } + out.Write("if (xfmem_numColorChans == 0u) {{\n" - " if ((components & {}u) != 0u)\n" - " o.colors_0 = rawcolor0;\n" + " if ((components & {}u) != 0u) // VB_HAS_COL0\n" + " o.colors_0 = {}0;\n" " else\n" " o.colors_0 = float4(1.0, 1.0, 1.0, 1.0);\n" "}}\n", - VB_HAS_COL0); - out.Write("if (xfmem_numColorChans < 2u) {{\n" - " if ((components & {}u) != 0u)\n" - " o.colors_1 = rawcolor1;\n" + VB_HAS_COL0, color_prefix); + out.Write("if (xfmem_numColorChans <= 1u) {{\n" + " if ((components & {}u) != 0u) // VB_HAS_COL1\n" + " o.colors_1 = {}1;\n" " else\n" " o.colors_1 = o.colors_0;\n" "}}\n", - VB_HAS_COL1); + VB_HAS_COL1, color_prefix); if (!host_config.fast_depth_calc) { @@ -238,26 +254,7 @@ ShaderCode GenVertexShader(APIType api_type, const ShaderHostConfig& host_config if (per_pixel_lighting) { out.Write("o.Normal = _norm0;\n" - "o.WorldPos = pos.xyz;\n" - "// Pass through the vertex colors unmodified so we can evaluate the lighting\n" - "// in the same manner.\n"); - out.Write("if ((components & {}u) != 0u) // VB_HAS_COL0\n" - " o.colors_0 = vertex_color_0;\n", - VB_HAS_COL0); - out.Write("if ((components & {}u) != 0u) // VB_HAS_COL1\n" - " o.colors_1 = vertex_color_1;\n", - VB_HAS_COL1); - } - else - { - out.Write("// The number of colors available to TEV is determined by numColorChans.\n" - "// We have to provide the fields to match the interface, so set to zero\n" - "// if it's not enabled.\n" - "if (xfmem_numColorChans == 0u)\n" - " o.colors_0 = float4(0.0, 0.0, 0.0, 0.0);\n" - "if (xfmem_numColorChans <= 1u)\n" - " o.colors_1 = float4(0.0, 0.0, 0.0, 0.0);\n" - "\n"); + "o.WorldPos = pos.xyz;\n"); } // If we can disable the incorrect depth clipping planes using depth clamping, then we can do