Optimized Shader::setParameter functions, by using a cache internally (#316, #358)

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<int, const Texture*> TextureTable;
+    typedef std::map<std::string, int> 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

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());
+        int location = getParamLocation(name);
-        if (location == -1)
+        if (location != -1)
         {
-            err() << "Texture \"" << name << "\" not found in shader" << std::endl;
+            // Store the location -> texture mapping
-            return;
+            TextureTable::iterator it = m_textures.find(location);
-        }
+            if (it == m_textures.end())
-
-        // 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<std::size_t>(maxUnits))
             {
-                err() << "Impossible to use texture \"" << name << "\" for shader: all available texture units are used" << std::endl;
+                // New entry, make sure there are enough texture units
-                return;
+                static const GLint maxUnits = getMaxTextureUnits();
-            }
+                if (m_textures.size() + 1 >= static_cast<std::size_t>(maxUnits))
+                {
+                    err() << "Impossible to use texture \"" << name << "\" for shader: all available texture units are used" << std::endl;
+                    return;
+                }
 
-            m_textures[location] = &texture;
+                m_textures[location] = &texture;
-        }
+            }
-        else
+            else
-        {
+            {
-            // Location already used, just replace the texture
+                // Location already used, just replace the texture
-            it->second = &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());
+        m_currentTexture = getParamLocation(name);
-        if (m_currentTexture == -1)
-            err() << "Texture \"" << name << "\" not found in shader" << std::endl;
     }
 }
 
@@ -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