From: Steinar H. Gunderson Date: Sat, 2 Feb 2013 01:35:18 +0000 (+0100) Subject: Add a new alpha handling method, INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK. X-Git-Tag: 1.0~141 X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=7af4d1b54ba141fdb74cd13ddc6110708855d157 Add a new alpha handling method, INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK. This should fix most of the problems where you only process inputs without alpha, but the framework still inserts an AlphaDivisionEffect at the end because it loses track of the blank alpha state throughout the pipeline and the output expects postmultiplied alpha. There's one important case left, though: MixEffect will always reset tha alpha state to premultiplied. We should probably fix that too at some later stage. --- diff --git a/blur_effect.h b/blur_effect.h index 4af11a2..062807d 100644 --- a/blur_effect.h +++ b/blur_effect.h @@ -24,7 +24,7 @@ public: virtual bool needs_texture_bounce() const { return true; } virtual bool needs_mipmaps() const { return true; } virtual bool needs_srgb_primaries() const { return false; } - virtual AlphaHandling alpha_handling() const { return INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA; } + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } virtual void inform_input_size(unsigned input_num, unsigned width, unsigned height); diff --git a/deconvolution_sharpen_effect.h b/deconvolution_sharpen_effect.h index 293916f..a0b405a 100644 --- a/deconvolution_sharpen_effect.h +++ b/deconvolution_sharpen_effect.h @@ -39,6 +39,7 @@ public: } void set_gl_state(GLuint glsl_program_num, const std::string &prefix, unsigned *sampler_num); + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } private: // Input size. diff --git a/diffusion_effect.h b/diffusion_effect.h index 624990c..2c635d0 100644 --- a/diffusion_effect.h +++ b/diffusion_effect.h @@ -44,6 +44,7 @@ public: OverlayMatteEffect(); virtual std::string effect_type_id() const { return "OverlayMatteEffect"; } std::string output_fragment_shader(); + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } unsigned num_inputs() const { return 2; } diff --git a/effect.h b/effect.h index 43849df..771648c 100644 --- a/effect.h +++ b/effect.h @@ -99,7 +99,8 @@ public: // This is the most natural format for processing, and the default in // most of Movit (just like linear light is). // - // If you set INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA, all of your inputs + // If you set INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA or + // INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK, all of your inputs // (if any) are guaranteed to also be in premultiplied alpha. // Otherwise, you can get postmultiplied or premultiplied alpha; // you won't know. If you have multiple inputs, you will get the same @@ -121,9 +122,19 @@ public: // If you set this, you should also set needs_linear_light(). INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA, - // Keeps the type of alpha unchanged from input to output. - // Usually appropriate if you process all color channels - // in a linear fashion, and do not change alpha. + // Like INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA, but also guarantees + // that if you get blank alpha in, you also keep blank alpha out. + // This is a somewhat weaker guarantee than DONT_CARE_ALPHA_TYPE, + // but is still useful in many situations, and appropriate when + // e.g. you don't touch alpha at all. + // + // Does not make sense for inputs. + INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK, + + // Keeps the type of alpha (premultiplied, postmultiplied, blank) + // unchanged from input to output. Usually appropriate if you + // process all color channels in a linear fashion, do not change + // alpha, and do not produce any new pixels thare have alpha != 1.0. // // Does not make sense for inputs. DONT_CARE_ALPHA_TYPE, diff --git a/effect_chain.cpp b/effect_chain.cpp index bd039b0..0f8876d 100644 --- a/effect_chain.cpp +++ b/effect_chain.cpp @@ -782,6 +782,7 @@ void EffectChain::find_color_spaces_for_inputs() case Effect::OUTPUT_POSTMULTIPLIED_ALPHA: node->output_alpha_type = ALPHA_POSTMULTIPLIED; break; + case Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK: case Effect::DONT_CARE_ALPHA_TYPE: default: assert(false); @@ -894,6 +895,7 @@ void EffectChain::propagate_alpha() // whether to divide away the old alpha or not. Effect::AlphaHandling alpha_handling = node->effect->alpha_handling(); assert(alpha_handling == Effect::INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA || + alpha_handling == Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK || alpha_handling == Effect::DONT_CARE_ALPHA_TYPE); // If the node has multiple inputs, check that they are all valid and @@ -933,16 +935,16 @@ void EffectChain::propagate_alpha() continue; } - if (alpha_handling == Effect::INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA) { + if (alpha_handling == Effect::INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA || + alpha_handling == Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK) { // If the effect has asked for premultiplied alpha, check that it has got it. if (any_postmultiplied) { node->output_alpha_type = ALPHA_INVALID; + } else if (!any_premultiplied && + alpha_handling == Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK) { + // Blank input alpha, and the effect preserves blank alpha. + node->output_alpha_type = ALPHA_BLANK; } else { - // In some rare cases, it might be advantageous to say - // that blank input alpha yields blank output alpha. - // However, this would cause a more complex Effect interface - // an effect would need to guarantee that it doesn't mess with - // blank alpha), so this is the simplest. node->output_alpha_type = ALPHA_PREMULTIPLIED; } } else { diff --git a/effect_chain_test.cpp b/effect_chain_test.cpp index e7f25e5..cea22e0 100644 --- a/effect_chain_test.cpp +++ b/effect_chain_test.cpp @@ -448,6 +448,64 @@ TEST(EffectChainTest, NoAlphaConversionsWithBlankAlpha) { expect_equal(data, out_data, 4, size); } +// An effect that does nothing, and specifies that it preserves blank alpha. +class BlankAlphaPreservingEffect : public Effect { +public: + BlankAlphaPreservingEffect() {} + virtual std::string effect_type_id() const { return "BlankAlphaPreservingEffect"; } + std::string output_fragment_shader() { return read_file("identity.frag"); } + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } +}; + +TEST(EffectChainTest, NoAlphaConversionsWithBlankAlphaPreservingEffect) { + const int size = 3; + float data[4 * size] = { + 0.0f, 0.0f, 1.0f, 1.0f, + 0.0f, 0.0f, 1.0f, 1.0f, + 0.0f, 0.0f, 1.0f, 1.0f, + }; + float out_data[4 * size]; + EffectChainTester tester(NULL, size, 1); + tester.get_chain()->add_input(new BlueInput()); + tester.get_chain()->add_effect(new BlankAlphaPreservingEffect()); + RewritingEffect *effect = new RewritingEffect(); + tester.get_chain()->add_effect(effect); + tester.run(out_data, GL_RGBA, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_POSTMULTIPLIED); + + Node *node = effect->replaced_node; + EXPECT_EQ(1, node->incoming_links.size()); + EXPECT_EQ(0, node->outgoing_links.size()); + + expect_equal(data, out_data, 4, size); +} + +// This is the counter-test to NoAlphaConversionsWithBlankAlphaPreservingEffect; +// just to be sure that with a normal INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA effect, +// an alpha conversion _should_ be inserted at the very end. (There is some overlap +// with other tests.) +TEST(EffectChainTest, AlphaConversionsWithNonBlankAlphaPreservingEffect) { + const int size = 3; + float data[4 * size] = { + 0.0f, 0.0f, 1.0f, 1.0f, + 0.0f, 0.0f, 1.0f, 1.0f, + 0.0f, 0.0f, 1.0f, 1.0f, + }; + float out_data[4 * size]; + EffectChainTester tester(NULL, size, 1); + tester.get_chain()->add_input(new BlueInput()); + tester.get_chain()->add_effect(new IdentityEffect()); // Not BlankAlphaPreservingEffect. + RewritingEffect *effect = new RewritingEffect(); + tester.get_chain()->add_effect(effect); + tester.run(out_data, GL_RGBA, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_POSTMULTIPLIED); + + Node *node = effect->replaced_node; + EXPECT_EQ(1, node->incoming_links.size()); + EXPECT_EQ(1, node->outgoing_links.size()); + EXPECT_EQ("AlphaDivisionEffect", node->outgoing_links[0]->effect->effect_type_id()); + + expect_equal(data, out_data, 4, size); +} + // Effectively scales down its input linearly by 4x (and repeating it), // which is not attainable without mipmaps. class MipmapNeedingEffect : public Effect { diff --git a/glow_effect.h b/glow_effect.h index 8b21e52..abadb84 100644 --- a/glow_effect.h +++ b/glow_effect.h @@ -42,6 +42,8 @@ public: HighlightCutoffEffect(); virtual std::string effect_type_id() const { return "HighlightCutoffEffect"; } std::string output_fragment_shader(); + + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } private: float cutoff; diff --git a/lift_gamma_gain_effect.h b/lift_gamma_gain_effect.h index 36f6a77..6835620 100644 --- a/lift_gamma_gain_effect.h +++ b/lift_gamma_gain_effect.h @@ -26,6 +26,7 @@ class LiftGammaGainEffect : public Effect { public: LiftGammaGainEffect(); virtual std::string effect_type_id() const { return "LiftGammaGainEffect"; } + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } std::string output_fragment_shader(); void set_gl_state(GLuint glsl_program_num, const std::string &prefix, unsigned *sampler_num); diff --git a/mix_effect.h b/mix_effect.h index c673965..967c06b 100644 --- a/mix_effect.h +++ b/mix_effect.h @@ -15,6 +15,10 @@ public: virtual bool needs_srgb_primaries() const { return false; } virtual unsigned num_inputs() const { return 2; } + // TODO: In the common case where a+b=1, it would be useful to be able to set + // alpha_handling() to INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK. However, right now + // we have no way of knowing that at instantiation time. + private: float strength_first, strength_second; }; diff --git a/overlay_effect.h b/overlay_effect.h index d26d917..4c6d37a 100644 --- a/overlay_effect.h +++ b/overlay_effect.h @@ -21,10 +21,12 @@ public: virtual bool needs_srgb_primaries() const { return false; } virtual unsigned num_inputs() const { return 2; } - // Actually, if either image has blank alpha, our output will have - // blank alpha, too. However, understanding that would require changes + // Actually, if _either_ image has blank alpha, our output will have + // blank alpha, too (this only tells the framework that having _both_ + // images with blank alpha would result in blank alpha). + // However, understanding that would require changes // to EffectChain, so postpone that optimization for later. - virtual AlphaHandling alpha_handling() const { return INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA; } + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } }; #endif // !defined(_OVERLAY_EFFECT_H) diff --git a/padding_effect.cpp b/padding_effect.cpp index 3c4919a..f8d30d4 100644 --- a/padding_effect.cpp +++ b/padding_effect.cpp @@ -89,13 +89,25 @@ bool PaddingEffect::needs_srgb_primaries() const return true; } -// If the border color is black, it doesn't matter if we're pre- or postmultiplied -// (or even blank, as a hack). Otherwise, it does. Effect::AlphaHandling PaddingEffect::alpha_handling() const { - if (border_color.r == 0.0 && border_color.g == 0.0 && border_color.b == 0.0) { + // If the border color is black, it doesn't matter if we're pre- or postmultiplied. + // Note that for non-solid black (i.e. alpha < 1.0), we're equally fine with + // pre- and postmultiplied, but later effects might change this status + // (consider e.g. blur), so setting DONT_CARE_ALPHA_TYPE is inappropriate, + // as it propagate blank alpha through this effect. + if (border_color.r == 0.0 && border_color.g == 0.0 && border_color.b == 0.0 && border_color.a == 1.0) { return DONT_CARE_ALPHA_TYPE; } + + // If the border color is solid, we preserve blank alpha, as we never output any + // new non-solid pixels. + if (border_color.a == 1.0) { + return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; + } + + // Otherwise, we're going to output our border color in premultiplied alpha, + // so the other pixels better be premultiplied as well. return INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA; } diff --git a/resample_effect.h b/resample_effect.h index a0ae1a8..b38728f 100644 --- a/resample_effect.h +++ b/resample_effect.h @@ -29,6 +29,7 @@ public: // down quite a lot. virtual bool needs_texture_bounce() const { return true; } virtual bool needs_srgb_primaries() const { return false; } + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } virtual void inform_input_size(unsigned input_num, unsigned width, unsigned height); diff --git a/resize_effect.h b/resize_effect.h index c11f4bc..3cb40e3 100644 --- a/resize_effect.h +++ b/resize_effect.h @@ -17,6 +17,7 @@ public: // in case we need to scale down a lot. virtual bool need_texture_bounce() const { return true; } virtual bool needs_mipmaps() const { return true; } + virtual AlphaHandling alpha_handling() const { return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK; } virtual bool changes_output_size() const { return true; } virtual void get_output_size(unsigned *width, unsigned *height, unsigned *virtual_width, unsigned *virtual_height) const;