BPMemory: Make TevKSel more clear

It stores both the konst selection value for alpha and color channels (for two tev stages per ksel), and half of a swap table row (there are 4 total swap tables, which can be used for swizzling the rasterized color and the texture color, and indices selecting which tables to use are stored per tev stage in the alpha combiner).  Since these are indexed very differently, the old code was hard to follow.

Source/Core/VideoBackends/Software/Tev.cpp

@@ -37,28 +37,28 @@ static inline s16 Clamp1024(s16 in)
   return std::clamp<s16>(in, -1024, 1023);
 }
 
-void Tev::SetRasColor(RasColorChan colorChan, int swaptable)
+void Tev::SetRasColor(RasColorChan colorChan, u32 swaptable)
 {
   switch (colorChan)
   {
   case RasColorChan::Color0:
   {
     const u8* color = Color[0];
-    RasColor.r = color[bpmem.tevksel[swaptable].swap1];
+    const auto& swap = bpmem.tevksel.GetSwapTable(swaptable);
-    RasColor.g = color[bpmem.tevksel[swaptable].swap2];
+    RasColor.r = color[u32(swap[ColorChannel::Red])];
-    swaptable++;
+    RasColor.g = color[u32(swap[ColorChannel::Green])];
-    RasColor.b = color[bpmem.tevksel[swaptable].swap1];
+    RasColor.b = color[u32(swap[ColorChannel::Blue])];
-    RasColor.a = color[bpmem.tevksel[swaptable].swap2];
+    RasColor.a = color[u32(swap[ColorChannel::Alpha])];
   }
   break;
   case RasColorChan::Color1:
   {
     const u8* color = Color[1];
-    RasColor.r = color[bpmem.tevksel[swaptable].swap1];
+    const auto& swap = bpmem.tevksel.GetSwapTable(swaptable);
-    RasColor.g = color[bpmem.tevksel[swaptable].swap2];
+    RasColor.r = color[u32(swap[ColorChannel::Red])];
-    swaptable++;
+    RasColor.g = color[u32(swap[ColorChannel::Green])];
-    RasColor.b = color[bpmem.tevksel[swaptable].swap1];
+    RasColor.b = color[u32(swap[ColorChannel::Blue])];
-    RasColor.a = color[bpmem.tevksel[swaptable].swap2];
+    RasColor.a = color[u32(swap[ColorChannel::Alpha])];
   }
   break;
   case RasColorChan::AlphaBump:
@@ -445,7 +445,6 @@ void Tev::Draw()
     const int stageNum2 = stageNum >> 1;
     const int stageOdd = stageNum & 1;
     const TwoTevStageOrders& order = bpmem.tevorders[stageNum2];
-    const TevKSel& kSel = bpmem.tevksel[stageNum2];
 
     // stage combiners
     const TevStageCombiner::ColorCombiner& cc = bpmem.combiners[stageNum].colorC;
@@ -484,25 +483,23 @@ void Tev::Draw()
         DebugUtil::DrawTempBuffer(texel, DIRECT_TFETCH + stageNum);
 #endif
 
-      int swaptable = ac.tswap * 2;
+      const auto& swap = bpmem.tevksel.GetSwapTable(ac.tswap);
-
+      TexColor.r = texel[u32(swap[ColorChannel::Red])];
-      TexColor.r = texel[bpmem.tevksel[swaptable].swap1];
+      TexColor.g = texel[u32(swap[ColorChannel::Green])];
-      TexColor.g = texel[bpmem.tevksel[swaptable].swap2];
+      TexColor.b = texel[u32(swap[ColorChannel::Blue])];
-      swaptable++;
+      TexColor.a = texel[u32(swap[ColorChannel::Alpha])];
-      TexColor.b = texel[bpmem.tevksel[swaptable].swap1];
-      TexColor.a = texel[bpmem.tevksel[swaptable].swap2];
     }
 
     // set konst for this stage
-    const auto kc = kSel.getKC(stageOdd);
+    const auto kc = bpmem.tevksel.GetKonstColor(stageNum);
-    const auto ka = kSel.getKA(stageOdd);
+    const auto ka = bpmem.tevksel.GetKonstAlpha(stageNum);
     StageKonst.r = m_KonstLUT[kc].r;
     StageKonst.g = m_KonstLUT[kc].g;
     StageKonst.b = m_KonstLUT[kc].b;
     StageKonst.a = m_KonstLUT[ka].a;
 
     // set color
-    SetRasColor(order.getColorChan(stageOdd), ac.rswap * 2);
+    SetRasColor(order.getColorChan(stageOdd), ac.rswap);
 
     // combine inputs
     InputRegType inputs[4];

Source/Core/VideoBackends/Software/Tev.h

@@ -200,7 +200,7 @@ class Tev
     INDIRECT = 32
   };
 
