From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Sat, 22 Mar 2014 13:54:43 +0000 (+0100)
Subject: Redo FBO association yet again, this time per-texture.
X-Git-Tag: 1.1~26
X-Git-Url: https://git.sesse.net/?p=movit;a=commitdiff_plain;h=0cdd6e9e30a807056699ed9b3fa023a23ce83af9

Redo FBO association yet again, this time per-texture.

According to http://adrienb.fr/blog/wp-content/uploads/2013/04/PortingSourceToLinux.pdf,
you want an FBO per-texture, not just format. And indeed, I can measure a very slight
performance improvement on both NVidia and ATI for this.
---

diff --git a/effect_chain.cpp b/effect_chain.cpp
index c3e1fba..ca6a777 100644
--- a/effect_chain.cpp
+++ b/effect_chain.cpp
@@ -1519,18 +1519,8 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 				CHECK(dither_effect->set_int("output_height", height));
 			}
 		} else {
-			fbo = resource_pool->create_fbo(context, GL_RGBA16F, phase->output_width, phase->output_height);
+			fbo = resource_pool->create_fbo(context, output_textures[phase]);
 			glBindFramebuffer(GL_FRAMEBUFFER, fbo);
-			check_error();
-			glFramebufferTexture2D(
-				GL_FRAMEBUFFER,
-			        GL_COLOR_ATTACHMENT0,
-				GL_TEXTURE_2D,
-				output_textures[phase],
-				0);
-			check_error();
-			GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
-			assert(status == GL_FRAMEBUFFER_COMPLETE);
 			glViewport(0, 0, phase->output_width, phase->output_height);
 		}
 
diff --git a/resource_pool.cpp b/resource_pool.cpp
index b6f6a31..a83add2 100644
--- a/resource_pool.cpp
+++ b/resource_pool.cpp
@@ -248,11 +248,25 @@ void ResourcePool::release_2d_texture(GLuint texture_num)
 		texture_formats.erase(free_texture_num);
 		glDeleteTextures(1, &free_texture_num);
 		check_error();
+
+		// Delete any FBO related to this texture.
+		for (list<GLuint>::iterator fbo_freelist_it = fbo_freelist.begin();
+		     fbo_freelist_it != fbo_freelist.end(); ) {
+			GLuint fbo_num = *fbo_freelist_it;
+			map<GLuint, FBO>::const_iterator format_it = fbo_formats.find(fbo_num);
+			assert(format_it != fbo_formats.end());
+			if (format_it->second.texture_num == free_texture_num) {
+				glDeleteFramebuffers(1, &fbo_num);
+				fbo_freelist.erase(fbo_freelist_it++);
+			} else {
+				++fbo_freelist_it;
+			}
+		}
 	}
 	pthread_mutex_unlock(&lock);
 }
 
-GLuint ResourcePool::create_fbo(void *context, GLint internal_format, GLsizei width, GLsizei height)
+GLuint ResourcePool::create_fbo(void *context, GLuint texture_num)
 {
 	pthread_mutex_lock(&lock);
 	// See if there's an FBO on the freelist we can use.
@@ -263,9 +277,7 @@ GLuint ResourcePool::create_fbo(void *context, GLint internal_format, GLsizei wi
 		map<GLuint, FBO>::const_iterator format_it = fbo_formats.find(fbo_num);
 		assert(format_it != fbo_formats.end());
 		if (format_it->second.context == context &&
-		    format_it->second.internal_format == internal_format &&
-		    format_it->second.width == width &&
-		    format_it->second.height == height) {
+		    format_it->second.texture_num == texture_num) {
 			fbo_freelist.erase(freelist_it);
 			pthread_mutex_unlock(&lock);
 			return fbo_num;
@@ -276,12 +288,23 @@ GLuint ResourcePool::create_fbo(void *context, GLint internal_format, GLsizei wi
 	GLuint fbo_num;
 	glGenFramebuffers(1, &fbo_num);
 	check_error();
+	glBindFramebuffer(GL_FRAMEBUFFER, fbo_num);
+	check_error();
+	glFramebufferTexture2D(
+		GL_FRAMEBUFFER,
+		GL_COLOR_ATTACHMENT0,
+		GL_TEXTURE_2D,
+		texture_num,
+		0);
+	check_error();
+	GLenum status = glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT);
+	assert(status == GL_FRAMEBUFFER_COMPLETE);
+	glBindFramebuffer(GL_FRAMEBUFFER, 0);
+	check_error();
 
 	FBO fbo_format;
 	fbo_format.context = context;
-	fbo_format.internal_format = internal_format;
-	fbo_format.width = width;
-	fbo_format.height = height;
+	fbo_format.texture_num = texture_num;
 	assert(fbo_formats.count(fbo_num) == 0);
 	fbo_formats.insert(make_pair(fbo_num, fbo_format));
 
diff --git a/resource_pool.h b/resource_pool.h
index 86d0059..4ae4110 100644
--- a/resource_pool.h
+++ b/resource_pool.h
@@ -60,17 +60,18 @@ public:
 	GLuint create_2d_texture(GLint internal_format, GLsizei width, GLsizei height);
 	void release_2d_texture(GLuint texture_num);
 
-	// Allocate an FBO used for the given internal format and dimensions,
-	// or fetch a previous used if possible. Keeps ownership of the FBO;
-	// you must call release_fbo() of deleting it when you no longer want it.
-	// You can get an appropriate context pointer from get_gl_context_identifier().
+	// Allocate an FBO with the the given texture bound as a framebuffer attachment,
+	// or fetch a previous used if possible. Unbinds GL_FRAMEBUFFER afterwards.
+	// Keeps ownership of the FBO; you must call release_fbo() of deleting
+	// it when you no longer want it. You can get an appropriate context
+	// pointer from get_gl_context_identifier().
 	//
 	// NOTE: In principle, the FBO doesn't have a resolution or pixel format;
 	// you can bind almost whatever texture you want to it. However, changing
-	// resolution or pixel formats can have an adverse effect on performance,
+	// textures can have an adverse effect on performance due to validation,
 	// in particular on NVidia cards. Also, keep in mind that FBOs are not
 	// shareable across contexts.
-	GLuint create_fbo(void *context, GLint internal_format, GLsizei width, GLsizei height);
+	GLuint create_fbo(void *context, GLuint texture_num);
 	void release_fbo(GLuint fbo_num);
 
 private:
@@ -118,8 +119,7 @@ private:
 
 	struct FBO {
 		void *context;
-		GLint internal_format;
-		GLsizei width, height;
+		GLuint texture_num;
 	};
 
 	// A mapping from FBO number to format details. This is filled if the