X-Git-Url: https://git.sesse.net/?p=movit;a=blobdiff_plain;f=effect_chain.cpp;h=6b7a857242e0572c7be6d66afe6026734ec99c6c;hp=8db5376bd333794a2dcaa3f4cdca55324b8b8d46;hb=1f1da18ac6acec9e16e66441160b3949173f1db3;hpb=b10c546f579c7ccb5939161e61a71cd18a3f9bbd

diff --git a/effect_chain.cpp b/effect_chain.cpp
index 8db5376..6b7a857 100644
--- a/effect_chain.cpp
+++ b/effect_chain.cpp
@@ -1,32 +1,34 @@
 #define GL_GLEXT_PROTOTYPES 1
 
-#include <stdio.h>
+#include <GL/glew.h>
+#include <assert.h>
+#include <locale.h>
 #include <math.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
-#include <assert.h>
-#include <GL/glew.h>
-
 #include <algorithm>
 #include <set>
 #include <stack>
 #include <vector>
 
-#include "util.h"
-#include "effect_chain.h"
-#include "gamma_expansion_effect.h"
-#include "gamma_compression_effect.h"
-#include "colorspace_conversion_effect.h"
-#include "alpha_multiplication_effect.h"
 #include "alpha_division_effect.h"
+#include "alpha_multiplication_effect.h"
+#include "colorspace_conversion_effect.h"
 #include "dither_effect.h"
-#include "input.h"
+#include "effect.h"
+#include "effect_chain.h"
+#include "gamma_compression_effect.h"
+#include "gamma_expansion_effect.h"
 #include "init.h"
+#include "input.h"
+#include "util.h"
 
 EffectChain::EffectChain(float aspect_nom, float aspect_denom)
 	: aspect_nom(aspect_nom),
 	  aspect_denom(aspect_denom),
 	  dither_effect(NULL),
-	  fbo(0),
 	  num_dither_bits(0),
 	  finalized(false) {}
 
@@ -45,13 +47,11 @@ EffectChain::~EffectChain()
 		glDeleteShader(phases[i]->fragment_shader);
 		delete phases[i];
 	}
