From 0bcd7d39f263a76103cee2a186882114bc5324af Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Thu, 25 Jun 2015 21:38:49 +0200 Subject: [PATCH] Made implementation of glCheck and alCheck macros more robust At the moment, glCheck(...) and alCheck(...) look like a function calls, which is misleading and can cause subtle bugs, for example when used in if statements. This change mitigates the situation by allowing those expressions to be used as single statements within if/else branches. Initializations of OpenGL handles that previously involved glCheck() calls now need to be split into separate declaration and assignment. --- src/SFML/Audio/ALCheck.hpp | 3 ++- src/SFML/Graphics/GLCheck.hpp | 3 ++- src/SFML/Graphics/RenderTextureImplFBO.cpp | 3 ++- src/SFML/Graphics/Shader.cpp | 24 ++++++++++++++-------- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/SFML/Audio/ALCheck.hpp b/src/SFML/Audio/ALCheck.hpp index f8511119c..34b762642 100644 --- a/src/SFML/Audio/ALCheck.hpp +++ b/src/SFML/Audio/ALCheck.hpp @@ -50,7 +50,8 @@ namespace priv #ifdef SFML_DEBUG // If in debug mode, perform a test on every call - #define alCheck(x) x; sf::priv::alCheckError(__FILE__, __LINE__); + // The do-while loop is needed so that alCheck can be used as a single statement in if/else branches + #define alCheck(expr) do { expr; sf::priv::alCheckError(__FILE__, __LINE__, #expr); } while (false) #else diff --git a/src/SFML/Graphics/GLCheck.hpp b/src/SFML/Graphics/GLCheck.hpp index 743afcb0c..94e386b37 100644 --- a/src/SFML/Graphics/GLCheck.hpp +++ b/src/SFML/Graphics/GLCheck.hpp @@ -43,7 +43,8 @@ namespace priv #ifdef SFML_DEBUG // In debug mode, perform a test on every OpenGL call - #define glCheck(x) x; sf::priv::glCheckError(__FILE__, __LINE__); + // The do-while loop is needed so that glCheck can be used as a single statement in if/else branches + #define glCheck(expr) do { expr; sf::priv::glCheckError(__FILE__, __LINE__, #expr); } while (false) #else diff --git a/src/SFML/Graphics/RenderTextureImplFBO.cpp b/src/SFML/Graphics/RenderTextureImplFBO.cpp index 952687261..20ff12c08 100644 --- a/src/SFML/Graphics/RenderTextureImplFBO.cpp +++ b/src/SFML/Graphics/RenderTextureImplFBO.cpp @@ -117,7 +117,8 @@ bool RenderTextureImplFBO::create(unsigned int width, unsigned int height, unsig glCheck(GLEXT_glFramebufferTexture2D(GLEXT_GL_FRAMEBUFFER, GLEXT_GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textureId, 0)); // A final check, just to be sure... - GLenum status = glCheck(GLEXT_glCheckFramebufferStatus(GLEXT_GL_FRAMEBUFFER)); + GLenum status; + glCheck(status = GLEXT_glCheckFramebufferStatus(GLEXT_GL_FRAMEBUFFER)); if (status != GLEXT_GL_FRAMEBUFFER_COMPLETE) { glCheck(GLEXT_glBindFramebuffer(GLEXT_GL_FRAMEBUFFER, 0)); diff --git a/src/SFML/Graphics/Shader.cpp b/src/SFML/Graphics/Shader.cpp index 5d87d10de..ff6a6ffe5 100644 --- a/src/SFML/Graphics/Shader.cpp +++ b/src/SFML/Graphics/Shader.cpp @@ -275,7 +275,8 @@ void Shader::setParameter(const std::string& name, float x) ensureGlContext(); // Enable program - GLEXT_GLhandle program = glCheck(GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); + GLEXT_GLhandle program; + glCheck(program = GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); glCheck(GLEXT_glUseProgramObject(castToGlHandle(m_shaderProgram))); // Get parameter location and assign it new values @@ -299,7 +300,8 @@ void Shader::setParameter(const std::string& name, float x, float y) ensureGlContext(); // Enable program - GLEXT_GLhandle program = glCheck(GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); + GLEXT_GLhandle program; + glCheck(program = GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); glCheck(GLEXT_glUseProgramObject(castToGlHandle(m_shaderProgram))); // Get parameter location and assign it new values @@ -323,7 +325,8 @@ void Shader::setParameter(const std::string& name, float x, float y, float z) ensureGlContext(); // Enable program - GLEXT_GLhandle program = glCheck(GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); + GLEXT_GLhandle program; + glCheck(program = GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); glCheck(GLEXT_glUseProgramObject(castToGlHandle(m_shaderProgram))); // Get parameter location and assign it new values @@ -347,7 +350,8 @@ void Shader::setParameter(const std::string& name, float x, float y, float z, fl ensureGlContext(); // Enable program - GLEXT_GLhandle program = glCheck(GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); + GLEXT_GLhandle program; + glCheck(program = GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); glCheck(GLEXT_glUseProgramObject(castToGlHandle(m_shaderProgram))); // Get parameter location and assign it new values @@ -392,7 +396,8 @@ void Shader::setParameter(const std::string& name, const Transform& transform) ensureGlContext(); // Enable program - GLEXT_GLhandle program = glCheck(GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); + GLEXT_GLhandle program; + glCheck(program = GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); glCheck(GLEXT_glUseProgramObject(castToGlHandle(m_shaderProgram))); // Get parameter location and assign it new values @@ -534,13 +539,15 @@ bool Shader::compile(const char* vertexShaderCode, const char* fragmentShaderCod m_params.clear(); // Create the program - GLEXT_GLhandle shaderProgram = glCheck(GLEXT_glCreateProgramObject()); + GLEXT_GLhandle shaderProgram; + glCheck(shaderProgram = GLEXT_glCreateProgramObject()); // Create the vertex shader if needed if (vertexShaderCode) { // Create and compile the shader - GLEXT_GLhandle vertexShader = glCheck(GLEXT_glCreateShaderObject(GLEXT_GL_VERTEX_SHADER)); + GLEXT_GLhandle vertexShader; + glCheck(vertexShader = GLEXT_glCreateShaderObject(GLEXT_GL_VERTEX_SHADER)); glCheck(GLEXT_glShaderSource(vertexShader, 1, &vertexShaderCode, NULL)); glCheck(GLEXT_glCompileShader(vertexShader)); @@ -567,7 +574,8 @@ bool Shader::compile(const char* vertexShaderCode, const char* fragmentShaderCod if (fragmentShaderCode) { // Create and compile the shader - GLEXT_GLhandle fragmentShader = glCheck(GLEXT_glCreateShaderObject(GLEXT_GL_FRAGMENT_SHADER)); + GLEXT_GLhandle fragmentShader; + glCheck(fragmentShader = GLEXT_glCreateShaderObject(GLEXT_GL_FRAGMENT_SHADER)); glCheck(GLEXT_glShaderSource(fragmentShader, 1, &fragmentShaderCode, NULL)); glCheck(GLEXT_glCompileShader(fragmentShader));