From 825c90789c229f502520bf0b665596d473f2636d Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Sat, 12 Dec 2015 14:31:40 +0100 Subject: [PATCH] Explicitly bind fragment shader outputs in order. Evidently ATI drivers use the freedom the standard gives them to assign these in another order than they are specified in the shader source, so we need to explicitly bind them, or YCbCrConversionEffectTest will fail in the multi-output tests. Originally reported by Iwan Gabovitch. --- effect_chain.cpp | 10 +++++++++- init.cpp | 8 ++++++-- resource_pool.cpp | 29 +++++++++++++++++++++++++---- resource_pool.h | 10 +++++++++- version.h | 2 +- widgets.cpp | 12 ++++++++++-- 6 files changed, 60 insertions(+), 11 deletions(-) diff --git a/effect_chain.cpp b/effect_chain.cpp index 8a72e17..c2402da 100644 --- a/effect_chain.cpp +++ b/effect_chain.cpp @@ -376,16 +376,23 @@ void EffectChain::compile_glsl_program(Phase *phase) frag_shader += string("#define INPUT ") + phase->effect_ids[phase->effects.back()] + "\n"; // If we're the last phase, add the right #defines for Y'CbCr multi-output as needed. + vector frag_shader_outputs; // In order. if (phase->output_node->outgoing_links.empty() && output_color_ycbcr) { switch (output_ycbcr_splitting) { case YCBCR_OUTPUT_INTERLEAVED: // No #defines set. + frag_shader_outputs.push_back("FragColor"); break; case YCBCR_OUTPUT_SPLIT_Y_AND_CBCR: frag_shader += "#define YCBCR_OUTPUT_SPLIT_Y_AND_CBCR 1\n"; + frag_shader_outputs.push_back("Y"); + frag_shader_outputs.push_back("Chroma"); break; case YCBCR_OUTPUT_PLANAR: frag_shader += "#define YCBCR_OUTPUT_PLANAR 1\n"; + frag_shader_outputs.push_back("Y"); + frag_shader_outputs.push_back("Cb"); + frag_shader_outputs.push_back("Cr"); break; default: assert(false); @@ -396,6 +403,7 @@ void EffectChain::compile_glsl_program(Phase *phase) // output needs to see it (YCbCrConversionEffect and DitherEffect // do, too). frag_shader_header += "#define YCBCR_ALSO_OUTPUT_RGBA 1\n"; + frag_shader_outputs.push_back("RGBA"); } } frag_shader.append(read_file("footer.frag")); @@ -439,7 +447,7 @@ void EffectChain::compile_glsl_program(Phase *phase) vert_shader[pos + needle.size() - 1] = '1'; } - phase->glsl_program_num = resource_pool->compile_glsl_program(vert_shader, frag_shader); + phase->glsl_program_num = resource_pool->compile_glsl_program(vert_shader, frag_shader, frag_shader_outputs); // Collect the resulting location numbers for each uniform. collect_uniform_locations(phase->glsl_program_num, &phase->uniforms_sampler2d); diff --git a/init.cpp b/init.cpp index 7781ba2..733d246 100644 --- a/init.cpp +++ b/init.cpp @@ -79,9 +79,11 @@ void measure_texel_subpixel_precision() glViewport(0, 0, width, 1); + vector frag_shader_outputs; GLuint glsl_program_num = resource_pool.compile_glsl_program( read_version_dependent_file("vs", "vert"), - read_version_dependent_file("texture1d", "frag")); + read_version_dependent_file("texture1d", "frag"), + frag_shader_outputs); glUseProgram(glsl_program_num); check_error(); glUniform1i(glGetUniformLocation(glsl_program_num, "tex"), 0); // Bind the 2D sampler. @@ -211,9 +213,11 @@ void measure_roundoff_problems() glViewport(0, 0, 512, 1); + vector frag_shader_outputs; GLuint glsl_program_num = resource_pool.compile_glsl_program( read_version_dependent_file("vs", "vert"), - read_version_dependent_file("texture1d", "frag")); + read_version_dependent_file("texture1d", "frag"), + frag_shader_outputs); glUseProgram(glsl_program_num); check_error(); glUniform1i(glGetUniformLocation(glsl_program_num, "tex"), 0); // Bind the 2D sampler. diff --git a/resource_pool.cpp b/resource_pool.cpp index be74b3d..944910b 100644 --- a/resource_pool.cpp +++ b/resource_pool.cpp @@ -100,11 +100,23 @@ void ResourcePool::delete_program(GLuint glsl_program_num) program_shaders.erase(shader_it); } -GLuint ResourcePool::compile_glsl_program(const string& vertex_shader, const string& fragment_shader) +GLuint ResourcePool::compile_glsl_program(const string& vertex_shader, + const string& fragment_shader, + const vector& fragment_shader_outputs) { GLuint glsl_program_num; pthread_mutex_lock(&lock); - const pair key(vertex_shader, fragment_shader); + + // Augment the fragment shader program text with the outputs, so that they become + // part of the key. Also potentially useful for debugging. + string fragment_shader_processed = fragment_shader; + for (unsigned output_index = 0; output_index < fragment_shader_outputs.size(); ++output_index) { + char buf[256]; + snprintf(buf, sizeof(buf), "// Bound output: %s\n", fragment_shader_outputs[output_index].c_str()); + fragment_shader_processed += buf; + } + + const pair key(vertex_shader, fragment_shader_processed); if (programs.count(key)) { // Already in the cache. Increment the refcount, or take it off the freelist // if it's zero. @@ -125,12 +137,21 @@ GLuint ResourcePool::compile_glsl_program(const string& vertex_shader, const str check_error(); GLuint vs_obj = compile_shader(vertex_shader, GL_VERTEX_SHADER); check_error(); - GLuint fs_obj = compile_shader(fragment_shader, GL_FRAGMENT_SHADER); + GLuint fs_obj = compile_shader(fragment_shader_processed, GL_FRAGMENT_SHADER); check_error(); glAttachShader(glsl_program_num, vs_obj); check_error(); glAttachShader(glsl_program_num, fs_obj); check_error(); + + // Bind the outputs, if we have multiple ones. + if (fragment_shader_outputs.size() > 1) { + for (unsigned output_index = 0; output_index < fragment_shader_outputs.size(); ++output_index) { + glBindFragDataLocation(glsl_program_num, output_index, + fragment_shader_outputs[output_index].c_str()); + } + } + glLinkProgram(glsl_program_num); check_error(); @@ -153,7 +174,7 @@ GLuint ResourcePool::compile_glsl_program(const string& vertex_shader, const str perror(filename); exit(1); } - fprintf(fp, "%s\n", fragment_shader.c_str()); + fprintf(fp, "%s\n", fragment_shader_processed.c_str()); fclose(fp); } diff --git a/resource_pool.h b/resource_pool.h index 8881958..5cc2e82 100644 --- a/resource_pool.h +++ b/resource_pool.h @@ -29,6 +29,7 @@ #include #include #include +#include namespace movit { @@ -56,7 +57,14 @@ public: // compiled program from the cache if possible. Keeps ownership of the // program; you must call release_glsl_program() instead of deleting it // when you no longer want it. - GLuint compile_glsl_program(const std::string& vertex_shader, const std::string& fragment_shader); + // + // If contains more than one value, the given + // outputs will be bound to fragment shader output colors in the order + // they appear in the vector. Otherwise, output order is undefined and + // determined by the OpenGL driver. + GLuint compile_glsl_program(const std::string& vertex_shader, + const std::string& fragment_shader, + const std::vector& frag_shader_outputs); void release_glsl_program(GLuint glsl_program_num); // Allocate a 2D texture of the given internal format and dimensions, diff --git a/version.h b/version.h index 1d6ab91..3f863c0 100644 --- a/version.h +++ b/version.h @@ -5,6 +5,6 @@ // changes, even within git versions. There is no specific version // documentation outside the regular changelogs, though. -#define MOVIT_VERSION 14 +#define MOVIT_VERSION 15 #endif // !defined(_MOVIT_VERSION_H) diff --git a/widgets.cpp b/widgets.cpp index 529d902..66ec7c5 100644 --- a/widgets.cpp +++ b/widgets.cpp @@ -1,12 +1,17 @@ #include #include +#include +#include + #include "resource_pool.h" #include "widgets.h" #include "util.h" #define HSV_WHEEL_SIZE 128 +using namespace std; + namespace movit { GLuint hsv_wheel_texnum = 0; @@ -188,12 +193,15 @@ void make_hsv_wheel_texture() void init_hsv_resources() { + vector frag_shader_outputs; textured_program_num = resource_pool.compile_glsl_program( read_version_dependent_file("vs", "vert"), - read_version_dependent_file("texture1d", "frag")); + read_version_dependent_file("texture1d", "frag"), + frag_shader_outputs); colored_program_num = resource_pool.compile_glsl_program( read_version_dependent_file("vs-color", "vert"), - read_version_dependent_file("color", "frag")); + read_version_dependent_file("color", "frag"), + frag_shader_outputs); make_hsv_wheel_texture(); } -- 2.39.5