[graphics] Fix issue with envmap math (#3862)

A few people in the discord noticed that the shiny part of the subrails looks different between PS2 and OpenGOAL. I looked into this and found a bug where the length of the normals was half what they should have been. Earlier this didn't matter because I normalized them in the shader, but at some point I removed that when refactoring other parts. I'm not completely convinced we have the intensity/tint multiplier working correctly, but this is at least a step in the right direction. https://github.com/user-attachments/assets/ee754011-08ac-4bee-94b2-d0e114b952ec https://github.com/user-attachments/assets/1079635a-0b92-4965-a991-972ef6048ddf For fun, here's this fix, plus a random doubling of the envmap effect strength: https://github.com/user-attachments/assets/726e7b8d-c020-42cf-a594-cad590c0ef7d --------- Co-authored-by: water111 <[email protected]>
open-goal · Feb 16, 2025 · af9a8dd · af9a8dd
1 parent 2a4d3d7
commit af9a8dd
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 13 deletions.
diff --git a/common/custom_data/TFrag3Data.cpp b/common/custom_data/TFrag3Data.cpp
@@ -93,10 +93,8 @@ math::Vector3f vopmsub(math::Vector3f acc, math::Vector3f a, math::Vector3f b) {
 }
 
 /*!
- * Compute the normal transformation for a TIE from the TIE matrix.
- * Note that this isn't identical to the original game - we're missing the vf14 scaling factor
- * For now, I just set this to 1, then normalize in the shader. Though I think we could avoid
- * this by figuring out the value of vf14 here (I am just too lazy right now).
+ * Compute the normal transformation for a TIE from the TIE matrix. This will return properly scaled
+ * normals.
  */
 std::array<math::Vector3f, 3> tie_normal_transform_v2(const std::array<math::Vector4f, 4>& m) {
   // let:
@@ -186,11 +184,21 @@ std::array<math::Vector3f, 3> tie_normal_transform_v2(const std::array<math::Vec
   // sqc2 vf12, -80(t8)
 }
 
+s16 saturate_for_s10(s16 s10) {
+  // our error should be 1 or less as an s8, or 4 as a s10.
+  ASSERT(s10 >= -520 && s10 < 520);
+  if (s10 < -512) {
+    return -512;
+  }
+  if (s10 > 511) {
+    return 511;
+  }
+  return s10;
+}
+
 u32 pack_to_gl_normal(s16 nx, s16 ny, s16 nz) {
-  ASSERT(nx >= -512 && nx <= 511);
-  ASSERT(ny >= -512 && ny <= 511);
-  ASSERT(nz >= -512 && nz <= 511);
-  return (nx & 0x3ff) | ((ny & 0x3ff) << 10) | ((nz & 0x3ff) << 20);
+  return (saturate_for_s10(nx) & 0x3ff) | ((saturate_for_s10(ny) & 0x3ff) << 10) |
+         ((saturate_for_s10(nz) & 0x3ff) << 20);
 }
 
 /*!
@@ -202,10 +210,10 @@ u32 unpack_tie_normal(const std::array<math::Vector3f, 3>& mat, s8 nx, s8 ny, s8
   nrm += mat[0] * nx;
   nrm += mat[1] * ny;
   nrm += mat[2] * nz;
-  // convert to s16 for OpenGL renderer
-  // nrm /= 0x100;  // number from EE asm
-  // nrm *= 0x200;  // for normalized s10 -> float conversion by OpenGL.
-  nrm *= 2;  // for normalized s10 -> float conversion by OpenGL.
+
+  // game used signed 8-bit normals, but OpenGL uses signed 10-bit
+  // multiply by 2^2 = 4
+  nrm *= 4;
 
   auto as_int = nrm.cast<s16>();
 

diff --git a/game/graphics/opengl_renderer/shaders/etie.vert b/game/graphics/opengl_renderer/shaders/etie.vert
@@ -31,7 +31,6 @@ void main() {
   vec3 nrm_vf23 = cam_no_persp[0].xyz * normal.x
                 + cam_no_persp[1].xyz * normal.y
                 + cam_no_persp[2].xyz * normal.z;
-  vec3 r_nrm = nrm_vf23;
 
   // transform the point
   vec4 vf17 = cam_no_persp[3];