Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit f358bde

Browse files
authored
Added test to assert the vulkan embedder threadsafe vkqueue usage (#45732)
fixes flutter/flutter#133933 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 9a32784 commit f358bde

File tree

7 files changed

+172
-10
lines changed

7 files changed

+172
-10
lines changed

shell/platform/embedder/BUILD.gn

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,10 @@ if (enable_unittests) {
361361
if (test_enable_metal) {
362362
sources += [ "tests/embedder_metal_unittests.mm" ]
363363
}
364+
365+
if (test_enable_vulkan) {
366+
sources += [ "tests/embedder_vk_unittests.cc" ]
367+
}
364368
}
365369

366370
executable("embedder_a11y_unittests") {

shell/platform/embedder/embedder.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,11 @@ typedef struct {
757757
/// The queue family index of the VkQueue supplied in the next field.
758758
uint32_t queue_family_index;
759759
/// VkQueue handle.
760+
/// The queue should not be used without protection from a mutex to make sure
761+
/// it is not used simultaneously with other threads. That mutex should match
762+
/// the one injected via the |get_instance_proc_address_callback|.
763+
/// There is a proposal to remove the need for the mutex at
764+
/// https://github.com/flutter/flutter/issues/134573.
760765
FlutterVulkanQueueHandle queue;
761766
/// The number of instance extensions available for enumerating in the next
762767
/// field.
@@ -780,6 +785,12 @@ typedef struct {
780785
/// For example: VK_KHR_GET_MEMORY_REQUIREMENTS_2_EXTENSION_NAME
781786
const char** enabled_device_extensions;
782787
/// The callback invoked when resolving Vulkan function pointers.
788+
/// At a bare minimum this should be used to swap out any calls that operate
789+
/// on vkQueue's for threadsafe variants that obtain locks for their duration.
790+
/// The functions to swap out are "vkQueueSubmit" and "vkQueueWaitIdle". An
791+
/// example of how to do that can be found in the test
792+
/// "EmbedderTest.CanSwapOutVulkanCalls" unit-test in
793+
/// //shell/platform/embedder/tests/embedder_vk_unittests.cc.
783794
FlutterVulkanInstanceProcAddressCallback get_instance_proc_address_callback;
784795
/// The callback invoked when the engine requests a VkImage from the embedder
785796
/// for rendering the next frame.

shell/platform/embedder/tests/embedder_config_builder.cc

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,18 @@ void EmbedderConfigBuilder::SetMetalRendererConfig(SkISize surface_size) {
205205
#endif
206206
}
207207

208-
void EmbedderConfigBuilder::SetVulkanRendererConfig(SkISize surface_size) {
208+
void EmbedderConfigBuilder::SetVulkanRendererConfig(
209+
SkISize surface_size,
210+
std::optional<FlutterVulkanInstanceProcAddressCallback>
211+
instance_proc_address_callback) {
209212
#ifdef SHELL_ENABLE_VULKAN
210213
renderer_config_.type = FlutterRendererType::kVulkan;
211-
renderer_config_.vulkan = vulkan_renderer_config_;
214+
FlutterVulkanRendererConfig vulkan_renderer_config = vulkan_renderer_config_;
215+
if (instance_proc_address_callback.has_value()) {
216+
vulkan_renderer_config.get_instance_proc_address_callback =
217+
instance_proc_address_callback.value();
218+
}
219+
renderer_config_.vulkan = vulkan_renderer_config;
212220
context_.SetupSurface(surface_size);
213221
#endif
214222
}
@@ -519,13 +527,7 @@ void EmbedderConfigBuilder::InitializeVulkanRendererConfig() {
519527
static_cast<EmbedderTestContextVulkan&>(context_)
520528
.vulkan_context_->device_->GetQueueHandle();
521529
vulkan_renderer_config_.get_instance_proc_address_callback =
522-
[](void* context, FlutterVulkanInstanceHandle instance,
523-
const char* name) -> void* {
524-
auto proc_addr = reinterpret_cast<EmbedderTestContextVulkan*>(context)
525-
->vulkan_context_->vk_->GetInstanceProcAddr(
526-
reinterpret_cast<VkInstance>(instance), name);
527-
return reinterpret_cast<void*>(proc_addr);
528-
};
530+
EmbedderTestContextVulkan::InstanceProcAddr;
529531
vulkan_renderer_config_.get_next_image_callback =
530532
[](void* context,
531533
const FlutterFrameInfo* frame_info) -> FlutterVulkanImage {

shell/platform/embedder/tests/embedder_config_builder.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@ class EmbedderConfigBuilder {
5353

5454
void SetMetalRendererConfig(SkISize surface_size);
5555

56-
void SetVulkanRendererConfig(SkISize surface_size);
56+
void SetVulkanRendererConfig(
57+
SkISize surface_size,
58+
std::optional<FlutterVulkanInstanceProcAddressCallback>
59+
instance_proc_address_callback = {});
5760

5861
// Used to explicitly set an `open_gl.fbo_callback`. Using this method will
5962
// cause your test to fail since the ctor for this class sets

shell/platform/embedder/tests/embedder_test_context_vulkan.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,15 @@ void EmbedderTestContextVulkan::SetupCompositor() {
5858
surface_size_, vulkan_context_->GetGrDirectContext());
5959
}
6060

61+
void* EmbedderTestContextVulkan::InstanceProcAddr(
62+
void* user_data,
63+
FlutterVulkanInstanceHandle instance,
64+
const char* name) {
65+
auto proc_addr = reinterpret_cast<EmbedderTestContextVulkan*>(user_data)
66+
->vulkan_context_->vk_->GetInstanceProcAddr(
67+
reinterpret_cast<VkInstance>(instance), name);
68+
return reinterpret_cast<void*>(proc_addr);
69+
}
70+
6171
} // namespace testing
6272
} // namespace flutter

shell/platform/embedder/tests/embedder_test_context_vulkan.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ class EmbedderTestContextVulkan : public EmbedderTestContext {
3333

3434
bool PresentImage(VkImage image);
3535

36+
static void* InstanceProcAddr(void* user_data,
37+
FlutterVulkanInstanceHandle instance,
38+
const char* name);
39+
3640
private:
3741
std::unique_ptr<TestVulkanSurface> surface_;
3842

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#define FML_USED_ON_EMBEDDER
6+
7+
#include <cstring>
8+
#include <string>
9+
#include <utility>
10+
#include <vector>
11+
12+
#include "embedder.h"
13+
#include "embedder_engine.h"
14+
#include "flutter/fml/synchronization/count_down_latch.h"
15+
#include "flutter/shell/platform/embedder/tests/embedder_config_builder.h"
16+
#include "flutter/shell/platform/embedder/tests/embedder_test.h"
17+
#include "flutter/shell/platform/embedder/tests/embedder_test_context_vulkan.h"
18+
#include "flutter/shell/platform/embedder/tests/embedder_unittests_util.h"
19+
#include "flutter/testing/testing.h"
20+
21+
// CREATE_NATIVE_ENTRY is leaky by design
22+
// NOLINTBEGIN(clang-analyzer-core.StackAddressEscape)
23+
24+
namespace flutter {
25+
namespace testing {
26+
27+
using EmbedderTest = testing::EmbedderTest;
28+
29+
////////////////////////////////////////////////////////////////////////////////
30+
// Notice: Other Vulkan unit tests exist in embedder_gl_unittests.cc.
31+
// See https://github.com/flutter/flutter/issues/134322
32+
////////////////////////////////////////////////////////////////////////////////
33+
34+
namespace {
35+
36+
struct VulkanProcInfo {
37+
decltype(vkGetInstanceProcAddr)* get_instance_proc_addr = nullptr;
38+
decltype(vkGetDeviceProcAddr)* get_device_proc_addr = nullptr;
39+
decltype(vkQueueSubmit)* queue_submit_proc_addr = nullptr;
40+
bool did_call_queue_submit = false;
41+
};
42+
43+
static_assert(std::is_trivially_destructible_v<VulkanProcInfo>);
44+
45+
VulkanProcInfo g_vulkan_proc_info;
46+
47+
VkResult QueueSubmit(VkQueue queue,
48+
uint32_t submitCount,
49+
const VkSubmitInfo* pSubmits,
50+
VkFence fence) {
51+
FML_DCHECK(g_vulkan_proc_info.queue_submit_proc_addr != nullptr);
52+
g_vulkan_proc_info.did_call_queue_submit = true;
53+
return g_vulkan_proc_info.queue_submit_proc_addr(queue, submitCount, pSubmits,
54+
fence);
55+
}
56+
57+
template <size_t N>
58+
int StrcmpFixed(const char* str1, const char (&str2)[N]) {
59+
return strncmp(str1, str2, N - 1);
60+
}
61+
62+
PFN_vkVoidFunction GetDeviceProcAddr(VkDevice device, const char* pName) {
63+
FML_DCHECK(g_vulkan_proc_info.get_device_proc_addr != nullptr);
64+
if (StrcmpFixed(pName, "vkQueueSubmit") == 0) {
65+
g_vulkan_proc_info.queue_submit_proc_addr =
66+
reinterpret_cast<decltype(vkQueueSubmit)*>(
67+
g_vulkan_proc_info.get_device_proc_addr(device, pName));
68+
return reinterpret_cast<PFN_vkVoidFunction>(QueueSubmit);
69+
}
70+
return g_vulkan_proc_info.get_device_proc_addr(device, pName);
71+
}
72+
73+
PFN_vkVoidFunction GetInstanceProcAddr(VkInstance instance, const char* pName) {
74+
FML_DCHECK(g_vulkan_proc_info.get_instance_proc_addr != nullptr);
75+
if (StrcmpFixed(pName, "vkGetDeviceProcAddr") == 0) {
76+
g_vulkan_proc_info.get_device_proc_addr =
77+
reinterpret_cast<decltype(vkGetDeviceProcAddr)*>(
78+
g_vulkan_proc_info.get_instance_proc_addr(instance, pName));
79+
return reinterpret_cast<PFN_vkVoidFunction>(GetDeviceProcAddr);
80+
}
81+
return g_vulkan_proc_info.get_instance_proc_addr(instance, pName);
82+
}
83+
84+
template <typename T, typename U>
85+
struct CheckSameSignature : std::false_type {};
86+
87+
template <typename Ret, typename... Args>
88+
struct CheckSameSignature<Ret(Args...), Ret(Args...)> : std::true_type {};
89+
90+
static_assert(CheckSameSignature<decltype(GetInstanceProcAddr),
91+
decltype(vkGetInstanceProcAddr)>::value);
92+
static_assert(CheckSameSignature<decltype(GetDeviceProcAddr),
93+
decltype(vkGetDeviceProcAddr)>::value);
94+
static_assert(
95+
CheckSameSignature<decltype(QueueSubmit), decltype(vkQueueSubmit)>::value);
96+
} // namespace
97+
98+
TEST_F(EmbedderTest, CanSwapOutVulkanCalls) {
99+
auto& context = GetEmbedderContext(EmbedderTestContextType::kVulkanContext);
100+
fml::AutoResetWaitableEvent latch;
101+
context.AddIsolateCreateCallback([&latch]() { latch.Signal(); });
102+
EmbedderConfigBuilder builder(context);
103+
builder.SetVulkanRendererConfig(
104+
SkISize::Make(1024, 1024),
105+
[](void* user_data, FlutterVulkanInstanceHandle instance,
106+
const char* name) -> void* {
107+
if (StrcmpFixed(name, "vkGetInstanceProcAddr") == 0) {
108+
g_vulkan_proc_info.get_instance_proc_addr =
109+
reinterpret_cast<decltype(vkGetInstanceProcAddr)*>(
110+
EmbedderTestContextVulkan::InstanceProcAddr(user_data,
111+
instance, name));
112+
return reinterpret_cast<void*>(GetInstanceProcAddr);
113+
}
114+
return EmbedderTestContextVulkan::InstanceProcAddr(user_data, instance,
115+
name);
116+
});
117+
auto engine = builder.LaunchEngine();
118+
ASSERT_TRUE(engine.is_valid());
119+
// Wait for the root isolate to launch.
120+
latch.Wait();
121+
engine.reset();
122+
EXPECT_TRUE(g_vulkan_proc_info.did_call_queue_submit);
123+
}
124+
125+
} // namespace testing
126+
} // namespace flutter
127+
128+
// NOLINTEND(clang-analyzer-core.StackAddressEscape)

0 commit comments

Comments
 (0)