fix: hash defines for disk cache

[alandtse]

Summary by CodeRabbit

• Chores
  • Shader cache filenames now include a stable, compact suffix representing shader-define variations, reducing cache conflicts and ensuring correct shader variants are loaded.
  • Profiling/diagnostic labels now include shader-variant information for clearer performance traces and troubleshooting.

[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: 65c51b6a-b0d7-4cac-810c-f3fcf4c63f21

📥 Commits

Reviewing files that changed from the base of the PR and between 6b4d50c and 10205a1.

📒 Files selected for processing (2)

• src/Utils/Format.cpp
• src/Utils/Format.h

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

• src/Utils/Format.cpp
• src/Utils/Format.h

---

📝 Walkthrough

Walkthrough

GetDiskPath and shader pipeline cache path construction now append a stable, hashed suffix derived from the shader defines string to on-disk shader cache filenames, via a new Util::GetShaderDefinesSuffix helper; no public APIs were altered.

Changes

Cohort / File(s)	Summary
Shader cache update src/ShaderCache.cpp	Adds include for Util.h, computes defines-based suffix, converts name to cached std::wstring, and inserts the suffix into disk cache filenames (from `"{:X}.pso
Frame annotations / pipeline path src/FrameAnnotations.cpp	Includes Util.h, obtains defines suffix, appends it to generated pipeline cache path (Data/ShaderCache/.../{descriptor}{}.pso) and incorporates the updated diskPath into the perf-event passName.
Utility: format helper src/Utils/Format.cpp, src/Utils/Format.h	Adds Util::GetShaderDefinesSuffix(const std::string&) which returns "" for empty input or an underscore-prefixed 8-hex uppercase FNV-1a hash (e.g., _1A2B3C4D) for non-empty defines strings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(VR): use only supported shaders from cache #1553 — similar changes to disk shader cache filename/path construction and descriptor formatting.

Suggested reviewers

• doodlum
• jiayev

Poem

🐰 I nibble bytes and hash each define,
A tiny suffix stitched in a line,
Cache files hum tidy, no names collide,
I hop with a badge of eight hexified pride,
A carrot for build-time, a hop for the tide.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding hashing of shader defines for disk cache filenames to ensure cache variants are properly distinguished.

_{✏️ 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]

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 1303-1321: Frame annotation disk paths are using the old hardcoded
format and thus miss the defines-based suffix; update the code that builds
diskPath in FrameAnnotations.cpp (the line using
std::format("Data/ShaderCache/{}/{:X}.pso", shader->fxpFilename, descriptor)) to
use the same logic as the shader cache — call the shared helper (e.g.
ShaderCache::GetDiskPath or GetDiskPath) with the shader name, descriptor and
ShaderClass::Pixel so the defines-based suffix is included, or reproduce the
exact suffix computation (globals::state->shaderDefinesString hash -> _{08X})
before formatting to ensure the path matches ShaderCache::GetDiskPath.

🪄 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

Run ID: 7618227d-5193-4dc1-ba99-b5e12f50336e

📥 Commits

Reviewing files that changed from the base of the PR and between c10de85 and 94e4127.

📒 Files selected for processing (1)

• src/ShaderCache.cpp

[github-actions]

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

src/FrameAnnotations.cpp (1)

38-40: Consider caching the defines suffix instead of recomputing per pass.

Lines 38-40 run inside a very hot hook. Hashing + formatting every call is avoidable when defines are mostly static. Prefer computing/storing the suffix once when defines change (e.g., in State::SetDefines) and reuse it here.

🤖 Prompt for AI Agents

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

In `@src/FrameAnnotations.cpp` around lines 38 - 40, The code repeatedly calls
Util::GetShaderDefinesSuffix(globals::state->shaderDefinesString) inside the hot
hook; cache that computed suffix on the State object and reuse it here instead
of recomputing per pass: add a cached string (e.g., State::shaderDefinesSuffix
or State::GetCachedDefinesSuffix()) and update State::SetDefines to recompute
and store the suffix when shaderDefinesString changes, then replace the inline
call in FrameAnnotations.cpp (where definesSuffix is created) to read the cached
value from globals::state.

src/Utils/Format.cpp (1)

268-274: PR metadata suggestion: tighten the commit scope and add issue linkage.

Suggested title: fix(shader-cache): hash defines in disk cache key
If this addresses a tracked bug, add Fixes #<issue-id> in the PR description.

As per coding guidelines: "When reviewing PRs, please provide suggestions for: 1) Conventional Commit Titles ... 2) Issue References ..."

🤖 Prompt for AI Agents

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

In `@src/Utils/Format.cpp` around lines 268 - 274, Update the PR metadata to
follow conventional commits and link any issue: set the PR title to
"fix(shader-cache): hash defines in disk cache key", and in the PR description
add an issue reference line like "Fixes #<issue-id>" if this change resolves a
tracked bug; mention the affected symbol GetShaderDefinesSuffix (and file
Utils/Format.cpp) in the description so reviewers can quickly correlate the
metadata to the code change.

🤖 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/Utils/Format.cpp`:
- Around line 268-274: GetShaderDefinesSuffix currently uses
std::hash<std::string> which is not stable across runs/toolchains; replace that
with a deterministic 32-bit FNV-1a implementation to compute the hash from
definesStr, cast the result to uint32_t and keep the existing
std::format("_{:08X}", ...) return; specifically modify GetShaderDefinesSuffix
to remove std::hash usage and call the new FNV-1a computation (operating on
definesStr.data()/size()) so on-disk cache keys remain stable.

In `@src/Utils/Format.h`:
- Around line 124-132: The docstring for GetShaderDefinesSuffix documents the
suffix format token as "_{08X}" but the implementation uses "_{:08X}"; update
the comment above GetShaderDefinesSuffix to state the correct token "_{:08X}"
(or otherwise match whatever the implementation produces) so the documentation
and the function implementation are consistent.

---

Nitpick comments:
In `@src/FrameAnnotations.cpp`:
- Around line 38-40: The code repeatedly calls
Util::GetShaderDefinesSuffix(globals::state->shaderDefinesString) inside the hot
hook; cache that computed suffix on the State object and reuse it here instead
of recomputing per pass: add a cached string (e.g., State::shaderDefinesSuffix
or State::GetCachedDefinesSuffix()) and update State::SetDefines to recompute
and store the suffix when shaderDefinesString changes, then replace the inline
call in FrameAnnotations.cpp (where definesSuffix is created) to read the cached
value from globals::state.

In `@src/Utils/Format.cpp`:
- Around line 268-274: Update the PR metadata to follow conventional commits and
link any issue: set the PR title to "fix(shader-cache): hash defines in disk
cache key", and in the PR description add an issue reference line like "Fixes
#<issue-id>" if this change resolves a tracked bug; mention the affected symbol
GetShaderDefinesSuffix (and file Utils/Format.cpp) in the description so
reviewers can quickly correlate the metadata to the code change.

🪄 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

Run ID: a40b73ed-00ec-4190-bae9-c26d52998985

📥 Commits

Reviewing files that changed from the base of the PR and between 94e4127 and 6b4d50c.

📒 Files selected for processing (4)

• src/FrameAnnotations.cpp
• src/ShaderCache.cpp
• src/Utils/Format.cpp
• src/Utils/Format.h

✅ Files skipped from review due to trivial changes (1)

• src/ShaderCache.cpp