fix(hair): correct hair indirect normal, marschner by default

[jiayev]

This pull request updates the hair rendering pipeline to improve realism and consistency, particularly when using the Marschner hair shading model. The main changes focus on correcting normal calculations for ambient and indirect lighting, refining the Marschner model's integration, and adjusting default parameters for better visual results.

Hair shading model improvements:

• Changed the default HairMode to Marschner (1) and increased BaseColorMult to 1.5 in HairSpecular settings for enhanced color vibrancy and realism.
• Corrected the tangent calculation for the Marschner model in Hair.hlsli, ensuring the light direction (L) is based on the tangent vector (T) rather than the normal (N). This affects both specular and diffuse lobe calculations for more accurate hair highlights.

Lighting and ambient integration fixes:

• Updated all ambient, IBL, and skylighting calculations in Lighting.hlsl to use the tangent-based normal (ambientNormal) when Marschner mode is active, resulting in more physically correct ambient and indirect lighting for hair. [1] [2]
• Disabled dynamic cubemap specular lighting for Marschner mode to avoid double-counting specular highlights, ensuring indirect specular is only added in non-Marschner modes.

Summary by CodeRabbit

• New Features
  • Improved hair lighting for more natural highlights and transmission.
  • Ambient lighting (skylight/IBL) now adapts to hair direction for more accurate shading.
  • Refined luminance handling for base color to avoid extremely low luma.
  • Updated indirect/specular blending for hair to reduce artifacts and improve consistency.
  • Default hair model switched to Marschner for higher realism.
  • Increased default base color intensity (1.5x) for richer appearance.

[coderabbitai]

Walkthrough

Updates hair shading and ambient lighting: constrains hair luma, reassigns a Marschner term to transmission in Hair.hlsli; revises HairMode 1 L-vector and indirect lobe weight assembly; introduces hair-aware ambientNormal in Lighting.hlsl for ambient/IBL/skylight paths; changes HairSpecular defaults (BaseColorMult, HairMode).

Changes

Cohort / File(s)	Summary
Hair shading logic (Marschner / Scheuermann / HairMode 1) features/.../Shaders/Hair/Hair.hlsli	Constrain luminance (max(RGBToLuminance(...), 1e-4)); in Scheuermann direct path add Marschner contribution to dirTransmission instead of dirSpecular; for HairMode==1 recompute L after tangent shift using T (normalize(V - T*dot(V,T))), reorder weight computation so diffuseLobeWeight starts from D_Marschner(...) and includes Kajiya‑Kay attenuation, and remove prior specularLobeWeightPrimary assignment.
Ambient / IBL normal handling package/Shaders/Lighting.hlsl	Add ambientNormal with hair-aware computation when HAIR/CS_HAIR and HairMode==1; replace worldNormal with ambientNormal for directional ambient, static/diffuse IBL, and skylighting; narrow hair dynamic cubemap specular branch to require hairSpecularSettings.Enabled && HairMode != 1.
Default hair specular settings src/Features/HairSpecular.h	Update Settings defaults: BaseColorMult 1.0f → 1.5f; HairMode 0 → 1.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant R as Renderer
  participant L as Lighting.hlsl
  participant H as Hair.hlsli

  R->>L: Evaluate lighting (viewDir, worldNormal, hair params)
  alt HAIR & HairMode==1
    L->>L: ambientNormal = normalize(viewDir - hairT * dot(viewDir,hairT))
  else
    L->>L: ambientNormal = worldNormal
  end
  L->>L: Use ambientNormal for ambient, IBL, skylight

  rect rgb(245,248,252)
    note over L,H: Hair indirect/direct evaluation
    L->>H: Request hair lobe weights & direct terms
    alt HairMode==1
      H->>H: If tangent-shift: adjust T
      H->>H: L = normalize(V - T * dot(V,T))
      H->>H: diffuseLobeWeight += D_Marschner(...) + Attenuation_KajiyaKay(...)
      H->>H: Accumulate Marschner direct -> dirTransmission
    else
      H->>H: Existing Kajiya‑Kay / other paths
    end
  end
  H-->>L: Return lobe weights, dirTransmission/dirSpecular
  L-->>R: Compose final shaded color

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• build(hlsl): fix warnings #1370 — Edits to the same GetHairDirectLightScheuermann area; overlaps with reassignment of Marschner contributions.
• fix(hair): remove inner gamma conversion #1338 — Changes hair specular weight/specularPath logic in Hair.hlsli; related to HairMode and lobe-weight adjustments.
• fix(hair): hair marschner indirect diffuse inconsistency #1284 — Adjusts HairMode==1 indirect lobe weighting and diffuse/specular handling; overlaps with L-vector and weight-reorder changes.

Suggested reviewers

• alandtse
• Pentalimbed

Poem

