From c73d53a425ee83a1c4f4ccfa69f97e930dbb5380 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 29 Jan 2024 11:01:03 -0800 Subject: [PATCH 01/13] [Impeller] implemented golden image tests for opengles --- impeller/golden_tests/BUILD.gn | 2 ++ .../golden_playground_test_mac.cc | 27 ++++++++++++++----- impeller/golden_tests/vulkan_screenshotter.mm | 4 --- tools/gn | 9 ++++--- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/impeller/golden_tests/BUILD.gn b/impeller/golden_tests/BUILD.gn index e26a1b614c127..efd302e762831 100644 --- a/impeller/golden_tests/BUILD.gn +++ b/impeller/golden_tests/BUILD.gn @@ -84,6 +84,8 @@ if (is_mac) { "//flutter/impeller/aiks:aiks_unittests_golden", "//flutter/impeller/fixtures", "//flutter/third_party/swiftshader", + "//third_party/angle:libEGL", + "//third_party/angle:libGLESv2", "//third_party/googletest:gtest", ] } diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index e61b45cfa226c..69ad4b4fc321b 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -143,6 +143,7 @@ bool ShouldTestHaveVulkanValidations() { struct GoldenPlaygroundTest::GoldenPlaygroundTestImpl { std::unique_ptr test_vulkan_playground; + std::unique_ptr test_opengl_playground; std::unique_ptr screenshotter; ISize window_size = ISize{1024, 768}; }; @@ -172,13 +173,27 @@ void GoldenPlaygroundTest::SetUp() { std::filesystem::path icd_path = target_path / "vk_swiftshader_icd.json"; setenv("VK_ICD_FILENAMES", icd_path.c_str(), 1); - if (GetBackend() != PlaygroundBackend::kMetal && - GetBackend() != PlaygroundBackend::kVulkan) { - GTEST_SKIP_("GoldenPlaygroundTest doesn't support this backend type."); - return; - } - bool enable_vulkan_validations = ShouldTestHaveVulkanValidations(); + switch (GetParam()) { + case PlaygroundBackend::kMetal: + pimpl_->screenshotter = std::make_unique(); + break; + case PlaygroundBackend::kVulkan: { + const std::unique_ptr& playground = + GetSharedVulkanPlayground(enable_vulkan_validations); + pimpl_->screenshotter = + std::make_unique(playground); + break; + } + case PlaygroundBackend::kOpenGLES: { + FML_CHECK(::glfwInit() == GLFW_TRUE); + pimpl_->test_opengl_playground = + PlaygroundImpl::Create(PlaygroundBackend::kOpenGLES, {}); + pimpl_->screenshotter = std::make_unique( + pimpl_->test_opengl_playground); + break; + } + } if (GetParam() == PlaygroundBackend::kMetal) { pimpl_->screenshotter = std::make_unique(); } else if (GetParam() == PlaygroundBackend::kVulkan) { diff --git a/impeller/golden_tests/vulkan_screenshotter.mm b/impeller/golden_tests/vulkan_screenshotter.mm index 57c109e565cf7..e295ceb20608a 100644 --- a/impeller/golden_tests/vulkan_screenshotter.mm +++ b/impeller/golden_tests/vulkan_screenshotter.mm @@ -6,8 +6,6 @@ #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/impeller/golden_tests/metal_screenshot.h" -#include "impeller/renderer/backend/vulkan/surface_context_vk.h" -#include "impeller/renderer/backend/vulkan/texture_vk.h" #define GLFW_INCLUDE_NONE #include "third_party/glfw/include/GLFW/glfw3.h" @@ -84,8 +82,6 @@ aiks_context, ISize(size.width * content_scale.x, size.height * content_scale.y)); std::shared_ptr texture = image->GetTexture(); - FML_CHECK(aiks_context.GetContext()->GetBackendType() == - Context::BackendType::kVulkan); return ReadTexture(aiks_context.GetContext(), texture); } diff --git a/tools/gn b/tools/gn index a9f2864d33b06..4f252f476af3d 100755 --- a/tools/gn +++ b/tools/gn @@ -717,9 +717,8 @@ def to_gn_args(args): # gen_snapshot, but the build defines otherwise make it look like the build is # for a host Windows build and make GN think we will be building ANGLE. # Angle is not used on Mac hosts as there are no tests for the OpenGL backend. - if (is_host_build(args) and - gn_args['host_os'] != 'mac') or (args.target_os == 'android' and - get_host_os() == 'win'): + if is_host_build(args) or (args.target_os == 'android' and + get_host_os() == 'win'): # Do not build unnecessary parts of the ANGLE tree. gn_args['angle_build_all'] = False gn_args['angle_has_astc_encoder'] = False @@ -728,6 +727,10 @@ def to_gn_args(args): # https://github.com/flutter/flutter/issues/114107 if get_host_os() == 'win': gn_args['angle_force_context_check_every_call'] = True + if get_host_os() == 'mac': + gn_args['angle_enable_metal'] = True + gn_args['angle_enable_gl'] = False + gn_args['angle_enable_vulkan'] = False # ANGLE and SwiftShader share build flags to enable X11 and Wayland, # but we only need these enabled for SwiftShader. From ae4c1f0a61fab71d367cb2c9305a276300728fdd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 11:12:13 -0800 Subject: [PATCH 02/13] swapped out proc table to look in angle --- .../golden_playground_test_mac.cc | 6 +++-- .../backend/gles/playground_impl_gles.cc | 23 +++++++++++++++---- .../backend/gles/playground_impl_gles.h | 2 ++ impeller/playground/switches.h | 1 + 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index 69ad4b4fc321b..331af41b02c32 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -187,8 +187,10 @@ void GoldenPlaygroundTest::SetUp() { } case PlaygroundBackend::kOpenGLES: { FML_CHECK(::glfwInit() == GLFW_TRUE); - pimpl_->test_opengl_playground = - PlaygroundImpl::Create(PlaygroundBackend::kOpenGLES, {}); + PlaygroundSwitches playground_switches; + playground_switches.use_angle = true; + pimpl_->test_opengl_playground = PlaygroundImpl::Create( + PlaygroundBackend::kOpenGLES, playground_switches); pimpl_->screenshotter = std::make_unique( pimpl_->test_opengl_playground); break; diff --git a/impeller/playground/backend/gles/playground_impl_gles.cc b/impeller/playground/backend/gles/playground_impl_gles.cc index c57e50d30ba6f..0b9bcfc8e2807 100644 --- a/impeller/playground/backend/gles/playground_impl_gles.cc +++ b/impeller/playground/backend/gles/playground_impl_gles.cc @@ -4,6 +4,8 @@ #include "impeller/playground/backend/gles/playground_impl_gles.h" +#include + #define GLFW_INCLUDE_NONE #include "third_party/glfw/include/GLFW/glfw3.h" @@ -58,7 +60,14 @@ void PlaygroundImplGLES::DestroyWindowHandle(WindowHandle handle) { PlaygroundImplGLES::PlaygroundImplGLES(PlaygroundSwitches switches) : PlaygroundImpl(switches), handle_(nullptr, &DestroyWindowHandle), - worker_(std::shared_ptr(new ReactorWorker())) { + worker_(std::shared_ptr(new ReactorWorker())), + use_angle_(switches.use_angle) { + + if (use_angle_) { + angle_glesv2_ = dlopen("libGLESv2.dylib", RTLD_LAZY); + FML_CHECK(angle_glesv2_ != nullptr); + } + ::glfwDefaultWindowHints(); #if FML_OS_MACOSX @@ -113,9 +122,15 @@ ShaderLibraryMappingsForPlayground() { // |PlaygroundImpl| std::shared_ptr PlaygroundImplGLES::GetContext() const { - auto resolver = [](const char* name) -> void* { - return reinterpret_cast(::glfwGetProcAddress(name)); - }; + auto resolver = use_angle_ ? [](const char* name) -> void* { + void* angle_glesv2 = dlopen("libGLESv2.dylib", RTLD_LAZY); + void* symbol = dlsym(angle_glesv2, name); + FML_CHECK(symbol); + return symbol; + } + : [](const char* name) -> void* { + return reinterpret_cast(::glfwGetProcAddress(name)); + }; auto gl = std::make_unique(resolver); if (!gl->IsValid()) { FML_LOG(ERROR) << "Proc table when creating a playground was invalid."; diff --git a/impeller/playground/backend/gles/playground_impl_gles.h b/impeller/playground/backend/gles/playground_impl_gles.h index 4bcf8fd69edd5..21a51fd9ab807 100644 --- a/impeller/playground/backend/gles/playground_impl_gles.h +++ b/impeller/playground/backend/gles/playground_impl_gles.h @@ -26,6 +26,8 @@ class PlaygroundImplGLES final : public PlaygroundImpl { using UniqueHandle = std::unique_ptr; UniqueHandle handle_; std::shared_ptr worker_; + const bool use_angle_; + void* angle_glesv2_; // |PlaygroundImpl| std::shared_ptr GetContext() const override; diff --git a/impeller/playground/switches.h b/impeller/playground/switches.h index 1e28a37b195c9..4142488ce7bc4 100644 --- a/impeller/playground/switches.h +++ b/impeller/playground/switches.h @@ -20,6 +20,7 @@ struct PlaygroundSwitches { // rendered in the playground. std::optional timeout; bool enable_vulkan_validation = false; + bool use_angle = false; PlaygroundSwitches(); From 639691ef18e57e3518c3b5baf52a855749687471 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 11:26:40 -0800 Subject: [PATCH 03/13] made sure glfw uses egl so that we get angle --- .../playground/backend/gles/playground_impl_gles.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/impeller/playground/backend/gles/playground_impl_gles.cc b/impeller/playground/backend/gles/playground_impl_gles.cc index 0b9bcfc8e2807..d5f12e5f26e10 100644 --- a/impeller/playground/backend/gles/playground_impl_gles.cc +++ b/impeller/playground/backend/gles/playground_impl_gles.cc @@ -62,7 +62,6 @@ PlaygroundImplGLES::PlaygroundImplGLES(PlaygroundSwitches switches) handle_(nullptr, &DestroyWindowHandle), worker_(std::shared_ptr(new ReactorWorker())), use_angle_(switches.use_angle) { - if (use_angle_) { angle_glesv2_ = dlopen("libGLESv2.dylib", RTLD_LAZY); FML_CHECK(angle_glesv2_ != nullptr); @@ -71,8 +70,14 @@ PlaygroundImplGLES::PlaygroundImplGLES(PlaygroundSwitches switches) ::glfwDefaultWindowHints(); #if FML_OS_MACOSX - // ES Profiles are not supported on Mac. - ::glfwWindowHint(GLFW_CLIENT_API, GLFW_OPENGL_API); + if (use_angle_) { + ::glfwWindowHint(GLFW_CONTEXT_CREATION_API, GLFW_EGL_CONTEXT_API); + ::glfwWindowHint(GLFW_CLIENT_API, GLFW_OPENGL_ES_API); + ::glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 2); + ::glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 0); + } else { + ::glfwWindowHint(GLFW_CLIENT_API, GLFW_OPENGL_API); + } #else // FML_OS_MACOSX ::glfwWindowHint(GLFW_CLIENT_API, GLFW_OPENGL_ES_API); ::glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 2); From 311f0022deeb3b86c07c62e8fd9268a4cbb66e30 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 13:14:11 -0800 Subject: [PATCH 04/13] got the shaders compiling correctly --- .../golden_playground_test_mac.cc | 58 +++++++------------ impeller/tools/impeller.gni | 2 - 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index 331af41b02c32..2ca030766adc4 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -55,47 +55,33 @@ const std::unique_ptr& GetSharedVulkanPlayground( } } // namespace +#define IMP_AIKSTEST(name) \ + "impeller_Play_AiksTest_" #name "_Metal", \ + "impeller_Play_AiksTest_" #name "_OpenGLES", \ + "impeller_Play_AiksTest_" #name "_Vulkan" + // If you add a new playground test to the aiks unittests and you do not want it // to also be a golden test, then add the test name here. static const std::vector kSkipTests = { - "impeller_Play_AiksTest_CanDrawPaintMultipleTimesInteractive_Metal", - "impeller_Play_AiksTest_CanDrawPaintMultipleTimesInteractive_Vulkan", - "impeller_Play_AiksTest_CanRenderLinearGradientManyColorsUnevenStops_Metal", - "impeller_Play_AiksTest_CanRenderLinearGradientManyColorsUnevenStops_" - "Vulkan", - "impeller_Play_AiksTest_CanRenderRadialGradient_Metal", - "impeller_Play_AiksTest_CanRenderRadialGradient_Vulkan", - "impeller_Play_AiksTest_CanRenderRadialGradientManyColors_Metal", - "impeller_Play_AiksTest_CanRenderRadialGradientManyColors_Vulkan", - "impeller_Play_AiksTest_CanRenderBackdropBlurInteractive_Metal", - "impeller_Play_AiksTest_CanRenderBackdropBlurInteractive_Vulkan", - "impeller_Play_AiksTest_ClippedBlurFilterRendersCorrectlyInteractive_Metal", - "impeller_Play_AiksTest_ClippedBlurFilterRendersCorrectlyInteractive_" - "Vulkan", - "impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_" - "Metal", - "impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_" - "Vulkan", - "impeller_Play_AiksTest_GaussianBlurRotatedAndClippedInteractive_Metal", - "impeller_Play_AiksTest_GaussianBlurRotatedAndClippedInteractive_Vulkan", - "impeller_Play_AiksTest_GradientStrokesRenderCorrectly_Metal", - "impeller_Play_AiksTest_GradientStrokesRenderCorrectly_Vulkan", - "impeller_Play_AiksTest_ColorWheel_Metal", - "impeller_Play_AiksTest_ColorWheel_Vulkan", - "impeller_Play_AiksTest_SceneColorSource_Metal", - "impeller_Play_AiksTest_SceneColorSource_Vulkan", - "impeller_Play_AiksTest_SolidStrokesRenderCorrectly_Metal", - "impeller_Play_AiksTest_SolidStrokesRenderCorrectly_Vulkan", - "impeller_Play_AiksTest_TextFrameSubpixelAlignment_Metal", - "impeller_Play_AiksTest_TextFrameSubpixelAlignment_Vulkan", + IMP_AIKSTEST(CanDrawPaintMultipleTimesInteractive), + IMP_AIKSTEST(CanRenderLinearGradientManyColorsUnevenStops), + IMP_AIKSTEST(CanRenderRadialGradient), + IMP_AIKSTEST(CanRenderRadialGradientManyColors), + IMP_AIKSTEST(CanRenderBackdropBlurInteractive), + IMP_AIKSTEST(ClippedBlurFilterRendersCorrectlyInteractive), + IMP_AIKSTEST(CoverageOriginShouldBeAccountedForInSubpasses), + IMP_AIKSTEST(GaussianBlurRotatedAndClippedInteractive), + IMP_AIKSTEST(GradientStrokesRenderCorrectly), + IMP_AIKSTEST(ColorWheel), + IMP_AIKSTEST(SceneColorSource), + IMP_AIKSTEST(SolidStrokesRenderCorrectly), + IMP_AIKSTEST(TextFrameSubpixelAlignment), // TextRotated is flakey and we can't seem to get it to stabilize on Skia // Gold. - "impeller_Play_AiksTest_TextRotated_Metal", - "impeller_Play_AiksTest_TextRotated_Vulkan", + IMP_AIKSTEST(TextRotated), // Runtime stage based tests get confused with a Metal context. "impeller_Play_AiksTest_CanRenderClippedRuntimeEffects_Vulkan", - "impeller_Play_AiksTest_CaptureContext_Metal", - "impeller_Play_AiksTest_CaptureContext_Vulkan", + IMP_AIKSTEST(CaptureContext), }; static const std::vector kVulkanDenyValidationTests = { @@ -289,8 +275,8 @@ std::shared_ptr GoldenPlaygroundTest::MakeContext() const { pimpl_->test_vulkan_playground); return pimpl_->test_vulkan_playground->GetContext(); } else { - FML_CHECK(false); - return nullptr; + /// On OpenGL we create a context for each test. + return GetContext(); } } diff --git a/impeller/tools/impeller.gni b/impeller/tools/impeller.gni index 2ab7ec0f0b07a..468829e49a7df 100644 --- a/impeller/tools/impeller.gni +++ b/impeller/tools/impeller.gni @@ -642,8 +642,6 @@ template("_impeller_shaders_gles") { intermediates_subdir = "gles" } if (is_mac) { - shader_target_flags = [ "--opengl-desktop" ] - } else { shader_target_flags = [ "--opengl-es" ] } From 528c273266d9d3f184c530853e1b4aa3c4b36c9f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 13:46:38 -0800 Subject: [PATCH 05/13] fixed linux shader compile flags --- impeller/tools/impeller.gni | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/impeller/tools/impeller.gni b/impeller/tools/impeller.gni index 468829e49a7df..326f48c93f131 100644 --- a/impeller/tools/impeller.gni +++ b/impeller/tools/impeller.gni @@ -641,9 +641,7 @@ template("_impeller_shaders_gles") { if (impeller_enable_metal || impeller_enable_vulkan) { intermediates_subdir = "gles" } - if (is_mac) { - shader_target_flags = [ "--opengl-es" ] - } + shader_target_flags = [ "--opengl-es" ] defines = [ "IMPELLER_TARGET_OPENGLES" ] } From 29d088f5ca7523cf0fbf8a369ea31f7b22d3f646 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 14:02:54 -0800 Subject: [PATCH 06/13] disabled for windows --- .../playground/backend/gles/playground_impl_gles.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/impeller/playground/backend/gles/playground_impl_gles.cc b/impeller/playground/backend/gles/playground_impl_gles.cc index d5f12e5f26e10..fc8f2b09c25bb 100644 --- a/impeller/playground/backend/gles/playground_impl_gles.cc +++ b/impeller/playground/backend/gles/playground_impl_gles.cc @@ -4,7 +4,11 @@ #include "impeller/playground/backend/gles/playground_impl_gles.h" +#define IMPELLER_PLAYGROUND_SUPPORTS_ANGLE FML_OS_MACOSX + +#if IMPELLER_PLAYGROUND_SUPPORTS_ANGLE #include +#endif #define GLFW_INCLUDE_NONE #include "third_party/glfw/include/GLFW/glfw3.h" @@ -63,7 +67,9 @@ PlaygroundImplGLES::PlaygroundImplGLES(PlaygroundSwitches switches) worker_(std::shared_ptr(new ReactorWorker())), use_angle_(switches.use_angle) { if (use_angle_) { +#if IMPELLER_PLAYGROUND_SUPPORTS_ANGLE angle_glesv2_ = dlopen("libGLESv2.dylib", RTLD_LAZY); +#endif FML_CHECK(angle_glesv2_ != nullptr); } @@ -128,8 +134,11 @@ ShaderLibraryMappingsForPlayground() { // |PlaygroundImpl| std::shared_ptr PlaygroundImplGLES::GetContext() const { auto resolver = use_angle_ ? [](const char* name) -> void* { + void* symbol; +#if IMPELLER_PLAYGROUND_SUPPORTS_ANGLE void* angle_glesv2 = dlopen("libGLESv2.dylib", RTLD_LAZY); - void* symbol = dlsym(angle_glesv2, name); + symbol = dlsym(angle_glesv2, name); +#endif FML_CHECK(symbol); return symbol; } From 91e459ef47612e81586eb14d1e352c454a1658ae Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 14:10:50 -0800 Subject: [PATCH 07/13] initialized variable --- impeller/playground/backend/gles/playground_impl_gles.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/playground/backend/gles/playground_impl_gles.cc b/impeller/playground/backend/gles/playground_impl_gles.cc index fc8f2b09c25bb..57c6f168a4bfb 100644 --- a/impeller/playground/backend/gles/playground_impl_gles.cc +++ b/impeller/playground/backend/gles/playground_impl_gles.cc @@ -134,7 +134,7 @@ ShaderLibraryMappingsForPlayground() { // |PlaygroundImpl| std::shared_ptr PlaygroundImplGLES::GetContext() const { auto resolver = use_angle_ ? [](const char* name) -> void* { - void* symbol; + void* symbol = nullptr; #if IMPELLER_PLAYGROUND_SUPPORTS_ANGLE void* angle_glesv2 = dlopen("libGLESv2.dylib", RTLD_LAZY); symbol = dlsym(angle_glesv2, name); From 97bad112d3e8da057bbdf5fd4cc7601597a22291 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 14:36:42 -0800 Subject: [PATCH 08/13] set cwd to grab dylibs --- testing/run_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index bc5fa72269889..18b3c2ccaebc3 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -1107,7 +1107,7 @@ def run_impeller_golden_tests(build_dir: str): 'golden_tests_harvester' ) with tempfile.TemporaryDirectory(prefix='impeller_golden') as temp_dir: - run_cmd([tests_path, '--working_dir=%s' % temp_dir]) + run_cmd([tests_path, '--working_dir=%s' % temp_dir], cwd=build_dir) with DirectoryChange(harvester_path): run_cmd(['dart', 'pub', 'get']) bin_path = Path('.').joinpath('bin' From 7460ff9f77ef1801dbfcf150e9028f3aaa303484 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 14:52:26 -0800 Subject: [PATCH 09/13] added opengles compilation --- ci/builders/mac_host_engine.json | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/builders/mac_host_engine.json b/ci/builders/mac_host_engine.json index b1836776bd38a..e6f5ff00a5c54 100644 --- a/ci/builders/mac_host_engine.json +++ b/ci/builders/mac_host_engine.json @@ -159,6 +159,7 @@ "--prebuilt-dart-sdk", "--build-embedder-examples", "--enable-impeller-vulkan", + "--enable-impeller-opengles", "--use-glfw-swiftshader" ], "name": "host_release", From 5752f5d049852c90385d25875506f33618ff966f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 15:39:16 -0800 Subject: [PATCH 10/13] turned off opengles impeller_unittests for now --- testing/run_tests.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 18b3c2ccaebc3..52f3e365a3a02 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -518,12 +518,16 @@ def make_test(name, flags=None, extra_env=None): 'METAL_DEBUG_ERROR_MODE': '0', # Enables metal validation. 'METAL_DEVICE_WRAPPER_TYPE': '1', # Enables metal validation. }) + mac_impeller_unittests_flags = shuffle_flags + [ + '--enable_vulkan_validation', + '--gtest_filter=-*OpenGLES' # These are covered in the golden tests. + ] # Impeller tests are only supported on macOS for now. run_engine_executable( build_dir, 'impeller_unittests', executable_filter, - shuffle_flags + ['--enable_vulkan_validation'], + mac_impeller_unittests_flags, coverage=coverage, extra_env=extra_env, # TODO(https://github.com/flutter/flutter/issues/123733): Remove this allowlist. From 78aa37fc3022b7550eaa1a2202a74f94b9acd276 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 30 Jan 2024 15:55:39 -0800 Subject: [PATCH 11/13] ++ --- testing/run_tests.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 52f3e365a3a02..6be8359ef6082 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -562,7 +562,10 @@ def make_test(name, flags=None, extra_env=None): build_dir, 'impeller_dart_unittests', executable_filter, - shuffle_flags + ['--enable_vulkan_validation'], + shuffle_flags + [ + '--enable_vulkan_validation', + '--gtest_filter=-*OpenGLES', # TODO(tbd) + ], coverage=coverage, extra_env=extra_env, ) From 058bc61304221dc675ce0eef1c2cc3b6f9175601 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 31 Jan 2024 09:24:29 -0800 Subject: [PATCH 12/13] started flipping the images based on the textures y scale --- impeller/golden_tests/vulkan_screenshotter.mm | 55 +++++++++++++++---- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/impeller/golden_tests/vulkan_screenshotter.mm b/impeller/golden_tests/vulkan_screenshotter.mm index e295ceb20608a..1b3111be9d199 100644 --- a/impeller/golden_tests/vulkan_screenshotter.mm +++ b/impeller/golden_tests/vulkan_screenshotter.mm @@ -13,6 +13,15 @@ namespace testing { namespace { + +using CGContextPtr = std::unique_ptr::type, + decltype(&CGContextRelease)>; +using CGImagePtr = std::unique_ptr::type, + decltype(&CGImageRelease)>; +using CGColorSpacePtr = + std::unique_ptr::type, + decltype(&CGColorSpaceRelease)>; + std::unique_ptr ReadTexture( const std::shared_ptr& surface_context, const std::shared_ptr& texture) { @@ -47,21 +56,43 @@ // TODO(gaaclarke): Replace CoreImage requirement with something // crossplatform. - CGColorSpaceRef color_space = CGColorSpaceCreateDeviceRGB(); + CGColorSpacePtr color_space(CGColorSpaceCreateDeviceRGB(), + &CGColorSpaceRelease); CGBitmapInfo bitmap_info = kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Little; - CGContextRef context = CGBitmapContextCreate( - device_buffer->OnGetContents(), texture->GetSize().width, - texture->GetSize().height, - /*bitsPerComponent=*/8, - /*bytesPerRow=*/texture->GetTextureDescriptor().GetBytesPerRow(), - color_space, bitmap_info); + CGContextPtr context( + CGBitmapContextCreate( + device_buffer->OnGetContents(), texture->GetSize().width, + texture->GetSize().height, + /*bitsPerComponent=*/8, + /*bytesPerRow=*/texture->GetTextureDescriptor().GetBytesPerRow(), + color_space.get(), bitmap_info), + &CGContextRelease); FML_CHECK(context); - CGImageRef image_ref = CGBitmapContextCreateImage(context); - FML_CHECK(image_ref); - CGContextRelease(context); - CGColorSpaceRelease(color_space); - return std::make_unique(image_ref); + CGImagePtr image(CGBitmapContextCreateImage(context.get()), &CGImageRelease); + FML_CHECK(image); + + // TODO(tbd): Perform the flip at the blit stage to avoid this slow + // copy. + if (texture->GetYCoordScale() == -1) { + CGContextPtr flipped_context( + CGBitmapContextCreate( + nullptr, texture->GetSize().width, texture->GetSize().height, + /*bitsPerComponent=*/8, + /*bytesPerRow=*/0, color_space.get(), bitmap_info), + &CGContextRelease); + CGContextTranslateCTM(flipped_context.get(), 0, texture->GetSize().height); + CGContextScaleCTM(flipped_context.get(), 1.0, -1.0); + CGContextDrawImage( + flipped_context.get(), + CGRectMake(0, 0, texture->GetSize().width, texture->GetSize().height), + image.get()); + CGImagePtr flipped_image(CGBitmapContextCreateImage(flipped_context.get()), + &CGImageRelease); + image.swap(flipped_image); + } + + return std::make_unique(image.release()); } } // namespace From 327502b5a89c3b6583a939987e219038c4c46499 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 31 Jan 2024 11:11:38 -0800 Subject: [PATCH 13/13] fixed BGRA vs RGBA --- impeller/golden_tests/vulkan_screenshotter.mm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/impeller/golden_tests/vulkan_screenshotter.mm b/impeller/golden_tests/vulkan_screenshotter.mm index 1b3111be9d199..0493909a53a18 100644 --- a/impeller/golden_tests/vulkan_screenshotter.mm +++ b/impeller/golden_tests/vulkan_screenshotter.mm @@ -59,7 +59,9 @@ CGColorSpacePtr color_space(CGColorSpaceCreateDeviceRGB(), &CGColorSpaceRelease); CGBitmapInfo bitmap_info = - kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Little; + texture->GetTextureDescriptor().format == PixelFormat::kB8G8R8A8UNormInt + ? kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Little + : kCGImageAlphaPremultipliedLast; CGContextPtr context( CGBitmapContextCreate( device_buffer->OnGetContents(), texture->GetSize().width,