From 2db9bfad111ad9a08986ea9bf7268ab198432e08 Mon Sep 17 00:00:00 2001
From: vrld <vrld@vrld.org>
Date: Sun, 13 Jan 2013 16:19:27 +0100
Subject: [PATCH] Fix PixelEffect texture unit pool.

Fixes two problems: (1) Overwriting externs with the same name and (2) not using unused texture units properly.

1) Sending two different images to two different effects using the same extern name would use the same image for both effects, e.g.:

	effect_1:send('img', img_1)
	effect_2:send('img', img_2) -- also changes img to point to img_2 in effect_1!

Fixed by localizing the name -> texture unit mapping for each pixel effect.

2) When a pixel effect was destroyed, the allocated texture units were not marked as being available again. Fixed by having a vector<bool> marking the free units instead of just a counter how many units are in use.


Still problematic: Sending more than GL_MAX_TEXTURE_IMAGE_UNITS (>= 16, depending on the implementation) images/canvases *in total* to pixel effects. Effects could hold references to used *texture-ids* and bind/unbind them dynamically on a effect switch. Con: Introduces additional complexity when switching effects. However, benchmarks by Alexander Szpakowski have shown the performance loss is negligible.
---
 src/modules/graphics/opengl/PixelEffect.cpp | 29 ++++++++++++++++-----
 src/modules/graphics/opengl/PixelEffect.h   |  8 +++---
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/modules/graphics/opengl/PixelEffect.cpp b/src/modules/graphics/opengl/PixelEffect.cpp
index 8e570fb27..9accd90ed 100644
--- a/src/modules/graphics/opengl/PixelEffect.cpp
+++ b/src/modules/graphics/opengl/PixelEffect.cpp
@@ -51,9 +51,7 @@ namespace opengl
 
 PixelEffect *PixelEffect::current = NULL;
 
-std::map<std::string, GLint> PixelEffect::_texture_unit_pool;
-GLint PixelEffect::_current_texture_unit = 0;
-GLint PixelEffect::_max_texture_units = 0;
+std::vector<bool> PixelEffect::_unit_available;
 
 GLint PixelEffect::getTextureUnit(const std::string &name)
 {
@@ -62,18 +60,35 @@ GLint PixelEffect::getTextureUnit(const std::string &name)
 	if (it != _texture_unit_pool.end())
 		return it->second;
 
-	if (++_current_texture_unit >= _max_texture_units)
+	GLint unit = -1;
+	for (int i = 1; i < _unit_available.size(); ++i)
+	{
+		if (_unit_available[i])
+		{
+			unit = i;
+			break;
+		}
+	}
+
+	if (unit == -1)
 		throw love::Exception("No more texture units available");
 
-	_texture_unit_pool[name] = _current_texture_unit;
-	return _current_texture_unit;
+	_unit_available[unit] = false;
+	_texture_unit_pool[name] = unit;
+	return unit;
 }
 
 PixelEffect::PixelEffect(const std::string &code)
 	: _program(0)
 	, _code(code)
 {
-	glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &_max_texture_units);
+	if (_unit_available.empty())
+	{
+		GLint max_units;
+		glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &max_units);
+		_unit_available.resize(max_units, true);
+		_unit_available[0] = false;
+	}
 	loadVolatile();
 }
 
diff --git a/src/modules/graphics/opengl/PixelEffect.h b/src/modules/graphics/opengl/PixelEffect.h
index e1818e735..b0d7fc704 100644
--- a/src/modules/graphics/opengl/PixelEffect.h
+++ b/src/modules/graphics/opengl/PixelEffect.h
@@ -24,6 +24,7 @@
 #include "common/Object.h"
 #include <string>
 #include <map>
+#include <vector>
 #include "OpenGL.h"
 #include "Image.h"
 #include "Canvas.h"
@@ -67,10 +68,9 @@ private:
 	std::map<std::string, GLint> _uniforms;
 
 	// texture unit pool for setting images
-	static std::map<std::string, GLint> _texture_unit_pool;
-	static GLint _current_texture_unit;
-	static GLint _max_texture_units;
-	static GLint getTextureUnit(const std::string &name);
+	std::map<std::string, GLint> _texture_unit_pool;
+	GLint getTextureUnit(const std::string &name);
+	static std::vector<bool> _unit_available;
 };
 
 } // opengl