Merge pull request #9651 from Pokechu22/oob-texcoord

Fix out of bounds tex coord behavior; always apply fb_addprev and tex coord wrapping

1 files changed, 40 insertions, 18 deletions

diff --git a/Source/Core/VideoBackends/Software/Tev.cpp b/Source/Core/VideoBackends/Software/Tev.cpp
index 65227339e4..be6f14d610 100644
--- a/Source/Core/VideoBackends/Software/Tev.cpp
+++ b/Source/Core/VideoBackends/Software/Tev.cpp
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <cmath>
+#include <cstring>
 
 #include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
@@ -485,20 +486,16 @@ void Tev::Indirect(unsigned int stageNum, s32 s, s32 t)
 
   // matrix multiply - results might overflow, but we don't care since we only use the lower 24 bits
   // of the result.
-  const int indmtxid = indirect.mid & 3;
-  if (indmtxid)
+  if (indirect.matrix_index != IndMtxIndex::Off)
   {
-    const IND_MTX& indmtx = bpmem.indmtx[indmtxid - 1];
-    const int scale =
-        ((u32)indmtx.col0.s0 << 0) | ((u32)indmtx.col1.s1 << 2) | ((u32)indmtx.col2.s2 << 4);
+    const IND_MTX& indmtx = bpmem.indmtx[static_cast<u32>(indirect.matrix_index.Value()) - 1];
 
-    int shift;
+    const int shift = 17 - indmtx.GetScale();
 
-    switch (indirect.mid & 12)
+    switch (indirect.matrix_id)
     {
-    case 0:
+    case IndMtxId::Indirect:
       // matrix values are S0.10, output format is S17.7, so divide by 8
-      shift = (17 - scale);
       indtevtrans[0] = (indmtx.col0.ma * indcoord[0] + indmtx.col1.mc * indcoord[1] +
                         indmtx.col2.me * indcoord[2]) >>
                        3;
@@ -506,25 +503,29 @@ void Tev::Indirect(unsigned int stageNum, s32 s, s32 t)
                         indmtx.col2.mf * indcoord[2]) >>
                        3;
       break;
-    case 4:  // s matrix
+    case IndMtxId::S:
       // s is S17.7, matrix elements are divided by 256, output is S17.7, so divide by 256. - TODO:
       // Maybe, since s is actually stored as S24, we should divide by 256*64?
-      shift = (17 - scale);
       indtevtrans[0] = s * indcoord[0] / 256;
       indtevtrans[1] = t * indcoord[0] / 256;
       break;
-    case 8:  // t matrix
-      shift = (17 - scale);
+    case IndMtxId::T:
       indtevtrans[0] = s * indcoord[1] / 256;
       indtevtrans[1] = t * indcoord[1] / 256;
       break;
     default:
+      PanicAlertFmt("Invalid indirect matrix ID {}", indirect.matrix_id);
       return;
     }
 
     indtevtrans[0] = shift >= 0 ? indtevtrans[0] >> shift : indtevtrans[0] << -shift;
     indtevtrans[1] = shift >= 0 ? indtevtrans[1] >> shift : indtevtrans[1] << -shift;
   }
+  else
+  {
+    // If matrix_index is Off (0), matrix_id should be Indirect (0)
+    ASSERT(indirect.matrix_id == IndMtxId::Indirect);
+  }
 
   if (indirect.fb_addprev)
   {
@@ -559,9 +560,16 @@ void Tev::Draw()
     const int stageNum2 = stageNum >> 1;
     const int stageOdd = stageNum & 1;
 
-    const u32 texcoordSel = bpmem.tevindref.getTexCoord(stageNum);
+    u32 texcoordSel = bpmem.tevindref.getTexCoord(stageNum);
     const u32 texmap = bpmem.tevindref.getTexMap(stageNum);
 
+    // Quirk: when the tex coord is not less than the number of tex gens (i.e. the tex coord does
+    // not exist), then tex coord 0 is used (though sometimes glitchy effects happen on console).
+    // This affects the Mario portrait in Luigi's Mansion, where the developers forgot to set
+    // the number of tex gens to 2 (bug 11462).
+    if (texcoordSel >= bpmem.genMode.numtexgens)
+      texcoordSel = 0;
+
     const TEXSCALE& texscale = bpmem.texscale[stageNum2];
     const s32 scaleS = stageOdd ? texscale.ss1 : texscale.ss0;
     const s32 scaleT = stageOdd ? texscale.ts1 : texscale.ts0;
@@ -592,8 +600,13 @@ void Tev::Draw()
     const TevStageCombiner::ColorCombiner& cc = bpmem.combiners[stageNum].colorC;
     const TevStageCombiner::AlphaCombiner& ac = bpmem.combiners[stageNum].alphaC;
 
-    const int texcoordSel = order.getTexCoord(stageOdd);
-    const int texmap = order.getTexMap(stageOdd);
+    u32 texcoordSel = order.getTexCoord(stageOdd);
+    const u32 texmap = order.getTexMap(stageOdd);
+
+    // Quirk: when the tex coord is not less than the number of tex gens (i.e. the tex coord does
+    // not exist), then tex coord 0 is used (though sometimes glitchy effects happen on console).
+    if (texcoordSel >= bpmem.genMode.numtexgens)
+      texcoordSel = 0;
 
     Indirect(stageNum, Uv[texcoordSel].s, Uv[texcoordSel].t);
 
@@ -603,8 +616,17 @@ void Tev::Draw()
       // RGBA
       u8 texel[4];
 
-      TextureSampler::Sample(TexCoord.s, TexCoord.t, TextureLod[stageNum], TextureLinear[stageNum],
-                             texmap, texel);
+      if (bpmem.genMode.numtexgens > 0)
+      {
+        TextureSampler::Sample(TexCoord.s, TexCoord.t, TextureLod[stageNum],
+                               TextureLinear[stageNum], texmap, texel);
+      }
+      else
+      {
+        // It seems like the result is always black when no tex coords are enabled, but further
+        // hardware testing is needed.
+        std::memset(texel, 0, 4);
+      }
 
 #if ALLOW_TEV_DUMPS
       if (g_ActiveConfig.bDumpTevTextureFetches)