From 4e9e212c8d95cda5beeb034e33d5ff1ac32734bd Mon Sep 17 00:00:00 2001 From: Pavel Rojtberg Date: Sun, 30 Oct 2022 21:23:07 +0100 Subject: [PATCH] GL3+: validate VertexDeclaration against shader inputs like D3D11 does. Catches bugs like missing tangents --- .../GL3Plus/src/GLSL/include/OgreGLSLShader.h | 2 ++ .../GL3Plus/src/GLSL/src/OgreGLSLShader.cpp | 16 +++++++++++++ .../GL3Plus/src/OgreGL3PlusRenderSystem.cpp | 2 ++ .../include/GLSL/OgreGLSLShaderCommon.h | 4 ++++ .../src/GLSL/OgreGLSLShaderCommon.cpp | 23 +++++++++++++++++++ 5 files changed, 47 insertions(+) diff --git a/RenderSystems/GL3Plus/src/GLSL/include/OgreGLSLShader.h b/RenderSystems/GL3Plus/src/GLSL/include/OgreGLSLShader.h index 993b7ee7a75..c8e80aeca6e 100644 --- a/RenderSystems/GL3Plus/src/GLSL/include/OgreGLSLShader.h +++ b/RenderSystems/GL3Plus/src/GLSL/include/OgreGLSLShader.h @@ -70,6 +70,8 @@ namespace Ogre { void extractUniforms(int block = -1) const; void extractBufferBlocks(GLenum type) const; + void extractAttributes(); + mutable HardwareBufferPtr mDefaultBuffer; bool mHasSamplerBinding; }; diff --git a/RenderSystems/GL3Plus/src/GLSL/src/OgreGLSLShader.cpp b/RenderSystems/GL3Plus/src/GLSL/src/OgreGLSLShader.cpp index 78ae254714e..8402289e2f2 100644 --- a/RenderSystems/GL3Plus/src/GLSL/src/OgreGLSLShader.cpp +++ b/RenderSystems/GL3Plus/src/GLSL/src/OgreGLSLShader.cpp @@ -486,6 +486,20 @@ namespace Ogre { mLinked = 0; } + void GLSLShader::extractAttributes() + { + GLint numAttributes = 0; + OGRE_CHECK_GL_ERROR(glGetProgramInterfaceiv(mGLProgramHandle, GL_PROGRAM_INPUT, GL_ACTIVE_RESOURCES, &numAttributes)); + GLenum prop = GL_LOCATION; + for (int attr = 0; attr < numAttributes; ++attr) + { + GLint val; + OGRE_CHECK_GL_ERROR( + glGetProgramResourceiv(mGLProgramHandle, GL_PROGRAM_INPUT, attr, 1, &prop, 1, NULL, &val)); + mActiveInputs.push_back(val); + } + } + void GLSLShader::extractUniforms(int block) const { GLint numUniforms = 0; @@ -642,6 +656,8 @@ namespace Ogre { extractUniforms(); extractBufferBlocks(GL_UNIFORM_BLOCK); extractBufferBlocks(GL_SHADER_STORAGE_BLOCK); + if(mType == GPT_VERTEX_PROGRAM) + extractAttributes(); return; } diff --git a/RenderSystems/GL3Plus/src/OgreGL3PlusRenderSystem.cpp b/RenderSystems/GL3Plus/src/OgreGL3PlusRenderSystem.cpp index e456f940f2e..c3bdf8c6465 100644 --- a/RenderSystems/GL3Plus/src/OgreGL3PlusRenderSystem.cpp +++ b/RenderSystems/GL3Plus/src/OgreGL3PlusRenderSystem.cpp @@ -1098,6 +1098,8 @@ namespace Ogre { if (updateVAO) vao->bindToGpu(this, op.vertexData->vertexBufferBinding, 0); + mCurrentShader[GPT_VERTEX_PROGRAM]->validateDeclaration(op.vertexData->vertexDeclaration); + // We treat index buffer binding inside VAO as volatile, always updating and never relying onto it, // as one shared vertex buffer could be rendered with several index buffers, from submeshes and/or LODs if (op.useIndexes) diff --git a/RenderSystems/GLSupport/include/GLSL/OgreGLSLShaderCommon.h b/RenderSystems/GLSupport/include/GLSL/OgreGLSLShaderCommon.h index 11b4dc80eb4..9ec0aa004cd 100644 --- a/RenderSystems/GLSupport/include/GLSL/OgreGLSLShaderCommon.h +++ b/RenderSystems/GLSupport/include/GLSL/OgreGLSLShaderCommon.h @@ -106,6 +106,8 @@ namespace Ogre { /// GLSL does not provide access to the low level code of the shader, so use this shader for binding as well GpuProgram* _getBindingDelegate(void) override { return this; } + + void validateDeclaration(const VertexDeclaration* decl); protected: /// GLSL does not provide access to the low level implementation of the shader, so this method s a no-op void createLowLevelImpl() override {} @@ -144,6 +146,8 @@ namespace Ogre { /// Pointer to the uniform cache for this shader GLUniformCache mUniformCache; + std::vector mActiveInputs; // only for vertex shaders + /// Keep track of the number of shaders created. static uint mShaderCount; }; diff --git a/RenderSystems/GLSupport/src/GLSL/OgreGLSLShaderCommon.cpp b/RenderSystems/GLSupport/src/GLSL/OgreGLSLShaderCommon.cpp index 657a5cb1b83..6f585df3e6d 100644 --- a/RenderSystems/GLSupport/src/GLSL/OgreGLSLShaderCommon.cpp +++ b/RenderSystems/GLSupport/src/GLSL/OgreGLSLShaderCommon.cpp @@ -36,6 +36,7 @@ THE SOFTWARE. #include "OgreGLSLShaderCommon.h" #include "OgreGLSLPreprocessor.h" +#include "OgreGLSLProgramCommon.h" namespace Ogre { //----------------------------------------------------------------------- @@ -44,6 +45,28 @@ namespace Ogre { GLSLShaderCommon::CmdAttach GLSLShaderCommon::msCmdAttach; GLSLShaderCommon::CmdColumnMajorMatrices GLSLShaderCommon::msCmdColumnMajorMatrices; + void GLSLShaderCommon::validateDeclaration(const VertexDeclaration* decl) + { + for(auto sloc : mActiveInputs) + { + bool found = false; + for (const VertexElement & elem : decl->getElements()) + { + auto eidx = GLSLProgramCommon::getFixedAttributeIndex(elem.getSemantic(), elem.getIndex()); + if(eidx == sloc) + { + found = true; + break; + } + } + if (!found) + { + LogManager::getSingleton().logError("Vertex Shader " + mName + " consumes input at location " + + std::to_string(sloc) + " but no such VertexElement provided"); + } + } + } + String GLSLShaderCommon::getResourceLogName() const { if(mLoadFromFile)