From 7554c86f7d48ed97d259a623527ba41b6f9df898 Mon Sep 17 00:00:00 2001 From: Laurent Gomila Date: Tue, 18 Jun 2013 11:55:21 +0200 Subject: [PATCH] Optimized Shader::setParameter functions, by using a cache internally (#316, #358) --- include/SFML/Graphics/Shader.hpp | 12 ++++ src/SFML/Graphics/Shader.cpp | 104 +++++++++++++++++++------------ 2 files changed, 75 insertions(+), 41 deletions(-) diff --git a/include/SFML/Graphics/Shader.hpp b/include/SFML/Graphics/Shader.hpp index 532f103c2..2fbbf6e98 100644 --- a/include/SFML/Graphics/Shader.hpp +++ b/include/SFML/Graphics/Shader.hpp @@ -507,10 +507,21 @@ private : //////////////////////////////////////////////////////////// void bindTextures() const; + //////////////////////////////////////////////////////////// + /// \brief Get the location ID of a shader parameter + /// + /// \param name Name of the parameter to search + /// + /// \return Location ID of the parameter, or -1 if not found + /// + //////////////////////////////////////////////////////////// + int getParamLocation(const std::string& name); + //////////////////////////////////////////////////////////// // Types //////////////////////////////////////////////////////////// typedef std::map TextureTable; + typedef std::map ParamTable; //////////////////////////////////////////////////////////// // Member data @@ -518,6 +529,7 @@ private : unsigned int m_shaderProgram; ///< OpenGL identifier for the program int m_currentTexture; ///< Location of the current texture in the shader TextureTable m_textures; ///< Texture variables in the shader, mapped to their location + ParamTable m_params; ///< Parameters location cache }; } // namespace sf diff --git a/src/SFML/Graphics/Shader.cpp b/src/SFML/Graphics/Shader.cpp index 32c49ee6c..072bc6e31 100644 --- a/src/SFML/Graphics/Shader.cpp +++ b/src/SFML/Graphics/Shader.cpp @@ -95,7 +95,9 @@ Shader::CurrentTextureType Shader::CurrentTexture; //////////////////////////////////////////////////////////// Shader::Shader() : m_shaderProgram (0), -m_currentTexture(-1) +m_currentTexture(-1), +m_textures (), +m_params () { } @@ -228,11 +230,9 @@ void Shader::setParameter(const std::string& name, float x) glCheck(glUseProgramObjectARB(m_shaderProgram)); // Get parameter location and assign it new values - GLint location = glGetUniformLocationARB(m_shaderProgram, name.c_str()); + GLint location = getParamLocation(name); if (location != -1) glCheck(glUniform1fARB(location, x)); - else - err() << "Parameter \"" << name << "\" not found in shader" << std::endl; // Disable program glCheck(glUseProgramObjectARB(program)); @@ -252,11 +252,9 @@ void Shader::setParameter(const std::string& name, float x, float y) glCheck(glUseProgramObjectARB(m_shaderProgram)); // Get parameter location and assign it new values - GLint location = glGetUniformLocationARB(m_shaderProgram, name.c_str()); + GLint location = getParamLocation(name); if (location != -1) glCheck(glUniform2fARB(location, x, y)); - else - err() << "Parameter \"" << name << "\" not found in shader" << std::endl; // Disable program glCheck(glUseProgramObjectARB(program)); @@ -276,11 +274,9 @@ void Shader::setParameter(const std::string& name, float x, float y, float z) glCheck(glUseProgramObjectARB(m_shaderProgram)); // Get parameter location and assign it new values - GLint location = glGetUniformLocationARB(m_shaderProgram, name.c_str()); + GLint location = getParamLocation(name); if (location != -1) glCheck(glUniform3fARB(location, x, y, z)); - else - err() << "Parameter \"" << name << "\" not found in shader" << std::endl; // Disable program glCheck(glUseProgramObjectARB(program)); @@ -300,11 +296,9 @@ void Shader::setParameter(const std::string& name, float x, float y, float z, fl glCheck(glUseProgramObjectARB(m_shaderProgram)); // Get parameter location and assign it new values - GLint location = glGetUniformLocationARB(m_shaderProgram, name.c_str()); + GLint location = getParamLocation(name); if (location != -1) glCheck(glUniform4fARB(location, x, y, z, w)); - else - err() << "Parameter \"" << name << "\" not found in shader" << std::endl; // Disable program glCheck(glUseProgramObjectARB(program)); @@ -345,11 +339,9 @@ void Shader::setParameter(const std::string& name, const sf::Transform& transfor glCheck(glUseProgramObjectARB(m_shaderProgram)); // Get parameter location and assign it new values - GLint location = glGetUniformLocationARB(m_shaderProgram, name.c_str()); + GLint location = getParamLocation(name); if (location != -1) glCheck(glUniformMatrix4fvARB(location, 1, GL_FALSE, transform.getMatrix())); - else - err() << "Parameter \"" << name << "\" not found in shader" << std::endl; // Disable program glCheck(glUseProgramObjectARB(program)); @@ -365,31 +357,28 @@ void Shader::setParameter(const std::string& name, const Texture& texture) ensureGlContext(); // Find the location of the variable in the shader - int location = glGetUniformLocationARB(m_shaderProgram, name.c_str()); - if (location == -1) + int location = getParamLocation(name); + if (location != -1) { - err() << "Texture \"" << name << "\" not found in shader" << std::endl; - return; - } - - // Store the location -> texture mapping - TextureTable::iterator it = m_textures.find(location); - if (it == m_textures.end()) - { - // New entry, make sure there are enough texture units - static const GLint maxUnits = getMaxTextureUnits(); - if (m_textures.size() + 1 >= static_cast(maxUnits)) + // Store the location -> texture mapping + TextureTable::iterator it = m_textures.find(location); + if (it == m_textures.end()) { - err() << "Impossible to use texture \"" << name << "\" for shader: all available texture units are used" << std::endl; - return; - } + // New entry, make sure there are enough texture units + static const GLint maxUnits = getMaxTextureUnits(); + if (m_textures.size() + 1 >= static_cast(maxUnits)) + { + err() << "Impossible to use texture \"" << name << "\" for shader: all available texture units are used" << std::endl; + return; + } - m_textures[location] = &texture; - } - else - { - // Location already used, just replace the texture - it->second = &texture; + m_textures[location] = &texture; + } + else + { + // Location already used, just replace the texture + it->second = &texture; + } } } } @@ -403,9 +392,7 @@ void Shader::setParameter(const std::string& name, CurrentTextureType) ensureGlContext(); // Find the location of the variable in the shader - m_currentTexture = glGetUniformLocationARB(m_shaderProgram, name.c_str()); - if (m_currentTexture == -1) - err() << "Texture \"" << name << "\" not found in shader" << std::endl; + m_currentTexture = getParamLocation(name); } } @@ -467,6 +454,11 @@ bool Shader::compile(const char* vertexShaderCode, const char* fragmentShaderCod if (m_shaderProgram) glCheck(glDeleteObjectARB(m_shaderProgram)); + // Reset the internal state + m_currentTexture = -1; + m_textures.clear(); + m_params.clear(); + // Create the program m_shaderProgram = glCreateProgramObjectARB(); @@ -564,4 +556,34 @@ void Shader::bindTextures() const glCheck(glActiveTextureARB(GL_TEXTURE0_ARB)); } + +//////////////////////////////////////////////////////////// +int Shader::getParamLocation(const std::string& name) +{ + // Check the cache + ParamTable::const_iterator it = m_params.find(name); + if (it != m_params.end()) + { + // Already in cache, return it + return it->second; + } + else + { + // Not in cache, request the location from OpenGL + int location = glGetUniformLocationARB(m_shaderProgram, name.c_str()); + if (location != -1) + { + // Location found: add it to the cache + m_params.insert(std::make_pair(name, location)); + } + else + { + // Error: location not found + err() << "Parameter \"" << name << "\" not found in shader" << std::endl; + } + + return location; + } +} + } // namespace sf