Skip to content

fix: X4500 overlapping registers for FACEGEN + TRUE_PBR #2030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alandtse merged 3 commits into from
Mar 30, 2026
+11 −5
Merged

fix: X4500 overlapping registers for FACEGEN + TRUE_PBR #2030

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcfe36e
Initial plan
Copilot Mar 29, 2026
4b07061
Fix X4500 overlapping register semantics in Lighting.hlsl for FACEGEN…
Copilot Mar 29, 2026
8c2a720
style: 🎨 apply pre-commit.ci formatting
pre-commit-ci[bot] Mar 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions package/Shaders/Lighting.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,10 @@ SamplerState SampEnvSampler : register(s4);
SamplerState SampEnvMaskSampler : register(s5);
# endif

# if defined(TRUE_PBR)
# if defined(TRUE_PBR) && !defined(FACEGEN)
SamplerState SampParallaxSampler : register(s4);
# endif
# if defined(TRUE_PBR)

@alandtse alandtse Mar 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot shouldn't this be an elseif?

Copilot AI Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No — using #elif here would be incorrect. When TRUE_PBR && !FACEGEN, we need both SampParallaxSampler and SampRMAOSSampler declared. If the second block were #elif defined(TRUE_PBR) instead of a fresh #if, SampRMAOSSampler would only be declared in the TRUE_PBR + FACEGEN case, not in the normal TRUE_PBR (non-FACEGEN) case — breaking the RMAOS sample at line 1794 (rawRMAOS = TexRMAOSSampler.SampleBias(SampRMAOSSampler, ...)) which has no FACEGEN guard of its own. The two separate #if blocks are necessary.

SamplerState SampRMAOSSampler : register(s5);
# endif

Expand Down Expand Up @@ -503,8 +505,10 @@ TextureCube<float4> TexEnvSampler : register(t4);
Texture2D<float4> TexEnvMaskSampler : register(t5);
# endif

# if defined(TRUE_PBR)
# if defined(TRUE_PBR) && !defined(FACEGEN)
Texture2D<float4> TexParallaxSampler : register(t4);
# endif
# if defined(TRUE_PBR)

@alandtse alandtse Mar 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question about elseif

Copilot AI Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reasoning as the sampler block above — TexRMAOSSampler is sampled at line 1794 inside a plain #if defined(TRUE_PBR) block (no FACEGEN exclusion), so it must be declared for all TRUE_PBR variants. An #elif would make TexRMAOSSampler only visible when TRUE_PBR && FACEGEN, breaking the non-FACEGEN path. Two separate #if blocks are required.

Texture2D<float4> TexRMAOSSampler : register(t5);
# endif

Expand Down Expand Up @@ -1110,6 +1114,7 @@ PS_OUTPUT main(PS_INPUT input, bool frontFace : SV_IsFrontFace)
sampledCoatColor.rgb *= Color::Diffuse(sampledCoatProperties.rgb);
sampledCoatColor.a *= sampledCoatProperties.a;
}
# if !defined(FACEGEN)
[branch] if (SharedData::extendedMaterialSettings.EnableParallax && (PBRFlags & PBR::Flags::HasDisplacement) != 0)
{
PBRParallax = true;
Expand Down Expand Up @@ -1140,7 +1145,8 @@ PS_OUTPUT main(PS_INPUT input, bool frontFace : SV_IsFrontFace)
if (SharedData::extendedMaterialSettings.EnableShadows && (parallaxShadowQuality > 0.0f || SharedData::extendedMaterialSettings.ExtendShadows))
sh0 = TexParallaxSampler.SampleLevel(SampParallaxSampler, uv, mipLevel).x;
}
# endif // TRUE_PBR
# endif // !FACEGEN
# endif // TRUE_PBR

# endif // EMAT

Expand Down Expand Up @@ -2470,7 +2476,7 @@ PS_OUTPUT main(PS_INPUT input, bool frontFace : SV_IsFrontFace)
# elif defined(EMAT_ENVMAP)
[branch] if (complexMaterialParallax)
dirDetailedShadow *= ExtendedMaterials::GetParallaxSoftShadowMultiplier(uv, mipLevel, dirLightDirectionTS, sh0, TexEnvMaskSampler, SampEnvMaskSampler, 3, lerp(parallaxShadowQuality, 1.0, SharedData::extendedMaterialSettings.ExtendShadows), screenNoise, displacementParams);
# elif defined(TRUE_PBR) && !defined(LODLANDSCAPE)
# elif defined(TRUE_PBR) && !defined(LODLANDSCAPE) && !defined(FACEGEN)
[branch] if (PBRParallax)
dirDetailedShadow *= ExtendedMaterials::GetParallaxSoftShadowMultiplier(uv, mipLevel, dirLightDirectionTS, sh0, TexParallaxSampler, SampParallaxSampler, 0, lerp(parallaxShadowQuality, 1.0, SharedData::extendedMaterialSettings.ExtendShadows), screenNoise, displacementParams);
# endif // LANDSCAPE
Expand Down Expand Up @@ -2672,7 +2678,7 @@ PS_OUTPUT main(PS_INPUT input, bool frontFace : SV_IsFrontFace)
# elif defined(EMAT_ENVMAP)
[branch] if (complexMaterialParallax)
parallaxShadow = ExtendedMaterials::GetParallaxSoftShadowMultiplier(uv, mipLevel, lightDirectionTS, sh0, TexEnvMaskSampler, SampEnvMaskSampler, 3, parallaxShadowQuality, screenNoise, displacementParams);
# elif defined(TRUE_PBR) && !defined(LODLANDSCAPE)
# elif defined(TRUE_PBR) && !defined(LODLANDSCAPE) && !defined(FACEGEN)
[branch] if (PBRParallax)
parallaxShadow = ExtendedMaterials::GetParallaxSoftShadowMultiplier(uv, mipLevel, lightDirectionTS, sh0, TexParallaxSampler, SampParallaxSampler, 0, parallaxShadowQuality, screenNoise, displacementParams);
# endif
Expand Down
Loading