Fix a bug where PaddingEffect could create assertion errors.

The ratinale is explained in the comment, but in short, PaddingEffect
could convert blank to premultiplied alpha without realizing that
it then needed linear light (since premultiplied alpha in our case
is defined as being in linear light). Also added a unit test.

Reported by Christophe Thommeret.

diff --git a/padding_effect.cpp b/padding_effect.cpp
index 40876b89e9a6edca0cf2c89146894c1d72f15070..00d475ac2fb23995780920569ee09203abb91177 100644 (file)
--- a/padding_effect.cpp
+++ b/padding_effect.cpp
@@ -64,12 +64,16 @@ void PaddingEffect::set_gl_state(GLuint glsl_program_num, const std::string &pre
 // differently in different modes.
 
 // 0.0 and 1.0 are interpreted the same, no matter the gamma ramp.
 // differently in different modes.
 
 // 0.0 and 1.0 are interpreted the same, no matter the gamma ramp.

-// Alpha is not affected by gamma.
+// Alpha is not affected by gamma per se, but the combination of
+// premultiplied alpha and non-linear gamma curve does not make sense,
+// so if could possibly be converting blank alpha to non-blank
+// (ie., premultiplied), we need our output to be in linear light.

 bool PaddingEffect::needs_linear_light() const
 {
        if ((border_color.r == 0.0 || border_color.r == 1.0) &&
            (border_color.g == 0.0 || border_color.g == 1.0) &&
 bool PaddingEffect::needs_linear_light() const
 {
        if ((border_color.r == 0.0 || border_color.r == 1.0) &&
            (border_color.g == 0.0 || border_color.g == 1.0) &&

-           (border_color.b == 0.0 || border_color.b == 1.0)) {
+           (border_color.b == 0.0 || border_color.b == 1.0) &&
+           border_color.a == 1.0) {

                return false;
        }
        return true;
                return false;
        }
        return true;

diff --git a/padding_effect_test.cpp b/padding_effect_test.cpp
index f7705883bc5167bb8c74ee5ce9ad2381e274fecf..c9c8ef1d7af1a0a9074697fb2d73f6d47ffe73e1 100644 (file)
--- a/padding_effect_test.cpp
+++ b/padding_effect_test.cpp
@@ -204,3 +204,39 @@ TEST(PaddingEffectTest, Crop) {
        tester.run(out_data, GL_RED, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_PREMULTIPLIED);
        expect_equal(expected_data, out_data, 1, 1);
 }
        tester.run(out_data, GL_RED, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_PREMULTIPLIED);
        expect_equal(expected_data, out_data, 1, 1);
 }

+
+TEST(PaddingEffectTest, AlphaIsCorrectEvenWithNonLinearInputsAndOutputs) {
+       float data[2 * 1] = {
+               1.0f,
+               0.8f,
+       };
+       float expected_data[4 * 4] = {
+               1.0f, 1.0f, 1.0f, 0.5f,
+               1.0f, 1.0f, 1.0f, 1.0f,
+               0.8f, 0.8f, 0.8f, 1.0f,
+               1.0f, 1.0f, 1.0f, 0.5f,
+       };
+       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, 1.0f, 1.0f, 0.5f);
+       CHECK(effect->set_vec4("border_color", (float *)&border_color));
+
+       tester.run(out_data, GL_RGBA, COLORSPACE_REC_601_625, GAMMA_REC_709, OUTPUT_ALPHA_FORMAT_POSTMULTIPLIED);
+       expect_equal(expected_data, out_data, 4, 4);
+}