Improved clarity of some comments, variable names, and functions

src/modules/graphics/opengl/Graphics.cpp

@@ -140,7 +140,7 @@ bool Graphics::setMode(int width, int height, bool fullscreen, bool vsync, int f
 
 	// Enable textures
 	glEnable(GL_TEXTURE_2D);
-	setActiveTextureUnit(GL_TEXTURE0);
+	setActiveTextureUnit(0);
 
 	// Set the viewport to top-left corner
 	glViewport(0, 0, width, height);

src/modules/graphics/opengl/OpenGL.cpp

@@ -32,7 +32,7 @@ namespace opengl
 
 static bool contextInitialized = false;
 
-static int curTextureUnitIndex = 0;
+static int curTextureUnit = 0;
 static std::vector<GLuint> textureUnits;
 
 void initializeContext()
@@ -60,10 +60,10 @@ void initializeContext()
 
 		textureUnits.resize(maxtextureunits, 0);
 
-		GLenum activetextureunit;
+		GLenum curgltextureunit;
-		glGetIntegerv(GL_ACTIVE_TEXTURE, (GLint *)&activetextureunit);
+		glGetIntegerv(GL_ACTIVE_TEXTURE, (GLint *)&curgltextureunit);
 
-		curTextureUnitIndex = activetextureunit - GL_TEXTURE0;
+		curTextureUnit = curgltextureunit - GL_TEXTURE0;
 
 		// retrieve currently bound textures for each texture unit
 		for (size_t i = 0; i < textureUnits.size(); ++i)
@@ -77,15 +77,15 @@ void initializeContext()
 		}
 
 		if (GLEE_VERSION_1_3)
-			glActiveTexture(activetextureunit);
+			glActiveTexture(curgltextureunit);
 		else
-			glActiveTextureARB(activetextureunit);
+			glActiveTextureARB(curgltextureunit);
 	}
 	else
 	{
 		// multitexturing not supported, so we only have 1 texture unit
 		textureUnits.resize(1, 0);
-		curTextureUnitIndex = 0;
+		curTextureUnit = 0;
 
 		glGetIntegerv(GL_TEXTURE_BINDING_2D, (GLint *) &textureUnits[0]);
 	}
@@ -96,58 +96,54 @@ void uninitializeContext()
 	contextInitialized = false;
 }
 
-void setActiveTextureUnit(GLenum textureunit)
+void setActiveTextureUnit(int textureunit)
 {
 	initializeContext();
 
-	int textureunitindex = textureunit - GL_TEXTURE0;
+	if (textureunit < 0 || (size_t) textureunit >= textureUnits.size())
+		throw love::Exception("Invalid texture unit index (%d).", textureunit);
 
-	if (textureunitindex < 0 || (size_t) textureunitindex >= textureUnits.size())
+	if (textureunit != curTextureUnit)
-		throw love::Exception("Invalid texture unit index.");
-
-	if (textureunitindex != curTextureUnitIndex)
 	{
 		if (GLEE_VERSION_1_3)
-			glActiveTexture(textureunit);
+			glActiveTexture(GL_TEXTURE0 + textureunit);
 		else if (GLEE_ARB_multitexture)
-			glActiveTextureARB(textureunit);
+			glActiveTextureARB(GL_TEXTURE0 + textureunit);
 		else
 			throw love::Exception("Multitexturing not supported.");
 	}
 
-	curTextureUnitIndex = textureunitindex;
+	curTextureUnit = textureunit;
 }
 
 void bindTexture(GLuint texture)
 {
 	initializeContext();
 
-	if (texture != textureUnits[curTextureUnitIndex])
+	if (texture != textureUnits[curTextureUnit])
 	{
-		textureUnits[curTextureUnitIndex] = texture;
+		textureUnits[curTextureUnit] = texture;
 		glBindTexture(GL_TEXTURE_2D, texture);
 	}
 }
 
-void bindTextureToUnit(GLuint texture, GLenum textureunit, bool restoreprev)
+void bindTextureToUnit(GLuint texture, int textureunit, bool restoreprev)
 {
 	initializeContext();
 
-	int textureunitindex = textureunit - GL_TEXTURE0;
+	if (textureunit < 0 || (size_t) textureunit >= textureUnits.size())
-
-	if (textureunitindex < 0 || (size_t) textureunitindex >= textureUnits.size())
 		throw love::Exception("Invalid texture unit index.");
 
-	if (texture != textureUnits[textureunitindex])
+	if (texture != textureUnits[textureunit])
 	{
-		int oldtexunitindex = curTextureUnitIndex;
+		int oldtextureunit = curTextureUnit;
 		setActiveTextureUnit(textureunit);
 
-		textureUnits[textureunitindex] = texture;
+		textureUnits[textureunit] = texture;
 		glBindTexture(GL_TEXTURE_2D, texture);
 
 		if (restoreprev)
-			setActiveTextureUnit(GL_TEXTURE0 + oldtexunitindex);
+			setActiveTextureUnit(oldtextureunit);
 	}
 }
 
