From b6dfcba94b2f3dd649845c42b60bcbff251b91d8 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 3 Oct 2023 16:23:25 -0600 Subject: [PATCH] [Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` (flutter/engine#46433) Unblocks testing https://github.com/flutter/flutter/issues/135715. See `README.md` for details. I made one non-testing change to the proc_table itself, which is if a function call will fail, we will print out what is about to fail in the validation log. It was useful when debugging the test creation itself, and it's only enabled when GL call checks are enabled anyway. @gaaclarke I originally implemented it with `FML_THREAD_LOCAL`, but figured doing a global lock essentially was the same thing, and would prevent parallel test runs from stepping on each-other in weird ways? /cc @chinmaygarde for visibility. --- .../flutter/ci/licenses_golden/excluded_files | 1 + engine/src/flutter/impeller/BUILD.gn | 4 + .../impeller/renderer/backend/gles/BUILD.gn | 14 ++ .../renderer/backend/gles/proc_table_gles.h | 13 +- .../renderer/backend/gles/test/README.md | 48 ++++++ .../renderer/backend/gles/test/mock_gles.cc | 147 ++++++++++++++++++ .../renderer/backend/gles/test/mock_gles.h | 57 +++++++ .../backend/gles/test/mock_gles_unittests.cc | 48 ++++++ 8 files changed, 331 insertions(+), 1 deletion(-) create mode 100644 engine/src/flutter/impeller/renderer/backend/gles/test/README.md create mode 100644 engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc create mode 100644 engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h create mode 100644 engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles_unittests.cc diff --git a/engine/src/flutter/ci/licenses_golden/excluded_files b/engine/src/flutter/ci/licenses_golden/excluded_files index 44865d3cf14..e9dec480e9a 100644 --- a/engine/src/flutter/ci/licenses_golden/excluded_files +++ b/engine/src/flutter/ci/licenses_golden/excluded_files @@ -154,6 +154,7 @@ ../../../flutter/impeller/golden_tests_harvester/test ../../../flutter/impeller/image/README.md ../../../flutter/impeller/playground +../../../flutter/impeller/renderer/backend/gles/test ../../../flutter/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc diff --git a/engine/src/flutter/impeller/BUILD.gn b/engine/src/flutter/impeller/BUILD.gn index c89a89f7f34..9214c02f119 100644 --- a/engine/src/flutter/impeller/BUILD.gn +++ b/engine/src/flutter/impeller/BUILD.gn @@ -109,6 +109,10 @@ impeller_component("impeller_unittests") { deps += [ "//flutter/impeller/renderer/backend/vulkan:vulkan_unittests" ] } + if (impeller_enable_opengles) { + deps += [ "//flutter/impeller/renderer/backend/gles:gles_unittests" ] + } + if (glfw_vulkan_library != "") { deps += [ "//third_party/swiftshader", diff --git a/engine/src/flutter/impeller/renderer/backend/gles/BUILD.gn b/engine/src/flutter/impeller/renderer/backend/gles/BUILD.gn index 7141abc3304..39b112fcb92 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/BUILD.gn +++ b/engine/src/flutter/impeller/renderer/backend/gles/BUILD.gn @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//flutter/vulkan/config.gni") import("../../../tools/impeller.gni") config("gles_config") { @@ -10,6 +11,19 @@ config("gles_config") { include_dirs = [ "//third_party/angle/include" ] } +impeller_component("gles_unittests") { + testonly = true + sources = [ + "test/mock_gles.cc", + "test/mock_gles.h", + "test/mock_gles_unittests.cc", + ] + deps = [ + ":gles", + "//flutter/testing:testing_lib", + ] +} + impeller_component("gles") { public_configs = [] diff --git a/engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.h index e3f5e30b49c..831c909e7a6 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.h @@ -21,6 +21,9 @@ bool GLErrorIsFatal(GLenum value); struct AutoErrorCheck { const PFNGLGETERRORPROC error_fn; + + // TODO(matanlurey) Change to string_view. + // https://github.com/flutter/flutter/issues/135922 const char* name; AutoErrorCheck(PFNGLGETERRORPROC error, const char* name) @@ -49,6 +52,9 @@ template struct GLProc { using GLFunctionType = T; + // TODO(matanlurey) Change to string_view. + // https://github.com/flutter/flutter/issues/135922 + //---------------------------------------------------------------------------- /// The name of the GL function. /// @@ -75,6 +81,11 @@ struct GLProc { auto operator()(Args&&... args) const { #ifdef 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 + // 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."; #endif // IMPELLER_DEBUG #ifdef IMPELLER_TRACE_ALL_GL_CALLS TRACE_EVENT0("impeller", name); @@ -85,7 +96,6 @@ struct GLProc { constexpr bool IsAvailable() const { return function != nullptr; } void Reset() { - name = nullptr; function = nullptr; error_fn = nullptr; } @@ -200,6 +210,7 @@ class ProcTableGLES { public: using Resolver = std::function; explicit ProcTableGLES(Resolver resolver); + ProcTableGLES(ProcTableGLES&& other) = default; ~ProcTableGLES(); diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/README.md b/engine/src/flutter/impeller/renderer/backend/gles/test/README.md new file mode 100644 index 00000000000..dd9d028e1b8 --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/README.md @@ -0,0 +1,48 @@ +# `MockGLES` + +This directory contains a mock implementation of the GLES backend. + +Most functions are implemented as no-ops, have a default implementation that is not configurable, or just record the call. The latter is useful for testing: + +```cc +TEST(MockGLES, Example) { + // Creates a mock GLES implementation and sets it as the current one. + auto mock_gles = MockGLES::Init(); + auto& gl = mock_gles->GetProcTable(); + + // Call the proc table methods as usual, or pass the proc table to a class + // that needs it. + gl.PushDebugGroupKHR(GL_DEBUG_SOURCE_APPLICATION_KHR, 0, -1, "test"); + gl.PopDebugGroupKHR(); + + // Method names are recorded and can be inspected. + // + // Note that many built-ins, like glGetString, are not recorded (otherwise the // logs would be much bigger and less useful). + auto calls = mock_gles->GetCapturedCalls(); + EXPECT_EQ(calls, std::vector( + {"PushDebugGroupKHR", "PopDebugGroupKHR"})); +} +``` + +To add a new function, do the following: + +1. Add a new top-level method to [`mock_gles.cc`](mock_gles.cc): + + ```cc + void glFooBar() { + recordCall("glFooBar"); + } + ``` + +2. Edit the `kMockResolver`, and add a new `else if` clause: + + ```diff + + else if (strcmp(name, "glFooBar") == 0) { + + return reinterpret_cast(&glFooBar); + } else { + return reinterpret_cast(&glDoNothing); + } + ``` + +It's possible we'll want to add a more sophisticated mechanism for mocking +besides capturing calls, but this is a good start. PRs welcome! diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc new file mode 100644 index 00000000000..3d4f9802b92 --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc @@ -0,0 +1,147 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "GLES3/gl3.h" +#include "fml/logging.h" +#include "impeller/renderer/backend/gles/proc_table_gles.h" +#include "impeller/renderer/backend/gles/test/mock_gles.h" + +namespace impeller { +namespace testing { + +// OpenGLES is not thread safe. +// +// This mutex is used to ensure that only one test is using the mock at a time. +static std::mutex g_test_lock; + +static std::weak_ptr g_mock_gles; + +// Has friend visibility into MockGLES to record calls. +void RecordGLCall(const char* name) { + if (auto mock_gles = g_mock_gles.lock()) { + mock_gles->RecordCall(name); + } +} + +template +struct CheckSameSignature : std::false_type {}; + +template +struct CheckSameSignature : std::true_type {}; + +// This is a stub function that does nothing/records nothing. +void doNothing() {} + +auto const kMockVendor = (unsigned char*)"MockGLES"; +auto const kMockVersion = (unsigned char*)"3.0"; +auto const kExtensions = std::vector{ + (unsigned char*)"GL_KHR_debug" // +}; + +const unsigned char* mockGetString(GLenum name) { + switch (name) { + case GL_VENDOR: + return kMockVendor; + case GL_VERSION: + return kMockVersion; + case GL_SHADING_LANGUAGE_VERSION: + return kMockVersion; + default: + return (unsigned char*)""; + } +} + +static_assert(CheckSameSignature::value); + +const unsigned char* mockGetStringi(GLenum name, GLuint index) { + switch (name) { + case GL_EXTENSIONS: + return kExtensions[index]; + default: + return (unsigned char*)""; + } +} + +static_assert(CheckSameSignature::value); + +void mockGetIntegerv(GLenum name, int* value) { + switch (name) { + case GL_NUM_EXTENSIONS: { + *value = kExtensions.size(); + } break; + case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS: + *value = 8; + break; + default: + *value = 0; + break; + } +} + +static_assert(CheckSameSignature::value); + +GLenum mockGetError() { + return GL_NO_ERROR; +} + +static_assert(CheckSameSignature::value); + +void mockPopDebugGroupKHR() { + RecordGLCall("PopDebugGroupKHR"); +} + +static_assert(CheckSameSignature::value); + +void mockPushDebugGroupKHR(GLenum source, + GLuint id, + GLsizei length, + const GLchar* message) { + RecordGLCall("PushDebugGroupKHR"); +} + +static_assert(CheckSameSignature::value); + +std::shared_ptr MockGLES::Init() { + // If we cannot obtain a lock, MockGLES is already being used elsewhere. + FML_CHECK(g_test_lock.try_lock()) + << "MockGLES is already being used by another test."; + auto mock_gles = std::shared_ptr(new MockGLES()); + g_mock_gles = mock_gles; + return mock_gles; +} + +const ProcTableGLES::Resolver kMockResolver = [](const char* name) { + if (strcmp(name, "glPopDebugGroupKHR") == 0) { + return reinterpret_cast(&mockPopDebugGroupKHR); + } else if (strcmp(name, "glPushDebugGroupKHR") == 0) { + return reinterpret_cast(&mockPushDebugGroupKHR); + } else if (strcmp(name, "glGetString") == 0) { + return reinterpret_cast(&mockGetString); + } else if (strcmp(name, "glGetStringi") == 0) { + return reinterpret_cast(&mockGetStringi); + } else if (strcmp(name, "glGetIntegerv") == 0) { + return reinterpret_cast(&mockGetIntegerv); + } else if (strcmp(name, "glGetError") == 0) { + return reinterpret_cast(&mockGetError); + } else { + return reinterpret_cast(&doNothing); + } +}; + +MockGLES::MockGLES() : proc_table_(kMockResolver) {} + +MockGLES::~MockGLES() { + g_test_lock.unlock(); +} + +} // namespace testing +} // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h new file mode 100644 index 00000000000..ec548edce16 --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h @@ -0,0 +1,57 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include +#include "fml/macros.h" +#include "impeller/renderer/backend/gles/proc_table_gles.h" + +namespace impeller { +namespace testing { + +/// @brief Provides a mocked version of the |ProcTableGLES| class. +/// +/// Typically, Open GLES at runtime will be provided the host's GLES bindings +/// (as function pointers). This class maintains a set of function pointers that +/// appear to be GLES functions, but are actually just stubs that record +/// invocations. +/// +/// See `README.md` for more information. +class MockGLES final { + public: + /// @brief Returns an initialized |MockGLES| instance. + /// + /// This method overwrites mocked global GLES function pointers to record + /// invocations on this instance of |MockGLES|. As such, it should only be + /// called once per test. + static std::shared_ptr Init(); + + /// @brief Returns a configured |ProcTableGLES| instance. + const ProcTableGLES& GetProcTable() const { return proc_table_; } + + /// @brief Returns a vector of the names of all recorded calls. + /// + /// Calls are cleared after this method is called. + std::vector GetCapturedCalls() { + std::vector calls = captured_calls_; + captured_calls_.clear(); + return calls; + } + + ~MockGLES(); + + private: + friend void RecordGLCall(const char* name); + + MockGLES(); + + void RecordCall(const char* name) { captured_calls_.emplace_back(name); } + + const ProcTableGLES proc_table_; + std::vector captured_calls_; + + FML_DISALLOW_COPY_AND_ASSIGN(MockGLES); +}; + +} // namespace testing +} // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles_unittests.cc b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles_unittests.cc new file mode 100644 index 00000000000..a6c248b04ff --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles_unittests.cc @@ -0,0 +1,48 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/testing/testing.h" // IWYU pragma: keep +#include "gtest/gtest.h" +#include "impeller/renderer/backend/gles/proc_table_gles.h" +#include "impeller/renderer/backend/gles/test/mock_gles.h" + +namespace impeller { +namespace testing { + +// This test just checks that the proc table is initialized correctly. +// +// If this test doesn't pass, no test that uses the proc table will pass. +TEST(MockGLES, CanInitialize) { + auto mock_gles = MockGLES::Init(); + + EXPECT_EQ(mock_gles->GetProcTable().GetString(GL_VENDOR), + (unsigned char*)"MockGLES"); +} + +// Tests we can call two functions and capture the calls. +TEST(MockGLES, CapturesPushAndPopDebugGroup) { + auto mock_gles = MockGLES::Init(); + + auto& gl = mock_gles->GetProcTable(); + gl.PushDebugGroupKHR(GL_DEBUG_SOURCE_APPLICATION_KHR, 0, -1, "test"); + gl.PopDebugGroupKHR(); + + auto calls = mock_gles->GetCapturedCalls(); + EXPECT_EQ(calls, std::vector( + {"PushDebugGroupKHR", "PopDebugGroupKHR"})); +} + +// Tests that if we call a function we have not mocked, it's OK. +TEST(MockGLES, CanCallUnmockedFunction) { + auto mock_gles = MockGLES::Init(); + + auto& gl = mock_gles->GetProcTable(); + gl.DeleteFramebuffers(1, nullptr); + + // Test should still complete. + // If we end up mocking DeleteFramebuffers, delete this test. +} + +} // namespace testing +} // namespace impeller