From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Sat, 15 Mar 2014 02:04:31 +0000 (+0100)
Subject: Fix a bug where repeated vertical FFTs would reverse the output.
X-Git-Tag: 1.0~10
X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=6462d4df1986c76e363cf21ee0c7734d1b312635

Fix a bug where repeated vertical FFTs would reverse the output.

Unfortunately, the tests didn't catch this, as the Repeat test used
an even number of passes (being of size 64), which reversed things
back into place. It now tries a wider range of sizes to make sure
everything is okay.
---

diff --git a/fft_pass_effect.cpp b/fft_pass_effect.cpp
index 9c8aeb0..4069be9 100644
--- a/fft_pass_effect.cpp
+++ b/fft_pass_effect.cpp
@@ -117,15 +117,17 @@ void FFTPassEffect::set_gl_state(GLuint glsl_program_num, const string &prefix,
 		// is that we can have multiple FFTs along the same line,
 		// and want to reuse the support texture by repeating it.
 		int base = k * stride * 2 + offset;
-		int support_texture_index;
+		int support_texture_index = i;
+		int src1 = base;
+		int src2 = base + stride;
 		if (direction == FFTPassEffect::VERTICAL) {
 			// Compensate for OpenGL's bottom-left convention.
-			support_texture_index = fft_size - i - 1;
-		} else {
-			support_texture_index = i;
+			support_texture_index = fft_size - support_texture_index - 1;
+			src1 = fft_size - src1 - 1;
+			src2 = fft_size - src2 - 1;
 		}
-		tmp[support_texture_index * 4 + 0] = fp64_to_fp16((base - support_texture_index) / double(input_size));
-		tmp[support_texture_index * 4 + 1] = fp64_to_fp16((base + stride - support_texture_index) / double(input_size));
+		tmp[support_texture_index * 4 + 0] = fp64_to_fp16((src1 - support_texture_index) / double(input_size));
+		tmp[support_texture_index * 4 + 1] = fp64_to_fp16((src2 - support_texture_index) / double(input_size));
 		tmp[support_texture_index * 4 + 2] = fp64_to_fp16(twiddle_real);
 		tmp[support_texture_index * 4 + 3] = fp64_to_fp16(twiddle_imag);
 	}
diff --git a/fft_pass_effect.frag b/fft_pass_effect.frag
index 800653a..edcf695 100644
--- a/fft_pass_effect.frag
+++ b/fft_pass_effect.frag
@@ -7,8 +7,8 @@ uniform sampler2D PREFIX(support_tex);
 vec4 FUNCNAME(vec2 tc) {
 #if DIRECTION_VERTICAL
 	vec4 support = texture2D(PREFIX(support_tex), vec2(tc.y * PREFIX(num_repeats), 0.0));
-        vec4 c1 = INPUT(vec2(tc.x, 1.0 - (tc.y + support.x)));
-        vec4 c2 = INPUT(vec2(tc.x, 1.0 - (tc.y + support.y)));
+        vec4 c1 = INPUT(vec2(tc.x, tc.y + support.x));
+        vec4 c2 = INPUT(vec2(tc.x, tc.y + support.y));
 #else
 	vec4 support = texture2D(PREFIX(support_tex), vec2(tc.x * PREFIX(num_repeats), 0.0));
         vec4 c1 = INPUT(vec2(tc.x + support.x, tc.y));
diff --git a/fft_pass_effect_test.cpp b/fft_pass_effect_test.cpp
index eb4e37c..284aa5f 100644
--- a/fft_pass_effect_test.cpp
+++ b/fft_pass_effect_test.cpp
@@ -125,35 +125,36 @@ TEST(FFTPassEffectTest, SingleFrequency) {
 }
 
 TEST(FFTPassEffectTest, Repeat) {
-	const int fft_size = 64;
-	const int num_repeats = 31;  // Prime, to make things more challenging.
-	float data[num_repeats * fft_size * 4] = { 0 };
-	float expected_data[num_repeats * fft_size * 4], out_data[num_repeats * fft_size * 4];
-
 	srand(12345);
-	for (int i = 0; i < num_repeats * fft_size * 4; ++i) {
-		data[i] = uniform_random();
-	}
+	for (int fft_size = 2; fft_size < 512; fft_size *= 2) {
+		const int num_repeats = 31;  // Prime, to make things more challenging.
+		float data[num_repeats * fft_size * 4];
+		float expected_data[num_repeats * fft_size * 4], out_data[num_repeats * fft_size * 4];
 
-	for (int i = 0; i < num_repeats; ++i) {
-		run_fft(data + i * fft_size * 4, expected_data + i * fft_size * 4, fft_size, false);
-	}
+		for (int i = 0; i < num_repeats * fft_size * 4; ++i) {
+			data[i] = uniform_random();
+		}
 
-	{
-		// Horizontal.
-		EffectChainTester tester(data, num_repeats * fft_size, 1, FORMAT_RGBA_PREMULTIPLIED_ALPHA, COLORSPACE_sRGB, GAMMA_LINEAR);
-		setup_fft(tester.get_chain(), fft_size, false);
-		tester.run(out_data, GL_RGBA, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_PREMULTIPLIED);
+		for (int i = 0; i < num_repeats; ++i) {
+			run_fft(data + i * fft_size * 4, expected_data + i * fft_size * 4, fft_size, false);
+		}
 
-		expect_equal(expected_data, out_data, 4, num_repeats * fft_size);
-	}
-	{
-		// Vertical.
-		EffectChainTester tester(data, 1, num_repeats * fft_size, FORMAT_RGBA_PREMULTIPLIED_ALPHA, COLORSPACE_sRGB, GAMMA_LINEAR);
-		setup_fft(tester.get_chain(), fft_size, false, false, FFTPassEffect::VERTICAL);
-		tester.run(out_data, GL_RGBA, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_PREMULTIPLIED);
+		{
+			// Horizontal.
+			EffectChainTester tester(data, num_repeats * fft_size, 1, FORMAT_RGBA_PREMULTIPLIED_ALPHA, COLORSPACE_sRGB, GAMMA_LINEAR);
+			setup_fft(tester.get_chain(), fft_size, false);
+			tester.run(out_data, GL_RGBA, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_PREMULTIPLIED);
 
-		expect_equal(expected_data, out_data, 4, num_repeats * fft_size);
+			expect_equal(expected_data, out_data, 4, num_repeats * fft_size);
+		}
+		{
+			// Vertical.
+			EffectChainTester tester(data, 1, num_repeats * fft_size, FORMAT_RGBA_PREMULTIPLIED_ALPHA, COLORSPACE_sRGB, GAMMA_LINEAR);
+			setup_fft(tester.get_chain(), fft_size, false, false, FFTPassEffect::VERTICAL);
+			tester.run(out_data, GL_RGBA, COLORSPACE_sRGB, GAMMA_LINEAR, OUTPUT_ALPHA_FORMAT_PREMULTIPLIED);
+
+			expect_equal(expected_data, out_data, 4, num_repeats * fft_size);
+		}
 	}
 }