Merge pull request #306 from TomHarte/ShaderUniforms

Formalises naming of shader inputs and related guarantees.

Outputs/CRT/Internals/CRTOpenGL.cpp

@@ -306,10 +306,26 @@ void OpenGLOutputBuilder::prepare_source_vertex_array() {
 		glBindVertexArray(source_vertex_array_);
 		array_builder.bind_input();
 
-		composite_input_shader_program_->enable_vertex_attribute_with_pointer("inputStart", 2, GL_UNSIGNED_SHORT, GL_FALSE, SourceVertexSize, (void *)SourceVertexOffsetOfInputStart, 1);
-		composite_input_shader_program_->enable_vertex_attribute_with_pointer("outputStart", 2, GL_UNSIGNED_SHORT, GL_FALSE, SourceVertexSize, (void *)SourceVertexOffsetOfOutputStart, 1);
-		composite_input_shader_program_->enable_vertex_attribute_with_pointer("ends", 2, GL_UNSIGNED_SHORT, GL_FALSE, SourceVertexSize, (void *)SourceVertexOffsetOfEnds, 1);
-		composite_input_shader_program_->enable_vertex_attribute_with_pointer("phaseTimeAndAmplitude", 3, GL_UNSIGNED_BYTE, GL_FALSE, SourceVertexSize, (void *)SourceVertexOffsetOfPhaseTimeAndAmplitude, 1);
+		using Shader = OpenGL::IntermediateShader;
+		composite_input_shader_program_->enable_vertex_attribute_with_pointer(
+			Shader::get_input_name(Shader::Input::InputStart),
+			2, GL_UNSIGNED_SHORT, GL_FALSE, SourceVertexSize,
+			(void *)SourceVertexOffsetOfInputStart, 1);
+
+		composite_input_shader_program_->enable_vertex_attribute_with_pointer(
+			Shader::get_input_name(Shader::Input::OutputStart),
+			2, GL_UNSIGNED_SHORT, GL_FALSE, SourceVertexSize,
+			(void *)SourceVertexOffsetOfOutputStart, 1);
+
+		composite_input_shader_program_->enable_vertex_attribute_with_pointer(
+			Shader::get_input_name(Shader::Input::Ends),
+			2, GL_UNSIGNED_SHORT, GL_FALSE, SourceVertexSize,
+			(void *)SourceVertexOffsetOfEnds, 1);
+
+		composite_input_shader_program_->enable_vertex_attribute_with_pointer(
+			Shader::get_input_name(Shader::Input::PhaseTimeAndAmplitude),
+			3, GL_UNSIGNED_BYTE, GL_FALSE, SourceVertexSize,
+			(void *)SourceVertexOffsetOfPhaseTimeAndAmplitude, 1);
 	}
 }
 
@@ -324,8 +340,17 @@ void OpenGLOutputBuilder::prepare_output_vertex_array() {
 	if(output_shader_program_) {
 		glBindVertexArray(output_vertex_array_);
 		array_builder.bind_output();
-		output_shader_program_->enable_vertex_attribute_with_pointer("horizontal", 2, GL_UNSIGNED_SHORT, GL_FALSE, OutputVertexSize, (void *)OutputVertexOffsetOfHorizontal, 1);
-		output_shader_program_->enable_vertex_attribute_with_pointer("vertical", 2, GL_UNSIGNED_SHORT, GL_FALSE, OutputVertexSize, (void *)OutputVertexOffsetOfVertical, 1);
+		
+		using Shader = OpenGL::OutputShader;
+		output_shader_program_->enable_vertex_attribute_with_pointer(
+			Shader::get_input_name(Shader::Input::Horizontal),
+			2, GL_UNSIGNED_SHORT, GL_FALSE, OutputVertexSize,
+			(void *)OutputVertexOffsetOfHorizontal, 1);
+
+		output_shader_program_->enable_vertex_attribute_with_pointer(
+			Shader::get_input_name(Shader::Input::Vertical),
+			2, GL_UNSIGNED_SHORT, GL_FALSE, OutputVertexSize,
+			(void *)OutputVertexOffsetOfVertical, 1);
 	}
 }
 

Outputs/CRT/Internals/Shaders/IntermediateShader.cpp

