From 0d33ddeccff9e46fe7ed6b04fde782d20d3e6ea3 Mon Sep 17 00:00:00 2001 From: ZXShady <153229951+ZXShady@users.noreply.github.com> Date: Fri, 20 Sep 2024 08:34:22 +0100 Subject: [PATCH] Change glCheck macro to be usable as an expression --- src/SFML/Graphics/GLCheck.hpp | 51 ++++++++++++---------- src/SFML/Graphics/RenderTextureImplFBO.cpp | 7 +-- src/SFML/Graphics/Shader.cpp | 19 ++++---- src/SFML/Graphics/Texture.cpp | 6 +-- src/SFML/Graphics/VertexBuffer.cpp | 12 ++--- src/SFML/Window/EGLCheck.cpp | 2 +- src/SFML/Window/EGLCheck.hpp | 48 ++++++++++++-------- 7 files changed, 75 insertions(+), 70 deletions(-) diff --git a/src/SFML/Graphics/GLCheck.hpp b/src/SFML/Graphics/GLCheck.hpp index fb2799120c..023ac63166 100644 --- a/src/SFML/Graphics/GLCheck.hpp +++ b/src/SFML/Graphics/GLCheck.hpp @@ -31,31 +31,10 @@ #include #include - +#include namespace sf::priv { -//////////////////////////////////////////////////////////// -/// Let's define a macro to quickly check every OpenGL API call -//////////////////////////////////////////////////////////// -#ifdef SFML_DEBUG - -// In debug mode, perform a test on every OpenGL call -// 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 - -// Else, we don't add any overhead -#define glCheck(expr) (expr) - -#endif - //////////////////////////////////////////////////////////// /// \brief Check the last OpenGL error /// @@ -68,4 +47,32 @@ namespace sf::priv //////////////////////////////////////////////////////////// bool glCheckError(const std::filesystem::path& file, unsigned int line, std::string_view expression); +//////////////////////////////////////////////////////////// +/// Let's define a macro to quickly check every OpenGL API call +//////////////////////////////////////////////////////////// +#ifdef SFML_DEBUG +// In debug mode, perform a test on every OpenGL call +// The lamdba allows us to call glCheck as an expression and acts as a single statement perfect for if/else statements +#define glCheck(...) \ + [](auto&& glCheckInternalFunction) \ + { \ + if constexpr (!std::is_void_v) \ + { \ + const auto glCheckInternalReturnValue = glCheckInternalFunction(); \ + sf::priv::glCheckError(__FILE__, static_cast(__LINE__), #__VA_ARGS__); \ + return glCheckInternalReturnValue; \ + } \ + else \ + { \ + glCheckInternalFunction(); \ + sf::priv::glCheckError(__FILE__, static_cast(__LINE__), #__VA_ARGS__); \ + } \ + }([&]() { return __VA_ARGS__; }) +#else + +// Else, we don't add any overhead +#define glCheck(...) (__VA_ARGS__) + +#endif + } // namespace sf::priv diff --git a/src/SFML/Graphics/RenderTextureImplFBO.cpp b/src/SFML/Graphics/RenderTextureImplFBO.cpp index f089689071..ed6e842eef 100644 --- a/src/SFML/Graphics/RenderTextureImplFBO.cpp +++ b/src/SFML/Graphics/RenderTextureImplFBO.cpp @@ -430,9 +430,7 @@ bool RenderTextureImplFBO::createFrameBuffer() glCheck(GLEXT_glFramebufferTexture2D(GLEXT_GL_FRAMEBUFFER, GLEXT_GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, m_textureId, 0)); // A final check, just to be sure... - GLenum status = 0; - glCheck(status = GLEXT_glCheckFramebufferStatus(GLEXT_GL_FRAMEBUFFER)); - if (status != GLEXT_GL_FRAMEBUFFER_COMPLETE) + if (glCheck(GLEXT_glCheckFramebufferStatus(GLEXT_GL_FRAMEBUFFER)) != GLEXT_GL_FRAMEBUFFER_COMPLETE) { glCheck(GLEXT_glBindFramebuffer(GLEXT_GL_FRAMEBUFFER, 0)); err() << "Impossible to create render texture (failed to link the target texture to the frame buffer)" << std::endl; @@ -486,8 +484,7 @@ bool RenderTextureImplFBO::createFrameBuffer() } // A final check, just to be sure... - glCheck(status = GLEXT_glCheckFramebufferStatus(GLEXT_GL_FRAMEBUFFER)); - if (status != GLEXT_GL_FRAMEBUFFER_COMPLETE) + if (glCheck(GLEXT_glCheckFramebufferStatus(GLEXT_GL_FRAMEBUFFER)) != GLEXT_GL_FRAMEBUFFER_COMPLETE) { glCheck(GLEXT_glBindFramebuffer(GLEXT_GL_FRAMEBUFFER, 0)); err() << "Impossible to create render texture (failed to link the render buffers to the multisample frame " diff --git a/src/SFML/Graphics/Shader.cpp b/src/SFML/Graphics/Shader.cpp index 5950c524a9..9367f190d7 100644 --- a/src/SFML/Graphics/Shader.cpp +++ b/src/SFML/Graphics/Shader.cpp @@ -185,7 +185,7 @@ struct Shader::UniformBinder if (currentProgram) { // Enable program object - glCheck(savedProgram = GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); + savedProgram = glCheck(GLEXT_glGetHandle(GLEXT_GL_PROGRAM_OBJECT)); if (currentProgram != savedProgram) glCheck(GLEXT_glUseProgramObject(currentProgram)); @@ -865,17 +865,15 @@ bool Shader::compile(std::string_view vertexShaderCode, std::string_view geometr } // Create the program - GLEXT_GLhandle shaderProgram{}; - glCheck(shaderProgram = GLEXT_glCreateProgramObject()); + const GLEXT_GLhandle shaderProgram = glCheck(GLEXT_glCreateProgramObject()); // Create the vertex shader if needed if (!vertexShaderCode.empty()) { // Create and compile the shader - GLEXT_GLhandle vertexShader{}; - glCheck(vertexShader = GLEXT_glCreateShaderObject(GLEXT_GL_VERTEX_SHADER)); - const GLcharARB* sourceCode = vertexShaderCode.data(); - const auto sourceCodeLength = static_cast(vertexShaderCode.length()); + const GLEXT_GLhandle vertexShader = glCheck(GLEXT_glCreateShaderObject(GLEXT_GL_VERTEX_SHADER)); + const GLcharARB* sourceCode = vertexShaderCode.data(); + const auto sourceCodeLength = static_cast(vertexShaderCode.length()); glCheck(GLEXT_glShaderSource(vertexShader, 1, &sourceCode, &sourceCodeLength)); glCheck(GLEXT_glCompileShader(vertexShader)); @@ -929,10 +927,9 @@ bool Shader::compile(std::string_view vertexShaderCode, std::string_view geometr if (!fragmentShaderCode.empty()) { // Create and compile the shader - GLEXT_GLhandle fragmentShader{}; - glCheck(fragmentShader = GLEXT_glCreateShaderObject(GLEXT_GL_FRAGMENT_SHADER)); - const GLcharARB* sourceCode = fragmentShaderCode.data(); - const auto sourceCodeLength = static_cast(fragmentShaderCode.length()); + const GLEXT_GLhandle fragmentShader = glCheck(GLEXT_glCreateShaderObject(GLEXT_GL_FRAGMENT_SHADER)); + const GLcharARB* sourceCode = fragmentShaderCode.data(); + const auto sourceCodeLength = static_cast(fragmentShaderCode.length()); glCheck(GLEXT_glShaderSource(fragmentShader, 1, &sourceCode, &sourceCodeLength)); glCheck(GLEXT_glCompileShader(fragmentShader)); diff --git a/src/SFML/Graphics/Texture.cpp b/src/SFML/Graphics/Texture.cpp index 47bef635a3..9fafb1dd92 100644 --- a/src/SFML/Graphics/Texture.cpp +++ b/src/SFML/Graphics/Texture.cpp @@ -648,11 +648,9 @@ void Texture::update(const Texture& texture, Vector2u dest) GLEXT_glFramebufferTexture2D(GLEXT_GL_DRAW_FRAMEBUFFER, GLEXT_GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, m_texture, 0)); // A final check, just to be sure... - GLenum sourceStatus = 0; - glCheck(sourceStatus = GLEXT_glCheckFramebufferStatus(GLEXT_GL_READ_FRAMEBUFFER)); + const GLenum sourceStatus = glCheck(GLEXT_glCheckFramebufferStatus(GLEXT_GL_READ_FRAMEBUFFER)); - GLenum destStatus = 0; - glCheck(destStatus = GLEXT_glCheckFramebufferStatus(GLEXT_GL_DRAW_FRAMEBUFFER)); + const GLenum destStatus = glCheck(GLEXT_glCheckFramebufferStatus(GLEXT_GL_DRAW_FRAMEBUFFER)); if ((sourceStatus == GLEXT_GL_FRAMEBUFFER_COMPLETE) && (destStatus == GLEXT_GL_FRAMEBUFFER_COMPLETE)) { diff --git a/src/SFML/Graphics/VertexBuffer.cpp b/src/SFML/Graphics/VertexBuffer.cpp index f0ee9b8e7b..a83b4bdf60 100644 --- a/src/SFML/Graphics/VertexBuffer.cpp +++ b/src/SFML/Graphics/VertexBuffer.cpp @@ -236,23 +236,19 @@ bool VertexBuffer::update([[maybe_unused]] const VertexBuffer& vertexBuffer) nullptr, VertexBufferImpl::usageToGlEnum(m_usage))); - void* destination = nullptr; - glCheck(destination = GLEXT_glMapBuffer(GLEXT_GL_ARRAY_BUFFER, GLEXT_GL_WRITE_ONLY)); + void* const destination = glCheck(GLEXT_glMapBuffer(GLEXT_GL_ARRAY_BUFFER, GLEXT_GL_WRITE_ONLY)); glCheck(GLEXT_glBindBuffer(GLEXT_GL_ARRAY_BUFFER, vertexBuffer.m_buffer)); - void* source = nullptr; - glCheck(source = GLEXT_glMapBuffer(GLEXT_GL_ARRAY_BUFFER, GLEXT_GL_READ_ONLY)); + const void* const source = glCheck(GLEXT_glMapBuffer(GLEXT_GL_ARRAY_BUFFER, GLEXT_GL_READ_ONLY)); std::memcpy(destination, source, sizeof(Vertex) * vertexBuffer.m_size); - GLboolean sourceResult = GL_FALSE; - glCheck(sourceResult = GLEXT_glUnmapBuffer(GLEXT_GL_ARRAY_BUFFER)); + const GLboolean sourceResult = glCheck(GLEXT_glUnmapBuffer(GLEXT_GL_ARRAY_BUFFER)); glCheck(GLEXT_glBindBuffer(GLEXT_GL_ARRAY_BUFFER, m_buffer)); - GLboolean destinationResult = GL_FALSE; - glCheck(destinationResult = GLEXT_glUnmapBuffer(GLEXT_GL_ARRAY_BUFFER)); + const GLboolean destinationResult = glCheck(GLEXT_glUnmapBuffer(GLEXT_GL_ARRAY_BUFFER)); glCheck(GLEXT_glBindBuffer(GLEXT_GL_ARRAY_BUFFER, 0)); diff --git a/src/SFML/Window/EGLCheck.cpp b/src/SFML/Window/EGLCheck.cpp index 26c54475f7..129578cc83 100644 --- a/src/SFML/Window/EGLCheck.cpp +++ b/src/SFML/Window/EGLCheck.cpp @@ -42,7 +42,7 @@ bool eglCheckError(const std::filesystem::path& file, unsigned int line, std::st { const auto logError = [&](const char* error, const char* description) { - err() << "An internal EGL call failed in " << file.filename() << " (" << line << ") : " + err() << "An internal EGL call failed in " << file.filename() << "(" << line << ")." << "\nExpression:\n " << expression << "\nError description:\n " << error << "\n " << description << '\n' << std::endl; diff --git a/src/SFML/Window/EGLCheck.hpp b/src/SFML/Window/EGLCheck.hpp index 68e4f07224..9376b38e64 100644 --- a/src/SFML/Window/EGLCheck.hpp +++ b/src/SFML/Window/EGLCheck.hpp @@ -35,37 +35,47 @@ namespace sf::priv { +//////////////////////////////////////////////////////////// +/// \brief Check the last EGL error +/// +/// \param file Source file where the call is located +/// \param line Line number of the source file where the call is located +/// \param expression The evaluated expression as a string +/// +/// \return `false` if an error occurred, `true` otherwise +/// +//////////////////////////////////////////////////////////// +bool eglCheckError(const std::filesystem::path& file, unsigned int line, std::string_view expression); + //////////////////////////////////////////////////////////// /// Let's define a macro to quickly check every EGL API call //////////////////////////////////////////////////////////// #ifdef SFML_DEBUG // In debug mode, perform a test on every EGL call -// The do-while loop is needed so that glCheck can be used as a single statement in if/else branches -#define eglCheck(expr) \ - do \ - { \ - expr; \ - sf::priv::eglCheckError(__FILE__, __LINE__, #expr); \ - } while (false) +// The lamdba allows us to call eglCheck as an expression and acts as a single statement perfect for if/else statements +#define eglCheck(...) \ + [](auto&& eglCheckInternalFunction) \ + { \ + if constexpr (!std::is_void_v) \ + { \ + const auto eglCheckInternalReturnValue = eglCheckInternalFunction(); \ + sf::priv::eglCheckError(__FILE__, static_cast(__LINE__), #__VA_ARGS__); \ + return eglCheckInternalReturnValue; \ + } \ + else \ + { \ + eglCheckInternalFunction(); \ + sf::priv::eglCheckError(__FILE__, static_cast(__LINE__), #__VA_ARGS__); \ + } \ + }([&]() { return __VA_ARGS__; }) #else // Else, we don't add any overhead -#define eglCheck(x) (x) +#define eglCheck(...) (__VA_ARGS__) #endif -//////////////////////////////////////////////////////////// -/// \brief Check the last EGL error -/// -/// \param file Source file where the call is located -/// \param line Line number of the source file where the call is located -/// \param expression The evaluated expression as a string -/// -/// \return `false` if an error occurred, `true` otherwise -/// -//////////////////////////////////////////////////////////// -bool eglCheckError(const std::filesystem::path& file, unsigned int line, std::string_view expression); } // namespace sf::priv