From 12d38a0114657cc468e93f7eb0f97b2b1ac8f924 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sun, 24 Jan 2021 13:16:05 +0000 Subject: Big tidy up of shader wrapper --- Jamroot.jam | 1 + application/main.cpp | 2 +- gfx/gl/shader.cpp | 59 +++++++++++++++++++--------------------------------- gfx/gl/shader.h | 32 +++++++++++++++------------- utility/glRef.hpp | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 54 deletions(-) create mode 100644 utility/glRef.hpp diff --git a/Jamroot.jam b/Jamroot.jam index 397a167..9b79df6 100644 --- a/Jamroot.jam +++ b/Jamroot.jam @@ -26,6 +26,7 @@ project : requirements tidy:hicpp-* tidy:hicpp-signed-bitwise tidy:hicpp-named-parameter + tidy:hicpp-no-array-decay tidy:performance-* tidy:no tidy:iwyu.json diff --git a/application/main.cpp b/application/main.cpp index ed3c1c7..63ceac0 100644 --- a/application/main.cpp +++ b/application/main.cpp @@ -66,7 +66,7 @@ public: World world; world.create(); - Shader shader("./res/basicShader"); + Shader shader; Camera camera({0.0F, 0.0F, -5.0F}, 70.0F, (float)DISPLAY_WIDTH / (float)DISPLAY_HEIGHT, 0.1F, 100.0F); SDL_Event e; diff --git a/gfx/gl/shader.cpp b/gfx/gl/shader.cpp index 33fdf61..e59d431 100644 --- a/gfx/gl/shader.cpp +++ b/gfx/gl/shader.cpp @@ -1,20 +1,16 @@ #include "shader.h" #include "transform.h" +#include #include #include #include #include +#include -Shader::Shader(const std::string &) : - m_program {glCreateProgram()}, m_shaders {CreateShader( - (GLchar *)(basicShader_vs), basicShader_vs_len, GL_VERTEX_SHADER), - CreateShader( - (GLchar *)basicShader_fs, basicShader_fs_len, GL_FRAGMENT_SHADER)}, - m_uniforms {} +Shader::Shader() : mvp_uniform {}, normal_uniform {}, lightDir_uniform {} { - for (auto m_shader : m_shaders) { - glAttachShader(m_program, m_shader); - } + glAttachShader(m_program, Source {{basicShader_vs, basicShader_vs_len}, GL_VERTEX_SHADER}.id); + glAttachShader(m_program, Source {{basicShader_fs, basicShader_fs_len}, GL_FRAGMENT_SHADER}.id); glBindAttribLocation(m_program, 0, "position"); glBindAttribLocation(m_program, 1, "texCoord"); @@ -26,18 +22,9 @@ Shader::Shader(const std::string &) : glValidateProgram(m_program); CheckShaderError(m_program, GL_VALIDATE_STATUS, true, "Invalid shader program"); - m_uniforms = {glGetUniformLocation(m_program, "MVP"), glGetUniformLocation(m_program, "Normal"), - glGetUniformLocation(m_program, "lightDirection")}; -} - -Shader::~Shader() -{ - for (auto m_shader : m_shaders) { - glDetachShader(m_program, m_shader); - glDeleteShader(m_shader); - } - - glDeleteProgram(m_program); + mvp_uniform = glGetUniformLocation(m_program, "MVP"); + normal_uniform = glGetUniformLocation(m_program, "Normal"); + lightDir_uniform = glGetUniformLocation(m_program, "lightDirection"); } void @@ -52,13 +39,13 @@ Shader::Update(const Transform & transform, const Camera & camera) const glm::mat4 MVP = transform.GetMVP(camera); glm::mat4 Normal = transform.GetModel(); - glUniformMatrix4fv(m_uniforms[0], 1, GL_FALSE, &MVP[0][0]); - glUniformMatrix4fv(m_uniforms[1], 1, GL_FALSE, &Normal[0][0]); - glUniform3f(m_uniforms[2], 0.0F, 0.0F, 1.0F); + glUniformMatrix4fv(mvp_uniform, 1, GL_FALSE, &MVP[0][0]); + glUniformMatrix4fv(normal_uniform, 1, GL_FALSE, &Normal[0][0]); + glUniform3f(lightDir_uniform, 0.0F, 0.0F, 1.0F); } void -Shader::CheckShaderError(GLuint shader, GLuint flag, bool isProgram, const std::string & errorMessage) +Shader::CheckShaderError(GLuint shader, GLuint flag, bool isProgram, std::string_view errorMessage) { GLint success = 0; std::array error {}; @@ -78,23 +65,19 @@ Shader::CheckShaderError(GLuint shader, GLuint flag, bool isProgram, const std:: glGetShaderInfoLog(shader, error.size(), nullptr, error.data()); } - throw std::runtime_error {errorMessage + ": '" + std::string {error.data(), error.size()} + "'"}; + throw std::runtime_error {std::string {errorMessage} + ": '" + std::string {error.data(), error.size()} + "'"}; } } -GLuint -Shader::CreateShader(const GLchar * text, GLint len, unsigned int type) +Shader::Source::Source(const std::basic_string_view text, GLuint type) : + Source {(const GLchar *)(text.data()), (GLint)(text.length()), type} { - GLuint shader = glCreateShader(type); - - if (shader == 0) { - throw std::runtime_error {"Error compiling shader type " + std::to_string(type)}; - } - - glShaderSource(shader, 1, &text, &len); - glCompileShader(shader); +} - CheckShaderError(shader, GL_COMPILE_STATUS, false, "Error compiling shader!"); +Shader::Source::Source(const GLchar * text, GLint len, unsigned int type) : id {type} +{ + glShaderSource(id, 1, &text, &len); + glCompileShader(id); - return shader; + CheckShaderError(id, GL_COMPILE_STATUS, false, "Error compiling shader!"); } diff --git a/gfx/gl/shader.h b/gfx/gl/shader.h index 2072199..e253f50 100644 --- a/gfx/gl/shader.h +++ b/gfx/gl/shader.h @@ -2,34 +2,36 @@ #define SHADER_INCLUDED_H #include -#include -#include -#include +#include +#include class Camera; class Transform; class Shader { public: - explicit Shader(const std::string & fileName); - virtual ~Shader(); - - NO_COPY(Shader); - NO_MOVE(Shader); + Shader(); void Bind() const; void Update(const Transform & transform, const Camera & camera) const; private: - static constexpr unsigned int NUM_SHADERS = 2; - static constexpr unsigned int NUM_UNIFORMS = 3; + class Source { + public: + using ShaderRef = glRef; + + Source(const std::basic_string_view text, GLuint type); + Source(const GLchar * text, GLint len, GLuint type); + + ShaderRef id; + }; + + static void CheckShaderError(GLuint shader, GLuint flag, bool isProgram, std::string_view errorMessage); - static void CheckShaderError(GLuint shader, GLuint flag, bool isProgram, const std::string & errorMessage); - static GLuint CreateShader(const GLchar * text, GLint len, unsigned int type); + using ProgramRef = glRef; - GLuint m_program; - std::array m_shaders; - std::array m_uniforms; + ProgramRef m_program; + GLint mvp_uniform, normal_uniform, lightDir_uniform; }; #endif diff --git a/utility/glRef.hpp b/utility/glRef.hpp new file mode 100644 index 0000000..aae6629 --- /dev/null +++ b/utility/glRef.hpp @@ -0,0 +1,56 @@ +#ifndef GLREF_H +#define GLREF_H + +#include +#include + +template class glRef { +public: + template explicit glRef(Args &&... args) : id {get(fixed..., std::forward(args)...)} + { + if (!id) { + throw std::runtime_error("Get function failed"); + } + }; + + glRef(glRef && other) : id {other.id} + { + other.id = {}; + } + + ~glRef() + { + if (id) { + release(id); + } + } + + NO_COPY(glRef); + + const auto & + operator=(glRef && other) + { + if (id) { + release(id); + } + id = other.id; + other.id = {}; + return *this; + } + + auto + operator*() const + { + return id; + } + + operator IdType() const + { + return id; + } + +private: + IdType id; +}; + +#endif -- cgit v1.2.3