From 0f95d36dbc910e96e2e970cc16eb9dd95016e571 Mon Sep 17 00:00:00 2001 From: Benau Date: Fri, 19 Jan 2018 14:41:33 +0800 Subject: [PATCH] Add proper sharing and deletion of shader files --- src/graphics/shader.hpp | 25 ++++++--- src/graphics/shader_based_renderer.cpp | 2 +- src/graphics/shader_files_manager.cpp | 74 ++++++++++++++++---------- src/graphics/shader_files_manager.hpp | 43 ++++++++++++--- src/graphics/sp/sp_shader.cpp | 20 ++++--- src/graphics/sp/sp_shader.hpp | 4 ++ src/utils/debug.cpp | 2 +- 7 files changed, 120 insertions(+), 50 deletions(-) diff --git a/src/graphics/shader.hpp b/src/graphics/shader.hpp index 02dad67de..e2353a824 100644 --- a/src/graphics/shader.hpp +++ b/src/graphics/shader.hpp @@ -54,6 +54,7 @@ protected: /** OpenGL's program id. */ GLuint m_program; + std::vector > 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); - GLint is_deleted = GL_TRUE; - glGetShaderiv(shader_id, GL_DELETE_STATUS, &is_deleted); - if (is_deleted == GL_FALSE) - glDeleteShader(shader_id); + if (shader_id) + { + m_shaders.push_back(shader_id); + glAttachShader(m_program, *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 // ------------------------------------------------------------------------ diff --git a/src/graphics/shader_based_renderer.cpp b/src/graphics/shader_based_renderer.cpp index 1bee9a130..19b7a1047 100644 --- a/src/graphics/shader_based_renderer.cpp +++ b/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(); } // ---------------------------------------------------------------------------- diff --git a/src/graphics/shader_files_manager.cpp b/src/graphics/shader_files_manager.cpp index f35c884b0..c437a979a 100644 --- a/src/graphics/shader_files_manager.cpp +++ b/src/graphics/shader_files_manager.cpp @@ -54,16 +54,18 @@ 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()) { Log::error("ShaderFilesManager", "Can not open '%s'.", file.c_str()); return; } - + const std::string stk_include = "#stk_include"; std::string line; @@ -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); - glCompileShader(id); + glShaderSource(*ss, 1, &source_pointer, &len); + 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()); - glGetShaderiv(id, GL_INFO_LOG_LENGTH, &info_length); + Log::error("ShaderFilesManager", "Error in shader %s", + 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::const_iterator i = - m_shader_files_loaded.find(file); + auto i = m_shader_files_loaded.find(full_path); assert(i == m_shader_files_loaded.end()); #endif - const GLuint id = loadShader(file, type); - m_shader_files_loaded[file] = id; - return id; + SharedShader ss = loadShader(full_path, type); + m_shader_files_loaded[full_path] = ss; + 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); + // add to the cache now + return addShaderFile(full_path, type); } // getShaderFile #endif // !SERVER_ONLY diff --git a/src/graphics/shader_files_manager.hpp b/src/graphics/shader_files_manager.hpp index f0ad1fbd9..7a90edbc6 100644 --- a/src/graphics/shader_files_manager.hpp +++ b/src/graphics/shader_files_manager.hpp @@ -25,34 +25,63 @@ #include "utils/singleton.hpp" #include +#include #include +#include #include +typedef std::shared_ptr SharedShader; + class ShaderFilesManager : public Singleton, NoCopy { private: /** * Map from a filename to a shader indentifier. Used for caching shaders. */ - std::unordered_map m_shader_files_loaded; + std::unordered_map 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 diff --git a/src/graphics/sp/sp_shader.cpp b/src/graphics/sp/sp_shader.cpp index 63b2188bc..30e5fb490 100644 --- a/src/graphics/sp/sp_shader.cpp +++ b/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); - GLint is_deleted = GL_TRUE; - glGetShaderiv(shader_id, GL_DELETE_STATUS, &is_deleted); - if (is_deleted == GL_FALSE) - glDeleteShader(shader_id); + if (shader_id) + { + m_shaders.insert(shader_id); + glAttachShader(m_program[rp], *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 diff --git a/src/graphics/sp/sp_shader.hpp b/src/graphics/sp/sp_shader.hpp index b4b9feaf8..1a23f7aa5 100644 --- a/src/graphics/sp/sp_shader.hpp +++ b/src/graphics/sp/sp_shader.hpp @@ -27,7 +27,9 @@ #include #include #include +#include #include +#include #include #include @@ -80,6 +82,8 @@ class SPShader : public NoCopy, public SPPerObjectUniform private: std::string m_name; + std::unordered_set > m_shaders; + GLuint m_program[RP_COUNT]; std::vector > m_samplers[RP_COUNT]; diff --git a/src/utils/debug.cpp b/src/utils/debug.cpp index 7316477ec..be047d23f 100644 --- a/src/utils/debug.cpp +++ b/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: