From def1361aca1a92196e2e4f2686ed78919711270b Mon Sep 17 00:00:00 2001 From: Garrett Brown Date: Sun, 1 Sep 2019 18:10:51 -0700 Subject: [PATCH] Refactor GL Shaders Documentation fix Code style and whitespace fixes Fix log line Fix memory leak Use empty() instead of size() Fix initialization warning Warning was: Member 'VAO' was not initialized in this constructor Fix order of #includes --- .../VideoRenderers/RPRendererOpenGL.cpp | 13 +++++++----- .../cores/RetroPlayer/shaders/gl/ShaderGL.cpp | 19 +++++++++--------- xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.h | 8 +++++--- .../RetroPlayer/shaders/gl/ShaderLutGL.cpp | 6 +++--- .../RetroPlayer/shaders/gl/ShaderPresetGL.cpp | 20 +++++-------------- .../shaders/gl/ShaderTextureGL.cpp | 2 +- .../RetroPlayer/shaders/gl/ShaderUtilsGL.cpp | 3 ++- .../RetroPlayer/shaders/gl/ShaderUtilsGL.h | 1 + 8 files changed, 35 insertions(+), 37 deletions(-) diff --git a/xbmc/cores/RetroPlayer/rendering/VideoRenderers/RPRendererOpenGL.cpp b/xbmc/cores/RetroPlayer/rendering/VideoRenderers/RPRendererOpenGL.cpp index 22bd9dd897f37..4cdb706bed375 100644 --- a/xbmc/cores/RetroPlayer/rendering/VideoRenderers/RPRendererOpenGL.cpp +++ b/xbmc/cores/RetroPlayer/rendering/VideoRenderers/RPRendererOpenGL.cpp @@ -40,9 +40,11 @@ RenderBufferPoolVector CRendererFactoryOpenGL::CreateBufferPools(CRenderContext CRPRendererOpenGL::CRPRendererOpenGL(const CRenderSettings &renderSettings, CRenderContext &context, std::shared_ptr bufferPool) : CRPBaseRenderer(renderSettings, context, std::move(bufferPool)) { + // Initialize CRPBaseRenderer + m_shaderPreset.reset(new SHADER::CShaderPresetGL(m_context)); + // Initialize CRPRendererOpenGL m_clearColour = m_context.UseLimitedColor() ? (16.0f / 0xff) : 0.0f; - m_shaderPreset.reset(new SHADER::CShaderPresetGL(m_context)); } CRPRendererOpenGL::~CRPRendererOpenGL() = default; @@ -324,15 +326,16 @@ void CRPRendererOpenGL::Render(uint8_t alpha) renderTargetTexture->CreateTextureObject(); - auto source = new SHADER::CShaderTextureGL(*sourceTexture); - auto target = new SHADER::CShaderTextureGL(*renderTargetTexture); - if (!m_shaderPreset->RenderUpdate(destPoints, source, target)) + SHADER::CShaderTextureGL source(*sourceTexture); + SHADER::CShaderTextureGL target(*renderTargetTexture); + if (!m_shaderPreset->RenderUpdate(destPoints, &source, &target)) { m_shadersNeedUpdate = false; m_bUseShaderPreset = false; } } - else { + else + { m_context.EnableGUIShader(GL_SHADER_METHOD::TEXTURE); GLubyte colour[4]; diff --git a/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.cpp b/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.cpp index c792c99e1d3c9..585d4752e0603 100644 --- a/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.cpp +++ b/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.cpp @@ -8,19 +8,19 @@ #include "ShaderGL.h" #include "ShaderTextureGL.h" -#include "Application.h" +#include "ShaderUtilsGL.h" #include "cores/RetroPlayer/rendering/RenderContext.h" #include "cores/RetroPlayer/shaders/IShaderLut.h" #include "rendering/gl/RenderSystemGL.h" #include "utils/log.h" #include "utils/URIUtils.h" -#include "ShaderUtilsGL.h" +#include "Application.h" using namespace KODI; using namespace SHADER; CShaderGL::CShaderGL(RETRO::CRenderContext &context) : - m_context(context) + m_context(context) { } @@ -28,7 +28,7 @@ bool CShaderGL::Create(const std::string& shaderSource, const std::string& shade IShaderSampler* sampler, ShaderLutVec luts, float2 viewPortSize, unsigned frameCountMod) { - //TODO:Remove sampler input from IShader.h + // TODO:Remove sampler input from IShader.h if(shaderPath.empty()) { CLog::Log(LOGERROR, "ShaderGL: Can't load empty shader path"); @@ -45,7 +45,7 @@ bool CShaderGL::Create(const std::string& shaderSource, const std::string& shade std::string defineVertex = "#define VERTEX\n"; std::string defineFragment; - if (m_shaderParameters.size() == 0) + if (m_shaderParameters.empty()) defineFragment = "#define FRAGMENT\n"; else defineFragment = "#define FRAGMENT\n#define PARAMETER_UNIFORM\n"; @@ -161,7 +161,7 @@ void CShaderGL::PrepareParameters(CPoint *dest, bool isLastPass, uint64_t frameC m_VertexCoords[3][0] = -m_outputSize.x / 2; m_VertexCoords[3][1] = m_outputSize.y / 2; } - else // last pass + else // last pass { // bottom left x,y m_VertexCoords[0][0] = dest[3].x - m_outputSize.x / 2; @@ -243,7 +243,7 @@ void CShaderGL::GetUniformLocs() m_MVPMatrixLoc = glGetUniformLocation(m_shaderProgram, "MVPMatrix"); } -//TODO:Change name of this method in IShader.h to CreateInputs +// TODO:Change name of this method in IShader.h to CreateInputs bool CShaderGL::CreateInputBuffer() { GetUniformLocs(); @@ -251,7 +251,7 @@ bool CShaderGL::CreateInputBuffer() return true; } -//TODO:Change name of this method in IShader.h to UpdateInputs +// TODO:Change name of this method in IShader.h to UpdateInputs void CShaderGL::UpdateInputBuffer(uint64_t frameCount) { glUseProgram(m_shaderProgram); @@ -290,6 +290,7 @@ void CShaderGL::SetSizes(const float2 &prevSize, const float2 &nextSize) m_outputSize = nextSize; } -bool CShaderGL::CreateVertexBuffer(unsigned vertCount, unsigned vertSize) { +bool CShaderGL::CreateVertexBuffer(unsigned vertCount, unsigned vertSize) +{ return false; } diff --git a/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.h b/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.h index 5395870e2ac21..298b3cc6d8193 100644 --- a/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.h +++ b/xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.h @@ -11,8 +11,8 @@ #include "ShaderTextureGL.h" #include "cores/RetroPlayer/shaders/IShader.h" #include "cores/RetroPlayer/shaders/gl/ShaderTypesGL.h" -#include "rendering/gl/GLShader.h" #include "guilib/TextureGL.h" +#include "rendering/gl/GLShader.h" #include @@ -82,7 +82,7 @@ class CShaderGL : public IShader // Resolution of the texture that holds the input //float2 m_textureSize; - GLuint m_shaderProgram; + GLuint m_shaderProgram = 0; // Projection matrix std::array, 4> m_MVP; @@ -103,7 +103,9 @@ class CShaderGL : public IShader GLint m_InputSizeLoc = -1; GLint m_MVPMatrixLoc = -1; - GLuint VAO, EBO, VBO[3]; + GLuint VAO = 0; + GLuint EBO = 0; + GLuint VBO[3] = {}; private: uniformInputs GetInputData(uint64_t frameCount = 0); diff --git a/xbmc/cores/RetroPlayer/shaders/gl/ShaderLutGL.cpp b/xbmc/cores/RetroPlayer/shaders/gl/ShaderLutGL.cpp index 2e90287421353..d68a2f16bb5ae 100644 --- a/xbmc/cores/RetroPlayer/shaders/gl/ShaderLutGL.cpp +++ b/xbmc/cores/RetroPlayer/shaders/gl/ShaderLutGL.cpp @@ -48,7 +48,7 @@ std::unique_ptr CShaderLutGL::CreateLUTTexture(RETRO::CRenderCon CGLTexture* texture = static_cast(CGLTexture::LoadFromFile(lut.path)); - if(!texture) + if (!texture) { CLog::Log(LOGERROR, "Couldn't open LUT %s", lut.path); return std::unique_ptr(); @@ -68,8 +68,8 @@ std::unique_ptr CShaderLutGL::CreateLUTTexture(RETRO::CRenderCon GLfloat blackBorder[4] = { 0.0f, 0.0f, 0.0f, 0.0f }; glTexParameterfv(GL_TEXTURE_2D, GL_TEXTURE_BORDER_COLOR, blackBorder); - if(lut.mipmap) + if (lut.mipmap) texture->SetMipmapping(); return std::unique_ptr(new CShaderTextureGL(texture)); -} \ No newline at end of file +} diff --git a/xbmc/cores/RetroPlayer/shaders/gl/ShaderPresetGL.cpp b/xbmc/cores/RetroPlayer/shaders/gl/ShaderPresetGL.cpp index 1ff2c14391e1b..7d70e03dfeb4d 100644 --- a/xbmc/cores/RetroPlayer/shaders/gl/ShaderPresetGL.cpp +++ b/xbmc/cores/RetroPlayer/shaders/gl/ShaderPresetGL.cpp @@ -12,19 +12,17 @@ #include "cores/RetroPlayer/rendering/RenderContext.h" #include "cores/RetroPlayer/shaders/ShaderPresetFactory.h" #include "cores/RetroPlayer/shaders/gl/ShaderGL.h" +#include "rendering/gl/RenderSystemGL.h" +#include "utils/log.h" #include "ServiceBroker.h" -#include #include -#include #define MAX_FLOAT 3.402823466E+38 - using namespace KODI; using namespace SHADER; - CShaderPresetGL::CShaderPresetGL(RETRO::CRenderContext &context, unsigned int videoWidth, unsigned int videoHeight) : m_context(context) , m_videoSize(videoWidth, videoHeight) @@ -153,13 +151,14 @@ bool CShaderPresetGL::Update() auto updateFailed = [this](const std::string& msg) { m_failedPaths.insert(m_presetPath); - auto message = "CShaderPresetDX::Update: " + msg + ". Disabling video shaders."; + auto message = "CShaderPresetGL::Update: " + msg + ". Disabling video shaders."; CLog::Log(LOGWARNING, message.c_str()); DisposeShaders(); return false; }; - if (m_bPresetNeedsUpdate && !HasPathFailed(m_presetPath)) { + if (m_bPresetNeedsUpdate && !HasPathFailed(m_presetPath)) + { DisposeShaders(); if (m_presetPath.empty()) @@ -195,7 +194,6 @@ void CShaderPresetGL::SetVideoSize(const unsigned videoWidth, const unsigned vid { m_videoSize = {videoWidth, videoHeight}; m_textureSize = CShaderUtils::GetOptimalTextureSize(m_videoSize); - } bool CShaderPresetGL::SetShaderPreset(const std::string &shaderPresetPath) @@ -447,11 +445,3 @@ bool CShaderPresetGL::HasPathFailed(const std::string &path) const { return m_failedPaths.find(path) != m_failedPaths.end(); } - - - - - - - - diff --git a/xbmc/cores/RetroPlayer/shaders/gl/ShaderTextureGL.cpp b/xbmc/cores/RetroPlayer/shaders/gl/ShaderTextureGL.cpp index 93379e293dd23..a175a42e02483 100644 --- a/xbmc/cores/RetroPlayer/shaders/gl/ShaderTextureGL.cpp +++ b/xbmc/cores/RetroPlayer/shaders/gl/ShaderTextureGL.cpp @@ -6,8 +6,8 @@ * See LICENSES/README.md for more information. */ -#include #include "ShaderTextureGL.h" +#include "utils/log.h" using namespace KODI; using namespace SHADER; diff --git a/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.cpp b/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.cpp index 9908c584468cc..889642f1a4428 100644 --- a/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.cpp +++ b/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.cpp @@ -6,9 +6,10 @@ * See LICENSES/README.md for more information. */ +#include "ShaderUtilsGL.h" + #include #include -#include "ShaderUtilsGL.h" using namespace KODI; using namespace SHADER; diff --git a/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.h b/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.h index 95cdb5fc926e7..bbd4093330c4e 100644 --- a/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.h +++ b/xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.h @@ -23,5 +23,6 @@ class CShaderUtilsGL static GLint TranslateWrapType(WRAP_TYPE wrap); static void MoveVersionToFirstLine(std::string& source, std::string& defineVertex, std::string& defineFragment); }; + } }