From 331c883c777a0ed0302b0901427a462bb30c5596 Mon Sep 17 00:00:00 2001 From: Alexander Szpakowski Date: Tue, 15 Jan 2013 20:20:35 -0400 Subject: [PATCH] Improved clarity of some comments, variable names, and functions --- src/modules/graphics/opengl/Graphics.cpp | 2 +- src/modules/graphics/opengl/OpenGL.cpp | 57 ++++++++++--------- src/modules/graphics/opengl/OpenGL.h | 40 ++++++------- src/modules/graphics/opengl/ShaderEffect.cpp | 56 +++++++++--------- src/modules/graphics/opengl/ShaderEffect.h | 38 +++++-------- .../graphics/opengl/wrap_ShaderEffect.cpp | 2 +- 6 files changed, 93 insertions(+), 102 deletions(-) diff --git a/src/modules/graphics/opengl/Graphics.cpp b/src/modules/graphics/opengl/Graphics.cpp index f64aeec68..2815bd363 100644 --- a/src/modules/graphics/opengl/Graphics.cpp +++ b/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); diff --git a/src/modules/graphics/opengl/OpenGL.cpp b/src/modules/graphics/opengl/OpenGL.cpp index b0be8fcb2..1d49cb24d 100644 --- a/src/modules/graphics/opengl/OpenGL.cpp +++ b/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 textureUnits; void initializeContext() @@ -60,10 +60,10 @@ void initializeContext() textureUnits.resize(maxtextureunits, 0); - GLenum activetextureunit; - glGetIntegerv(GL_ACTIVE_TEXTURE, (GLint *)&activetextureunit); + GLenum curgltextureunit; + 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()) - throw love::Exception("Invalid texture unit index."); - - if (textureunitindex != curTextureUnitIndex) + if (textureunit != curTextureUnit) { 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 (textureunitindex < 0 || (size_t) textureunitindex >= textureUnits.size()) + if (textureunit < 0 || (size_t) textureunit >= 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::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); diff --git a/src/modules/graphics/opengl/OpenGL.h b/src/modules/graphics/opengl/OpenGL.h index 39439b1e2..b26fc6a8b 100644 --- a/src/modules/graphics/opengl/OpenGL.h +++ b/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 - * - * @param textureunit The GL texture unit to set + * Helper for setting the active texture unit. + * + * @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 - * - * @param texture The texture to bind - * @param textureunit The texture unit to switch to - * @param resoreprev Restore previous texture unit when done + * Helper for binding a texture to a specific texture unit. + * + * @param textureunit Index in the range of [0, maxtextureunits-1] + * @param resoreprev Restore previously bound 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 - * @param f The image filter to set + * Sets the image filter mode for the currently bound texture. */ 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 - * @param w The wrap mode to set + * Sets the image wrap mode for the currently bound texture. */ 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(); diff --git a/src/modules/graphics/opengl/ShaderEffect.cpp b/src/modules/graphics/opengl/ShaderEffect.cpp index 592cfe59d..706f5111c 100644 --- a/src/modules/graphics/opengl/ShaderEffect.cpp +++ b/src/modules/graphics/opengl/ShaderEffect.cpp @@ -18,6 +18,8 @@ * 3. This notice may not be removed or altered from any source distribution. **/ +#include + #include "ShaderEffect.h" #include "Graphics.h" @@ -34,8 +36,8 @@ namespace // reattaches the originally active program when destroyed struct TemporaryAttacher { - TemporaryAttacher(ShaderEffect *sp) - : cureffect(sp) + TemporaryAttacher(ShaderEffect *effect) + : 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) - continue; - - _texturecounters[i] = std::max(_texturecounters[i] - 1, 0); + if (_activetextureunits[i] > 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) - continue; - - bindTextureToUnit(_activetextureunits[i], GL_TEXTURE0 + i + 1, false); + if (_activetextureunits[i] > 0) + bindTextureToUnit(_activetextureunits[i], 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 - bindTextureToUnit(texture, GL_TEXTURE0 + texture_unit, false); - glUniform1i(location, texture_unit); + // bind texture to assigned texture unit and send uniform to shader program + bindTextureToUnit(texture, textureunit, false); + glUniform1i(location, textureunit); // reset texture unit - setActiveTextureUnit(GL_TEXTURE0); - - // 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; + setActiveTextureUnit(0); // 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::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::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; - return nextunitindex; + _textureunitpool[name] = textureunit; + return textureunit; } void ShaderEffect::checkSetUniformError() diff --git a/src/modules/graphics/opengl/ShaderEffect.h b/src/modules/graphics/opengl/ShaderEffect.h index d203d678c..d5e01ef9c 100644 --- a/src/modules/graphics/opengl/ShaderEffect.h +++ b/src/modules/graphics/opengl/ShaderEffect.h @@ -39,8 +39,10 @@ namespace opengl class ShaderEffect : public Object, public Volatile { public: + + // pointer to currently active ShaderEffect. + static ShaderEffect *current; - // Different types of shaders. // 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. - * - * @param shadersources Map of shader types to source codes. + * Sources must contain at least one vertex or fragment shader. **/ ShaderEffect(const ShaderSources &shadersources); @@ -71,7 +71,7 @@ public: /** * Binds this ShaderEffect's program to be used when rendering. - * + * * @param temporary True if we just want to send values to the shader with no intention of rendering. **/ void attach(bool temporary = false); @@ -89,10 +89,10 @@ public: /** * Send at least one float or vector value to this ShaderEffect as a uniform. - * + * * @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. **/ @@ -100,7 +100,7 @@ public: /** * Send at least one matrix to this ShaderEffect as a uniform. - * + * * @param name The name of the uniform variable in the source code. * @param size Number of rows/columns in the matrix. * @param m Pointer to the first element of the first matrix. @@ -110,9 +110,8 @@ 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 &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 _uniforms; // texture unit pool for setting images - std::map _textureunitpool; // _textureunitpool[name] = textureunitindex - std::vector _activetextureunits; // _activetextureunits[textureunitindex-1] = textureid + std::map _textureunitpool; // _textureunitpool[name] = textureunit + std::vector _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 diff --git a/src/modules/graphics/opengl/wrap_ShaderEffect.cpp b/src/modules/graphics/opengl/wrap_ShaderEffect.cpp index 487534f16..50e8e69f3 100644 --- a/src/modules/graphics/opengl/wrap_ShaderEffect.cpp +++ b/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)