chore(emat): auto detect terrain parallax

[doodlum]

Summary by CodeRabbit

• New Features

  • Adaptive parallax rendering now chooses between modern and legacy parallax paths for materials, terrain and shadows, improving depth and consistency across scenes.
  • Improved shadow sampling and mip/co‑ordinate selection reduce artifacts and stabilize detail at varying distances.

• UI

  • Checkbox renamed to "Enable Legacy Terrain" for clearer terrain mode selection.

[coderabbitai]

Walkthrough

Adds a TRUE_PBR-gated branching in shaders to select between EnableParallax and legacy terrain-parallax (with THLandHasDisplacement fallback), introduces a global ExtraFeatureFlags bit for terrain displacement, toggles that bit from TerrainHelper, and renames one UI checkbox to "Enable Legacy Terrain".

Changes

Cohort / File(s)	Summary
Shader parallax path selection package/Shaders/Lighting.hlsl	Wraps terrain-parallax logic with #if TRUE_PBR branches so mip selection, parallax coordinate computation, and parallax shadow sampling use EnableParallax when TRUE_PBR is defined; otherwise they fall back to EnableTerrainParallax with THLandHasDisplacement-aware paths where applicable.
Permutation flags package/Shaders/Common/Permutation.hlsli	Adds THLandHasDisplacement = (1 << 6) to Permutation::ExtraFeatureFlags.
Terrain feature wiring src/Features/TerrainHelper.cpp	Clears new ExtraFeatureDescriptor bit 6 at start; when any per-texture parallax is present, sets the corresponding per-texture bits (0–5) and sets bit 6 to indicate overall terrain displacement.
UI label change src/Features/ExtendedMaterials.cpp	Changes checkbox label from "Enable Terrain" to "Enable Legacy Terrain"; behavior and DataLoaded() invocation unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant TH as TerrainHelper (C++)
  participant PF as Permutation Flags
  participant FS as Fragment Shader (Lighting.hlsl)

  rect rgba(245,250,240,0.6)
  TH->>PF: Clear ExtraFeatureFlags bit6
  note right of TH: iterate textures
  alt any per-texture parallax found
    TH->>PF: Set per-texture bits (0..5) and set bit6 (THLandHasDisplacement)
  else none found
    TH-->>PF: leave bit6 cleared
  end
  end

  rect rgba(240,245,255,0.6)
  FS->>PF: Read ExtraFeatureFlags + TRUE_PBR macro
  alt TRUE_PBR defined
    FS->>FS: Use EnableParallax path (mip, coords, shadow sampling)
  else TRUE_PBR not defined
    alt THLandHasDisplacement set
      FS->>FS: Use terrain-with-displacement parallax path
    else
      FS->>FS: Use legacy EnableTerrainParallax path
    end
  end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix(terrain helper): Fix for nullptr lookup and running without plugin #1176 — modifies TerrainHelper::BSLightingShader_SetupMaterial and ExtraFeatureDescriptor handling; closely related to the terrain-flag wiring changes.

Suggested reviewers

• alandtse

Poem

hop, hop — a flag set in the night,
two shader paths choose left or right.
I nibble bits and toggle light,
parallax dances — rabbit's delight. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.
Title Check	✅ Passed	The title succinctly and accurately summarizes the core change of the pull request—updating EMAT to automatically detect terrain parallax—using clear, concise phrasing without extraneous detail. It follows conventional commit style, is specific to the primary modification, and would be easily understood by reviewers scanning the commit history.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pbr-terrain-check

---

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.}

[github-actions]

