Fix the rotation direction of the normals

[BlackYps]

WIP

Checklist

• Changes are annotated, including comments where useful
• Changes are documented in a changelog snippet according to the guidelines.
• Request 2-3 reviewers from the list of reviewers and their areas of knowledge.

Summary by CodeRabbit

• Bug Fixes
  • Fixed terrain normal vector rotation calculations to ensure correct lighting and visual appearance on rotated terrain surfaces. The correction updates how surface normals are oriented during rotation operations across multiple terrain layers, resolving visual artifacts and ensuring proper terrain appearance rendering.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change reverses the order of vector-matrix multiplication for rotating 2D normal components in two terrain pixel shaders (Terrain200NormalsPS and Terrain200BNormalsPS). The multiplication order is changed from mul(vector, matrix) to mul(matrix, vector) for normals across multiple strata, affecting how rotations are applied to normal vectors.

Changes

Cohort / File(s)	Summary
Terrain Normal Rotation effects/terrain.fx	Reverses vector-matrix multiplication order in Terrain200NormalsPS and Terrain200BNormalsPS for stratum 0, 2, 4, and 6 normal rotations. Changes mul(stratumXNormal.xy, rotationMatrix) to mul(rotationMatrix, stratumXNormal.xy) to alter how normal vectors are rotated during rendering.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A twist of matrices, a spin anew,
Normals rotate in a different view,
Left times right becomes right times left,
Terrain shaders dance—of rotation bereft,
Precision in pixels, a mathematical plight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is largely incomplete, containing only 'WIP' with no actual content describing the changes, testing performed, or context about the fix.	Add a detailed description of the changes, explain what rotation issue was fixed and why the vector-matrix order reversal solves it, document testing performed, and complete the checklist items before merging.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix the rotation direction of the normals' directly reflects the main change, which reverses vector-matrix multiplication order for normal rotations in shader code.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch shader-texture-fix

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

effects/terrain.fx (1)

1841-1847: sampleNormal helper has the same missing inverse rotation on normalRotated — not currently active.

normalRotated is sampled at mul(position, rotationMatrix) (clockwise UV rotation) but its XY components are not un-rotated before being passed to splatBlendNormal. This means the rotated sample contributes incorrectly-oriented normals to the blend. This does not affect any currently rendered output because the only callers (TerrainPBRNormalsPS / technique TerrainPBR) are fully commented out, but the bug would reappear if those techniques were re-enabled.

🛠️ Proposed fix for when this code is un-commented

 float3 sampleNormal(sampler2D s, float2 position, uniform float2 scale, float mask) {
     // 30° rotation
     float2x2 rotationMatrix = float2x2(float2(0.866, -0.5), float2(0.5, 0.866));
     float3 normal = tex2D(s, position * scale).rgb;
     float3 normalRotated = tex2D(s, mul(position, rotationMatrix) * scale).rgb;
-    return splatBlendNormal(normalize(2 * normal - 1), normalize(2 * normalRotated - 1), 0.5, mask, 0.03);
+    float3 normalRotatedDecoded = normalize(2 * normalRotated - 1);
+    normalRotatedDecoded.xy = mul(rotationMatrix, normalRotatedDecoded.xy);
+    return splatBlendNormal(normalize(2 * normal - 1), normalRotatedDecoded, 0.5, mask, 0.03);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@effects/terrain.fx` around lines 1841 - 1847, The sampled rotated normal
(normalRotated) is not being un-rotated before blending, so compute the normal
vector from that sample (e.g., vec3 nRot = normalize(2 * normalRotated - 1)),
apply the inverse rotation to its XY components (since rotationMatrix is a pure
rotation the inverse is its transpose, e.g., float2x2 invRot =
transpose(rotationMatrix); nRot.xy = mul(nRot.xy, invRot)), and then pass the
corrected nRot into splatBlendNormal instead of the raw rotated sample; keep the
other call parameters (0.5, mask, 0.03) and the original normal handling
unchanged so the function (sampleNormal) uses properly un-rotated normals for
splatBlendNormal.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@effects/terrain.fx`:
- Around line 1841-1847: The sampled rotated normal (normalRotated) is not being
un-rotated before blending, so compute the normal vector from that sample (e.g.,
vec3 nRot = normalize(2 * normalRotated - 1)), apply the inverse rotation to its
XY components (since rotationMatrix is a pure rotation the inverse is its
transpose, e.g., float2x2 invRot = transpose(rotationMatrix); nRot.xy =
mul(nRot.xy, invRot)), and then pass the corrected nRot into splatBlendNormal
instead of the raw rotated sample; keep the other call parameters (0.5, mask,
0.03) and the original normal handling unchanged so the function (sampleNormal)
uses properly un-rotated normals for splatBlendNormal.