From 29e779231add6ebc74dbc92348f85acf74e31644 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Thu, 14 Dec 2023 13:09:32 -0800 Subject: [PATCH 01/11] [Impeller] Turned on new blur. (#48472) This new blur should perform faster since it scales down the image before blurring it. Jonah did early testing of it and found it to be faster. Scrolling around with the blur perf bug it seems faster. It also has a wider test bed and is hopefully easier to maintain since it contains all of its logic for both directions. testing: There are existing blur tests and we've backfilled more as we've added features to this blur. fixes https://github.com/flutter/flutter/issues/131580 fixes https://github.com/flutter/flutter/issues/138259 - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- impeller/aiks/aiks_unittests.cc | 6 +-- ...irectional_gaussian_blur_filter_contents.h | 3 ++ .../contents/filters/filter_contents.cc | 40 ++----------------- 3 files changed, 10 insertions(+), 39 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 03cf5495b6b86..f59a3f00edceb 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3758,7 +3758,7 @@ TEST_P(AiksTest, GaussianBlurSetsMipCountOnPass) { canvas.Restore(); Picture picture = canvas.EndRecordingAsPicture(); - EXPECT_EQ(1, picture.pass->GetRequiredMipCount()); + EXPECT_EQ(4, picture.pass->GetRequiredMipCount()); } TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { @@ -3782,7 +3782,7 @@ TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { max_mip_count = std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); } - EXPECT_EQ(max_mip_count, 1lu); + EXPECT_EQ(max_mip_count, 4lu); } TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) { @@ -3809,7 +3809,7 @@ TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) { max_mip_count = std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); } - EXPECT_EQ(max_mip_count, 1lu); + EXPECT_EQ(max_mip_count, 4lu); EXPECT_EQ(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), std::string::npos); } diff --git a/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.h index 2ea5df044f2cb..548d1d409cf28 100644 --- a/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.h @@ -43,6 +43,9 @@ namespace impeller { /// - `FilterContents::MakeGaussianBlur` /// - //flutter/impeller/entity/shaders/gaussian_blur/gaussian_blur.glsl /// +///\deprecated Previously 2 of these were chained to do 2D blurs, use +/// \ref GaussianBlurFilterContents instead since it has better +/// performance. class DirectionalGaussianBlurFilterContents final : public FilterContents { public: DirectionalGaussianBlurFilterContents(); diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index 235a53134c5e3..6ed61b0235744 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -50,11 +50,7 @@ std::shared_ptr FilterContents::MakeDirectionalGaussianBlur( return blur; } -#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER const int32_t FilterContents::kBlurFilterRequiredMipCount = 4; -#else -const int32_t FilterContents::kBlurFilterRequiredMipCount = 1; -#endif std::shared_ptr FilterContents::MakeGaussianBlur( const FilterInput::Ref& input, @@ -62,38 +58,10 @@ std::shared_ptr FilterContents::MakeGaussianBlur( Sigma sigma_y, BlurStyle blur_style, Entity::TileMode tile_mode) { - constexpr bool use_new_filter = -#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER - true; -#else - false; -#endif - - // TODO(https://github.com/flutter/flutter/issues/131580): Remove once the new - // blur handles all cases. - if (use_new_filter) { - auto blur = std::make_shared( - sigma_x.sigma, sigma_y.sigma, tile_mode); - blur->SetInputs({input}); - return blur; - } - std::shared_ptr x_blur = MakeDirectionalGaussianBlur( - /*input=*/input, - /*sigma=*/sigma_x, - /*direction=*/Point(1, 0), - /*blur_style=*/BlurStyle::kNormal, - /*tile_mode=*/tile_mode, - /*is_second_pass=*/false, - /*secondary_sigma=*/{}); - std::shared_ptr y_blur = MakeDirectionalGaussianBlur( - /*input=*/FilterInput::Make(x_blur), - /*sigma=*/sigma_y, - /*direction=*/Point(0, 1), - /*blur_style=*/blur_style, - /*tile_mode=*/tile_mode, - /*is_second_pass=*/true, - /*secondary_sigma=*/sigma_x); - return y_blur; + auto blur = std::make_shared( + sigma_x.sigma, sigma_y.sigma, tile_mode); + blur->SetInputs({input}); + return blur; } std::shared_ptr FilterContents::MakeBorderMaskBlur( From b3c96e10d03ec08eb578fec13f8ba4301c66cf7e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 12 Jan 2024 16:20:32 -0800 Subject: [PATCH 02/11] chinmays vulkan patch --- impeller/BUILD.gn | 4 --- .../backend/vulkan/playground_impl_vk.cc | 34 +++++++++++++++++-- impeller/playground/playground.cc | 2 +- impeller/tools/impeller.gni | 7 ---- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index 3bfe85a0fb072..3a69d7db48a8b 100644 --- a/impeller/BUILD.gn +++ b/impeller/BUILD.gn @@ -34,10 +34,6 @@ config("impeller_public_config") { defines += [ "IMPELLER_ENABLE_VULKAN=1" ] } - if (impeller_enable_vulkan_playgrounds) { - defines += [ "IMPELLER_ENABLE_VULKAN_PLAYGROUNDS=1" ] - } - if (impeller_trace_all_gl_calls) { defines += [ "IMPELLER_TRACE_ALL_GL_CALLS" ] } diff --git a/impeller/playground/backend/vulkan/playground_impl_vk.cc b/impeller/playground/backend/vulkan/playground_impl_vk.cc index d05004f4eb43a..b42c11011724d 100644 --- a/impeller/playground/backend/vulkan/playground_impl_vk.cc +++ b/impeller/playground/backend/vulkan/playground_impl_vk.cc @@ -175,10 +175,38 @@ void PlaygroundImplVK::InitGlobalVulkanInstance() { application_info.setPEngineName("PlaygroundImplVK"); application_info.setPApplicationName("PlaygroundImplVK"); - auto instance_result = - vk::createInstanceUnique(vk::InstanceCreateInfo({}, &application_info)); + CapabilitiesVK caps(false); + auto enabled_layers = caps.GetEnabledLayers(); + auto enabled_extensions = caps.GetEnabledInstanceExtensions(); + + FML_CHECK(enabled_layers.has_value() && enabled_extensions.has_value()); + + std::vector enabled_layers_c; + std::vector enabled_extensions_c; + + for (const auto& layer : enabled_layers.value()) { + enabled_layers_c.push_back(layer.c_str()); + } + + for (const auto& ext : enabled_extensions.value()) { + enabled_extensions_c.push_back(ext.c_str()); + } + + vk::InstanceCreateInfo instance_info; + instance_info.setPEnabledLayerNames(enabled_layers_c); + instance_info.setPEnabledExtensionNames(enabled_extensions_c); + instance_info.setPApplicationInfo(&application_info); + + if (std::find(enabled_extensions->begin(), enabled_extensions->end(), + "VK_KHR_portability_enumeration") != + enabled_extensions->end()) { + instance_info.flags |= vk::InstanceCreateFlagBits::eEnumeratePortabilityKHR; + } + + auto instance_result = vk::createInstanceUnique(instance_info); FML_CHECK(instance_result.result == vk::Result::eSuccess) - << "Unable to initialize global Vulkan instance"; + << "Unable to initialize global Vulkan instance: " + << vk::to_string(instance_result.result); global_instance_ = std::move(instance_result.value); } diff --git a/impeller/playground/playground.cc b/impeller/playground/playground.cc index c4f4f1338c391..bf2e2c28e0685 100644 --- a/impeller/playground/playground.cc +++ b/impeller/playground/playground.cc @@ -102,7 +102,7 @@ bool Playground::SupportsBackend(PlaygroundBackend backend) { return false; #endif // IMPELLER_ENABLE_OPENGLES case PlaygroundBackend::kVulkan: -#if IMPELLER_ENABLE_VULKAN && IMPELLER_ENABLE_VULKAN_PLAYGROUNDS +#if IMPELLER_ENABLE_VULKAN return true; #else // IMPELLER_ENABLE_VULKAN return false; diff --git a/impeller/tools/impeller.gni b/impeller/tools/impeller.gni index 2ab7ec0f0b07a..3aa51175c0b93 100644 --- a/impeller/tools/impeller.gni +++ b/impeller/tools/impeller.gni @@ -25,13 +25,6 @@ declare_args() { impeller_enable_vulkan = (is_linux || is_win || is_android || enable_unittests) && target_os != "fuchsia" - # Whether playgrounds should run with Vulkan. - # - # impeller_enable_vulkan may be true in build environments that run tests but - # do not have a Vulkan ICD present. - impeller_enable_vulkan_playgrounds = - (is_linux || is_win || is_android) && target_os != "fuchsia" - # Whether to use a prebuilt impellerc. # If this is the empty string, impellerc will be built. # If it is non-empty, it should be the absolute path to impellerc. From 330e1acd701d268868fcd367f6271cf48f1a8260 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 12 Jan 2024 16:28:39 -0800 Subject: [PATCH 03/11] started setting TextureVK::mipmaps_generated_ --- impeller/renderer/backend/vulkan/blit_command_vk.cc | 1 + impeller/renderer/backend/vulkan/texture_vk.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/impeller/renderer/backend/vulkan/blit_command_vk.cc b/impeller/renderer/backend/vulkan/blit_command_vk.cc index 6f9a9a55c7f1a..0cd334658d023 100644 --- a/impeller/renderer/backend/vulkan/blit_command_vk.cc +++ b/impeller/renderer/backend/vulkan/blit_command_vk.cc @@ -353,6 +353,7 @@ bool BlitGenerateMipmapCommandVK::Encode(CommandEncoderVK& encoder) const { // state so it doesn't try to perform redundant transitions under the hood. src.SetLayoutWithoutEncoding(vk::ImageLayout::eShaderReadOnlyOptimal); + src.SetMipMapGenerated(); return true; } diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h index 4df1dcc37b1bb..56ebb217d6749 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.h +++ b/impeller/renderer/backend/vulkan/texture_vk.h @@ -38,6 +38,8 @@ class TextureVK final : public Texture, public BackendCast { std::shared_ptr GetTextureSource() const; + void SetMipMapGenerated() { mipmap_generated_ = true; } + private: std::weak_ptr context_; std::shared_ptr source_; From 5107cd69c2be113db135ca57774f1b389c4631bb Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 16 Jan 2024 09:43:22 -0800 Subject: [PATCH 04/11] made tests only want metal mips --- impeller/aiks/aiks_unittests.cc | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index f59a3f00edceb..27b8fac2582c4 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3749,6 +3749,9 @@ TEST_P(AiksTest, GuassianBlurUpdatesMipmapContents) { } TEST_P(AiksTest, GaussianBlurSetsMipCountOnPass) { + int32_t blur_required_mip_count = + GetParam() == PlaygroundBackend::kMetal ? 4 : 1; + Canvas canvas; canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()}); canvas.SaveLayer({}, std::nullopt, @@ -3758,10 +3761,13 @@ TEST_P(AiksTest, GaussianBlurSetsMipCountOnPass) { canvas.Restore(); Picture picture = canvas.EndRecordingAsPicture(); - EXPECT_EQ(4, picture.pass->GetRequiredMipCount()); + EXPECT_EQ(blur_required_mip_count, picture.pass->GetRequiredMipCount()); } TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { + size_t blur_required_mip_count = + GetParam() == PlaygroundBackend::kMetal ? 4 : 1; + Canvas canvas; canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()}); canvas.SaveLayer({}, std::nullopt, @@ -3782,11 +3788,14 @@ TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { max_mip_count = std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); } - EXPECT_EQ(max_mip_count, 4lu); + EXPECT_EQ(max_mip_count, blur_required_mip_count); } TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) { fml::testing::LogCapture log_capture; + size_t blur_required_mip_count = + GetParam() == PlaygroundBackend::kMetal ? 4 : 1; + Canvas canvas; canvas.DrawPaint({.color = Color::Wheat()}); canvas.SaveLayer({.blend_mode = BlendMode::kMultiply}); @@ -3809,12 +3818,14 @@ TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) { max_mip_count = std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); } - EXPECT_EQ(max_mip_count, 4lu); + EXPECT_EQ(max_mip_count, blur_required_mip_count); EXPECT_EQ(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), std::string::npos); } TEST_P(AiksTest, GaussianBlurMipMapImageFilter) { + size_t blur_required_mip_count = + GetParam() == PlaygroundBackend::kMetal ? 4 : 1; fml::testing::LogCapture log_capture; Canvas canvas; canvas.SaveLayer( @@ -3835,7 +3846,7 @@ TEST_P(AiksTest, GaussianBlurMipMapImageFilter) { max_mip_count = std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); } - EXPECT_EQ(max_mip_count, 1lu); + EXPECT_EQ(max_mip_count, blur_required_mip_count); EXPECT_EQ(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), std::string::npos); } From c92e348d90259ddb8003ce24fc3a15702995048a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 16 Jan 2024 11:31:27 -0800 Subject: [PATCH 05/11] turned on for macos ios only --- .../contents/filters/filter_contents.cc | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index 6ed61b0235744..dd7afffdab29f 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -50,7 +50,15 @@ std::shared_ptr FilterContents::MakeDirectionalGaussianBlur( return blur; } +#if defined(FML_OS_MACOSX) || defined(FML_OS_IOS) +#define IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER 1 +#endif + +#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER const int32_t FilterContents::kBlurFilterRequiredMipCount = 4; +#else +const int32_t FilterContents::kBlurFilterRequiredMipCount = 1; +#endif std::shared_ptr FilterContents::MakeGaussianBlur( const FilterInput::Ref& input, @@ -58,10 +66,38 @@ std::shared_ptr FilterContents::MakeGaussianBlur( Sigma sigma_y, BlurStyle blur_style, Entity::TileMode tile_mode) { - auto blur = std::make_shared( - sigma_x.sigma, sigma_y.sigma, tile_mode); - blur->SetInputs({input}); - return blur; + constexpr bool use_new_filter = +#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER + true; +#else + false; +#endif + + // TODO(https://github.com/flutter/flutter/issues/131580): Remove once the new + // blur handles all cases. + if (use_new_filter) { + auto blur = std::make_shared( + sigma_x.sigma, sigma_y.sigma, tile_mode); + blur->SetInputs({input}); + return blur; + } + std::shared_ptr x_blur = MakeDirectionalGaussianBlur( + /*input=*/input, + /*sigma=*/sigma_x, + /*direction=*/Point(1, 0), + /*blur_style=*/BlurStyle::kNormal, + /*tile_mode=*/tile_mode, + /*is_second_pass=*/false, + /*secondary_sigma=*/{}); + std::shared_ptr y_blur = MakeDirectionalGaussianBlur( + /*input=*/FilterInput::Make(x_blur), + /*sigma=*/sigma_y, + /*direction=*/Point(0, 1), + /*blur_style=*/blur_style, + /*tile_mode=*/tile_mode, + /*is_second_pass=*/true, + /*secondary_sigma=*/sigma_x); + return y_blur; } std::shared_ptr FilterContents::MakeBorderMaskBlur( From 7a095b331b6cb14d28e5e4a8f52b6d95f7520463 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 16 Jan 2024 12:38:01 -0800 Subject: [PATCH 06/11] Revert "started setting TextureVK::mipmaps_generated_" This reverts commit cae5c506650b92435f18e5d28d03817f3b5ea0f3. --- impeller/renderer/backend/vulkan/blit_command_vk.cc | 1 - impeller/renderer/backend/vulkan/texture_vk.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/blit_command_vk.cc b/impeller/renderer/backend/vulkan/blit_command_vk.cc index 0cd334658d023..6f9a9a55c7f1a 100644 --- a/impeller/renderer/backend/vulkan/blit_command_vk.cc +++ b/impeller/renderer/backend/vulkan/blit_command_vk.cc @@ -353,7 +353,6 @@ bool BlitGenerateMipmapCommandVK::Encode(CommandEncoderVK& encoder) const { // state so it doesn't try to perform redundant transitions under the hood. src.SetLayoutWithoutEncoding(vk::ImageLayout::eShaderReadOnlyOptimal); - src.SetMipMapGenerated(); return true; } diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h index 56ebb217d6749..4df1dcc37b1bb 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.h +++ b/impeller/renderer/backend/vulkan/texture_vk.h @@ -38,8 +38,6 @@ class TextureVK final : public Texture, public BackendCast { std::shared_ptr GetTextureSource() const; - void SetMipMapGenerated() { mipmap_generated_ = true; } - private: std::weak_ptr context_; std::shared_ptr source_; From ee976a45e43e9ce576d955f6974ecdc6b924ec67 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 16 Jan 2024 12:38:07 -0800 Subject: [PATCH 07/11] Revert "chinmays vulkan patch" This reverts commit 590a2457ced174716756b6c75f5b1781748dab65. --- impeller/BUILD.gn | 4 +++ .../backend/vulkan/playground_impl_vk.cc | 34 ++----------------- impeller/playground/playground.cc | 2 +- impeller/tools/impeller.gni | 7 ++++ 4 files changed, 15 insertions(+), 32 deletions(-) diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index 3a69d7db48a8b..3bfe85a0fb072 100644 --- a/impeller/BUILD.gn +++ b/impeller/BUILD.gn @@ -34,6 +34,10 @@ config("impeller_public_config") { defines += [ "IMPELLER_ENABLE_VULKAN=1" ] } + if (impeller_enable_vulkan_playgrounds) { + defines += [ "IMPELLER_ENABLE_VULKAN_PLAYGROUNDS=1" ] + } + if (impeller_trace_all_gl_calls) { defines += [ "IMPELLER_TRACE_ALL_GL_CALLS" ] } diff --git a/impeller/playground/backend/vulkan/playground_impl_vk.cc b/impeller/playground/backend/vulkan/playground_impl_vk.cc index b42c11011724d..d05004f4eb43a 100644 --- a/impeller/playground/backend/vulkan/playground_impl_vk.cc +++ b/impeller/playground/backend/vulkan/playground_impl_vk.cc @@ -175,38 +175,10 @@ void PlaygroundImplVK::InitGlobalVulkanInstance() { application_info.setPEngineName("PlaygroundImplVK"); application_info.setPApplicationName("PlaygroundImplVK"); - CapabilitiesVK caps(false); - auto enabled_layers = caps.GetEnabledLayers(); - auto enabled_extensions = caps.GetEnabledInstanceExtensions(); - - FML_CHECK(enabled_layers.has_value() && enabled_extensions.has_value()); - - std::vector enabled_layers_c; - std::vector enabled_extensions_c; - - for (const auto& layer : enabled_layers.value()) { - enabled_layers_c.push_back(layer.c_str()); - } - - for (const auto& ext : enabled_extensions.value()) { - enabled_extensions_c.push_back(ext.c_str()); - } - - vk::InstanceCreateInfo instance_info; - instance_info.setPEnabledLayerNames(enabled_layers_c); - instance_info.setPEnabledExtensionNames(enabled_extensions_c); - instance_info.setPApplicationInfo(&application_info); - - if (std::find(enabled_extensions->begin(), enabled_extensions->end(), - "VK_KHR_portability_enumeration") != - enabled_extensions->end()) { - instance_info.flags |= vk::InstanceCreateFlagBits::eEnumeratePortabilityKHR; - } - - auto instance_result = vk::createInstanceUnique(instance_info); + auto instance_result = + vk::createInstanceUnique(vk::InstanceCreateInfo({}, &application_info)); FML_CHECK(instance_result.result == vk::Result::eSuccess) - << "Unable to initialize global Vulkan instance: " - << vk::to_string(instance_result.result); + << "Unable to initialize global Vulkan instance"; global_instance_ = std::move(instance_result.value); } diff --git a/impeller/playground/playground.cc b/impeller/playground/playground.cc index bf2e2c28e0685..c4f4f1338c391 100644 --- a/impeller/playground/playground.cc +++ b/impeller/playground/playground.cc @@ -102,7 +102,7 @@ bool Playground::SupportsBackend(PlaygroundBackend backend) { return false; #endif // IMPELLER_ENABLE_OPENGLES case PlaygroundBackend::kVulkan: -#if IMPELLER_ENABLE_VULKAN +#if IMPELLER_ENABLE_VULKAN && IMPELLER_ENABLE_VULKAN_PLAYGROUNDS return true; #else // IMPELLER_ENABLE_VULKAN return false; diff --git a/impeller/tools/impeller.gni b/impeller/tools/impeller.gni index 3aa51175c0b93..2ab7ec0f0b07a 100644 --- a/impeller/tools/impeller.gni +++ b/impeller/tools/impeller.gni @@ -25,6 +25,13 @@ declare_args() { impeller_enable_vulkan = (is_linux || is_win || is_android || enable_unittests) && target_os != "fuchsia" + # Whether playgrounds should run with Vulkan. + # + # impeller_enable_vulkan may be true in build environments that run tests but + # do not have a Vulkan ICD present. + impeller_enable_vulkan_playgrounds = + (is_linux || is_win || is_android) && target_os != "fuchsia" + # Whether to use a prebuilt impellerc. # If this is the empty string, impellerc will be built. # If it is non-empty, it should be the absolute path to impellerc. From e57fc5bc53722e9330360f63370e0b32d2c06fbf Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 16 Jan 2024 14:33:50 -0800 Subject: [PATCH 08/11] Revert "turned on for macos ios only" This reverts commit 3d8682ed25773aebb4fd01f101aafb4d1a855d26. --- .../contents/filters/filter_contents.cc | 44 ++----------------- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index dd7afffdab29f..6ed61b0235744 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -50,15 +50,7 @@ std::shared_ptr FilterContents::MakeDirectionalGaussianBlur( return blur; } -#if defined(FML_OS_MACOSX) || defined(FML_OS_IOS) -#define IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER 1 -#endif - -#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER const int32_t FilterContents::kBlurFilterRequiredMipCount = 4; -#else -const int32_t FilterContents::kBlurFilterRequiredMipCount = 1; -#endif std::shared_ptr FilterContents::MakeGaussianBlur( const FilterInput::Ref& input, @@ -66,38 +58,10 @@ std::shared_ptr FilterContents::MakeGaussianBlur( Sigma sigma_y, BlurStyle blur_style, Entity::TileMode tile_mode) { - constexpr bool use_new_filter = -#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER - true; -#else - false; -#endif - - // TODO(https://github.com/flutter/flutter/issues/131580): Remove once the new - // blur handles all cases. - if (use_new_filter) { - auto blur = std::make_shared( - sigma_x.sigma, sigma_y.sigma, tile_mode); - blur->SetInputs({input}); - return blur; - } - std::shared_ptr x_blur = MakeDirectionalGaussianBlur( - /*input=*/input, - /*sigma=*/sigma_x, - /*direction=*/Point(1, 0), - /*blur_style=*/BlurStyle::kNormal, - /*tile_mode=*/tile_mode, - /*is_second_pass=*/false, - /*secondary_sigma=*/{}); - std::shared_ptr y_blur = MakeDirectionalGaussianBlur( - /*input=*/FilterInput::Make(x_blur), - /*sigma=*/sigma_y, - /*direction=*/Point(0, 1), - /*blur_style=*/blur_style, - /*tile_mode=*/tile_mode, - /*is_second_pass=*/true, - /*secondary_sigma=*/sigma_x); - return y_blur; + auto blur = std::make_shared( + sigma_x.sigma, sigma_y.sigma, tile_mode); + blur->SetInputs({input}); + return blur; } std::shared_ptr FilterContents::MakeBorderMaskBlur( From a37ff1b1e2f95d431011194039d1e04d5b643743 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 16 Jan 2024 14:39:23 -0800 Subject: [PATCH 09/11] disabled mipmap generation for vulkan --- impeller/aiks/aiks_unittests.cc | 18 ++++++++++++++---- impeller/entity/entity_pass.cc | 6 ++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 27b8fac2582c4..ce98716d02735 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3819,8 +3819,13 @@ TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) { std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); - EXPECT_EQ(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), - std::string::npos); + if (GetParam() == PlaygroundBackend::kMetal) { + EXPECT_EQ(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), + std::string::npos); + } else { + EXPECT_NE(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), + std::string::npos); + } } TEST_P(AiksTest, GaussianBlurMipMapImageFilter) { @@ -3847,8 +3852,13 @@ TEST_P(AiksTest, GaussianBlurMipMapImageFilter) { std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); - EXPECT_EQ(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), - std::string::npos); + if (GetParam() == PlaygroundBackend::kMetal) { + EXPECT_EQ(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), + std::string::npos); + } else { + EXPECT_NE(log_capture.str().find(GaussianBlurFilterContents::kNoMipsError), + std::string::npos); + } } } // namespace testing diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 9323692ffac1b..a7e3e277e56ce 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -256,6 +256,12 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, /// What's important is the `StorageMode` of the textures, which cannot be /// changed for the lifetime of the textures. + if (context->GetBackendType() != Context::BackendType::kMetal) { + // TODO(tbd): Implement mip map generation on vulkan. + // TODO(tbd): Implement mip map generation on opengles. + mip_count = 1; + } + RenderTarget target; if (context->GetCapabilities()->SupportsOffscreenMSAA()) { target = RenderTarget::CreateOffscreenMSAA( From 733b7e3f28a038792201611bf7c4d9692d2986d0 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 16 Jan 2024 15:27:39 -0800 Subject: [PATCH 10/11] fixed vulkan/opengles test --- impeller/aiks/aiks_unittests.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index ce98716d02735..6d92b8585ab8c 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3749,9 +3749,6 @@ TEST_P(AiksTest, GuassianBlurUpdatesMipmapContents) { } TEST_P(AiksTest, GaussianBlurSetsMipCountOnPass) { - int32_t blur_required_mip_count = - GetParam() == PlaygroundBackend::kMetal ? 4 : 1; - Canvas canvas; canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()}); canvas.SaveLayer({}, std::nullopt, @@ -3761,7 +3758,7 @@ TEST_P(AiksTest, GaussianBlurSetsMipCountOnPass) { canvas.Restore(); Picture picture = canvas.EndRecordingAsPicture(); - EXPECT_EQ(blur_required_mip_count, picture.pass->GetRequiredMipCount()); + EXPECT_EQ(4, picture.pass->GetRequiredMipCount()); } TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { From 0d8dfd5f327b7e7501386eccd2899d2bd43949b8 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 17 Jan 2024 15:55:14 -0800 Subject: [PATCH 11/11] updated comment --- impeller/entity/entity_pass.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index a7e3e277e56ce..bff88bb587303 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -257,8 +257,10 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, /// changed for the lifetime of the textures. if (context->GetBackendType() != Context::BackendType::kMetal) { - // TODO(tbd): Implement mip map generation on vulkan. - // TODO(tbd): Implement mip map generation on opengles. + // TODO(https://github.com/flutter/flutter/issues/141495): Implement mip map + // generation on vulkan. + // TODO(https://github.com/flutter/flutter/issues/141732): Implement mip map + // generation on opengles. mip_count = 1; }