@@ -8,6 +8,7 @@
 
 #include "IntermediateShader.hpp"
 
+#include <cassert>
 #include <cstring>
 #include <sstream>
 
@@ -15,14 +16,16 @@
 
 using namespace OpenGL;
 
-namespace {
-	const OpenGL::Shader::AttributeBinding bindings[] = {
-		{"inputStart", 0},
-		{"outputStart", 1},
-		{"ends", 2},
-		{"phaseTimeAndAmplitude", 3},
-		{nullptr, 0}
-	};
+std::string IntermediateShader::get_input_name(Input input) {
+	switch(input) {
+		case Input::InputStart:				return "inputStart";
+		case Input::OutputStart:			return "outputStart";
+		case Input::Ends:					return "ends";
+		case Input::PhaseTimeAndAmplitude:	return "phaseTimeAndAmplitude";
+
+		// Intended to be unreachable.
+		default: assert(false);
+	}
 }
 
 std::unique_ptr<IntermediateShader> IntermediateShader::make_shader(const std::string &fragment_shader, bool use_usampler, bool input_is_inputPosition) {
@@ -30,10 +33,10 @@ std::unique_ptr<IntermediateShader> IntermediateShader::make_shader(const std::s
 	vertex_shader <<
 		"#version 150\n"
 
-		"in vec2 inputStart;"
-		"in vec2 outputStart;"
-		"in vec2 ends;"
-		"in vec3 phaseTimeAndAmplitude;"
+		"in vec2 " << get_input_name(Input::InputStart) << ";"
+		"in vec2 " << get_input_name(Input::OutputStart) << ";"
+		"in vec2 " << get_input_name(Input::Ends) << ";"
+		"in vec3 " << get_input_name(Input::PhaseTimeAndAmplitude) << ";"
 
 		"uniform ivec2 outputTextureSize;"
 		"uniform float extension;"
@@ -102,7 +105,12 @@ std::unique_ptr<IntermediateShader> IntermediateShader::make_shader(const std::s
 			"gl_Position = vec4(eyePosition, 0.0, 1.0);"
 		"}";
 
-	return std::unique_ptr<IntermediateShader>(new IntermediateShader(vertex_shader.str(), fragment_shader, bindings));
+	return std::unique_ptr<IntermediateShader>(new IntermediateShader(vertex_shader.str(), fragment_shader, {
+		{get_input_name(Input::InputStart), 0},
+		{get_input_name(Input::OutputStart), 1},
+		{get_input_name(Input::Ends), 2},
+		{get_input_name(Input::PhaseTimeAndAmplitude), 3}
+	}));
 }
 
 std::unique_ptr<IntermediateShader> IntermediateShader::make_source_conversion_shader(const std::string &composite_shader, const std::string &rgb_shader) {

Outputs/CRT/Internals/Shaders/IntermediateShader.hpp

@@ -19,6 +19,27 @@ namespace OpenGL {
 class IntermediateShader: public Shader {
 public:
 	using Shader::Shader;
+	
+	enum class Input {
+		/// Contains the 2d start position of this run's input data.
+		InputStart,
+		/// Contains the 2d start position of this run's output position.
+		OutputStart,
+		/// A 2d vector comprised of (the final x position for input, the final x position for output).
+		Ends,
+		/// A 3d vector recording the colour subcarrier's (phase, time, amplitude) at the start of this span of data.
+		PhaseTimeAndAmplitude
+	};
+
+	/*!
+		Obtains the name of a designated input. Designated inputs are guaranteed to have the same attribute location
+		across multiple instances of IntermediateShader. So binding a vertex array to these inputs for any instance of
+		IntermediateShader allows that array to work with all instances of IntermediateShader.
+
+		@param input The input to query.
+		@returns The name used in this shader's source for the nominated input.
+	*/
+	static std::string get_input_name(Input input);
 
 	/*!
 		Constructs and returns an intermediate shader that will take runs from the inputPositions,

Outputs/CRT/Internals/Shaders/OutputShader.cpp

@@ -8,17 +8,20 @@
 
 #include "OutputShader.hpp"
 
+#include <cassert>
 #include <cmath>
 #include <sstream>
 
 using namespace OpenGL;
 
-namespace {
-	const OpenGL::Shader::AttributeBinding bindings[] = {
-		{"position", 0},
-		{"srcCoordinates", 1},
-		{nullptr, 0}
-	};
+std::string OutputShader::get_input_name(Input input) {
+	switch(input) {
+		case Input::Horizontal:	return "horizontal";
+		case Input::Vertical:	return "vertical";
+
+		// Intended to be unreachable.
+		default: assert(false);
+	}
 }
 
 std::unique_ptr<OutputShader> OutputShader::make_shader(const char *fragment_methods, const char *colour_expression, bool use_usampler) {
@@ -28,8 +31,8 @@ std::unique_ptr<OutputShader> OutputShader::make_shader(const char *fragment_met
 	vertex_shader <<
 		"#version 150\n"
 
-		"in vec2 horizontal;"
-		"in vec2 vertical;"
+		"in vec2 " << get_input_name(Input::Horizontal) << ";"
+		"in vec2 " << get_input_name(Input::Vertical) << ";"
 
 		"uniform vec2 boundsOrigin;"
 		"uniform vec2 boundsSize;"
@@ -83,7 +86,10 @@ std::unique_ptr<OutputShader> OutputShader::make_shader(const char *fragment_met
 			"fragColour = vec4(pow(" << colour_expression << ", vec3(gamma)), 0.5);"//*cos(lateralVarying)
 		"}";
 
-	return std::unique_ptr<OutputShader>(new OutputShader(vertex_shader.str(), fragment_shader.str(), bindings));
+	return std::unique_ptr<OutputShader>(new OutputShader(vertex_shader.str(), fragment_shader.str(), {
+		{get_input_name(Input::Horizontal), 0},
+		{get_input_name(Input::Vertical), 1}
+	}));
 }
 
 void OutputShader::set_output_size(unsigned int output_width, unsigned int output_height, Outputs::CRT::Rect visible_area) {

Outputs/CRT/Internals/Shaders/OutputShader.hpp

@@ -17,6 +17,23 @@ namespace OpenGL {
 
 class OutputShader: public Shader {
 public:
+	enum class Input {
+		/// A 2d vector; the first element is the horizontal start of this scan, the second element is the end.
+		Horizontal,
+		/// A 2d vector; the first element is the vertical start of this scan, the second element is the end.
+		Vertical
+	};
+
+	/*!
+		Obtains the name of a designated input. Designated inputs are guaranteed to have the same attribute location
+		across multiple instances of OutputShader. So binding a vertex array to these inputs for any instance of
+		OutputShader allows that array to work with all instances of OutputShader.
+
+		@param input The input to query.
+		@returns The name used in this shader's source for the nominated input.
+	*/
+	static std::string get_input_name(Input input);
+
 	/*!
 		Constructs and returns an instance of OutputShader. OutputShaders are intended to read source data
 		from a texture and draw a single raster scan containing that data as output.

Outputs/CRT/Internals/Shaders/Shader.cpp

@@ -43,7 +43,7 @@ GLuint Shader::compile_shader(const std::string &source, GLenum type) {
 	return shader;
 }
 
-Shader::Shader(const std::string &vertex_shader, const std::string &fragment_shader, const AttributeBinding *attribute_bindings) {
+Shader::Shader(const std::string &vertex_shader, const std::string &fragment_shader, const std::vector<AttributeBinding> &attribute_bindings) {
 	shader_program_ = glCreateProgram();
 	GLuint vertex = compile_shader(vertex_shader, GL_VERTEX_SHADER);
 	GLuint fragment = compile_shader(fragment_shader, GL_FRAGMENT_SHADER);
@@ -51,11 +51,8 @@ Shader::Shader(const std::string &vertex_shader, const std::string &fragment_sha
 	glAttachShader(shader_program_, vertex);
 	glAttachShader(shader_program_, fragment);
 
-	if(attribute_bindings) {
-		while(attribute_bindings->name) {
-			glBindAttribLocation(shader_program_, attribute_bindings->index, attribute_bindings->name);
-			attribute_bindings++;
-		}
+	for(auto &binding : attribute_bindings) {
+		glBindAttribLocation(shader_program_, binding.index, binding.name.c_str());
 	}
 
 	glLinkProgram(shader_program_);
@@ -95,15 +92,15 @@ void Shader::unbind() {
 	glUseProgram(0);
 }
 
-GLint Shader::get_attrib_location(const GLchar *name) {
-	return glGetAttribLocation(shader_program_, name);
+GLint Shader::get_attrib_location(const std::string &name) {
+	return glGetAttribLocation(shader_program_, name.c_str());
 }
 
-GLint Shader::get_uniform_location(const GLchar *name) {
-	return glGetUniformLocation(shader_program_, name);
+GLint Shader::get_uniform_location(const std::string &name) {
+	return glGetUniformLocation(shader_program_, name.c_str());
 }
 
-void Shader::enable_vertex_attribute_with_pointer(const char *name, GLint size, GLenum type, GLboolean normalised, GLsizei stride, const GLvoid *pointer, GLuint divisor) {
+void Shader::enable_vertex_attribute_with_pointer(const std::string &name, GLint size, GLenum type, GLboolean normalised, GLsizei stride, const GLvoid *pointer, GLuint divisor) {
 	GLint location = get_attrib_location(name);
 	glEnableVertexAttribArray((GLuint)location);
 	glVertexAttribPointer((GLuint)location, size, type, normalised, stride, pointer);

Outputs/CRT/Internals/Shaders/Shader.hpp

@@ -10,10 +10,11 @@
 #define Shader_hpp
 
 #include "../OpenGL.hpp"
-#include <string>
+
 #include <functional>
-#include <vector>
 #include <mutex>
+#include <string>
+#include <vector>
 
 namespace OpenGL {
 
@@ -31,7 +32,7 @@ public:
 	};
 
 	struct AttributeBinding {
-		const GLchar *const name;
+		const std::string name;
 		const GLuint index;
 	};
 
@@ -39,9 +40,9 @@ public:
 		Attempts to compile a shader, throwing @c VertexShaderCompilationError, @c FragmentShaderCompilationError or @c ProgramLinkageError upon failure.
 		@param vertex_shader The vertex shader source code.
 		@param fragment_shader The fragment shader source code.
-		@param attribute_bindings Either @c nullptr or an array terminated by an entry with a @c nullptr-name of attribute bindings.
+		@param attribute_bindings A vector of attribute bindings.
 	*/
-	Shader(const std::string &vertex_shader, const std::string &fragment_shader, const AttributeBinding *attribute_bindings);
+	Shader(const std::string &vertex_shader, const std::string &fragment_shader, const std::vector<AttributeBinding> &attribute_bindings = {});
 	~Shader();
 
 	/*!
@@ -63,14 +64,14 @@ public:
 		@param name The name of the attribute to locate.
 		@returns The location of the requested attribute.
 	*/
-	GLint get_attrib_location(const GLchar *name);
+	GLint get_attrib_location(const std::string &name);
 
 	/*!
 		Performs a @c glGetUniformLocation call.
 		@param name The name of the uniform to locate.
 		@returns The location of the requested uniform.
 	*/
-	GLint get_uniform_location(const GLchar *name);
+	GLint get_uniform_location(const std::string &name);
 
 	/*!
 		Shorthand for an appropriate sequence of:
@@ -79,7 +80,7 @@ public:
 		* @c glVertexAttribPointer;
 		* @c glVertexAttribDivisor.
 	*/
-	void enable_vertex_attribute_with_pointer(const char *name, GLint size, GLenum type, GLboolean normalised, GLsizei stride, const GLvoid *pointer, GLuint divisor);
+	void enable_vertex_attribute_with_pointer(const std::string &name, GLint size, GLenum type, GLboolean normalised, GLsizei stride, const GLvoid *pointer, GLuint divisor);
 
 	/*!
 		All @c set_uniforms queue up the requested uniform changes. Changes are applied automatically the next time the shader is bound.

Outputs/CRT/Internals/TextureTarget.cpp

@@ -82,7 +82,7 @@ void TextureTarget::draw(float aspect_ratio) {
 			"{"
 				"fragColour = texture(texID, texCoordVarying);"
 			"}";
-		pixel_shader_.reset(new Shader(vertex_shader, fragment_shader, nullptr));
+		pixel_shader_.reset(new Shader(vertex_shader, fragment_shader));
 		pixel_shader_->bind();
 
 		glGenVertexArrays(1, &drawing_vertex_array_);