Using provided base ref: 5fc0ef9
Using base ref: 5fc0ef9
Base commit date: 2025-10-05T23:02:41+01:00 (Sunday, October 05, 2025 11:02 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5fc0ef9 and 5324ec2.

📒 Files selected for processing (1)

• package/Shaders/Lighting.hlsl (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

• package/Shaders/Lighting.hlsl

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
• GitHub Check: Build plugin and addons
• GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)

🔇 Additional comments (2)

package/Shaders/Lighting.hlsl (2)

1234-1238: LGTM! Conditional parallax check aligns PBR terrain with PBR materials.

The conditional branching correctly differentiates terrain parallax handling: PBR terrain now checks EnableParallax (consistent with non-terrain PBR at line 1146), while non-PBR terrain continues using EnableTerrainParallax. This unifies parallax control for PBR materials.

---

2346-2350: LGTM! Parallax shadow logic mirrors the mip level computation pattern.

The conditional branching for shadow calculations correctly mirrors the pattern introduced at lines 1234-1238, ensuring PBR terrain uses EnableParallax while non-PBR terrain uses EnableTerrainParallax.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

package/Shaders/Lighting.hlsl (1)

2608-2612: Fix inconsistent spacing before opening brace (duplicate of line 2347 issue).

Lines 2609 and 2611 have the same spacing issue as line 2347. Please apply consistent spacing before all opening braces.

Apply this diff to fix the spacing:

 #					if defined(TRUE_PBR)
-					if (SharedData::extendedMaterialSettings.EnableParallax)
+					if (SharedData::extendedMaterialSettings.EnableParallax) {
 #					else
-					if (SharedData::extendedMaterialSettings.EnableTerrainParallax || (SharedData::extendedMaterialSettings.EnableParallax && Permutation::ExtraFeatureDescriptor & Permutation::ExtraFeatureFlags::THLandHasDisplacement))
+					if (SharedData::extendedMaterialSettings.EnableTerrainParallax || (SharedData::extendedMaterialSettings.EnableParallax && Permutation::ExtraFeatureDescriptor & Permutation::ExtraFeatureFlags::THLandHasDisplacement)) {
 #					endif

🧹 Nitpick comments (2)

package/Shaders/Lighting.hlsl (2)

2346-2350: Fix inconsistent spacing before opening brace.

Line 2347 has EnableParallax){ with no space before the opening brace, while the surrounding code uses consistent spacing. This minor style inconsistency should be fixed for uniformity.

Apply this diff to fix the spacing:

 #			if defined(TRUE_PBR)
-	if (SharedData::extendedMaterialSettings.EnableParallax){
+	if (SharedData::extendedMaterialSettings.EnableParallax) {
 #			else
-	if (SharedData::extendedMaterialSettings.EnableTerrainParallax || (SharedData::extendedMaterialSettings.EnableParallax && Permutation::ExtraFeatureDescriptor & Permutation::ExtraFeatureFlags::THLandHasDisplacement)){
+	if (SharedData::extendedMaterialSettings.EnableTerrainParallax || (SharedData::extendedMaterialSettings.EnableParallax && Permutation::ExtraFeatureDescriptor & Permutation::ExtraFeatureFlags::THLandHasDisplacement)) {
 #			endif

---

1234-1238: Consider adding explanatory comments for the conditional logic.

The fallback condition EnableTerrainParallax || (EnableParallax && THLandHasDisplacement) is repeated three times but lacks explanation. Adding a brief comment at the first occurrence would improve code clarity and maintainability.

For example:

 #		if defined(EMAT)
 #			if defined(TRUE_PBR)
 	if (SharedData::extendedMaterialSettings.EnableParallax) {
 #			else
+	// Legacy terrain parallax: use EnableTerrainParallax or EnableParallax with displacement flag
 	if (SharedData::extendedMaterialSettings.EnableTerrainParallax || (SharedData::extendedMaterialSettings.EnableParallax && Permutation::ExtraFeatureDescriptor & Permutation::ExtraFeatureFlags::THLandHasDisplacement)) {
 #			endif

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f942ec and 6bf53fe.

📒 Files selected for processing (1)

• package/Shaders/Lighting.hlsl (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

• package/Shaders/Lighting.hlsl

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
• GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
• GitHub Check: Build plugin and addons

🔇 Additional comments (1)

package/Shaders/Lighting.hlsl (1)

1234-1238: LGTM! Parallax logic correctly branches by TRUE_PBR.

The conditional structure properly separates the PBR parallax path from the legacy terrain parallax path with THLandHasDisplacement fallback. The operator precedence is correct (& binds tighter than &&), and the formatting is consistent.

[github-actions]

✅ A pre-release build is available for this PR:
Download