From 292bb596dce203377521f8ca4c28e982345c6b1a Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 20 Mar 2024 14:30:23 +0000 Subject: [PATCH] Add GL input attribute dynamic binding information to pipe state --- .../PipelineState/GLPipelineStateViewer.cpp | 4 +- .../VulkanPipelineStateViewer.cpp | 6 +- renderdoc/api/replay/gl_pipestate.h | 14 ++- renderdoc/api/replay/pipestate.inl | 87 +++++++------------ renderdoc/driver/gl/gl_common.h | 3 + renderdoc/driver/gl/gl_replay.cpp | 21 +++++ renderdoc/driver/gl/gl_shader_refl.cpp | 53 +++++++++++ .../driver/gl/wrappers/gl_draw_funcs.cpp | 19 ++-- renderdoc/replay/renderdoc_serialise.inl | 3 +- 9 files changed, 129 insertions(+), 81 deletions(-) diff --git a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp index 0108c1ad7..b65882f60 100644 --- a/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/GLPipelineStateViewer.cpp @@ -1394,9 +1394,7 @@ void GLPipelineStateViewer::setState() if(state.vertexShader.shaderResourceId != ResourceId()) { - int attrib = -1; - if(i < state.vertexShader.bindpointMapping.inputAttributes.count()) - attrib = state.vertexShader.bindpointMapping.inputAttributes[i]; + int attrib = a.boundShaderInput; if(attrib >= 0 && attrib < state.vertexShader.reflection->inputSignature.count()) { diff --git a/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp index 0807af0c1..53b42add2 100644 --- a/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/VulkanPipelineStateViewer.cpp @@ -2342,11 +2342,9 @@ void VulkanPipelineStateViewer::setState() if(state.vertexShader.resourceId != ResourceId()) { - int attrib = -1; - if((int32_t)a.location < state.vertexShader.bindpointMapping.inputAttributes.count()) - attrib = state.vertexShader.bindpointMapping.inputAttributes[a.location]; + uint32_t attrib = a.location; - if(attrib >= 0 && attrib < state.vertexShader.reflection->inputSignature.count()) + if(attrib < state.vertexShader.reflection->inputSignature.size()) { name = state.vertexShader.reflection->inputSignature[attrib].varName; usedSlot = true; diff --git a/renderdoc/api/replay/gl_pipestate.h b/renderdoc/api/replay/gl_pipestate.h index abbde0c4d..aa0397793 100644 --- a/renderdoc/api/replay/gl_pipestate.h +++ b/renderdoc/api/replay/gl_pipestate.h @@ -43,7 +43,8 @@ struct VertexAttribute bool operator==(const VertexAttribute &o) const { - return enabled == o.enabled && floatCast == o.floatCast && format == o.format && + return enabled == o.enabled && floatCast == o.floatCast && + boundShaderInput == o.boundShaderInput && format == o.format && !memcmp(&genericValue, &o.genericValue, sizeof(genericValue)) && vertexBufferSlot == o.vertexBufferSlot && byteOffset == o.byteOffset; } @@ -53,6 +54,8 @@ struct VertexAttribute return enabled < o.enabled; if(!(floatCast == o.floatCast)) return floatCast < o.floatCast; + if(!(boundShaderInput == o.boundShaderInput)) + return boundShaderInput < o.boundShaderInput; if(!(format == o.format)) return format < o.format; if(memcmp(&genericValue, &o.genericValue, sizeof(genericValue)) < 0) @@ -73,6 +76,15 @@ glVertexAttribIFormat) so they will be cast. )"); bool floatCast = false; + DOCUMENT(R"(This lists which shader input is bound to this attribute, as an index in the +:data:`ShaderReflection.inputSignature` list. + +If any value is set to ``-1`` then the attribute is unbound. + +:type: int +)"); + int32_t boundShaderInput = -1; + DOCUMENT(R"(The format describing how the vertex attribute is interpreted. :type: ResourceFormat diff --git a/renderdoc/api/replay/pipestate.inl b/renderdoc/api/replay/pipestate.inl index 1f8496a5d..5a651c199 100644 --- a/renderdoc/api/replay/pipestate.inl +++ b/renderdoc/api/replay/pipestate.inl @@ -858,13 +858,7 @@ rdcarray PipeState::GetVertexInputs() const int num = 0; for(int i = 0; i < attrs.count(); i++) { - int attrib = -1; - if(m_GL->vertexShader.reflection != NULL) - attrib = m_GL->vertexShader.bindpointMapping.inputAttributes[i]; - else - attrib = i; - - if(attrib >= 0) + if(attrs[i].boundShaderInput >= 0) num++; } @@ -887,15 +881,14 @@ rdcarray PipeState::GetVertexInputs() const if(m_GL->vertexShader.reflection != NULL) { - int attrib = m_GL->vertexShader.bindpointMapping.inputAttributes[i]; + int attrib = attrs[i].boundShaderInput; + + if(attrib == -1 || attrib >= m_GL->vertexShader.reflection->inputSignature.count()) + continue; const SigParameter &sigParam = m_GL->vertexShader.reflection->inputSignature[attrib]; - if(attrib >= 0 && attrib < m_GL->vertexShader.reflection->inputSignature.count()) - ret[a].name = sigParam.varName; - - if(attrib == -1) - continue; + ret[a].name = sigParam.varName; VarType varType = sigParam.varType; @@ -937,59 +930,37 @@ rdcarray PipeState::GetVertexInputs() const { const rdcarray &attrs = m_Vulkan->vertexInput.attributes; - int num = 0; - for(int i = 0; i < attrs.count(); i++) - { - int attrib = -1; - if(m_Vulkan->vertexShader.reflection != NULL) - { - if(attrs[i].location < - (uint32_t)m_Vulkan->vertexShader.bindpointMapping.inputAttributes.count()) - attrib = m_Vulkan->vertexShader.bindpointMapping.inputAttributes[attrs[i].location]; - } - else - attrib = i; - - if(attrib >= 0) - num++; - } - - int a = 0; rdcarray ret; - ret.resize(num); - for(int i = 0; i < attrs.count() && a < num; i++) + ret.resize(attrs.size()); + for(size_t i = 0; i < attrs.size(); i++) { - ret[a].name = "attr" + ToStr((uint32_t)i); - memset(&ret[a].genericValue, 0, sizeof(PixelValue)); - ret[a].vertexBuffer = (int)attrs[i].binding; - ret[a].byteOffset = attrs[i].byteOffset; - ret[a].perInstance = false; - ret[a].instanceRate = 1; - if(attrs[i].binding < (uint32_t)m_Vulkan->vertexInput.bindings.count()) + ret[i].name = "attr" + ToStr((uint32_t)i); + memset(&ret[i].genericValue, 0, sizeof(PixelValue)); + ret[i].vertexBuffer = (int)attrs[i].binding; + ret[i].byteOffset = attrs[i].byteOffset; + ret[i].perInstance = false; + ret[i].instanceRate = 1; + if(attrs[i].binding < m_Vulkan->vertexInput.bindings.size()) { - ret[a].perInstance = m_Vulkan->vertexInput.bindings[attrs[i].binding].perInstance; - ret[a].instanceRate = m_Vulkan->vertexInput.bindings[attrs[i].binding].instanceDivisor; + ret[i].perInstance = m_Vulkan->vertexInput.bindings[attrs[i].binding].perInstance; + ret[i].instanceRate = m_Vulkan->vertexInput.bindings[attrs[i].binding].instanceDivisor; } - ret[a].format = attrs[i].format; - ret[a].used = true; - ret[a].genericEnabled = false; + ret[i].format = attrs[i].format; + ret[i].used = true; + ret[i].genericEnabled = false; if(m_Vulkan->vertexShader.reflection != NULL) { - int attrib = -1; - - if(attrs[i].location < - (uint32_t)m_Vulkan->vertexShader.bindpointMapping.inputAttributes.count()) - attrib = m_Vulkan->vertexShader.bindpointMapping.inputAttributes[attrs[i].location]; - - if(attrib >= 0 && attrib < m_Vulkan->vertexShader.reflection->inputSignature.count()) - ret[a].name = m_Vulkan->vertexShader.reflection->inputSignature[attrib].varName; - - if(attrib == -1) - continue; + const rdcarray &sig = m_Vulkan->vertexShader.reflection->inputSignature; + for(const SigParameter &attr : sig) + { + if(attr.regIndex == attrs[i].location && attr.systemValue == ShaderBuiltin::Undefined) + { + ret[i].name = attr.varName; + break; + } + } } - - a++; } return ret; diff --git a/renderdoc/driver/gl/gl_common.h b/renderdoc/driver/gl/gl_common.h index 82b0f3c7b..dc6bd2f57 100644 --- a/renderdoc/driver/gl/gl_common.h +++ b/renderdoc/driver/gl/gl_common.h @@ -618,6 +618,9 @@ GLint GetNumVertexBuffers(); struct ShaderReflection; struct ShaderBindpointMapping; +void EvaluateVertexAttributeBinds(GLuint curProg, const ShaderReflection *refl, bool spirv, + rdcarray &vertexAttrBindings); + void EvaluateSPIRVBindpointMapping(GLuint curProg, int shadIdx, const ShaderReflection *refl, ShaderBindpointMapping &mapping); diff --git a/renderdoc/driver/gl/gl_replay.cpp b/renderdoc/driver/gl/gl_replay.cpp index 4342e2ae0..c8d77665b 100644 --- a/renderdoc/driver/gl/gl_replay.cpp +++ b/renderdoc/driver/gl/gl_replay.cpp @@ -1078,6 +1078,7 @@ void GLReplay::SavePipelineState(uint32_t eventId) stages[i]->bindpointMapping = ShaderBindpointMapping(); } + rdcarray vertexAttrBindings; if(curProg == 0) { drv.glGetIntegerv(eGL_PROGRAM_PIPELINE_BINDING, (GLint *)&curProg); @@ -1119,10 +1120,16 @@ void GLReplay::SavePipelineState(uint32_t eventId) spirv[i] = true; EvaluateSPIRVBindpointMapping(curProg, (int)i, refls[i], stages[i]->bindpointMapping); + + if(i == 0) + EvaluateVertexAttributeBinds(curProg, refls[i], true, vertexAttrBindings); } else { GetBindpointMapping(curProg, (int)i, refls[i], stages[i]->bindpointMapping); + + if(i == 0) + EvaluateVertexAttributeBinds(curProg, refls[i], false, vertexAttrBindings); } mappings[i] = &stages[i]->bindpointMapping; @@ -1164,10 +1171,16 @@ void GLReplay::SavePipelineState(uint32_t eventId) spirv[i] = true; EvaluateSPIRVBindpointMapping(curProg, (int)i, refls[i], stages[i]->bindpointMapping); + + if(i == 0) + EvaluateVertexAttributeBinds(curProg, refls[i], true, vertexAttrBindings); } else { GetBindpointMapping(curProg, (int)i, refls[i], stages[i]->bindpointMapping); + + if(i == 0) + EvaluateVertexAttributeBinds(curProg, refls[i], false, vertexAttrBindings); } mappings[i] = &stages[i]->bindpointMapping; @@ -1182,6 +1195,14 @@ void GLReplay::SavePipelineState(uint32_t eventId) } } + for(size_t i = 0; i < pipe.vertexInput.attributes.size(); i++) + { + if(i < vertexAttrBindings.size()) + pipe.vertexInput.attributes[i].boundShaderInput = vertexAttrBindings[i]; + else + pipe.vertexInput.attributes[i].boundShaderInput = -1; + } + // !!!NOTE!!! This function will MODIFY the refls[] binding arrays. // See inside this function for what it does and why. for(size_t i = 0; i < ARRAY_COUNT(refls); i++) diff --git a/renderdoc/driver/gl/gl_shader_refl.cpp b/renderdoc/driver/gl/gl_shader_refl.cpp index 60c3742e7..06aab166a 100644 --- a/renderdoc/driver/gl/gl_shader_refl.cpp +++ b/renderdoc/driver/gl/gl_shader_refl.cpp @@ -2394,6 +2394,59 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref // TODO: fill in Interfaces with shader subroutines? } +void EvaluateVertexAttributeBinds(GLuint curProg, const ShaderReflection *refl, bool spirv, + rdcarray &vertexAttrBindings) +{ + GLint numVAttribBindings = 16; + GL.glGetIntegerv(eGL_MAX_VERTEX_ATTRIBS, &numVAttribBindings); + + vertexAttrBindings.resize(numVAttribBindings); + for(int32_t i = 0; i < numVAttribBindings; i++) + vertexAttrBindings[i] = -1; + + if(!refl) + return; + + if(spirv) + { + for(size_t i = 0; i < refl->inputSignature.size(); i++) + if(refl->inputSignature[i].systemValue == ShaderBuiltin::Undefined) + + return; + } + + for(int32_t i = 0; i < refl->inputSignature.count(); i++) + { + // skip system inputs, as some drivers will return a location for them + if(refl->inputSignature[i].systemValue != ShaderBuiltin::Undefined) + continue; + + // SPIR-V has fixed bindings + if(spirv) + { + vertexAttrBindings[refl->inputSignature[i].regIndex] = (int32_t)i; + continue; + } + + int32_t matrixRow = 0; + rdcstr varName = refl->inputSignature[i].varName; + + int32_t offs = varName.find(":col"); + if(offs >= 0) + { + matrixRow = varName[offs + 4] - '0'; + varName.resize(offs); + } + + GLint loc = GL.glGetAttribLocation(curProg, varName.c_str()); + + if(loc >= 0 && loc < numVAttribBindings) + { + vertexAttrBindings[loc + matrixRow] = i; + } + } +} + void GetBindpointMapping(GLuint curProg, int shadIdx, const ShaderReflection *refl, ShaderBindpointMapping &mapping) { diff --git a/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp index 906069d5c..b1ffcf4ba 100644 --- a/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp +++ b/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp @@ -220,23 +220,14 @@ bool WrappedOpenGL::Check_SafeDraw(bool indexed) { const ShaderData &shaderDetails = m_Shaders[vs]; - ShaderBindpointMapping mapping; + rdcarray vertexAttrBindings; + EvaluateVertexAttributeBinds(prog, shaderDetails.reflection, !shaderDetails.spirvWords.empty(), + vertexAttrBindings); - // get bindpoint mapping - if(!shaderDetails.spirvWords.empty()) - { - mapping = shaderDetails.mapping; - EvaluateSPIRVBindpointMapping(prog, 0, shaderDetails.reflection, mapping); - } - else - { - GetBindpointMapping(prog, 0, shaderDetails.reflection, mapping); - } - - for(int attrib = 0; attrib < mapping.inputAttributes.count(); attrib++) + for(int attrib = 0; attrib < vertexAttrBindings.count(); attrib++) { // skip attributes that don't map to the shader, they're unused - int reflIndex = mapping.inputAttributes[attrib]; + int reflIndex = vertexAttrBindings[attrib]; if(reflIndex >= 0 && reflIndex < shaderDetails.reflection->inputSignature.count()) { // check that this attribute is in-bounds, and enabled. If so then the driver will read from diff --git a/renderdoc/replay/renderdoc_serialise.inl b/renderdoc/replay/renderdoc_serialise.inl index fe9ec5520..bb67ee5d6 100644 --- a/renderdoc/replay/renderdoc_serialise.inl +++ b/renderdoc/replay/renderdoc_serialise.inl @@ -1785,12 +1785,13 @@ void DoSerialise(SerialiserType &ser, GLPipe::VertexAttribute &el) { SERIALISE_MEMBER(enabled); SERIALISE_MEMBER(floatCast); + SERIALISE_MEMBER(boundShaderInput); SERIALISE_MEMBER(format); SERIALISE_MEMBER(genericValue); SERIALISE_MEMBER(vertexBufferSlot); SERIALISE_MEMBER(byteOffset); - SIZE_CHECK(32); + SIZE_CHECK(40); } template