From b4f3c41aae508c11825270a252d4d2cbd0c9639c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 24 Nov 2017 18:45:24 -0500 Subject: [PATCH] Formalises naming of shader inputs and related guarantees. --- Outputs/CRT/Internals/CRTOpenGL.cpp | 37 ++++++++++++++++--- .../Internals/Shaders/IntermediateShader.cpp | 30 ++++++++------- .../Internals/Shaders/IntermediateShader.hpp | 21 +++++++++++ .../CRT/Internals/Shaders/OutputShader.cpp | 20 +++++----- .../CRT/Internals/Shaders/OutputShader.hpp | 17 +++++++++ Outputs/CRT/Internals/Shaders/Shader.cpp | 19 ++++------ Outputs/CRT/Internals/Shaders/Shader.hpp | 17 +++++---- Outputs/CRT/Internals/TextureTarget.cpp | 2 +- 8 files changed, 115 insertions(+), 48 deletions(-) diff --git a/Outputs/CRT/Internals/CRTOpenGL.cpp b/Outputs/CRT/Internals/CRTOpenGL.cpp index 77b06b964..2c9ca50b8 100644 --- a/Outputs/CRT/Internals/CRTOpenGL.cpp +++ b/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); } } diff --git a/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp b/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp index 35a726711..eab7369f9 100644 --- a/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp +++ b/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp @@ -15,14 +15,13 @@ 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"; + } } std::unique_ptr IntermediateShader::make_shader(const std::string &fragment_shader, bool use_usampler, bool input_is_inputPosition) { @@ -30,10 +29,10 @@ std::unique_ptr 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 +101,12 @@ std::unique_ptr IntermediateShader::make_shader(const std::s "gl_Position = vec4(eyePosition, 0.0, 1.0);" "}"; - return std::unique_ptr(new IntermediateShader(vertex_shader.str(), fragment_shader, bindings)); + return std::unique_ptr(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::make_source_conversion_shader(const std::string &composite_shader, const std::string &rgb_shader) { diff --git a/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp b/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp index 22e852837..96c9c86dc 100644 --- a/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp +++ b/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, diff --git a/Outputs/CRT/Internals/Shaders/OutputShader.cpp b/Outputs/CRT/Internals/Shaders/OutputShader.cpp index e418112f6..664703061 100644 --- a/Outputs/CRT/Internals/Shaders/OutputShader.cpp +++ b/Outputs/CRT/Internals/Shaders/OutputShader.cpp @@ -13,12 +13,11 @@ 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"; + } } std::unique_ptr OutputShader::make_shader(const char *fragment_methods, const char *colour_expression, bool use_usampler) { @@ -28,8 +27,8 @@ std::unique_ptr 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 +82,10 @@ std::unique_ptr OutputShader::make_shader(const char *fragment_met "fragColour = vec4(pow(" << colour_expression << ", vec3(gamma)), 0.5);"//*cos(lateralVarying) "}"; - return std::unique_ptr(new OutputShader(vertex_shader.str(), fragment_shader.str(), bindings)); + return std::unique_ptr(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) { diff --git a/Outputs/CRT/Internals/Shaders/OutputShader.hpp b/Outputs/CRT/Internals/Shaders/OutputShader.hpp index 02db19cdb..55fbea7a4 100644 --- a/Outputs/CRT/Internals/Shaders/OutputShader.hpp +++ b/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. diff --git a/Outputs/CRT/Internals/Shaders/Shader.cpp b/Outputs/CRT/Internals/Shaders/Shader.cpp index cb6bffecc..482107576 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.cpp +++ b/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 &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); diff --git a/Outputs/CRT/Internals/Shaders/Shader.hpp b/Outputs/CRT/Internals/Shaders/Shader.hpp index 0aaaf3087..c2947fe74 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.hpp +++ b/Outputs/CRT/Internals/Shaders/Shader.hpp @@ -10,10 +10,11 @@ #define Shader_hpp #include "../OpenGL.hpp" -#include + #include -#include #include +#include +#include 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 &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. diff --git a/Outputs/CRT/Internals/TextureTarget.cpp b/Outputs/CRT/Internals/TextureTarget.cpp index aea41edaa..5742249bf 100644 --- a/Outputs/CRT/Internals/TextureTarget.cpp +++ b/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_);