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

Commit 4e46e8f

Browse files
authored
Revert #46131, don't store vkImage, reset vkComandPool synchronously (#46166)
Closes flutter/flutter#135086. Reverts #46131. This PR bundles together 3 changes that removes all validation errors on the `macrobenchmark` apps I could manually find: 1. Reverts #46131, which did not fix the original issue. 2. Added `kResetOnBackgroundThread = false`, which drops performance benefits, but doesn't cause threading issues. 3. Stop tracking `image` for Swapchain presentation (was hitting Vulkan assertion errors about acquired images). /cc @gaaclarke I'd love to talk about how we could run the macrobenchmarks app on CI, with validation errors, after landing.
1 parent a0261dd commit 4e46e8f

File tree

3 files changed

+11
-12
lines changed

3 files changed

+11
-12
lines changed

impeller/renderer/backend/vulkan/command_pool_vk.cc

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ class BackgroundCommandPoolVK final {
5656
std::weak_ptr<CommandPoolRecyclerVK> recycler_;
5757
};
5858

59+
static bool kResetOnBackgroundThread = false;
60+
5961
CommandPoolVK::~CommandPoolVK() {
6062
auto const context = context_.lock();
6163
if (!context) {
@@ -66,10 +68,13 @@ CommandPoolVK::~CommandPoolVK() {
6668
return;
6769
}
6870

69-
UniqueResourceVKT<BackgroundCommandPoolVK> pool(
70-
context->GetResourceManager(),
71-
BackgroundCommandPoolVK(std::move(pool_), std::move(collected_buffers_),
72-
recycler));
71+
auto reset_pool_when_dropped = BackgroundCommandPoolVK(
72+
std::move(pool_), std::move(collected_buffers_), recycler);
73+
74+
if (kResetOnBackgroundThread) {
75+
UniqueResourceVKT<BackgroundCommandPoolVK> pool(
76+
context->GetResourceManager(), std::move(reset_pool_when_dropped));
77+
}
7378
}
7479

7580
// TODO(matanlurey): Return a status_or<> instead of {} when we have one.

impeller/renderer/backend/vulkan/surface_context_vk.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ std::unique_ptr<Surface> SurfaceContextVK::AcquireNextSurface() {
8080
if (auto allocator = parent_->GetResourceAllocator()) {
8181
allocator->DidAcquireSurfaceFrame();
8282
}
83+
parent_->GetCommandPoolRecycler()->Dispose();
8384
return surface;
8485
}
8586

impeller/renderer/backend/vulkan/swapchain_impl_vk.cc

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -424,13 +424,6 @@ bool SwapchainImplVK::Present(const std::shared_ptr<SwapchainImageVK>& image,
424424
//----------------------------------------------------------------------------
425425
/// Transition the image to color-attachment-optimal.
426426
///
427-
428-
// Increment the frame count right before allocating the cmd buffer below to
429-
// force this to use the next frame's pool. This cmd buffer is completely
430-
// untracked, and so we may end up resetting the cmd pool before all buffers
431-
// have been collected.
432-
context.GetCommandPoolRecycler()->Dispose();
433-
434427
sync->final_cmd_buffer = context.CreateCommandBuffer();
435428
if (!sync->final_cmd_buffer) {
436429
return false;
@@ -477,7 +470,7 @@ bool SwapchainImplVK::Present(const std::shared_ptr<SwapchainImageVK>& image,
477470
}
478471
}
479472

480-
auto task = [&, index, image, current_frame = current_frame_] {
473+
auto task = [&, index, current_frame = current_frame_] {
481474
auto context_strong = context_.lock();
482475
if (!context_strong) {
483476
return;

0 commit comments

Comments
 (0)