fix: mismatched shadercache compiler flags#2277
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCompileShader's D3DCompile flag construction was revised: developer mode uses ChangesShader Compilation Flags
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.20.0)OpenGrep fatal error (exit code 2): [00.18][ERROR]: Error: exception Unix_error: No such file or directory stat src/ShaderCache.cpp Comment |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Pull request overview
Aligns ShaderCache’s runtime shader compilation flags with the established policy used by src/Utils/D3D.cpp::CompileShader, so live compilation behavior matches disk-cache expectations and user-facing compile settings.
Changes:
- Update
ShaderCache.cppcompilation flags to include strictness + optimization in non-developer mode. - Respect
enablePartialPrecisionandenableAvoidFlowControltoggles when compiling viaShaderCache. - Skip FXC validation when disk cache is enabled (to match the existing
Utils/D3D.cppbehavior).
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ShaderCache.cpp`:
- Around line 1453-1462: The shader cache must include a snapshot of the compile
flags in the shader identity so in-flight compiles cannot repopulate the cache
with stale-flag blobs; capture the exact flags computed here (the uint32_t flags
built from globals::state->enablePartialPrecision,
globals::state->enableAvoidFlowControl, globals::state->IsDeveloperMode(), and
useDiskCache) when creating/enqueuing the compile task and include that snapshot
in the cache key/Task identity (the enqueue/lookup path in ShaderCache and any
task/Job struct) so both lookup and later insertion use the same flag bits
rather than reading atomics at completion time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d599f9d-1dad-4734-9e2d-864e018f222e
📒 Files selected for processing (1)
src/ShaderCache.cpp
|
✅ A pre-release build is available for this PR: |
|
Please resolve the conversations so we know it's done. |
|
Resolved |
|
You need to literally click on the resolve conversation button. They're still open. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> (cherry picked from commit 0a6ad65)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> (cherry picked from commit 0a6ad65)
fixes mismatch between live & disk caching respecting compiler flags. Developer mode remains unchanged.
Allows greater comparison to disk caching and also the end user's experience, where live compiling is slower than the disk cause it doesn't inherit the flags correctly.
Summary by CodeRabbit