From 0d0fd310754b849959d746e93c85e5777edfd57d Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Thu, 24 Feb 2022 17:10:17 -0800
Subject: [PATCH 01/15] Stop normalizing light directions

This normalization was added in 02ac5e95c84a1d9a46df1dc4102342fb653e36ee (and changed to use floats in 4bf031c0646e91b35777f1ba4e2b0328063bb666).  However, this normalization introduces NaN values in some cases, which is causing problems for the version of Mesa in use on FifoCI (currently 20.3.5).  Although Mesa's NaN behavior is corrected by https://gitlab.freedesktop.org/mesa/mesa/-/commit/b3f3287eac066eae16dce0e47aad3229dcff8257 (21.2.0), FifoCI is currently stuck with the older version.
---
 .../Core/VideoCommon/VertexShaderManager.cpp  | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/Source/Core/VideoCommon/VertexShaderManager.cpp b/Source/Core/VideoCommon/VertexShaderManager.cpp
index 2408281da6..101bcbc4aa 100644
--- a/Source/Core/VideoCommon/VertexShaderManager.cpp
+++ b/Source/Core/VideoCommon/VertexShaderManager.cpp
@@ -263,22 +263,9 @@ void VertexShaderManager::SetConstants(const std::vector<std::string>& textures)
       dstlight.pos[1] = light.dpos[1];
       dstlight.pos[2] = light.dpos[2];
 
-      // TODO: Hardware testing is needed to confirm that this normalization is correct
-      auto sanitize = [](float f) {
-        if (std::isnan(f))
-          return 0.0f;
-        else if (std::isinf(f))
-          return f > 0.0f ? 1.0f : -1.0f;
-        else
-          return f;
-      };
-      double norm = double(light.ddir[0]) * double(light.ddir[0]) +
-                    double(light.ddir[1]) * double(light.ddir[1]) +
-                    double(light.ddir[2]) * double(light.ddir[2]);
-      norm = 1.0 / sqrt(norm);
-      dstlight.dir[0] = sanitize(static_cast<float>(light.ddir[0] * norm));
-      dstlight.dir[1] = sanitize(static_cast<float>(light.ddir[1] * norm));
-      dstlight.dir[2] = sanitize(static_cast<float>(light.ddir[2] * norm));
+      dstlight.dir[0] = light.ddir[0];
+      dstlight.dir[1] = light.ddir[1];
+      dstlight.dir[2] = light.ddir[2];
     }
     dirty = true;
 

From 40f548d7e0376cc17bc934e0ceb724b9963b4ba1 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Thu, 24 Feb 2022 15:36:54 -0800
Subject: [PATCH 02/15] Refactor LightingShaderGen for readability

---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 150 +++++++++++-------
 Source/Core/VideoCommon/XFMemory.h            |   3 +-
 2 files changed, 95 insertions(+), 58 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index b92e63fe9e..5af01db616 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -11,17 +11,12 @@
 #include "VideoCommon/XFMemory.h"
 
 static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_data, int index,
-                                int litchan_index, bool alpha)
+                                AttenuationFunc attn_func, DiffuseFunc diffuse_func, bool alpha)
 {
   const char* swizzle = alpha ? "a" : "rgb";
   const char* swizzle_components = (alpha) ? "" : "3";
 
-  const auto attnfunc =
-      static_cast<AttenuationFunc>((uid_data.attnfunc >> (2 * litchan_index)) & 0x3);
-  const auto diffusefunc =
-      static_cast<DiffuseFunc>((uid_data.diffusefunc >> (2 * litchan_index)) & 0x3);
-
-  switch (attnfunc)
+  switch (attn_func)
   {
   case AttenuationFunc::None:
   case AttenuationFunc::Dir:
@@ -36,7 +31,7 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
                  LIGHT_DIR_PARAMS(index));
     object.Write("cosAttn = " LIGHT_COSATT ".xyz;\n", LIGHT_COSATT_PARAMS(index));
     object.Write("distAttn = {}(" LIGHT_DISTATT ".xyz);\n",
-                 (diffusefunc == DiffuseFunc::None) ? "" : "normalize",
+                 (diffuse_func == DiffuseFunc::None) ? "" : "normalize",
                  LIGHT_DISTATT_PARAMS(index));
     object.Write("attn = max(0.0f, dot(cosAttn, float3(1.0, attn, attn*attn))) / dot(distAttn, "
                  "float3(1.0, attn, attn*attn));\n");
@@ -56,7 +51,7 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
     break;
   }
 
