From e9f0fb5e6ae193a5a853ac5aef82927b6a81267a Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Mon, 10 Mar 2014 23:59:28 +0100 Subject: [PATCH] Formalize the notion of messing with sampler state. This kills a lot of the assumptions that have been going around, and should allow us to deal much better with the situation when we have two or more inputs to an effect (where you basically can't predict the sampler number used reliably); there's still an edge case that's documented with a TODO, but this is generally much better. --- effect_chain.cpp | 18 ++++++++++++++++++ effect_chain.h | 19 +++++++++++++++++++ effect_chain_test.cpp | 12 +++++++++++- fft_pass_effect.cpp | 17 ++++++++--------- fft_pass_effect.h | 3 +++ resample_effect.cpp | 3 ++- resample_effect.h | 2 ++ slice_effect.cpp | 11 +++++------ slice_effect.h | 2 ++ 9 files changed, 70 insertions(+), 17 deletions(-) diff --git a/effect_chain.cpp b/effect_chain.cpp index 22ba189..c886155 100644 --- a/effect_chain.cpp +++ b/effect_chain.cpp @@ -149,6 +149,15 @@ void EffectChain::insert_node_between(Node *sender, Node *middle, Node *receiver assert(middle->incoming_links.size() == middle->effect->num_inputs()); } +GLenum EffectChain::get_input_sampler(Node *node, unsigned input_num) const +{ + assert(node->effect->needs_texture_bounce()); + assert(input_num < node->incoming_links.size()); + assert(node->incoming_links[input_num]->bound_sampler_num >= 0); + assert(node->incoming_links[input_num]->bound_sampler_num < 8); + return GL_TEXTURE0 + node->incoming_links[input_num]->bound_sampler_num; +} + void EffectChain::find_all_nonlinear_inputs(Node *node, vector *nonlinear_inputs) { if (node->output_gamma_curve == GAMMA_LINEAR && @@ -1480,6 +1489,7 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height for (unsigned sampler = 0; sampler < phases[phase]->inputs.size(); ++sampler) { glActiveTexture(GL_TEXTURE0 + sampler); Node *input = phases[phase]->inputs[sampler]; + input->bound_sampler_num = sampler; glBindTexture(GL_TEXTURE_2D, output_textures[input->phase]); check_error(); if (phases[phase]->input_needs_mipmaps) { @@ -1533,8 +1543,16 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height unsigned sampler_num = phases[phase]->inputs.size(); for (unsigned i = 0; i < phases[phase]->effects.size(); ++i) { Node *node = phases[phase]->effects[i]; + unsigned old_sampler_num = sampler_num; node->effect->set_gl_state(glsl_program_num, phases[phase]->effect_ids[node], &sampler_num); check_error(); + + if (node->effect->is_single_texture()) { + assert(sampler_num - old_sampler_num == 1); + node->bound_sampler_num = old_sampler_num; + } else { + node->bound_sampler_num = -1; + } } // Now draw! diff --git a/effect_chain.h b/effect_chain.h index 642f079..4417de9 100644 --- a/effect_chain.h +++ b/effect_chain.h @@ -71,6 +71,15 @@ private: // phases as inputs, instead of Node. Phase *phase; + // If the effect has is_single_texture(), or if the output went to RTT + // and that texture has been bound to a sampler, the sampler number + // will be stored here. + // + // TODO: Can an RTT texture be used as inputs to multiple effects + // within the same phase? If so, we have a problem with modifying + // sampler state here. + int bound_sampler_num; + // Used during the building of the effect chain. Colorspace output_color_space; GammaCurve output_gamma_curve; @@ -173,6 +182,16 @@ public: void replace_receiver(Node *old_receiver, Node *new_receiver); void replace_sender(Node *new_sender, Node *receiver); void insert_node_between(Node *sender, Node *middle, Node *receiver); + Node *find_node_for_effect(Effect *effect) { return node_map[effect]; } + + // Get the OpenGL sampler (GL_TEXTURE0, GL_TEXTURE1, etc.) for the + // input of the given node, so that one can modify the sampler state + // directly. Only valid to call during set_gl_state(). + // + // Also, for this to be allowed, 's effect must have + // needs_texture_bounce() set, so that it samples directly from a + // single-sampler input, or from an RTT texture. + GLenum get_input_sampler(Node *node, unsigned input_num) const; // Get the current resource pool assigned to this EffectChain. // Primarily to let effects allocate textures as needed. diff --git a/effect_chain_test.cpp b/effect_chain_test.cpp index cf725f1..b1f0815 100644 --- a/effect_chain_test.cpp +++ b/effect_chain_test.cpp @@ -522,17 +522,27 @@ class MipmapNeedingEffect : public Effect { public: MipmapNeedingEffect() {} virtual bool needs_mipmaps() const { return true; } + + // To be allowed to mess with the sampler state. + virtual bool needs_texture_bounce() const { return true; } + virtual string effect_type_id() const { return "MipmapNeedingEffect"; } string output_fragment_shader() { return read_file("mipmap_needing_effect.frag"); } + virtual void inform_added(EffectChain *chain) { this->chain = chain; } + void set_gl_state(GLuint glsl_program_num, const string& prefix, unsigned *sampler_num) { - glActiveTexture(GL_TEXTURE0); + Node *self = chain->find_node_for_effect(this); + glActiveTexture(chain->get_input_sampler(self, 0)); check_error(); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT); check_error(); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT); check_error(); } + +private: + EffectChain *chain; }; TEST(EffectChainTest, MipmapGenerationWorks) { diff --git a/fft_pass_effect.cpp b/fft_pass_effect.cpp index c2b47f0..9c8aeb0 100644 --- a/fft_pass_effect.cpp +++ b/fft_pass_effect.cpp @@ -1,6 +1,7 @@ #include #include +#include "effect_chain.h" #include "effect_util.h" #include "fp16.h" #include "fft_pass_effect.h" @@ -40,15 +41,13 @@ void FFTPassEffect::set_gl_state(GLuint glsl_program_num, const string &prefix, int input_size = (direction == VERTICAL) ? input_height : input_width; - // See the comments on changes_output_size() in the .h file to see - // why this is legal. It is _needed_ because it counteracts the - // precision issues we get because we sample the input texture with - // normalized coordinates (especially when the repeat count along - // the axis is not a power of two); we very rapidly end up in narrowly - // missing a texel center, which causes precision loss to propagate - // throughout the FFT. - assert(*sampler_num == 1); - glActiveTexture(GL_TEXTURE0); + // This is needed because it counteracts the precision issues we get + // because we sample the input texture with normalized coordinates + // (especially when the repeat count along the axis is not a power of + // two); we very rapidly end up in narrowly missing a texel center, + // which causes precision loss to propagate throughout the FFT. + Node *self = chain->find_node_for_effect(this); + glActiveTexture(chain->get_input_sampler(self, 0)); check_error(); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); check_error(); diff --git a/fft_pass_effect.h b/fft_pass_effect.h index 1fd4ea2..460a946 100644 --- a/fft_pass_effect.h +++ b/fft_pass_effect.h @@ -98,10 +98,13 @@ public: *width = *virtual_width = input_width; *height = *virtual_height = input_height; } + + virtual void inform_added(EffectChain *chain) { this->chain = chain; } enum Direction { HORIZONTAL = 0, VERTICAL = 1 }; private: + EffectChain *chain; int input_width, input_height; GLuint tex; int fft_size; diff --git a/resample_effect.cpp b/resample_effect.cpp index 4af60de..a677487 100644 --- a/resample_effect.cpp +++ b/resample_effect.cpp @@ -413,7 +413,8 @@ void SingleResamplePassEffect::set_gl_state(GLuint glsl_program_num, const strin // We specifically do not want mipmaps on the input texture; // they break minification. - glActiveTexture(GL_TEXTURE0); + Node *self = chain->find_node_for_effect(this); + glActiveTexture(chain->get_input_sampler(self, 0)); check_error(); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR); check_error(); diff --git a/resample_effect.h b/resample_effect.h index 628c61a..44587bf 100644 --- a/resample_effect.h +++ b/resample_effect.h @@ -73,6 +73,7 @@ public: virtual bool needs_srgb_primaries() const { return false; } virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } + virtual void inform_added(EffectChain *chain) { this->chain = chain; } virtual void inform_input_size(unsigned input_num, unsigned width, unsigned height) { if (parent != NULL) { parent->inform_input_size(input_num, width, height); @@ -93,6 +94,7 @@ private: void update_texture(GLuint glsl_program_num, const std::string &prefix, unsigned *sampler_num); ResampleEffect *parent; + EffectChain *chain; Direction direction; GLuint texnum; int input_width, input_height, output_width, output_height; diff --git a/slice_effect.cpp b/slice_effect.cpp index 09b91b9..15f5392 100644 --- a/slice_effect.cpp +++ b/slice_effect.cpp @@ -1,5 +1,6 @@ #include +#include "effect_chain.h" #include "slice_effect.h" #include "effect_util.h" #include "util.h" @@ -60,12 +61,10 @@ void SliceEffect::set_gl_state(GLuint glsl_program_num, const string &prefix, un set_uniform_float(glsl_program_num, prefix, "slice_offset_to_input_coord", float(output_slice_size) / float(input_height)); } - // Normalized coordinates could potentially cause blurring of the - // image; it's not critical, but we have set changes_output_size() - // and needs_texture_bounce(), so simply turning off the interpolation - // is allowed. - assert(*sampler_num == 1); - glActiveTexture(GL_TEXTURE0); + // Normalized coordinates could potentially cause blurring of the image. + // It isn't critical, but still good practice. + Node *self = chain->find_node_for_effect(this); + glActiveTexture(chain->get_input_sampler(self, 0)); check_error(); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); check_error(); diff --git a/slice_effect.h b/slice_effect.h index 6380c16..17202d7 100644 --- a/slice_effect.h +++ b/slice_effect.h @@ -29,10 +29,12 @@ public: unsigned *virtual_width, unsigned *virtual_height) const; void set_gl_state(GLuint glsl_program_num, const std::string &prefix, unsigned *sampler_num); + virtual void inform_added(EffectChain *chain) { this->chain = chain; } enum Direction { HORIZONTAL = 0, VERTICAL = 1 }; private: + EffectChain *chain; int input_width, input_height; int input_slice_size, output_slice_size; Direction direction; -- 2.39.2