From df031b5d9321eb7d0235ba38ce7b48af2d9e7e36 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 30 Jul 2018 14:46:16 -0700 Subject: [PATCH] Fix procedural rendering with custom uniforms --- libraries/gl/src/gl/GLShaders.cpp | 14 +++++ libraries/gl/src/gl/GLShaders.h | 1 + .../gpu-gl-common/src/gpu/gl/GLBackend.h | 4 +- .../src/gpu/gl/GLBackendShader.cpp | 36 +++++++++++++ libraries/gpu-gl-common/src/gpu/gl/GLShader.h | 2 - libraries/gpu-gl/src/gpu/gl41/GL41Backend.h | 1 - .../gpu-gl/src/gpu/gl41/GL41BackendShader.cpp | 51 +------------------ 7 files changed, 54 insertions(+), 55 deletions(-) diff --git a/libraries/gl/src/gl/GLShaders.cpp b/libraries/gl/src/gl/GLShaders.cpp index f4a7dc1fa2..03b807fbbf 100644 --- a/libraries/gl/src/gl/GLShaders.cpp +++ b/libraries/gl/src/gl/GLShaders.cpp @@ -110,6 +110,20 @@ Uniforms Uniform::load(GLuint glprogram, const std::vector& cnames) return load(glprogram, indices); } + +template +std::vector toCNames(const C& container, F lambda) { + std::vector result; + result.reserve(container.size()); + std::transform(container.begin(), container.end(), std::back_inserter(result), lambda); + return result; +} + +Uniforms Uniform::load(GLuint glprogram, const std::vector& names) { + auto cnames = toCNames(names, [](const std::string& name) { return name.c_str(); }); + return load(glprogram, cnames); +} + void UniformBlock::load(GLuint glprogram, int index) { this->index = index; GLint length = 0; diff --git a/libraries/gl/src/gl/GLShaders.h b/libraries/gl/src/gl/GLShaders.h index 7666bba823..5d16bd443e 100644 --- a/libraries/gl/src/gl/GLShaders.h +++ b/libraries/gl/src/gl/GLShaders.h @@ -40,6 +40,7 @@ struct Uniform : public ShaderBinding { static Vector load(GLuint glprogram); static Vector load(GLuint glprogram, const std::vector& indices); static Vector load(GLuint glprogram, const std::vector& names); + static Vector load(GLuint glprogram, const std::vector& names); template static Vector loadByName(GLuint glprogram, const C& names) { diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h index 0d47d75ba4..2be13bbae4 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h @@ -260,7 +260,7 @@ public: bool isTextureManagementSparseEnabled() const override { return (_textureManagement._sparseCapable && Texture::getEnableSparseTextures()); } protected: - virtual GLint getRealUniformLocation(GLint location) const { return location; } + virtual GLint getRealUniformLocation(GLint location) const; void recycle() const override; @@ -494,7 +494,7 @@ protected: } _pipeline; // Backend dependant compilation of the shader - virtual void postLinkProgram(ShaderObject& programObject, const Shader& program) const {} + virtual void postLinkProgram(ShaderObject& programObject, const Shader& program) const; virtual GLShader* compileBackendProgram(const Shader& program, const Shader::CompilationHandler& handler); virtual GLShader* compileBackendShader(const Shader& shader, const Shader::CompilationHandler& handler); virtual std::string getBackendShaderHeader() const = 0; diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gl-common/src/gpu/gl/GLBackendShader.cpp index 7379417574..88d2e8609f 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gl-common/src/gpu/gl/GLBackendShader.cpp @@ -244,6 +244,42 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program, const Shader:: return object; } +static const GLint INVALID_UNIFORM_INDEX = -1; + +GLint GLBackend::getRealUniformLocation(GLint location) const { + auto& shader = _pipeline._programShader->_shaderObjects[(GLShader::Version)isStereo()]; + auto itr = shader.uniformRemap.find(location); + if (itr == shader.uniformRemap.end()) { + // This shouldn't happen, because we use reflection to determine all the possible + // uniforms. If someone is requesting a uniform that isn't in the remapping structure + // that's a bug from the calling code, because it means that location wasn't in the + // reflection + qWarning() << "Unexpected location requested for shader"; + return INVALID_UNIFORM_INDEX; + } + return itr->second; +} + +void GLBackend::postLinkProgram(ShaderObject& shaderObject, const Shader& program) const { + const auto& glprogram = shaderObject.glprogram; + const auto& expectedUniforms = program.getUniforms(); + const auto expectedLocationsByName = expectedUniforms.getLocationsByName(); + const auto uniforms = ::gl::Uniform::load(glprogram, expectedUniforms.getNames()); + auto& uniformRemap = shaderObject.uniformRemap; + + // Pre-initialize all the uniforms with an invalid location + for (const auto& entry : expectedLocationsByName) { + uniformRemap[entry.second] = INVALID_UNIFORM_INDEX; + } + + // Now load up all the actual found uniform location + for (const auto& uniform : uniforms) { + const auto& name = uniform.name; + const auto& expectedLocation = expectedLocationsByName.at(name); + const auto& location = uniform.binding; + uniformRemap[expectedLocation] = location; + } +} GLBackend::ElementResource GLBackend::getFormatFromGLUniform(GLenum gltype) { switch (gltype) { diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLShader.h b/libraries/gpu-gl-common/src/gpu/gl/GLShader.h index 8e3eafcb0f..5d5d8a4a3c 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLShader.h +++ b/libraries/gpu-gl-common/src/gpu/gl/GLShader.h @@ -17,10 +17,8 @@ struct ShaderObject { GLuint glshader { 0 }; GLuint glprogram { 0 }; -#if defined(Q_OS_MAC) using LocationMap = std::unordered_map ; LocationMap uniformRemap; -#endif }; class GLShader : public GPUObject { diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h b/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h index 1adb62c2d0..8cf44a33d8 100644 --- a/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h +++ b/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h @@ -173,7 +173,6 @@ protected: std::string getBackendShaderHeader() const override; void postLinkProgram(ShaderObject& programObject, const Shader& program) const override; - GLint getRealUniformLocation(GLint location) const override; }; } } diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41BackendShader.cpp b/libraries/gpu-gl/src/gpu/gl41/GL41BackendShader.cpp index 798da06b01..2c3010e0f5 100644 --- a/libraries/gpu-gl/src/gpu/gl41/GL41BackendShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl41/GL41BackendShader.cpp @@ -22,46 +22,8 @@ std::string GL41Backend::getBackendShaderHeader() const { return header; } -template -std::vector toCNames(const C& container, F lambda) { - std::vector result; - result.reserve(container.size()); - std::transform(container.begin(), container.end(), std::back_inserter(result), lambda); - return result; -} - -#if defined(Q_OS_MAC) -ShaderObject::LocationMap buildRemap(GLuint glprogram, const gpu::Shader::SlotSet& slotSet) { - static const GLint INVALID_INDEX = -1; - ShaderObject::LocationMap result; - const auto expectedNames = slotSet.getNames(); - const auto count = expectedNames.size(); - std::vector indices; - indices.resize(count); - glGetUniformIndices(glprogram, (GLsizei)count, - toCNames(expectedNames, [](const std::string& name) { return name.c_str(); }).data(), - (GLuint*)indices.data()); - - const auto expectedLocationsByName = slotSet.getLocationsByName(); - for (size_t i = 0; i < count; ++i) { - const auto& index = indices[i]; - const auto& name = expectedNames[i]; - const auto& expectedLocation = expectedLocationsByName.at(name); - if (INVALID_INDEX == index) { - result[expectedLocation] = gpu::Shader::INVALID_LOCATION; - continue; - } - - ::gl::Uniform uniformInfo(glprogram, index); - if (expectedLocation != uniformInfo.binding) { - result[expectedLocation] = uniformInfo.binding; - } - } - return result; -} -#endif - void GL41Backend::postLinkProgram(ShaderObject& programObject, const Shader& program) const { + Parent::postLinkProgram(programObject, program); const auto& glprogram = programObject.glprogram; // For the UBOs, use glUniformBlockBinding to fixup the locations based on the reflection { @@ -106,14 +68,3 @@ void GL41Backend::postLinkProgram(ShaderObject& programObject, const Shader& pro } -GLint GL41Backend::getRealUniformLocation(GLint location) const { - GLint result = location; -#if defined(Q_OS_MAC) - auto& shader = _pipeline._programShader->_shaderObjects[(GLShader::Version)isStereo()]; - auto itr = shader.uniformRemap.find(location); - if (itr != shader.uniformRemap.end()) { - result = itr->second; - } -#endif - return result; -}