From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Tue, 18 Mar 2014 21:16:36 +0000 (+0100)
Subject: Keep FBOs around in EffectChain again.
X-Git-Tag: 1.1~42
X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=6aff20a0932dc9b668b77981390539dd710cf06d

Keep FBOs around in EffectChain again.

Seemingly creating and deleting them is crazy expensive on NVidia
(~3 ms for a create/delete pair), so 6dea8d2 caused a performance
regression at high frame rates. Now we instead keep one around per
context (they cannot be shared), which brings us basically back
to where we were performance-wise.

Reported by Christophe Thommeret.
---

diff --git a/effect_chain.cpp b/effect_chain.cpp
index c5e10e8..8d3c61d 100644
--- a/effect_chain.cpp
+++ b/effect_chain.cpp
@@ -59,6 +59,11 @@ EffectChain::~EffectChain()
 	if (owns_resource_pool) {
 		delete resource_pool;
 	}
+	for (map<void *, GLuint>::const_iterator fbo_it = fbos.begin();
+	     fbo_it != fbos.end(); ++fbo_it) {
+		glDeleteFramebuffers(1, &fbo_it->second);
+		check_error();
+	}
 }
 
 Input *EffectChain::add_input(Input *input)
@@ -1405,6 +1410,7 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 	// Save original viewport.
 	GLuint x = 0, y = 0;
 	GLuint fbo = 0;
+	void *context = get_gl_context_identifier();
 
 	if (width == 0 && height == 0) {
 		GLint viewport[4];
@@ -1424,8 +1430,13 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 	check_error();
 
 	if (phases.size() > 1) {
-		glGenFramebuffers(1, &fbo);
-		check_error();
+		if (fbos.count(context) == 0) {
+			glGenFramebuffers(1, &fbo);
+			check_error();
+			fbos.insert(make_pair(context, fbo));
+		} else {
+			fbo = fbos[context];
+		}
 		glBindFramebuffer(GL_FRAMEBUFFER, fbo);
 		check_error();
 	}
@@ -1564,11 +1575,6 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 
 	glBindFramebuffer(GL_FRAMEBUFFER, 0);
 	check_error();
-
-	if (fbo != 0) {
-		glDeleteFramebuffers(1, &fbo);
-		check_error();
-	}
 }
 
 }  // namespace movit
diff --git a/effect_chain.h b/effect_chain.h
index 5994949..505e55a 100644
--- a/effect_chain.h
+++ b/effect_chain.h
@@ -280,6 +280,7 @@ private:
 	std::map<Effect *, Node *> node_map;
 	Effect *dither_effect;
 
+	std::map<void *, GLuint> fbos;  // One for each OpenGL context.
 	std::vector<Input *> inputs;  // Also contained in nodes.
 	std::vector<Phase *> phases;
 
diff --git a/util.cpp b/util.cpp
index 310e7be..1e66c30 100644
--- a/util.cpp
+++ b/util.cpp
@@ -9,6 +9,14 @@
 #include "init.h"
 #include "util.h"
 
+#if defined(__DARWIN__)
+#include <OpenGL/OpenGL.h>
+#elif defined(WIN32)
+#include <GL/wglew.h>
+#else
+#include <GL/glxew.h>
+#endif
+
 using namespace std;
 
 namespace movit {
@@ -233,4 +241,15 @@ unsigned next_power_of_two(unsigned v)
 	return v;
 }
 
+void *get_gl_context_identifier()
+{
+#if defined(__DARWIN__)
+	return (void *)CGLGetCurrentContext();
+#elif defined(WIN32)
+	return (void *)wglGetCurrentContext();
+#else
+	return (void *)glXGetCurrentContext();
+#endif
+}
+
 }  // namespace movit
diff --git a/util.h b/util.h
index c5dafed..1ecdc00 100644
--- a/util.h
+++ b/util.h
@@ -56,6 +56,12 @@ void cleanup_vertex_attribute(GLuint glsl_program_num, const std::string &attrib
 // If v is not already a power of two, return the first higher power of two.
 unsigned next_power_of_two(unsigned v);
 
+// Get a pointer that represents the current OpenGL context, in a cross-platform way.
+// This is not intended for anything but identification (ie., so you can associate
+// different FBOs with different contexts); you should probably not try to cast it
+// back into anything you intend to pass into OpenGL.
+void *get_gl_context_identifier();
+
 }  // namespace movit
 
 #ifdef NDEBUG