Add proper sharing and deletion of shader files

src/graphics/shader.hpp

@@ -54,6 +54,7 @@ protected:
 
     /** OpenGL's program id. */
     GLuint m_program;
+    std::vector<std::shared_ptr<GLuint> > m_shaders;
 
     // ========================================================================
     /** Ends recursion. */
@@ -67,13 +68,13 @@ protected:
     void loadAndAttachShader(GLint shader_type, const std::string &name,
                              Types ... args)
     {
-        GLint shader_id = ShaderFilesManager::getInstance()
+        auto shader_id = ShaderFilesManager::getInstance()
             ->getShaderFile(name, shader_type);
-        glAttachShader(m_program, shader_id);
+        if (shader_id)
-        GLint is_deleted = GL_TRUE;
+        {
-        glGetShaderiv(shader_id, GL_DELETE_STATUS, &is_deleted);
+            m_shaders.push_back(shader_id);
-        if (is_deleted == GL_FALSE)
+            glAttachShader(m_program, *shader_id);
-            glDeleteShader(shader_id);
+        }
         loadAndAttachShader(args...);
     }   // loadAndAttachShader
     // ------------------------------------------------------------------------
@@ -87,6 +88,10 @@ protected:
 
 public:
         ShaderBase();
+        ~ShaderBase()
+        {
+            glDeleteProgram(m_program);
+        }
     int loadTFBProgram(const std::string &vertex_file_path,
                        const char **varyings,
                        unsigned varyingscount);
@@ -346,7 +351,8 @@ public:
 
         GLint Result = GL_FALSE;
         glGetProgramiv(m_program, GL_LINK_STATUS, &Result);
-        if (Result == GL_FALSE) {
+        if (Result == GL_FALSE)
+        {
             int info_length;
             Log::error("Shader", "Error when linking these shaders :");
             printFileList(args...);
@@ -356,6 +362,11 @@ public:
             Log::error("Shader", error_message);
             delete[] error_message;
         }
+        // After linking all shaders can be detached
+        for (auto shader : m_shaders)
+        {
+            glDetachShader(m_program, *shader);
+        }
     }   // loadProgram
 
     // ------------------------------------------------------------------------

src/graphics/shader_based_renderer.cpp

@@ -639,9 +639,9 @@ ShaderBasedRenderer::~ShaderBasedRenderer()
     delete m_spherical_harmonics;
     delete m_skybox;
     delete m_rtts;
-    ShaderFilesManager::kill();
     ShaderBase::killShaders();
     SP::destroy();
+    ShaderFilesManager::kill();
 }
 
 // ----------------------------------------------------------------------------

src/graphics/shader_files_manager.cpp

