diff --git a/VULKAN_SHADER_LINKING_PROBLEM.md b/VULKAN_SHADER_LINKING_PROBLEM.md index 0c19bbf..71dabcd 100644 --- a/VULKAN_SHADER_LINKING_PROBLEM.md +++ b/VULKAN_SHADER_LINKING_PROBLEM.md @@ -238,3 +238,143 @@ The most likely cause is a shader binary format mismatch. The `PipelineCompilerS - [shaderc API](https://github.com/google/shaderc) - [SPIR-V Specification](https://www.khronos.org/registry/spir-v/) - [Vulkan GLSL Specification](https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.html#shaders) + + +# BGFX Vulkan Shader Crash Fix + +## Problem Summary + +The application was crashing with `SIGSEGV` in `bgfx::vk::ShaderVK::create()` during shader creation on the render thread. + +**Root Cause:** In `shaderc_spirv.cpp` line 685-686, `GL_INT` uniform types were being incorrectly mapped to `UniformType::Sampler`, causing integer data uniforms (`u_numActiveLightSources`, `u_lightData.type`, `noise_octaves`) to be treated as texture samplers. + +## The Bug + +```cpp +// BEFORE (WRONG): +case 0x1404: // GL_INT: + un.type = UniformType::Sampler; // ❌ INCORRECT + break; +``` + +**Why this crashes:** +1. Integer uniforms like `u_numActiveLightSources` get marked as samplers +2. Vulkan backend tries to create descriptor bindings for these "samplers" +3. With `regIndex=0` and malformed sampler metadata, the descriptor layout creation fails +4. Results in segmentation fault in `ShaderVK::create()` + +## The Fix + +```cpp +// AFTER (CORRECT): +case 0x1404: // GL_INT: + // CRITICAL FIX: GL_INT should NOT be mapped to Sampler + // Integer uniforms are data uniforms, not texture samplers + // Map to Vec4 to maintain compatibility with bgfx uniform system + un.type = UniformType::Vec4; // ✅ CORRECT + if (bgfx::g_verbose) + { + BX_TRACE("Uniform %s: GL_INT mapped to Vec4 (regIndex=%d, NOT a sampler)", + un.name.c_str(), un.regIndex); + } + break; +``` + +**Additional improvements:** +- Added trace logging to track GL_INT uniform handling +- Added logging for unknown uniform types in the default case + +## Evidence from Log Analysis + +Your `sdl3_app.log` showed: + +- **Line 490-496:** `ceiling:fragment` shader had integer uniforms incorrectly tagged as `baseType=Sampler` + - `u_numActiveLightSources` (int) → marked as Sampler ❌ + - `u_lightData.type` (int) → marked as Sampler ❌ + - `noise_octaves` (int) → marked as Sampler ❌ + - One with `regIndex=0` which triggers the crash + +- **Line 1041:** SIGSEGV immediately after shader creation, confirming the malformed sampler metadata theory + +## How to Apply the Fix + +### Option 1: Apply the Patch File +```bash +cd /path/to/your/project/src +patch -p0 < shaderc_spirv.patch +``` + +### Option 2: Manual Edit +Edit `shaderc_spirv.cpp` around line 685: +1. Change `un.type = UniformType::Sampler;` to `un.type = UniformType::Vec4;` +2. Add the trace logging (optional but recommended) + +### Option 3: Replace the Entire File +Use the fixed version: `shaderc_spirv_fixed.cpp` + +## Rebuild Instructions + +After applying the fix: + +```bash +# Rebuild the shader compiler +python scripts/dev_commands.py build --target shaderc + +# Or full rebuild +python scripts/dev_commands.py build + +# Run with verbose logging to see the new traces +python scripts/dev_commands.py run -- --json-file-in config/your_config.json +``` + +## Expected Outcome + +After the fix: +- Integer uniforms will be correctly classified as `Vec4` data uniforms +- No more malformed sampler bindings in Vulkan +- The SIGSEGV crash in `ShaderVK::create()` will be resolved +- With verbose logging enabled, you'll see: `"Uniform u_numActiveLightSources: GL_INT mapped to Vec4 (regIndex=X, NOT a sampler)"` + +## Additional Notes + +**Why Map to Vec4?** +- BGFX's uniform system expects data uniforms to be Vec4-compatible +- This maintains compatibility while preventing sampler misclassification +- Integer uniforms in shaders are typically used for counts, flags, or indices + +**Actual Texture Samplers:** +- Real texture samplers are handled separately in lines 767-808 +- They use `resourcesrefl.separate_images` from SPIRV-Cross reflection +- They correctly set `UniformType::Sampler | kUniformSamplerBit` + +## Testing Recommendations + +1. **Run with verbose logging first:** + ```bash + BGFX_VERBOSE=1 python scripts/dev_commands.py run + ``` + +2. **Check that integer uniforms are now Vec4:** + - Look for the new trace messages in the log + - Verify no more "invalid sampler binding" errors + +3. **Test all shader variants:** + - Test with MaterialX-generated shaders + - Test GUI shaders + - Test ceiling/lighting shaders + +4. **Verify no regression:** + - Actual texture samplers should still work correctly + - Check that textures are still rendering properly + +## Files Modified + +- `shaderc_spirv.cpp` (lines 685-705) + +## Related Issues + +This fix addresses: +- SIGSEGV in `bgfx::vk::ShaderVK::create()` +- "No valid uniforms" warnings for shaders with integer uniforms +- Malformed descriptor layouts in Vulkan backend +- Use-after-free-like symptoms (though actual cause was malformed metadata) diff --git a/src/bgfx_tools/shaderc/shaderc_spirv.cpp b/src/bgfx_tools/shaderc/shaderc_spirv.cpp index c29d8a1..fb0aec6 100644 --- a/src/bgfx_tools/shaderc/shaderc_spirv.cpp +++ b/src/bgfx_tools/shaderc/shaderc_spirv.cpp @@ -680,10 +680,19 @@ namespace bgfx { namespace spirv un.regIndex = uint16_t(offset); un.regCount = un.num; - switch (program->getUniformType(ii) ) + const uint32_t uniformType = program->getUniformType(ii); + + switch (uniformType) { case 0x1404: // GL_INT: - un.type = UniformType::Sampler; + case 0x8B53: // GL_INT_VEC2: + case 0x8B54: // GL_INT_VEC3: + case 0x8B55: // GL_INT_VEC4: + un.type = UniformType::Vec4; + BX_TRACE("shaderc_spirv: int uniform mapped to Vec4 (%s, glType=0x%X, offset=%u)", + un.name.c_str(), + uniformType, + offset); break; case 0x8B52: // GL_FLOAT_VEC4: