feat(shader): Update Vulkan shader handling with uniform blocks and improved compilation process

2026-04-24 13:44:58 +00:00 · 2026-01-07 18:59:52 +00:00
parent 8955c51f20
commit 700d18e0b6
3 changed files with 239 additions and 49 deletions
--- a/VULKAN_SHADER_LINKING_PROBLEM.md
+++ b/VULKAN_SHADER_LINKING_PROBLEM.md
@@ -0,0 +1,229 @@
+# Vulkan Shader Linking Problem
+
+## Problem Description
+
+The project is experiencing failures when trying to create bgfx shader programs for Vulkan rendering of the GUI overlay system. Specifically:
+
+- GUI overlay shaders (vertex and fragment) compile successfully to SPIR-V binaries
+- Individual shaders are created successfully in bgfx
+- However, `bgfx::createProgram()` fails to link the vertex and fragment shaders together
+- This results in broken GUI overlay rendering (no 2D UI elements displayed)
+
+The error manifests as: `"BgfxGuiService::CreateProgram: bgfx::createProgram failed to link shaders"`
+
+**Note**: The GUI system is a separate 2D overlay library and does not affect 3D scene rendering (walls/ceiling/floor). The test comments suggesting otherwise appear to be incorrect.
+
+## Current Status
+
+- ✅ Shaders compile from GLSL to SPIR-V using shaderc library
+- ✅ SPIR-V binaries include bgfx-compatible headers and uniform metadata
+- ✅ `bgfx::createShader()` succeeds for both vertex and fragment shaders
+- ❌ `bgfx::createProgram()` fails to link the shaders
+- ❌ GUI overlay rendering is broken (no 2D UI elements)
+
+**Clarification**: This affects only the 2D GUI overlay system, not 3D scene geometry rendering.
+
+## Background: bgfx Architecture
+
+bgfx is a cross-platform graphics library that abstracts different rendering APIs (OpenGL, Vulkan, DirectX, Metal). For Vulkan, bgfx:
+
+1. **Shader Compilation**: Expects pre-compiled shader binaries in a custom format
+2. **Binary Format**: SPIR-V data preceded by bgfx metadata (version, hash, uniform info)
+3. **Program Linking**: Links vertex and fragment shaders based on uniform compatibility
+4. **Vulkan Translation**: Handles Vulkan-specific details (descriptor sets, pipeline layouts)
+
+### bgfx Shader Binary Format
+
+bgfx shader binaries consist of:
+- **Header**: 4-byte magic number, 4-byte hash, SPIR-V size
+- **SPIR-V Data**: Compiled shader bytecode
+- **Uniform Metadata**: Information about uniforms (names, types, registers)
+
+For Vulkan, uniforms must be properly declared with binding locations.
+
+## How the PipelineCompilerService Works
+
+The `PipelineCompilerService` is our custom shader compilation service that replaces external shaderc executables.
+
+### API Overview
+
+```cpp
+class PipelineCompilerService {
+public:
+    bool Compile(const std::string& inputPath,
+                 const std::string& outputPath,
+                 const std::vector<std::string>& args);
+    std::optional<std::string> GetLastError() const;
+};
+```
+
+### Compilation Process
+
+1. **Input Reading**: Reads GLSL source from `inputPath`
+2. **Argument Parsing**: Extracts shader type (`--type vertex/fragment`) and profile (`--profile spirv`)
+3. **Shader Compilation**: Uses shaderc library to compile GLSL to SPIR-V
+   - Sets Vulkan target environment: `shaderc_target_env_vulkan`
+   - Generates SPIR-V 1.0 bytecode
+4. **Binary Formatting**: Creates bgfx-compatible binary
+   - Writes magic number (VSH/F SH + version)
+   - Writes hash (currently 0)
+   - Writes SPIR-V data
+   - Writes uniform metadata
+5. **Output Writing**: Saves binary to `outputPath`
+
+### Uniform Metadata Format
+
+For each uniform, writes:
+- **numUniforms** (uint16_t): Number of uniforms
+- **uniformName** (null-terminated string): Uniform name
+- **uniformType** (uint8_t): Type (4=Mat4, 5=Sampler2D)
+- **numElements** (uint8_t): Array size (1 for scalars)
+- **regIndex** (uint16_t): Register/binding index
+- **regCount** (uint16_t): Register count
+- **textureInfo** (uint8_t x 3): Texture component, dimension, format
+
+## How BgfxShaderCompiler Works
+
+The `BgfxShaderCompiler` integrates with the `PipelineCompilerService` to provide shader compilation within the application.
+
+### API Overview
+
+```cpp
+class BgfxShaderCompiler {
+public:
+    bgfx::ShaderHandle CompileShader(const std::string& label,
+                                     const std::string& source,
+                                     bool isVertex,
+                                     const std::vector<BgfxShaderUniform>& uniforms,
+                                     const std::vector<bgfx::Attrib::Enum>& attributes);
+};
+```
+
+### Compilation Flow
+
+1. **Renderer Detection**: Checks `bgfx::getRendererType()` to determine target API
+2. **OpenGL Path**: For OpenGL, passes raw GLSL source to `bgfx::createShader()`
+3. **Non-OpenGL Path**: 
+   - Writes GLSL source to temp file
+   - Calls `PipelineCompilerService::Compile()` with args like `{"--type", "vertex", "--profile", "spirv"}`
+   - Reads compiled binary from temp file
+   - Calls `bgfx::createShader()` with binary data
+4. **Cleanup**: Removes temp files
+
+### In-Memory Compilation Attempt
+
+The compiler first tries to use dlsym to find shaderc functions in loaded libraries, but this fails because our shaderc is linked statically.
+
+## How bgfx Program Creation Works
+
+### API Overview
+
+```cpp
+bgfx::ProgramHandle bgfx::createProgram(bgfx::ShaderHandle vs, bgfx::ShaderHandle fs, bool destroyShaders = false);
+```
+
+### Linking Process
+
+1. **Shader Validation**: Ensures both shaders are valid
+2. **Uniform Matching**: Matches uniforms between vertex and fragment shaders
+3. **Pipeline Creation**: For Vulkan, creates VkPipeline with appropriate descriptor sets
+4. **Error Handling**: Returns invalid handle on failure
+
+### Vulkan-Specific Requirements
+
+For Vulkan linking to succeed:
+- **Uniform Compatibility**: Uniforms must have matching names, types, and binding locations
+- **Descriptor Sets**: Proper binding assignments for uniform buffers and samplers
+- **SPIR-V Validity**: SPIR-V must be valid and contain required metadata
+
+## Attempted Solutions
+
+### 1. Initial Integration (Failed)
+- Used bgfx_tools shaderc executable
+- Failed due to missing dependencies and external process overhead
+
+### 2. Conan shaderc Integration (Failed)
+- Linked shaderc library statically
+- Compilation failed due to missing bgfx 3rdparty headers
+
+### 3. bgfx_tools Source Integration (Partial Success)
+- Copied bgfx_tools shaderc sources
+- Added bgfx_deps with required headers
+- Shaders compile but linking fails
+
+### 4. Shader Source Modifications
+- Updated shaders to use Vulkan uniform blocks:
+  ```glsl
+  layout (binding = 0) uniform UniformBuffer {
+      mat4 u_modelViewProj;
+  };
+  layout (binding = 1) uniform sampler2D s_tex;
+  ```
+- Changed from `#version 450` to Vulkan-compatible syntax
+
+### 5. Compilation Target Adjustments
+- Tried different shaderc target environments (OpenGL, Vulkan)
+- Settled on Vulkan target for proper SPIR-V generation
+
+### 6. Uniform Metadata Embedding
+- Manually append uniform information to SPIR-V binaries
+- Tried different uniform naming schemes (`u_modelViewProj` vs `UniformBuffer.u_modelViewProj`)
+
+## Key Findings
+
+### Successful Aspects
+- **SPIR-V Generation**: shaderc successfully compiles Vulkan GLSL to SPIR-V
+- **Binary Format**: bgfx accepts our custom binary format
+- **Individual Shaders**: `bgfx::createShader()` works for both vertex and fragment
+
+### Failure Points
+- **Program Linking**: `bgfx::createProgram()` fails despite valid individual shaders
+- **Uniform Metadata**: Current metadata format may not match bgfx Vulkan expectations
+- **Binding Mismatch**: Possible mismatch between shader bindings and bgfx register assignments
+
+### Technical Insights
+- bgfx Vulkan renderer requires SPIR-V with embedded uniform metadata
+- Uniform blocks in Vulkan need proper binding decoration in SPIR-V
+- bgfx may expect specific uniform naming conventions for Vulkan
+
+## Root Cause Hypothesis
+
+The most likely cause is incorrect uniform metadata in the shader binaries. For Vulkan uniform blocks:
+
+1. **Expected Format**: bgfx may expect uniform names like `UniformBuffer.u_modelViewProj` instead of just `u_modelViewProj`
+2. **Binding Assignment**: The regIndex values (0, 1) may not correspond to the Vulkan binding locations
+3. **Block vs Individual**: Uniform blocks may require different metadata structure than individual uniforms
+
+## Next Steps
+
+### Immediate Actions
+1. **Investigate bgfx Source**: Examine bgfx Vulkan shader loading code
+2. **Uniform Metadata Research**: Find correct format for Vulkan uniform blocks
+3. **SPIR-V Inspection**: Use spirv-dis to examine generated SPIR-V
+4. **bgfx Examples**: Compare with working bgfx Vulkan examples
+
+### Potential Solutions
+1. **Metadata Format Fix**: Adjust uniform metadata structure for uniform blocks
+2. **Naming Convention**: Try different uniform naming schemes
+3. **Binding Remapping**: Ensure regIndex matches Vulkan binding locations
+4. **SPIR-V Reflection**: Use SPIRV-Cross to extract correct uniform info
+
+### Validation Steps
+1. **Test with OpenGL**: Verify shaders work with OpenGL renderer
+2. **Compare Binaries**: Diff working vs broken shader binaries
+3. **Minimal Reproduction**: Create minimal test case for bgfx community
+
+## Files Involved
+
+- `src/services/impl/pipeline_compiler_service.cpp`: Shader compilation logic
+- `src/services/impl/bgfx_shader_compiler.cpp`: bgfx integration
+- `src/services/impl/bgfx_gui_service.cpp`: Shader sources and program creation (GUI overlay rendering)
+- `src/bgfx_deps/`: Required headers for shaderc compilation
+- `tests/test_vulkan_shader_linking.cpp`: Test case that validates GUI shader linking (note: test comments incorrectly suggest this affects 3D scene rendering)
+
+## References
+
+- [bgfx Documentation](https://bkaradzic.github.io/bgfx/)
+- [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)
--- a/src/services/impl/bgfx_gui_service.cpp
+++ b/src/services/impl/bgfx_gui_service.cpp
@@ -62,7 +62,9 @@ out VertexData
    layout (location = 1) vec2 texcoord;
 } vd;

