From 4eae67752095b00174871d34e9ab71957ec792fc Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 8 Feb 2024 09:44:52 -0800 Subject: [PATCH] [Impeller] Vulkan: Don't fail initialization if stencil-only textures aren't supported. (flutter/engine#50455) Closes https://github.com/flutter/flutter/issues/137108 We don't rely on stencil-only textures anymore in the 2D renderer. --- .../backend/vulkan/capabilities_vk.cc | 5 +- .../backend/vulkan/context_vk_unittests.cc | 54 +++++++++++++++++++ .../backend/vulkan/test/mock_vulkan.cc | 35 ++++++++---- .../backend/vulkan/test/mock_vulkan.h | 16 ++++++ 4 files changed, 96 insertions(+), 14 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc index 08d07918c4..812aae651a 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -254,7 +254,6 @@ static bool PhysicalDeviceSupportsRequiredFormats( const auto has_color_format = HasSuitableColorFormat(device, vk::Format::eB8G8R8A8Unorm); const auto has_stencil_format = - HasSuitableDepthStencilFormat(device, vk::Format::eS8Uint) || HasSuitableDepthStencilFormat(device, vk::Format::eD32SfloatS8Uint) || HasSuitableDepthStencilFormat(device, vk::Format::eD24UnormS8Uint); return has_color_format && has_stencil_format; @@ -351,10 +350,8 @@ bool CapabilitiesVK::SetPhysicalDevice(const vk::PhysicalDevice& device) { if (HasSuitableDepthStencilFormat(device, vk::Format::eS8Uint)) { default_stencil_format_ = PixelFormat::kS8UInt; - } else if (default_stencil_format_ != PixelFormat::kUnknown) { + } else if (default_depth_stencil_format_ != PixelFormat::kUnknown) { default_stencil_format_ = default_depth_stencil_format_; - } else { - return false; } device_properties_ = device.getProperties(); diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc index 53dd23b755..dd5410bb1a 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/testing/testing.h" // IWYU pragma: keep +#include "impeller/base/validation.h" #include "impeller/renderer/backend/vulkan/command_pool_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" @@ -143,5 +144,58 @@ TEST(ContextVKTest, CanCreateContextWithValidationLayers) { ASSERT_TRUE(capabilites_vk->AreValidationsEnabled()); } +// In Impeller's 2D renderer, we no longer use stencil-only formats. They're +// less widely supported than combined depth-stencil formats, so make sure we +// don't fail initialization if we can't find a suitable stencil format. +TEST(CapabilitiesVKTest, ContextInitializesWithNoStencilFormat) { + const std::shared_ptr context = + MockVulkanContextBuilder() + .SetPhysicalDeviceFormatPropertiesCallback( + [](VkPhysicalDevice physicalDevice, VkFormat format, + VkFormatProperties* pFormatProperties) { + if (format == VK_FORMAT_B8G8R8A8_UNORM) { + pFormatProperties->optimalTilingFeatures = + static_cast( + vk::FormatFeatureFlagBits::eColorAttachment); + } else if (format == VK_FORMAT_D32_SFLOAT_S8_UINT) { + pFormatProperties->optimalTilingFeatures = + static_cast( + vk::FormatFeatureFlagBits::eDepthStencilAttachment); + } + // Ignore just the stencil format. + }) + .Build(); + ASSERT_NE(context, nullptr); + const CapabilitiesVK* capabilites_vk = + reinterpret_cast(context->GetCapabilities().get()); + ASSERT_EQ(capabilites_vk->GetDefaultDepthStencilFormat(), + PixelFormat::kD32FloatS8UInt); + ASSERT_EQ(capabilites_vk->GetDefaultStencilFormat(), + PixelFormat::kD32FloatS8UInt); +} + +// Impeller's 2D renderer relies on hardware support for a combined +// depth-stencil format (widely supported). So fail initialization if a suitable +// one couldn't be found. That way we have an opportunity to fallback to +// OpenGLES. +TEST(CapabilitiesVKTest, + ContextFailsInitializationForNoCombinedDepthStencilFormat) { + ScopedValidationDisable disable_validation; + const std::shared_ptr context = + MockVulkanContextBuilder() + .SetPhysicalDeviceFormatPropertiesCallback( + [](VkPhysicalDevice physicalDevice, VkFormat format, + VkFormatProperties* pFormatProperties) { + if (format == VK_FORMAT_B8G8R8A8_UNORM) { + pFormatProperties->optimalTilingFeatures = + static_cast( + vk::FormatFeatureFlagBits::eColorAttachment); + } + // Ignore combined depth-stencil formats. + }) + .Build(); + ASSERT_EQ(context, nullptr); +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index af6ec8a35a..b037b348fe 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -153,19 +153,16 @@ VkResult vkEnumeratePhysicalDevices(VkInstance instance, return VK_SUCCESS; } +static thread_local std::function + g_format_properties_callback; + void vkGetPhysicalDeviceFormatProperties( VkPhysicalDevice physicalDevice, VkFormat format, VkFormatProperties* pFormatProperties) { - if (format == VK_FORMAT_B8G8R8A8_UNORM) { - pFormatProperties->optimalTilingFeatures = - static_cast( - vk::FormatFeatureFlagBits::eColorAttachment); - } else if (format == VK_FORMAT_S8_UINT) { - pFormatProperties->optimalTilingFeatures = - static_cast( - vk::FormatFeatureFlagBits::eDepthStencilAttachment); - } + g_format_properties_callback(physicalDevice, format, pFormatProperties); } void vkGetPhysicalDeviceProperties(VkPhysicalDevice physicalDevice, @@ -813,7 +810,24 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, } // namespace MockVulkanContextBuilder::MockVulkanContextBuilder() - : instance_extensions_({"VK_KHR_surface", "VK_MVK_macos_surface"}) {} + : instance_extensions_({"VK_KHR_surface", "VK_MVK_macos_surface"}), + format_properties_callback_([](VkPhysicalDevice physicalDevice, + VkFormat format, + VkFormatProperties* pFormatProperties) { + if (format == VK_FORMAT_B8G8R8A8_UNORM) { + pFormatProperties->optimalTilingFeatures = + static_cast( + vk::FormatFeatureFlagBits::eColorAttachment); + } else if (format == VK_FORMAT_D32_SFLOAT_S8_UINT) { + pFormatProperties->optimalTilingFeatures = + static_cast( + vk::FormatFeatureFlagBits::eDepthStencilAttachment); + } else if (format == VK_FORMAT_S8_UINT) { + pFormatProperties->optimalTilingFeatures = + static_cast( + vk::FormatFeatureFlagBits::eDepthStencilAttachment); + } + }) {} std::shared_ptr MockVulkanContextBuilder::Build() { auto message_loop = fml::ConcurrentMessageLoop::Create(); @@ -824,6 +838,7 @@ std::shared_ptr MockVulkanContextBuilder::Build() { } g_instance_extensions = instance_extensions_; g_instance_layers = instance_layers_; + g_format_properties_callback = format_properties_callback_; std::shared_ptr result = ContextVK::Create(std::move(settings)); return result; } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.h b/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.h index 78edcd4338..3febfe3bc7 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.h @@ -12,6 +12,7 @@ #include "impeller/base/thread.h" #include "impeller/renderer/backend/vulkan/context_vk.h" +#include "vulkan/vulkan_core.h" #include "vulkan/vulkan_enums.hpp" namespace impeller { @@ -88,10 +89,25 @@ class MockVulkanContextBuilder { return *this; } + /// Set the behavior of vkGetPhysicalDeviceFormatProperties, which needs to + /// respond differently for different formats. + MockVulkanContextBuilder& SetPhysicalDeviceFormatPropertiesCallback( + std::function + format_properties_callback) { + format_properties_callback_ = std::move(format_properties_callback); + return *this; + } + private: std::function settings_callback_; std::vector instance_extensions_; std::vector instance_layers_; + std::function + format_properties_callback_; }; /// @brief Override the image size returned by all swapchain images.