hop hop I nudge a photon bright,
tangents twist and strands find light. 🌤️
Marschner drifts to a transmission tune,
IBL hugs normals under the moon.
base color blooms — one point five — hoon! 🐇

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.
Title Check	✅ Passed	The title concisely and accurately captures the main changes: correcting the hair indirect normal computation and making Marschner the default hair model. These two points map directly to edits in Hair.hlsli, Lighting.hlsl, and HairSpecular.h, so the title is fully related and specific to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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: 7d9c28a
Using base ref: 7d9c28a
Base commit date: 2025-09-23T03:08:52+01:00 (Tuesday, September 23, 2025 03:08 AM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

features/Hair Specular/Shaders/Hair/Hair.hlsli (1)

239-243: Tangent-aligned L for Marschner indirect is correct.

This fixes the prior normal-aligned artifact.

Optionally cap the combined diffuse lobe weight to prevent >1 accumulation in extreme cases:

-            diffuseLobeWeight += GetHairDiffuseAttenuationKajiyaKay(T, V, L, 1, baseColor) * Math::PI;
+            diffuseLobeWeight += GetHairDiffuseAttenuationKajiyaKay(T, V, L, 1, baseColor) * Math::PI;
+            diffuseLobeWeight = saturate(diffuseLobeWeight);

📜 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 7d9c28a and cfddaa5.

📒 Files selected for processing (3)

• features/Hair Specular/Shaders/Hair/Hair.hlsli (3 hunks)
• package/Shaders/Lighting.hlsl (3 hunks)
• src/Features/HairSpecular.h (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{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:

• src/Features/HairSpecular.h
• package/Shaders/Lighting.hlsl
• features/Hair Specular/Shaders/Hair/Hair.hlsli

src/**/*.{cpp,cxx,cc,h,hpp,hxx}

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

src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code

Files:

• src/Features/HairSpecular.h

features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}

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

features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}: Place all feature shaders under features/YourFeature/Shaders/
Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Files:

• features/Hair Specular/Shaders/Hair/Hair.hlsli

🧠 Learnings (2)

📓 Common learnings

Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

📚 Learning: 2025-08-03T18:37:19.690Z

Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

Applied to 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: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
• GitHub Check: Build plugin and addons

🔇 Additional comments (5)

src/Features/HairSpecular.h (2)

49-49: Default to Marschner mode looks good.

Aligns with the shader changes and avoids specular double-counting paths.

---

43-43: Verify color headroom with BaseColorMult = 1.5.

With Marschner squaring baseColor in Lighting.hlsl, 1.5 can push bright tints toward clipping. Please sanity-check HDR headroom and UI ranges.

features/Hair Specular/Shaders/Hair/Hair.hlsli (1)

210-214: Confirm energy accounting after moving Marschner direct term to transmission.

Reclassifying D_Marschner(...) into dirTransmission plus disabling indirect spec in Marschner mode should avoid double-counting; please confirm scene energy balance didn’t change unintentionally (e.g., bright backlit hair).

package/Shaders/Lighting.hlsl (2)

2709-2716: Ambient/IBL/skylight on hair now use a hair-aware normal — good change.

The ambientNormal path with tangent-based projection for Marschner aligns ambient and indirect with the hair fiber direction.

Also applies to: 2718-2724, 2731-2734

---

2989-2992: Dynamic cubemap spec disabled for Marschner.

Prevents indirect specular double-counting; matches the transmission reclassification.

[github-actions]

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

features/Hair Specular/Shaders/Hair/Hair.hlsli (1)

121-123: Bug: V projection uses N·L instead of N·V in Marschner basis.

Vp should be V with its N component removed using NdotV, not NdotL. This miscomputes φ and downstream lobes.

Apply this diff:

-        const float3 Lp = L - NdotL * N;
-        const float3 Vp = V - NdotL * N;
+        const float3 Lp = L - NdotL * N;
+        const float3 Vp = V - NdotV * N;

🧹 Nitpick comments (1)

features/Hair Specular/Shaders/Hair/Hair.hlsli (1)

239-239: Guard normalization against zero-length when V ∥ T (prevents NaNs).

Grazing along-tangent views can make V - T*dot(V,T) ≈ 0. Normalize safely.

Apply this diff:

-            float3 L = normalize(V - T * dot(V, T));
+            float3 Lt = V - T * dot(V, T);
+            float invLen = rsqrt(dot(Lt, Lt) + EPSILON_DIVISION);
+            float3 L = Lt * invLen;

📜 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 cfddaa5 and e8ae628.

📒 Files selected for processing (1)

• features/Hair Specular/Shaders/Hair/Hair.hlsli (3 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{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:

• features/Hair Specular/Shaders/Hair/Hair.hlsli

features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}

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

features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}: Place all feature shaders under features/YourFeature/Shaders/
Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)

Files:

• features/Hair Specular/Shaders/Hair/Hair.hlsli

🧠 Learnings (1)

📓 Common learnings

Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.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: Build plugin and addons
• GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
• GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)

🔇 Additional comments (3)

features/Hair Specular/Shaders/Hair/Hair.hlsli (3)

188-190: Clamped luma avoids divide-by-zero — good fix.

This resolves the prior instability in scatterTint for near-black hair.

---

241-243: Diffuse lobe assembly for Marschner mode looks consistent.

Using T-based L for D_Marschner and adding Kajiya–Kay attenuation into diffuse weights aligns with the intended indirect model.

---

212-214: dirTransmission accumulation and dynamic cubemap gating verified
dirTransmission is only added once via the transmissionColor accumulator and then applied to color.xyz (no double-counting), and the dynamic cubemap specular call is correctly gated off when SharedData::hairSpecularSettings.HairMode == 1.