diff --git a/VULKAN_SHADER_LINKING_PROBLEM.md b/VULKAN_SHADER_LINKING_PROBLEM.md new file mode 100644 index 0000000..f294b7a --- /dev/null +++ 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& args); + std::optional 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& uniforms, + const std::vector& 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) \ No newline at end of file diff --git a/src/services/impl/bgfx_gui_service.cpp b/src/services/impl/bgfx_gui_service.cpp index 930b0c0..93ec8d9 100644 --- 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() { diff --git a/src/services/impl/pipeline_compiler_service.cpp b/src/services/impl/pipeline_compiler_service.cpp index 14f7db9..2a256a7 100644 --- 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(inputFile)), std::istreambuf_iterator()); - // 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(&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)