From c161e5f5d5a6277d7c71f229ba66bc5b458a19da Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 11 Nov 2024 15:30:25 -0800 Subject: [PATCH 1/2] [Impeller] Fix issues in the debug printers for framebuffer attachments. * CheckFramebufferStatus needs to be called with the enum for the target type and not the target itself. * Fix dumping the framebuffer type and object name. --- .../renderer/backend/gles/proc_table_gles.cc | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/impeller/renderer/backend/gles/proc_table_gles.cc b/impeller/renderer/backend/gles/proc_table_gles.cc index 0ae89339425ef..3e9d7d70e7b43 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.cc +++ b/impeller/renderer/backend/gles/proc_table_gles.cc @@ -9,6 +9,7 @@ #include "fml/closure.h" #include "impeller/base/allocation.h" #include "impeller/base/comparable.h" +#include "impeller/base/strings.h" #include "impeller/base/validation.h" #include "impeller/renderer/backend/gles/capabilities_gles.h" #include "impeller/renderer/capabilities.h" @@ -241,24 +242,24 @@ static const char* AttachmentTypeString(GLint type) { static std::string DescribeFramebufferAttachment(const ProcTableGLES& gl, GLenum attachment) { - GLint param = GL_NONE; + GLint type = GL_NONE; gl.GetFramebufferAttachmentParameteriv( GL_FRAMEBUFFER, // target attachment, // attachment GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE, // parameter name - ¶m // parameter + &type // parameter ); - if (param != GL_NONE) { - param = GL_NONE; + if (type != GL_NONE) { + GLint object = GL_NONE; gl.GetFramebufferAttachmentParameteriv( GL_FRAMEBUFFER, // target attachment, // attachment GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, // parameter name - ¶m // parameter + &object // parameter ); std::stringstream stream; - stream << AttachmentTypeString(param) << "(" << param << ")"; + stream << AttachmentTypeString(type) << "(" << object << ")"; return stream.str(); } @@ -268,11 +269,15 @@ static std::string DescribeFramebufferAttachment(const ProcTableGLES& gl, std::string ProcTableGLES::DescribeCurrentFramebuffer() const { GLint framebuffer = GL_NONE; GetIntegerv(GL_FRAMEBUFFER_BINDING, &framebuffer); + if (framebuffer == GL_NONE) { + return "The default framebuffer (FBO0) was bound."; + } if (IsFramebuffer(framebuffer) == GL_FALSE) { - return "No framebuffer or the default window framebuffer is bound."; + return SPrintF("The framebuffer binding (%d) was not a valid framebuffer.", + framebuffer); } - GLenum status = CheckFramebufferStatus(framebuffer); + GLenum status = CheckFramebufferStatus(GL_FRAMEBUFFER); std::stringstream stream; stream << "FBO " << ((framebuffer == GL_NONE) ? "(Default)" @@ -287,10 +292,10 @@ std::string ProcTableGLES::DescribeCurrentFramebuffer() const { stream << "Color Attachment: " << DescribeFramebufferAttachment(*this, GL_COLOR_ATTACHMENT0) << std::endl; - stream << "Color Attachment: " + stream << "Depth Attachment: " << DescribeFramebufferAttachment(*this, GL_DEPTH_ATTACHMENT) << std::endl; - stream << "Color Attachment: " + stream << "Stencil Attachment: " << DescribeFramebufferAttachment(*this, GL_STENCIL_ATTACHMENT) << std::endl; return stream.str(); @@ -303,7 +308,7 @@ bool ProcTableGLES::IsCurrentFramebufferComplete() const { // The default framebuffer is always complete. return true; } - GLenum status = CheckFramebufferStatus(framebuffer); + GLenum status = CheckFramebufferStatus(GL_FRAMEBUFFER); return status == GL_FRAMEBUFFER_COMPLETE; } From ffb4f1360bf58252f1d2123c9c71a664a5dee418 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 11 Nov 2024 16:10:04 -0800 Subject: [PATCH 2/2] Allow dumping GL calls to log. --- .../renderer/backend/gles/proc_table_gles.h | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/impeller/renderer/backend/gles/proc_table_gles.h b/impeller/renderer/backend/gles/proc_table_gles.h index 14086784dc76d..eadcb4f333057 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.h +++ b/impeller/renderer/backend/gles/proc_table_gles.h @@ -45,6 +45,31 @@ struct AutoErrorCheck { } }; +template +void BuildGLArgumentsStream(std::stringstream& stream, Type arg) { + stream << arg; +} + +constexpr void BuildGLArgumentsStream(std::stringstream& stream) {} + +template +void BuildGLArgumentsStream(std::stringstream& stream, + Type arg, + Rest... other_args) { + BuildGLArgumentsStream(stream, arg); + stream << ", "; + BuildGLArgumentsStream(stream, other_args...); +} + +template +[[nodiscard]] std::string BuildGLArguments(Type... args) { + std::stringstream stream; + stream << "("; + BuildGLArgumentsStream(stream, args...); + stream << ")"; + return stream.str(); +} + template struct GLProc { using GLFunctionType = T; @@ -66,6 +91,14 @@ struct GLProc { /// PFNGLGETERRORPROC error_fn = nullptr; + //---------------------------------------------------------------------------- + /// Whether the OpenGL call and its arguments should be logged. + /// + /// Only works in IMPELLER_DEBUG and for environments where traditional + /// tracing is hard. Expect log spam and only use during development. + /// + bool log_calls = false; + //---------------------------------------------------------------------------- /// @brief Call the GL function with the appropriate parameters. Lookup /// the documentation for the GL function being called to @@ -81,6 +114,10 @@ struct GLProc { // validation log will at least give us a hint as to what's going on. FML_CHECK(IsAvailable()) << "GL function " << name << " is not available. " << "This is likely due to a missing extension."; + if (log_calls) { + FML_LOG(IMPORTANT) << name + << BuildGLArguments(std::forward(args)...); + } #endif // defined(IMPELLER_DEBUG) && !defined(NDEBUG) return function(std::forward(args)...); }