feat(shader): Enhance shader validation and refactor uniform handling in BgfxGuiService

src/services/impl/bgfx_gui_service.cpp

@@ -298,7 +298,6 @@ void BgfxGuiService::InitializeResources() {
     if (configService_) {
         const auto& materialConfig = configService_->GetMaterialXConfig();
         if (materialConfig.enabled && materialConfig.shaderKey == "gui") {
-            usingMaterialX = true;
             try {
                 ShaderPaths generated = materialxGenerator_.Generate(materialConfig, {});
                 if (!generated.vertexSource.empty() && !generated.fragmentSource.empty()) {
@@ -306,6 +305,7 @@ void BgfxGuiService::InitializeResources() {
                     guiFragmentSourceOverride_ = std::move(generated.fragmentSource);
                     vertexSource = guiVertexSourceOverride_.c_str();
                     fragmentSource = guiFragmentSourceOverride_.c_str();
+                    usingMaterialX = true;
                     if (logger_) {
                         logger_->Trace("BgfxGuiService", "InitializeResources",
                                        "Using MaterialX GUI shaders");
@@ -321,9 +321,13 @@ void BgfxGuiService::InitializeResources() {
             }
         }
     }
-    
+
+    usesMaterialXShaders_ = usingMaterialX;
+    usesPredefinedModelViewProj_ = false;
+    modelViewProjUniform_ = BGFX_INVALID_HANDLE;
+
     // Create uniforms matching the shader we're using
-    if (usingMaterialX) {
+    if (usesMaterialXShaders_) {
         // MaterialX shaders use separate world and viewProjection matrices
         worldMatrixUniform_ = bgfx::createUniform("u_worldMatrix", bgfx::UniformType::Mat4);
         viewProjMatrixUniform_ = bgfx::createUniform("u_viewProjectionMatrix", bgfx::UniformType::Mat4);
@@ -334,13 +338,13 @@ void BgfxGuiService::InitializeResources() {
                            ", sampler=" + std::to_string(bgfx::isValid(sampler_)));
         }
     } else {
-        // Built-in shader uses combined modelViewProj matrix
-        modelViewProjUniform_ = bgfx::createUniform("u_modelViewProj", bgfx::UniformType::Mat4);
-        if (logger_) {
-            logger_->Trace("BgfxGuiService", "InitializeResources",
-                           "Built-in uniforms: modelViewProj=" + std::to_string(bgfx::isValid(modelViewProjUniform_)) +
-                           ", sampler=" + std::to_string(bgfx::isValid(sampler_)));
-        }
+        modelViewProjUniform_ = BGFX_INVALID_HANDLE;
+        usesPredefinedModelViewProj_ = false;
+    }
+
+    if (logger_) {
+        logger_->Trace("BgfxGuiService", "InitializeResources",
+                       "GUI shader mode=" + std::string(usesMaterialXShaders_ ? "materialx" : "builtin"));
     }
 
     program_ = CreateProgram(vertexSource, fragmentSource);
@@ -743,7 +747,7 @@ void BgfxGuiService::SubmitQuad(const GuiVertex& v0,
                        "], color=[" + std::to_string(v0.r) + "," + std::to_string(v0.g) + "," + std::to_string(v0.b) + "," + std::to_string(v0.a) +
                        "], uv=[" + std::to_string(v0.u) + "," + std::to_string(v0.v) + "]");
         logger_->Trace("BgfxGuiService", "SubmitQuad",
-                       "uniforms: mvp=" + std::to_string(bgfx::isValid(modelViewProjUniform_)) +
+                       "uniforms: mode=" + std::string(usesMaterialXShaders_ ? "materialx" : "builtin") +
                        ", sampler=" + std::to_string(bgfx::isValid(sampler_)) +
                        ", program=" + std::to_string(bgfx::isValid(program_)) +
                        ", texture=" + std::to_string(bgfx::isValid(texture)) +
@@ -755,19 +759,35 @@ void BgfxGuiService::SubmitQuad(const GuiVertex& v0,
                        std::to_string(viewProjection_[3]) + "]");
     }
 
+    if (!bgfx::isValid(sampler_)) {
+        if (logger_) {
+            logger_->Error("BgfxGuiService::SubmitQuad: Sampler uniform not initialized");
+        }
+        return;
+    }
+
     SetScissor(scissor);
     bgfx::setTransform(identity);
     
     // Use appropriate uniforms based on shader type
-    if (bgfx::isValid(modelViewProjUniform_)) {
-        // Built-in shader: single combined matrix
-        bgfx::setUniform(modelViewProjUniform_, viewProjection_.data());
-    } else if (bgfx::isValid(worldMatrixUniform_) && bgfx::isValid(viewProjMatrixUniform_)) {
+    if (usesMaterialXShaders_) {
+        if (!bgfx::isValid(worldMatrixUniform_) || !bgfx::isValid(viewProjMatrixUniform_)) {
+            if (logger_) {
+                logger_->Error("BgfxGuiService::SubmitQuad: MaterialX uniforms not initialized");
+            }
+            return;
+        }
         // MaterialX shader: separate matrices
         bgfx::setUniform(worldMatrixUniform_, identity);
         bgfx::setUniform(viewProjMatrixUniform_, viewProjection_.data());
-    } else if (logger_) {
-        logger_->Error("BgfxGuiService::SubmitQuad: No valid uniforms for shader!");
+    } else if (!usesPredefinedModelViewProj_) {
+        if (!bgfx::isValid(modelViewProjUniform_)) {
+            if (logger_) {
+                logger_->Error("BgfxGuiService::SubmitQuad: GUI modelViewProj uniform not initialized");
+            }
+            return;
+        }
+        bgfx::setUniform(modelViewProjUniform_, viewProjection_.data());
     }
     bgfx::setTexture(0, sampler_, texture);
     bgfx::setVertexBuffer(0, &tvb, 0, 4);
@@ -959,7 +979,7 @@ bgfx::TextureHandle BgfxGuiService::CreateTexture(const uint8_t* rgba,
 }
 
 bgfx::ProgramHandle BgfxGuiService::CreateProgram(const char* vertexSource,
-                                                  const char* fragmentSource) const {
+                                                  const char* fragmentSource) {
     if (!vertexSource || !fragmentSource) {
         if (logger_) {
             logger_->Error("BgfxGuiService::CreateProgram: null shader source");
@@ -988,6 +1008,10 @@ bgfx::ProgramHandle BgfxGuiService::CreateProgram(const char* vertexSource,
         }
         return BGFX_INVALID_HANDLE;
     }
+
+    if (!usesMaterialXShaders_) {
+        ResolveGuiMatrixUniform(vs);
+    }
     
     bgfx::ProgramHandle program = bgfx::createProgram(vs, fs, true);
     if (!bgfx::isValid(program) && logger_) {
@@ -1038,6 +1062,55 @@ bgfx::ShaderHandle BgfxGuiService::CreateShader(const std::string& label,
     return compiler.CompileShader(label, source, isVertex, uniforms, attributes);
 }
 
+void BgfxGuiService::ResolveGuiMatrixUniform(bgfx::ShaderHandle shader) {
+    modelViewProjUniform_ = BGFX_INVALID_HANDLE;
+    usesPredefinedModelViewProj_ = false;
+
+    if (!bgfx::isValid(shader)) {
+        usesPredefinedModelViewProj_ = true;
+        return;
+    }
+
+    const uint16_t uniformCount = bgfx::getShaderUniforms(shader, nullptr, 0);
+    std::string modelViewProjName = "predefined";
+    std::string uniformNames;
+
+    if (uniformCount > 0) {
+        std::vector<bgfx::UniformHandle> uniforms(uniformCount);
+        bgfx::getShaderUniforms(shader, uniforms.data(), uniformCount);
+
+        for (const auto& uniform : uniforms) {
+            bgfx::UniformInfo info{};
+            bgfx::getUniformInfo(uniform, info);
+            if (!uniformNames.empty()) {
+                uniformNames += ", ";
+            }
+            uniformNames += info.name;
+            if (std::strcmp(info.name, "u_modelViewProj") == 0 ||
+                std::strcmp(info.name, "UniformBuffer.u_modelViewProj") == 0) {
+                modelViewProjUniform_ = uniform;
+                modelViewProjName = info.name;
+                break;
+            }
+        }
+    }
+
+    if (!bgfx::isValid(modelViewProjUniform_)) {
+        usesPredefinedModelViewProj_ = true;
+    }
+
+    if (logger_) {
+        logger_->Trace("BgfxGuiService", "ResolveGuiMatrixUniform",
+                       "uniformCount=" + std::to_string(uniformCount) +
+                       ", modelViewProj=" + modelViewProjName +
+                       (uniformNames.empty() ? "" : ", uniforms=[" + uniformNames + "]"));
+        if (!uniformNames.empty() && modelViewProjName == "predefined") {
+            logger_->Warn("BgfxGuiService::ResolveGuiMatrixUniform: u_modelViewProj not found; uniforms=[" +
+                          uniformNames + "]");
+        }
+    }
+}
+
 void BgfxGuiService::PruneTextCache() {
     if (textCache_.size() <= maxTextCacheEntries_) {
         return;

src/services/impl/bgfx_gui_service.hpp

@@ -143,8 +143,9 @@ private:
                                       uint32_t width,
                                       uint32_t height,
                                       uint64_t flags) const;
-    bgfx::ProgramHandle CreateProgram(const char* vertexSource, const char* fragmentSource) const;
+    bgfx::ProgramHandle CreateProgram(const char* vertexSource, const char* fragmentSource);
     bgfx::ShaderHandle CreateShader(const std::string& label, const std::string& source, bool isVertex) const;
+    void ResolveGuiMatrixUniform(bgfx::ShaderHandle shader);
 
     std::shared_ptr<IConfigService> configService_;
     std::shared_ptr<ILogger> logger_;
@@ -173,6 +174,8 @@ private:
     uint16_t viewId_ = 1;
     bool initialized_ = false;
     bool loggedMissingResources_ = false;
+    bool usesMaterialXShaders_ = false;
+    bool usesPredefinedModelViewProj_ = false;
     uint64_t frameIndex_ = 0;
     size_t maxTextCacheEntries_ = 256;
     size_t maxSvgCacheEntries_ = 64;

src/services/impl/bgfx_shader_compiler.cpp

@@ -1,6 +1,7 @@
 #include "bgfx_shader_compiler.hpp"
 
 #include <algorithm>
+#include <cstdint>
 #include <cstring>
 #include <fstream>
 #include <stdexcept>
@@ -39,6 +40,154 @@ const char* ShadercTargetName(bgfx::RendererType::Enum type) {
     }
 }
 
+bool ValidateBgfxShaderBinary(const std::vector<char>& buffer,
+                              char expectedType,
+                              std::string& error) {
+    if (buffer.size() < sizeof(uint32_t) * 3 + sizeof(uint16_t)) {
+        error = "buffer too small for header";
+        return false;
+    }
+
+    size_t offset = 0;
+    auto readBytes = [&](void* out, size_t size) {
+        if (offset + size > buffer.size()) {
+            return false;
+        }
+        std::memcpy(out, buffer.data() + offset, size);
+        offset += size;
+        return true;
+    };
+
+    uint32_t magic = 0;
+    if (!readBytes(&magic, sizeof(magic))) {
+        error = "failed to read magic";
+        return false;
+    }
+
+    const uint32_t base = magic & 0x00FFFFFFu;
+    const uint32_t vsh = static_cast<uint32_t>('V')
+        | (static_cast<uint32_t>('S') << 8)
+        | (static_cast<uint32_t>('H') << 16);
+    const uint32_t fsh = static_cast<uint32_t>('F')
+        | (static_cast<uint32_t>('S') << 8)
+        | (static_cast<uint32_t>('H') << 16);
+    const uint32_t csh = static_cast<uint32_t>('C')
+        | (static_cast<uint32_t>('S') << 8)
+        | (static_cast<uint32_t>('H') << 16);
+    const uint32_t expectedBase = (expectedType == 'v') ? vsh : (expectedType == 'c' ? csh : fsh);
+    if (base != vsh && base != fsh && base != csh) {
+        error = "invalid magic";
+        return false;
+    }
+    if (base != expectedBase) {
+        error = "shader type mismatch";
+        return false;
+    }
+
+    const uint8_t version = static_cast<uint8_t>(magic >> 24);
+    const bool hasTexData = version >= 8;
+    const bool hasTexFormat = version >= 10;
+
+    uint32_t hashIn = 0;
+    uint32_t hashOut = 0;
+    if (!readBytes(&hashIn, sizeof(hashIn)) || !readBytes(&hashOut, sizeof(hashOut))) {
+        error = "failed to read hashes";
+        return false;
+    }
+
+    uint16_t uniformCount = 0;
+    if (!readBytes(&uniformCount, sizeof(uniformCount))) {
+        error = "failed to read uniform count";
+        return false;
+    }
+
+    for (uint16_t i = 0; i < uniformCount; ++i) {
+        uint8_t nameSize = 0;
+        if (!readBytes(&nameSize, sizeof(nameSize))) {
+            error = "failed to read uniform name size";
+            return false;
+        }
+        if (offset + nameSize > buffer.size()) {
+            error = "uniform name out of bounds";
+            return false;
+        }
+        offset += nameSize;
+
+        uint8_t type = 0;
+        uint8_t num = 0;
+        uint16_t regIndex = 0;
+        uint16_t regCount = 0;
+        if (!readBytes(&type, sizeof(type)) ||
+            !readBytes(&num, sizeof(num)) ||
+            !readBytes(&regIndex, sizeof(regIndex)) ||
+            !readBytes(&regCount, sizeof(regCount))) {
+            error = "failed to read uniform metadata";
+            return false;
+        }
+
+        if (hasTexData) {
+            uint8_t texComponent = 0;
+            uint8_t texDimension = 0;
+            if (!readBytes(&texComponent, sizeof(texComponent)) ||
+                !readBytes(&texDimension, sizeof(texDimension))) {
+                error = "failed to read texture metadata";
+                return false;
+            }
+        }
+        if (hasTexFormat) {
+            uint16_t texFormat = 0;
+            if (!readBytes(&texFormat, sizeof(texFormat))) {
+                error = "failed to read texture format";
+                return false;
+            }
+        }
+    }
+
+    uint32_t shaderSize = 0;
+    if (!readBytes(&shaderSize, sizeof(shaderSize))) {
+        error = "failed to read shader size";
+        return false;
+    }
+    if (shaderSize % 4 != 0) {
+        error = "shader size not aligned";
+        return false;
+    }
+    if (offset + shaderSize + 1 > buffer.size()) {
+        error = "shader code out of bounds";
+        return false;
+    }
+
+    if (shaderSize >= sizeof(uint32_t)) {
+        uint32_t spirvMagic = 0;
+        std::memcpy(&spirvMagic, buffer.data() + offset, sizeof(uint32_t));
+        if (spirvMagic != 0x07230203u) {
+            error = "invalid SPIR-V magic";
+            return false;
+        }
+    }
+
+    offset += shaderSize + 1;
+
+    uint8_t numAttrs = 0;
+    if (!readBytes(&numAttrs, sizeof(numAttrs))) {
+        error = "failed to read attribute count";
+        return false;
+    }
+    if (offset + static_cast<size_t>(numAttrs) * sizeof(uint16_t) > buffer.size()) {
+        error = "attribute list out of bounds";
+        return false;
+    }
+    offset += static_cast<size_t>(numAttrs) * sizeof(uint16_t);
+
+    uint16_t constantSize = 0;
+    if (!readBytes(&constantSize, sizeof(constantSize))) {
+        error = "failed to read constant size";
+        return false;
+    }
+
+    return true;
+}
+
 }  // namespace
 
 BgfxShaderCompiler::BgfxShaderCompiler(std::shared_ptr<ILogger> logger,
@@ -107,6 +256,15 @@ bgfx::ShaderHandle BgfxShaderCompiler::CompileShader(
                            ", target=" + std::string(target) +
                            ", size=" + std::to_string(outSize));
         }
+        std::string validationError;
+        if (!ValidateBgfxShaderBinary(buffer, isVertex ? 'v' : 'f', validationError)) {
+            compiledInMemory = false;
+            buffer.clear();
+            if (logger_) {
+                logger_->Error("BgfxShaderCompiler: invalid shader binary for " + label +
+                               " (" + validationError + ")");
+            }
+        }
     } else if (logger_) {
         logger_->Trace("BgfxShaderCompiler", "CompileShader",
                        "in-memory compile failed for " + label +
@@ -171,6 +329,15 @@ bgfx::ShaderHandle BgfxShaderCompiler::CompileShader(
         remove(tempOutputPath.c_str());
     }
 
+    std::string validationError;
+    if (!ValidateBgfxShaderBinary(buffer, isVertex ? 'v' : 'f', validationError)) {
+        if (logger_) {
+            logger_->Error("BgfxShaderCompiler: invalid shader binary for " + label +
+                           " (" + validationError + ")");
+        }
+        return BGFX_INVALID_HANDLE;
+    }
+
     uint32_t binSize = static_cast<uint32_t>(buffer.size());
     const bgfx::Memory* mem = bgfx::copy(buffer.data(), binSize);
     bgfx::ShaderHandle handle = bgfx::createShader(mem);
@@ -178,6 +345,9 @@ bgfx::ShaderHandle BgfxShaderCompiler::CompileShader(
         logger_->Error("BgfxShaderCompiler: bgfx::createShader failed for " + label +
                       " (binSize=" + std::to_string(binSize) + ")");
     } else if (logger_) {
+        logger_->Info("BgfxShaderCompiler: created shader " + label +
+                      " (binSize=" + std::to_string(binSize) +
+                      ", renderer=" + std::string(RendererTypeName(rendererType)) + ")");
         logger_->Trace("BgfxShaderCompiler", "CompileShader",
                        "label=" + label + " shader created successfully, handle=" + std::to_string(handle.idx));
     }