fix: vertex color applications

[soda3000]

closes issue #2155

Summary by CodeRabbit

• Bug Fixes
  • Improved vertex color tinting in PBR materials for consistent results across sRGB vs linear lighting modes.
  • Enhanced subsurface color tinting so vertex color interacts correctly under both linear and non-linear lighting.
  • Corrected emissive/glow composition to apply vertex colors properly in all PBR variants (including TRUE_PBR vs non-TRUE_PBR paths).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 85231a9d-4228-491b-b181-3bf25022e9e4

📥 Commits

Reviewing files that changed from the base of the PR and between 3be307e and bf9a9fa.

📒 Files selected for processing (1)

• package/Shaders/Lighting.hlsl

🚧 Files skipped from review as they are similar to previous changes (1)

• package/Shaders/Lighting.hlsl

---

📝 Walkthrough

Walkthrough

This change updates PBR vertex-color application in package/Shaders/Lighting.hlsl to apply vertex tinting, subsurface tinting, and emissive/glow composition with explicit sRGB↔linear conversions, branching behavior based on enableLinearLighting and handling the TRUE_PBR path separately.

Changes

Cohort / File(s)	Summary
PBR Color Space Handling package/Shaders/Lighting.hlsl	Replaced direct vertex tint multiplication with pbrVertexColor = Color::SrgbToLinear(input.Color.xyz) and added conditional sRGB↔linear conversions for: baseColor/metallic F0 vertex tinting, subsurface color tinting, and emissive/glow composition. Separate handling for enableLinearLighting enabled vs disabled and for the TRUE_PBR path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(truepbr): apply vertex color to base color for pbr metals #2060: Modifies package/Shaders/Lighting.hlsl to apply vertex color tinting to PBR baseColor and adjust emissive/TRUE_PBR handling (strong code-level overlap).

Suggested reviewers

• doodlum
• alandtse

Poem

🐰🎨 I hop through pixels, quick and merry,
From sRGB burrows to linear prairie,
I tint the base, subsurface, and glow,
So lighting shifts look soft and true—hip-hip-ho! 🥕✨

🚥 Pre-merge checks | ✅ 3

✅ 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 'fix: vertex color applications' directly matches the primary changes in the PR, which refactor vertex color tinting logic across PBR base color, subsurface color, and emissive/glow interactions.
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

---

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]

No actionable suggestions for changed features.

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

package/Shaders/Lighting.hlsl (1)

2797-2821: ⚠️ Potential issue | 🔴 Critical

Vertex color is applied twice for non-PBR Glowmap materials.

In the dev branch, emitColor *= input.Color.xyz was gated by #if defined(TRUE_PBR) with the comment: "Non-PBR folds emitColor into diffuseColor and the global color.xyz *= vertexColor (line 2918) already covers it." The PR removes this guard, applying vertex color to emissive unconditionally.

For non-PBR non-HAIR non-SKYLIGHTING:

• Emissive now gets multiplied by Color::SrgbToLinear(input.Color.xyz) in this block.
• Emissive is then added to diffuseColor (line 2825) and folded into final color.
• Later, color.xyz *= input.Color.xyz at line ~2972 applies vertex color again (non-linearized).

Result: vertex color is applied twice to emissive contributions in non-PBR paths, darkening/tinting them inconsistently vs. the vanilla behavior that the guard was designed to preserve.

🔧 Suggested fix: restore TRUE_PBR guard

 	[branch] if (hasEmissive)
 	{
 		float3 glowColor = Color::Glowmap(TexGlowSampler.Sample(SampGlowSampler, uv).xyz);
 
+#		if defined(TRUE_PBR)
 		float3 vertexColor = Color::SrgbToLinear(input.Color.xyz);
 
 		if (!SharedData::linearLightingSettings.enableLinearLighting) {
 			emitColor = Color::SrgbToLinear(emitColor);
 			glowColor = Color::SrgbToLinear(glowColor);
 			emitColor *= glowColor;
 			emitColor *= vertexColor;
 			emitColor = Color::LinearToSrgb(emitColor);
 		} else {
 			emitColor *= glowColor;
 			emitColor *= vertexColor;
 		}
+#		else
+		if (!SharedData::linearLightingSettings.enableLinearLighting) {
+			emitColor = Color::LinearToSrgb(Color::SrgbToLinear(emitColor) * Color::SrgbToLinear(glowColor));
+		} else {
+			emitColor *= glowColor;
+		}
+#		endif
 	}

🤖 Prompt for AI Agents

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

In `@package/Shaders/Lighting.hlsl` around lines 2797 - 2821, The emissive path
now multiplies emitColor by the vertex color unconditionally, causing vertex
color to be applied twice for non-PBR Glowmap materials; restore the original
TRUE_PBR guard so vertex color is only folded into emissive for PBR materials.
Concretely, wrap the emitColor *= vertexColor (the multiplication using
Color::SrgbToLinear(input.Color.xyz) inside the [branch] if (hasEmissive) block)
with `#if` defined(TRUE_PBR) / `#endif` so that non-PBR paths rely on the existing
diffuse/final color.xyz *= input.Color.xyz behavior; leave all other operations
(Color::Glowmap, Color::SrgbToLinear/LinearToSrgb,
SharedData::linearLightingSettings handling, and
PBRFlags/PBR::Flags::HasEmissive checks) unchanged.

🤖 Prompt for all review comments with AI agents

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

Outside diff comments:
In `@package/Shaders/Lighting.hlsl`:
- Around line 2797-2821: The emissive path now multiplies emitColor by the
vertex color unconditionally, causing vertex color to be applied twice for
non-PBR Glowmap materials; restore the original TRUE_PBR guard so vertex color
is only folded into emissive for PBR materials. Concretely, wrap the emitColor
*= vertexColor (the multiplication using Color::SrgbToLinear(input.Color.xyz)
inside the [branch] if (hasEmissive) block) with `#if` defined(TRUE_PBR) / `#endif`
so that non-PBR paths rely on the existing diffuse/final color.xyz *=
input.Color.xyz behavior; leave all other operations (Color::Glowmap,
Color::SrgbToLinear/LinearToSrgb, SharedData::linearLightingSettings handling,
and PBRFlags/PBR::Flags::HasEmissive checks) unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 343b04ce-65e6-4dbe-9a15-a3bc9542ff4a

📥 Commits

Reviewing files that changed from the base of the PR and between 36c1b0f and 3be307e.

📒 Files selected for processing (1)

• package/Shaders/Lighting.hlsl

[github-actions]

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