fix: X4500 overlapping registers for FACEGEN + TRUE_PBR - #2030
Conversation
… + TRUE_PBR combination Agent-Logs-Url: https://github.com/doodlum/skyrim-community-shaders/sessions/fe14c1b2-9d9d-460d-92f6-88976b50e7c2 Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Actionable Suggestions
|
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
✅ A pre-release build is available for this PR: |
alandtse
left a comment
There was a problem hiding this comment.
@copilot please review comments critically and address or explain why not.
| # if defined(TRUE_PBR) && !defined(FACEGEN) | ||
| SamplerState SampParallaxSampler : register(s4); | ||
| # endif | ||
| # if defined(TRUE_PBR) |
There was a problem hiding this comment.
@copilot shouldn't this be an elseif?
There was a problem hiding this comment.
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.
| # if defined(TRUE_PBR) && !defined(FACEGEN) | ||
| Texture2D<float4> TexParallaxSampler : register(t4); | ||
| # endif | ||
| # if defined(TRUE_PBR) |
There was a problem hiding this comment.
Same question about elseif
There was a problem hiding this comment.
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.
Both review comments have been addressed. The two separate |
…aders#2030) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit d2c620a)
…aders#2030) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
When
FACEGENandTRUE_PBRare both defined, the HLSL compiler throwserror X4500: overlapping register semanticsbecause both feature blocks unconditionally claim registerss4/t4—FACEGENfor detail sampler/texture,TRUE_PBRfor parallax sampler/texture.Changes
s4/t4): GuardSampParallaxSampler/TexParallaxSamplerwith!defined(FACEGEN). Split theTRUE_PBRblock soSampRMAOSSampler/TexRMAOSSampler(s5/t5) remain declared forFACEGEN+TRUE_PBR— those registers are free whenFACEGENis active (theENVMAPblock already has the same!defined(FACEGEN)guard).Parallax displacement block (EMAT/TRUE_PBR section): Wrap the
HasDisplacementparallax path with#if !defined(FACEGEN)— face geometry doesn't use parallax displacement, andTexParallaxSampler/SampParallaxSamplerare undeclared for this combination.PBRParallaxstays declared (remainsfalse) so downstream code compiles correctly.Shadow multiplier blocks (directional + point light): Add
!defined(FACEGEN)to the#elif defined(TRUE_PBR) && !defined(LODLANDSCAPE)branches that passTexParallaxSampler/SampParallaxSamplertoGetParallaxSoftShadowMultiplier— those resources are not declared forFACEGEN+TRUE_PBR.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.