From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Mon, 29 Feb 2016 23:59:37 +0000 (+0100)
Subject: Rework PaddingEffect alpha handling, which also fixes a long-standing assertion failure.
X-Git-Tag: 1.4.0~8
X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=76c0697ba13e98ffdf4a866fab05a4ec74cd5139

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

diff --git a/padding_effect.cpp b/padding_effect.cpp
index d96d104..48ad54f 100644
--- a/padding_effect.cpp
+++ b/padding_effect.cpp
@@ -101,23 +101,16 @@ bool PaddingEffect::needs_srgb_primaries() 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) {
-		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.
+	// 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;
 }
 	
diff --git a/padding_effect_test.cpp b/padding_effect_test.cpp
index a1a36b9..86ce9e9 100644
--- 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);
 }
 
+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,