-uniform mat4 u_modelViewProj;
+layout (binding = 0) uniform UniformBuffer {
+    mat4 u_modelViewProj;
+};

 void main()
 {
@@ -83,7 +85,7 @@ in VertexData

 layout (location = 0) out vec4 out_color;

-uniform sampler2D s_tex;
+layout (binding = 1) uniform sampler2D s_tex;

 void main()
 {
--- a/src/services/impl/pipeline_compiler_service.cpp
+++ b/src/services/impl/pipeline_compiler_service.cpp
@@ -40,48 +40,8 @@ bool PipelineCompilerService::Compile(const std::string& inputPath,
    }
    std::string source((std::istreambuf_iterator<char>(inputFile)), std::istreambuf_iterator<char>());

-    // Preprocess shader source for bgfx compatibility
+    // For bgfx Vulkan shaders, use the source as-is (it has proper Vulkan syntax)
    std::string processedSource = source;
-    
-    // Replace #version 450 with #version 330 for better compatibility
-    size_t versionPos = processedSource.find("#version 450");
-    if (versionPos != std::string::npos) {
-        processedSource.replace(versionPos, 12, "#version 330");
-    }
-    
-    // Remove layout(location=X) qualifiers for attributes
-    std::string::size_type pos = 0;
-    while ((pos = processedSource.find("layout (location =", pos)) != std::string::npos) {
-        size_t endPos = processedSource.find(")", pos);
-        if (endPos != std::string::npos) {
-            processedSource.erase(pos, endPos - pos + 1);
-        } else {
-            break;
-        }
-    }
-    
-    // Replace 'in' with 'attribute' for vertex shaders
-    if (isVertex) {
-        pos = 0;
-        while ((pos = processedSource.find("in ", pos)) != std::string::npos) {
-            processedSource.replace(pos, 3, "attribute ");
-            pos += 10; // skip past "attribute "
-        }
-        
-        // Replace 'out' with 'varying' for vertex shaders
-        pos = 0;
-        while ((pos = processedSource.find("out ", pos)) != std::string::npos) {
-            processedSource.replace(pos, 4, "varying ");
-            pos += 8; // skip past "varying "
-        }
-    } else {
-        // For fragment shaders, replace 'in' with 'varying'
-        pos = 0;
-        while ((pos = processedSource.find("in ", pos)) != std::string::npos) {
-            processedSource.replace(pos, 3, "varying ");
-            pos += 8; // skip past "varying "
-        }
-    }

    // Write output
    std::ofstream outputFile(outputPath, std::ios::binary);
@@ -101,10 +61,9 @@ bool PipelineCompilerService::Compile(const std::string& inputPath,
        shaderc::Compiler compiler;
        shaderc::CompileOptions options;

-        // Use default target environment but allow old-style GLSL
+        // Set target environment to Vulkan for Vulkan renderer
+        options.SetTargetEnvironment(shaderc_target_env_vulkan, shaderc_env_version_vulkan_1_0);
        options.SetTargetSpirv(shaderc_spirv_version_1_0);
-        // Allow relaxed rules for bgfx compatibility
-        options.SetIncluder(nullptr);

        shaderc::SpvCompilationResult result = compiler.CompileGlslToSpv(processedSource, kind, inputPath.c_str(), options);

@@ -136,12 +95,12 @@ bool PipelineCompilerService::Compile(const std::string& inputPath,

        // Write uniform information (bgfx expects this after the SPIR-V)
        if (isVertex) {
-            // Vertex shader uniforms: u_modelViewProj (mat4)
+            // Vertex shader uniforms: UniformBuffer.u_modelViewProj (mat4)
            uint16_t numUniforms = 1;
            outputFile.write(reinterpret_cast<const char*>(&numUniforms), sizeof(numUniforms));
            
-            // Uniform name (null-terminated)
-            const char* uniformName = "u_modelViewProj";
+            // Uniform name (null-terminated) - include block name for Vulkan
+            const char* uniformName = "UniformBuffer.u_modelViewProj";
            outputFile.write(uniformName, strlen(uniformName) + 1);
            
            // Uniform type (Mat4 = 4)