Big tidy up of shader wrapper

5 files changed, 96 insertions, 54 deletions

diff --git a/Jamroot.jam b/Jamroot.jam
index 397a167..9b79df6 100644
--- a/Jamroot.jam
+++ b/Jamroot.jam
@@ -26,6 +26,7 @@ project : requirements
 	<toolset>tidy:<checkxx>hicpp-*
 	<toolset>tidy:<xcheckxx>hicpp-signed-bitwise
 	<toolset>tidy:<xcheckxx>hicpp-named-parameter
+	<toolset>tidy:<xcheckxx>hicpp-no-array-decay
 	<toolset>tidy:<checkxx>performance-*
 	<toolset>tidy:<comments>no
 	<toolset>tidy:<mapping>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<Monkey>();
 
-		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 <array>

 #include <gfx/gl/shaders/fs-basicShader.h>

 #include <gfx/gl/shaders/vs-basicShader.h>

 #include <glm/glm.hpp>

 #include <stdexcept>

+#include <string>

 

-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<GLchar, 1024> 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<unsigned char> 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 <GL/glew.h>

-#include <array>

-#include <special_members.hpp>

-#include <string>

+#include <glRef.hpp>

+#include <string_view>

 

 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<GLuint, __glewCreateShader, __glewDeleteShader>;

+

+		Source(const std::basic_string_view<unsigned char> 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, __glewCreateProgram, __glewDeleteProgram>;

 

-	GLuint m_program;

-	std::array<GLuint, NUM_SHADERS> m_shaders;

-	std::array<GLint, NUM_UNIFORMS> 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 <special_members.hpp>
+#include <stdexcept>
+
+template<typename IdType, auto & get, auto & release, auto... fixed> class glRef {
+public:
+	template<typename... Args> explicit glRef(Args &&... args) : id {get(fixed..., std::forward<Args>(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