From a67df9865e03e3e1bc0603127907b7a3fda0070c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 16 Jun 2020 18:22:33 -0400 Subject: [PATCH] CShader: Make use of unique_ptr where applicable Prevents unsafe allocations by default. --- src/Core/OpenGL/CShader.cpp | 75 ++++++++++++++++------------------- src/Core/OpenGL/CShader.h | 10 +++-- src/Core/Render/CDrawUtil.cpp | 30 +++++++------- src/Core/Render/CDrawUtil.h | 14 +++---- 4 files changed, 62 insertions(+), 67 deletions(-) diff --git a/src/Core/OpenGL/CShader.cpp b/src/Core/OpenGL/CShader.cpp index 08cc52d8..6f80732c 100644 --- a/src/Core/OpenGL/CShader.cpp +++ b/src/Core/OpenGL/CShader.cpp @@ -5,14 +5,10 @@ #include #include -#include -bool gDebugDumpShaders = false; -uint64 gFailedCompileCount = 0; -uint64 gSuccessfulCompileCount = 0; - -CShader* CShader::spCurrentShader = nullptr; -int CShader::smNumShaders = 0; +static bool gDebugDumpShaders = false; +static uint64 gFailedCompileCount = 0; +static uint64 gSuccessfulCompileCount = 0; CShader::CShader() { @@ -30,18 +26,25 @@ CShader::CShader(const char *pkVertexSource, const char *pkPixelSource) CShader::~CShader() { - if (mVertexShaderExists) glDeleteShader(mVertexShader); - if (mPixelShaderExists) glDeleteShader(mPixelShader); - if (mProgramExists) glDeleteProgram(mProgram); + if (mVertexShaderExists) + glDeleteShader(mVertexShader); + + if (mPixelShaderExists) + glDeleteShader(mPixelShader); + + if (mProgramExists) + glDeleteProgram(mProgram); + + if (spCurrentShader == this) + spCurrentShader = nullptr; - if (spCurrentShader == this) spCurrentShader = 0; smNumShaders--; } bool CShader::CompileVertexSource(const char* pkSource) { mVertexShader = glCreateShader(GL_VERTEX_SHADER); - glShaderSource(mVertexShader, 1, (const GLchar**) &pkSource, NULL); + glShaderSource(mVertexShader, 1, (const GLchar**) &pkSource, nullptr); glCompileShader(mVertexShader); // Shader should be compiled - check for errors @@ -76,7 +79,7 @@ bool CShader::CompileVertexSource(const char* pkSource) bool CShader::CompilePixelSource(const char* pkSource) { mPixelShader = glCreateShader(GL_FRAGMENT_SHADER); - glShaderSource(mPixelShader, 1, (const GLchar**) &pkSource, NULL); + glShaderSource(mPixelShader, 1, (const GLchar**) &pkSource, nullptr); glCompileShader(mPixelShader); // Shader should be compiled - check for errors @@ -110,7 +113,8 @@ bool CShader::CompilePixelSource(const char* pkSource) bool CShader::LinkShaders() { - if ((!mVertexShaderExists) || (!mPixelShaderExists)) return false; + if (!mVertexShaderExists || !mPixelShaderExists) + return false; mProgram = glCreateProgram(); glAttachShader(mProgram, mVertexShader); @@ -133,17 +137,12 @@ bool CShader::LinkShaders() GLint LogLen; glGetProgramiv(mProgram, GL_INFO_LOG_LENGTH, &LogLen); - GLchar *pInfoLog = new GLchar[LogLen]; - glGetProgramInfoLog(mProgram, LogLen, NULL, pInfoLog); - - std::ofstream LinkOut; - LinkOut.open(*Out); + auto pInfoLog = std::unique_ptr(new GLchar[LogLen]); + glGetProgramInfoLog(mProgram, LogLen, nullptr, pInfoLog.get()); + std::ofstream LinkOut(*Out); if (LogLen > 0) - LinkOut << pInfoLog; - - LinkOut.close(); - delete[] pInfoLog; + LinkOut << pInfoLog.get(); gFailedCompileCount++; glDeleteProgram(mProgram); @@ -214,7 +213,7 @@ void CShader::SetCurrent() } // ************ STATIC ************ -CShader* CShader::FromResourceFile(const TString& rkShaderName) +std::unique_ptr CShader::FromResourceFile(const TString& rkShaderName) { TString VertexShaderFilename = gDataDir + "resources/shaders/" + rkShaderName + ".vs"; TString PixelShaderFilename = gDataDir + "resources/shaders/" + rkShaderName + ".ps"; @@ -227,7 +226,7 @@ CShader* CShader::FromResourceFile(const TString& rkShaderName) if (VertexShaderText.IsEmpty() || PixelShaderText.IsEmpty()) return nullptr; - CShader *pShader = new CShader(); + auto pShader = std::make_unique(); pShader->CompileVertexSource(*VertexShaderText); pShader->CompilePixelSource(*PixelShaderText); pShader->LinkShaders(); @@ -241,7 +240,7 @@ CShader* CShader::CurrentShader() void CShader::KillCachedShader() { - spCurrentShader = 0; + spCurrentShader = nullptr; } // ************ PRIVATE ************ @@ -258,26 +257,20 @@ void CShader::CacheCommonUniforms() void CShader::DumpShaderSource(GLuint Shader, const TString& rkOut) { - GLint SourceLen; + GLint SourceLen = 0; glGetShaderiv(Shader, GL_SHADER_SOURCE_LENGTH, &SourceLen); - GLchar *Source = new GLchar[SourceLen]; - glGetShaderSource(Shader, SourceLen, NULL, Source); + auto Source = std::unique_ptr(new GLchar[SourceLen]); + glGetShaderSource(Shader, SourceLen, nullptr, Source.get()); - GLint LogLen; + GLint LogLen = 0; glGetShaderiv(Shader, GL_INFO_LOG_LENGTH, &LogLen); - GLchar *pInfoLog = new GLchar[LogLen]; - glGetShaderInfoLog(Shader, LogLen, NULL, pInfoLog); + auto pInfoLog = std::unique_ptr(new GLchar[LogLen]); + glGetShaderInfoLog(Shader, LogLen, nullptr, pInfoLog.get()); - std::ofstream ShaderOut; - ShaderOut.open(*rkOut); + std::ofstream ShaderOut(*rkOut); if (SourceLen > 0) - ShaderOut << Source; + ShaderOut << Source.get(); if (LogLen > 0) - ShaderOut << pInfoLog; - - ShaderOut.close(); - - delete[] Source; - delete[] pInfoLog; + ShaderOut << pInfoLog.get(); } diff --git a/src/Core/OpenGL/CShader.h b/src/Core/OpenGL/CShader.h index b0334b18..6716c392 100644 --- a/src/Core/OpenGL/CShader.h +++ b/src/Core/OpenGL/CShader.h @@ -3,6 +3,8 @@ #include #include +#include +#include class CShader { @@ -20,11 +22,11 @@ class CShader GLuint mBoneTransformBlockIndex = 0; // Cached uniform locations - GLint mTextureUniforms[8] = {}; + std::array mTextureUniforms{}; GLint mNumLightsUniform = 0; - static int smNumShaders; - static CShader* spCurrentShader; + static inline int smNumShaders = 0; + static inline CShader* spCurrentShader = nullptr; public: CShader(); @@ -43,7 +45,7 @@ public: void SetCurrent(); // Static - static CShader* FromResourceFile(const TString& rkShaderName); + static std::unique_ptr FromResourceFile(const TString& rkShaderName); static CShader* CurrentShader(); static void KillCachedShader(); diff --git a/src/Core/Render/CDrawUtil.cpp b/src/Core/Render/CDrawUtil.cpp index 6c8f2ada..fe391689 100644 --- a/src/Core/Render/CDrawUtil.cpp +++ b/src/Core/Render/CDrawUtil.cpp @@ -318,7 +318,7 @@ void CDrawUtil::UseCollisionShader(bool IsFloor, bool IsUnstandable, const CColo CShader* CDrawUtil::GetTextShader() { Init(); - return mpTextShader; + return mpTextShader.get(); } void CDrawUtil::LoadCheckerboardTexture(uint32 GLTextureUnit) @@ -553,18 +553,18 @@ void CDrawUtil::InitTextures() void CDrawUtil::Shutdown() { - if (mDrawUtilInitialized) - { - debugf("Shutting down"); - mGridVertices = std::nullopt; - mSquareVertices = std::nullopt; - mLineVertices = std::nullopt; - mWireCubeVertices = std::nullopt; - delete mpColorShader; - delete mpColorShaderLighting; - delete mpTextureShader; - delete mpCollisionShader; - delete mpTextShader; - mDrawUtilInitialized = false; - } + if (!mDrawUtilInitialized) + return; + + debugf("Shutting down"); + mGridVertices.reset(); + mSquareVertices.reset(); + mLineVertices.reset(); + mWireCubeVertices.reset(); + mpColorShader.reset(); + mpColorShaderLighting.reset(); + mpTextureShader.reset(); + mpCollisionShader.reset(); + mpTextShader.reset(); + mDrawUtilInitialized = false; } diff --git a/src/Core/Render/CDrawUtil.h b/src/Core/Render/CDrawUtil.h index c90e33c4..28356e07 100644 --- a/src/Core/Render/CDrawUtil.h +++ b/src/Core/Render/CDrawUtil.h @@ -46,13 +46,13 @@ class CDrawUtil static inline TResPtr mpWireSphereModel; // Shaders - static inline CShader *mpColorShader = nullptr; - static inline CShader *mpColorShaderLighting = nullptr; - static inline CShader *mpBillboardShader = nullptr; - static inline CShader *mpLightBillboardShader = nullptr; - static inline CShader *mpTextureShader = nullptr; - static inline CShader *mpCollisionShader = nullptr; - static inline CShader *mpTextShader = nullptr; + static inline std::unique_ptr mpColorShader; + static inline std::unique_ptr mpColorShaderLighting; + static inline std::unique_ptr mpBillboardShader; + static inline std::unique_ptr mpLightBillboardShader; + static inline std::unique_ptr mpTextureShader; + static inline std::unique_ptr mpCollisionShader; + static inline std::unique_ptr mpTextShader; // Textures static inline TResPtr mpCheckerTexture;