@@ -54,9 +54,11 @@ const std::string& ShaderFilesManager::getHeader()
 
 // ----------------------------------------------------------------------------
 void ShaderFilesManager::readFile(const std::string& file, 
-                                  std::ostringstream& code)
+                                  std::ostringstream& code, bool not_header)
 {
-    std::ifstream stream(file_manager->getShader(file), std::ios::in);
+    std::ifstream stream(((file.find('/') != std::string::npos ||
+        file.find('\\') != std::string::npos) && not_header) ?
+        file : file_manager->getShader(file), std::ios::in);
 
     if (!stream.is_open())
     {
@@ -72,6 +74,7 @@ void ShaderFilesManager::readFile(const std::string& file,
         const std::size_t pos = line.find(stk_include);
 
         // load the custom file pointed by the #stk_include directive
+        // we only look for #stk_include in official shader directory
         if (pos != std::string::npos)
         {
             // find the start "
@@ -97,7 +100,7 @@ void ShaderFilesManager::readFile(const std::string& file,
             filename = filename.substr(0, pos);
             
             // read the whole include file
-            readFile(filename, code);
+            readFile(filename, code, false/*not_header*/);
         }
         else
         {
@@ -113,9 +116,16 @@ void ShaderFilesManager::readFile(const std::string& file,
  *  \param file Filename of the shader to load.
  *  \param type Type of the shader.
  */
-GLuint ShaderFilesManager::loadShader(const std::string &file, unsigned type)
+SharedShader ShaderFilesManager::loadShader(const std::string& full_path,
+                                            unsigned type)
 {
-    const GLuint id = glCreateShader(type);
+    GLuint* ss_ptr = new GLuint;
+    *ss_ptr = glCreateShader(type);
+    SharedShader ss(ss_ptr, [](GLuint* ss)
+    {
+        glDeleteShader(*ss);
+        delete ss;
+    });
 
     std::ostringstream code;
 #if !defined(USE_GLES2)
@@ -156,7 +166,7 @@ GLuint ShaderFilesManager::loadShader(const std::string &file, unsigned type)
         code << "#define Converts_10bit_Vector\n";
     }
 
-    code << "//" << file << "\n";
+    code << "//" << full_path << "\n";
     if (!CVS->isARBUniformBufferObjectUsable())
         code << "#define UBO_DISABLED\n";
     if (CVS->needsVertexIdWorkaround())
@@ -191,34 +201,37 @@ GLuint ShaderFilesManager::loadShader(const std::string &file, unsigned type)
 
     code << getHeader();
 
-    readFile(file, code);
+    readFile(full_path, code);
 
-    Log::info("ShaderFilesManager", "Compiling shader : %s", file.c_str());
+    Log::info("ShaderFilesManager", "Compiling shader: %s",
+        full_path.c_str());
     const std::string &source  = code.str();
     char const *source_pointer = source.c_str();
     int len                    = (int)source.size();
-    glShaderSource(id, 1, &source_pointer, &len);
+    glShaderSource(*ss, 1, &source_pointer, &len);
-    glCompileShader(id);
+    glCompileShader(*ss);
 
     GLint result = GL_FALSE;
-    glGetShaderiv(id, GL_COMPILE_STATUS, &result);
+    glGetShaderiv(*ss, GL_COMPILE_STATUS, &result);
     if (result == GL_FALSE)
     {
         // failed to compile
         int info_length;
-        Log::error("ShaderFilesManager", "Error in shader %s", file.c_str());
+        Log::error("ShaderFilesManager", "Error in shader %s",
-        glGetShaderiv(id, GL_INFO_LOG_LENGTH, &info_length);
+            full_path.c_str());
+        glGetShaderiv(*ss, GL_INFO_LOG_LENGTH, &info_length);
         if (info_length < 0)
             info_length = 1024;
         char *error_message = new char[info_length];
         error_message[0] = 0;
-        glGetShaderInfoLog(id, info_length, NULL, error_message);
+        glGetShaderInfoLog(*ss, info_length, NULL, error_message);
         Log::error("ShaderFilesManager", error_message);
         delete[] error_message;
+        return NULL;
     }
     glGetError();
 
-    return id;
+    return ss;
 } // loadShader
 
 // ----------------------------------------------------------------------------
@@ -226,34 +239,39 @@ GLuint ShaderFilesManager::loadShader(const std::string &file, unsigned type)
  *  \param file Filename of the shader to load.
  *  \param type Type of the shader.
  */
-GLuint ShaderFilesManager::addShaderFile(const std::string &file, unsigned type)
+SharedShader ShaderFilesManager::addShaderFile(const std::string& full_path,
+                                               unsigned type)
 {
 #ifdef DEBUG
     // Make sure no duplicated shader is added somewhere else
-    std::unordered_map<std::string, GLuint>::const_iterator i =
+    auto i = m_shader_files_loaded.find(full_path);
-        m_shader_files_loaded.find(file);
     assert(i == m_shader_files_loaded.end());
 #endif
 
-    const GLuint id = loadShader(file, type);
+    SharedShader ss = loadShader(full_path, type);
-    m_shader_files_loaded[file] = id;
+    m_shader_files_loaded[full_path] = ss;
-    return id;
+    return ss;
 }   // addShaderFile
 
 // ----------------------------------------------------------------------------
-/** Get a shader file. If the shader is not already in the cache it will be loaded and cached.
+/** Get a shader file. If the shader is not already in the cache it will be
+ *  loaded and cached.
  *  \param file Filename of the shader to load.
  *  \param type Type of the shader.
  */
-GLuint ShaderFilesManager::getShaderFile(const std::string &file, unsigned type)
+SharedShader ShaderFilesManager::getShaderFile(const std::string &file,
+                                               unsigned type)
 {
+    const std::string full_path = (file.find('/') != std::string::npos ||
+        file.find('\\') != std::string::npos) ?
+        file : file_manager->getShader(file);
     // found in cache
-    auto it = m_shader_files_loaded.find(file);
+    auto it = m_shader_files_loaded.find(full_path);
     if (it != m_shader_files_loaded.end())
         return it->second;
 
     // add to the cache now
-   return addShaderFile(file, type);
+    return addShaderFile(full_path, type);
 }   // getShaderFile
 
 #endif   // !SERVER_ONLY

src/graphics/shader_files_manager.hpp

@@ -25,34 +25,63 @@
 #include "utils/singleton.hpp"
 
 #include <algorithm>
+#include <cassert>
 #include <string>
+#include <memory>
 #include <unordered_map>
 
+typedef std::shared_ptr<GLuint> SharedShader;
+
 class ShaderFilesManager : public Singleton<ShaderFilesManager>, NoCopy
 {
 private:
     /**
      * Map from a filename to a shader indentifier. Used for caching shaders.
      */
-    std::unordered_map<std::string, GLuint> m_shader_files_loaded;
+    std::unordered_map<std::string, SharedShader> m_shader_files_loaded;
 
     // ------------------------------------------------------------------------
     const std::string& getHeader();
-    void readFile(const std::string& file, std::ostringstream& code);
+    // ------------------------------------------------------------------------
+    void readFile(const std::string& file, std::ostringstream& code,
+                  bool not_header = true);
+    // ------------------------------------------------------------------------
+    SharedShader addShaderFile(const std::string& full_path, unsigned type);
 
 public:
     // ------------------------------------------------------------------------
     ShaderFilesManager() {}
     // ------------------------------------------------------------------------
-    ~ShaderFilesManager()                                          { clean(); }
+    ~ShaderFilesManager()
+    {
+        clearAllShaderFiles();
+    }
     // ------------------------------------------------------------------------
-    void clean()                             { m_shader_files_loaded.clear(); }
+    void clearAllShaderFiles()
+    {
+        clearUnusedShaderFiles();
+        assert(m_shader_files_loaded.empty());
+    }
     // ------------------------------------------------------------------------
-    GLuint loadShader(const std::string &file, unsigned type);
+    void clearUnusedShaderFiles()
+    {
+        for (auto it = m_shader_files_loaded.begin();
+             it != m_shader_files_loaded.end();)
+        {
+            if (it->second.use_count() == 1 || !it->second)
+            {
+                it = m_shader_files_loaded.erase(it);
+            }
+            else
+            {
+                it++;
+            }
+        }
+    }
     // ------------------------------------------------------------------------
-    GLuint addShaderFile(const std::string &file, unsigned type);
+    SharedShader loadShader(const std::string& full_path, unsigned type);
     // ------------------------------------------------------------------------
-    GLuint getShaderFile(const std::string &file, unsigned type);
+    SharedShader getShaderFile(const std::string& file, unsigned type);
 
 };   // ShaderFilesManager
 

src/graphics/sp/sp_shader.cpp

@@ -60,13 +60,13 @@ void SPShader::addShaderFile(const std::string& name, GLint shader_type,
                              RenderPass rp)
 {
 #ifndef SERVER_ONLY
-    GLint shader_id = ShaderFilesManager::getInstance()
+    auto shader_id = ShaderFilesManager::getInstance()
         ->getShaderFile(name, shader_type);
-    glAttachShader(m_program[rp], shader_id);
+    if (shader_id)
-    GLint is_deleted = GL_TRUE;
+    {
-    glGetShaderiv(shader_id, GL_DELETE_STATUS, &is_deleted);
+        m_shaders.insert(shader_id);
-    if (is_deleted == GL_FALSE)
+        glAttachShader(m_program[rp], *shader_id);
-        glDeleteShader(shader_id);
+    }
 #endif
 }   // addShaderFile
 
@@ -88,6 +88,14 @@ void SPShader::linkShaderFiles(RenderPass rp)
         Log::error("SPShader", error_message);
         delete[] error_message;
     }
+    // After linking all shaders can be detached
+    GLuint shaders[10] = {};
+    GLsizei count = 0;
+    glGetAttachedShaders(m_program[rp], 10, &count, shaders);
+    for (unsigned i = 0; i < count; i++)
+    {
+        glDetachShader(m_program[rp], shaders[i]);
+    }
 #endif
 }   // linkShaderFiles
 

src/graphics/sp/sp_shader.hpp

@@ -27,7 +27,9 @@
 #include <cstring>
 #include <functional>
 #include <ostream>
+#include <memory>
 #include <string>
+#include <unordered_set>
 #include <unordered_map>
 #include <vector>
 
@@ -80,6 +82,8 @@ class SPShader : public NoCopy, public SPPerObjectUniform
 private:
     std::string m_name;
 
+    std::unordered_set<std::shared_ptr<GLuint> > m_shaders;
+
     GLuint m_program[RP_COUNT];
 
     std::vector<std::pair<unsigned, unsigned> > m_samplers[RP_COUNT];

src/utils/debug.cpp

@@ -248,8 +248,8 @@ bool handleContextMenuAction(s32 cmd_id)
     case DEBUG_GRAPHICS_RELOAD_SHADERS:
 #ifndef SERVER_ONLY
         Log::info("Debug", "Reloading shaders...");
-        ShaderFilesManager::getInstance()->clean();
         ShaderBase::killShaders();
+        ShaderFilesManager::getInstance()->clearAllShaderFiles();
 #endif
         break;
     case DEBUG_GRAPHICS_RESET: