From fd0d12a4f45d199ef259c2a5d311377185e726b6 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Mon, 10 Apr 2023 23:50:09 +1000 Subject: [PATCH] GL: Fix shader/memory leak --- src/common/gl/program.cpp | 44 +++++----- src/common/gl/program.h | 3 +- src/common/gl/shader_cache.cpp | 94 ++++++++------------- src/common/gl/shader_cache.h | 19 ++--- src/core/gpu_hw_opengl.cpp | 32 ++++--- src/frontend-common/opengl_host_display.cpp | 10 +-- 6 files changed, 83 insertions(+), 119 deletions(-) diff --git a/src/common/gl/program.cpp b/src/common/gl/program.cpp index 756b1dc48..204260eee 100644 --- a/src/common/gl/program.cpp +++ b/src/common/gl/program.cpp @@ -84,43 +84,41 @@ void Program::ResetLastProgram() s_last_program_id = 0; } -bool Program::Compile(const std::string_view vertex_shader, const std::string_view geometry_shader, - const std::string_view fragment_shader) +bool Program::Compile(const std::string_view vertex_shader,const std::string_view fragment_shader) { - GLuint vertex_shader_id = 0; + if (m_vertex_shader_id != 0) + { + glDeleteShader(m_vertex_shader_id); + m_vertex_shader_id = 0; + } + if (m_fragment_shader_id != 0) + { + glDeleteShader(m_fragment_shader_id); + m_fragment_shader_id = 0; + } + if (!vertex_shader.empty()) { - vertex_shader_id = CompileShader(GL_VERTEX_SHADER, vertex_shader); - if (vertex_shader_id == 0) + m_vertex_shader_id = CompileShader(GL_VERTEX_SHADER, vertex_shader); + if (m_vertex_shader_id == 0) return false; } - GLuint geometry_shader_id = 0; - if (!geometry_shader.empty()) - { - geometry_shader_id = CompileShader(GL_GEOMETRY_SHADER, geometry_shader); - if (geometry_shader_id == 0) - return false; - } - - GLuint fragment_shader_id = 0; if (!fragment_shader.empty()) { - fragment_shader_id = CompileShader(GL_FRAGMENT_SHADER, fragment_shader); - if (fragment_shader_id == 0) + m_fragment_shader_id = CompileShader(GL_FRAGMENT_SHADER, fragment_shader); + if (m_fragment_shader_id == 0) { - glDeleteShader(vertex_shader_id); + glDeleteShader(m_fragment_shader_id); return false; } } m_program_id = glCreateProgram(); - if (vertex_shader_id != 0) - glAttachShader(m_program_id, vertex_shader_id); - if (geometry_shader_id != 0) - glAttachShader(m_program_id, geometry_shader_id); - if (fragment_shader_id != 0) - glAttachShader(m_program_id, fragment_shader_id); + if (m_vertex_shader_id != 0) + glAttachShader(m_program_id, m_vertex_shader_id); + if (m_fragment_shader_id != 0) + glAttachShader(m_program_id, m_fragment_shader_id); return true; } diff --git a/src/common/gl/program.h b/src/common/gl/program.h index 8cff51f37..8fe43b507 100644 --- a/src/common/gl/program.h +++ b/src/common/gl/program.h @@ -22,8 +22,7 @@ public: bool IsVaild() const { return m_program_id != 0; } bool IsBound() const { return s_last_program_id == m_program_id; } - bool Compile(const std::string_view vertex_shader, const std::string_view geometry_shader, - const std::string_view fragment_shader); + bool Compile(const std::string_view vertex_shader, const std::string_view fragment_shader); bool CreateFromBinary(const void* data, u32 data_length, u32 data_format); diff --git a/src/common/gl/shader_cache.cpp b/src/common/gl/shader_cache.cpp index 25d4f89b5..286a73cab 100644 --- a/src/common/gl/shader_cache.cpp +++ b/src/common/gl/shader_cache.cpp @@ -9,8 +9,6 @@ #include "../string_util.h" Log_SetChannel(GL::ShaderCache); -namespace GL { - #pragma pack(push, 1) struct CacheIndexEntry { @@ -29,34 +27,30 @@ struct CacheIndexEntry }; #pragma pack(pop) -ShaderCache::ShaderCache() = default; +GL::ShaderCache::ShaderCache() = default; -ShaderCache::~ShaderCache() +GL::ShaderCache::~ShaderCache() { Close(); } -bool ShaderCache::CacheIndexKey::operator==(const CacheIndexKey& key) const +bool GL::ShaderCache::CacheIndexKey::operator==(const CacheIndexKey& key) const { return ( vertex_source_hash_low == key.vertex_source_hash_low && vertex_source_hash_high == key.vertex_source_hash_high && - vertex_source_length == key.vertex_source_length && geometry_source_hash_low == key.geometry_source_hash_low && - geometry_source_hash_high == key.geometry_source_hash_high && - geometry_source_length == key.geometry_source_length && fragment_source_hash_low == key.fragment_source_hash_low && + vertex_source_length == key.vertex_source_length && fragment_source_hash_low == key.fragment_source_hash_low && fragment_source_hash_high == key.fragment_source_hash_high && fragment_source_length == key.fragment_source_length); } -bool ShaderCache::CacheIndexKey::operator!=(const CacheIndexKey& key) const +bool GL::ShaderCache::CacheIndexKey::operator!=(const CacheIndexKey& key) const { return ( vertex_source_hash_low != key.vertex_source_hash_low || vertex_source_hash_high != key.vertex_source_hash_high || - vertex_source_length != key.vertex_source_length || geometry_source_hash_low != key.geometry_source_hash_low || - geometry_source_hash_high != key.geometry_source_hash_high || - geometry_source_length != key.geometry_source_length || fragment_source_hash_low != key.fragment_source_hash_low || + vertex_source_length != key.vertex_source_length || fragment_source_hash_low != key.fragment_source_hash_low || fragment_source_hash_high != key.fragment_source_hash_high || fragment_source_length != key.fragment_source_length); } -void ShaderCache::Open(bool is_gles, std::string_view base_path, u32 version) +void GL::ShaderCache::Open(bool is_gles, std::string_view base_path, u32 version) { m_base_path = base_path; m_version = version; @@ -87,7 +81,7 @@ void ShaderCache::Open(bool is_gles, std::string_view base_path, u32 version) } } -bool ShaderCache::CreateNew(const std::string& index_filename, const std::string& blob_filename) +bool GL::ShaderCache::CreateNew(const std::string& index_filename, const std::string& blob_filename) { if (FileSystem::FileExists(index_filename.c_str())) { @@ -131,7 +125,7 @@ bool ShaderCache::CreateNew(const std::string& index_filename, const std::string return true; } -bool ShaderCache::ReadExisting(const std::string& index_filename, const std::string& blob_filename) +bool GL::ShaderCache::ReadExisting(const std::string& index_filename, const std::string& blob_filename) { m_index_file = FileSystem::OpenCFile(index_filename.c_str(), "r+b"); if (!m_index_file) @@ -178,10 +172,9 @@ bool ShaderCache::ReadExisting(const std::string& index_filename, const std::str return false; } - const CacheIndexKey key{ - entry.vertex_source_hash_low, entry.vertex_source_hash_high, entry.vertex_source_length, - entry.geometry_source_hash_low, entry.geometry_source_hash_high, entry.geometry_source_length, - entry.fragment_source_hash_low, entry.fragment_source_hash_high, entry.fragment_source_length}; + const CacheIndexKey key{entry.vertex_source_hash_low, entry.vertex_source_hash_high, + entry.vertex_source_length, entry.fragment_source_hash_low, + entry.fragment_source_hash_high, entry.fragment_source_length}; const CacheIndexData data{entry.file_offset, entry.blob_size, entry.blob_format}; m_index.emplace(key, data); } @@ -190,7 +183,7 @@ bool ShaderCache::ReadExisting(const std::string& index_filename, const std::str return true; } -void ShaderCache::Close() +void GL::ShaderCache::Close() { m_index.clear(); if (m_index_file) @@ -199,7 +192,7 @@ void ShaderCache::Close() std::fclose(m_blob_file); } -bool ShaderCache::Recreate() +bool GL::ShaderCache::Recreate() { Close(); @@ -209,9 +202,8 @@ bool ShaderCache::Recreate() return CreateNew(index_filename, blob_filename); } -ShaderCache::CacheIndexKey ShaderCache::GetCacheKey(const std::string_view& vertex_shader, - const std::string_view& geometry_shader, - const std::string_view& fragment_shader) +GL::ShaderCache::CacheIndexKey GL::ShaderCache::GetCacheKey(const std::string_view& vertex_shader, + const std::string_view& fragment_shader) { union ShaderHash { @@ -224,7 +216,6 @@ ShaderCache::CacheIndexKey ShaderCache::GetCacheKey(const std::string_view& vert }; ShaderHash vertex_hash = {}; - ShaderHash geometry_hash = {}; ShaderHash fragment_hash = {}; MD5Digest digest; @@ -234,13 +225,6 @@ ShaderCache::CacheIndexKey ShaderCache::GetCacheKey(const std::string_view& vert digest.Final(vertex_hash.bytes); } - if (!geometry_shader.empty()) - { - digest.Reset(); - digest.Update(geometry_shader.data(), static_cast(geometry_shader.length())); - digest.Final(geometry_hash.bytes); - } - if (!fragment_shader.empty()) { digest.Reset(); @@ -249,31 +233,30 @@ ShaderCache::CacheIndexKey ShaderCache::GetCacheKey(const std::string_view& vert } return CacheIndexKey{vertex_hash.low, vertex_hash.high, static_cast(vertex_shader.length()), - geometry_hash.low, geometry_hash.high, static_cast(geometry_shader.length()), fragment_hash.low, fragment_hash.high, static_cast(fragment_shader.length())}; } -std::string ShaderCache::GetIndexFileName() const +std::string GL::ShaderCache::GetIndexFileName() const { return Path::Combine(m_base_path, "gl_programs.idx"); } -std::string ShaderCache::GetBlobFileName() const +std::string GL::ShaderCache::GetBlobFileName() const { return Path::Combine(m_base_path, "gl_programs.bin"); } -std::optional ShaderCache::GetProgram(const std::string_view vertex_shader, - const std::string_view geometry_shader, - const std::string_view fragment_shader, const PreLinkCallback& callback) +std::optional GL::ShaderCache::GetProgram(const std::string_view vertex_shader, + const std::string_view fragment_shader, + const PreLinkCallback& callback) { if (!m_program_binary_supported || !m_blob_file) - return CompileProgram(vertex_shader, geometry_shader, fragment_shader, callback, false); + return CompileProgram(vertex_shader, fragment_shader, callback, false); - const auto key = GetCacheKey(vertex_shader, geometry_shader, fragment_shader); + const auto key = GetCacheKey(vertex_shader, fragment_shader); auto iter = m_index.find(key); if (iter == m_index.end()) - return CompileAndAddProgram(key, vertex_shader, geometry_shader, fragment_shader, callback); + return CompileAndAddProgram(key, vertex_shader, fragment_shader, callback); std::vector data(iter->second.blob_size); if (std::fseek(m_blob_file, iter->second.file_offset, SEEK_SET) != 0 || @@ -290,18 +273,17 @@ std::optional ShaderCache::GetProgram(const std::string_view vertex_sha Log_WarningPrintf( "Failed to create program from binary, this may be due to a driver or GPU Change. Recreating cache."); if (!Recreate()) - return CompileProgram(vertex_shader, geometry_shader, fragment_shader, callback, false); + return CompileProgram(vertex_shader, fragment_shader, callback, false); else - return CompileAndAddProgram(key, vertex_shader, geometry_shader, fragment_shader, callback); + return CompileAndAddProgram(key, vertex_shader, fragment_shader, callback); } -std::optional ShaderCache::CompileProgram(const std::string_view& vertex_shader, - const std::string_view& geometry_shader, - const std::string_view& fragment_shader, - const PreLinkCallback& callback, bool set_retrievable) +std::optional GL::ShaderCache::CompileProgram(const std::string_view& vertex_shader, + const std::string_view& fragment_shader, + const PreLinkCallback& callback, bool set_retrievable) { Program prog; - if (!prog.Compile(vertex_shader, geometry_shader, fragment_shader)) + if (!prog.Compile(vertex_shader, fragment_shader)) return std::nullopt; if (callback) @@ -316,13 +298,12 @@ std::optional ShaderCache::CompileProgram(const std::string_view& verte return std::optional(std::move(prog)); } -std::optional ShaderCache::CompileAndAddProgram(const CacheIndexKey& key, - const std::string_view& vertex_shader, - const std::string_view& geometry_shader, - const std::string_view& fragment_shader, - const PreLinkCallback& callback) +std::optional GL::ShaderCache::CompileAndAddProgram(const CacheIndexKey& key, + const std::string_view& vertex_shader, + const std::string_view& fragment_shader, + const PreLinkCallback& callback) { - std::optional prog = CompileProgram(vertex_shader, geometry_shader, fragment_shader, callback, true); + std::optional prog = CompileProgram(vertex_shader, fragment_shader, callback, true); if (!prog) return std::nullopt; @@ -343,9 +324,6 @@ std::optional ShaderCache::CompileAndAddProgram(const CacheIndexKey& ke entry.vertex_source_hash_low = key.vertex_source_hash_low; entry.vertex_source_hash_high = key.vertex_source_hash_high; entry.vertex_source_length = key.vertex_source_length; - entry.geometry_source_hash_low = key.geometry_source_hash_low; - entry.geometry_source_hash_high = key.geometry_source_hash_high; - entry.geometry_source_length = key.geometry_source_length; entry.fragment_source_hash_low = key.fragment_source_hash_low; entry.fragment_source_hash_high = key.fragment_source_hash_high; entry.fragment_source_length = key.fragment_source_length; @@ -364,5 +342,3 @@ std::optional ShaderCache::CompileAndAddProgram(const CacheIndexKey& ke m_index.emplace(key, data); return prog; } - -} // namespace GL diff --git a/src/common/gl/shader_cache.h b/src/common/gl/shader_cache.h index b89d5f0cb..a65278831 100644 --- a/src/common/gl/shader_cache.h +++ b/src/common/gl/shader_cache.h @@ -25,20 +25,17 @@ public: void Open(bool is_gles, std::string_view base_path, u32 version); - std::optional GetProgram(const std::string_view vertex_shader, const std::string_view geometry_shader, - const std::string_view fragment_shader, const PreLinkCallback& callback = {}); + std::optional GetProgram(const std::string_view vertex_shader, const std::string_view fragment_shader, + const PreLinkCallback& callback = {}); private: - static constexpr u32 FILE_VERSION = 3; + static constexpr u32 FILE_VERSION = 4; struct CacheIndexKey { u64 vertex_source_hash_low; u64 vertex_source_hash_high; u32 vertex_source_length; - u64 geometry_source_hash_low; - u64 geometry_source_hash_high; - u32 geometry_source_length; u64 fragment_source_hash_low; u64 fragment_source_hash_high; u32 fragment_source_length; @@ -53,7 +50,6 @@ private: { std::size_t h = 0; hash_combine(h, e.vertex_source_hash_low, e.vertex_source_hash_high, e.vertex_source_length, - e.geometry_source_hash_low, e.geometry_source_hash_high, e.geometry_source_length, e.fragment_source_hash_low, e.fragment_source_hash_high, e.fragment_source_length); return h; } @@ -68,8 +64,7 @@ private: using CacheIndex = std::unordered_map; - static CacheIndexKey GetCacheKey(const std::string_view& vertex_shader, const std::string_view& geometry_shader, - const std::string_view& fragment_shader); + static CacheIndexKey GetCacheKey(const std::string_view& vertex_shader, const std::string_view& fragment_shader); std::string GetIndexFileName() const; std::string GetBlobFileName() const; @@ -79,11 +74,9 @@ private: void Close(); bool Recreate(); - std::optional CompileProgram(const std::string_view& vertex_shader, const std::string_view& geometry_shader, - const std::string_view& fragment_shader, const PreLinkCallback& callback, - bool set_retrievable); + std::optional CompileProgram(const std::string_view& vertex_shader, const std::string_view& fragment_shader, + const PreLinkCallback& callback, bool set_retrievable); std::optional CompileAndAddProgram(const CacheIndexKey& key, const std::string_view& vertex_shader, - const std::string_view& geometry_shader, const std::string_view& fragment_shader, const PreLinkCallback& callback); std::string m_base_path; diff --git a/src/core/gpu_hw_opengl.cpp b/src/core/gpu_hw_opengl.cpp index 72de52f6d..214307a25 100644 --- a/src/core/gpu_hw_opengl.cpp +++ b/src/core/gpu_hw_opengl.cpp @@ -541,7 +541,7 @@ bool GPU_HW_OpenGL::CompilePrograms() } }; - std::optional prog = shader_cache.GetProgram(batch_vs, {}, fs, link_callback); + std::optional prog = shader_cache.GetProgram(batch_vs, fs, link_callback); if (!prog) return false; @@ -571,11 +571,10 @@ bool GPU_HW_OpenGL::CompilePrograms() const std::string fs = shadergen.GenerateDisplayFragmentShader( ConvertToBoolUnchecked(depth_24bit), static_cast(interlaced), m_chroma_smoothing); - std::optional prog = - shader_cache.GetProgram(vs, {}, fs, [this, use_binding_layout](GL::Program& prog) { - if (!IsGLES() && !use_binding_layout) - prog.BindFragData(0, "o_col0"); - }); + std::optional prog = shader_cache.GetProgram(vs, fs, [this, use_binding_layout](GL::Program& prog) { + if (!IsGLES() && !use_binding_layout) + prog.BindFragData(0, "o_col0"); + }); if (!prog) return false; @@ -595,7 +594,7 @@ bool GPU_HW_OpenGL::CompilePrograms() for (u8 interlaced = 0; interlaced < 2; interlaced++) { std::optional prog = shader_cache.GetProgram( - shadergen.GenerateScreenQuadVertexShader(), {}, + shadergen.GenerateScreenQuadVertexShader(), shadergen.GenerateVRAMFillFragmentShader(ConvertToBoolUnchecked(wrapped), ConvertToBoolUnchecked(interlaced)), [this, use_binding_layout](GL::Program& prog) { if (!IsGLES() && !use_binding_layout) @@ -613,7 +612,7 @@ bool GPU_HW_OpenGL::CompilePrograms() } std::optional prog = - shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), {}, shadergen.GenerateVRAMReadFragmentShader(), + shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), shadergen.GenerateVRAMReadFragmentShader(), [this, use_binding_layout](GL::Program& prog) { if (!IsGLES() && !use_binding_layout) prog.BindFragData(0, "o_col0"); @@ -630,12 +629,11 @@ bool GPU_HW_OpenGL::CompilePrograms() m_vram_read_program = std::move(*prog); progress.Increment(); - prog = - shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), {}, shadergen.GenerateVRAMCopyFragmentShader(), - [this, use_binding_layout](GL::Program& prog) { - if (!IsGLES() && !use_binding_layout) - prog.BindFragData(0, "o_col0"); - }); + prog = shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), shadergen.GenerateVRAMCopyFragmentShader(), + [this, use_binding_layout](GL::Program& prog) { + if (!IsGLES() && !use_binding_layout) + prog.BindFragData(0, "o_col0"); + }); if (!prog) return false; @@ -648,7 +646,7 @@ bool GPU_HW_OpenGL::CompilePrograms() m_vram_copy_program = std::move(*prog); progress.Increment(); - prog = shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), {}, + prog = shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), shadergen.GenerateVRAMUpdateDepthFragmentShader()); if (!prog) return false; @@ -660,7 +658,7 @@ bool GPU_HW_OpenGL::CompilePrograms() if (m_use_texture_buffer_for_vram_writes || m_use_ssbo_for_vram_writes) { - prog = shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), {}, + prog = shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), shadergen.GenerateVRAMWriteFragmentShader(m_use_ssbo_for_vram_writes), [this, use_binding_layout](GL::Program& prog) { if (!IsGLES() && !use_binding_layout) @@ -682,7 +680,7 @@ bool GPU_HW_OpenGL::CompilePrograms() if (m_downsample_mode == GPUDownsampleMode::Box) { - prog = shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), {}, + prog = shader_cache.GetProgram(shadergen.GenerateScreenQuadVertexShader(), shadergen.GenerateBoxSampleDownsampleFragmentShader(), [this, use_binding_layout](GL::Program& prog) { if (!IsGLES() && !use_binding_layout) diff --git a/src/frontend-common/opengl_host_display.cpp b/src/frontend-common/opengl_host_display.cpp index 8546927b2..d9e120fe4 100644 --- a/src/frontend-common/opengl_host_display.cpp +++ b/src/frontend-common/opengl_host_display.cpp @@ -490,9 +490,9 @@ void main() } )"; - if (!m_display_program.Compile(GetGLSLVersionHeader() + fullscreen_quad_vertex_shader, {}, + if (!m_display_program.Compile(GetGLSLVersionHeader() + fullscreen_quad_vertex_shader, GetGLSLVersionHeader() + display_fragment_shader) || - !m_cursor_program.Compile(GetGLSLVersionHeader() + fullscreen_quad_vertex_shader, {}, + !m_cursor_program.Compile(GetGLSLVersionHeader() + fullscreen_quad_vertex_shader, GetGLSLVersionHeader() + cursor_fragment_shader)) { Log_ErrorPrintf("Failed to compile display shaders"); @@ -586,8 +586,8 @@ void main() } )"; - if (!m_display_program.Compile(fullscreen_quad_vertex_shader, {}, display_fragment_shader) || - !m_cursor_program.Compile(fullscreen_quad_vertex_shader, {}, cursor_fragment_shader)) + if (!m_display_program.Compile(fullscreen_quad_vertex_shader, display_fragment_shader) || + !m_cursor_program.Compile(fullscreen_quad_vertex_shader, cursor_fragment_shader)) { Log_ErrorPrintf("Failed to compile display shaders"); return false; @@ -879,7 +879,7 @@ bool OpenGLHostDisplay::SetPostProcessingChain(const std::string_view& config) PostProcessingStage stage; stage.uniforms_size = shader.GetUniformsSize(); - if (!stage.program.Compile(vs, {}, ps)) + if (!stage.program.Compile(vs, ps)) { Log_InfoPrintf("Failed to compile post-processing program, disabling."); m_post_processing_stages.clear();