-  switch (diffusefunc)
+  switch (diffuse_func)
   {
   case DiffuseFunc::None:
     object.Write("lacc.{} += int{}(round(attn * float{}(" LIGHT_COL ")));\n", swizzle,
@@ -66,7 +61,7 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   case DiffuseFunc::Clamp:
     object.Write("lacc.{} += int{}(round(attn * {}dot(ldir, _normal)) * float{}(" LIGHT_COL
                  ")));\n",
-                 swizzle, swizzle_components, diffusefunc != DiffuseFunc::Sign ? "max(0.0," : "(",
+                 swizzle, swizzle_components, diffuse_func != DiffuseFunc::Sign ? "max(0.0," : "(",
                  swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
     break;
   default:
@@ -84,22 +79,40 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
 void GenerateLightingShaderCode(ShaderCode& object, const LightingUidData& uid_data,
                                 std::string_view in_color_name, std::string_view dest)
 {
-  for (u32 j = 0; j < NUM_XF_COLOR_CHANNELS; j++)
+  for (u32 chan = 0; chan < NUM_XF_COLOR_CHANNELS; chan++)
   {
-    object.Write("{{\n");
+    // Data for alpha is stored after all colors
+    const u32 chan_a = chan + NUM_XF_COLOR_CHANNELS;
 
-    const bool colormatsource = !!(uid_data.matsource & (1 << j));
-    if (colormatsource)  // from vertex
-      object.Write("int4 mat = int4(round({}{} * 255.0));\n", in_color_name, j);
-    else  // from color
-      object.Write("int4 mat = {}[{}];\n", I_MATERIALS, j + 2);
+    const auto color_matsource = static_cast<MatSource>((uid_data.matsource >> chan) & 1);
+    const auto color_ambsource = static_cast<AmbSource>((uid_data.ambsource >> chan) & 1);
+    const bool color_enable = ((uid_data.enablelighting >> chan) & 1) != 0;
+    const auto alpha_matsource = static_cast<MatSource>((uid_data.matsource >> chan_a) & 1);
+    const auto alpha_ambsource = static_cast<AmbSource>((uid_data.ambsource >> chan_a) & 1);
+    const bool alpha_enable = ((uid_data.enablelighting >> chan_a) & 1) != 0;
 
-    if ((uid_data.enablelighting & (1 << j)) != 0)
+    object.Write("{{\n"
+                 "// Lighting for channel {}:\n"
+                 "// Color material source: {}\n"
+                 "// Color ambient source: {}\n"
+                 "// Color lighting enabled: {}\n"
+                 "// Alpha material source: {}\n"
+                 "// Alpha ambient source: {}\n"
+                 "// Alpha lighting enabled: {}\n",
+                 chan, color_matsource, color_ambsource, color_enable, alpha_matsource,
+                 alpha_ambsource, alpha_enable);
+
+    if (color_matsource == MatSource::Vertex)
+      object.Write("int4 mat = int4(round({}{} * 255.0));\n", in_color_name, chan);
+    else  // from material color register
+      object.Write("int4 mat = {}[{}];\n", I_MATERIALS, chan + 2);
+
+    if (color_enable)
     {
-      if ((uid_data.ambsource & (1 << j)) != 0)  // from vertex
-        object.Write("lacc = int4(round({}{} * 255.0));\n", in_color_name, j);
-      else  // from color
-        object.Write("lacc = {}[{}];\n", I_MATERIALS, j);
+      if (color_ambsource == AmbSource::Vertex)
+        object.Write("lacc = int4(round({}{} * 255.0));\n", in_color_name, chan);
+      else  // from ambient color register
+        object.Write("lacc = {}[{}];\n", I_MATERIALS, chan);
     }
     else
     {
@@ -107,71 +120,94 @@ void GenerateLightingShaderCode(ShaderCode& object, const LightingUidData& uid_d
     }
 
     // check if alpha is different
-    const bool alphamatsource = !!(uid_data.matsource & (1 << (j + 2)));
-    if (alphamatsource != colormatsource)
+    if (color_matsource != alpha_matsource)
     {
-      if (alphamatsource)  // from vertex
-        object.Write("mat.w = int(round({}{}.w * 255.0));\n", in_color_name, j);
-      else  // from color
-        object.Write("mat.w = {}[{}].w;\n", I_MATERIALS, j + 2);
+      if (alpha_matsource == MatSource::Vertex)
+        object.Write("mat.w = int(round({}{}.w * 255.0));\n", in_color_name, chan);
+      else  // from material color register
+        object.Write("mat.w = {}[{}].w;\n", I_MATERIALS, chan + 2);
     }
 
-    if ((uid_data.enablelighting & (1 << (j + 2))) != 0)
+    if (alpha_enable)
     {
-      if ((uid_data.ambsource & (1 << (j + 2))) != 0)  // from vertex
-        object.Write("lacc.w = int(round({}{}.w * 255.0));\n", in_color_name, j);
-      else  // from color
-        object.Write("lacc.w = {}[{}].w;\n", I_MATERIALS, j);
+      if (alpha_ambsource == AmbSource::Vertex)  // from vertex
+        object.Write("lacc.w = int(round({}{}.w * 255.0));\n", in_color_name, chan);
+      else  // from ambient color register
+        object.Write("lacc.w = {}[{}].w;\n", I_MATERIALS, chan);
     }
     else
     {
       object.Write("lacc.w = 255;\n");
     }
 
-    if ((uid_data.enablelighting & (1 << j)) != 0)  // Color lights
+    if (color_enable)
     {
-      for (int i = 0; i < 8; ++i)
+      const auto attnfunc = static_cast<AttenuationFunc>((uid_data.attnfunc >> (2 * chan)) & 3);
+      const auto diffusefunc = static_cast<DiffuseFunc>((uid_data.diffusefunc >> (2 * chan)) & 3);
+      const u32 light_mask =
+          (uid_data.light_mask >> (NUM_XF_LIGHTS * chan)) & ((1 << NUM_XF_LIGHTS) - 1);
+      object.Write("// Color attenuation function: {}\n", attnfunc);
+      object.Write("// Color diffuse function: {}\n", diffusefunc);
+      object.Write("// Color light mask: {:08b}\n", light_mask);
+      for (u32 light = 0; light < NUM_XF_LIGHTS; light++)
       {
-        if ((uid_data.light_mask & (1 << (i + 8 * j))) != 0)
-          GenerateLightShader(object, uid_data, i, j, false);
+        if ((light_mask & (1 << light)) != 0)
+        {
+          object.Write("// Color light {}\n", light);
+          GenerateLightShader(object, uid_data, light, attnfunc, diffusefunc, false);
+        }
       }
     }
-    if ((uid_data.enablelighting & (1 << (j + 2))) != 0)  // Alpha lights
+    if (alpha_enable)  // Alpha lights
     {
-      for (int i = 0; i < 8; ++i)
+      const auto attnfunc = static_cast<AttenuationFunc>((uid_data.attnfunc >> (2 * chan_a)) & 3);
+      const auto diffusefunc = static_cast<DiffuseFunc>((uid_data.diffusefunc >> (2 * chan_a)) & 3);
+      const u32 light_mask =
+          (uid_data.light_mask >> (NUM_XF_LIGHTS * chan_a)) & ((1 << NUM_XF_LIGHTS) - 1);
+      object.Write("// Alpha attenuation function: {}\n", attnfunc);
+      object.Write("// Alpha diffuse function: {}\n", diffusefunc);
+      object.Write("// Alpha light mask function: {:08b}\n", light_mask);
+      for (u32 light = 0; light < NUM_XF_LIGHTS; light++)
       {
-        if ((uid_data.light_mask & (1 << (i + 8 * (j + 2)))) != 0)
-          GenerateLightShader(object, uid_data, i, j + 2, true);
+        if ((light_mask & (1 << light)) != 0)
+        {
+          object.Write("// Alpha light {}\n", light);
+          GenerateLightShader(object, uid_data, light, attnfunc, diffusefunc, true);
+        }
       }
     }
     object.Write("lacc = clamp(lacc, 0, 255);\n");
-    object.Write("{}{} = float4((mat * (lacc + (lacc >> 7))) >> 8) / 255.0;\n", dest, j);
+    object.Write("{}{} = float4((mat * (lacc + (lacc >> 7))) >> 8) / 255.0;\n", dest, chan);
     object.Write("}}\n");
   }
 }
 
 void GetLightingShaderUid(LightingUidData& uid_data)
 {
-  for (u32 j = 0; j < NUM_XF_COLOR_CHANNELS; j++)
+  for (u32 chan = 0; chan < NUM_XF_COLOR_CHANNELS; chan++)
   {
-    uid_data.matsource |= static_cast<u32>(xfmem.color[j].matsource.Value()) << j;
-    uid_data.matsource |= static_cast<u32>(xfmem.alpha[j].matsource.Value()) << (j + 2);
-    uid_data.enablelighting |= xfmem.color[j].enablelighting << j;
-    uid_data.enablelighting |= xfmem.alpha[j].enablelighting << (j + 2);
+    // Data for alpha is stored after all colors
+    const u32 chan_a = chan + NUM_XF_COLOR_CHANNELS;
 
-    if ((uid_data.enablelighting & (1 << j)) != 0)  // Color lights
+    uid_data.matsource |= static_cast<u32>(xfmem.color[chan].matsource.Value()) << chan;
+    uid_data.matsource |= static_cast<u32>(xfmem.alpha[chan].matsource.Value()) << chan_a;
+    uid_data.enablelighting |= xfmem.color[chan].enablelighting << chan;
+    uid_data.enablelighting |= xfmem.alpha[chan].enablelighting << chan_a;
+
+    if ((uid_data.enablelighting & (1 << chan)) != 0)  // Color lights
     {
-      uid_data.ambsource |= static_cast<u32>(xfmem.color[j].ambsource.Value()) << j;
-      uid_data.attnfunc |= static_cast<u32>(xfmem.color[j].attnfunc.Value()) << (2 * j);
-      uid_data.diffusefunc |= static_cast<u32>(xfmem.color[j].diffusefunc.Value()) << (2 * j);
-      uid_data.light_mask |= xfmem.color[j].GetFullLightMask() << (8 * j);
+      uid_data.ambsource |= static_cast<u32>(xfmem.color[chan].ambsource.Value()) << chan;
+      uid_data.attnfunc |= static_cast<u32>(xfmem.color[chan].attnfunc.Value()) << (2 * chan);
+      uid_data.diffusefunc |= static_cast<u32>(xfmem.color[chan].diffusefunc.Value()) << (2 * chan);
+      uid_data.light_mask |= xfmem.color[chan].GetFullLightMask() << (NUM_XF_LIGHTS * chan);
     }
-    if ((uid_data.enablelighting & (1 << (j + 2))) != 0)  // Alpha lights
+    if ((uid_data.enablelighting & (1 << chan_a)) != 0)  // Alpha lights
     {
-      uid_data.ambsource |= static_cast<u32>(xfmem.alpha[j].ambsource.Value()) << (j + 2);
-      uid_data.attnfunc |= static_cast<u32>(xfmem.alpha[j].attnfunc.Value()) << (2 * (j + 2));
-      uid_data.diffusefunc |= static_cast<u32>(xfmem.alpha[j].diffusefunc.Value()) << (2 * (j + 2));
-      uid_data.light_mask |= xfmem.alpha[j].GetFullLightMask() << (8 * (j + 2));
+      uid_data.ambsource |= static_cast<u32>(xfmem.alpha[chan].ambsource.Value()) << chan_a;
+      uid_data.attnfunc |= static_cast<u32>(xfmem.alpha[chan].attnfunc.Value()) << (2 * chan_a);
+      uid_data.diffusefunc |= static_cast<u32>(xfmem.alpha[chan].diffusefunc.Value())
+                              << (2 * chan_a);
+      uid_data.light_mask |= xfmem.alpha[chan].GetFullLightMask() << (NUM_XF_LIGHTS * chan_a);
     }
   }
 }
diff --git a/Source/Core/VideoCommon/XFMemory.h b/Source/Core/VideoCommon/XFMemory.h
index e0a2696317..5dce25951c 100644
--- a/Source/Core/VideoCommon/XFMemory.h
+++ b/Source/Core/VideoCommon/XFMemory.h
@@ -14,6 +14,7 @@
 #include "VideoCommon/CPMemory.h"
 
 constexpr size_t NUM_XF_COLOR_CHANNELS = 2;
+constexpr size_t NUM_XF_LIGHTS = 8;
 
 // Lighting
 
@@ -430,7 +431,7 @@ struct alignas(16) XFMemory
   float normalMatrices[96];                 // 0x0400 - 0x045f
   u32 unk1[160];                            // 0x0460 - 0x04ff
   float postMatrices[256];                  // 0x0500 - 0x05ff
-  Light lights[8];                          // 0x0600 - 0x067f
+  Light lights[NUM_XF_LIGHTS];              // 0x0600 - 0x067f
   u32 unk2[2432];                           // 0x0680 - 0x0fff
   u32 error;                                // 0x1000
   u32 diag;                                 // 0x1001

From bbe682013d9518f877bf46520f886465c5c0edf5 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Fri, 25 Feb 2022 15:38:18 -0800
Subject: [PATCH 03/15] Remove ldir == 0 special-case

This special-case was added in 06d1b8c63ae63035fc95c350da8f1879eca7fc25, but it would only trigger if the light's position exactly matches a vertex's position, which seems fairly unlikely (and I don't think has been hardware tested).  It also only exists in the no attenuation case, which seems odd (it was initially considered (in 08e3ade696db9dc85f1a329a7706cecea1285987) to apply it to all cases, but that wasn't done for unclear reasons).  Plus, normalizing a zero-length vector produces an undefined result.
---
 Source/Core/VideoBackends/Software/TransformUnit.cpp | 2 --
 Source/Core/VideoCommon/LightingShaderGen.cpp        | 1 -
 Source/Core/VideoCommon/UberShaderCommon.cpp         | 2 --
 3 files changed, 5 deletions(-)

diff --git a/Source/Core/VideoBackends/Software/TransformUnit.cpp b/Source/Core/VideoBackends/Software/TransformUnit.cpp
index 6fcf42df9e..b12864e741 100644
--- a/Source/Core/VideoBackends/Software/TransformUnit.cpp
+++ b/Source/Core/VideoBackends/Software/TransformUnit.cpp
@@ -221,8 +221,6 @@ static float CalculateLightAttn(const LightPointer* light, Vec3* _ldir, const Ve
   case AttenuationFunc::Dir:
   {
     ldir = ldir.Normalized();
-    if (ldir == Vec3(0.0f, 0.0f, 0.0f))
-      ldir = normal;
     break;
   }
   case AttenuationFunc::Spec:
diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index 5af01db616..27e5483b52 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -22,7 +22,6 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   case AttenuationFunc::Dir:
     object.Write("ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n", LIGHT_POS_PARAMS(index));
     object.Write("attn = 1.0;\n");
-    object.Write("if (length(ldir) == 0.0)\n\t ldir = _normal;\n");
     break;
   case AttenuationFunc::Spec:
     object.Write("ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n", LIGHT_POS_PARAMS(index));
diff --git a/Source/Core/VideoCommon/UberShaderCommon.cpp b/Source/Core/VideoCommon/UberShaderCommon.cpp
index 2af2f47927..c7f7ee9897 100644
--- a/Source/Core/VideoCommon/UberShaderCommon.cpp
+++ b/Source/Core/VideoCommon/UberShaderCommon.cpp
@@ -25,8 +25,6 @@ void WriteLightingFunction(ShaderCode& out)
   out.Write("  case {:s}:\n", AttenuationFunc::Dir);
   out.Write("    ldir = normalize(" I_LIGHTS "[index].pos.xyz - pos.xyz);\n"
             "    attn = 1.0;\n"
-            "    if (length(ldir) == 0.0)\n"
-            "      ldir = normal;\n"
             "    break;\n\n");
   out.Write("  case {:s}:\n", AttenuationFunc::Spec);
   out.Write("    ldir = normalize(" I_LIGHTS "[index].pos.xyz - pos.xyz);\n"

From 35d02401d98a69adc4bdb01eaf33376296af246f Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Fri, 25 Feb 2022 16:27:21 -0800
Subject: [PATCH 04/15] Indent lighting code and use proper local variables

---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 89 ++++++++++---------
 Source/Core/VideoCommon/PixelShaderGen.cpp    |  4 -
 Source/Core/VideoCommon/VertexShaderGen.cpp   |  6 +-
 3 files changed, 46 insertions(+), 53 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index 27e5483b52..52e3d9cf11 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -13,6 +13,7 @@
 static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_data, int index,
                                 AttenuationFunc attn_func, DiffuseFunc diffuse_func, bool alpha)
 {
+  object.Write("  {{ // {} light {}\n", alpha ? "Alpha" : "Color", index);
   const char* swizzle = alpha ? "a" : "rgb";
   const char* swizzle_components = (alpha) ? "" : "3";
 
@@ -20,30 +21,32 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   {
   case AttenuationFunc::None:
   case AttenuationFunc::Dir:
-    object.Write("ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n", LIGHT_POS_PARAMS(index));
-    object.Write("attn = 1.0;\n");
+    object.Write("    float3 ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n",
+                 LIGHT_POS_PARAMS(index));
+    object.Write("    float attn = 1.0;\n");
     break;
   case AttenuationFunc::Spec:
-    object.Write("ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n", LIGHT_POS_PARAMS(index));
-    object.Write("attn = (dot(_normal, ldir) >= 0.0) ? max(0.0, dot(_normal, " LIGHT_DIR
+    object.Write("    float3 ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n",
+                 LIGHT_POS_PARAMS(index));
+    object.Write("    float attn = (dot(_normal, ldir) >= 0.0) ? max(0.0, dot(_normal, " LIGHT_DIR
                  ".xyz)) : 0.0;\n",
                  LIGHT_DIR_PARAMS(index));
-    object.Write("cosAttn = " LIGHT_COSATT ".xyz;\n", LIGHT_COSATT_PARAMS(index));
-    object.Write("distAttn = {}(" LIGHT_DISTATT ".xyz);\n",
+    object.Write("    float3 cosAttn = " LIGHT_COSATT ".xyz;\n", LIGHT_COSATT_PARAMS(index));
+    object.Write("    float3 distAttn = {}(" LIGHT_DISTATT ".xyz);\n",
                  (diffuse_func == DiffuseFunc::None) ? "" : "normalize",
                  LIGHT_DISTATT_PARAMS(index));
-    object.Write("attn = max(0.0f, dot(cosAttn, float3(1.0, attn, attn*attn))) / dot(distAttn, "
+    object.Write("    attn = max(0.0f, dot(cosAttn, float3(1.0, attn, attn*attn))) / dot(distAttn, "
                  "float3(1.0, attn, attn*attn));\n");
     break;
   case AttenuationFunc::Spot:
-    object.Write("ldir = " LIGHT_POS ".xyz - pos.xyz;\n", LIGHT_POS_PARAMS(index));
-    object.Write("dist2 = dot(ldir, ldir);\n"
-                 "dist = sqrt(dist2);\n"
-                 "ldir = ldir / dist;\n"
-                 "attn = max(0.0, dot(ldir, " LIGHT_DIR ".xyz));\n",
+    object.Write("    float3 ldir = " LIGHT_POS ".xyz - pos.xyz;\n", LIGHT_POS_PARAMS(index));
+    object.Write("    float dist2 = dot(ldir, ldir);\n"
+                 "    float dist = sqrt(dist2);\n"
+                 "    ldir = ldir / dist;\n"
+                 "    float attn = max(0.0, dot(ldir, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
     // attn*attn may overflow
-    object.Write("attn = max(0.0, " LIGHT_COSATT ".x + " LIGHT_COSATT ".y*attn + " LIGHT_COSATT
+    object.Write("    attn = max(0.0, " LIGHT_COSATT ".x + " LIGHT_COSATT ".y*attn + " LIGHT_COSATT
                  ".z*attn*attn) / dot(" LIGHT_DISTATT ".xyz, float3(1.0,dist,dist2));\n",
                  LIGHT_COSATT_PARAMS(index), LIGHT_COSATT_PARAMS(index), LIGHT_COSATT_PARAMS(index),
                  LIGHT_DISTATT_PARAMS(index));
@@ -53,12 +56,12 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   switch (diffuse_func)
   {
   case DiffuseFunc::None:
-    object.Write("lacc.{} += int{}(round(attn * float{}(" LIGHT_COL ")));\n", swizzle,
+    object.Write("    lacc.{} += int{}(round(attn * float{}(" LIGHT_COL ")));\n", swizzle,
                  swizzle_components, swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
     break;
   case DiffuseFunc::Sign:
   case DiffuseFunc::Clamp:
-    object.Write("lacc.{} += int{}(round(attn * {}dot(ldir, _normal)) * float{}(" LIGHT_COL
+    object.Write("    lacc.{} += int{}(round(attn * {}dot(ldir, _normal)) * float{}(" LIGHT_COL
                  ")));\n",
                  swizzle, swizzle_components, diffuse_func != DiffuseFunc::Sign ? "max(0.0," : "(",
                  swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
@@ -67,7 +70,7 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
     ASSERT(false);
   }
 
-  object.Write("\n");
+  object.Write("  }}\n");
 }
 
 // vertex shader
@@ -91,52 +94,52 @@ void GenerateLightingShaderCode(ShaderCode& object, const LightingUidData& uid_d
     const bool alpha_enable = ((uid_data.enablelighting >> chan_a) & 1) != 0;
 
     object.Write("{{\n"
-                 "// Lighting for channel {}:\n"
-                 "// Color material source: {}\n"
-                 "// Color ambient source: {}\n"
-                 "// Color lighting enabled: {}\n"
-                 "// Alpha material source: {}\n"
-                 "// Alpha ambient source: {}\n"
-                 "// Alpha lighting enabled: {}\n",
+                 "  // Lighting for channel {}:\n"
+                 "  // Color material source: {}\n"
+                 "  // Color ambient source: {}\n"
+                 "  // Color lighting enabled: {}\n"
+                 "  // Alpha material source: {}\n"
+                 "  // Alpha ambient source: {}\n"
+                 "  // Alpha lighting enabled: {}\n",
                  chan, color_matsource, color_ambsource, color_enable, alpha_matsource,
                  alpha_ambsource, alpha_enable);
 
     if (color_matsource == MatSource::Vertex)
-      object.Write("int4 mat = int4(round({}{} * 255.0));\n", in_color_name, chan);
+      object.Write("  int4 mat = int4(round({}{} * 255.0));\n", in_color_name, chan);
     else  // from material color register
-      object.Write("int4 mat = {}[{}];\n", I_MATERIALS, chan + 2);
+      object.Write("  int4 mat = {}[{}];\n", I_MATERIALS, chan + 2);
 
     if (color_enable)
     {
       if (color_ambsource == AmbSource::Vertex)
-        object.Write("lacc = int4(round({}{} * 255.0));\n", in_color_name, chan);
+        object.Write("  int4 lacc = int4(round({}{} * 255.0));\n", in_color_name, chan);
       else  // from ambient color register
-        object.Write("lacc = {}[{}];\n", I_MATERIALS, chan);
+        object.Write("  int4 lacc = {}[{}];\n", I_MATERIALS, chan);
     }
     else
     {
-      object.Write("lacc = int4(255, 255, 255, 255);\n");
+      object.Write("  int4 lacc = int4(255, 255, 255, 255);\n");
     }
 
     // check if alpha is different
     if (color_matsource != alpha_matsource)
     {
       if (alpha_matsource == MatSource::Vertex)
-        object.Write("mat.w = int(round({}{}.w * 255.0));\n", in_color_name, chan);
+        object.Write("  mat.w = int(round({}{}.w * 255.0));\n", in_color_name, chan);
       else  // from material color register
-        object.Write("mat.w = {}[{}].w;\n", I_MATERIALS, chan + 2);
+        object.Write("  mat.w = {}[{}].w;\n", I_MATERIALS, chan + 2);
     }
 
     if (alpha_enable)
     {
       if (alpha_ambsource == AmbSource::Vertex)  // from vertex
-        object.Write("lacc.w = int(round({}{}.w * 255.0));\n", in_color_name, chan);
+        object.Write("  lacc.w = int(round({}{}.w * 255.0));\n", in_color_name, chan);
       else  // from ambient color register
-        object.Write("lacc.w = {}[{}].w;\n", I_MATERIALS, chan);
+        object.Write("  lacc.w = {}[{}].w;\n", I_MATERIALS, chan);
     }
     else
     {
-      object.Write("lacc.w = 255;\n");
+      object.Write("  lacc.w = 255;\n");
     }
 
     if (color_enable)
@@ -145,38 +148,36 @@ void GenerateLightingShaderCode(ShaderCode& object, const LightingUidData& uid_d
       const auto diffusefunc = static_cast<DiffuseFunc>((uid_data.diffusefunc >> (2 * chan)) & 3);
       const u32 light_mask =
           (uid_data.light_mask >> (NUM_XF_LIGHTS * chan)) & ((1 << NUM_XF_LIGHTS) - 1);
-      object.Write("// Color attenuation function: {}\n", attnfunc);
-      object.Write("// Color diffuse function: {}\n", diffusefunc);
-      object.Write("// Color light mask: {:08b}\n", light_mask);
+      object.Write("  // Color attenuation function: {}\n", attnfunc);
+      object.Write("  // Color diffuse function: {}\n", diffusefunc);
+      object.Write("  // Color light mask: {:08b}\n", light_mask);
       for (u32 light = 0; light < NUM_XF_LIGHTS; light++)
       {
         if ((light_mask & (1 << light)) != 0)
         {
-          object.Write("// Color light {}\n", light);
           GenerateLightShader(object, uid_data, light, attnfunc, diffusefunc, false);
         }
       }
     }
-    if (alpha_enable)  // Alpha lights
+    if (alpha_enable)
     {
       const auto attnfunc = static_cast<AttenuationFunc>((uid_data.attnfunc >> (2 * chan_a)) & 3);
       const auto diffusefunc = static_cast<DiffuseFunc>((uid_data.diffusefunc >> (2 * chan_a)) & 3);
       const u32 light_mask =
           (uid_data.light_mask >> (NUM_XF_LIGHTS * chan_a)) & ((1 << NUM_XF_LIGHTS) - 1);
-      object.Write("// Alpha attenuation function: {}\n", attnfunc);
-      object.Write("// Alpha diffuse function: {}\n", diffusefunc);
-      object.Write("// Alpha light mask function: {:08b}\n", light_mask);
+      object.Write("  // Alpha attenuation function: {}\n", attnfunc);
+      object.Write("  // Alpha diffuse function: {}\n", diffusefunc);
+      object.Write("  // Alpha light mask function: {:08b}\n", light_mask);
       for (u32 light = 0; light < NUM_XF_LIGHTS; light++)
       {
         if ((light_mask & (1 << light)) != 0)
         {
-          object.Write("// Alpha light {}\n", light);
           GenerateLightShader(object, uid_data, light, attnfunc, diffusefunc, true);
         }
       }
     }
-    object.Write("lacc = clamp(lacc, 0, 255);\n");
-    object.Write("{}{} = float4((mat * (lacc + (lacc >> 7))) >> 8) / 255.0;\n", dest, chan);
+    object.Write("  lacc = clamp(lacc, 0, 255);\n");
+    object.Write("  {}{} = float4((mat * (lacc + (lacc >> 7))) >> 8) / 255.0;\n", dest, chan);
     object.Write("}}\n");
   }
 }
diff --git a/Source/Core/VideoCommon/PixelShaderGen.cpp b/Source/Core/VideoCommon/PixelShaderGen.cpp
index 57b2113bc3..e8b720f1f5 100644
--- a/Source/Core/VideoCommon/PixelShaderGen.cpp
+++ b/Source/Core/VideoCommon/PixelShaderGen.cpp
@@ -955,10 +955,6 @@ ShaderCode GeneratePixelShaderCode(APIType api_type, const ShaderHostConfig& hos
     out.Write("\tfloat3 _normal = normalize(Normal.xyz);\n\n"
               "\tfloat3 pos = WorldPos;\n");
 
-    out.Write("\tint4 lacc;\n"
-              "\tfloat3 ldir, h, cosAttn, distAttn;\n"
-              "\tfloat dist, dist2, attn;\n");
-
     // TODO: Our current constant usage code isn't able to handle more than one buffer.
     //       So we can't mark the VS constant as used here. But keep them here as reference.
     // out.SetConstantsUsed(C_PLIGHT_COLORS, C_PLIGHT_COLORS+7); // TODO: Can be optimized further
diff --git a/Source/Core/VideoCommon/VertexShaderGen.cpp b/Source/Core/VideoCommon/VertexShaderGen.cpp
index ff555b74fe..b294ee37a2 100644
--- a/Source/Core/VideoCommon/VertexShaderGen.cpp
+++ b/Source/Core/VideoCommon/VertexShaderGen.cpp
@@ -366,10 +366,6 @@ ShaderCode GenerateVertexShaderCode(APIType api_type, const ShaderHostConfig& ho
   out.Write("o.pos = float4(dot(" I_PROJECTION "[0], pos), dot(" I_PROJECTION
             "[1], pos), dot(" I_PROJECTION "[2], pos), dot(" I_PROJECTION "[3], pos));\n");
 
-  out.Write("int4 lacc;\n"
-            "float3 ldir, h, cosAttn, distAttn;\n"
-            "float dist, dist2, attn;\n");
-
   GenerateLightingShaderCode(out, uid_data->lighting, "vertex_color_", "o.colors_");
 
   // transform texcoords
@@ -433,7 +429,7 @@ ShaderCode GenerateVertexShaderCode(APIType api_type, const ShaderHostConfig& ho
     case TexGenType::EmbossMap:  // calculate tex coords into bump map
 
       // transform the light dir into tangent space
-      out.Write("ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n",
+      out.Write("float3 ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n",
                 LIGHT_POS_PARAMS(texinfo.embosslightshift));
       out.Write(
           "o.tex{}.xyz = o.tex{}.xyz + float3(dot(ldir, _tangent), dot(ldir, _binormal), 0.0);\n",

From a5e531d9faecd1cf2c6b1a87f0f3894c67147cba Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Fri, 25 Feb 2022 17:03:50 -0800
Subject: [PATCH 05/15] Remove normalization of distAttn

distAttn is coefficients to a polynomial, not a vector in space; it doesn't make sense for it to be normalized.  This was added in f475e367f2d1d4952cf4e9b8c86f68f9e09f3b73.
---
 Source/Core/VideoBackends/Software/TransformUnit.cpp | 2 --
 Source/Core/VideoCommon/LightingShaderGen.cpp        | 4 +---
 Source/Core/VideoCommon/UberShaderCommon.cpp         | 7 ++-----
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/Source/Core/VideoBackends/Software/TransformUnit.cpp b/Source/Core/VideoBackends/Software/TransformUnit.cpp
index b12864e741..8429a60a43 100644
--- a/Source/Core/VideoBackends/Software/TransformUnit.cpp
+++ b/Source/Core/VideoBackends/Software/TransformUnit.cpp
@@ -230,8 +230,6 @@ static float CalculateLightAttn(const LightPointer* light, Vec3* _ldir, const Ve
     Vec3 attLen = Vec3(1.0, attn, attn * attn);
     Vec3 cosAttn = light->cosatt;
     Vec3 distAttn = light->distatt;
-    if (chan.diffusefunc != DiffuseFunc::None)
-      distAttn = distAttn.Normalized();
 
     attn = SafeDivide(std::max(0.0f, attLen * cosAttn), attLen * distAttn);
     break;
diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index 52e3d9cf11..1e4687fc40 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -32,9 +32,7 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
                  ".xyz)) : 0.0;\n",
                  LIGHT_DIR_PARAMS(index));
     object.Write("    float3 cosAttn = " LIGHT_COSATT ".xyz;\n", LIGHT_COSATT_PARAMS(index));
-    object.Write("    float3 distAttn = {}(" LIGHT_DISTATT ".xyz);\n",
-                 (diffuse_func == DiffuseFunc::None) ? "" : "normalize",
-                 LIGHT_DISTATT_PARAMS(index));
+    object.Write("    float3 distAttn = " LIGHT_DISTATT ".xyz);\n", LIGHT_DISTATT_PARAMS(index));
     object.Write("    attn = max(0.0f, dot(cosAttn, float3(1.0, attn, attn*attn))) / dot(distAttn, "
                  "float3(1.0, attn, attn*attn));\n");
     break;
diff --git a/Source/Core/VideoCommon/UberShaderCommon.cpp b/Source/Core/VideoCommon/UberShaderCommon.cpp
index c7f7ee9897..06f0c0985c 100644
--- a/Source/Core/VideoCommon/UberShaderCommon.cpp
+++ b/Source/Core/VideoCommon/UberShaderCommon.cpp
@@ -30,11 +30,8 @@ void WriteLightingFunction(ShaderCode& out)
   out.Write("    ldir = normalize(" I_LIGHTS "[index].pos.xyz - pos.xyz);\n"
             "    attn = (dot(normal, ldir) >= 0.0) ? max(0.0, dot(normal, " I_LIGHTS
             "[index].dir.xyz)) : 0.0;\n"
-            "    cosAttn = " I_LIGHTS "[index].cosatt.xyz;\n");
-  out.Write("    if (diffusefunc == {:s})\n", DiffuseFunc::None);
-  out.Write("      distAttn = " I_LIGHTS "[index].distatt.xyz;\n"
-            "    else\n"
-            "      distAttn = normalize(" I_LIGHTS "[index].distatt.xyz);\n"
+            "    cosAttn = " I_LIGHTS "[index].cosatt.xyz;\n"
+            "    distAttn = " I_LIGHTS "[index].distatt.xyz;\n"
             "    attn = max(0.0, dot(cosAttn, float3(1.0, attn, attn*attn))) / dot(distAttn, "
             "float3(1.0, attn, attn*attn));\n"
             "    break;\n\n");

From f419a405584087270bd56a24df281dc1543282dc Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Fri, 25 Feb 2022 19:37:23 -0800
Subject: [PATCH 06/15] Refactor LightingShaderGen further

---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 82 ++++++++++++-------
 1 file changed, 53 insertions(+), 29 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index 1e4687fc40..b5c7e36d9e 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -17,57 +17,81 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   const char* swizzle = alpha ? "a" : "rgb";
   const char* swizzle_components = (alpha) ? "" : "3";
 
+  // Create a normalized vector pointing from the light to the current position.  We manualy
+  // normalize instead of using normalize() because the raw distance is needed for spot lights.
+  object.Write("    float3 ldir = " LIGHT_POS ".xyz - pos.xyz;\n", LIGHT_POS_PARAMS(index));
+  object.Write("    float dist2 = dot(ldir, ldir);\n"
+               "    float dist = sqrt(dist2);\n"
+               "    ldir = ldir / dist;\n");
+
   switch (attn_func)
   {
   case AttenuationFunc::None:
   case AttenuationFunc::Dir:
-    object.Write("    float3 ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n",
-                 LIGHT_POS_PARAMS(index));
     object.Write("    float attn = 1.0;\n");
     break;
   case AttenuationFunc::Spec:
-    object.Write("    float3 ldir = normalize(" LIGHT_POS ".xyz - pos.xyz);\n",
-                 LIGHT_POS_PARAMS(index));
-    object.Write("    float attn = (dot(_normal, ldir) >= 0.0) ? max(0.0, dot(_normal, " LIGHT_DIR
-                 ".xyz)) : 0.0;\n",
+    object.Write("    float cosine = 0.0;\n"
+                 "    // Ensure that the object is facing the light\n"
+                 "    if (dot(_normal, ldir) >= 0.0) {{\n"
+                 "      // Compute the cosine of the angle between the object normal\n"
+                 "      // and the half-angle direction for the viewer\n"
+                 "      // (assuming the half-angle direction is a unit vector)\n"
+                 "      cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
-    object.Write("    float3 cosAttn = " LIGHT_COSATT ".xyz;\n", LIGHT_COSATT_PARAMS(index));
-    object.Write("    float3 distAttn = " LIGHT_DISTATT ".xyz);\n", LIGHT_DISTATT_PARAMS(index));
-    object.Write("    attn = max(0.0f, dot(cosAttn, float3(1.0, attn, attn*attn))) / dot(distAttn, "
-                 "float3(1.0, attn, attn*attn));\n");
+    object.Write("    }}\n"
+                 "    // Specular lights use the angle for the denominator as well\n"
+                 "    dist = cosine;\n"
+                 "    dist2 = dist * dist;\n");
     break;
   case AttenuationFunc::Spot:
-    object.Write("    float3 ldir = " LIGHT_POS ".xyz - pos.xyz;\n", LIGHT_POS_PARAMS(index));
-    object.Write("    float dist2 = dot(ldir, ldir);\n"
-                 "    float dist = sqrt(dist2);\n"
-                 "    ldir = ldir / dist;\n"
-                 "    float attn = max(0.0, dot(ldir, " LIGHT_DIR ".xyz));\n",
+    object.Write("    // Compute the cosine of the angle between the vector to the object\n"
+                 "    // and the light's direction (assuming the direction is a unit vector)\n"
+                 "    float cosine = max(0.0, dot(ldir, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
-    // attn*attn may overflow
-    object.Write("    attn = max(0.0, " LIGHT_COSATT ".x + " LIGHT_COSATT ".y*attn + " LIGHT_COSATT
-                 ".z*attn*attn) / dot(" LIGHT_DISTATT ".xyz, float3(1.0,dist,dist2));\n",
-                 LIGHT_COSATT_PARAMS(index), LIGHT_COSATT_PARAMS(index), LIGHT_COSATT_PARAMS(index),
-                 LIGHT_DISTATT_PARAMS(index));
     break;
   }
 
+  if (attn_func == AttenuationFunc::Spot || attn_func == AttenuationFunc::Spec)
+  {
+    object.Write("    float3 cosAttn = " LIGHT_COSATT ".xyz;\n", LIGHT_COSATT_PARAMS(index));
+    object.Write("    float3 distAttn = " LIGHT_DISTATT ".xyz;\n", LIGHT_DISTATT_PARAMS(index));
+    object.Write("    float cosine2 = cosine * cosine;\n"
+                 ""
+                 "    // This is equivalent to dot(cosAttn, float3(1.0, attn, attn*attn)),\n"
+                 "    // except with spot lights games often don't set the direction value,\n"
+                 "    // as they configure cosAttn to (1, 0, 0).  GX light objects are often\n"
+                 "    // stack-allocated, so those values are uninitialized and may be\n"
+                 "    // arbitrary garbage, including NaN or Inf, or become Inf when squared.\n"
+                 "    float numerator = cosAttn.x;                            // constant term\n"
+                 "    if (cosAttn.y != 0.0) numerator += cosAttn.y * cosine;  // linear term\n"
+                 "    if (cosAttn.z != 0.0) numerator += cosAttn.z * cosine2; // quadratic term\n"
+                 "    // Same with the denominator, though this generally is not garbage\n"
+                 "    // Note that VertexShaderManager ensures that distAttn is not zero, which\n"
+                 "    // should prevent division by zero (TODO: what does real hardware do?)\n"
+                 "    float denominator = distAttn.x;                           // constant term\n"
+                 "    if (distAttn.y != 0.0) denominator += distAttn.y * dist;  // linear term\n"
+                 "    if (distAttn.z != 0.0) denominator += distAttn.z * dist2; // quadratic term\n"
+                 ""
+                 "    float attn = max(0.0f, numerator / denominator);\n");
+  }
+
   switch (diffuse_func)
   {
   case DiffuseFunc::None:
-    object.Write("    lacc.{} += int{}(round(attn * float{}(" LIGHT_COL ")));\n", swizzle,
-                 swizzle_components, swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
+    object.Write("    float diffuse = 1.0;\n");
     break;
   case DiffuseFunc::Sign:
-  case DiffuseFunc::Clamp:
-    object.Write("    lacc.{} += int{}(round(attn * {}dot(ldir, _normal)) * float{}(" LIGHT_COL
-                 ")));\n",
-                 swizzle, swizzle_components, diffuse_func != DiffuseFunc::Sign ? "max(0.0," : "(",
-                 swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
+  default:  // Confirmed by hardware testing that invalid values use this
+    object.Write("    float diffuse = dot(ldir, _normal);\n");
+    break;
+  case DiffuseFunc::Clamp:
+    object.Write("    float diffuse = max(0.0, dot(ldir, _normal));\n");
     break;
-  default:
-    ASSERT(false);
   }
 
+  object.Write("    lacc.{} += int{}(round(attn * diffuse * float{}(" LIGHT_COL ")));\n", swizzle,
+               swizzle_components, swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
   object.Write("  }}\n");
 }
 

From 0c34ca7b89412e5cecaf1da106e780628b2704d7 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 27 Feb 2022 12:05:21 -0800
Subject: [PATCH 07/15] Further refactoring attempt

---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 55 +++++++++++--------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index b5c7e36d9e..af40cce48e 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -22,25 +22,48 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   object.Write("    float3 ldir = " LIGHT_POS ".xyz - pos.xyz;\n", LIGHT_POS_PARAMS(index));
   object.Write("    float dist2 = dot(ldir, ldir);\n"
                "    float dist = sqrt(dist2);\n"
-               "    ldir = ldir / dist;\n");
+               "    ldir = ldir / dist;\n\n");
+
+  switch (diffuse_func)
+  {
+  case DiffuseFunc::None:
+    object.Write("    float diffuse = 1.0;\n\n");
+    break;
+  case DiffuseFunc::Sign:
+  default:  // Confirmed by hardware testing that invalid values use this
+    object.Write("    float diffuse = dot(ldir, _normal);\n\n");
+    break;
+  case DiffuseFunc::Clamp:
+    object.Write("    float diffuse = max(0.0, dot(ldir, _normal));\n\n");
+    break;
+  }
 
   switch (attn_func)
   {
   case AttenuationFunc::None:
+    if (diffuse_func == DiffuseFunc::None)
+    {
+      object.Write("    float attn = 1.0;\n");
+      break;
+    }
+    else
+    {
+      object.Write("    lacc.{} += int{}(round(255 * sign(diffuse)));\n", swizzle,
+                   swizzle_components);
+      object.Write("  }}\n");
+      return;  // Return, not break!
+    }
   case AttenuationFunc::Dir:
     object.Write("    float attn = 1.0;\n");
     break;
   case AttenuationFunc::Spec:
     object.Write("    float cosine = 0.0;\n"
-                 "    // Ensure that the object is facing the light\n"
-                 "    if (dot(_normal, ldir) >= 0.0) {{\n"
-                 "      // Compute the cosine of the angle between the object normal\n"
-                 "      // and the half-angle direction for the viewer\n"
-                 "      // (assuming the half-angle direction is a unit vector)\n"
-                 "      cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
+                 "    // Compute the cosine of the angle between the object normal\n"
+                 "    // and the half-angle direction for the viewer\n"
+                 "    // (assuming the half-angle direction is a unit vector)\n"
+                 "    cosine = max(0.0, dot(_normal, -" LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
-    object.Write("    }}\n"
-                 "    // Specular lights use the angle for the denominator as well\n"
+    object.Write("    // Specular lights use the angle for the denominator as well\n"
                  "    dist = cosine;\n"
                  "    dist2 = dist * dist;\n");
     break;
@@ -76,20 +99,6 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
                  "    float attn = max(0.0f, numerator / denominator);\n");
   }
 
-  switch (diffuse_func)
-  {
-  case DiffuseFunc::None:
-    object.Write("    float diffuse = 1.0;\n");
-    break;
-  case DiffuseFunc::Sign:
-  default:  // Confirmed by hardware testing that invalid values use this
-    object.Write("    float diffuse = dot(ldir, _normal);\n");
-    break;
-  case DiffuseFunc::Clamp:
-    object.Write("    float diffuse = max(0.0, dot(ldir, _normal));\n");
-    break;
-  }
-
   object.Write("    lacc.{} += int{}(round(attn * diffuse * float{}(" LIGHT_COL ")));\n", swizzle,
                swizzle_components, swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
   object.Write("  }}\n");

From c63ea902f0e462f0d9afecebbc38a4bcb3b97cc0 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 27 Feb 2022 12:06:03 -0800
Subject: [PATCH 08/15] Revert "Further refactoring attempt"

This reverts commit 334ca8df73c2686ac1c793313fc70c97a6142bea.
---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 55 ++++++++-----------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index af40cce48e..b5c7e36d9e 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -22,48 +22,25 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   object.Write("    float3 ldir = " LIGHT_POS ".xyz - pos.xyz;\n", LIGHT_POS_PARAMS(index));
   object.Write("    float dist2 = dot(ldir, ldir);\n"
                "    float dist = sqrt(dist2);\n"
-               "    ldir = ldir / dist;\n\n");
-
-  switch (diffuse_func)
-  {
-  case DiffuseFunc::None:
-    object.Write("    float diffuse = 1.0;\n\n");
-    break;
-  case DiffuseFunc::Sign:
-  default:  // Confirmed by hardware testing that invalid values use this
-    object.Write("    float diffuse = dot(ldir, _normal);\n\n");
-    break;
-  case DiffuseFunc::Clamp:
-    object.Write("    float diffuse = max(0.0, dot(ldir, _normal));\n\n");
-    break;
-  }
+               "    ldir = ldir / dist;\n");
 
   switch (attn_func)
   {
   case AttenuationFunc::None:
-    if (diffuse_func == DiffuseFunc::None)
-    {
-      object.Write("    float attn = 1.0;\n");
-      break;
-    }
-    else
-    {
-      object.Write("    lacc.{} += int{}(round(255 * sign(diffuse)));\n", swizzle,
-                   swizzle_components);
-      object.Write("  }}\n");
-      return;  // Return, not break!
-    }
   case AttenuationFunc::Dir:
     object.Write("    float attn = 1.0;\n");
     break;
   case AttenuationFunc::Spec:
     object.Write("    float cosine = 0.0;\n"
-                 "    // Compute the cosine of the angle between the object normal\n"
-                 "    // and the half-angle direction for the viewer\n"
-                 "    // (assuming the half-angle direction is a unit vector)\n"
-                 "    cosine = max(0.0, dot(_normal, -" LIGHT_DIR ".xyz));\n",
+                 "    // Ensure that the object is facing the light\n"
+                 "    if (dot(_normal, ldir) >= 0.0) {{\n"
+                 "      // Compute the cosine of the angle between the object normal\n"
+                 "      // and the half-angle direction for the viewer\n"
+                 "      // (assuming the half-angle direction is a unit vector)\n"
+                 "      cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
-    object.Write("    // Specular lights use the angle for the denominator as well\n"
+    object.Write("    }}\n"
+                 "    // Specular lights use the angle for the denominator as well\n"
                  "    dist = cosine;\n"
                  "    dist2 = dist * dist;\n");
     break;
@@ -99,6 +76,20 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
                  "    float attn = max(0.0f, numerator / denominator);\n");
   }
 
+  switch (diffuse_func)
+  {
+  case DiffuseFunc::None:
+    object.Write("    float diffuse = 1.0;\n");
+    break;
+  case DiffuseFunc::Sign:
+  default:  // Confirmed by hardware testing that invalid values use this
+    object.Write("    float diffuse = dot(ldir, _normal);\n");
+    break;
+  case DiffuseFunc::Clamp:
+    object.Write("    float diffuse = max(0.0, dot(ldir, _normal));\n");
+    break;
+  }
+
   object.Write("    lacc.{} += int{}(round(attn * diffuse * float{}(" LIGHT_COL ")));\n", swizzle,
                swizzle_components, swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
   object.Write("  }}\n");

From 8a53fe4f46d9caa638954363d3de7ef43fcdd4cc Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 27 Feb 2022 12:21:49 -0800
Subject: [PATCH 09/15] Correctly handle attenuation func of none

---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index b5c7e36d9e..906907bb04 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -27,6 +27,15 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
   switch (attn_func)
   {
   case AttenuationFunc::None:
+    // This logic correctly reproduces the behavior (if diffuse > 0, then lacc is 255, if it's < 0
+    // lacc is 0, and if it's equal to 0 lacc is unchanged, but with DiffuseFunc::None lacc instead
+    // has the light's color added to it), but may be an overly jank implementation (and might give
+    // incorrect results for a light value of 1/256, for instance; testing is needed)
+    if (diffuse_func == DiffuseFunc::None)
+      object.Write("    float attn = 1.0;\n");
+    else
+      object.Write("    float attn = 1024.0;\n");
+    break;
   case AttenuationFunc::Dir:
     object.Write("    float attn = 1.0;\n");
     break;

From 137b81338f96772aed252b59b2ed4ad8a60ecdb4 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 27 Feb 2022 12:50:24 -0800
Subject: [PATCH 10/15] Remove front-facing check for specular lights

This is basically a copy of the diffusefunc; it doesn't make sense for it to be implemented in both places (though I haven't confirmed that it isn't)
---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index 906907bb04..130fc27e31 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -41,15 +41,12 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
     break;
   case AttenuationFunc::Spec:
     object.Write("    float cosine = 0.0;\n"
-                 "    // Ensure that the object is facing the light\n"
-                 "    if (dot(_normal, ldir) >= 0.0) {{\n"
-                 "      // Compute the cosine of the angle between the object normal\n"
-                 "      // and the half-angle direction for the viewer\n"
-                 "      // (assuming the half-angle direction is a unit vector)\n"
-                 "      cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
+                 "    // Compute the cosine of the angle between the object normal\n"
+                 "    // and the half-angle direction for the viewer\n"
+                 "    // (assuming the half-angle direction is a unit vector)\n"
+                 "    cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
-    object.Write("    }}\n"
-                 "    // Specular lights use the angle for the denominator as well\n"
+    object.Write("    // Specular lights use the angle for the denominator as well\n"
                  "    dist = cosine;\n"
                  "    dist2 = dist * dist;\n");
     break;

From a7cd938ef78ca896f1048c2eb1dbb437c730b244 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 27 Feb 2022 13:58:21 -0800
Subject: [PATCH 11/15] Revert "Remove front-facing check for specular lights"

This reverts commit 0b425bbb41daa1f613d99641cf8da35cc901a4a1.  The patent says this exists.
---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index 130fc27e31..906907bb04 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -41,12 +41,15 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
     break;
   case AttenuationFunc::Spec:
     object.Write("    float cosine = 0.0;\n"
-                 "    // Compute the cosine of the angle between the object normal\n"
-                 "    // and the half-angle direction for the viewer\n"
-                 "    // (assuming the half-angle direction is a unit vector)\n"
-                 "    cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
+                 "    // Ensure that the object is facing the light\n"
+                 "    if (dot(_normal, ldir) >= 0.0) {{\n"
+                 "      // Compute the cosine of the angle between the object normal\n"
+                 "      // and the half-angle direction for the viewer\n"
+                 "      // (assuming the half-angle direction is a unit vector)\n"
+                 "      cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
-    object.Write("    // Specular lights use the angle for the denominator as well\n"
+    object.Write("    }}\n"
+                 "    // Specular lights use the angle for the denominator as well\n"
                  "    dist = cosine;\n"
                  "    dist2 = dist * dist;\n");
     break;

From 8ab57474a04fd22555c1cb53db1fc10759723510 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 27 Feb 2022 15:06:50 -0800
Subject: [PATCH 12/15] Down this route lies madness

---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index 906907bb04..a3aa628071 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -49,7 +49,9 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
                  "      cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
     object.Write("    }}\n"
-                 "    // Specular lights use the angle for the denominator as well\n"
+                 "    ldir = " LIGHT_DIR ".xyz;\n",
+                 LIGHT_DIR_PARAMS(index));
+    object.Write("    // Specular lights use the angle for the denominator as well\n"
                  "    dist = cosine;\n"
                  "    dist2 = dist * dist;\n");
     break;
@@ -99,6 +101,9 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
     break;
   }
 
+  if (attn_func == AttenuationFunc::Spec && diffuse_func != DiffuseFunc::None)
+    object.Write("    diffuse *= (2048.0 + 1024.0);\n");
+
   object.Write("    lacc.{} += int{}(round(attn * diffuse * float{}(" LIGHT_COL ")));\n", swizzle,
                swizzle_components, swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
   object.Write("  }}\n");

From fca66eaf53d8e8ee28b192088c1eb01bb14bbca4 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 27 Feb 2022 15:06:52 -0800
Subject: [PATCH 13/15] Revert "Down this route lies madness"

This reverts commit 6cad59caf4add6d36f03d21d1785e445ca9bd999.
---
 Source/Core/VideoCommon/LightingShaderGen.cpp | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/Source/Core/VideoCommon/LightingShaderGen.cpp b/Source/Core/VideoCommon/LightingShaderGen.cpp
index a3aa628071..906907bb04 100644
--- a/Source/Core/VideoCommon/LightingShaderGen.cpp
+++ b/Source/Core/VideoCommon/LightingShaderGen.cpp
@@ -49,9 +49,7 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
                  "      cosine = max(0.0, dot(_normal, " LIGHT_DIR ".xyz));\n",
                  LIGHT_DIR_PARAMS(index));
     object.Write("    }}\n"
-                 "    ldir = " LIGHT_DIR ".xyz;\n",
-                 LIGHT_DIR_PARAMS(index));
-    object.Write("    // Specular lights use the angle for the denominator as well\n"
+                 "    // Specular lights use the angle for the denominator as well\n"
                  "    dist = cosine;\n"
                  "    dist2 = dist * dist;\n");
     break;
@@ -101,9 +99,6 @@ static void GenerateLightShader(ShaderCode& object, const LightingUidData& uid_d
     break;
   }
 
-  if (attn_func == AttenuationFunc::Spec && diffuse_func != DiffuseFunc::None)
-    object.Write("    diffuse *= (2048.0 + 1024.0);\n");
-
   object.Write("    lacc.{} += int{}(round(attn * diffuse * float{}(" LIGHT_COL ")));\n", swizzle,
                swizzle_components, swizzle_components, LIGHT_COL_PARAMS(index, swizzle));
   object.Write("  }}\n");

From 694f4b7acb1ab31d3e531fc77c73cab5fb892ebc Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Mon, 28 Feb 2022 12:11:17 -0800
Subject: [PATCH 14/15] Software: Panic alert on specular lighting use - is it
 actually being used by things other than Mario Tennis?

---
 Source/Core/VideoBackends/Software/TransformUnit.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Source/Core/VideoBackends/Software/TransformUnit.cpp b/Source/Core/VideoBackends/Software/TransformUnit.cpp
index 8429a60a43..8897d1e367 100644
--- a/Source/Core/VideoBackends/Software/TransformUnit.cpp
+++ b/Source/Core/VideoBackends/Software/TransformUnit.cpp
@@ -225,6 +225,7 @@ static float CalculateLightAttn(const LightPointer* light, Vec3* _ldir, const Ve
   }
   case AttenuationFunc::Spec:
   {
+    PanicAlertFmt("Specular lighting in use!");
     ldir = ldir.Normalized();
     attn = (ldir * normal) >= 0.0 ? std::max(0.0f, light->dir * normal) : 0;
     Vec3 attLen = Vec3(1.0, attn, attn * attn);

From 3f96e6c7ee2eda81e4e8a4c9cc59c5f4f3ca1bcb Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Mon, 28 Feb 2022 12:11:43 -0800
Subject: [PATCH 15/15] Same with the None mode

---
 Source/Core/VideoBackends/Software/TransformUnit.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Source/Core/VideoBackends/Software/TransformUnit.cpp b/Source/Core/VideoBackends/Software/TransformUnit.cpp
index 8897d1e367..a967e85ca3 100644
--- a/Source/Core/VideoBackends/Software/TransformUnit.cpp
+++ b/Source/Core/VideoBackends/Software/TransformUnit.cpp
@@ -218,6 +218,11 @@ static float CalculateLightAttn(const LightPointer* light, Vec3* _ldir, const Ve
   switch (chan.attnfunc)
   {
   case AttenuationFunc::None:
+  {
+    PanicAlertFmt("None lighting in use!");
+    ldir = ldir.Normalized();
+    break;
+  }
   case AttenuationFunc::Dir:
   {
     ldir = ldir.Normalized();