-	if (fbo != 0) {
-		glDeleteFramebuffers(1, &fbo);
-	}
 }
 
 Input *EffectChain::add_input(Input *input)
 {
+	assert(!finalized);
 	inputs.push_back(input);
 	add_node(input);
 	return input;
@@ -59,12 +59,17 @@ Input *EffectChain::add_input(Input *input)
 
 void EffectChain::add_output(const ImageFormat &format, OutputAlphaFormat alpha_format)
 {
+	assert(!finalized);
 	output_format = format;
 	output_alpha_format = alpha_format;
 }
 
 Node *EffectChain::add_node(Effect *effect)
 {
+	for (unsigned i = 0; i < nodes.size(); ++i) {
+		assert(nodes[i]->effect != effect);
+	}
+
 	char effect_id[256];
 	sprintf(effect_id, "eff%u", (unsigned)nodes.size());
 
@@ -154,6 +159,7 @@ void EffectChain::find_all_nonlinear_inputs(Node *node, std::vector<Node *> *non
 
 Effect *EffectChain::add_effect(Effect *effect, const std::vector<Effect *> &inputs)
 {
+	assert(!finalized);
 	assert(inputs.size() == effect->num_inputs());
 	Node *node = add_node(effect);
 	for (unsigned i = 0; i < inputs.size(); ++i) {
@@ -229,8 +235,10 @@ Phase *EffectChain::compile_glsl_program(
 		frag_shader += "\n";
 	}
 
-	for (unsigned i = 0; i < effects.size(); ++i) {
-		Node *node = effects[i];
+	std::vector<Node *> sorted_effects = topological_sort(effects);
+
+	for (unsigned i = 0; i < sorted_effects.size(); ++i) {
+		Node *node = sorted_effects[i];
 
 		if (node->incoming_links.size() == 1) {
 			frag_shader += std::string("#define INPUT ") + node->incoming_links[0]->effect_id + "\n";
@@ -261,13 +269,13 @@ Phase *EffectChain::compile_glsl_program(
 
 		input_needs_mipmaps |= node->effect->needs_mipmaps();
 	}
-	for (unsigned i = 0; i < effects.size(); ++i) {
-		Node *node = effects[i];
+	for (unsigned i = 0; i < sorted_effects.size(); ++i) {
+		Node *node = sorted_effects[i];
 		if (node->effect->num_inputs() == 0) {
 			CHECK(node->effect->set_int("needs_mipmaps", input_needs_mipmaps));
 		}
 	}
-	frag_shader += std::string("#define INPUT ") + effects.back()->effect_id + "\n";
+	frag_shader += std::string("#define INPUT ") + sorted_effects.back()->effect_id + "\n";
 	frag_shader.append(read_file("footer.frag"));
 
 	if (movit_debug_level == MOVIT_DEBUG_ON) {
@@ -300,7 +308,7 @@ Phase *EffectChain::compile_glsl_program(
 	phase->fragment_shader = fs_obj;
 	phase->input_needs_mipmaps = input_needs_mipmaps;
 	phase->inputs = true_inputs;
-	phase->effects = effects;
+	phase->effects = sorted_effects;
 
 	return phase;
 }
@@ -315,9 +323,9 @@ Phase *EffectChain::compile_glsl_program(
 // without any explicit recursion.
 void EffectChain::construct_glsl_programs(Node *output)
 {
-	// Which effects have already been completed in this phase?
+	// Which effects have already been completed?
 	// We need to keep track of it, as an effect with multiple outputs
-	// could otherwise be calculate multiple times.
+	// could otherwise be calculated multiple times.
 	std::set<Node *> completed_effects;
 
 	// Effects in the current phase, as well as inputs (outputs from other phases
@@ -343,9 +351,16 @@ void EffectChain::construct_glsl_programs(Node *output)
 			Node *node = effects_todo_this_phase.top();
 			effects_todo_this_phase.pop();
 
-			// This should currently only happen for effects that are phase outputs,
-			// and we throw those out separately below.
-			assert(completed_effects.count(node) == 0);
+			// This should currently only happen for effects that are inputs
+			// (either true inputs or phase outputs). We special-case inputs,
+			// and then deduplicate phase outputs in compile_glsl_program().
+			if (node->effect->num_inputs() == 0) {
+				if (find(this_phase_effects.begin(), this_phase_effects.end(), node) != this_phase_effects.end()) {
+					continue;
+				}
+			} else {
+				assert(completed_effects.count(node) == 0);
+			}
 
 			this_phase_effects.push_back(node);
 			completed_effects.insert(node);
@@ -442,85 +457,46 @@ void EffectChain::output_dot(const char *filename)
 	}
 
 	fprintf(fp, "digraph G {\n");
+	fprintf(fp, "  output [shape=box label=\"(output)\"];\n");
 	for (unsigned i = 0; i < nodes.size(); ++i) {
 		// Find out which phase this event belongs to.
-		int in_phase = -1;
+		std::vector<int> in_phases;
 		for (unsigned j = 0; j < phases.size(); ++j) {
 			const Phase* p = phases[j];
 			if (std::find(p->effects.begin(), p->effects.end(), nodes[i]) != p->effects.end()) {
-				assert(in_phase == -1);
-				in_phase = j;
+				in_phases.push_back(j);
 			}
 		}
 
-		if (in_phase == -1) {
+		if (in_phases.empty()) {
 			fprintf(fp, "  n%ld [label=\"%s\"];\n", (long)nodes[i], nodes[i]->effect->effect_type_id().c_str());
-		} else {
+		} else if (in_phases.size() == 1) {
 			fprintf(fp, "  n%ld [label=\"%s\" style=\"filled\" fillcolor=\"/accent8/%d\"];\n",
 				(long)nodes[i], nodes[i]->effect->effect_type_id().c_str(),
-				(in_phase % 8) + 1);
+				(in_phases[0] % 8) + 1);
+		} else {
+			// If we had new enough Graphviz, style="wedged" would probably be ideal here.
+			// But alas.
+			fprintf(fp, "  n%ld [label=\"%s [in multiple phases]\" style=\"filled\" fillcolor=\"/accent8/%d\"];\n",
+				(long)nodes[i], nodes[i]->effect->effect_type_id().c_str(),
+				(in_phases[0] % 8) + 1);
 		}
-		for (unsigned j = 0; j < nodes[i]->outgoing_links.size(); ++j) {
-			std::vector<std::string> labels;
 
-			if (nodes[i]->outgoing_links[j]->effect->needs_texture_bounce()) {
-				labels.push_back("needs_bounce");
-			}
-			if (nodes[i]->effect->changes_output_size()) {
-				labels.push_back("resize");
-			}
-
-			switch (nodes[i]->output_color_space) {
-			case COLORSPACE_INVALID:
-				labels.push_back("spc[invalid]");
-				break;
-			case COLORSPACE_REC_601_525:
-				labels.push_back("spc[rec601-525]");
-				break;
-			case COLORSPACE_REC_601_625:
-				labels.push_back("spc[rec601-625]");
-				break;
-			default:
-				break;
-			}
+		char from_node_id[256];
+		snprintf(from_node_id, 256, "n%ld", (long)nodes[i]);
 
-			switch (nodes[i]->output_gamma_curve) {
-			case GAMMA_INVALID:
-				labels.push_back("gamma[invalid]");
-				break;
-			case GAMMA_sRGB:
-				labels.push_back("gamma[sRGB]");
-				break;
-			case GAMMA_REC_601:  // and GAMMA_REC_709
-				labels.push_back("gamma[rec601/709]");
-				break;
-			default:
-				break;
-			}
+		for (unsigned j = 0; j < nodes[i]->outgoing_links.size(); ++j) {
+			char to_node_id[256];
+			snprintf(to_node_id, 256, "n%ld", (long)nodes[i]->outgoing_links[j]);
 
-			switch (nodes[i]->output_alpha_type) {
-			case ALPHA_INVALID:
-				labels.push_back("alpha[invalid]");
-				break;
-			case ALPHA_BLANK:
-				labels.push_back("alpha[blank]");
-				break;
-			case ALPHA_POSTMULTIPLIED:
-				labels.push_back("alpha[postmult]");
-				break;
-			default:
-				break;
-			}
+			std::vector<std::string> labels = get_labels_for_edge(nodes[i], nodes[i]->outgoing_links[j]);
+			output_dot_edge(fp, from_node_id, to_node_id, labels);
+		}
 
-			if (labels.empty()) {
-				fprintf(fp, "  n%ld -> n%ld;\n", (long)nodes[i], (long)nodes[i]->outgoing_links[j]);
-			} else {
-				std::string label = labels[0];
-				for (unsigned k = 1; k < labels.size(); ++k) {
-					label += ", " + labels[k];
-				}
-				fprintf(fp, "  n%ld -> n%ld [label=\"%s\"];\n", (long)nodes[i], (long)nodes[i]->outgoing_links[j], label.c_str());
-			}
+		if (nodes[i]->outgoing_links.empty() && !nodes[i]->disabled) {
+			// Output node.
+			std::vector<std::string> labels = get_labels_for_edge(nodes[i], NULL);
+			output_dot_edge(fp, from_node_id, "output", labels);
 		}
 	}
 	fprintf(fp, "}\n");
@@ -528,16 +504,102 @@ void EffectChain::output_dot(const char *filename)
 	fclose(fp);
 }
 
-unsigned EffectChain::fit_rectangle_to_aspect(unsigned width, unsigned height)
+std::vector<std::string> EffectChain::get_labels_for_edge(const Node *from, const Node *to)
+{
+	std::vector<std::string> labels;
+
+	if (to != NULL && to->effect->needs_texture_bounce()) {
+		labels.push_back("needs_bounce");
+	}
+	if (from->effect->changes_output_size()) {
+		labels.push_back("resize");
+	}
+
+	switch (from->output_color_space) {
+	case COLORSPACE_INVALID:
+		labels.push_back("spc[invalid]");
+		break;
+	case COLORSPACE_REC_601_525:
+		labels.push_back("spc[rec601-525]");
+		break;
+	case COLORSPACE_REC_601_625:
+		labels.push_back("spc[rec601-625]");
+		break;
+	default:
+		break;
+	}
+
+	switch (from->output_gamma_curve) {
+	case GAMMA_INVALID:
+		labels.push_back("gamma[invalid]");
+		break;
+	case GAMMA_sRGB:
+		labels.push_back("gamma[sRGB]");
+		break;
+	case GAMMA_REC_601:  // and GAMMA_REC_709
+		labels.push_back("gamma[rec601/709]");
+		break;
+	default:
+		break;
+	}
+
+	switch (from->output_alpha_type) {
+	case ALPHA_INVALID:
+		labels.push_back("alpha[invalid]");
+		break;
+	case ALPHA_BLANK:
+		labels.push_back("alpha[blank]");
+		break;
+	case ALPHA_POSTMULTIPLIED:
+		labels.push_back("alpha[postmult]");
+		break;
+	default:
+		break;
+	}
+
+	return labels;
+}
+
+void EffectChain::output_dot_edge(FILE *fp,
+                                  const std::string &from_node_id,
+                                  const std::string &to_node_id,
+                                  const std::vector<std::string> &labels)
 {
+	if (labels.empty()) {
+		fprintf(fp, "  %s -> %s;\n", from_node_id.c_str(), to_node_id.c_str());
+	} else {
+		std::string label = labels[0];
+		for (unsigned k = 1; k < labels.size(); ++k) {
+			label += ", " + labels[k];
+		}
+		fprintf(fp, "  %s -> %s [label=\"%s\"];\n", from_node_id.c_str(), to_node_id.c_str(), label.c_str());
+	}
+}
+
+void EffectChain::size_rectangle_to_fit(unsigned width, unsigned height, unsigned *output_width, unsigned *output_height)
+{
+	unsigned scaled_width, scaled_height;
+
 	if (float(width) * aspect_denom >= float(height) * aspect_nom) {
 		// Same aspect, or W/H > aspect (image is wider than the frame).
-		// In either case, keep width.
-		return width;
+		// In either case, keep width, and adjust height.
+		scaled_width = width;
+		scaled_height = lrintf(width * aspect_denom / aspect_nom);
 	} else {
 		// W/H < aspect (image is taller than the frame), so keep height,
-		// and adjust width correspondingly.
-		return lrintf(height * aspect_nom / aspect_denom);
+		// and adjust width.
+		scaled_width = lrintf(height * aspect_nom / aspect_denom);
+		scaled_height = height;
+	}
+
+	// We should be consistently larger or smaller then the existing choice,
+	// since we have the same aspect.
+	assert(!(scaled_width < *output_width && scaled_height > *output_height));
+	assert(!(scaled_height < *output_height && scaled_width > *output_width));
+
+	if (scaled_width >= *output_width && scaled_height >= *output_height) {
+		*output_width = scaled_width;
+		*output_height = scaled_height;
 	}
 }
 
@@ -561,8 +623,8 @@ void EffectChain::inform_input_sizes(Phase *phase)
 	}
 	for (unsigned i = 0; i < phase->inputs.size(); ++i) {
 		Node *input = phase->inputs[i];
-		input->output_width = input->phase->output_width;
-		input->output_height = input->phase->output_height;
+		input->output_width = input->phase->virtual_output_width;
+		input->output_height = input->phase->virtual_output_height;
 		assert(input->output_width != 0);
 		assert(input->output_height != 0);
 	}
@@ -605,20 +667,25 @@ void EffectChain::find_output_size(Phase *phase)
 
 	// If the last effect explicitly sets an output size, use that.
 	if (output_node->effect->changes_output_size()) {
-		output_node->effect->get_output_size(&phase->output_width, &phase->output_height);
+		output_node->effect->get_output_size(&phase->output_width, &phase->output_height,
+		                                     &phase->virtual_output_width, &phase->virtual_output_height);
 		return;
 	}
 
-	// If not, look at the input phases and textures.
-	// We select the largest one (by fit into the current aspect).
-	unsigned best_width = 0;
+	// If all effects have the same size, use that.
+	unsigned output_width = 0, output_height = 0;
+	bool all_inputs_same_size = true;
+
 	for (unsigned i = 0; i < phase->inputs.size(); ++i) {
 		Node *input = phase->inputs[i];
 		assert(input->phase->output_width != 0);
 		assert(input->phase->output_height != 0);
-		unsigned width = fit_rectangle_to_aspect(input->phase->output_width, input->phase->output_height);
-		if (width > best_width) {
-			best_width = width;
+		if (output_width == 0 && output_height == 0) {
+			output_width = input->phase->virtual_output_width;
+			output_height = input->phase->virtual_output_height;
+		} else if (output_width != input->phase->virtual_output_width ||
+		           output_height != input->phase->virtual_output_height) {
+			all_inputs_same_size = false;
 		}
 	}
 	for (unsigned i = 0; i < phase->effects.size(); ++i) {
@@ -628,37 +695,71 @@ void EffectChain::find_output_size(Phase *phase)
 		}
 
 		Input *input = static_cast<Input *>(effect);
-		unsigned width = fit_rectangle_to_aspect(input->get_width(), input->get_height());
-		if (width > best_width) {
-			best_width = width;
+		if (output_width == 0 && output_height == 0) {
+			output_width = input->get_width();
+			output_height = input->get_height();
+		} else if (output_width != input->get_width() ||
+		           output_height != input->get_height()) {
+			all_inputs_same_size = false;
+		}
+	}
+
+	if (all_inputs_same_size) {
+		assert(output_width != 0);
+		assert(output_height != 0);
+		phase->virtual_output_width = phase->output_width = output_width;
+		phase->virtual_output_height = phase->output_height = output_height;
+		return;
+	}
+
+	// If not, fit all the inputs into the current aspect, and select the largest one. 
+	output_width = 0;
+	output_height = 0;
+	for (unsigned i = 0; i < phase->inputs.size(); ++i) {
+		Node *input = phase->inputs[i];
+		assert(input->phase->output_width != 0);
+		assert(input->phase->output_height != 0);
+		size_rectangle_to_fit(input->phase->output_width, input->phase->output_height, &output_width, &output_height);
+	}
+	for (unsigned i = 0; i < phase->effects.size(); ++i) {
+		Effect *effect = phase->effects[i]->effect;
+		if (effect->num_inputs() != 0) {
+			continue;
 		}
+
+		Input *input = static_cast<Input *>(effect);
+		size_rectangle_to_fit(input->get_width(), input->get_height(), &output_width, &output_height);
 	}
-	assert(best_width != 0);
-	phase->output_width = best_width;
-	phase->output_height = best_width * aspect_denom / aspect_nom;
+	assert(output_width != 0);
+	assert(output_height != 0);
+	phase->virtual_output_width = phase->output_width = output_width;
+	phase->virtual_output_height = phase->output_height = output_height;
 }
 
-void EffectChain::sort_nodes_topologically()
+void EffectChain::sort_all_nodes_topologically()
 {
-	std::set<Node *> visited_nodes;
+	nodes = topological_sort(nodes);
+}
+
+std::vector<Node *> EffectChain::topological_sort(const std::vector<Node *> &nodes)
+{
+	std::set<Node *> nodes_left_to_visit(nodes.begin(), nodes.end());
 	std::vector<Node *> sorted_list;
 	for (unsigned i = 0; i < nodes.size(); ++i) {
-		if (nodes[i]->incoming_links.size() == 0) {
-			topological_sort_visit_node(nodes[i], &visited_nodes, &sorted_list);
-		}
+		topological_sort_visit_node(nodes[i], &nodes_left_to_visit, &sorted_list);
 	}
 	reverse(sorted_list.begin(), sorted_list.end());
-	nodes = sorted_list;
+	return sorted_list;
 }
 
-void EffectChain::topological_sort_visit_node(Node *node, std::set<Node *> *visited_nodes, std::vector<Node *> *sorted_list)
+void EffectChain::topological_sort_visit_node(Node *node, std::set<Node *> *nodes_left_to_visit, std::vector<Node *> *sorted_list)
 {
-	if (visited_nodes->count(node) != 0) {
+	if (nodes_left_to_visit->count(node) == 0) {
 		return;
 	}
-	visited_nodes->insert(node);
+	nodes_left_to_visit->erase(node);
 	for (unsigned i = 0; i < node->outgoing_links.size(); ++i) {
-		topological_sort_visit_node(node->outgoing_links[i], visited_nodes, sorted_list);
+		topological_sort_visit_node(node->outgoing_links[i], nodes_left_to_visit, sorted_list);
 	}
 	sorted_list->push_back(node);
 }
@@ -680,16 +781,21 @@ void EffectChain::find_color_spaces_for_inputs()
 			case Effect::OUTPUT_BLANK_ALPHA:
 				node->output_alpha_type = ALPHA_BLANK;
 				break;
-			case Effect::INPUT_AND_OUTPUT_ALPHA_PREMULTIPLIED:
+			case Effect::INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA:
 				node->output_alpha_type = ALPHA_PREMULTIPLIED;
 				break;
-			case Effect::OUTPUT_ALPHA_POSTMULTIPLIED:
+			case Effect::OUTPUT_POSTMULTIPLIED_ALPHA:
 				node->output_alpha_type = ALPHA_POSTMULTIPLIED;
 				break;
+			case Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK:
 			case Effect::DONT_CARE_ALPHA_TYPE:
 			default:
 				assert(false);
 			}
+
+			if (node->output_alpha_type == ALPHA_PREMULTIPLIED) {
+				assert(node->output_gamma_curve == GAMMA_LINEAR);
+			}
 		}
 	}
 }
@@ -700,7 +806,7 @@ void EffectChain::find_color_spaces_for_inputs()
 void EffectChain::propagate_gamma_and_color_space()
 {
 	// We depend on going through the nodes in order.
-	sort_nodes_topologically();
+	sort_all_nodes_topologically();
 
 	for (unsigned i = 0; i < nodes.size(); ++i) {
 		Node *node = nodes[i];
@@ -742,7 +848,7 @@ void EffectChain::propagate_gamma_and_color_space()
 void EffectChain::propagate_alpha()
 {
 	// We depend on going through the nodes in order.
-	sort_nodes_topologically();
+	sort_all_nodes_topologically();
 
 	for (unsigned i = 0; i < nodes.size(); ++i) {
 		Node *node = nodes[i];
@@ -785,7 +891,7 @@ void EffectChain::propagate_alpha()
 		}
 
 		// Only inputs can have unconditional alpha output (OUTPUT_BLANK_ALPHA
-		// or OUTPUT_ALPHA_POSTMULTIPLIED), and they have already been
+		// or OUTPUT_POSTMULTIPLIED_ALPHA), and they have already been
 		// taken care of above. Rationale: Even if you could imagine
 		// e.g. an effect that took in an image and set alpha=1.0
 		// unconditionally, it wouldn't make any sense to have it as
@@ -793,7 +899,8 @@ void EffectChain::propagate_alpha()
 		// got its input pre- or postmultiplied, so it wouldn't know
 		// whether to divide away the old alpha or not.
 		Effect::AlphaHandling alpha_handling = node->effect->alpha_handling();
-		assert(alpha_handling == Effect::INPUT_AND_OUTPUT_ALPHA_PREMULTIPLIED ||
+		assert(alpha_handling == Effect::INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA ||
+		       alpha_handling == Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK ||
 		       alpha_handling == Effect::DONT_CARE_ALPHA_TYPE);
 
 		// If the node has multiple inputs, check that they are all valid and
@@ -833,16 +940,16 @@ void EffectChain::propagate_alpha()
 			continue;
 		}
 
-		if (alpha_handling == Effect::INPUT_AND_OUTPUT_ALPHA_PREMULTIPLIED) {
+		if (alpha_handling == Effect::INPUT_AND_OUTPUT_PREMULTIPLIED_ALPHA ||
+		    alpha_handling == Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK) {
 			// If the effect has asked for premultiplied alpha, check that it has got it.
 			if (any_postmultiplied) {
 				node->output_alpha_type = ALPHA_INVALID;
+			} else if (!any_premultiplied &&
+			           alpha_handling == Effect::INPUT_PREMULTIPLIED_ALPHA_KEEP_BLANK) {
+				// Blank input alpha, and the effect preserves blank alpha.
+				node->output_alpha_type = ALPHA_BLANK;
 			} else {
-				// In some rare cases, it might be advantageous to say
-				// that blank input alpha yields blank output alpha.
-				// However, this would cause a more complex Effect interface
-				// an effect would need to guarantee that it doesn't mess with
-				// blank alpha), so this is the simplest.
 				node->output_alpha_type = ALPHA_PREMULTIPLIED;
 			}
 		} else {
@@ -897,7 +1004,7 @@ void EffectChain::fix_internal_color_spaces()
 			}
 
 			// Go through each input that is not sRGB, and insert
-			// a colorspace conversion before it.
+			// a colorspace conversion after it.
 			for (unsigned j = 0; j < node->incoming_links.size(); ++j) {
 				Node *input = node->incoming_links[j];
 				assert(input->output_color_space != COLORSPACE_INVALID);
@@ -908,7 +1015,8 @@ void EffectChain::fix_internal_color_spaces()
 				CHECK(conversion->effect->set_int("source_space", input->output_color_space));
 				CHECK(conversion->effect->set_int("destination_space", COLORSPACE_sRGB));
 				conversion->output_color_space = COLORSPACE_sRGB;
-				insert_node_between(input, conversion, node);
+				replace_sender(input, conversion);
+				connect_nodes(input, conversion);
 			}
 
 			// Re-sort topologically, and propagate the new information.
@@ -988,7 +1096,8 @@ void EffectChain::fix_internal_alpha(unsigned step)
 					conversion = add_node(new AlphaDivisionEffect());
 				}
 				conversion->output_alpha_type = desired_type;
-				insert_node_between(input, conversion, node);
+				replace_sender(input, conversion);
+				connect_nodes(input, conversion);
 			}
 
 			// Re-sort topologically, and propagate the new information.
@@ -1039,14 +1148,14 @@ void EffectChain::fix_output_alpha()
 		return;
 	}
 	if (output->output_alpha_type == ALPHA_PREMULTIPLIED &&
-	    output_alpha_format == OUTPUT_ALPHA_POSTMULTIPLIED) {
+	    output_alpha_format == OUTPUT_ALPHA_FORMAT_POSTMULTIPLIED) {
 		Node *conversion = add_node(new AlphaDivisionEffect());
 		connect_nodes(output, conversion);
 		propagate_alpha();
 		propagate_gamma_and_color_space();
 	}
 	if (output->output_alpha_type == ALPHA_POSTMULTIPLIED &&
-	    output_alpha_format == OUTPUT_ALPHA_PREMULTIPLIED) {
+	    output_alpha_format == OUTPUT_ALPHA_FORMAT_PREMULTIPLIED) {
 		Node *conversion = add_node(new AlphaMultiplicationEffect());
 		connect_nodes(output, conversion);
 		propagate_alpha();
@@ -1171,7 +1280,7 @@ void EffectChain::fix_internal_gamma_by_inserting_nodes(unsigned step)
 			}
 
 			// If not, go through each input that is not linear gamma,
-			// and insert a gamma conversion before it.
+			// and insert a gamma conversion after it.
 			for (unsigned j = 0; j < node->incoming_links.size(); ++j) {
 				Node *input = node->incoming_links[j];
 				assert(input->output_gamma_curve != GAMMA_INVALID);
@@ -1181,7 +1290,8 @@ void EffectChain::fix_internal_gamma_by_inserting_nodes(unsigned step)
 				Node *conversion = add_node(new GammaExpansionEffect());
 				CHECK(conversion->effect->set_int("source_curve", input->output_gamma_curve));
 				conversion->output_gamma_curve = GAMMA_LINEAR;
-				insert_node_between(input, conversion, node);
+				replace_sender(input, conversion);
+				connect_nodes(input, conversion);
 			}
 
 			// Re-sort topologically, and propagate the new information.
@@ -1258,6 +1368,10 @@ Node *EffectChain::find_output_node()
 
 void EffectChain::finalize()
 {
+	// Save the current locale, and set it to C, so that we can output decimal
+	// numbers with printf and be sure to get them in the format mandated by GLSL.
+	char *saved_locale = setlocale(LC_NUMERIC, "C");
+
 	// Output the graph as it is before we do any conversions on it.
 	output_dot("step0-start.dot");
 
@@ -1319,8 +1433,6 @@ void EffectChain::finalize()
 	// since otherwise this turns into an (albeit simple)
 	// register allocation problem.
 	if (phases.size() > 1) {
-		glGenFramebuffers(1, &fbo);
-
 		for (unsigned i = 0; i < phases.size() - 1; ++i) {
 			inform_input_sizes(phases[i]);
 			find_output_size(phases[i]);
@@ -1350,6 +1462,7 @@ void EffectChain::finalize()
 	assert(phases[0]->inputs.empty());
 	
 	finalized = true;
+	setlocale(LC_NUMERIC, saved_locale);
 }
 
 void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height)
@@ -1358,6 +1471,7 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 
 	// Save original viewport.
 	GLuint x = 0, y = 0;
+	GLuint fbo = 0;
 
 	if (width == 0 && height == 0) {
 		GLint viewport[4];
@@ -1384,6 +1498,8 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 	glLoadIdentity();
 
 	if (phases.size() > 1) {
+		glGenFramebuffers(1, &fbo);
+		check_error();
 		glBindFramebuffer(GL_FRAMEBUFFER, fbo);
 		check_error();
 	}
@@ -1447,6 +1563,8 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 			// Last phase goes to the output the user specified.
 			glBindFramebuffer(GL_FRAMEBUFFER, dest_fbo);
 			check_error();
+			GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
+			assert(status == GL_FRAMEBUFFER_COMPLETE);
 			glViewport(x, y, width, height);
 			if (dither_effect != NULL) {
 				CHECK(dither_effect->set_int("output_width", width));
@@ -1461,6 +1579,8 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 				output_node->output_texture,
 				0);
 			check_error();
+			GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
+			assert(status == GL_FRAMEBUFFER_COMPLETE);
 			glViewport(0, 0, phases[phase]->output_width, phases[phase]->output_height);
 		}
 
@@ -1495,4 +1615,12 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 			node->effect->clear_gl_state();
 		}
 	}
+
+	glBindFramebuffer(GL_FRAMEBUFFER, 0);
+	check_error();
+
+	if (fbo != 0) {
+		glDeleteFramebuffers(1, &fbo);
+		check_error();
+	}
 }