From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Wed, 9 Oct 2013 20:24:56 +0000 (+0200)
Subject: Fix a bug where PaddingEffect could create assertion errors.
X-Git-Tag: 1.0~114
X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=5c97329dd35909847e2120b0b368b2723ffe5a44;hp=58e7b9a8164bdaad7b0c698b2d0d80db53d79a5c

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 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);
+}