@@ -155,6 +151,7 @@ void deleteTexture(GLuint texture)
 {
 	initializeContext();
 
+	// glDeleteTextures binds texture 0 to all texture units the deleted texture was bound to
 	std::vector<GLuint>::iterator it;
 	for (it = textureUnits.begin(); it != textureUnits.end(); ++it)
 	{
@@ -167,6 +164,8 @@ void deleteTexture(GLuint texture)
 
 void setTextureFilter(const graphics::Image::Filter &f)
 {
+	initializeContext();
+
 	GLint gmin, gmag;
 
 	if (f.mipmap == Image::FILTER_NONE)
@@ -208,6 +207,8 @@ void setTextureFilter(const graphics::Image::Filter &f)
 
 graphics::Image::Filter getTextureFilter()
 {
+	initializeContext();
+
 	GLint gmin, gmag;
 	glGetTexParameteriv(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, &gmin);
 	glGetTexParameteriv(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, &gmag);
@@ -257,6 +258,8 @@ graphics::Image::Filter getTextureFilter()
 
 void setTextureWrap(const graphics::Image::Wrap &w)
 {
+	initializeContext();
+
 	GLint gs, gt;
 
 	switch (w.s)
@@ -287,6 +290,8 @@ void setTextureWrap(const graphics::Image::Wrap &w)
 
 graphics::Image::Wrap getTextureWrap()
 {
+	initializeContext();
+
 	GLint gs, gt;
 
 	glGetTexParameteriv(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, &gs);

src/modules/graphics/opengl/OpenGL.h

@@ -31,61 +31,61 @@ namespace graphics
 namespace opengl
 {
 
+/**
+ * Initializes some required context state,
+ * based on current and default OpenGL state.
+ **/
 void initializeContext();
 
+/**
+ * Marks current context state as invalid.
+ **/
 void uninitializeContext();
 
 /**
- * Helper for setting the active texture unit
+ * Helper for setting the active texture unit.
  *
- * @param textureunit The GL texture unit to set
+ * @param textureunit Index in the range of [0, maxtextureunits-1]
  **/
-void setActiveTextureUnit(GLenum textureunit);
+void setActiveTextureUnit(int textureunit);
 
 /**
  * Helper for binding an OpenGL texture.
  * Makes sure we aren't redundantly binding textures.
- * 
- * @param texture The texture to bind.
  **/
 void bindTexture(GLuint texture);
 
 /**
- * Helper for binding a texture to a specific texture unit
+ * Helper for binding a texture to a specific texture unit.
  *
- * @param texture The texture to bind
+ * @param textureunit Index in the range of [0, maxtextureunits-1]
- * @param textureunit The texture unit to switch to
+ * @param resoreprev Restore previously bound texture unit when done.
- * @param resoreprev Restore previous texture unit when done
  **/
-void bindTextureToUnit(GLuint texture, GLenum textureunit, bool restoreprev);
+void bindTextureToUnit(GLuint texture, int textureunit, bool restoreprev);
 
 /**
  * Helper for deleting an OpenGL texture.
  * Cleans up if the texture is currently bound.
- * 
- * @param texture The texture to delete.
  **/
 void deleteTexture(GLuint texture);
 
 /**
- * Sets the image filter mode for the currently bound texture
+ * Sets the image filter mode for the currently bound texture.
- * @param f The image filter to set
  */
 void setTextureFilter(const graphics::Image::Filter &f);
 
 /**
- * Returns the image filter mode for the currently bound texture
+ * Returns the image filter mode for the currently bound texture.
  */
 graphics::Image::Filter getTextureFilter();
 
 /**
- * Sets the image wrap mode for the currently bound texture
+ * Sets the image wrap mode for the currently bound texture.
- * @param w The wrap mode to set
  */
 void setTextureWrap(const graphics::Image::Wrap &w);
 
 /**
- * Returns the image wrap mode for the currently bound texture
+ * Returns the image wrap mode for the currently bound texture.
  */
 graphics::Image::Wrap getTextureWrap();
 

src/modules/graphics/opengl/ShaderEffect.cpp

@@ -18,6 +18,8 @@
  * 3. This notice may not be removed or altered from any source distribution.
  **/
 
+#include <algorithm>
+
 #include "ShaderEffect.h"
 #include "Graphics.h"
 
@@ -34,8 +36,8 @@ namespace
 	// reattaches the originally active program when destroyed
 	struct TemporaryAttacher
 	{
-		TemporaryAttacher(ShaderEffect *sp)
+		TemporaryAttacher(ShaderEffect *effect)
-		: cureffect(sp)
+		: cureffect(effect)
 		, preveffect(ShaderEffect::current)
 		{
 			cureffect->attach(true);
@@ -231,10 +233,8 @@ void ShaderEffect::unloadVolatile()
 	// decrement global texture id counters for texture units which had textures bound from this shader
 	for (size_t i = 0; i < _activetextureunits.size(); ++i)
 	{
-		if (_activetextureunits[i] == 0)
+		if (_activetextureunits[i] > 0)
-			continue;
+			_texturecounters[i] = std::max(_texturecounters[i] - 1, 0);
-
-		_texturecounters[i] = std::max(_texturecounters[i] - 1, 0);
 	}
 
 	// active texture list is probably invalid, clear it
@@ -273,12 +273,10 @@ void ShaderEffect::attach(bool temporary)
 		// note: list potentially contains texture ids of deleted/invalid textures!
 		for (size_t i = 0; i < _activetextureunits.size(); ++i)
 		{
-			if (_activetextureunits[i] == 0)
+			if (_activetextureunits[i] > 0)
-				continue;
+				bindTextureToUnit(_activetextureunits[i], i + 1, false);
-
-			bindTextureToUnit(_activetextureunits[i], GL_TEXTURE0 + i + 1, false);
 		}
-		setActiveTextureUnit(GL_TEXTURE0);
+		setActiveTextureUnit(0);
 	}
 }
 
@@ -352,24 +350,24 @@ void ShaderEffect::sendTexture(const std::string &name, GLuint texture)
 {
 	TemporaryAttacher attacher(this);
 	GLint location = getUniformLocation(name);
-	GLint texture_unit = getTextureUnit(name);
+	int textureunit = getTextureUnit(name);
 
-	// bind texture to assigned texture unit and send uniform to bound shader program
+	// bind texture to assigned texture unit and send uniform to shader program
-	bindTextureToUnit(texture, GL_TEXTURE0 + texture_unit, false);
+	bindTextureToUnit(texture, textureunit, false);
-	glUniform1i(location, texture_unit);
+	glUniform1i(location, textureunit);
 
 	// reset texture unit
-	setActiveTextureUnit(GL_TEXTURE0);
+	setActiveTextureUnit(0);
-
-	// increment global shader texture id counter for this texture unit, if we haven't already
-	if (_activetextureunits[texture_unit-1] == 0)
-		++_texturecounters[texture_unit-1];
-
-	// store texture id so it can be re-bound to the proper texture unit when necessary
-	_activetextureunits[texture_unit-1] = texture;
 
 	// throw error if needed
 	checkSetUniformError();
+
+	// increment global shader texture id counter for this texture unit, if we haven't already
+	if (_activetextureunits[textureunit-1] == 0)
+		++_texturecounters[textureunit-1];
+
+	// store texture id so it can be re-bound to the proper texture unit when necessary
+	_activetextureunits[textureunit-1] = texture;
 }
 
 void ShaderEffect::sendImage(const std::string &name, const Image &image)
@@ -400,20 +398,20 @@ GLint ShaderEffect::getUniformLocation(const std::string &name)
 	return location;
 }
 
-GLint ShaderEffect::getTextureUnit(const std::string &name)
+int ShaderEffect::getTextureUnit(const std::string &name)
 {
 	std::map<std::string, GLint>::const_iterator it = _textureunitpool.find(name);
 
 	if (it != _textureunitpool.end())
 		return it->second;
 
-	int nextunitindex = 1;
+	int textureunit = 1;
 
 	// prefer texture units which are unused by all other shaders
 	std::vector<int>::iterator nextfreeunit = std::find(_texturecounters.begin(), _texturecounters.end(), 0);
 
 	if (nextfreeunit != _texturecounters.end())
-		nextunitindex = std::distance(_texturecounters.begin(), nextfreeunit) + 1; // we don't want to use unit 0
+		textureunit = std::distance(_texturecounters.begin(), nextfreeunit) + 1; // we don't want to use unit 0
 	else
 	{
 		// no completely unused texture units exist, try to use next free slot in our own list
@@ -422,11 +420,11 @@ GLint ShaderEffect::getTextureUnit(const std::string &name)
 		if (nexttexunit == _activetextureunits.end())
 			throw love::Exception("No more texture units available for shader.");
 
-		nextunitindex = std::distance(_activetextureunits.begin(), nexttexunit) + 1; // we don't want to use unit 0
+		textureunit = std::distance(_activetextureunits.begin(), nexttexunit) + 1; // we don't want to use unit 0
 	}
 
-	_textureunitpool[name] = nextunitindex;
+	_textureunitpool[name] = textureunit;
-	return nextunitindex;
+	return textureunit;
 }
 
 void ShaderEffect::checkSetUniformError()

src/modules/graphics/opengl/ShaderEffect.h

@@ -40,7 +40,9 @@ class ShaderEffect : public Object, public Volatile
 {
 public:
 
-	// Different types of shaders.
+	// pointer to currently active ShaderEffect.
+	static ShaderEffect *current;
+	
 	// Only vertex and fragment shaders have guaranteed support in all ShaderEffects.
 	enum ShaderType
 	{
@@ -57,9 +59,7 @@ public:
 
 	/**
 	 * Creates a new ShaderEffect using a list of source codes.
-	 * Must contain at least one vertex or fragment shader source.
+	 * Sources must contain at least one vertex or fragment shader.
-	 * 
-	 * @param shadersources Map of shader types to source codes.
 	 **/
 	ShaderEffect(const ShaderSources &shadersources);
 
@@ -92,7 +92,7 @@ public:
 	 *
 	 * @param name The name of the uniform variable in the source code.
 	 * @param size Number of elements in each vector to send.
-	 *             A value of 1 indicates a single-component vector, AKA a float.
+	 *             A value of 1 indicates a single-component vector (a float).
 	 * @param vec Pointer to the float or vector values.
 	 * @param count Number of float or vector values.
 	 **/
@@ -112,7 +112,6 @@ public:
 	 * Send an image to this ShaderEffect as a uniform.
 	 *
 	 * @param name The name of the uniform variable in the source code.
-	 * @param image The image to send.
 	 **/
 	void sendImage(const std::string &name, const Image &image);
 
@@ -120,23 +119,12 @@ public:
 	 * Send a canvas to this ShaderEffect as a uniform.
 	 *
 	 * @param name The name of the uniform variable in the source code.
-	 * @param canvas The canvas to send.
 	 **/
 	void sendCanvas(const std::string &name, const Canvas &canvas);
 
-	/**
-	 * Returns the maximum GLSL version supported on this system.
-	 **/
 	static std::string getGLSLVersion();
-
-	/**
-	 * Returns whether ShaderEffects are supported on this system.
-	 **/
 	static bool isSupported();
 
-	// pointer to currently active ShaderEffect.
-	static ShaderEffect *current;
-
 private:
 
 	GLint getUniformLocation(const std::string &name);
@@ -145,7 +133,7 @@ private:
 	GLuint createShader(ShaderType type, const std::string &code);
 	void createProgram(const std::vector<GLuint> &shaderids);
 
-	GLint getTextureUnit(const std::string &name);
+	int getTextureUnit(const std::string &name);
 
 	void sendTexture(const std::string &name, GLuint texture);
 
@@ -158,10 +146,10 @@ private:
 	std::map<std::string, GLint> _uniforms;
 
 	// texture unit pool for setting images
-	std::map<std::string, GLint> _textureunitpool; // _textureunitpool[name] = textureunitindex
+	std::map<std::string, GLint> _textureunitpool; // _textureunitpool[name] = textureunit
-	std::vector<GLuint> _activetextureunits; // _activetextureunits[textureunitindex-1] = textureid
+	std::vector<GLuint> _activetextureunits; // _activetextureunits[textureunit-1] = textureid
 
-	// total max GPU texture units for shaders
+	// max GPU texture units available for sent images
 	static GLint _maxtextureunits;
 
 	// counts total number of textures bound to each texture unit in all shaders

src/modules/graphics/opengl/wrap_ShaderEffect.cpp

@@ -132,7 +132,7 @@ int w_ShaderEffect_sendFloat(lua_State *L)
 	else if (lua_istable(L, 3))
 		return _sendVectors(L, effect, name, count);
 
-	return luaL_typerror(L, 3, "number, boolean or table");
+	return luaL_typerror(L, 3, "number, boolean, or table");
 }
 
 int w_ShaderEffect_sendMatrix(lua_State *L)