diff --git a/impeller/renderer/backend/gles/proc_table_gles.cc b/impeller/renderer/backend/gles/proc_table_gles.cc index ee4af806d244e..2307f5325d990 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.cc +++ b/impeller/renderer/backend/gles/proc_table_gles.cc @@ -83,17 +83,11 @@ ProcTableGLES::ProcTableGLES( // NOLINT(google-readability-function-size) resolver = WrappedResolver(resolver); - auto error_fn = reinterpret_cast(resolver("glGetError")); - if (!error_fn) { - VALIDATION_LOG << "Could not resolve " << "glGetError"; - return; - } - #define IMPELLER_PROC(proc_ivar) \ if (auto fn_ptr = resolver(proc_ivar.name)) { \ proc_ivar.function = \ reinterpret_cast(fn_ptr); \ - proc_ivar.error_fn = error_fn; \ + RegisterProc(&proc_ivar); \ } else { \ VALIDATION_LOG << "Could not resolve " << proc_ivar.name; \ return; \ @@ -119,7 +113,7 @@ ProcTableGLES::ProcTableGLES( // NOLINT(google-readability-function-size) if (auto fn_ptr = resolver(proc_ivar.name)) { \ proc_ivar.function = \ reinterpret_cast(fn_ptr); \ - proc_ivar.error_fn = error_fn; \ + RegisterProc(&proc_ivar); \ } FOR_EACH_IMPELLER_GLES3_PROC(IMPELLER_PROC); FOR_EACH_IMPELLER_EXT_PROC(IMPELLER_PROC); @@ -144,6 +138,10 @@ ProcTableGLES::ProcTableGLES( // NOLINT(google-readability-function-size) // builds to identify threading violations in the engine. UseProgram.enforce_one_thread = true; +#ifndef NDEBUG + SetDebugGLErrorChecking(true); +#endif // NDEBUG + is_valid_ = true; } @@ -431,4 +429,62 @@ std::string ProcTableGLES::GetProgramInfoLogString(GLuint program) const { static_cast(length)}; } +void ProcTableGLES::RegisterProc(GLProcBase* proc) { + if (proc && proc->name) { + debug_known_procs_[proc->name] = proc; + } +} + +void ProcTableGLES::IterateDebugProcs( + const std::function& iterator) const { + if (!iterator) { + return; + } + for (const auto& proc : debug_known_procs_) { + if (!iterator(proc.second)) { + return; + } + } +} + +void ProcTableGLES::SetDebugGLCallLogging(bool log) const { + IterateDebugProcs([log](GLProcBase* proc) -> bool { + proc->log_calls = log; + return true; + }); +} + +void ProcTableGLES::SetDebugGLCallLogging(bool log, + const char* function_name) const { + IterateDebugProcs([log, function_name](GLProcBase* proc) -> bool { + if (proc->name == function_name) { + proc->log_calls = log; + return false; + } + return true; + }); +} + +void ProcTableGLES::SetDebugGLErrorChecking(bool check) const { + FML_DCHECK(GetError.function != nullptr); + PFNGLGETERRORPROC error_fn = check ? GetError.function : nullptr; + IterateDebugProcs([error_fn](GLProcBase* proc) -> bool { + proc->error_fn = error_fn; + return true; + }); +} + +void ProcTableGLES::SetDebugGLErrorChecking(bool check, + const char* function_name) const { + FML_DCHECK(GetError.function != nullptr); + PFNGLGETERRORPROC error_fn = check ? GetError.function : nullptr; + IterateDebugProcs([error_fn, function_name](GLProcBase* proc) -> bool { + if (proc->name == function_name) { + proc->error_fn = error_fn; + return false; + } + return true; + }); +} + } // namespace impeller diff --git a/impeller/renderer/backend/gles/proc_table_gles.h b/impeller/renderer/backend/gles/proc_table_gles.h index b24523afa160d..71a0f3bc7cb58 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.h +++ b/impeller/renderer/backend/gles/proc_table_gles.h @@ -6,6 +6,7 @@ #define FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_PROC_TABLE_GLES_H_ #include +#include #include #include #include @@ -72,21 +73,13 @@ template return stream.str(); } -template -struct GLProc { - using GLFunctionType = T; - +struct GLProcBase { //---------------------------------------------------------------------------- /// The name of the GL function. /// // TODO(135922) Change to string_view. const char* name = nullptr; - //---------------------------------------------------------------------------- - /// The pointer to the GL function. - /// - GLFunctionType* function = nullptr; - //---------------------------------------------------------------------------- /// An optional error function. If present, all calls will be followed by an /// error check. @@ -109,6 +102,20 @@ struct GLProc { /// thread. bool enforce_one_thread = false; + explicit GLProcBase(const char* p_name) : name(p_name) {} +}; + +template +struct GLProc : public GLProcBase { + using GLFunctionType = T; + + explicit GLProc(const char* p_name) : GLProcBase(p_name) {} + + //---------------------------------------------------------------------------- + /// The pointer to the GL function. + /// + GLFunctionType* function = nullptr; + //---------------------------------------------------------------------------- /// @brief Call the GL function with the appropriate parameters. Lookup /// the documentation for the GL function being called to @@ -117,7 +124,7 @@ struct GLProc { /// template auto operator()(Args&&... args) const { -#if defined(IMPELLER_DEBUG) && !defined(NDEBUG) +#if defined(IMPELLER_DEBUG) AutoErrorCheck error(error_fn, name); // We check for the existence of extensions, and reset the function pointer // but it's still called unconditionally below, and will segfault. This @@ -138,7 +145,7 @@ struct GLProc { "thread. As of this addition, the design of the engine should be " "using non-raster threads only for uploading images."; } -#endif // defined(IMPELLER_DEBUG) && !defined(NDEBUG) +#endif // defined(IMPELLER_DEBUG) return function(std::forward(args)...); } @@ -201,6 +208,7 @@ struct GLProc { PROC(GenVertexArrays); \ PROC(GetActiveUniform); \ PROC(GetBooleanv); \ + PROC(GetError); \ PROC(GetFloatv); \ PROC(GetFramebufferAttachmentParameteriv); \ PROC(GetIntegerv); \ @@ -208,6 +216,7 @@ struct GLProc { PROC(GetProgramiv); \ PROC(GetShaderInfoLog); \ PROC(GetShaderiv); \ + PROC(GetShaderSource); \ PROC(GetString); \ PROC(GetStringi); \ PROC(GetUniformLocation); \ @@ -227,9 +236,9 @@ struct GLProc { PROC(StencilMaskSeparate); \ PROC(StencilOpSeparate); \ PROC(TexImage2D); \ - PROC(TexSubImage2D); \ - PROC(TexParameteri); \ PROC(TexParameterfv); \ + PROC(TexParameteri); \ + PROC(TexSubImage2D); \ PROC(Uniform1fv); \ PROC(Uniform1i); \ PROC(Uniform2fv); \ @@ -239,7 +248,6 @@ struct GLProc { PROC(UseProgram); \ PROC(VertexAttribPointer); \ PROC(Viewport); \ - PROC(GetShaderSource); \ PROC(ReadPixels); // Calls specific to OpenGLES. @@ -293,7 +301,7 @@ class ProcTableGLES { ~ProcTableGLES(); #define IMPELLER_PROC(name) \ - GLProc name = {"gl" #name, nullptr}; + GLProc name = GLProc{"gl" #name}; FOR_EACH_IMPELLER_PROC(IMPELLER_PROC); FOR_EACH_IMPELLER_ES_ONLY_PROC(IMPELLER_PROC); @@ -332,6 +340,55 @@ class ProcTableGLES { void PopDebugGroup() const; + //---------------------------------------------------------------------------- + /// @brief Set if all OpenGL function calls in this proc table log their + /// invocation and arguments. + /// + /// Example: + /// ``` + /// glDepthMask(1) + /// glViewport(0, 0, 2048, 1536) + /// glDepthRangef(0, 1) + /// glDisable(2884) + /// glFrontFace(2304) + /// ``` + /// + /// @warning Call logging is only available in IMPELLER_DEBUG builds. + /// + /// @param[in] log If logging should be enabled. + /// + void SetDebugGLCallLogging(bool log) const; + + //---------------------------------------------------------------------------- + /// @brief Set if the a specific OpenGL function call logs its invocation + /// and arguments. + /// + /// @warning Call logging is only available in IMPELLER_DEBUG builds. + /// + /// @param[in] log If logging should be enabled. + /// + void SetDebugGLCallLogging(bool log, const char* function_name) const; + + //---------------------------------------------------------------------------- + /// @brief Set if glGetError is called and trapped on all OpenGL function + /// calls in this proc table. + /// + /// @warning GL error checking is only available in IMPELLER_DEBUG builds. + /// + /// @param[in] check If error checking should be performed. + /// + void SetDebugGLErrorChecking(bool check) const; + + //---------------------------------------------------------------------------- + /// @brief Set if glGetError is called and trapped on a specific OpenGL + /// function in this proc table. + /// + /// @warning GL error checking is only available in IMPELLER_DEBUG builds. + /// + /// @param[in] check If error checking should be performed. + /// + void SetDebugGLErrorChecking(bool check, const char* function_name) const; + // Visible For testing. std::optional ComputeShaderWithDefines( const fml::Mapping& mapping, @@ -342,10 +399,16 @@ class ProcTableGLES { std::unique_ptr description_; std::shared_ptr capabilities_; GLint debug_label_max_length_ = 0; + std::map debug_known_procs_; ProcTableGLES(const ProcTableGLES&) = delete; ProcTableGLES& operator=(const ProcTableGLES&) = delete; + + void IterateDebugProcs( + const std::function& iterator) const; + + void RegisterProc(GLProcBase* proc); }; } // namespace impeller diff --git a/impeller/renderer/backend/gles/test/proc_table_gles_unittests.cc b/impeller/renderer/backend/gles/test/proc_table_gles_unittests.cc index b7f188f950299..b542c3f757c48 100644 --- a/impeller/renderer/backend/gles/test/proc_table_gles_unittests.cc +++ b/impeller/renderer/backend/gles/test/proc_table_gles_unittests.cc @@ -33,5 +33,39 @@ TEST(ProcTableGLES, ResolvesCorrectClearDepthProcOnDesktopGL) { FOR_EACH_IMPELLER_ES_ONLY_PROC(EXPECT_UNAVAILABLE); } +TEST(ProcTableGLES, DebuggingOptionsAreRespected) { +#ifndef IMPELLER_DEBUG + if ((true)) { + GTEST_SKIP() << "Debugging is only available in IMPELLER_DEBUG"; + } +#endif // IMPELLER_DEBUG + auto mock_gles = MockGLES::Init(std::nullopt, "OpenGL ES 3.0"); + auto& gl = mock_gles->GetProcTable(); + // Check call logging. + { + gl.SetDebugGLCallLogging(false); + ASSERT_FALSE(gl.Clear.log_calls); + gl.SetDebugGLCallLogging(true); + ASSERT_TRUE(gl.Clear.log_calls); + gl.SetDebugGLCallLogging(false); + ASSERT_FALSE(gl.Clear.log_calls); + gl.SetDebugGLCallLogging(true, "glClear"); + ASSERT_TRUE(gl.Clear.log_calls); + gl.SetDebugGLCallLogging(false, "glClear"); + ASSERT_FALSE(gl.Clear.log_calls); + } + // Check error checking. + { + gl.SetDebugGLErrorChecking(true); + ASSERT_NE(gl.Clear.error_fn, nullptr); + gl.SetDebugGLErrorChecking(false); + ASSERT_EQ(gl.Clear.error_fn, nullptr); + gl.SetDebugGLErrorChecking(true, "glClear"); + ASSERT_NE(gl.Clear.error_fn, nullptr); + gl.SetDebugGLErrorChecking(false, "glClear"); + ASSERT_EQ(gl.Clear.error_fn, nullptr); + } +} + } // namespace testing } // namespace impeller diff --git a/impeller/toolkit/glvk/proc_table.h b/impeller/toolkit/glvk/proc_table.h index 9a2b8035c1af5..4e46b914a22bc 100644 --- a/impeller/toolkit/glvk/proc_table.h +++ b/impeller/toolkit/glvk/proc_table.h @@ -87,7 +87,8 @@ class ProcTable { /// bool IsValid() const; -#define GLVK_PROC(name) GLProc name = {"gl" #name, nullptr}; +#define GLVK_PROC(name) \ + GLProc name = GLProc("gl" #name); FOR_EACH_GLVK_PROC(GLVK_PROC); diff --git a/impeller/toolkit/interop/context.cc b/impeller/toolkit/interop/context.cc index 15e32e25045d1..c75d20a3de336 100644 --- a/impeller/toolkit/interop/context.cc +++ b/impeller/toolkit/interop/context.cc @@ -108,4 +108,44 @@ AiksContext& Context::GetAiksContext() { return context_; } +void Context::SetDebugGLCallLogging(const char* function_name_or_null, + bool enable) { +#if IMPELLER_ENABLE_OPENGLES + if (context_.GetContext()->GetBackendType() != + impeller::Context::BackendType::kOpenGLES) { + VALIDATION_LOG << "Not an OpenGL context."; + return; + } + auto& gl = + ContextGLES::Cast(*context_.GetContext()).GetReactor()->GetProcTable(); + if (function_name_or_null == nullptr) { + gl.SetDebugGLCallLogging(enable); + } else { + gl.SetDebugGLCallLogging(enable, function_name_or_null); + } +#else // IMPELLER_ENABLE_OPENGLES + VALIDATION_LOG << "OpenGL unavailable." +#endif // IMPELLER_ENABLE_OPENGLES +} + +void Context::SetDebugGLErrorChecking(const char* function_name_or_null, + bool enable) { +#if IMPELLER_ENABLE_OPENGLES + if (context_.GetContext()->GetBackendType() != + impeller::Context::BackendType::kOpenGLES) { + VALIDATION_LOG << "Not an OpenGL context."; + return; + } + auto& gl = + ContextGLES::Cast(*context_.GetContext()).GetReactor()->GetProcTable(); + if (function_name_or_null == nullptr) { + gl.SetDebugGLErrorChecking(enable); + } else { + gl.SetDebugGLErrorChecking(enable, function_name_or_null); + } +#else // IMPELLER_ENABLE_OPENGLES + VALIDATION_LOG << "OpenGL unavailable." +#endif // IMPELLER_ENABLE_OPENGLES +} + } // namespace impeller::interop diff --git a/impeller/toolkit/interop/context.h b/impeller/toolkit/interop/context.h index 3002ca69945a1..61eba27a6ca5a 100644 --- a/impeller/toolkit/interop/context.h +++ b/impeller/toolkit/interop/context.h @@ -37,6 +37,10 @@ class Context final AiksContext& GetAiksContext(); + void SetDebugGLCallLogging(const char* function_name_or_null, bool enable); + + void SetDebugGLErrorChecking(const char* function_name_or_null, bool enable); + private: impeller::AiksContext context_; std::shared_ptr backend_data_; diff --git a/impeller/toolkit/interop/impeller.cc b/impeller/toolkit/interop/impeller.cc index 7681efe515f9c..f996cf7a2778c 100644 --- a/impeller/toolkit/interop/impeller.cc +++ b/impeller/toolkit/interop/impeller.cc @@ -1152,4 +1152,18 @@ bool ImpellerTypographyContextRegisterFont(ImpellerTypographyContext context, family_name_alias); } +IMPELLER_EXTERN_C +void ImpellerContextSetDebugGLCallLogging(ImpellerContext context, + const char* opengl_function_name, + bool enable) { + GetPeer(context)->SetDebugGLCallLogging(opengl_function_name, enable); +} + +IMPELLER_EXTERN_C +void ImpellerContextSetDebugGLErrorChecking(ImpellerContext context, + const char* opengl_function_name, + bool enable) { + GetPeer(context)->SetDebugGLErrorChecking(opengl_function_name, enable); +} + } // namespace impeller::interop diff --git a/impeller/toolkit/interop/impeller.h b/impeller/toolkit/interop/impeller.h index fe1b3bb98094f..97faed0c6b750 100644 --- a/impeller/toolkit/interop/impeller.h +++ b/impeller/toolkit/interop/impeller.h @@ -625,6 +625,71 @@ ImpellerContextCreateOpenGLESNew( ImpellerProcAddressCallback IMPELLER_NONNULL gl_proc_address_callback, void* IMPELLER_NULLABLE gl_proc_address_callback_user_data); +//------------------------------------------------------------------------------ +/// @brief Only available in instrumented builds with IMPELLER_DEBUG +/// enabled, setting GL call logging will log all OpenGL calls along +/// with their arguments. +/// +/// This becomes spammy and is only meant to be used as a debugging +/// aid in environments where traditional graphics debuggers like +/// RenderDoc are not available. +/// +/// If the function name is NULL, the option applies to all OpenGL +/// calls made by Impeller. +/// +/// Only calls made by Impeller will be logged. Impeller doesn't +/// know about OpenGL calls made by the embedder. +/// +/// Expect logging in the form of: +/// +/// ``` +/// glDepthMask(1) +/// glViewport(0, 0, 2048, 1536) +/// glDepthRangef(0, 1) +/// glDisable(2884) +/// glFrontFace(2304) +/// ``` +/// +/// This function does nothing if the context is not an OpenGL +/// context. +/// +/// @param[in] context The context +/// @param[in] opengl_function_name The opengl function name or NULL if the +/// option must be set for all OpenGL methods. +/// @param[in] enable If logging must be enabled. +/// +IMPELLER_EXPORT void ImpellerContextSetDebugGLCallLogging( + ImpellerContext IMPELLER_NONNULL context, + const char* IMPELLER_NULLABLE opengl_function_name, + bool enable); + +//------------------------------------------------------------------------------ +/// @brief Only available in instrumented builds with IMPELLER_DEBUG +/// enabled, setting GL error checking will trap on all OpenGL error +/// after OpenGL calls made by Impeller. +/// +/// There is appreciable overhead to this form of debugging and it +/// must only be used in environments where traditional graphics +/// debuggers like RenderDoc are not available. +/// +/// If the function name is NULL, the option applies to all OpenGL +/// calls made by Impeller. +/// +/// Only calls made by Impeller will be logged. Impeller doesn't +/// know about OpenGL calls made by the embedder. +/// +/// This function does nothing if the context is not an OpenGL +/// context. +/// +/// @param[in] context The context +/// @param[in] opengl_function_name The opengl function name +/// @param[in] enable The enable +/// +IMPELLER_EXPORT void ImpellerContextSetDebugGLErrorChecking( + ImpellerContext IMPELLER_NONNULL context, + const char* IMPELLER_NULLABLE opengl_function_name, + bool enable); + //------------------------------------------------------------------------------ /// @brief Retain a strong reference to the object. The object can be NULL /// in which case this method is a no-op.