Allow inputs to say they cannot support mipmaps.

Really only FlatInput can easily support mipmaps; for things like YCbCrInput
that combine multiple inputs, it's hard (probably not downright impossible,
but at least not immediately obvious without thinking about it a bit) and for
FFTInput it makes no sense.

Thus, we allow an input to say that it can't do this, and then bounce it
to a texture if needed. Hopefully this should happen quite rarely.

diff --git a/effect_chain.cpp b/effect_chain.cpp
index 21c4a0d1d2cf84e8a2eb3f80153715557aab8d1c..b00f214aa0f535e26570bca1f0df258814cc9858 100644 (file)
--- a/effect_chain.cpp
+++ b/effect_chain.cpp
@@ -87,6 +87,7 @@ Node *EffectChain::add_node(Effect *effect)
        node->output_color_space = COLORSPACE_INVALID;
        node->output_gamma_curve = GAMMA_INVALID;
        node->output_alpha_type = ALPHA_INVALID;
+       node->needs_mipmaps = false;
 
        nodes.push_back(node);
        node_map[effect] = node;
@@ -310,6 +311,10 @@ Phase *EffectChain::construct_phase(Node *output, map<Node *, Phase *> *complete
                Node *node = effects_todo_this_phase.top();
                effects_todo_this_phase.pop();
 
+               if (node->effect->needs_mipmaps()) {
+                       node->needs_mipmaps = true;
+               }
+
                // This should currently only happen for effects that are inputs
                // (either true inputs or phase outputs). We special-case inputs,
                // and then deduplicate phase outputs below.
@@ -334,6 +339,21 @@ Phase *EffectChain::construct_phase(Node *output, map<Node *, Phase *> *complete
                                start_new_phase = true;
                        }
 
+                       // Propagate information about needing mipmaps down the chain,
+                       // breaking the phase if we notice an incompatibility.
+                       //
+                       // Note that we cannot do this propagation as a normal pass,
+                       // because it needs information about where the phases end
+                       // (we should not propagate the flag across phases).
+                       if (node->needs_mipmaps) {
+                               if (deps[i]->effect->num_inputs() == 0) {
+                                       Input *input = static_cast<Input *>(deps[i]->effect);
+                                       start_new_phase |= !input->can_supply_mipmaps();
+                               } else {
+                                       deps[i]->needs_mipmaps = true;
+                               }
+                       }
+
                        if (deps[i]->outgoing_links.size() > 1) {
                                if (!deps[i]->effect->is_single_texture()) {
                                        // More than one effect uses this as the input,
@@ -392,7 +412,9 @@ Phase *EffectChain::construct_phase(Node *output, map<Node *, Phase *> *complete
        for (unsigned i = 0; i < phase->effects.size(); ++i) {
                Node *node = phase->effects[i];
                if (node->effect->num_inputs() == 0) {
-                       CHECK(node->effect->set_int("needs_mipmaps", phase->input_needs_mipmaps));
+                       Input *input = static_cast<Input *>(node->effect);
+                       assert(!phase->input_needs_mipmaps || input->can_supply_mipmaps());
+                       CHECK(input->set_int("needs_mipmaps", phase->input_needs_mipmaps));
                }
        }
 

diff --git a/effect_chain.h b/effect_chain.h
index 593232cf01e036f47728ae1fd3d2c756bf6d3d85..358deedbb5e34064e4d2c52119db4c4c30413dca 100644 (file)
--- a/effect_chain.h
+++ b/effect_chain.h
@@ -83,6 +83,7 @@ private:
        Colorspace output_color_space;
        GammaCurve output_gamma_curve;
        AlphaType output_alpha_type;
+       bool needs_mipmaps;  // Directly or indirectly.
 
        friend class EffectChain;
 };

diff --git a/effect_chain_test.cpp b/effect_chain_test.cpp
index dd4ffa2254835978c77b7243a563867fd07a9411..516dd54ea00e84eaeaac7e2afd2e58b1f545343e 100644 (file)
--- a/effect_chain_test.cpp
+++ b/effect_chain_test.cpp
@@ -604,6 +604,81 @@ TEST(EffectChainTest, MipmapGenerationWorks) {
        expect_equal(expected_data, out_data, 4, 16);
 }
 
+class NonMipmapCapableInput : public FlatInput {
+public:
+       NonMipmapCapableInput(ImageFormat format, MovitPixelFormat pixel_format, GLenum type, unsigned width, unsigned height)
+               : FlatInput(format, pixel_format, type, width, height) {}
+
+       virtual bool can_supply_mipmaps() const { return false; }
+       bool set_int(const std::string& key, int value) {
+               if (key == "needs_mipmaps") {
+                       assert(value == 0);
+               }
+               return FlatInput::set_int(key, value);
+       }
+};
+
+// The same test as MipmapGenerationWorks, but with an input that refuses
+// to supply mipmaps.
+TEST(EffectChainTest, MipmapsWithNonMipmapCapableInput) {
+       float data[] = {  // In 4x4 blocks.
+               1.0f, 0.0f, 0.0f, 0.0f,
+               0.0f, 0.0f, 0.0f, 0.0f,
+               0.0f, 0.0f, 0.0f, 0.0f,
+               0.0f, 0.0f, 0.0f, 1.0f,
+
+               0.0f, 0.0f, 0.0f, 0.0f,
+               0.0f, 0.5f, 0.0f, 0.0f,
+               0.0f, 0.0f, 1.0f, 0.0f,
+               0.0f, 0.0f, 0.0f, 0.0f,
+
+               1.0f, 1.0f, 1.0f, 1.0f,
+               1.0f, 1.0f, 1.0f, 1.0f,
+               1.0f, 1.0f, 1.0f, 1.0f,
+               1.0f, 1.0f, 1.0f, 1.0f,
+
+               0.0f, 0.0f, 0.0f, 0.0f,
+               0.0f, 1.0f, 1.0f, 0.0f,
+               0.0f, 1.0f, 1.0f, 0.0f,
+               0.0f, 0.0f, 0.0f, 0.0f,
+       };
+       float expected_data[] = {  // Repeated four times each way.
+               0.125f,   0.125f,   0.125f,   0.125f,
+               0.09375f, 0.09375f, 0.09375f, 0.09375f,
+               1.0f,     1.0f,     1.0f,     1.0f,
+               0.25f,    0.25f,    0.25f,    0.25f,
+
+               0.125f,   0.125f,   0.125f,   0.125f,
+               0.09375f, 0.09375f, 0.09375f, 0.09375f,
+               1.0f,     1.0f,     1.0f,     1.0f,
+               0.25f,    0.25f,    0.25f,    0.25f,
+
+               0.125f,   0.125f,   0.125f,   0.125f,
+               0.09375f, 0.09375f, 0.09375f, 0.09375f,
+               1.0f,     1.0f,     1.0f,     1.0f,
+               0.25f,    0.25f,    0.25f,    0.25f,
+
+               0.125f,   0.125f,   0.125f,   0.125f,
+               0.09375f, 0.09375f, 0.09375f, 0.09375f,
+               1.0f,     1.0f,     1.0f,     1.0f,
+               0.25f,    0.25f,    0.25f,    0.25f,
+       };
+       float out_data[4 * 16];
+       EffectChainTester tester(NULL, 4, 16, FORMAT_GRAYSCALE);
+
+       ImageFormat format;
+       format.color_space = COLORSPACE_sRGB;
+       format.gamma_curve = GAMMA_LINEAR;
+
+       NonMipmapCapableInput *input = new NonMipmapCapableInput(format, FORMAT_GRAYSCALE, GL_FLOAT, 4, 16);
+       input->set_pixel_data(data);
+       tester.get_chain()->add_input(input);
+       tester.get_chain()->add_effect(new MipmapNeedingEffect());
+       tester.run(out_data, GL_RED, COLORSPACE_sRGB, GAMMA_LINEAR);
+
+       expect_equal(expected_data, out_data, 4, 16);
+}
+
 TEST(EffectChainTest, ResizeDownByFourThenUpByFour) {
        float data[] = {  // In 4x4 blocks.
                1.0f, 0.0f, 0.0f, 0.0f,

diff --git a/fft_input.h b/fft_input.h
index b57723cd61a92251bf6a870a6542d585fbf7d28d..a75414cb473ce181ebb3717229cc970a17e00a4e 100644 (file)
--- a/fft_input.h
+++ b/fft_input.h
@@ -48,6 +48,7 @@ public:
        virtual AlphaHandling alpha_handling() const { return INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA; }
        virtual bool is_single_texture() const { return true; }
        virtual bool can_output_linear_gamma() const { return true; }
+       virtual bool can_supply_mipmaps() const { return false; }
 
        // Tells the input where to fetch the actual pixel data. Note that if you change
        // this data, you must either call set_pixel_data() again (using the same pointer

diff --git a/input.h b/input.h
index 7652e1793a4599d01fb726bfbf939741ac01d2aa..08f18da927c161b4ca5c778afee98d34f49b0768 100644 (file)
--- a/input.h
+++ b/input.h
@@ -24,6 +24,10 @@ public:
        // to activate it.)
        virtual bool can_output_linear_gamma() const = 0;
 
+       // Whether this input can supply mipmaps if asked to (by setting
+       // the "needs_mipmaps" integer parameter set to 1).
+       virtual bool can_supply_mipmaps() const { return true; }
+
        virtual unsigned get_width() const = 0;
        virtual unsigned get_height() const = 0;
        virtual Colorspace get_color_space() const = 0;

diff --git a/ycbcr_input.cpp b/ycbcr_input.cpp
index 25fb97932129ca0f7bcb804a59ea4b8b3b9de191..cf6632f2391bcf8fc49afed13b11513dc428b565 100644 (file)
--- a/ycbcr_input.cpp
+++ b/ycbcr_input.cpp
@@ -65,7 +65,6 @@ YCbCrInput::YCbCrInput(const ImageFormat &image_format,
                        unsigned width, unsigned height)
        : image_format(image_format),
          ycbcr_format(ycbcr_format),
-         needs_mipmaps(false),
          width(width),
          height(height),
          resource_pool(NULL)
@@ -84,8 +83,6 @@ YCbCrInput::YCbCrInput(const ImageFormat &image_format,
        heights[2] = height / ycbcr_format.chroma_subsampling_y;
 
        pixel_data[0] = pixel_data[1] = pixel_data[2] = NULL;
-
-       register_int("needs_mipmaps", &needs_mipmaps);
 }
 
 YCbCrInput::~YCbCrInput()

diff --git a/ycbcr_input.h b/ycbcr_input.h
index bf6d800eeb0c918c1b535bd2f6bf2c65c0bfa354..30492770ac436af8d2ff209e085b921fc549947b 100644 (file)
--- a/ycbcr_input.h
+++ b/ycbcr_input.h
@@ -58,6 +58,7 @@ public:
        unsigned get_height() const { return height; }
        Colorspace get_color_space() const { return image_format.color_space; }
        GammaCurve get_gamma_curve() const { return image_format.gamma_curve; }
+       virtual bool can_supply_mipmaps() const { return false; }
 
        // Tells the input where to fetch the actual pixel data. Note that if you change
        // this data, you must either call set_pixel_data() again (using the same pointer
@@ -95,8 +96,6 @@ private:
        YCbCrFormat ycbcr_format;
        GLuint pbos[3], texture_num[3];
 
-       int needs_mipmaps;
-
        unsigned width, height, widths[3], heights[3];
        const unsigned char *pixel_data[3];
        unsigned pitch[3];

diff --git a/ycbcr_input_test.cpp b/ycbcr_input_test.cpp
index 6895b3834a62602aa293c012957638684b471074..55bb536101b400593c698d1ec4bc9874db54094c 100644 (file)
--- a/ycbcr_input_test.cpp
+++ b/ycbcr_input_test.cpp
@@ -1,5 +1,4 @@
 // Unit tests for YCbCrInput.
-// FIXME: This class really ought to support mipmaps.
 
 #include <epoxy/gl.h>
 #include <stddef.h>