-  void SetRasColor(RasColorChan colorChan, int swaptable);
+  void SetRasColor(RasColorChan colorChan, u32 swaptable);
 
   void DrawColorRegular(const TevStageCombiner::ColorCombiner& cc, const InputRegType inputs[4]);
   void DrawColorCompare(const TevStageCombiner::ColorCombiner& cc, const InputRegType inputs[4]);

Source/Core/VideoCommon/BPMemory.h

@@ -11,6 +11,7 @@
 #include "Common/BitUtils.h"
 #include "Common/CommonTypes.h"
 #include "Common/EnumFormatter.h"
+#include "Common/EnumMap.h"
 #include "Common/Inline.h"
 
 // X.h defines None to be 0 and Always to be 2, which causes problems with some of the enums
@@ -1838,6 +1839,19 @@ struct fmt::formatter<TevReg>
   }
 };
 
+enum class ColorChannel : u32
+{
+  Red = 0,
+  Green = 1,
+  Blue = 2,
+  Alpha = 3,
+};
+template <>
+struct fmt::formatter<ColorChannel> : EnumFormatter<ColorChannel::Alpha>
+{
+  formatter() : EnumFormatter({"Red", "Green", "Blue", "Alpha"}) {}
+};
+
 enum class KonstSel : u32
 {
   V1 = 0,
@@ -1912,16 +1926,13 @@ struct fmt::formatter<KonstSel> : EnumFormatter<KonstSel::K3_A>
 
 union TevKSel
 {
-  BitField<0, 2, u32> swap1;
+  BitField<0, 2, ColorChannel> swap_rb;  // Odd ksel number: red; even: blue
-  BitField<2, 2, u32> swap2;
+  BitField<2, 2, ColorChannel> swap_ga;  // Odd ksel number: green; even: alpha
-  BitField<4, 5, KonstSel> kcsel0;
+  BitField<4, 5, KonstSel> kcsel_even;
-  BitField<9, 5, KonstSel> kasel0;
+  BitField<9, 5, KonstSel> kasel_even;
-  BitField<14, 5, KonstSel> kcsel1;
+  BitField<14, 5, KonstSel> kcsel_odd;
-  BitField<19, 5, KonstSel> kasel1;
+  BitField<19, 5, KonstSel> kasel_odd;
   u32 hex;
-
-  KonstSel getKC(int i) const { return i ? kcsel1.Value() : kcsel0.Value(); }
-  KonstSel getKA(int i) const { return i ? kasel1.Value() : kasel0.Value(); }
 };
 template <>
 struct fmt::formatter<TevKSel>
@@ -1933,8 +1944,36 @@ struct fmt::formatter<TevKSel>
     return fmt::format_to(ctx.out(),
                           "Swap 1: {}\nSwap 2: {}\nColor sel 0: {}\nAlpha sel 0: {}\n"
                           "Color sel 1: {}\nAlpha sel 1: {}",
-                          ksel.swap1, ksel.swap2, ksel.kcsel0, ksel.kasel0, ksel.kcsel1,
+                          ksel.swap_rb, ksel.swap_ga, ksel.kcsel_even, ksel.kasel_even,
-                          ksel.kasel1);
+                          ksel.kcsel_odd, ksel.kasel_odd);
+  }
+};
+
+struct AllTevKSels
+{
+  std::array<TevKSel, 8> ksel;
+
+  KonstSel GetKonstColor(u32 tev_stage) const
+  {
+    const u32 ksel_num = tev_stage >> 1;
+    const bool odd = tev_stage & 1;
+    const auto& cur_ksel = ksel[ksel_num];
+    return odd ? cur_ksel.kcsel_odd.Value() : cur_ksel.kcsel_even.Value();
+  }
+  KonstSel GetKonstAlpha(u32 tev_stage) const
+  {
+    const u32 ksel_num = tev_stage >> 1;
+    const bool odd = tev_stage & 1;
+    const auto& cur_ksel = ksel[ksel_num];
+    return odd ? cur_ksel.kasel_odd.Value() : cur_ksel.kasel_even.Value();
+  }
+  Common::EnumMap<ColorChannel, ColorChannel::Alpha> GetSwapTable(u32 swap_table_id) const
+  {
+    const u32 rg_ksel_num = swap_table_id << 1;
+    const u32 ba_ksel_num = rg_ksel_num + 1;
+    const auto& rg_ksel = ksel[rg_ksel_num];
+    const auto& ba_ksel = ksel[ba_ksel_num];
+    return {rg_ksel.swap_rb, rg_ksel.swap_ga, ba_ksel.swap_rb, ba_ksel.swap_ga};
   }
 };
 
