feat(shader): Enhance shader attribute location handling and add logging for conflicts in MaterialX shader generation

This commit is contained in:
2026-01-07 11:59:16 +00:00
parent d7ac5db449
commit c7dc2c7829
4 changed files with 135 additions and 19 deletions

View File

@@ -282,10 +282,10 @@ BgfxGraphicsBackend::BgfxGraphicsBackend(std::shared_ptr<IConfigService> configS
", platformService=" + std::string(platformService_ ? "set" : "null"));
}
vertexLayout_.begin()
.add(bgfx::Attrib::Position, 3, bgfx::AttribType::Float)
.add(bgfx::Attrib::Normal, 3, bgfx::AttribType::Float)
.add(bgfx::Attrib::TexCoord0, 2, bgfx::AttribType::Float)
.add(bgfx::Attrib::Color0, 3, bgfx::AttribType::Float)
.add(bgfx::Attrib::Position, 3, bgfx::AttribType::Float) // location 0
.add(bgfx::Attrib::Normal, 3, bgfx::AttribType::Float) // location 1
.add(bgfx::Attrib::TexCoord0, 2, bgfx::AttribType::Float) // location 2
.add(bgfx::Attrib::Color0, 3, bgfx::AttribType::Float) // location 3
.end();
const std::array<float, 16> identity = {
@@ -1056,22 +1056,32 @@ void BgfxGraphicsBackend::Draw(GraphicsDeviceHandle device, GraphicsPipelineHand
const auto& vb = vertexIt->second;
const auto& ib = indexIt->second;
uint32_t startVertex = static_cast<uint32_t>(std::max(0, vertexOffset));
uint32_t availableVertices = vb->vertexCount > startVertex
? vb->vertexCount - startVertex
: 0;
if (availableVertices == 0) {
return;
if (logger_) {
logger_->Trace("BgfxGraphicsBackend", "Draw",
"vertexOffset=" + std::to_string(vertexOffset) +
", indexOffset=" + std::to_string(indexOffset) +
", indexCount=" + std::to_string(indexCount) +
", totalVertices=" + std::to_string(vb->vertexCount));
}
bgfx::setTransform(modelMatrix.data());
// When using indexed rendering with a vertex offset, bgfx expects:
// - setVertexBuffer: (handle, startVertex=0, numVertices=all)
// - setIndexBuffer: (handle, firstIndex, numIndices)
// The indices in the index buffer are already adjusted to reference the correct vertices
// in the combined vertex buffer, so we should NOT apply vertexOffset again here.
// Using the full vertex buffer ensures all vertex data is accessible.
// NOTE: Do NOT call bgfx::setTransform() when using MaterialX shaders with explicit uniforms.
// MaterialX shaders receive transformation matrices via explicit uniforms (u_worldMatrix,
// u_worldViewProjectionMatrix, etc.) set in ApplyMaterialXUniforms(). Calling setTransform()
// causes conflicts, especially with Vulkan, resulting in garbage/rainbow artifacts.
ApplyMaterialXUniforms(modelMatrix);
for (const auto& binding : pipelineIt->second->textures) {
if (bgfx::isValid(binding.sampler) && bgfx::isValid(binding.texture)) {
bgfx::setTexture(binding.stage, binding.sampler, binding.texture);
}
}
bgfx::setVertexBuffer(0, vb->handle, startVertex, availableVertices);
bgfx::setVertexBuffer(0, vb->handle);
bgfx::setIndexBuffer(ib->handle, indexOffset, indexCount);
bgfx::setState(BGFX_STATE_WRITE_RGB | BGFX_STATE_WRITE_A | BGFX_STATE_WRITE_Z |
BGFX_STATE_DEPTH_TEST_LESS | BGFX_STATE_CULL_CW | BGFX_STATE_MSAA);

View File

@@ -58,7 +58,7 @@ layout(location = 2) in vec2 inTexCoord;
layout(location = 0) out vec4 fragColor;
layout(location = 1) out vec2 fragTexCoord;
layout(std140) uniform GuiUniforms {
layout(std140, binding = 0) uniform GuiUniforms {
mat4 u_modelViewProj;
};
@@ -77,7 +77,7 @@ layout(location = 1) in vec2 fragTexCoord;
layout(location = 0) out vec4 outColor;
uniform sampler2D s_tex;
layout(binding = 0) uniform sampler2D s_tex;
void main() {
outColor = fragColor * texture(s_tex, fragTexCoord);
@@ -260,10 +260,23 @@ void BgfxGuiService::InitializeResources() {
modelViewProjUniform_ = bgfx::createUniform("u_modelViewProj", bgfx::UniformType::Mat4);
sampler_ = bgfx::createUniform("s_tex", bgfx::UniformType::Sampler);
if (logger_) {
logger_->Trace("BgfxGuiService", "InitializeResources",
"Uniforms created: modelViewProj=" + std::to_string(bgfx::isValid(modelViewProjUniform_)) +
", sampler=" + std::to_string(bgfx::isValid(sampler_)));
}
program_ = CreateProgram(kGuiVertexSource, kGuiFragmentSource);
const uint32_t whitePixel = 0xffffffff;
whiteTexture_ = CreateTexture(reinterpret_cast<const uint8_t*>(&whitePixel), 1, 1, kGuiSamplerFlags);
if (logger_) {
logger_->Trace("BgfxGuiService", "InitializeResources",
"Resources created: program=" + std::to_string(bgfx::isValid(program_)) +
", whiteTexture=" + std::to_string(bgfx::isValid(whiteTexture_)));
}
if (!bgfx::isValid(program_) && logger_) {
logger_->Error("BgfxGuiService::InitializeResources: Failed to create GUI shader program");
@@ -357,6 +370,16 @@ void BgfxGuiService::ApplyGuiView(uint32_t width, uint32_t height) {
if (logger_ && (previousWidth != width || previousHeight != height)) {
logger_->Trace("BgfxGuiService", "ApplyGuiView",
"viewport=" + std::to_string(width) + "x" + std::to_string(height));
logger_->Trace("BgfxGuiService", "ApplyGuiView",
"projection[0-3]=[" + std::to_string(proj[0]) + "," +
std::to_string(proj[1]) + "," +
std::to_string(proj[2]) + "," +
std::to_string(proj[3]) + "]");
logger_->Trace("BgfxGuiService", "ApplyGuiView",
"projection[12-15]=[" + std::to_string(proj[12]) + "," +
std::to_string(proj[13]) + "," +
std::to_string(proj[14]) + "," +
std::to_string(proj[15]) + "]");
}
bgfx::setViewTransform(viewId_, view, proj);
@@ -638,10 +661,30 @@ void BgfxGuiService::SubmitQuad(const GuiVertex& v0,
float identity[16];
bx::mtxIdentity(identity);
if (logger_) {
logger_->Trace("BgfxGuiService", "SubmitQuad",
"vertex[0]: pos=[" + std::to_string(v0.x) + "," + std::to_string(v0.y) + "," + std::to_string(v0.z) +
"], 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_)) +
", sampler=" + std::to_string(bgfx::isValid(sampler_)) +
", program=" + std::to_string(bgfx::isValid(program_)) +
", texture=" + std::to_string(bgfx::isValid(texture)) +
", viewId=" + std::to_string(viewId_));
logger_->Trace("BgfxGuiService", "SubmitQuad",
"viewProjection[0-3]=[" + std::to_string(viewProjection_[0]) + "," +
std::to_string(viewProjection_[1]) + "," +
std::to_string(viewProjection_[2]) + "," +
std::to_string(viewProjection_[3]) + "]");
}
SetScissor(scissor);
bgfx::setTransform(identity);
if (bgfx::isValid(modelViewProjUniform_)) {
bgfx::setUniform(modelViewProjUniform_, viewProjection_.data());
} else if (logger_) {
logger_->Error("BgfxGuiService::SubmitQuad: modelViewProjUniform_ is invalid!");
}
bgfx::setTexture(0, sampler_, texture);
bgfx::setVertexBuffer(0, &tvb, 0, 4);
@@ -835,12 +878,20 @@ bgfx::TextureHandle BgfxGuiService::CreateTexture(const uint8_t* rgba,
bgfx::ProgramHandle BgfxGuiService::CreateProgram(const char* vertexSource,
const char* fragmentSource) const {
if (!vertexSource || !fragmentSource) {
if (logger_) {
logger_->Error("BgfxGuiService::CreateProgram: null shader source");
}
return BGFX_INVALID_HANDLE;
}
bgfx::ShaderHandle vs = CreateShader("gui_vertex", vertexSource, true);
bgfx::ShaderHandle fs = CreateShader("gui_fragment", fragmentSource, false);
if (!bgfx::isValid(vs) || !bgfx::isValid(fs)) {
if (logger_) {
logger_->Error("BgfxGuiService::CreateProgram: shader compilation failed (vs=" +
std::to_string(bgfx::isValid(vs)) + ", fs=" +
std::to_string(bgfx::isValid(fs)) + ")");
}
if (bgfx::isValid(vs)) {
bgfx::destroy(vs);
}
@@ -849,7 +900,14 @@ bgfx::ProgramHandle BgfxGuiService::CreateProgram(const char* vertexSource,
}
return BGFX_INVALID_HANDLE;
}
return bgfx::createProgram(vs, fs, true);
bgfx::ProgramHandle program = bgfx::createProgram(vs, fs, true);
if (!bgfx::isValid(program) && logger_) {
logger_->Error("BgfxGuiService::CreateProgram: bgfx::createProgram failed to link shaders");
} else if (logger_) {
logger_->Trace("BgfxGuiService", "CreateProgram", "GUI program created successfully");
}
return program;
}
bgfx::ShaderHandle BgfxGuiService::CreateShader(const std::string& label,
@@ -884,7 +942,10 @@ bgfx::ShaderHandle BgfxGuiService::CreateShader(const std::string& label,
shaderc::CompileOptions options;
options.SetTargetEnvironment(shaderc_target_env_vulkan, shaderc_env_version_vulkan_1_1);
options.SetAutoBindUniforms(true);
options.SetAutoMapLocations(true);
// Do NOT use SetAutoMapLocations - it overrides explicit layout(location=N) declarations
// and assigns locations alphabetically by variable name, breaking the vertex layout.
// GUI shaders already specify explicit locations matching the VertexLayout.
// options.SetAutoMapLocations(true);
shaderc_shader_kind kind = isVertex ? shaderc_vertex_shader : shaderc_fragment_shader;
auto result = compiler.CompileGlslToSpv(source, kind, label.c_str(), options);

View File

@@ -156,13 +156,34 @@ void GraphicsService::RenderScene(const std::vector<RenderCommand>& commands,
// Set the view-projection matrix for the frame
backend_->SetViewState(viewState);
// Execute draw calls
// Execute draw calls with explicit fallback shader support
for (const auto& command : commands) {
auto it = pipelines_.find(command.shaderKey);
if (it != pipelines_.end()) {
backend_->Draw(device_, it->second, vertexBuffer_, indexBuffer_,
command.indexOffset, command.indexCount, command.vertexOffset,
command.modelMatrix);
} else {
// Try explicit fallback shaders in order
bool drawn = false;
for (const auto& fallbackKey : command.fallbackShaderKeys) {
auto fallbackIt = pipelines_.find(fallbackKey);
if (fallbackIt != pipelines_.end()) {
logger_->Warn("GraphicsService::RenderScene: Pipeline not found for shaderKey='" +
command.shaderKey + "', using fallback='" + fallbackKey + "'");
backend_->Draw(device_, fallbackIt->second, vertexBuffer_, indexBuffer_,
command.indexOffset, command.indexCount, command.vertexOffset,
command.modelMatrix);
drawn = true;
break;
}
}
if (!drawn) {
logger_->Warn("GraphicsService::RenderScene: Pipeline not found for shaderKey='" +
command.shaderKey + "' and no fallback shaders available or found");
}
}
}
}

View File

@@ -15,6 +15,7 @@
#include <cctype>
#include <fstream>
#include <optional>
#include <set>
#include <unordered_set>
#include <stdexcept>
#include <string>
@@ -482,8 +483,12 @@ std::string ConvertIndividualInputsToBlock(const std::string& source,
// location=0: position (vec3)
// location=1: normal (vec3)
// location=2: texcoord (vec2)
// location=3: color/tangent (vec3/vec4)
// location=3: color (vec3)
// Note: tangent is NOT in our Vertex struct, so it gets assigned to location 4+
std::map<std::string, int> nameToLocation;
std::set<int> usedLocations;
int nextAvailableLocation = 4; // Start assigning non-standard attributes from location 4
for (const auto& [loc, type, name] : inputs) {
int newLoc = loc; // default: keep original
@@ -493,8 +498,26 @@ std::string ConvertIndividualInputsToBlock(const std::string& source,
newLoc = 1;
} else if (name.find("texcoord") != std::string::npos) {
newLoc = 2;
} else if (name.find("color") != std::string::npos || name.find("tangent") != std::string::npos) {
} else if (name.find("color") != std::string::npos) {
newLoc = 3;
} else if (name.find("tangent") != std::string::npos) {
// Tangent is not in our Vertex struct, assign to unused location
newLoc = nextAvailableLocation++;
if (logger) {
logger->Trace("MaterialXShaderGenerator", "ConvertIndividualInputsToBlock",
"Tangent attribute found but not in Vertex struct, assigning location " +
std::to_string(newLoc) + " (original was " + std::to_string(loc) + ")");
}
}
// Check for location conflicts and assign a new location if needed
while (usedLocations.count(newLoc) > 0) {
newLoc = nextAvailableLocation++;
if (logger) {
logger->Trace("MaterialXShaderGenerator", "ConvertIndividualInputsToBlock",
"Location conflict for " + name + ", assigning location " +
std::to_string(newLoc));
}
}
if (logger && newLoc != loc) {
@@ -504,6 +527,7 @@ std::string ConvertIndividualInputsToBlock(const std::string& source,
}
nameToLocation[name] = newLoc;
usedLocations.insert(newLoc);
}
// Sort by remapped location