From 091516e3cb91ea231c84f56d0e180831e5e1565e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 15 May 2016 14:59:59 -0400 Subject: [PATCH] The semantics might need better exposition but: fixed `UniformXfv` calls plus matrix calls, documented new semantics on all setters. --- .../Internals/Shaders/IntermediateShader.hpp | 18 ++++++----- .../CRT/Internals/Shaders/OutputShader.hpp | 10 +++--- Outputs/CRT/Internals/Shaders/Shader.cpp | 32 ++++++++++--------- Outputs/CRT/Internals/Shaders/Shader.hpp | 6 +++- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp b/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp index 6b0170a5b..a419da8be 100644 --- a/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp +++ b/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp @@ -55,34 +55,36 @@ public: static std::unique_ptr make_rgb_filter_shader(); /*! - Binds this shader and configures it for output to an area of `output_width` and `output_height` pixels. + Queues the configuration of this shader for output to an area of `output_width` and `output_height` pixels + to occur upon the next `bind`. */ void set_output_size(unsigned int output_width, unsigned int output_height); /*! - Binds this shader and sets the texture unit (as an enum, e.g. `GL_TEXTURE0`) to sample as source data. + Queues setting the texture unit (as an enum, e.g. `GL_TEXTURE0`) for source data to occur upon the next `bind`. */ void set_source_texture_unit(GLenum unit); /*! - Binds this shader and sets filtering coefficients for a lowpass filter based on the cutoff. + Queues setting filtering coefficients for a lowpass filter based on the cutoff frequency to occur upon the next `bind`. */ void set_filter_coefficients(float sampling_rate, float cutoff_frequency); /*! - Binds this shader and configures filtering to separate luminance and chrominance based on a colour - subcarrier of the given frequency. + Queues configuration of filtering to separate luminance and chrominance based on a colour + subcarrier of the given frequency to occur upon the next `bind`. */ void set_separation_frequency(float sampling_rate, float colour_burst_frequency); /*! - Binds this shader and sets the number of colour phase cycles per sample, indicating whether output - geometry should be extended so that a complete colour cycle is included at both the beginning and end. + Queues setting of the number of colour phase cycles per sample, indicating whether output + geometry should be extended so that a complete colour cycle is included at both the beginning and end, + to occur upon the next `bind`. */ void set_phase_cycles_per_sample(float phase_cycles_per_sample, bool extend_runs_to_full_cycle); /*! - Binds this shader and sets the matrices that convert between RGB and chrominance/luminance. + Queues setting the matrices that convert between RGB and chrominance/luminance to occur on the next `bind`. */ void set_colour_conversion_matrices(float *fromRGB, float *toRGB); diff --git a/Outputs/CRT/Internals/Shaders/OutputShader.hpp b/Outputs/CRT/Internals/Shaders/OutputShader.hpp index f577d366b..9b4b7db39 100644 --- a/Outputs/CRT/Internals/Shaders/OutputShader.hpp +++ b/Outputs/CRT/Internals/Shaders/OutputShader.hpp @@ -42,18 +42,20 @@ public: using Shader::Shader; /*! - Binds this shader and configures it for output to an area of `output_width` and `output_height` pixels, ensuring - the largest possible drawing size that allows everything within `visible_area` to be visible. + Queues configuration for output to an area of `output_width` and `output_height` pixels, ensuring + the largest possible drawing size that allows everything within `visible_area` to be visible, to + occur upon the next `bind`. */ void set_output_size(unsigned int output_width, unsigned int output_height, Outputs::CRT::Rect visible_area); /*! - Binds this shader and sets the texture unit (as an enum, e.g. `GL_TEXTURE0`) to sample as source data. + Queues setting of the texture unit (as an enum, e.g. `GL_TEXTURE0`) for source data upon the next `bind`. */ void set_source_texture_unit(GLenum unit); /*! - Binds this shader and configures its understanding of how to map from the source vertex stream to screen coordinates. + Queues configuring this shader's understanding of how to map from the source vertex stream to screen coordinates, + to occur upon the next `bind`. */ void set_timing(unsigned int height_of_display, unsigned int cycles_per_line, unsigned int horizontal_scan_period, unsigned int vertical_scan_period, unsigned int vertical_period_divider); }; diff --git a/Outputs/CRT/Internals/Shaders/Shader.cpp b/Outputs/CRT/Internals/Shaders/Shader.cpp index 59cccd05c..f4968b752 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.cpp +++ b/Outputs/CRT/Internals/Shaders/Shader.cpp @@ -123,13 +123,7 @@ void Shader::enable_vertex_attribute_with_pointer(const char *name, GLint size, } // The various set_uniforms... -GLint fglGetUniformLocation(GLuint program, const GLchar *name) -{ - GLint result = glGetUniformLocation(program, name); - printf("Resolved %s to %d\n", name, result); - return result; -} -#define location() fglGetUniformLocation(_shader_program, name.c_str()) +#define location() glGetUniformLocation(_shader_program, name.c_str()) void Shader::set_uniform(const std::string &name, GLint value) { enqueue_function([name, value, this] { @@ -220,8 +214,10 @@ void Shader::set_uniform(const std::string &name, GLuint value1, GLuint value2, void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLint *values) { - GLint *values_copy = new GLint[count]; - memcpy(values_copy, values, sizeof(GLint) * (size_t)count); + size_t number_of_values = (size_t)count * (size_t)size; + GLint *values_copy = new GLint[number_of_values]; + memcpy(values_copy, values, sizeof(*values) * (size_t)number_of_values); + enqueue_function([name, size, count, values_copy, this] { switch(size) { @@ -236,8 +232,10 @@ void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, con void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLfloat *values) { - GLfloat *values_copy = new GLfloat[count]; - memcpy(values_copy, values, sizeof(GLfloat) * (size_t)count); + size_t number_of_values = (size_t)count * (size_t)size; + GLfloat *values_copy = new GLfloat[number_of_values]; + memcpy(values_copy, values, sizeof(*values) * (size_t)number_of_values); + enqueue_function([name, size, count, values_copy, this] { switch(size) { @@ -252,8 +250,10 @@ void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, con void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLuint *values) { - GLuint *values_copy = new GLuint[count]; - memcpy(values_copy, values, sizeof(GLuint) * (size_t)count); + size_t number_of_values = (size_t)count * (size_t)size; + GLuint *values_copy = new GLuint[number_of_values]; + memcpy(values_copy, values, sizeof(*values) * (size_t)number_of_values); + enqueue_function([name, size, count, values_copy, this] { switch(size) { @@ -273,8 +273,10 @@ void Shader::set_uniform_matrix(const std::string &name, GLint size, bool transp void Shader::set_uniform_matrix(const std::string &name, GLint size, GLsizei count, bool transpose, const GLfloat *values) { - GLfloat *values_copy = new GLfloat[count*size]; - memcpy(values_copy, values, sizeof(GLfloat) * (size_t)count * (size_t)size); + size_t number_of_values = (size_t)count * (size_t)size * (size_t)size; + GLfloat *values_copy = new GLfloat[number_of_values]; + memcpy(values_copy, values, sizeof(*values) * number_of_values); + enqueue_function([name, size, count, transpose, values_copy, this] { GLboolean glTranspose = transpose ? GL_TRUE : GL_FALSE; switch(size) diff --git a/Outputs/CRT/Internals/Shaders/Shader.hpp b/Outputs/CRT/Internals/Shaders/Shader.hpp index cea47d957..ead7ce397 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.hpp +++ b/Outputs/CRT/Internals/Shaders/Shader.hpp @@ -47,6 +47,8 @@ public: Performs an @c glUseProgram to make this the active shader unless: (i) it was the previous shader bound; and (ii) no calls have been received to unbind in the interim. + + Subsequently performs all work queued up for the next bind irrespective of whether a @c glUseProgram call occurred. */ void bind(); @@ -106,9 +108,11 @@ private: GLuint compile_shader(const char *source, GLenum type); GLuint _shader_program; - void enqueue_function(std::function function); void flush_functions(); std::list> _enqueued_functions; + +protected: + void enqueue_function(std::function function); }; }