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

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<IRenderBufferPool> 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];

xbmc/cores/RetroPlayer/shaders/gl/ShaderGL.cpp

@@ -8,27 +8,27 @@
 
 #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)
 {
 }
 
 bool CShaderGL::Create(const std::string& shaderSource, const std::string& shaderPath, ShaderParameterMap shaderParameters,
         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,15 +243,15 @@ 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();
   UpdateInputBuffer(0);
   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;
 }

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 <stdint.h>
 
@@ -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<std::array<GLfloat, 4>, 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);

xbmc/cores/RetroPlayer/shaders/gl/ShaderLutGL.cpp

@@ -48,7 +48,7 @@ std::unique_ptr<IShaderTexture> CShaderLutGL::CreateLUTTexture(RETRO::CRenderCon
 
   CGLTexture* texture = static_cast<CGLTexture*>(CGLTexture::LoadFromFile(lut.path));
 
-  if(!texture)
+  if (!texture)
   {
     CLog::Log(LOGERROR, "Couldn't open LUT %s", lut.path);
     return std::unique_ptr<IShaderTexture>();
@@ -68,8 +68,8 @@ std::unique_ptr<IShaderTexture> 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<IShaderTexture>(new CShaderTextureGL(texture));
-}
+}

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 <xbmc/utils/log.h>
 
 #include <regex>
-#include <xbmc/rendering/gl/RenderSystemGL.h>
 
 #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();
 }
-
-
-
-
-
-
-
-

xbmc/cores/RetroPlayer/shaders/gl/ShaderTextureGL.cpp

@@ -6,8 +6,8 @@
  *  See LICENSES/README.md for more information.
  */
 
-#include <xbmc/utils/log.h>
 #include "ShaderTextureGL.h"
+#include "utils/log.h"
 
 using namespace KODI;
 using namespace SHADER;

xbmc/cores/RetroPlayer/shaders/gl/ShaderUtilsGL.cpp

@@ -6,9 +6,10 @@
  *  See LICENSES/README.md for more information.
  */
 
+#include "ShaderUtilsGL.h"
+
 #include <fstream>
 #include <sstream>
-#include "ShaderUtilsGL.h"
 
 using namespace KODI;
 using namespace SHADER;

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);
 };
+
 }
 }