@@ -2415,7 +2454,7 @@ struct BPMemory
   AlphaTest alpha_test;               // 0xf3
   ZTex1 ztex1;                        // 0xf4
   ZTex2 ztex2;                        // 0xf5
-  TevKSel tevksel[8];                 // 0xf6-0xfd
+  AllTevKSels tevksel;                // 0xf6-0xfd
   u32 bpMask;                         // 0xfe
   u32 unknown18;                      // 0xff
 

Source/Core/VideoCommon/PixelShaderGen.cpp

@@ -167,6 +167,8 @@ constexpr Common::EnumMap<const char*, TevOutput::Color2> tev_a_output_table{
     "c2.a",
 };
 
+constexpr Common::EnumMap<char, ColorChannel::Alpha> rgba_swizzle{'r', 'g', 'b', 'a'};
+
 PixelShaderUid GetPixelShaderUid()
 {
   PixelShaderUid out;
@@ -254,22 +256,22 @@ PixelShaderUid GetPixelShaderUid()
         ac.a == TevAlphaArg::RasAlpha || ac.b == TevAlphaArg::RasAlpha ||
         ac.c == TevAlphaArg::RasAlpha || ac.d == TevAlphaArg::RasAlpha)
     {
-      const int i = bpmem.combiners[n].alphaC.rswap;
+      const auto ras_swap_table = bpmem.tevksel.GetSwapTable(bpmem.combiners[n].alphaC.rswap);
-      uid_data->stagehash[n].tevksel_swap1a = bpmem.tevksel[i * 2].swap1;
+      uid_data->stagehash[n].ras_swap_r = ras_swap_table[ColorChannel::Red];
-      uid_data->stagehash[n].tevksel_swap2a = bpmem.tevksel[i * 2].swap2;
+      uid_data->stagehash[n].ras_swap_g = ras_swap_table[ColorChannel::Green];
-      uid_data->stagehash[n].tevksel_swap1b = bpmem.tevksel[i * 2 + 1].swap1;
+      uid_data->stagehash[n].ras_swap_b = ras_swap_table[ColorChannel::Blue];
-      uid_data->stagehash[n].tevksel_swap2b = bpmem.tevksel[i * 2 + 1].swap2;
+      uid_data->stagehash[n].ras_swap_a = ras_swap_table[ColorChannel::Alpha];
       uid_data->stagehash[n].tevorders_colorchan = bpmem.tevorders[n / 2].getColorChan(n & 1);
     }
 
     uid_data->stagehash[n].tevorders_enable = bpmem.tevorders[n / 2].getEnable(n & 1);
     if (uid_data->stagehash[n].tevorders_enable)
     {
-      const int i = bpmem.combiners[n].alphaC.tswap;
+      const auto tex_swap_table = bpmem.tevksel.GetSwapTable(bpmem.combiners[n].alphaC.tswap);
-      uid_data->stagehash[n].tevksel_swap1c = bpmem.tevksel[i * 2].swap1;
+      uid_data->stagehash[n].tex_swap_r = tex_swap_table[ColorChannel::Red];
-      uid_data->stagehash[n].tevksel_swap2c = bpmem.tevksel[i * 2].swap2;
+      uid_data->stagehash[n].tex_swap_g = tex_swap_table[ColorChannel::Green];
-      uid_data->stagehash[n].tevksel_swap1d = bpmem.tevksel[i * 2 + 1].swap1;
+      uid_data->stagehash[n].tex_swap_b = tex_swap_table[ColorChannel::Blue];
-      uid_data->stagehash[n].tevksel_swap2d = bpmem.tevksel[i * 2 + 1].swap2;
+      uid_data->stagehash[n].tex_swap_a = tex_swap_table[ColorChannel::Alpha];
       uid_data->stagehash[n].tevorders_texmap = bpmem.tevorders[n / 2].getTexMap(n & 1);
     }
 
