Rework PaddingEffect alpha handling, which also fixes a long-standing assertion failure.

diff --git a/padding_effect.cpp b/padding_effect.cpp
index d96d1046f37b9615c873ff23618eabfe4d6ee531..48ad54ff3e83264cb6d2016a74e3753336e3074f 100644 (file)
--- a/padding_effect.cpp
+++ b/padding_effect.cpp
@@ -101,23 +101,16 @@ bool PaddingEffect::needs_srgb_primaries() const
 
 Effect::AlphaHandling PaddingEffect::alpha_handling() const
 {
 
 Effect::AlphaHandling PaddingEffect::alpha_handling() const
 {

-       // 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 the border color is solid, it doesn't matter if we're pre- or postmultiplied.

        if (border_color.a == 1.0) {
        if (border_color.a == 1.0) {

-               return INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK;
+               return DONT_CARE_ALPHA_TYPE;

        }
 
        // Otherwise, we're going to output our border color in premultiplied alpha,
        // so the other pixels better be premultiplied as well.
        }
 
        // Otherwise, we're going to output our border color in premultiplied alpha,
        // so the other pixels better be premultiplied as well.

+       // Note that for non-solid black (i.e. alpha < 1.0), we're equally fine with
+       // pre- and postmultiplied, but we are _not_ fine with blank being passed through,
+       // and we don't have a way to specify that.

        return INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA;
 }
        
        return INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA;
 }
        

diff --git a/padding_effect_test.cpp b/padding_effect_test.cpp
index a1a36b9c29e3f6ff22c9dcc3fbd0e40431cf9ef9..86ce9e9d8feb2b82f8f7c1bf929ac0b8f31e6c87 100644 (file)
--- a/padding_effect_test.cpp
+++ b/padding_effect_test.cpp
@@ -242,6 +242,42 @@ TEST(PaddingEffectTest, AlphaIsCorrectEvenWithNonLinearInputsAndOutputs) {
        expect_equal(expected_data, out_data, 4, 4);
 }
 
        expect_equal(expected_data, out_data, 4, 4);
 }
 

+TEST(PaddingEffectTest, RedBorder) {  // Not black nor white, but still a saturated primary.
+       float data[2 * 1] = {
+               1.0f,
+               0.8f,
+       };
+       float expected_data[4 * 4] = {
+               1.0f, 0.0f, 0.0f, 1.0f,
+               1.0f, 1.0f, 1.0f, 1.0f,
+               0.8f, 0.8f, 0.8f, 1.0f,
+               1.0f, 0.0f, 0.0f, 1.0f,
+       };
+       float out_data[4 * 4];
+
+       EffectChainTester tester(NULL, 1, 4);
+
+       ImageFormat format;
+       format.color_space = COLORSPACE_REC_601_625;
+       format.gamma_curve = GAMMA_REC_709;
+
+       FlatInput *input = new FlatInput(format, FORMAT_GRAYSCALE, GL_FLOAT, 1, 2);
+       input->set_pixel_data(data);
+       tester.get_chain()->add_input(input);
+
+       Effect *effect = tester.get_chain()->add_effect(new PaddingEffect());
+       CHECK(effect->set_int("width", 1));
+       CHECK(effect->set_int("height", 4));
+       CHECK(effect->set_float("left", 0.0f));
+       CHECK(effect->set_float("top", 1.0f));
+
+       RGBATuple border_color(1.0f, 0.0f, 0.0f, 1.0f);
+       CHECK(effect->set_vec4("border_color", (float *)&border_color));
+
+       tester.run(out_data, GL_RGBA, COLORSPACE_REC_709, GAMMA_REC_709, OUTPUT_ALPHA_FORMAT_POSTMULTIPLIED);
+       expect_equal(expected_data, out_data, 4, 4);
+}
+

 TEST(PaddingEffectTest, BorderOffsetTopAndBottom) {
        float data[2 * 2] = {
                1.0f, 0.5f,
 TEST(PaddingEffectTest, BorderOffsetTopAndBottom) {
        float data[2 * 2] = {
                1.0f, 0.5f,