From 30f335fa35648aae70da35680c8c8be4cd50b8bd Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 13 May 2016 22:08:32 -0400 Subject: [PATCH 1/6] Switched pervasively to using the named slot uniform setters on `Shader`. --- .../Internals/Shaders/IntermediateShader.cpp | 36 ++---- .../Internals/Shaders/IntermediateShader.hpp | 9 -- .../CRT/Internals/Shaders/OutputShader.cpp | 27 ++-- .../CRT/Internals/Shaders/OutputShader.hpp | 3 - Outputs/CRT/Internals/Shaders/Shader.cpp | 116 ++++++++++++++++++ Outputs/CRT/Internals/Shaders/Shader.hpp | 26 ++++ 6 files changed, 160 insertions(+), 57 deletions(-) diff --git a/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp b/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp index ea5aaba11..ed82601ea 100644 --- a/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp +++ b/Outputs/CRT/Internals/Shaders/IntermediateShader.cpp @@ -89,15 +89,6 @@ std::unique_ptr IntermediateShader::make_shader(const char * std::unique_ptr shader = std::unique_ptr(new IntermediateShader(vertex_shader, fragment_shader, bindings)); free(vertex_shader); - shader->texIDUniform = shader->get_uniform_location("texID"); - shader->outputTextureSizeUniform = shader->get_uniform_location("outputTextureSize"); - shader->phaseCyclesPerTickUniform = shader->get_uniform_location("phaseCyclesPerTick"); - shader->extensionUniform = shader->get_uniform_location("extension"); - shader->weightsUniform = shader->get_uniform_location("weights"); - shader->rgbToLumaChromaUniform = shader->get_uniform_location("rgbToLumaChroma"); - shader->lumaChromaToRGBUniform = shader->get_uniform_location("lumaChromaToRGB"); - shader->offsetsUniform = shader->get_uniform_location("offsets"); - return shader; } @@ -395,28 +386,24 @@ std::unique_ptr IntermediateShader::make_rgb_filter_shader() void IntermediateShader::set_output_size(unsigned int output_width, unsigned int output_height) { - bind(); - glUniform2i(outputTextureSizeUniform, (GLint)output_width, (GLint)output_height); + set_uniform("outputTextureSize", (GLint)output_width, (GLint)output_height); } void IntermediateShader::set_source_texture_unit(GLenum unit) { - bind(); - glUniform1i(texIDUniform, (GLint)(unit - GL_TEXTURE0)); + set_uniform("texID", (GLint)(unit - GL_TEXTURE0)); } void IntermediateShader::set_filter_coefficients(float sampling_rate, float cutoff_frequency) { - bind(); - // The process below: the source texture will have bilinear filtering enabled; so by // sampling at non-integral offsets from the centre the shader can get a weighted sum // of two source pixels, then scale that once, to do two taps per sample. However // that works only if the two coefficients being joined have the same sign. So the // number of usable taps is between 11 and 21 depending on the values that come out. // Perform a linear search for the highest number of taps we can use with 11 samples. - float weights[12]; - float offsets[5]; + GLfloat weights[12]; + GLfloat offsets[5]; unsigned int taps = 21; while(1) { @@ -458,26 +445,23 @@ void IntermediateShader::set_filter_coefficients(float sampling_rate, float cuto taps -= 2; } - glUniform4fv(weightsUniform, 3, weights); - glUniform1fv(offsetsUniform, 5, offsets); + set_uniform("weights", 4, 3, weights); + set_uniform("offsets", 1, 5, offsets); } void IntermediateShader::set_separation_frequency(float sampling_rate, float colour_burst_frequency) { - // TODO: apply separately-formed filters for luminance and chrominance set_filter_coefficients(sampling_rate, colour_burst_frequency); } void IntermediateShader::set_phase_cycles_per_sample(float phase_cycles_per_sample, bool extend_runs_to_full_cycle) { - bind(); - glUniform1f(phaseCyclesPerTickUniform, phase_cycles_per_sample); - glUniform1f(extensionUniform, extend_runs_to_full_cycle ? ceilf(1.0f / phase_cycles_per_sample) : 0.0f); + set_uniform("phaseCyclesPerTick", (GLfloat)phase_cycles_per_sample); + set_uniform("extension", extend_runs_to_full_cycle ? ceilf(1.0f / phase_cycles_per_sample) : 0.0f); } void IntermediateShader::set_colour_conversion_matrices(float *fromRGB, float *toRGB) { - bind(); - glUniformMatrix3fv(lumaChromaToRGBUniform, 1, GL_FALSE, toRGB); - glUniformMatrix3fv(rgbToLumaChromaUniform, 1, GL_FALSE, fromRGB); + set_uniform_matrix("lumaChromaToRGB", 3, false, toRGB); + set_uniform_matrix("rgbToLumaChroma", 3, false, fromRGB); } diff --git a/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp b/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp index 821a5a717..6b0170a5b 100644 --- a/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp +++ b/Outputs/CRT/Internals/Shaders/IntermediateShader.hpp @@ -88,15 +88,6 @@ public: private: static std::unique_ptr make_shader(const char *fragment_shader, bool use_usampler, bool input_is_inputPosition); - - GLint texIDUniform; - GLint outputTextureSizeUniform; - GLint weightsUniform; - GLint phaseCyclesPerTickUniform; - GLint extensionUniform; - GLint rgbToLumaChromaUniform; - GLint lumaChromaToRGBUniform; - GLint offsetsUniform; }; } diff --git a/Outputs/CRT/Internals/Shaders/OutputShader.cpp b/Outputs/CRT/Internals/Shaders/OutputShader.cpp index 3576fcb79..682e62304 100644 --- a/Outputs/CRT/Internals/Shaders/OutputShader.cpp +++ b/Outputs/CRT/Internals/Shaders/OutputShader.cpp @@ -86,45 +86,34 @@ std::unique_ptr OutputShader::make_shader(const char *fragment_met free(vertex_shader); free(fragment_shader); - result->boundsSizeUniform = result->get_uniform_location("boundsSize"); - result->boundsOriginUniform = result->get_uniform_location("boundsOrigin"); - result->texIDUniform = result->get_uniform_location("texID"); - result->scanNormalUniform = result->get_uniform_location("scanNormal"); - result->positionConversionUniform = result->get_uniform_location("positionConversion"); - return result; } void OutputShader::set_output_size(unsigned int output_width, unsigned int output_height, Outputs::CRT::Rect visible_area) { - bind(); - GLfloat outputAspectRatioMultiplier = ((float)output_width / (float)output_height) / (4.0f / 3.0f); GLfloat bonusWidth = (outputAspectRatioMultiplier - 1.0f) * visible_area.size.width; visible_area.origin.x -= bonusWidth * 0.5f * visible_area.size.width; visible_area.size.width *= outputAspectRatioMultiplier; - glUniform2f(boundsOriginUniform, (GLfloat)visible_area.origin.x, (GLfloat)visible_area.origin.y); - glUniform2f(boundsSizeUniform, (GLfloat)visible_area.size.width, (GLfloat)visible_area.size.height); + set_uniform("boundsOrigin", (GLfloat)visible_area.origin.x, (GLfloat)visible_area.origin.y); + set_uniform("boundsSize", (GLfloat)visible_area.size.width, (GLfloat)visible_area.size.height); } void OutputShader::set_source_texture_unit(GLenum unit) { - bind(); - glUniform1i(texIDUniform, (GLint)(unit - GL_TEXTURE0)); + set_uniform("texID", (GLint)(unit - GL_TEXTURE0)); } void OutputShader::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) { - bind(); - - float scan_angle = atan2f(1.0f / (float)height_of_display, 1.0f); - float scan_normal[] = { -sinf(scan_angle), cosf(scan_angle)}; - float multiplier = (float)cycles_per_line / ((float)height_of_display * (float)horizontal_scan_period); + GLfloat scan_angle = atan2f(1.0f / (float)height_of_display, 1.0f); + GLfloat scan_normal[] = { -sinf(scan_angle), cosf(scan_angle)}; + GLfloat multiplier = (float)cycles_per_line / ((float)height_of_display * (float)horizontal_scan_period); scan_normal[0] *= multiplier; scan_normal[1] *= multiplier; - glUniform2f(scanNormalUniform, scan_normal[0], scan_normal[1]); - glUniform2f(positionConversionUniform, horizontal_scan_period, vertical_scan_period / (unsigned int)vertical_period_divider); + set_uniform("scanNormal", scan_normal[0], scan_normal[1]); + set_uniform("positionConversion", (GLfloat)horizontal_scan_period, (GLfloat)vertical_scan_period / (GLfloat)vertical_period_divider); } diff --git a/Outputs/CRT/Internals/Shaders/OutputShader.hpp b/Outputs/CRT/Internals/Shaders/OutputShader.hpp index 855069e2c..f577d366b 100644 --- a/Outputs/CRT/Internals/Shaders/OutputShader.hpp +++ b/Outputs/CRT/Internals/Shaders/OutputShader.hpp @@ -56,9 +56,6 @@ public: Binds this shader and configures its understanding of how to map from the source vertex stream to screen coordinates. */ 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); - - private: - GLint boundsOriginUniform, boundsSizeUniform, texIDUniform, scanNormalUniform, positionConversionUniform; }; } diff --git a/Outputs/CRT/Internals/Shaders/Shader.cpp b/Outputs/CRT/Internals/Shaders/Shader.cpp index 78dd2fe8e..336e99edd 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.cpp +++ b/Outputs/CRT/Internals/Shaders/Shader.cpp @@ -120,3 +120,119 @@ void Shader::enable_vertex_attribute_with_pointer(const char *name, GLint size, glVertexAttribPointer((GLuint)location, size, type, normalised, stride, pointer); glVertexAttribDivisor((GLuint)location, divisor); } + +// The various set_uniforms... +GLint Shader::location_for_bound_uniform(const GLchar *name) +{ + bind(); + return glGetUniformLocation(_shader_program, name); +} + +void Shader::set_uniform(const GLchar *name, GLint value) +{ + glUniform1i(location_for_bound_uniform(name), value); +} + +void Shader::set_uniform(const GLchar *name, GLint value1, GLint value2) +{ + glUniform2i(location_for_bound_uniform(name), value1, value2); +} + +void Shader::set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3) +{ + glUniform3i(location_for_bound_uniform(name), value1, value2, value3); +} + +void Shader::set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3, GLint value4) +{ + glUniform4i(location_for_bound_uniform(name), value1, value2, value3, value4); +} + +void Shader::set_uniform(const GLchar *name, GLint size, GLsizei count, const GLint *values) +{ + switch(size) + { + case 1: glUniform1iv(location_for_bound_uniform(name), count, values); break; + case 2: glUniform2iv(location_for_bound_uniform(name), count, values); break; + case 3: glUniform3iv(location_for_bound_uniform(name), count, values); break; + case 4: glUniform4iv(location_for_bound_uniform(name), count, values); break; + } +} + +void Shader::set_uniform(const GLchar *name, GLfloat value) +{ + glUniform1f(location_for_bound_uniform(name), value); +} + +void Shader::set_uniform(const GLchar *name, GLfloat value1, GLfloat value2) +{ + glUniform2f(location_for_bound_uniform(name), value1, value2); +} + +void Shader::set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3) +{ + glUniform3f(location_for_bound_uniform(name), value1, value2, value3); +} + +void Shader::set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3, GLfloat value4) +{ + glUniform4f(location_for_bound_uniform(name), value1, value2, value3, value4); +} + +void Shader::set_uniform(const GLchar *name, GLint size, GLsizei count, const GLfloat *values) +{ + switch(size) + { + case 1: glUniform1fv(location_for_bound_uniform(name), count, values); break; + case 2: glUniform2fv(location_for_bound_uniform(name), count, values); break; + case 3: glUniform3fv(location_for_bound_uniform(name), count, values); break; + case 4: glUniform4fv(location_for_bound_uniform(name), count, values); break; + } +} + +void Shader::set_uniform(const GLchar *name, GLuint value) +{ + glUniform1ui(location_for_bound_uniform(name), value); +} + +void Shader::set_uniform(const GLchar *name, GLuint value1, GLuint value2) +{ + glUniform2ui(location_for_bound_uniform(name), value1, value2); +} + +void Shader::set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3) +{ + glUniform3ui(location_for_bound_uniform(name), value1, value2, value3); +} + +void Shader::set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3, GLuint value4) +{ + glUniform4ui(location_for_bound_uniform(name), value1, value2, value3, value4); +} + +void Shader::set_uniform(const GLchar *name, GLint size, GLsizei count, const GLuint *values) +{ + switch(size) + { + case 1: glUniform1uiv(location_for_bound_uniform(name), count, values); break; + case 2: glUniform2uiv(location_for_bound_uniform(name), count, values); break; + case 3: glUniform3uiv(location_for_bound_uniform(name), count, values); break; + case 4: glUniform4uiv(location_for_bound_uniform(name), count, values); break; + } +} + +void Shader::set_uniform_matrix(const GLchar *name, GLint size, bool transpose, const GLfloat *values) +{ + set_uniform_matrix(name, size, 1, transpose, values); +} + +void Shader::set_uniform_matrix(const GLchar *name, GLint size, GLsizei count, bool transpose, const GLfloat *values) +{ + GLboolean glTranspose = transpose ? GL_TRUE : GL_FALSE; + switch(size) + { + case 2: glUniformMatrix2fv(location_for_bound_uniform(name), count, glTranspose, values); break; + case 3: glUniformMatrix3fv(location_for_bound_uniform(name), count, glTranspose, values); break; + case 4: glUniformMatrix4fv(location_for_bound_uniform(name), count, glTranspose, values); break; + } +} diff --git a/Outputs/CRT/Internals/Shaders/Shader.hpp b/Outputs/CRT/Internals/Shaders/Shader.hpp index 94d24cb2f..6cc183b5f 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.hpp +++ b/Outputs/CRT/Internals/Shaders/Shader.hpp @@ -75,9 +75,35 @@ public: */ void enable_vertex_attribute_with_pointer(const char *name, GLint size, GLenum type, GLboolean normalised, GLsizei stride, const GLvoid *pointer, GLuint divisor); + /*! + All @c set_uniforms bind the current shader, look up the uniform by name and set the supplied value or values. + */ + void set_uniform(const GLchar *name, GLint value); + void set_uniform(const GLchar *name, GLint value1, GLint value2); + void set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3); + void set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3, GLint value4); + void set_uniform(const GLchar *name, GLint size, GLsizei count, const GLint *values); + + void set_uniform(const GLchar *name, GLfloat value); + void set_uniform(const GLchar *name, GLfloat value1, GLfloat value2); + void set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3); + void set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3, GLfloat value4); + void set_uniform(const GLchar *name, GLint size, GLsizei count, const GLfloat *values); + + void set_uniform(const GLchar *name, GLuint value); + void set_uniform(const GLchar *name, GLuint value1, GLuint value2); + void set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3); + void set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3, GLuint value4); + void set_uniform(const GLchar *name, GLint size, GLsizei count, const GLuint *values); + + void set_uniform_matrix(const GLchar *name, GLint size, bool transpose, const GLfloat *values); + void set_uniform_matrix(const GLchar *name, GLint size, GLsizei count, bool transpose, const GLfloat *values); + private: GLuint compile_shader(const char *source, GLenum type); GLuint _shader_program; + + GLint location_for_bound_uniform(const GLchar *name); }; } From cca53598d3fab1ef092f60308716638545bf7ec1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 14 May 2016 16:52:24 -0400 Subject: [PATCH 2/6] Made another run at 2600 colours. --- Machines/Atari2600/Atari2600.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Machines/Atari2600/Atari2600.cpp b/Machines/Atari2600/Atari2600.cpp index 5615bcebb..835352a92 100644 --- a/Machines/Atari2600/Atari2600.cpp +++ b/Machines/Atari2600/Atari2600.cpp @@ -39,7 +39,7 @@ void Machine::setup_output(float aspect_ratio) "uint y = c & 14u;" "uint iPhase = (c >> 4);" - "float phaseOffset = 6.283185308 * float(iPhase + 13u) / 14.0;" + "float phaseOffset = 6.283185308 * float(iPhase - 1u) / 13.0;" "return (float(y) / 14.0) * (1.0 - amplitude) + step(1, iPhase) * amplitude * cos(phase + phaseOffset);" "}"); _crt->set_output_device(Outputs::CRT::Television); @@ -55,8 +55,10 @@ void Machine::switch_region() "uint y = c & 14u;" "uint iPhase = (c >> 4);" - "float phaseOffset = (0.5 + 2.0 * (float(iPhase&1u) - 0.5) * (float((iPhase >> 1) + (iPhase&1u)) / 14.0));" - "return (float(y) / 14.0) * (1.0 - amplitude) + step(4, (iPhase + 2u) & 15u) * amplitude * cos(phase + 6.283185308 * phaseOffset);" + "uint direction = iPhase & 1u;" + "float phaseOffset = float(7u - direction) + (float(direction) - 0.5) * 2.0 * float(iPhase >> 1);" + "phaseOffset *= 6.283185308 / 12.0;" + "return (float(y) / 14.0) * (1.0 - amplitude) + step(4, (iPhase + 2u) & 15u) * amplitude * cos(phase + phaseOffset);" "}"); _crt->set_new_timing(228, 312, Outputs::CRT::ColourSpace::YUV, 228, 1); } @@ -171,6 +173,9 @@ void Machine::get_output_pixel(uint8_t *pixel, int offset) } // store colour +// static int lc; +// if(_vSyncEnabled) lc = 0; else lc += (offset == 159) ? 1 : 0; +// *pixel = (uint8_t)(((offset / 10) << 4) | (((lc >> 4)&7) << 1)); *pixel = outputColour; } From 492dc7ccbfea6e2286f704d3c5e328f9b2684097 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 14 May 2016 18:06:55 -0400 Subject: [PATCH 3/6] Made an attempt to queue uniform changes until the next call to `bind`. That's to allow usage from other threads. --- Outputs/CRT/Internals/Shaders/Shader.cpp | 170 ++++++++++++++--------- Outputs/CRT/Internals/Shaders/Shader.hpp | 43 +++--- 2 files changed, 132 insertions(+), 81 deletions(-) diff --git a/Outputs/CRT/Internals/Shaders/Shader.cpp b/Outputs/CRT/Internals/Shaders/Shader.cpp index 336e99edd..70c574e1a 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.cpp +++ b/Outputs/CRT/Internals/Shaders/Shader.cpp @@ -95,6 +95,7 @@ void Shader::bind() glUseProgram(_shader_program); bound_shader = this; } + flush_functions(); } void Shader::unbind() @@ -122,117 +123,162 @@ void Shader::enable_vertex_attribute_with_pointer(const char *name, GLint size, } // The various set_uniforms... -GLint Shader::location_for_bound_uniform(const GLchar *name) +#define location() glGetUniformLocation(_shader_program, name.c_str()) +void Shader::set_uniform(const std::string &name, GLint value) { - bind(); - return glGetUniformLocation(_shader_program, name); + enqueue_function([name, value, this] { + glUniform1i(location(), value); + }); } -void Shader::set_uniform(const GLchar *name, GLint value) +void Shader::set_uniform(const std::string &name, GLuint value) { - glUniform1i(location_for_bound_uniform(name), value); + enqueue_function([name, value, this] { + glUniform1ui(location(), value); + }); } -void Shader::set_uniform(const GLchar *name, GLint value1, GLint value2) +void Shader::set_uniform(const std::string &name, GLfloat value) { - glUniform2i(location_for_bound_uniform(name), value1, value2); + enqueue_function([name, value, this] { + glUniform1f(location(), value); + }); } -void Shader::set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3) + +void Shader::set_uniform(const std::string &name, GLint value1, GLint value2) { - glUniform3i(location_for_bound_uniform(name), value1, value2, value3); + enqueue_function([name, value1, value2, this] { + glUniform2i(location(), value1, value2); + }); } -void Shader::set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3, GLint value4) +void Shader::set_uniform(const std::string &name, GLfloat value1, GLfloat value2) { - glUniform4i(location_for_bound_uniform(name), value1, value2, value3, value4); + enqueue_function([name, value1, value2, this] { + glUniform2f(location(), value1, value2); + }); } -void Shader::set_uniform(const GLchar *name, GLint size, GLsizei count, const GLint *values) +void Shader::set_uniform(const std::string &name, GLuint value1, GLuint value2) { - switch(size) - { - case 1: glUniform1iv(location_for_bound_uniform(name), count, values); break; - case 2: glUniform2iv(location_for_bound_uniform(name), count, values); break; - case 3: glUniform3iv(location_for_bound_uniform(name), count, values); break; - case 4: glUniform4iv(location_for_bound_uniform(name), count, values); break; - } + enqueue_function([name, value1, value2, this] { + glUniform2ui(location(), value1, value2); + }); } -void Shader::set_uniform(const GLchar *name, GLfloat value) + +void Shader::set_uniform(const std::string &name, GLint value1, GLint value2, GLint value3) { - glUniform1f(location_for_bound_uniform(name), value); + enqueue_function([name, value1, value2, value3, this] { + glUniform3i(location(), value1, value2, value3); + }); } -void Shader::set_uniform(const GLchar *name, GLfloat value1, GLfloat value2) +void Shader::set_uniform(const std::string &name, GLfloat value1, GLfloat value2, GLfloat value3) { - glUniform2f(location_for_bound_uniform(name), value1, value2); + enqueue_function([name, value1, value2, value3, this] { + glUniform3f(location(), value1, value2, value3); + }); } -void Shader::set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3) +void Shader::set_uniform(const std::string &name, GLuint value1, GLuint value2, GLuint value3) { - glUniform3f(location_for_bound_uniform(name), value1, value2, value3); + enqueue_function([name, value1, value2, value3, this] { + glUniform3ui(location(), value1, value2, value3); + }); } -void Shader::set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3, GLfloat value4) + +void Shader::set_uniform(const std::string &name, GLint value1, GLint value2, GLint value3, GLint value4) { - glUniform4f(location_for_bound_uniform(name), value1, value2, value3, value4); + enqueue_function([name, value1, value2, value3, value4, this] { + glUniform4i(location(), value1, value2, value3, value4); + }); } -void Shader::set_uniform(const GLchar *name, GLint size, GLsizei count, const GLfloat *values) +void Shader::set_uniform(const std::string &name, GLfloat value1, GLfloat value2, GLfloat value3, GLfloat value4) { - switch(size) - { - case 1: glUniform1fv(location_for_bound_uniform(name), count, values); break; - case 2: glUniform2fv(location_for_bound_uniform(name), count, values); break; - case 3: glUniform3fv(location_for_bound_uniform(name), count, values); break; - case 4: glUniform4fv(location_for_bound_uniform(name), count, values); break; - } + enqueue_function([name, value1, value2, value3, value4, this] { + glUniform4f(location(), value1, value2, value3, value4); + }); } -void Shader::set_uniform(const GLchar *name, GLuint value) +void Shader::set_uniform(const std::string &name, GLuint value1, GLuint value2, GLuint value3, GLuint value4) { - glUniform1ui(location_for_bound_uniform(name), value); + enqueue_function([name, value1, value2, value3, value4, this] { + glUniform4ui(location(), value1, value2, value3, value4); + }); } -void Shader::set_uniform(const GLchar *name, GLuint value1, GLuint value2) + +void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLint *values) { - glUniform2ui(location_for_bound_uniform(name), value1, value2); + enqueue_function([name, size, count, values, this] { + switch(size) + { + case 1: glUniform1iv(location(), count, values); break; + case 2: glUniform2iv(location(), count, values); break; + case 3: glUniform3iv(location(), count, values); break; + case 4: glUniform4iv(location(), count, values); break; + } + }); } -void Shader::set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3) +void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLfloat *values) { - glUniform3ui(location_for_bound_uniform(name), value1, value2, value3); + enqueue_function([name, size, count, values, this] { + switch(size) + { + case 1: glUniform1fv(location(), count, values); break; + case 2: glUniform2fv(location(), count, values); break; + case 3: glUniform3fv(location(), count, values); break; + case 4: glUniform4fv(location(), count, values); break; + } + }); } -void Shader::set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3, GLuint value4) +void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLuint *values) { - glUniform4ui(location_for_bound_uniform(name), value1, value2, value3, value4); + enqueue_function([name, size, count, values, this] { + switch(size) + { + case 1: glUniform1uiv(location(), count, values); break; + case 2: glUniform2uiv(location(), count, values); break; + case 3: glUniform3uiv(location(), count, values); break; + case 4: glUniform4uiv(location(), count, values); break; + } + }); } -void Shader::set_uniform(const GLchar *name, GLint size, GLsizei count, const GLuint *values) -{ - switch(size) - { - case 1: glUniform1uiv(location_for_bound_uniform(name), count, values); break; - case 2: glUniform2uiv(location_for_bound_uniform(name), count, values); break; - case 3: glUniform3uiv(location_for_bound_uniform(name), count, values); break; - case 4: glUniform4uiv(location_for_bound_uniform(name), count, values); break; - } -} - -void Shader::set_uniform_matrix(const GLchar *name, GLint size, bool transpose, const GLfloat *values) +void Shader::set_uniform_matrix(const std::string &name, GLint size, bool transpose, const GLfloat *values) { set_uniform_matrix(name, size, 1, transpose, values); } -void Shader::set_uniform_matrix(const GLchar *name, GLint size, GLsizei count, bool transpose, const GLfloat *values) +void Shader::set_uniform_matrix(const std::string &name, GLint size, GLsizei count, bool transpose, const GLfloat *values) { + enqueue_function([name, size, count, transpose, values, this] { GLboolean glTranspose = transpose ? GL_TRUE : GL_FALSE; - switch(size) - { - case 2: glUniformMatrix2fv(location_for_bound_uniform(name), count, glTranspose, values); break; - case 3: glUniformMatrix3fv(location_for_bound_uniform(name), count, glTranspose, values); break; - case 4: glUniformMatrix4fv(location_for_bound_uniform(name), count, glTranspose, values); break; - } + switch(size) + { + case 2: glUniformMatrix2fv(location(), count, glTranspose, values); break; + case 3: glUniformMatrix3fv(location(), count, glTranspose, values); break; + case 4: glUniformMatrix4fv(location(), count, glTranspose, values); break; + } + }); +} + +void Shader::enqueue_function(std::function function) +{ + _enqueued_functions.push_back(function); +} + +void Shader::flush_functions() +{ + for(std::function function : _enqueued_functions) + { + function(); + } + _enqueued_functions.clear(); } diff --git a/Outputs/CRT/Internals/Shaders/Shader.hpp b/Outputs/CRT/Internals/Shaders/Shader.hpp index 6cc183b5f..cea47d957 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.hpp +++ b/Outputs/CRT/Internals/Shaders/Shader.hpp @@ -10,6 +10,9 @@ #define Shader_hpp #include "OpenGL.hpp" +#include +#include +#include namespace OpenGL { @@ -76,34 +79,36 @@ public: void enable_vertex_attribute_with_pointer(const char *name, GLint size, GLenum type, GLboolean normalised, GLsizei stride, const GLvoid *pointer, GLuint divisor); /*! - All @c set_uniforms bind the current shader, look up the uniform by name and set the supplied value or values. + All @c set_uniforms queue up the requested uniform changes. Changes are applied automatically the next time the shader is bound. */ - void set_uniform(const GLchar *name, GLint value); - void set_uniform(const GLchar *name, GLint value1, GLint value2); - void set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3); - void set_uniform(const GLchar *name, GLint value1, GLint value2, GLint value3, GLint value4); - void set_uniform(const GLchar *name, GLint size, GLsizei count, const GLint *values); + void set_uniform(const std::string &name, GLint value); + void set_uniform(const std::string &name, GLint value1, GLint value2); + void set_uniform(const std::string &name, GLint value1, GLint value2, GLint value3); + void set_uniform(const std::string &name, GLint value1, GLint value2, GLint value3, GLint value4); + void set_uniform(const std::string &name, GLint size, GLsizei count, const GLint *values); - void set_uniform(const GLchar *name, GLfloat value); - void set_uniform(const GLchar *name, GLfloat value1, GLfloat value2); - void set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3); - void set_uniform(const GLchar *name, GLfloat value1, GLfloat value2, GLfloat value3, GLfloat value4); - void set_uniform(const GLchar *name, GLint size, GLsizei count, const GLfloat *values); + void set_uniform(const std::string &name, GLfloat value); + void set_uniform(const std::string &name, GLfloat value1, GLfloat value2); + void set_uniform(const std::string &name, GLfloat value1, GLfloat value2, GLfloat value3); + void set_uniform(const std::string &name, GLfloat value1, GLfloat value2, GLfloat value3, GLfloat value4); + void set_uniform(const std::string &name, GLint size, GLsizei count, const GLfloat *values); - void set_uniform(const GLchar *name, GLuint value); - void set_uniform(const GLchar *name, GLuint value1, GLuint value2); - void set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3); - void set_uniform(const GLchar *name, GLuint value1, GLuint value2, GLuint value3, GLuint value4); - void set_uniform(const GLchar *name, GLint size, GLsizei count, const GLuint *values); + void set_uniform(const std::string &name, GLuint value); + void set_uniform(const std::string &name, GLuint value1, GLuint value2); + void set_uniform(const std::string &name, GLuint value1, GLuint value2, GLuint value3); + void set_uniform(const std::string &name, GLuint value1, GLuint value2, GLuint value3, GLuint value4); + void set_uniform(const std::string &name, GLint size, GLsizei count, const GLuint *values); - void set_uniform_matrix(const GLchar *name, GLint size, bool transpose, const GLfloat *values); - void set_uniform_matrix(const GLchar *name, GLint size, GLsizei count, bool transpose, const GLfloat *values); + void set_uniform_matrix(const std::string &name, GLint size, bool transpose, const GLfloat *values); + void set_uniform_matrix(const std::string &name, GLint size, GLsizei count, bool transpose, const GLfloat *values); private: GLuint compile_shader(const char *source, GLenum type); GLuint _shader_program; - GLint location_for_bound_uniform(const GLchar *name); + void enqueue_function(std::function function); + void flush_functions(); + std::list> _enqueued_functions; }; } From 328fabcd10ee3ab3c9fd2d500fe3277bffb6ef93 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 14 May 2016 18:15:10 -0400 Subject: [PATCH 4/6] Ensured `values` aren't dangling pointers. --- Outputs/CRT/Internals/Shaders/Shader.cpp | 60 +++++++++++++++--------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/Outputs/CRT/Internals/Shaders/Shader.cpp b/Outputs/CRT/Internals/Shaders/Shader.cpp index 70c574e1a..59cccd05c 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.cpp +++ b/Outputs/CRT/Internals/Shaders/Shader.cpp @@ -123,7 +123,13 @@ void Shader::enable_vertex_attribute_with_pointer(const char *name, GLint size, } // The various set_uniforms... -#define location() glGetUniformLocation(_shader_program, name.c_str()) +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()) void Shader::set_uniform(const std::string &name, GLint value) { enqueue_function([name, value, this] { @@ -214,40 +220,49 @@ 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) { - enqueue_function([name, size, count, values, this] { + GLint *values_copy = new GLint[count]; + memcpy(values_copy, values, sizeof(GLint) * (size_t)count); + enqueue_function([name, size, count, values_copy, this] { switch(size) { - case 1: glUniform1iv(location(), count, values); break; - case 2: glUniform2iv(location(), count, values); break; - case 3: glUniform3iv(location(), count, values); break; - case 4: glUniform4iv(location(), count, values); break; + case 1: glUniform1iv(location(), count, values_copy); break; + case 2: glUniform2iv(location(), count, values_copy); break; + case 3: glUniform3iv(location(), count, values_copy); break; + case 4: glUniform4iv(location(), count, values_copy); break; } + delete[] values_copy; }); } void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLfloat *values) { - enqueue_function([name, size, count, values, this] { + GLfloat *values_copy = new GLfloat[count]; + memcpy(values_copy, values, sizeof(GLfloat) * (size_t)count); + enqueue_function([name, size, count, values_copy, this] { switch(size) { - case 1: glUniform1fv(location(), count, values); break; - case 2: glUniform2fv(location(), count, values); break; - case 3: glUniform3fv(location(), count, values); break; - case 4: glUniform4fv(location(), count, values); break; + case 1: glUniform1fv(location(), count, values_copy); break; + case 2: glUniform2fv(location(), count, values_copy); break; + case 3: glUniform3fv(location(), count, values_copy); break; + case 4: glUniform4fv(location(), count, values_copy); break; } + delete[] values_copy; }); } void Shader::set_uniform(const std::string &name, GLint size, GLsizei count, const GLuint *values) { - enqueue_function([name, size, count, values, this] { + GLuint *values_copy = new GLuint[count]; + memcpy(values_copy, values, sizeof(GLuint) * (size_t)count); + enqueue_function([name, size, count, values_copy, this] { switch(size) { - case 1: glUniform1uiv(location(), count, values); break; - case 2: glUniform2uiv(location(), count, values); break; - case 3: glUniform3uiv(location(), count, values); break; - case 4: glUniform4uiv(location(), count, values); break; + case 1: glUniform1uiv(location(), count, values_copy); break; + case 2: glUniform2uiv(location(), count, values_copy); break; + case 3: glUniform3uiv(location(), count, values_copy); break; + case 4: glUniform4uiv(location(), count, values_copy); break; } + delete[] values_copy; }); } @@ -258,14 +273,17 @@ 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) { - enqueue_function([name, size, count, transpose, values, this] { - GLboolean glTranspose = transpose ? GL_TRUE : GL_FALSE; + GLfloat *values_copy = new GLfloat[count*size]; + memcpy(values_copy, values, sizeof(GLfloat) * (size_t)count * (size_t)size); + enqueue_function([name, size, count, transpose, values_copy, this] { + GLboolean glTranspose = transpose ? GL_TRUE : GL_FALSE; switch(size) { - case 2: glUniformMatrix2fv(location(), count, glTranspose, values); break; - case 3: glUniformMatrix3fv(location(), count, glTranspose, values); break; - case 4: glUniformMatrix4fv(location(), count, glTranspose, values); break; + case 2: glUniformMatrix2fv(location(), count, glTranspose, values_copy); break; + case 3: glUniformMatrix3fv(location(), count, glTranspose, values_copy); break; + case 4: glUniformMatrix4fv(location(), count, glTranspose, values_copy); break; } + delete[] values_copy; }); } From 091516e3cb91ea231c84f56d0e180831e5e1565e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 15 May 2016 14:59:59 -0400 Subject: [PATCH 5/6] 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); }; } From 00a2b42080e57508fb2639d40328174eb93eeafa Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 15 May 2016 15:19:52 -0400 Subject: [PATCH 6/6] Made thread-safe. --- Outputs/CRT/Internals/Shaders/Shader.cpp | 4 ++++ Outputs/CRT/Internals/Shaders/Shader.hpp | 2 ++ 2 files changed, 6 insertions(+) diff --git a/Outputs/CRT/Internals/Shaders/Shader.cpp b/Outputs/CRT/Internals/Shaders/Shader.cpp index f4968b752..f0e2fbb26 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.cpp +++ b/Outputs/CRT/Internals/Shaders/Shader.cpp @@ -291,14 +291,18 @@ void Shader::set_uniform_matrix(const std::string &name, GLint size, GLsizei cou void Shader::enqueue_function(std::function function) { + _function_mutex.lock(); _enqueued_functions.push_back(function); + _function_mutex.unlock(); } void Shader::flush_functions() { + _function_mutex.lock(); for(std::function function : _enqueued_functions) { function(); } _enqueued_functions.clear(); + _function_mutex.unlock(); } diff --git a/Outputs/CRT/Internals/Shaders/Shader.hpp b/Outputs/CRT/Internals/Shaders/Shader.hpp index ead7ce397..b28335032 100644 --- a/Outputs/CRT/Internals/Shaders/Shader.hpp +++ b/Outputs/CRT/Internals/Shaders/Shader.hpp @@ -13,6 +13,7 @@ #include #include #include +#include namespace OpenGL { @@ -110,6 +111,7 @@ private: void flush_functions(); std::list> _enqueued_functions; + std::mutex _function_mutex; protected: void enqueue_function(std::function function);