@@ -277,8 +279,8 @@ PixelShaderUid GetPixelShaderUid()
         cc.d == TevColorArg::Konst || ac.a == TevAlphaArg::Konst || ac.b == TevAlphaArg::Konst ||
         ac.c == TevAlphaArg::Konst || ac.d == TevAlphaArg::Konst)
     {
-      uid_data->stagehash[n].tevksel_kc = bpmem.tevksel[n / 2].getKC(n & 1);
+      uid_data->stagehash[n].tevksel_kc = bpmem.tevksel.GetKonstColor(n);
-      uid_data->stagehash[n].tevksel_ka = bpmem.tevksel[n / 2].getKA(n & 1);
+      uid_data->stagehash[n].tevksel_ka = bpmem.tevksel.GetKonstAlpha(n);
     }
   }
 
@@ -1412,30 +1414,18 @@ static void WriteStage(ShaderCode& out, const pixel_shader_uid_data* uid_data, i
       ac.c == TevAlphaArg::RasAlpha || ac.d == TevAlphaArg::RasAlpha)
   {
     // Generate swizzle string to represent the Ras color channel swapping
-    const char rasswap[5] = {
+    out.Write("\trastemp = {}.{}{}{}{};\n", tev_ras_table[stage.tevorders_colorchan],
-        "rgba"[stage.tevksel_swap1a],
+              rgba_swizzle[stage.ras_swap_r], rgba_swizzle[stage.ras_swap_g],
-        "rgba"[stage.tevksel_swap2a],
+              rgba_swizzle[stage.ras_swap_b], rgba_swizzle[stage.ras_swap_a]);
-        "rgba"[stage.tevksel_swap1b],
-        "rgba"[stage.tevksel_swap2b],
-        '\0',
-    };
-
-    out.Write("\trastemp = {}.{};\n", tev_ras_table[stage.tevorders_colorchan], rasswap);
   }
 
   if (stage.tevorders_enable && uid_data->genMode_numtexgens > 0)
   {
     // Generate swizzle string to represent the texture color channel swapping
-    const char texswap[5] = {
+    out.Write("\ttextemp = sampleTextureWrapper({}u, tevcoord.xy, layer).{}{}{}{};\n",
-        "rgba"[stage.tevksel_swap1c],
+              stage.tevorders_texmap, rgba_swizzle[stage.tex_swap_r],
-        "rgba"[stage.tevksel_swap2c],
+              rgba_swizzle[stage.tex_swap_g], rgba_swizzle[stage.tex_swap_b],
-        "rgba"[stage.tevksel_swap1d],
+              rgba_swizzle[stage.tex_swap_a]);
-        "rgba"[stage.tevksel_swap2d],
-        '\0',
-    };
-
-    out.Write("\ttextemp = sampleTextureWrapper({0}u, tevcoord.xy, layer).{1};\n",
-              stage.tevorders_texmap, texswap);
   }
   else if (uid_data->genMode_numtexgens == 0)
   {

Source/Core/VideoCommon/PixelShaderGen.h

@@ -125,7 +125,7 @@ struct pixel_shader_uid_data
   {
     // TODO: Can save a lot space by removing the padding bits
     u32 cc : 24;
-    u32 ac : 24;  // tswap and rswap are left blank (encoded into the tevksel fields below)
+    u32 ac : 24;  // tswap and rswap are left blank (decoded into the swap fields below)
 
     u32 tevorders_texmap : 3;
     u32 tevorders_texcoord : 3;
@@ -133,18 +133,19 @@ struct pixel_shader_uid_data
     RasColorChan tevorders_colorchan : 3;
     u32 pad1 : 7;
 
-    // TODO: Clean up the swapXY mess
+    // TODO: We could save space by storing the 4 swap tables elsewhere and only storing references
+    // to which table is used (the tswap and rswap fields), instead of duplicating them here
     u32 tevind : 21;
-    u32 tevksel_swap1a : 2;
+    ColorChannel ras_swap_r : 2;
-    u32 tevksel_swap2a : 2;
+    ColorChannel ras_swap_g : 2;
-    u32 tevksel_swap1b : 2;
+    ColorChannel ras_swap_b : 2;
-    u32 tevksel_swap2b : 2;
+    ColorChannel ras_swap_a : 2;
     u32 pad2 : 2;
 
-    u32 tevksel_swap1c : 2;
+    ColorChannel tex_swap_r : 2;
-    u32 tevksel_swap2c : 2;
+    ColorChannel tex_swap_g : 2;
-    u32 tevksel_swap1d : 2;
+    ColorChannel tex_swap_b : 2;
-    u32 tevksel_swap2d : 2;
+    ColorChannel tex_swap_a : 2;
     KonstSel tevksel_kc : 5;
     KonstSel tevksel_ka : 5;
     u32 pad3 : 14;

Source/Core/VideoCommon/UberShaderPixel.cpp

@@ -271,12 +271,12 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
             "  // AKA: Color Channel Swapping\n"
             "\n"
             "  int4 ret;\n");
-  out.Write("  ret.r = color[{}];\n", BitfieldExtract<&TevKSel::swap1>("bpmem_tevksel(s * 2u)"));
+  out.Write("  ret.r = color[{}];\n", BitfieldExtract<&TevKSel::swap_rb>("bpmem_tevksel(s * 2u)"));
-  out.Write("  ret.g = color[{}];\n", BitfieldExtract<&TevKSel::swap2>("bpmem_tevksel(s * 2u)"));
+  out.Write("  ret.g = color[{}];\n", BitfieldExtract<&TevKSel::swap_ga>("bpmem_tevksel(s * 2u)"));
   out.Write("  ret.b = color[{}];\n",
-            BitfieldExtract<&TevKSel::swap1>("bpmem_tevksel(s * 2u + 1u)"));
+            BitfieldExtract<&TevKSel::swap_rb>("bpmem_tevksel(s * 2u + 1u)"));
   out.Write("  ret.a = color[{}];\n",
-            BitfieldExtract<&TevKSel::swap2>("bpmem_tevksel(s * 2u + 1u)"));
+            BitfieldExtract<&TevKSel::swap_ga>("bpmem_tevksel(s * 2u + 1u)"));
   out.Write("  return ret;\n"
             "}}\n\n");
 
@@ -1240,12 +1240,12 @@ ShaderCode GenPixelShader(APIType api_type, const ShaderHostConfig& host_config,
             "  uint tevksel = bpmem_tevksel(ss.stage>>1);\n"
             "  if ((ss.stage & 1u) == 0u)\n"
             "    return int4(konstLookup[{}].rgb, konstLookup[{}].a);\n",
-            BitfieldExtract<&TevKSel::kcsel0>("tevksel"),
+            BitfieldExtract<&TevKSel::kcsel_even>("tevksel"),
-            BitfieldExtract<&TevKSel::kasel0>("tevksel"));
+            BitfieldExtract<&TevKSel::kasel_even>("tevksel"));
   out.Write("  else\n"
             "    return int4(konstLookup[{}].rgb, konstLookup[{}].a);\n",
-            BitfieldExtract<&TevKSel::kcsel1>("tevksel"),
+            BitfieldExtract<&TevKSel::kcsel_odd>("tevksel"),
-            BitfieldExtract<&TevKSel::kasel1>("tevksel"));
+            BitfieldExtract<&TevKSel::kasel_odd>("tevksel"));
   out.Write("}}\n");
 
   return out;