From 2ff38d22fc8eeea8c79b0ed5bca95c64a25dcb1e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 14 Sep 2023 09:42:51 -0700 Subject: [PATCH 1/7] [Impeller] transitioned mock vulkan context to the builder pattern --- .../vulkan/blit_command_vk_unittests.cc | 8 ++--- .../vulkan/command_encoder_vk_unittests.cc | 4 +-- .../backend/vulkan/context_vk_unittests.cc | 15 ++++---- .../vulkan/pass_bindings_cache_unittests.cc | 8 ++--- .../backend/vulkan/test/mock_vulkan.cc | 7 ++-- .../backend/vulkan/test/mock_vulkan.h | 34 ++++++++++++------- .../vulkan/test/mock_vulkan_unittests.cc | 2 +- 7 files changed, 45 insertions(+), 33 deletions(-) diff --git a/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc b/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc index 44206c2e7ddb5..e1172651353b7 100644 --- a/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc @@ -11,7 +11,7 @@ namespace impeller { namespace testing { TEST(BlitCommandVkTest, BlitCopyTextureToTextureCommandVK) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); auto pool = CommandPoolVK::GetThreadLocal(context.get()); auto encoder = std::make_unique(context)->Create(); BlitCopyTextureToTextureCommandVK cmd; @@ -28,7 +28,7 @@ TEST(BlitCommandVkTest, BlitCopyTextureToTextureCommandVK) { } TEST(BlitCommandVkTest, BlitCopyTextureToBufferCommandVK) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); auto encoder = std::make_unique(context)->Create(); BlitCopyTextureToBufferCommandVK cmd; cmd.source = context->GetResourceAllocator()->CreateTexture({ @@ -44,7 +44,7 @@ TEST(BlitCommandVkTest, BlitCopyTextureToBufferCommandVK) { } TEST(BlitCommandVkTest, BlitCopyBufferToTextureCommandVK) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); auto encoder = std::make_unique(context)->Create(); BlitCopyBufferToTextureCommandVK cmd; cmd.destination = context->GetResourceAllocator()->CreateTexture({ @@ -62,7 +62,7 @@ TEST(BlitCommandVkTest, BlitCopyBufferToTextureCommandVK) { } TEST(BlitCommandVkTest, BlitGenerateMipmapCommandVK) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); auto encoder = std::make_unique(context)->Create(); BlitGenerateMipmapCommandVK cmd; cmd.texture = context->GetResourceAllocator()->CreateTexture({ diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk_unittests.cc b/impeller/renderer/backend/vulkan/command_encoder_vk_unittests.cc index 0fa38fdb82985..2314796f54415 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk_unittests.cc @@ -18,7 +18,7 @@ TEST(CommandEncoderVKTest, DeleteEncoderAfterThreadDies) { // command buffers before it cleans up its command pool. std::shared_ptr> called_functions; { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); called_functions = GetMockVulkanFunctions(context->GetDevice()); std::shared_ptr encoder; std::thread thread([&] { @@ -46,7 +46,7 @@ TEST(CommandEncoderVKTest, CleanupAfterSubmit) { { fml::AutoResetWaitableEvent wait_for_submit; fml::AutoResetWaitableEvent wait_for_thread_join; - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); std::thread thread([&] { CommandEncoderFactoryVK factory(context); std::shared_ptr encoder = factory.Create(); diff --git a/impeller/renderer/backend/vulkan/context_vk_unittests.cc b/impeller/renderer/backend/vulkan/context_vk_unittests.cc index 8a577be6d79bc..48ae3a08be1d4 100644 --- a/impeller/renderer/backend/vulkan/context_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/context_vk_unittests.cc @@ -14,7 +14,7 @@ TEST(ContextVKTest, DeletesCommandPools) { std::weak_ptr weak_context; std::weak_ptr weak_pool; { - std::shared_ptr context = CreateMockVulkanContext(); + std::shared_ptr context = MockVulkanContextBuilder().Build(); std::shared_ptr pool = CommandPoolVK::GetThreadLocal(context.get()); weak_pool = pool; @@ -30,7 +30,7 @@ TEST(ContextVKTest, DeletePipelineAfterContext) { std::shared_ptr> pipeline; std::shared_ptr> functions; { - std::shared_ptr context = CreateMockVulkanContext(); + std::shared_ptr context = MockVulkanContextBuilder().Build(); PipelineDescriptor pipeline_desc; pipeline_desc.SetVertexDescriptor(std::make_shared()); PipelineFuture pipeline_future = @@ -49,7 +49,7 @@ TEST(ContextVKTest, DeleteShaderFunctionAfterContext) { std::shared_ptr shader_function; std::shared_ptr> functions; { - std::shared_ptr context = CreateMockVulkanContext(); + std::shared_ptr context = MockVulkanContextBuilder().Build(); PipelineDescriptor pipeline_desc; pipeline_desc.SetVertexDescriptor(std::make_shared()); std::vector data = {0x03, 0x02, 0x23, 0x07}; @@ -71,7 +71,7 @@ TEST(ContextVKTest, DeletePipelineLibraryAfterContext) { std::shared_ptr pipeline_library; std::shared_ptr> functions; { - std::shared_ptr context = CreateMockVulkanContext(); + std::shared_ptr context = MockVulkanContextBuilder().Build(); PipelineDescriptor pipeline_desc; pipeline_desc.SetVertexDescriptor(std::make_shared()); pipeline_library = context->GetPipelineLibrary(); @@ -86,8 +86,11 @@ TEST(ContextVKTest, DeletePipelineLibraryAfterContext) { TEST(ContextVKTest, CanCreateContextInAbsenceOfValidationLayers) { // The mocked methods don't report the presence of a validation layer but we // explicitly ask for validation. Context creation should continue anyway. - auto context = CreateMockVulkanContext( - [](auto& settings) { settings.enable_validation = true; }); + auto context = MockVulkanContextBuilder() + .SetSettingsCallback([](auto& settings) { + settings.enable_validation = true; + }) + .Build(); ASSERT_NE(context, nullptr); } diff --git a/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc b/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc index bab2a25db7f35..e76213c200493 100644 --- a/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc +++ b/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc @@ -24,7 +24,7 @@ int32_t CountStringViewInstances(const std::vector& strings, } // namespace TEST(PassBindingsCacheTest, bindPipeline) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); PassBindingsCache cache; auto encoder = std::make_unique(context)->Create(); auto buffer = encoder->GetCommandBuffer(); @@ -38,7 +38,7 @@ TEST(PassBindingsCacheTest, bindPipeline) { } TEST(PassBindingsCacheTest, setStencilReference) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); PassBindingsCache cache; auto encoder = std::make_unique(context)->Create(); auto buffer = encoder->GetCommandBuffer(); @@ -53,7 +53,7 @@ TEST(PassBindingsCacheTest, setStencilReference) { } TEST(PassBindingsCacheTest, setScissor) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); PassBindingsCache cache; auto encoder = std::make_unique(context)->Create(); auto buffer = encoder->GetCommandBuffer(); @@ -66,7 +66,7 @@ TEST(PassBindingsCacheTest, setScissor) { } TEST(PassBindingsCacheTest, setViewport) { - auto context = CreateMockVulkanContext(); + auto context = MockVulkanContextBuilder().Build(); PassBindingsCache cache; auto encoder = std::make_unique(context)->Create(); auto buffer = encoder->GetCommandBuffer(); diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index aafb3cf410153..726ecf304abf9 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -513,13 +513,12 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, } // namespace -std::shared_ptr CreateMockVulkanContext( - const std::function& settings_callback) { +std::shared_ptr MockVulkanContextBuilder::Build() { auto message_loop = fml::ConcurrentMessageLoop::Create(); ContextVK::Settings settings; settings.proc_address_callback = GetMockVulkanProcAddress; - if (settings_callback) { - settings_callback(settings); + if (settings_callback_) { + settings_callback_(settings); } return ContextVK::Create(std::move(settings)); } diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.h b/impeller/renderer/backend/vulkan/test/mock_vulkan.h index 2984db473ff09..fababd3997f26 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.h +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.h @@ -16,18 +16,28 @@ namespace testing { std::shared_ptr> GetMockVulkanFunctions( VkDevice device); -//------------------------------------------------------------------------------ -/// @brief Create a Vulkan context with Vulkan functions mocked. The caller -/// is given a chance to tinker on the settings right before a -/// context is created. -/// -/// @param[in] settings_callback The settings callback -/// -/// @return A context if one can be created. -/// -std::shared_ptr CreateMockVulkanContext( - const std::function& settings_callback = - nullptr); +class MockVulkanContextBuilder { + public: + //------------------------------------------------------------------------------ + /// @brief Create a Vulkan context with Vulkan functions mocked. The + /// caller is given a chance to tinker on the settings right + /// before a context is created. + /// + /// @return A context if one can be created. + /// + std::shared_ptr Build(); + + /// A callback that allows the modification of the ContextVK::Settings before + /// the context is made. + MockVulkanContextBuilder& SetSettingsCallback( + const std::function& settings_callback) { + settings_callback_ = settings_callback; + return *this; + } + + private: + std::function settings_callback_; +}; } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc index 05cc7682e004d..de28491e69fed 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc @@ -14,7 +14,7 @@ TEST(MockVulkanContextTest, IsThreadSafe) { // In a typical app, there is a single ContextVK per app, shared b/w threads. // // This test ensures that the (mock) ContextVK is thread-safe. - auto const context = CreateMockVulkanContext(); + auto const context = MockVulkanContextBuilder().Build(); // Spawn two threads, and have them create a CommandPoolVK each. std::thread thread1([&context]() { From 60b871d888583b9707d6a907e862a3e854b25b51 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 14 Sep 2023 10:44:09 -0700 Subject: [PATCH 2/7] made creating a context with validation layers work --- .../backend/vulkan/context_vk_unittests.cc | 12 +++++ .../backend/vulkan/test/mock_vulkan.cc | 54 ++++++++++++++++--- .../backend/vulkan/test/mock_vulkan.h | 16 ++++++ 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/impeller/renderer/backend/vulkan/context_vk_unittests.cc b/impeller/renderer/backend/vulkan/context_vk_unittests.cc index 48ae3a08be1d4..b69cdfdf4bfaf 100644 --- a/impeller/renderer/backend/vulkan/context_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/context_vk_unittests.cc @@ -94,5 +94,17 @@ TEST(ContextVKTest, CanCreateContextInAbsenceOfValidationLayers) { ASSERT_NE(context, nullptr); } +TEST(ContextVKTest, CanCreateContextWithValidationLayers) { + auto context = + MockVulkanContextBuilder() + .SetSettingsCallback( + [](auto& settings) { settings.enable_validation = true; }) + .SetInstanceExtensions( + {"VK_KHR_surface", "VK_MVK_macos_surface", "VK_EXT_debug_utils"}) + .SetInstanceLayers({"VK_LAYER_KHRONOS_validation"}) + .Build(); + ASSERT_NE(context, nullptr); +} + } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 726ecf304abf9..3272fb2bd17c5 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -54,25 +54,39 @@ class MockDevice final { void noop() {} +thread_local std::vector g_instance_extensions; + VkResult vkEnumerateInstanceExtensionProperties( const char* pLayerName, uint32_t* pPropertyCount, VkExtensionProperties* pProperties) { if (!pProperties) { - *pPropertyCount = 2; - + *pPropertyCount = g_instance_extensions.size(); } else { - strcpy(pProperties[0].extensionName, "VK_KHR_surface"); - pProperties[0].specVersion = 0; - strcpy(pProperties[1].extensionName, "VK_MVK_macos_surface"); - pProperties[1].specVersion = 0; + uint32_t count = 0; + for (const std::string& ext : g_instance_extensions) { + strcpy(pProperties[count].extensionName, ext.c_str()); + pProperties[count].specVersion = 0; + count++; + } } return VK_SUCCESS; } +thread_local std::vector g_instance_layers; + VkResult vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, VkLayerProperties* pProperties) { - *pPropertyCount = 0; + if (!pProperties) { + *pPropertyCount = g_instance_layers.size(); + } else { + uint32_t count = 0; + for (const std::string& layer : g_instance_layers) { + strcpy(pProperties[count].layerName, layer.c_str()); + pProperties[count].specVersion = 0; + count++; + } + } return VK_SUCCESS; } @@ -415,6 +429,20 @@ VkResult vkGetFenceStatus(VkDevice device, VkFence fence) { return VK_SUCCESS; } +VkResult vkCreateDebugUtilsMessengerEXT( + VkInstance instance, + const VkDebugUtilsMessengerCreateInfoEXT* pCreateInfo, + const VkAllocationCallbacks* pAllocator, + VkDebugUtilsMessengerEXT* pMessenger) { + return VK_SUCCESS; +} + +VkResult vkSetDebugUtilsObjectNameEXT( + VkDevice device, + const VkDebugUtilsObjectNameInfoEXT* pNameInfo) { + return VK_SUCCESS; +} + PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, const char* pName) { if (strcmp("vkEnumerateInstanceExtensionProperties", pName) == 0) { @@ -507,12 +535,19 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance, return (PFN_vkVoidFunction)vkWaitForFences; } else if (strcmp("vkGetFenceStatus", pName) == 0) { return (PFN_vkVoidFunction)vkGetFenceStatus; + } else if (strcmp("vkCreateDebugUtilsMessengerEXT", pName) == 0) { + return (PFN_vkVoidFunction)vkCreateDebugUtilsMessengerEXT; + } else if (strcmp("vkSetDebugUtilsObjectNameEXT", pName) == 0) { + return (PFN_vkVoidFunction)vkSetDebugUtilsObjectNameEXT; } return noop; } } // namespace +MockVulkanContextBuilder::MockVulkanContextBuilder() + : instance_extensions_({"VK_KHR_surface", "VK_MVK_macos_surface"}) {} + std::shared_ptr MockVulkanContextBuilder::Build() { auto message_loop = fml::ConcurrentMessageLoop::Create(); ContextVK::Settings settings; @@ -520,7 +555,10 @@ std::shared_ptr MockVulkanContextBuilder::Build() { if (settings_callback_) { settings_callback_(settings); } - return ContextVK::Create(std::move(settings)); + g_instance_extensions = instance_extensions_; + g_instance_layers = instance_layers_; + std::shared_ptr result = ContextVK::Create(std::move(settings)); + return result; } std::shared_ptr> GetMockVulkanFunctions( diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.h b/impeller/renderer/backend/vulkan/test/mock_vulkan.h index fababd3997f26..41c82c6da2a90 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.h +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.h @@ -18,6 +18,8 @@ std::shared_ptr> GetMockVulkanFunctions( class MockVulkanContextBuilder { public: + MockVulkanContextBuilder(); + //------------------------------------------------------------------------------ /// @brief Create a Vulkan context with Vulkan functions mocked. The /// caller is given a chance to tinker on the settings right @@ -35,8 +37,22 @@ class MockVulkanContextBuilder { return *this; } + MockVulkanContextBuilder& SetInstanceExtensions( + const std::vector& instance_extensions) { + instance_extensions_ = instance_extensions; + return *this; + } + + MockVulkanContextBuilder& SetInstanceLayers( + const std::vector& instance_layers) { + instance_layers_ = instance_layers; + return *this; + } + private: std::function settings_callback_; + std::vector instance_extensions_; + std::vector instance_layers_; }; } // namespace testing From 67e90553c80f3d69044ab1c7f1285e31dc25018a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 14 Sep 2023 10:50:11 -0700 Subject: [PATCH 3/7] added capabilties asserts --- impeller/renderer/backend/vulkan/context_vk_unittests.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/impeller/renderer/backend/vulkan/context_vk_unittests.cc b/impeller/renderer/backend/vulkan/context_vk_unittests.cc index b69cdfdf4bfaf..11f8b1599bc21 100644 --- a/impeller/renderer/backend/vulkan/context_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/context_vk_unittests.cc @@ -92,6 +92,9 @@ TEST(ContextVKTest, CanCreateContextInAbsenceOfValidationLayers) { }) .Build(); ASSERT_NE(context, nullptr); + const CapabilitiesVK* capabilites_vk = + reinterpret_cast(context->GetCapabilities().get()); + ASSERT_FALSE(capabilites_vk->AreValidationsEnabled()); } TEST(ContextVKTest, CanCreateContextWithValidationLayers) { @@ -104,6 +107,9 @@ TEST(ContextVKTest, CanCreateContextWithValidationLayers) { .SetInstanceLayers({"VK_LAYER_KHRONOS_validation"}) .Build(); ASSERT_NE(context, nullptr); + const CapabilitiesVK* capabilites_vk = + reinterpret_cast(context->GetCapabilities().get()); + ASSERT_TRUE(capabilites_vk->AreValidationsEnabled()); } } // namespace testing From ba4bc1ebaa197ec844e6e1cf9d4949f25574a53d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 14 Sep 2023 12:29:48 -0700 Subject: [PATCH 4/7] moved to fml thread_local --- impeller/renderer/backend/vulkan/test/mock_vulkan.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 3272fb2bd17c5..5067426d83885 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -5,6 +5,7 @@ #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" #include #include "fml/macros.h" +#include "fml/thread_local.h" #include "impeller/base/thread_safety.h" namespace impeller { @@ -54,7 +55,7 @@ class MockDevice final { void noop() {} -thread_local std::vector g_instance_extensions; +FML_THREAD_LOCAL std::vector g_instance_extensions; VkResult vkEnumerateInstanceExtensionProperties( const char* pLayerName, @@ -73,7 +74,7 @@ VkResult vkEnumerateInstanceExtensionProperties( return VK_SUCCESS; } -thread_local std::vector g_instance_layers; +FML_THREAD_LOCAL std::vector g_instance_layers; VkResult vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, VkLayerProperties* pProperties) { From 004f79b7ba8062d235fd277dd392db8353c01502 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 14 Sep 2023 12:50:37 -0700 Subject: [PATCH 5/7] removed strcpy --- impeller/renderer/backend/vulkan/test/mock_vulkan.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 5067426d83885..4d8e1ddbb152b 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" +#include #include #include "fml/macros.h" #include "fml/thread_local.h" @@ -13,6 +14,12 @@ namespace testing { namespace { +void StrcpyChecked(char* dst, const char* src) { + static constexpr size_t kMaxStrSize = 1024; + size_t result = strlcpy(dst, src, kMaxStrSize); + FML_CHECK(result < kMaxStrSize); +} + struct MockCommandBuffer { explicit MockCommandBuffer( std::shared_ptr> called_functions) @@ -66,7 +73,7 @@ VkResult vkEnumerateInstanceExtensionProperties( } else { uint32_t count = 0; for (const std::string& ext : g_instance_extensions) { - strcpy(pProperties[count].extensionName, ext.c_str()); + StrcpyChecked(pProperties[count].extensionName, ext.c_str()); pProperties[count].specVersion = 0; count++; } @@ -83,7 +90,7 @@ VkResult vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, } else { uint32_t count = 0; for (const std::string& layer : g_instance_layers) { - strcpy(pProperties[count].layerName, layer.c_str()); + StrcpyChecked(pProperties[count].layerName, layer.c_str()); pProperties[count].specVersion = 0; count++; } From da1aada63eba01bfbeb55ddda96f010d99e05b7e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 14 Sep 2023 13:07:00 -0700 Subject: [PATCH 6/7] switched to strncpy --- impeller/renderer/backend/vulkan/test/mock_vulkan.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 4d8e1ddbb152b..75cee3d40d8b3 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -14,12 +14,6 @@ namespace testing { namespace { -void StrcpyChecked(char* dst, const char* src) { - static constexpr size_t kMaxStrSize = 1024; - size_t result = strlcpy(dst, src, kMaxStrSize); - FML_CHECK(result < kMaxStrSize); -} - struct MockCommandBuffer { explicit MockCommandBuffer( std::shared_ptr> called_functions) @@ -73,7 +67,8 @@ VkResult vkEnumerateInstanceExtensionProperties( } else { uint32_t count = 0; for (const std::string& ext : g_instance_extensions) { - StrcpyChecked(pProperties[count].extensionName, ext.c_str()); + strncpy(pProperties[count].extensionName, ext.c_str(), + VK_MAX_EXTENSION_NAME_SIZE); pProperties[count].specVersion = 0; count++; } @@ -90,7 +85,8 @@ VkResult vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, } else { uint32_t count = 0; for (const std::string& layer : g_instance_layers) { - StrcpyChecked(pProperties[count].layerName, layer.c_str()); + strncpy(pProperties[count].layerName, layer.c_str(), + VK_MAX_EXTENSION_NAME_SIZE); pProperties[count].specVersion = 0; count++; } From 630f91593d3a833acbad94dbd75e1ec7757bfd23 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 14 Sep 2023 13:10:32 -0700 Subject: [PATCH 7/7] moved to sizeof since its a bit more robust --- impeller/renderer/backend/vulkan/test/mock_vulkan.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index 75cee3d40d8b3..ed311ac017a81 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -68,7 +68,7 @@ VkResult vkEnumerateInstanceExtensionProperties( uint32_t count = 0; for (const std::string& ext : g_instance_extensions) { strncpy(pProperties[count].extensionName, ext.c_str(), - VK_MAX_EXTENSION_NAME_SIZE); + sizeof(VkExtensionProperties::extensionName)); pProperties[count].specVersion = 0; count++; } @@ -86,7 +86,7 @@ VkResult vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, uint32_t count = 0; for (const std::string& layer : g_instance_layers) { strncpy(pProperties[count].layerName, layer.c_str(), - VK_MAX_EXTENSION_NAME_SIZE); + sizeof(VkLayerProperties::layerName)); pProperties[count].specVersion = 0; count++; }