From 5c97329dd35909847e2120b0b368b2723ffe5a44 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Wed, 9 Oct 2013 22:24:56 +0200 Subject: [PATCH] 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. --- padding_effect.cpp | 8 ++++++-- padding_effect_test.cpp | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/padding_effect.cpp b/padding_effect.cpp index 40876b8..00d475a 100644 --- 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. -// 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) && - (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; diff --git a/padding_effect_test.cpp b/padding_effect_test.cpp index f770588..c9c8ef1 100644 --- 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); } + +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); +} -- 2.39.2