BabylonJS / BabylonJS/BabylonNative
Shader cache is not invalidated when compiler output changes
- Dominant language
- C++
- Stars
- 919
- Forks
- 162
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 19
Description
[Filed by Copilot on behalf of @bghgary]
### Summary
`ShaderCacheImpl`'s key describes the shader *source*, but the value it stores depends on the *toolchain* that compiled it. Nothing ties the two together, so a change that alters compiler output for unchanged GLSL silently keeps serving stale entries.
```cpp
ShaderCacheImpl::ShaderHash ShaderCacheImpl::Hash(std::string_view vertexSource, std::string_view fragmentSource)
{
std::string normalizeVertexSource = NormalizeLineEndings(vertexSource);
std::string normalizeFragmentSource = NormalizeLineEndings(fragmentSource);
return {XXH3_64bits(normalizeVertexSource.data(), normalizeVertexSource.size()),
XXH3_64bits(normalizeFragmentSource.data(), normalizeFragmentSource.size())};
}
```
`CACHE_VERSION` exists to cover this, but it is hand-maintained and has been bumped twice in the cache's lifetime, both times for a *format* change rather than a content change:
| Commit | Change |
| --- | --- |
| `dd56a4d0` (#1402) | introduced, `CACHE_VERSION = 1` |
| `ec95b268` (#1598) | `1 -> 2`, precompiled shader support |
### What can change the output without changing the source
- **BabylonNative's own `ShaderCompiler`.** #1796 changes the sampler name written into the bgfx blob and the `UniformStages` key. Because `Program.cpp` resolves uniforms by name (`uniformStages.find(info.name)` and `uniformNameToIndex[info.name]`), a stale entry reproduces the exact bug that PR fixes: the sampler never binds.
- **SPIRV-Cross** (`a512817ddbcd879a3929aef7d1d762871bdf8635`) — the reserved-keyword renaming behind #1796 lives here.
- **glslang** (`284e4301e5a6b44b279635276588db7cdd942624`) — GLSL to SPIR-V.
- **bgfx**, via bgfx.cmake (`6c5515826c428337b7cbc0a5a7cc6d12325ce72b`) — blob layout and uniform table.
That last one is not hypothetical: #1795 moved the bgfx.cmake pin and merged without touching `CACHE_VERSION`.
### Suggested direction
Derive the version instead of hand-maintaining it. CMake already holds every dependency SHA above, so at configure time it can digest them together with a content hash of `Plugins/ShaderCompiler/Source/*` and the selected `GRAPHICS_API`, and `configure_file` the result into a generated header:
```cpp
constexpr uint32_t SHADER_CACHE_VERSION = /* digest(dependency pins + compiler sources + graphics api) */;
```
`CACHE_VERSION` then becomes that constant. Both PRs above would have invalidated automatically, with no author having to notice.
Keeping this as the whole-file version check rather than folding the digest into `Hash()` is deliberate: `Load` already discards the file cleanly on mismatch, whereas a key-based scheme leaves superseded entries in the file forever as unreachable garbage.
### Tradeoffs
- Over-invalidates: a comment-only edit under `Plugins/ShaderCompiler/Source/` costs one full recompile. That is strictly safer than today's under-invalidation and is a one-time startup cost.
- Hash file contents rather than a git SHA, so dirty working trees are covered and configure does not need git.
- For the precompiled shader flow added in #1598, this enforces by construction what is currently convention: a shipped cache is only consumed by a build that would have produced it.
### Note
#1796 should still bump `CACHE_VERSION` by hand; this issue is the follow-up so the next change does not depend on someone noticing.
Contributor guide
Research direction
Start by tracing ShaderCacheImpl's CACHE_VERSION use through Load, then inspect the CMake dependency pins, GRAPHICS_API selection, and Plugins/ShaderCompiler/Source/* inputs. Follow how configure_file generates headers and replace the hand-maintained version with a digest of those inputs. Done means compiler, dependency, source, or graphics API changes produce a mismatch that causes Load to discard the cache.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp
- Domain
- build-system, compilers, computer-graphics
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100