From d9ed174607dd119e100947892da8a1c72e7de9d5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 13:29:59 -0700 Subject: [PATCH 01/13] [Impeller] fixed memory management of the device --- .../backend/vulkan/command_encoder_vk.cc | 6 +-- .../backend/vulkan/command_encoder_vk.h | 4 +- .../backend/vulkan/command_pool_vk.cc | 4 +- .../renderer/backend/vulkan/command_pool_vk.h | 2 +- .../renderer/backend/vulkan/context_vk.cc | 6 +-- impeller/renderer/backend/vulkan/context_vk.h | 2 +- .../backend/vulkan/descriptor_pool_vk.cc | 9 +++-- .../backend/vulkan/descriptor_pool_vk.h | 4 +- .../renderer/backend/vulkan/render_pass_vk.cc | 6 +-- .../backend/vulkan/swapchain_image_vk.cc | 4 +- .../backend/vulkan/swapchain_image_vk.h | 4 +- .../backend/vulkan/swapchain_impl_vk.cc | 38 ++++++++++--------- 12 files changed, 47 insertions(+), 42 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index fd6cbec90be2a..e71d63cce5dcd 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -14,7 +14,7 @@ namespace impeller { class TrackedObjectsVK { public: - explicit TrackedObjectsVK(const vk::Device& device, + explicit TrackedObjectsVK(const vk::UniqueDevice* device, const std::shared_ptr& pool) : desc_pool_(device) { if (!pool) { @@ -95,7 +95,7 @@ class TrackedObjectsVK { FML_DISALLOW_COPY_AND_ASSIGN(TrackedObjectsVK); }; -CommandEncoderVK::CommandEncoderVK(vk::Device device, +CommandEncoderVK::CommandEncoderVK(const vk::UniqueDevice* device, const std::shared_ptr& queue, const std::shared_ptr& pool, std::shared_ptr fence_waiter) @@ -137,7 +137,7 @@ bool CommandEncoderVK::Submit() { if (command_buffer.end() != vk::Result::eSuccess) { return false; } - auto [fence_result, fence] = device_.createFenceUnique({}); + auto [fence_result, fence] = (*device_)->createFenceUnique({}); if (fence_result != vk::Result::eSuccess) { return false; } diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.h b/impeller/renderer/backend/vulkan/command_encoder_vk.h index 9a4581cdf9c9b..edf765b305edf 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.h +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.h @@ -69,13 +69,13 @@ class CommandEncoderVK { friend class ::impeller::testing:: BlitCommandVkTest_BlitGenerateMipmapCommandVK_Test; - vk::Device device_ = {}; + const vk::UniqueDevice* device_ = nullptr; std::shared_ptr queue_; std::shared_ptr fence_waiter_; std::shared_ptr tracked_objects_; bool is_valid_ = false; - CommandEncoderVK(vk::Device device, + CommandEncoderVK(const vk::UniqueDevice* device, const std::shared_ptr& queue, const std::shared_ptr& pool, std::shared_ptr fence_waiter); diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index 25b8a86eba563..75854c2b4735b 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -74,7 +74,7 @@ CommandPoolVK::CommandPoolVK(const ContextVK* context) pool_info.queueFamilyIndex = context->GetGraphicsQueue()->GetIndex().family; pool_info.flags = vk::CommandPoolCreateFlagBits::eTransient; - auto pool = context->GetDevice().createCommandPoolUnique(pool_info); + auto pool = (*context->GetDevice())->createCommandPoolUnique(pool_info); if (pool.result != vk::Result::eSuccess) { return; } @@ -113,7 +113,7 @@ vk::UniqueCommandBuffer CommandPoolVK::CreateGraphicsCommandBuffer() { alloc_info.commandPool = graphics_pool_.get(); alloc_info.commandBufferCount = 1u; alloc_info.level = vk::CommandBufferLevel::ePrimary; - auto [result, buffers] = device_.allocateCommandBuffersUnique(alloc_info); + auto [result, buffers] = (*device_)->allocateCommandBuffersUnique(alloc_info); if (result != vk::Result::eSuccess) { return {}; } diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.h b/impeller/renderer/backend/vulkan/command_pool_vk.h index 7399e22686075..a2bdcd04fe9c5 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.h +++ b/impeller/renderer/backend/vulkan/command_pool_vk.h @@ -38,7 +38,7 @@ class CommandPoolVK { private: const std::thread::id owner_id_; - vk::Device device_ = {}; + const vk::UniqueDevice* device_ = nullptr; vk::UniqueCommandPool graphics_pool_; Mutex buffers_to_collect_mutex_; std::set> buffers_to_collect_ diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index d41ccd0a5fa5a..d9b622c1533a1 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -421,8 +421,8 @@ vk::Instance ContextVK::GetInstance() const { return *instance_; } -vk::Device ContextVK::GetDevice() const { - return *device_; +const vk::UniqueDevice* ContextVK::GetDevice() const { + return &device_; } std::unique_ptr ContextVK::AcquireNextSurface() { @@ -489,7 +489,7 @@ std::unique_ptr ContextVK::CreateGraphicsCommandEncoder() return nullptr; } auto encoder = std::unique_ptr(new CommandEncoderVK( - *device_, // + &device_, // queues_.graphics_queue, // tls_pool, // fence_waiter_ // diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index cc1376649bbbd..029474cae2d80 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -106,7 +106,7 @@ class ContextVK final : public Context, public BackendCast { vk::Instance GetInstance() const; - vk::Device GetDevice() const; + const vk::UniqueDevice* GetDevice() const; [[nodiscard]] bool SetWindowSurface(vk::UniqueSurfaceKHR surface); diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc index ac53c63ea6499..2d7ace0c0282b 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -9,11 +9,12 @@ namespace impeller { -DescriptorPoolVK::DescriptorPoolVK(vk::Device device) : device_(device) {} +DescriptorPoolVK::DescriptorPoolVK(const vk::UniqueDevice* device) + : device_(device) {} DescriptorPoolVK::~DescriptorPoolVK() = default; -static vk::UniqueDescriptorPool CreatePool(const vk::Device& device, +static vk::UniqueDescriptorPool CreatePool(const vk::UniqueDevice* device, uint32_t pool_count) { TRACE_EVENT0("impeller", "CreateDescriptorPool"); std::vector pools = { @@ -28,7 +29,7 @@ static vk::UniqueDescriptorPool CreatePool(const vk::Device& device, pool_info.setMaxSets(pools.size() * pool_count); pool_info.setPoolSizes(pools); - auto [result, pool] = device.createDescriptorPoolUnique(pool_info); + auto [result, pool] = (*device)->createDescriptorPoolUnique(pool_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Unable to create a descriptor pool"; } @@ -44,7 +45,7 @@ std::optional DescriptorPoolVK::AllocateDescriptorSet( vk::DescriptorSetAllocateInfo set_info; set_info.setDescriptorPool(pool.value()); set_info.setSetLayouts(layout); - auto [result, sets] = device_.allocateDescriptorSets(set_info); + auto [result, sets] = (*device_)->allocateDescriptorSets(set_info); if (result == vk::Result::eErrorOutOfPoolMemory) { return GrowPool() ? AllocateDescriptorSet(layout) : std::nullopt; } diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h index 4af9c0bc8b13a..54c053286ba81 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h @@ -26,7 +26,7 @@ namespace impeller { /// class DescriptorPoolVK { public: - explicit DescriptorPoolVK(vk::Device device); + explicit DescriptorPoolVK(const vk::UniqueDevice* device); ~DescriptorPoolVK(); @@ -34,7 +34,7 @@ class DescriptorPoolVK { const vk::DescriptorSetLayout& layout); private: - const vk::Device device_; + const vk::UniqueDevice* device_; uint32_t pool_size_ = 31u; std::queue pools_; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 23adb1829ba33..78280df47a0ec 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -127,7 +127,7 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( render_pass_desc.setSubpassCount(1u); auto [result, pass] = - context.GetDevice().createRenderPassUnique(render_pass_desc); + (*context.GetDevice())->createRenderPassUnique(render_pass_desc); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Failed to create render pass: " << vk::to_string(result); return {}; @@ -232,7 +232,7 @@ SharedHandleVK RenderPassVK::CreateVKFramebuffer( fb_info.setAttachments(attachments); auto [result, framebuffer] = - context.GetDevice().createFramebufferUnique(fb_info); + (*context.GetDevice())->createFramebufferUnique(fb_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create framebuffer: " << vk::to_string(result); @@ -390,7 +390,7 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, return false; } - context.GetDevice().updateDescriptorSets(writes, {}); + (*context.GetDevice())->updateDescriptorSets(writes, {}); encoder.GetCommandBuffer().bindDescriptorSets( vk::PipelineBindPoint::eGraphics, // bind point diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc index c87797df753ab..a507ecfa259f2 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc @@ -7,7 +7,7 @@ namespace impeller { SwapchainImageVK::SwapchainImageVK(TextureDescriptor desc, - vk::Device device, + const vk::UniqueDevice* device, vk::Image image) : TextureSourceVK(desc), image_(image) { vk::ImageViewCreateInfo view_info; @@ -20,7 +20,7 @@ SwapchainImageVK::SwapchainImageVK(TextureDescriptor desc, view_info.subresourceRange.levelCount = desc.mip_count; view_info.subresourceRange.layerCount = ToArrayLayerCount(desc.type); - auto [view_result, view] = device.createImageViewUnique(view_info); + auto [view_result, view] = (*device)->createImageViewUnique(view_info); if (view_result != vk::Result::eSuccess) { return; } diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.h b/impeller/renderer/backend/vulkan/swapchain_image_vk.h index db905e834948c..0d51c7cb247fe 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h @@ -14,7 +14,9 @@ namespace impeller { class SwapchainImageVK final : public TextureSourceVK { public: - SwapchainImageVK(TextureDescriptor desc, vk::Device device, vk::Image image); + SwapchainImageVK(TextureDescriptor desc, + const vk::UniqueDevice* device, + vk::Image image); // |TextureSourceVK| ~SwapchainImageVK() override; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 9e4deb598fb11..b409b74d26e44 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -21,11 +21,11 @@ struct FrameSynchronizer { vk::UniqueSemaphore present_ready; bool is_valid = false; - explicit FrameSynchronizer(const vk::Device& device) { - auto acquire_res = device.createFenceUnique( + explicit FrameSynchronizer(const vk::UniqueDevice* device) { + auto acquire_res = (*device)->createFenceUnique( vk::FenceCreateInfo{vk::FenceCreateFlagBits::eSignaled}); - auto render_res = device.createSemaphoreUnique({}); - auto present_res = device.createSemaphoreUnique({}); + auto render_res = (*device)->createSemaphoreUnique({}); + auto present_res = (*device)->createSemaphoreUnique({}); if (acquire_res.result != vk::Result::eSuccess || render_res.result != vk::Result::eSuccess || present_res.result != vk::Result::eSuccess) { @@ -106,7 +106,7 @@ static std::optional ChooseAlphaCompositionMode( static std::optional ChoosePresentQueue( const vk::PhysicalDevice& physical_device, - const vk::Device& device, + const vk::UniqueDevice* device, const vk::SurfaceKHR& surface) { const auto families = physical_device.getQueueFamilyProperties(); for (size_t family_index = 0u; family_index < families.size(); @@ -114,7 +114,7 @@ static std::optional ChoosePresentQueue( auto [result, supported] = physical_device.getSurfaceSupportKHR(family_index, surface); if (result == vk::Result::eSuccess && supported) { - return device.getQueue(family_index, 0u); + return (*device)->getQueue(family_index, 0u); } } return std::nullopt; @@ -207,7 +207,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, swapchain_info.oldSwapchain = old_swapchain; auto [swapchain_result, swapchain] = - vk_context.GetDevice().createSwapchainKHRUnique(swapchain_info); + (*vk_context.GetDevice())->createSwapchainKHRUnique(swapchain_info); if (swapchain_result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create swapchain: " << vk::to_string(swapchain_result); @@ -215,7 +215,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, } auto [images_result, images] = - vk_context.GetDevice().getSwapchainImagesKHR(*swapchain); + (*vk_context.GetDevice())->getSwapchainImagesKHR(*swapchain); if (images_result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not get swapchain images."; return; @@ -242,10 +242,10 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, } ContextVK::SetDebugName( - vk_context.GetDevice(), swapchain_image->GetImage(), + (*vk_context.GetDevice()).get(), swapchain_image->GetImage(), "SwapchainImage" + std::to_string(swapchain_images.size())); ContextVK::SetDebugName( - vk_context.GetDevice(), swapchain_image->GetImageView(), + (*vk_context.GetDevice()).get(), swapchain_image->GetImageView(), "SwapchainImageView" + std::to_string(swapchain_images.size())); swapchain_images.emplace_back(swapchain_image); @@ -284,7 +284,7 @@ bool SwapchainImplVK::IsValid() const { void SwapchainImplVK::WaitIdle() const { if (auto context = context_.lock()) { [[maybe_unused]] auto result = - ContextVK::Cast(*context).GetDevice().waitIdle(); + (*ContextVK::Cast(*context).GetDevice())->waitIdle(); } } @@ -321,7 +321,7 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { //---------------------------------------------------------------------------- /// Wait on the host for the synchronizer fence. /// - if (!sync->WaitForFence(context.GetDevice())) { + if (!sync->WaitForFence(context.GetDevice()->get())) { VALIDATION_LOG << "Could not wait for fence."; return {}; } @@ -329,12 +329,14 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { //---------------------------------------------------------------------------- /// Get the next image index. /// - auto [acq_result, index] = context.GetDevice().acquireNextImageKHR( - *swapchain_, // swapchain - std::numeric_limits::max(), // timeout (nanoseconds) - *sync->render_ready, // signal semaphore - nullptr // fence - ); + auto [acq_result, index] = + (*context.GetDevice()) + ->acquireNextImageKHR( + *swapchain_, // swapchain + std::numeric_limits::max(), // timeout (nanoseconds) + *sync->render_ready, // signal semaphore + nullptr // fence + ); if (acq_result == vk::Result::eSuboptimalKHR || acq_result == vk::Result::eErrorOutOfDateKHR) { From fbe63b93a196c6461f135f45731bbd4776175e3d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 13:43:16 -0700 Subject: [PATCH 02/13] cleaned up initialization of the device --- .../renderer/backend/vulkan/context_vk.cc | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index d9b622c1533a1..63886c8c92a7a 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -273,11 +273,12 @@ void ContextVK::Setup(Settings settings) { device_info.setPEnabledFeatures(&required_features.value()); // Device layers are deprecated and ignored. - auto device = physical_device->createDeviceUnique(device_info); - if (device.result != vk::Result::eSuccess) { + auto device_result = physical_device->createDeviceUnique(device_info); + if (device_result.result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create logical device."; return; } + vk::UniqueDevice device = std::move(device_result.value); if (!caps->SetDevice(physical_device.value())) { VALIDATION_LOG << "Capabilities could not be updated."; @@ -291,7 +292,7 @@ void ContextVK::Setup(Settings settings) { weak_from_this(), // application_info.apiVersion, // physical_device.value(), // - device.value.get(), // + device.get(), // instance.value.get(), // dispatcher.vkGetInstanceProcAddr, // dispatcher.vkGetDeviceProcAddr // @@ -306,7 +307,7 @@ void ContextVK::Setup(Settings settings) { /// Setup the pipeline library. /// auto pipeline_library = std::shared_ptr( - new PipelineLibraryVK(device.value.get(), // + new PipelineLibraryVK(device.get(), // caps, // std::move(settings.cache_directory), // settings.worker_task_runner // @@ -317,11 +318,11 @@ void ContextVK::Setup(Settings settings) { return; } - auto sampler_library = std::shared_ptr( - new SamplerLibraryVK(device.value.get())); + auto sampler_library = + std::shared_ptr(new SamplerLibraryVK(device.get())); auto shader_library = std::shared_ptr( - new ShaderLibraryVK(device.value.get(), // + new ShaderLibraryVK(device.get(), // settings.shader_libraries_data) // ); @@ -334,7 +335,7 @@ void ContextVK::Setup(Settings settings) { /// Create the fence waiter. /// auto fence_waiter = - std::shared_ptr(new FenceWaiterVK(device.value.get())); + std::shared_ptr(new FenceWaiterVK(device.get())); if (!fence_waiter->IsValid()) { VALIDATION_LOG << "Could not create fence waiter."; return; @@ -343,7 +344,7 @@ void ContextVK::Setup(Settings settings) { //---------------------------------------------------------------------------- /// Fetch the queues. /// - QueuesVK queues(device.value.get(), // + QueuesVK queues(device.get(), // graphics_queue.value(), // compute_queue.value(), // transfer_queue.value() // @@ -363,7 +364,7 @@ void ContextVK::Setup(Settings settings) { instance_ = std::move(instance.value); debug_report_ = std::move(debug_report); physical_device_ = physical_device.value(); - device_ = std::move(device.value); + device_ = std::move(device); allocator_ = std::move(allocator); shader_library_ = std::move(shader_library); sampler_library_ = std::move(sampler_library); From a2d047e3eef9b2b3afe3527b8f4933c7fcbcaa93 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 14:06:50 -0700 Subject: [PATCH 03/13] switched to raw device pointers --- .../backend/vulkan/command_encoder_vk.cc | 6 +-- .../backend/vulkan/command_encoder_vk.h | 4 +- .../backend/vulkan/command_pool_vk.cc | 4 +- .../renderer/backend/vulkan/command_pool_vk.h | 2 +- .../renderer/backend/vulkan/context_vk.cc | 8 ++-- impeller/renderer/backend/vulkan/context_vk.h | 8 ++-- .../backend/vulkan/descriptor_pool_vk.cc | 8 ++-- .../backend/vulkan/descriptor_pool_vk.h | 4 +- .../backend/vulkan/pipeline_library_vk.cc | 6 +-- impeller/renderer/backend/vulkan/queue_vk.cc | 6 +-- .../renderer/backend/vulkan/render_pass_vk.cc | 6 +-- .../backend/vulkan/sampler_library_vk.cc | 3 +- .../backend/vulkan/shader_library_vk.cc | 2 +- .../backend/vulkan/swapchain_image_vk.cc | 4 +- .../backend/vulkan/swapchain_image_vk.h | 2 +- .../backend/vulkan/swapchain_impl_vk.cc | 44 +++++++++---------- 16 files changed, 58 insertions(+), 59 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index e71d63cce5dcd..89422e9b46255 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -14,7 +14,7 @@ namespace impeller { class TrackedObjectsVK { public: - explicit TrackedObjectsVK(const vk::UniqueDevice* device, + explicit TrackedObjectsVK(const vk::Device* device, const std::shared_ptr& pool) : desc_pool_(device) { if (!pool) { @@ -95,7 +95,7 @@ class TrackedObjectsVK { FML_DISALLOW_COPY_AND_ASSIGN(TrackedObjectsVK); }; -CommandEncoderVK::CommandEncoderVK(const vk::UniqueDevice* device, +CommandEncoderVK::CommandEncoderVK(const vk::Device* device, const std::shared_ptr& queue, const std::shared_ptr& pool, std::shared_ptr fence_waiter) @@ -137,7 +137,7 @@ bool CommandEncoderVK::Submit() { if (command_buffer.end() != vk::Result::eSuccess) { return false; } - auto [fence_result, fence] = (*device_)->createFenceUnique({}); + auto [fence_result, fence] = device_->createFenceUnique({}); if (fence_result != vk::Result::eSuccess) { return false; } diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.h b/impeller/renderer/backend/vulkan/command_encoder_vk.h index edf765b305edf..0eda06f67c2e8 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.h +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.h @@ -69,13 +69,13 @@ class CommandEncoderVK { friend class ::impeller::testing:: BlitCommandVkTest_BlitGenerateMipmapCommandVK_Test; - const vk::UniqueDevice* device_ = nullptr; + const vk::Device* device_ = nullptr; std::shared_ptr queue_; std::shared_ptr fence_waiter_; std::shared_ptr tracked_objects_; bool is_valid_ = false; - CommandEncoderVK(const vk::UniqueDevice* device, + CommandEncoderVK(const vk::Device* device, const std::shared_ptr& queue, const std::shared_ptr& pool, std::shared_ptr fence_waiter); diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index 75854c2b4735b..a2860b5da631e 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -74,7 +74,7 @@ CommandPoolVK::CommandPoolVK(const ContextVK* context) pool_info.queueFamilyIndex = context->GetGraphicsQueue()->GetIndex().family; pool_info.flags = vk::CommandPoolCreateFlagBits::eTransient; - auto pool = (*context->GetDevice())->createCommandPoolUnique(pool_info); + auto pool = context->GetDevice()->createCommandPoolUnique(pool_info); if (pool.result != vk::Result::eSuccess) { return; } @@ -113,7 +113,7 @@ vk::UniqueCommandBuffer CommandPoolVK::CreateGraphicsCommandBuffer() { alloc_info.commandPool = graphics_pool_.get(); alloc_info.commandBufferCount = 1u; alloc_info.level = vk::CommandBufferLevel::ePrimary; - auto [result, buffers] = (*device_)->allocateCommandBuffersUnique(alloc_info); + auto [result, buffers] = device_->allocateCommandBuffersUnique(alloc_info); if (result != vk::Result::eSuccess) { return {}; } diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.h b/impeller/renderer/backend/vulkan/command_pool_vk.h index a2bdcd04fe9c5..c1c540aad900f 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.h +++ b/impeller/renderer/backend/vulkan/command_pool_vk.h @@ -38,7 +38,7 @@ class CommandPoolVK { private: const std::thread::id owner_id_; - const vk::UniqueDevice* device_ = nullptr; + const vk::Device* device_ = nullptr; vk::UniqueCommandPool graphics_pool_; Mutex buffers_to_collect_mutex_; std::set> buffers_to_collect_ diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 63886c8c92a7a..8cee3d1d2eca1 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -379,7 +379,7 @@ void ContextVK::Setup(Settings settings) { /// Label all the relevant objects. This happens after setup so that the debug /// messengers have had a chance to be setup. /// - SetDebugName(device_.get(), device_.get(), "ImpellerDevice"); + SetDebugName(GetDevice(), device_.get(), "ImpellerDevice"); } // |Context| @@ -422,8 +422,8 @@ vk::Instance ContextVK::GetInstance() const { return *instance_; } -const vk::UniqueDevice* ContextVK::GetDevice() const { - return &device_; +const vk::Device* ContextVK::GetDevice() const { + return &device_.get(); } std::unique_ptr ContextVK::AcquireNextSurface() { @@ -490,7 +490,7 @@ std::unique_ptr ContextVK::CreateGraphicsCommandEncoder() return nullptr; } auto encoder = std::unique_ptr(new CommandEncoderVK( - &device_, // + &device_.get(), // queues_.graphics_queue, // tls_pool, // fence_waiter_ // diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 029474cae2d80..687b08a2e4be3 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -77,11 +77,11 @@ class ContextVK final : public Context, public BackendCast { template bool SetDebugName(T handle, std::string_view label) const { - return SetDebugName(*device_, handle, label); + return SetDebugName(GetDevice(), handle, label); } template - static bool SetDebugName(vk::Device device, + static bool SetDebugName(const vk::Device* device, T handle, std::string_view label) { if (!HasValidationLayers()) { @@ -96,7 +96,7 @@ class ContextVK final : public Context, public BackendCast { info.pObjectName = label.data(); info.objectHandle = reinterpret_cast(c_handle); - if (device.setDebugUtilsObjectNameEXT(info) != vk::Result::eSuccess) { + if (device->setDebugUtilsObjectNameEXT(info) != vk::Result::eSuccess) { VALIDATION_LOG << "Unable to set debug name: " << label; return false; } @@ -106,7 +106,7 @@ class ContextVK final : public Context, public BackendCast { vk::Instance GetInstance() const; - const vk::UniqueDevice* GetDevice() const; + const vk::Device* GetDevice() const; [[nodiscard]] bool SetWindowSurface(vk::UniqueSurfaceKHR surface); diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc index 2d7ace0c0282b..da98179f559b5 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -9,12 +9,12 @@ namespace impeller { -DescriptorPoolVK::DescriptorPoolVK(const vk::UniqueDevice* device) +DescriptorPoolVK::DescriptorPoolVK(const vk::Device* device) : device_(device) {} DescriptorPoolVK::~DescriptorPoolVK() = default; -static vk::UniqueDescriptorPool CreatePool(const vk::UniqueDevice* device, +static vk::UniqueDescriptorPool CreatePool(const vk::Device* device, uint32_t pool_count) { TRACE_EVENT0("impeller", "CreateDescriptorPool"); std::vector pools = { @@ -29,7 +29,7 @@ static vk::UniqueDescriptorPool CreatePool(const vk::UniqueDevice* device, pool_info.setMaxSets(pools.size() * pool_count); pool_info.setPoolSizes(pools); - auto [result, pool] = (*device)->createDescriptorPoolUnique(pool_info); + auto [result, pool] = device->createDescriptorPoolUnique(pool_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Unable to create a descriptor pool"; } @@ -45,7 +45,7 @@ std::optional DescriptorPoolVK::AllocateDescriptorSet( vk::DescriptorSetAllocateInfo set_info; set_info.setDescriptorPool(pool.value()); set_info.setSetLayouts(layout); - auto [result, sets] = (*device_)->allocateDescriptorSets(set_info); + auto [result, sets] = device_->allocateDescriptorSets(set_info); if (result == vk::Result::eErrorOutOfPoolMemory) { return GrowPool() ? AllocateDescriptorSet(layout) : std::nullopt; } diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h index 54c053286ba81..2db7fc5515af6 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h @@ -26,7 +26,7 @@ namespace impeller { /// class DescriptorPoolVK { public: - explicit DescriptorPoolVK(const vk::UniqueDevice* device); + explicit DescriptorPoolVK(const vk::Device* device); ~DescriptorPoolVK(); @@ -34,7 +34,7 @@ class DescriptorPoolVK { const vk::DescriptorSetLayout& layout); private: - const vk::UniqueDevice* device_; + const vk::Device* device_; uint32_t pool_size_ = 31u; std::queue pools_; diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 8717e35a02b34..0a699a85f4c03 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -285,7 +285,7 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( return nullptr; } - ContextVK::SetDebugName(device_, descs_layout.get(), + ContextVK::SetDebugName(&device_, descs_layout.get(), "Descriptor Set Layout " + desc.GetLabel()); //---------------------------------------------------------------------------- @@ -321,9 +321,9 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( return nullptr; } - ContextVK::SetDebugName(device_, *pipeline_layout.value, + ContextVK::SetDebugName(&device_, *pipeline_layout.value, "Pipeline Layout " + desc.GetLabel()); - ContextVK::SetDebugName(device_, *pipeline, "Pipeline " + desc.GetLabel()); + ContextVK::SetDebugName(&device_, *pipeline, "Pipeline " + desc.GetLabel()); return std::make_unique(weak_from_this(), // desc, // diff --git a/impeller/renderer/backend/vulkan/queue_vk.cc b/impeller/renderer/backend/vulkan/queue_vk.cc index a0c82344d1fbb..044b55e35d1c2 100644 --- a/impeller/renderer/backend/vulkan/queue_vk.cc +++ b/impeller/renderer/backend/vulkan/queue_vk.cc @@ -45,14 +45,14 @@ QueuesVK::QueuesVK(const vk::Device& device, // Always setup the graphics queue. graphics_queue = std::make_shared(graphics, vk_graphics); - ContextVK::SetDebugName(device, vk_graphics, "ImpellerGraphicsQ"); + ContextVK::SetDebugName(&device, vk_graphics, "ImpellerGraphicsQ"); // Setup the compute queue if its different from the graphics queue. if (compute == graphics) { compute_queue = graphics_queue; } else { compute_queue = std::make_shared(compute, vk_compute); - ContextVK::SetDebugName(device, vk_compute, "ImpellerComputeQ"); + ContextVK::SetDebugName(&device, vk_compute, "ImpellerComputeQ"); } // Setup the transfer queue if its different from the graphics or compute @@ -63,7 +63,7 @@ QueuesVK::QueuesVK(const vk::Device& device, transfer_queue = compute_queue; } else { transfer_queue = std::make_shared(transfer, vk_transfer); - ContextVK::SetDebugName(device, vk_transfer, "ImpellerTransferQ"); + ContextVK::SetDebugName(&device, vk_transfer, "ImpellerTransferQ"); } } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 78280df47a0ec..befbb43bb00d8 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -127,7 +127,7 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( render_pass_desc.setSubpassCount(1u); auto [result, pass] = - (*context.GetDevice())->createRenderPassUnique(render_pass_desc); + context.GetDevice()->createRenderPassUnique(render_pass_desc); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Failed to create render pass: " << vk::to_string(result); return {}; @@ -232,7 +232,7 @@ SharedHandleVK RenderPassVK::CreateVKFramebuffer( fb_info.setAttachments(attachments); auto [result, framebuffer] = - (*context.GetDevice())->createFramebufferUnique(fb_info); + context.GetDevice()->createFramebufferUnique(fb_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create framebuffer: " << vk::to_string(result); @@ -390,7 +390,7 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, return false; } - (*context.GetDevice())->updateDescriptorSets(writes, {}); + context.GetDevice()->updateDescriptorSets(writes, {}); encoder.GetCommandBuffer().bindDescriptorSets( vk::PipelineBindPoint::eGraphics, // bind point diff --git a/impeller/renderer/backend/vulkan/sampler_library_vk.cc b/impeller/renderer/backend/vulkan/sampler_library_vk.cc index 8d4c82e39cc15..62bb55100915e 100644 --- a/impeller/renderer/backend/vulkan/sampler_library_vk.cc +++ b/impeller/renderer/backend/vulkan/sampler_library_vk.cc @@ -56,7 +56,8 @@ std::shared_ptr SamplerLibraryVK::GetSampler( } if (!desc.label.empty()) { - ContextVK::SetDebugName(device_, sampler->GetSampler(), desc.label.c_str()); + ContextVK::SetDebugName(&device_, sampler->GetSampler(), + desc.label.c_str()); } samplers_[desc] = sampler; diff --git a/impeller/renderer/backend/vulkan/shader_library_vk.cc b/impeller/renderer/backend/vulkan/shader_library_vk.cc index dd2d4b96caa63..c258c5eae9225 100644 --- a/impeller/renderer/backend/vulkan/shader_library_vk.cc +++ b/impeller/renderer/backend/vulkan/shader_library_vk.cc @@ -156,7 +156,7 @@ bool ShaderLibraryVK::RegisterFunction( const auto key_name = VKShaderNameToShaderKeyName(name, stage); vk::UniqueShaderModule shader_module = std::move(module.value); - ContextVK::SetDebugName(device_, *shader_module, "Shader " + name); + ContextVK::SetDebugName(&device_, *shader_module, "Shader " + name); WriterLock lock(functions_mutex_); functions_[ShaderKey{key_name, stage}] = std::shared_ptr( diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc index a507ecfa259f2..c754cd57ccd1b 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc @@ -7,7 +7,7 @@ namespace impeller { SwapchainImageVK::SwapchainImageVK(TextureDescriptor desc, - const vk::UniqueDevice* device, + const vk::Device* device, vk::Image image) : TextureSourceVK(desc), image_(image) { vk::ImageViewCreateInfo view_info; @@ -20,7 +20,7 @@ SwapchainImageVK::SwapchainImageVK(TextureDescriptor desc, view_info.subresourceRange.levelCount = desc.mip_count; view_info.subresourceRange.layerCount = ToArrayLayerCount(desc.type); - auto [view_result, view] = (*device)->createImageViewUnique(view_info); + auto [view_result, view] = device->createImageViewUnique(view_info); if (view_result != vk::Result::eSuccess) { return; } diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.h b/impeller/renderer/backend/vulkan/swapchain_image_vk.h index 0d51c7cb247fe..8693a2f172c8d 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h @@ -15,7 +15,7 @@ namespace impeller { class SwapchainImageVK final : public TextureSourceVK { public: SwapchainImageVK(TextureDescriptor desc, - const vk::UniqueDevice* device, + const vk::Device* device, vk::Image image); // |TextureSourceVK| diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index b409b74d26e44..8036262eb39b3 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -21,11 +21,11 @@ struct FrameSynchronizer { vk::UniqueSemaphore present_ready; bool is_valid = false; - explicit FrameSynchronizer(const vk::UniqueDevice* device) { - auto acquire_res = (*device)->createFenceUnique( + explicit FrameSynchronizer(const vk::Device* device) { + auto acquire_res = device->createFenceUnique( vk::FenceCreateInfo{vk::FenceCreateFlagBits::eSignaled}); - auto render_res = (*device)->createSemaphoreUnique({}); - auto present_res = (*device)->createSemaphoreUnique({}); + auto render_res = device->createSemaphoreUnique({}); + auto present_res = device->createSemaphoreUnique({}); if (acquire_res.result != vk::Result::eSuccess || render_res.result != vk::Result::eSuccess || present_res.result != vk::Result::eSuccess) { @@ -40,8 +40,8 @@ struct FrameSynchronizer { ~FrameSynchronizer() = default; - bool WaitForFence(const vk::Device& device) { - if (auto result = device.waitForFences( + bool WaitForFence(const vk::Device* device) { + if (auto result = device->waitForFences( *acquire, // fence true, // wait all std::numeric_limits::max() // timeout (ns) @@ -50,7 +50,7 @@ struct FrameSynchronizer { VALIDATION_LOG << "Fence wait failed: " << vk::to_string(result); return false; } - if (auto result = device.resetFences(*acquire); + if (auto result = device->resetFences(*acquire); result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not reset fence: " << vk::to_string(result); return false; @@ -106,7 +106,7 @@ static std::optional ChooseAlphaCompositionMode( static std::optional ChoosePresentQueue( const vk::PhysicalDevice& physical_device, - const vk::UniqueDevice* device, + const vk::Device* device, const vk::SurfaceKHR& surface) { const auto families = physical_device.getQueueFamilyProperties(); for (size_t family_index = 0u; family_index < families.size(); @@ -114,7 +114,7 @@ static std::optional ChoosePresentQueue( auto [result, supported] = physical_device.getSurfaceSupportKHR(family_index, surface); if (result == vk::Result::eSuccess && supported) { - return (*device)->getQueue(family_index, 0u); + return device->getQueue(family_index, 0u); } } return std::nullopt; @@ -207,7 +207,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, swapchain_info.oldSwapchain = old_swapchain; auto [swapchain_result, swapchain] = - (*vk_context.GetDevice())->createSwapchainKHRUnique(swapchain_info); + vk_context.GetDevice()->createSwapchainKHRUnique(swapchain_info); if (swapchain_result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create swapchain: " << vk::to_string(swapchain_result); @@ -215,7 +215,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, } auto [images_result, images] = - (*vk_context.GetDevice())->getSwapchainImagesKHR(*swapchain); + vk_context.GetDevice()->getSwapchainImagesKHR(*swapchain); if (images_result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not get swapchain images."; return; @@ -242,10 +242,10 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, } ContextVK::SetDebugName( - (*vk_context.GetDevice()).get(), swapchain_image->GetImage(), + vk_context.GetDevice(), swapchain_image->GetImage(), "SwapchainImage" + std::to_string(swapchain_images.size())); ContextVK::SetDebugName( - (*vk_context.GetDevice()).get(), swapchain_image->GetImageView(), + vk_context.GetDevice(), swapchain_image->GetImageView(), "SwapchainImageView" + std::to_string(swapchain_images.size())); swapchain_images.emplace_back(swapchain_image); @@ -284,7 +284,7 @@ bool SwapchainImplVK::IsValid() const { void SwapchainImplVK::WaitIdle() const { if (auto context = context_.lock()) { [[maybe_unused]] auto result = - (*ContextVK::Cast(*context).GetDevice())->waitIdle(); + ContextVK::Cast(*context).GetDevice()->waitIdle(); } } @@ -321,7 +321,7 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { //---------------------------------------------------------------------------- /// Wait on the host for the synchronizer fence. /// - if (!sync->WaitForFence(context.GetDevice()->get())) { + if (!sync->WaitForFence(context.GetDevice())) { VALIDATION_LOG << "Could not wait for fence."; return {}; } @@ -329,14 +329,12 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { //---------------------------------------------------------------------------- /// Get the next image index. /// - auto [acq_result, index] = - (*context.GetDevice()) - ->acquireNextImageKHR( - *swapchain_, // swapchain - std::numeric_limits::max(), // timeout (nanoseconds) - *sync->render_ready, // signal semaphore - nullptr // fence - ); + auto [acq_result, index] = context.GetDevice()->acquireNextImageKHR( + *swapchain_, // swapchain + std::numeric_limits::max(), // timeout (nanoseconds) + *sync->render_ready, // signal semaphore + nullptr // fence + ); if (acq_result == vk::Result::eSuboptimalKHR || acq_result == vk::Result::eErrorOutOfDateKHR) { From 00a190696b1f5da774c0f924cb59e83335403877 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 15:13:30 -0700 Subject: [PATCH 04/13] introduced device holder --- impeller/renderer/backend/metal/context_mtl.h | 3 ++- .../renderer/backend/metal/surface_mtl.mm | 6 ++--- .../renderer/backend/vulkan/context_vk.cc | 15 +++++------ impeller/renderer/backend/vulkan/context_vk.h | 9 +++++-- .../renderer/backend/vulkan/device_holder.h | 17 ++++++++++++ .../backend/vulkan/pipeline_cache_vk.cc | 27 ++++++++++++++----- .../backend/vulkan/pipeline_cache_vk.h | 5 ++-- .../backend/vulkan/pipeline_library_vk.cc | 27 ++++++++++++------- .../backend/vulkan/pipeline_library_vk.h | 4 +-- impeller/renderer/context.h | 2 +- 10 files changed, 80 insertions(+), 35 deletions(-) create mode 100644 impeller/renderer/backend/vulkan/device_holder.h diff --git a/impeller/renderer/backend/metal/context_mtl.h b/impeller/renderer/backend/metal/context_mtl.h index c0e86de607526..f6a5bfd25232e 100644 --- a/impeller/renderer/backend/metal/context_mtl.h +++ b/impeller/renderer/backend/metal/context_mtl.h @@ -22,7 +22,8 @@ namespace impeller { class ContextMTL final : public Context, - public BackendCast { + public BackendCast, + public std::enable_shared_from_this { public: static std::shared_ptr Create( const std::vector& shader_library_paths); diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index 4bf510cdfe304..55e88039f1c51 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -112,9 +112,9 @@ render_target_desc.SetColorAttachment(color0, 0u); // The constructor is private. So make_unique may not be used. - return std::unique_ptr( - new SurfaceMTL(context->weak_from_this(), render_target_desc, resolve_tex, - drawable, requires_blit, clip_rect)); + return std::unique_ptr(new SurfaceMTL(context, render_target_desc, + resolve_tex, drawable, + requires_blit, clip_rect)); } SurfaceMTL::SurfaceMTL(const std::weak_ptr& context, diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 8cee3d1d2eca1..cc0274a340256 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -278,7 +278,7 @@ void ContextVK::Setup(Settings settings) { VALIDATION_LOG << "Could not create logical device."; return; } - vk::UniqueDevice device = std::move(device_result.value); + device_ = std::move(device_result.value); if (!caps->SetDevice(physical_device.value())) { VALIDATION_LOG << "Capabilities could not be updated."; @@ -292,7 +292,7 @@ void ContextVK::Setup(Settings settings) { weak_from_this(), // application_info.apiVersion, // physical_device.value(), // - device.get(), // + device_.get(), // instance.value.get(), // dispatcher.vkGetInstanceProcAddr, // dispatcher.vkGetDeviceProcAddr // @@ -307,7 +307,7 @@ void ContextVK::Setup(Settings settings) { /// Setup the pipeline library. /// auto pipeline_library = std::shared_ptr( - new PipelineLibraryVK(device.get(), // + new PipelineLibraryVK(weak_from_this(), // caps, // std::move(settings.cache_directory), // settings.worker_task_runner // @@ -319,10 +319,10 @@ void ContextVK::Setup(Settings settings) { } auto sampler_library = - std::shared_ptr(new SamplerLibraryVK(device.get())); + std::shared_ptr(new SamplerLibraryVK(device_.get())); auto shader_library = std::shared_ptr( - new ShaderLibraryVK(device.get(), // + new ShaderLibraryVK(device_.get(), // settings.shader_libraries_data) // ); @@ -335,7 +335,7 @@ void ContextVK::Setup(Settings settings) { /// Create the fence waiter. /// auto fence_waiter = - std::shared_ptr(new FenceWaiterVK(device.get())); + std::shared_ptr(new FenceWaiterVK(device_.get())); if (!fence_waiter->IsValid()) { VALIDATION_LOG << "Could not create fence waiter."; return; @@ -344,7 +344,7 @@ void ContextVK::Setup(Settings settings) { //---------------------------------------------------------------------------- /// Fetch the queues. /// - QueuesVK queues(device.get(), // + QueuesVK queues(device_.get(), // graphics_queue.value(), // compute_queue.value(), // transfer_queue.value() // @@ -364,7 +364,6 @@ void ContextVK::Setup(Settings settings) { instance_ = std::move(instance.value); debug_report_ = std::move(debug_report); physical_device_ = physical_device.value(); - device_ = std::move(device); allocator_ = std::move(allocator); shader_library_ = std::move(shader_library); sampler_library_ = std::move(sampler_library); diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 687b08a2e4be3..0d51d4aad5103 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -12,6 +12,7 @@ #include "flutter/fml/unique_fd.h" #include "impeller/base/backend_cast.h" #include "impeller/core/formats.h" +#include "impeller/renderer/backend/vulkan/device_holder.h" #include "impeller/renderer/backend/vulkan/pipeline_library_vk.h" #include "impeller/renderer/backend/vulkan/queue_vk.h" #include "impeller/renderer/backend/vulkan/sampler_library_vk.h" @@ -30,7 +31,10 @@ class CommandEncoderVK; class DebugReportVK; class FenceWaiterVK; -class ContextVK final : public Context, public BackendCast { +class ContextVK final : public Context, + public BackendCast, + public DeviceHolder, + public std::enable_shared_from_this { public: struct Settings { PFN_vkGetInstanceProcAddr proc_address_callback = nullptr; @@ -106,7 +110,8 @@ class ContextVK final : public Context, public BackendCast { vk::Instance GetInstance() const; - const vk::Device* GetDevice() const; + // |DeviceHolder| + const vk::Device* GetDevice() const override; [[nodiscard]] bool SetWindowSurface(vk::UniqueSurfaceKHR surface); diff --git a/impeller/renderer/backend/vulkan/device_holder.h b/impeller/renderer/backend/vulkan/device_holder.h new file mode 100644 index 0000000000000..cdfb7fe478c2b --- /dev/null +++ b/impeller/renderer/backend/vulkan/device_holder.h @@ -0,0 +1,17 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include "impeller/renderer/backend/vulkan/vk.h" + +namespace impeller { + +class DeviceHolder { + public: + virtual ~DeviceHolder() = default; + virtual const vk::Device* GetDevice() const = 0; +}; + +} diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc index 310c2fbb763ec..ba703afa99b0f 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc @@ -53,12 +53,13 @@ static std::unique_ptr OpenCacheFile( } PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, - vk::Device device, + std::weak_ptr device, fml::UniqueFD cache_directory) : caps_(std::move(caps)), device_(device), cache_directory_(std::move(cache_directory)) { - if (!caps_ || !device_) { + std::shared_ptr strong_device = device_.lock(); + if (!caps_ || !strong_device) { return; } @@ -80,7 +81,8 @@ PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, cache_info.pInitialData = existing_cache_data->GetMapping(); } - auto [result, existing_cache] = device_.createPipelineCacheUnique(cache_info); + auto [result, existing_cache] = + strong_device->GetDevice()->createPipelineCacheUnique(cache_info); if (result == vk::Result::eSuccess) { cache_ = std::move(existing_cache); @@ -92,7 +94,8 @@ PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, << vk::to_string(result) << ". Starting with a fresh cache."; cache_info.pInitialData = nullptr; cache_info.initialDataSize = 0u; - auto [result2, new_cache] = device_.createPipelineCacheUnique(cache_info); + auto [result2, new_cache] = + strong_device->GetDevice()->createPipelineCacheUnique(cache_info); if (result2 == vk::Result::eSuccess) { cache_ = std::move(new_cache); } else { @@ -112,8 +115,14 @@ bool PipelineCacheVK::IsValid() const { vk::UniquePipeline PipelineCacheVK::CreatePipeline( const vk::GraphicsPipelineCreateInfo& info) { + std::shared_ptr strong_device = device_.lock(); + if (!strong_device) { + return {}; + } + Lock lock(cache_mutex_); - auto [result, pipeline] = device_.createGraphicsPipelineUnique(*cache_, info); + auto [result, pipeline] = + strong_device->GetDevice()->createGraphicsPipelineUnique(*cache_, info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create graphics pipeline: " << vk::to_string(result); @@ -122,11 +131,17 @@ vk::UniquePipeline PipelineCacheVK::CreatePipeline( } std::shared_ptr PipelineCacheVK::CopyPipelineCacheData() const { + std::shared_ptr strong_device = device_.lock(); + if (!strong_device) { + return nullptr; + } + if (!IsValid()) { return nullptr; } Lock lock(cache_mutex_); - auto [result, data] = device_.getPipelineCacheData(*cache_); + auto [result, data] = + strong_device->GetDevice()->getPipelineCacheData(*cache_); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not get pipeline cache data to persist."; return nullptr; diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h index b6713ebb6cc10..a146e24dc24a6 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h @@ -8,6 +8,7 @@ #include "flutter/fml/macros.h" #include "impeller/base/thread.h" #include "impeller/renderer/backend/vulkan/capabilities_vk.h" +#include "impeller/renderer/backend/vulkan/device_holder.h" #include "impeller/renderer/backend/vulkan/vk.h" namespace impeller { @@ -15,7 +16,7 @@ namespace impeller { class PipelineCacheVK { public: explicit PipelineCacheVK(std::shared_ptr caps, - vk::Device device, + std::weak_ptr device, fml::UniqueFD cache_directory); ~PipelineCacheVK(); @@ -28,7 +29,7 @@ class PipelineCacheVK { private: const std::shared_ptr caps_; - const vk::Device device_; + std::weak_ptr device_; const fml::UniqueFD cache_directory_; mutable Mutex cache_mutex_; vk::UniquePipelineCache cache_ IPLR_GUARDED_BY(cache_mutex_); diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 0a699a85f4c03..fb9f8dbb6a9e2 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -19,7 +19,7 @@ namespace impeller { PipelineLibraryVK::PipelineLibraryVK( - const vk::Device& device, + std::weak_ptr device, std::shared_ptr caps, fml::UniqueFD cache_directory, std::shared_ptr worker_task_runner) @@ -71,7 +71,7 @@ static vk::AttachmentDescription CreatePlaceholderAttachmentDescription( /// spec: /// https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/chap8.html#renderpass-compatibility /// -static vk::UniqueRenderPass CreateRenderPass(const vk::Device& device, +static vk::UniqueRenderPass CreateRenderPass(const vk::Device* device, const PipelineDescriptor& desc) { std::vector attachments; @@ -117,7 +117,7 @@ static vk::UniqueRenderPass CreateRenderPass(const vk::Device& device, render_pass_desc.setPSubpasses(&subpass_desc); render_pass_desc.setSubpassCount(1u); - auto [result, pass] = device.createRenderPassUnique(render_pass_desc); + auto [result, pass] = device->createRenderPassUnique(render_pass_desc); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Failed to create render pass for pipeline '" << desc.GetLabel() << "'. Error: " << vk::to_string(result); @@ -225,7 +225,12 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( blend_state.setAttachments(attachment_blend_state); pipeline_info.setPColorBlendState(&blend_state); - auto render_pass = CreateRenderPass(device_, desc); + std::shared_ptr strong_device = device_.lock(); + if (!strong_device) { + return nullptr; + } + + auto render_pass = CreateRenderPass(strong_device->GetDevice(), desc); if (render_pass) { pipeline_info.setBasePipelineHandle(VK_NULL_HANDLE); pipeline_info.setSubpass(0); @@ -279,13 +284,14 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( descs_layout_info.setBindings(desc_bindings); auto [descs_result, descs_layout] = - device_.createDescriptorSetLayoutUnique(descs_layout_info); + strong_device->GetDevice()->createDescriptorSetLayoutUnique( + descs_layout_info); if (descs_result != vk::Result::eSuccess) { VALIDATION_LOG << "unable to create uniform descriptors"; return nullptr; } - ContextVK::SetDebugName(&device_, descs_layout.get(), + ContextVK::SetDebugName(strong_device->GetDevice(), descs_layout.get(), "Descriptor Set Layout " + desc.GetLabel()); //---------------------------------------------------------------------------- @@ -293,8 +299,8 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( /// vk::PipelineLayoutCreateInfo pipeline_layout_info; pipeline_layout_info.setSetLayouts(descs_layout.get()); - auto pipeline_layout = - device_.createPipelineLayoutUnique(pipeline_layout_info); + auto pipeline_layout = strong_device->GetDevice()->createPipelineLayoutUnique( + pipeline_layout_info); if (pipeline_layout.result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create pipeline layout for pipeline " << desc.GetLabel() << ": " @@ -321,9 +327,10 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( return nullptr; } - ContextVK::SetDebugName(&device_, *pipeline_layout.value, + ContextVK::SetDebugName(strong_device->GetDevice(), *pipeline_layout.value, "Pipeline Layout " + desc.GetLabel()); - ContextVK::SetDebugName(&device_, *pipeline, "Pipeline " + desc.GetLabel()); + ContextVK::SetDebugName(strong_device->GetDevice(), *pipeline, + "Pipeline " + desc.GetLabel()); return std::make_unique(weak_from_this(), // desc, // diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/impeller/renderer/backend/vulkan/pipeline_library_vk.h index 3e0c84ea4b70c..fb01b69dd424e 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.h @@ -34,7 +34,7 @@ class PipelineLibraryVK final private: friend ContextVK; - vk::Device device_; + std::weak_ptr device_; std::shared_ptr pso_cache_; std::shared_ptr worker_task_runner_; Mutex pipelines_mutex_; @@ -43,7 +43,7 @@ class PipelineLibraryVK final bool is_valid_ = false; PipelineLibraryVK( - const vk::Device& device, + std::weak_ptr device, std::shared_ptr caps, fml::UniqueFD cache_directory, std::shared_ptr worker_task_runner); diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index f270b0033eeb3..36bf48c6ed16f 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -19,7 +19,7 @@ class CommandBuffer; class PipelineLibrary; class Allocator; -class Context : public std::enable_shared_from_this { +class Context { public: virtual ~Context(); From a93423ef10c9229b9399b19e5b4cc6f3e6519243 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 15:53:03 -0700 Subject: [PATCH 05/13] put the device_ setter back where it was --- impeller/renderer/backend/vulkan/context_vk.cc | 14 ++++++++------ .../renderer/backend/vulkan/device_holder.h | 2 +- .../backend/vulkan/pipeline_cache_vk.cc | 18 ++++++++---------- .../backend/vulkan/pipeline_cache_vk.h | 5 +++-- .../backend/vulkan/pipeline_library_vk.cc | 8 +++++--- .../backend/vulkan/pipeline_library_vk.h | 5 +++-- 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index cc0274a340256..186030969c617 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -278,7 +278,7 @@ void ContextVK::Setup(Settings settings) { VALIDATION_LOG << "Could not create logical device."; return; } - device_ = std::move(device_result.value); + vk::UniqueDevice device = std::move(device_result.value); if (!caps->SetDevice(physical_device.value())) { VALIDATION_LOG << "Capabilities could not be updated."; @@ -292,7 +292,7 @@ void ContextVK::Setup(Settings settings) { weak_from_this(), // application_info.apiVersion, // physical_device.value(), // - device_.get(), // + device.get(), // instance.value.get(), // dispatcher.vkGetInstanceProcAddr, // dispatcher.vkGetDeviceProcAddr // @@ -308,6 +308,7 @@ void ContextVK::Setup(Settings settings) { /// auto pipeline_library = std::shared_ptr( new PipelineLibraryVK(weak_from_this(), // + &device.get(), // caps, // std::move(settings.cache_directory), // settings.worker_task_runner // @@ -319,10 +320,10 @@ void ContextVK::Setup(Settings settings) { } auto sampler_library = - std::shared_ptr(new SamplerLibraryVK(device_.get())); + std::shared_ptr(new SamplerLibraryVK(device.get())); auto shader_library = std::shared_ptr( - new ShaderLibraryVK(device_.get(), // + new ShaderLibraryVK(device.get(), // settings.shader_libraries_data) // ); @@ -335,7 +336,7 @@ void ContextVK::Setup(Settings settings) { /// Create the fence waiter. /// auto fence_waiter = - std::shared_ptr(new FenceWaiterVK(device_.get())); + std::shared_ptr(new FenceWaiterVK(device.get())); if (!fence_waiter->IsValid()) { VALIDATION_LOG << "Could not create fence waiter."; return; @@ -344,7 +345,7 @@ void ContextVK::Setup(Settings settings) { //---------------------------------------------------------------------------- /// Fetch the queues. /// - QueuesVK queues(device_.get(), // + QueuesVK queues(device.get(), // graphics_queue.value(), // compute_queue.value(), // transfer_queue.value() // @@ -364,6 +365,7 @@ void ContextVK::Setup(Settings settings) { instance_ = std::move(instance.value); debug_report_ = std::move(debug_report); physical_device_ = physical_device.value(); + device_ = std::move(device); allocator_ = std::move(allocator); shader_library_ = std::move(shader_library); sampler_library_ = std::move(sampler_library); diff --git a/impeller/renderer/backend/vulkan/device_holder.h b/impeller/renderer/backend/vulkan/device_holder.h index cdfb7fe478c2b..9a127472eb0c6 100644 --- a/impeller/renderer/backend/vulkan/device_holder.h +++ b/impeller/renderer/backend/vulkan/device_holder.h @@ -14,4 +14,4 @@ class DeviceHolder { virtual const vk::Device* GetDevice() const = 0; }; -} +} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc index ba703afa99b0f..159e12af44bd0 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc @@ -53,13 +53,13 @@ static std::unique_ptr OpenCacheFile( } PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, - std::weak_ptr device, + std::weak_ptr device_holder, + const vk::Device* device, fml::UniqueFD cache_directory) : caps_(std::move(caps)), - device_(device), + device_holder_(device_holder), cache_directory_(std::move(cache_directory)) { - std::shared_ptr strong_device = device_.lock(); - if (!caps_ || !strong_device) { + if (!caps_ || !device || !*device) { return; } @@ -81,8 +81,7 @@ PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, cache_info.pInitialData = existing_cache_data->GetMapping(); } - auto [result, existing_cache] = - strong_device->GetDevice()->createPipelineCacheUnique(cache_info); + auto [result, existing_cache] = device->createPipelineCacheUnique(cache_info); if (result == vk::Result::eSuccess) { cache_ = std::move(existing_cache); @@ -94,8 +93,7 @@ PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, << vk::to_string(result) << ". Starting with a fresh cache."; cache_info.pInitialData = nullptr; cache_info.initialDataSize = 0u; - auto [result2, new_cache] = - strong_device->GetDevice()->createPipelineCacheUnique(cache_info); + auto [result2, new_cache] = device->createPipelineCacheUnique(cache_info); if (result2 == vk::Result::eSuccess) { cache_ = std::move(new_cache); } else { @@ -115,7 +113,7 @@ bool PipelineCacheVK::IsValid() const { vk::UniquePipeline PipelineCacheVK::CreatePipeline( const vk::GraphicsPipelineCreateInfo& info) { - std::shared_ptr strong_device = device_.lock(); + std::shared_ptr strong_device = device_holder_.lock(); if (!strong_device) { return {}; } @@ -131,7 +129,7 @@ vk::UniquePipeline PipelineCacheVK::CreatePipeline( } std::shared_ptr PipelineCacheVK::CopyPipelineCacheData() const { - std::shared_ptr strong_device = device_.lock(); + std::shared_ptr strong_device = device_holder_.lock(); if (!strong_device) { return nullptr; } diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h index a146e24dc24a6..9c0ccefb67db8 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h @@ -16,7 +16,8 @@ namespace impeller { class PipelineCacheVK { public: explicit PipelineCacheVK(std::shared_ptr caps, - std::weak_ptr device, + std::weak_ptr device_holder, + const vk::Device* device, fml::UniqueFD cache_directory); ~PipelineCacheVK(); @@ -29,7 +30,7 @@ class PipelineCacheVK { private: const std::shared_ptr caps_; - std::weak_ptr device_; + std::weak_ptr device_holder_; const fml::UniqueFD cache_directory_; mutable Mutex cache_mutex_; vk::UniquePipelineCache cache_ IPLR_GUARDED_BY(cache_mutex_); diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index fb9f8dbb6a9e2..6e62e30c3cb9d 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -19,12 +19,14 @@ namespace impeller { PipelineLibraryVK::PipelineLibraryVK( - std::weak_ptr device, + std::weak_ptr device_holder, + const vk::Device* device, std::shared_ptr caps, fml::UniqueFD cache_directory, std::shared_ptr worker_task_runner) - : device_(device), + : device_holder_(device_holder), pso_cache_(std::make_shared(std::move(caps), + device_holder, device, std::move(cache_directory))), worker_task_runner_(std::move(worker_task_runner)) { @@ -225,7 +227,7 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( blend_state.setAttachments(attachment_blend_state); pipeline_info.setPColorBlendState(&blend_state); - std::shared_ptr strong_device = device_.lock(); + std::shared_ptr strong_device = device_holder_.lock(); if (!strong_device) { return nullptr; } diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/impeller/renderer/backend/vulkan/pipeline_library_vk.h index fb01b69dd424e..cde216610fbfc 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.h @@ -34,7 +34,7 @@ class PipelineLibraryVK final private: friend ContextVK; - std::weak_ptr device_; + std::weak_ptr device_holder_; std::shared_ptr pso_cache_; std::shared_ptr worker_task_runner_; Mutex pipelines_mutex_; @@ -43,7 +43,8 @@ class PipelineLibraryVK final bool is_valid_ = false; PipelineLibraryVK( - std::weak_ptr device, + std::weak_ptr device_holder, + const vk::Device* device, std::shared_ptr caps, fml::UniqueFD cache_directory, std::shared_ptr worker_task_runner); From bd8537cc2c635640446c2ae77236efeee476c0a5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 17:02:59 -0700 Subject: [PATCH 06/13] migrated command pool --- impeller/renderer/backend/vulkan/command_pool_vk.cc | 9 +++++++-- impeller/renderer/backend/vulkan/command_pool_vk.h | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index a2860b5da631e..aa36fb1a7fd8a 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -79,7 +79,7 @@ CommandPoolVK::CommandPoolVK(const ContextVK* context) return; } - device_ = context->GetDevice(); + device_holder_ = context->weak_from_this(); graphics_pool_ = std::move(pool.value); is_valid_ = true; } @@ -102,6 +102,10 @@ vk::CommandPool CommandPoolVK::GetGraphicsCommandPool() const { } vk::UniqueCommandBuffer CommandPoolVK::CreateGraphicsCommandBuffer() { + std::shared_ptr strong_device = device_holder_.lock(); + if (!strong_device) { + return {}; + } if (std::this_thread::get_id() != owner_id_) { return {}; } @@ -113,7 +117,8 @@ vk::UniqueCommandBuffer CommandPoolVK::CreateGraphicsCommandBuffer() { alloc_info.commandPool = graphics_pool_.get(); alloc_info.commandBufferCount = 1u; alloc_info.level = vk::CommandBufferLevel::ePrimary; - auto [result, buffers] = device_->allocateCommandBuffersUnique(alloc_info); + auto [result, buffers] = + strong_device->GetDevice()->allocateCommandBuffersUnique(alloc_info); if (result != vk::Result::eSuccess) { return {}; } diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.h b/impeller/renderer/backend/vulkan/command_pool_vk.h index c1c540aad900f..a059da8390419 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.h +++ b/impeller/renderer/backend/vulkan/command_pool_vk.h @@ -10,6 +10,7 @@ #include "flutter/fml/macros.h" #include "impeller/base/thread.h" +#include "impeller/renderer/backend/vulkan/device_holder.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" @@ -38,7 +39,7 @@ class CommandPoolVK { private: const std::thread::id owner_id_; - const vk::Device* device_ = nullptr; + std::weak_ptr device_holder_; vk::UniqueCommandPool graphics_pool_; Mutex buffers_to_collect_mutex_; std::set> buffers_to_collect_ From 8a4350023f26ab9f2c85d94dca9f147a3e3fc063 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 17:35:04 -0700 Subject: [PATCH 07/13] moved over commandencoder --- .../vulkan/blit_command_vk_unittests.cc | 12 ++++----- .../backend/vulkan/command_encoder_vk.cc | 27 ++++++++++++------- .../backend/vulkan/command_encoder_vk.h | 5 ++-- .../renderer/backend/vulkan/context_vk.cc | 2 +- .../backend/vulkan/descriptor_pool_vk.cc | 20 +++++++++++--- .../backend/vulkan/descriptor_pool_vk.h | 5 ++-- 6 files changed, 46 insertions(+), 25 deletions(-) diff --git a/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc b/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc index a6f871d64c694..63975c955e8dd 100644 --- a/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc @@ -13,8 +13,8 @@ namespace testing { TEST(BlitCommandVkTest, BlitCopyTextureToTextureCommandVK) { auto context = CreateMockVulkanContext(); auto pool = CommandPoolVK::GetThreadLocal(context.get()); - CommandEncoderVK encoder(context->GetDevice(), context->GetGraphicsQueue(), - pool, context->GetFenceWaiter()); + CommandEncoderVK encoder(context, context->GetGraphicsQueue(), pool, + context->GetFenceWaiter()); BlitCopyTextureToTextureCommandVK cmd; cmd.source = context->GetResourceAllocator()->CreateTexture({ .size = ISize(100, 100), @@ -31,8 +31,8 @@ TEST(BlitCommandVkTest, BlitCopyTextureToTextureCommandVK) { TEST(BlitCommandVkTest, BlitCopyTextureToBufferCommandVK) { auto context = CreateMockVulkanContext(); auto pool = CommandPoolVK::GetThreadLocal(context.get()); - CommandEncoderVK encoder(context->GetDevice(), context->GetGraphicsQueue(), - pool, context->GetFenceWaiter()); + CommandEncoderVK encoder(context, context->GetGraphicsQueue(), pool, + context->GetFenceWaiter()); BlitCopyTextureToBufferCommandVK cmd; cmd.source = context->GetResourceAllocator()->CreateTexture({ .size = ISize(100, 100), @@ -49,8 +49,8 @@ TEST(BlitCommandVkTest, BlitCopyTextureToBufferCommandVK) { TEST(BlitCommandVkTest, BlitGenerateMipmapCommandVK) { auto context = CreateMockVulkanContext(); auto pool = CommandPoolVK::GetThreadLocal(context.get()); - CommandEncoderVK encoder(context->GetDevice(), context->GetGraphicsQueue(), - pool, context->GetFenceWaiter()); + CommandEncoderVK encoder(context, context->GetGraphicsQueue(), pool, + context->GetFenceWaiter()); BlitGenerateMipmapCommandVK cmd; cmd.texture = context->GetResourceAllocator()->CreateTexture({ .size = ISize(100, 100), diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index 89422e9b46255..e4afd8c8f52e4 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -14,9 +14,9 @@ namespace impeller { class TrackedObjectsVK { public: - explicit TrackedObjectsVK(const vk::Device* device, + explicit TrackedObjectsVK(std::weak_ptr device_holder, const std::shared_ptr& pool) - : desc_pool_(device) { + : desc_pool_(device_holder) { if (!pool) { return; } @@ -95,12 +95,14 @@ class TrackedObjectsVK { FML_DISALLOW_COPY_AND_ASSIGN(TrackedObjectsVK); }; -CommandEncoderVK::CommandEncoderVK(const vk::Device* device, - const std::shared_ptr& queue, - const std::shared_ptr& pool, - std::shared_ptr fence_waiter) +CommandEncoderVK::CommandEncoderVK( + std::weak_ptr device_holder, + const std::shared_ptr& queue, + const std::shared_ptr& pool, + std::shared_ptr fence_waiter) : fence_waiter_(std::move(fence_waiter)), - tracked_objects_(std::make_shared(device, pool)) { + tracked_objects_( + std::make_shared(device_holder, pool)) { if (!fence_waiter_ || !tracked_objects_->IsValid() || !queue) { return; } @@ -111,7 +113,7 @@ CommandEncoderVK::CommandEncoderVK(const vk::Device* device, VALIDATION_LOG << "Could not begin command buffer."; return; } - device_ = device; + device_holder_ = device_holder; queue_ = queue; is_valid_ = true; } @@ -137,7 +139,12 @@ bool CommandEncoderVK::Submit() { if (command_buffer.end() != vk::Result::eSuccess) { return false; } - auto [fence_result, fence] = device_->createFenceUnique({}); + std::shared_ptr strong_device = device_holder_.lock(); + if (!strong_device) { + return false; + } + auto [fence_result, fence] = + strong_device->GetDevice()->createFenceUnique({}); if (fence_result != vk::Result::eSuccess) { return false; } @@ -166,7 +173,7 @@ void CommandEncoderVK::Reset() { tracked_objects_.reset(); queue_ = nullptr; - device_ = nullptr; + device_holder_ = {}; is_valid_ = false; } diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.h b/impeller/renderer/backend/vulkan/command_encoder_vk.h index 0eda06f67c2e8..d160385472702 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.h +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.h @@ -10,6 +10,7 @@ #include "flutter/fml/macros.h" #include "impeller/renderer/backend/vulkan/command_pool_vk.h" #include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" +#include "impeller/renderer/backend/vulkan/device_holder.h" #include "impeller/renderer/backend/vulkan/queue_vk.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" @@ -69,13 +70,13 @@ class CommandEncoderVK { friend class ::impeller::testing:: BlitCommandVkTest_BlitGenerateMipmapCommandVK_Test; - const vk::Device* device_ = nullptr; + std::weak_ptr device_holder_; std::shared_ptr queue_; std::shared_ptr fence_waiter_; std::shared_ptr tracked_objects_; bool is_valid_ = false; - CommandEncoderVK(const vk::Device* device, + CommandEncoderVK(std::weak_ptr device_holder, const std::shared_ptr& queue, const std::shared_ptr& pool, std::shared_ptr fence_waiter); diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 186030969c617..2e7f64c1a4435 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -491,7 +491,7 @@ std::unique_ptr ContextVK::CreateGraphicsCommandEncoder() return nullptr; } auto encoder = std::unique_ptr(new CommandEncoderVK( - &device_.get(), // + weak_from_this(), // queues_.graphics_queue, // tls_pool, // fence_waiter_ // diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc index da98179f559b5..61429a53e6086 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -9,8 +9,11 @@ namespace impeller { -DescriptorPoolVK::DescriptorPoolVK(const vk::Device* device) - : device_(device) {} +DescriptorPoolVK::DescriptorPoolVK( + std::weak_ptr device_holder) + : device_holder_(device_holder) { + FML_DCHECK(device_holder.lock()); +} DescriptorPoolVK::~DescriptorPoolVK() = default; @@ -45,7 +48,12 @@ std::optional DescriptorPoolVK::AllocateDescriptorSet( vk::DescriptorSetAllocateInfo set_info; set_info.setDescriptorPool(pool.value()); set_info.setSetLayouts(layout); - auto [result, sets] = device_->allocateDescriptorSets(set_info); + std::shared_ptr strong_device = device_holder_.lock(); + if (!strong_device) { + return std::nullopt; + } + auto [result, sets] = + strong_device->GetDevice()->allocateDescriptorSets(set_info); if (result == vk::Result::eErrorOutOfPoolMemory) { return GrowPool() ? AllocateDescriptorSet(layout) : std::nullopt; } @@ -66,7 +74,11 @@ std::optional DescriptorPoolVK::GetDescriptorPool() { bool DescriptorPoolVK::GrowPool() { const auto new_pool_size = Allocation::NextPowerOfTwoSize(pool_size_ + 1u); - auto new_pool = CreatePool(device_, new_pool_size); + std::shared_ptr strong_device = device_holder_.lock(); + if (!strong_device) { + return false; + } + auto new_pool = CreatePool(strong_device->GetDevice(), new_pool_size); if (!new_pool) { return false; } diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h index 2db7fc5515af6..5fb05d5827cb9 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h @@ -8,6 +8,7 @@ #include #include "flutter/fml/macros.h" +#include "impeller/renderer/backend/vulkan/device_holder.h" #include "impeller/renderer/backend/vulkan/vk.h" namespace impeller { @@ -26,7 +27,7 @@ namespace impeller { /// class DescriptorPoolVK { public: - explicit DescriptorPoolVK(const vk::Device* device); + explicit DescriptorPoolVK(std::weak_ptr device_holder); ~DescriptorPoolVK(); @@ -34,7 +35,7 @@ class DescriptorPoolVK { const vk::DescriptorSetLayout& layout); private: - const vk::Device* device_; + std::weak_ptr device_holder_; uint32_t pool_size_ = 31u; std::queue pools_; From 48eacc430d9efb855cf2530ef4ab3ddd6c840b93 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 17:52:03 -0700 Subject: [PATCH 08/13] started cleaning up pools when devices are deleted --- impeller/renderer/backend/vulkan/command_encoder_vk.cc | 10 ++++++---- impeller/renderer/backend/vulkan/command_pool_vk.cc | 7 +++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index e4afd8c8f52e4..e5a873099e812 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -156,10 +156,12 @@ bool CommandEncoderVK::Submit() { return false; } - return fence_waiter_->AddFence( - std::move(fence), [tracked_objects = std::move(tracked_objects_)] { - // Nothing to do, we just drop the tracked objects on the floor. - }); + return fence_waiter_->AddFence(std::move(fence), + [tracked_objects = std::move(tracked_objects_), + strong_device = std::move(strong_device)] { + // Nothing to do, we just drop the tracked + // objects on the floor. + }); } vk::CommandBuffer CommandEncoderVK::GetCommandBuffer() const { diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index aa36fb1a7fd8a..791911ddefc97 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -15,10 +15,6 @@ namespace impeller { using CommandPoolMap = std::map>; - -// TODO(https://github.com/flutter/flutter/issues/125571): This is storing tons -// of CommandPoolVK's in the test runner since many contexts are created on the -// same thread. We need to come up with a different way to clean these up. FML_THREAD_LOCAL fml::ThreadLocalUniquePtr tls_command_pool; static Mutex g_all_pools_mutex; @@ -52,6 +48,9 @@ std::shared_ptr CommandPoolVK::GetThreadLocal( } void CommandPoolVK::ClearAllPools(const ContextVK* context) { + if (tls_command_pool.get()) { + tls_command_pool.get()->erase(context->GetHash()); + } Lock pool_lock(g_all_pools_mutex); if (auto found = g_all_pools.find(context); found != g_all_pools.end()) { for (auto& weak_pool : found->second) { From f904baf4c1added05e4daed85559841740132178 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 17:53:26 -0700 Subject: [PATCH 09/13] fixed opengles context --- impeller/renderer/backend/gles/context_gles.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index 5387f9aa908e3..2faea35232a04 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -18,7 +18,8 @@ namespace impeller { class ContextGLES final : public Context, - public BackendCast { + public BackendCast, + public std::enable_shared_from_this { public: static std::shared_ptr Create( std::unique_ptr gl, From 5a0736e5ad142aa96916f348164f2fe295bbde35 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 18:03:40 -0700 Subject: [PATCH 10/13] updated licenses --- ci/licenses_golden/licenses_flutter | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index a1989101ea127..c2ee613eefb34 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1489,6 +1489,7 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/device_buffer_vk.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/device_buffer_vk.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/device_holder.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc + ../../../flutter/LICENSE @@ -4090,6 +4091,7 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/device_buffer_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/device_buffer_vk.h +FILE: ../../../flutter/impeller/renderer/backend/vulkan/device_holder.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.cc FILE: ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk.h FILE: ../../../flutter/impeller/renderer/backend/vulkan/formats_vk.cc From e693bc853b03691dfadbbdd7c79dd2b28e17b973 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 18:18:26 -0700 Subject: [PATCH 11/13] lint --- impeller/renderer/backend/vulkan/pipeline_library_vk.cc | 2 +- impeller/renderer/backend/vulkan/pipeline_library_vk.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 6e62e30c3cb9d..504cd76bd49a8 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -19,7 +19,7 @@ namespace impeller { PipelineLibraryVK::PipelineLibraryVK( - std::weak_ptr device_holder, + const std::weak_ptr& device_holder, const vk::Device* device, std::shared_ptr caps, fml::UniqueFD cache_directory, diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/impeller/renderer/backend/vulkan/pipeline_library_vk.h index cde216610fbfc..63466ca320d8e 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.h @@ -43,7 +43,7 @@ class PipelineLibraryVK final bool is_valid_ = false; PipelineLibraryVK( - std::weak_ptr device_holder, + const std::weak_ptr& device_holder, const vk::Device* device, std::shared_ptr caps, fml::UniqueFD cache_directory, From 11f45742b46197fa5569b4fa17da7e5bbee06504 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 8 May 2023 09:08:48 -0700 Subject: [PATCH 12/13] Revert "started cleaning up pools when devices are deleted" This reverts commit 48eacc430d9efb855cf2530ef4ab3ddd6c840b93. --- impeller/renderer/backend/vulkan/command_encoder_vk.cc | 10 ++++------ impeller/renderer/backend/vulkan/command_pool_vk.cc | 7 ++++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index e5a873099e812..e4afd8c8f52e4 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -156,12 +156,10 @@ bool CommandEncoderVK::Submit() { return false; } - return fence_waiter_->AddFence(std::move(fence), - [tracked_objects = std::move(tracked_objects_), - strong_device = std::move(strong_device)] { - // Nothing to do, we just drop the tracked - // objects on the floor. - }); + return fence_waiter_->AddFence( + std::move(fence), [tracked_objects = std::move(tracked_objects_)] { + // Nothing to do, we just drop the tracked objects on the floor. + }); } vk::CommandBuffer CommandEncoderVK::GetCommandBuffer() const { diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index 791911ddefc97..aa36fb1a7fd8a 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -15,6 +15,10 @@ namespace impeller { using CommandPoolMap = std::map>; + +// TODO(https://github.com/flutter/flutter/issues/125571): This is storing tons +// of CommandPoolVK's in the test runner since many contexts are created on the +// same thread. We need to come up with a different way to clean these up. FML_THREAD_LOCAL fml::ThreadLocalUniquePtr tls_command_pool; static Mutex g_all_pools_mutex; @@ -48,9 +52,6 @@ std::shared_ptr CommandPoolVK::GetThreadLocal( } void CommandPoolVK::ClearAllPools(const ContextVK* context) { - if (tls_command_pool.get()) { - tls_command_pool.get()->erase(context->GetHash()); - } Lock pool_lock(g_all_pools_mutex); if (auto found = g_all_pools.find(context); found != g_all_pools.end()) { for (auto& weak_pool : found->second) { From 95a715f6b488645c71a0dfeae082fb4a812206da Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 8 May 2023 13:29:54 -0700 Subject: [PATCH 13/13] jasons feedback --- .../backend/vulkan/command_encoder_vk.cc | 3 +-- .../backend/vulkan/command_pool_vk.cc | 4 +-- .../renderer/backend/vulkan/context_vk.cc | 6 ++--- impeller/renderer/backend/vulkan/context_vk.h | 6 ++--- .../backend/vulkan/descriptor_pool_vk.cc | 6 ++--- .../renderer/backend/vulkan/device_holder.h | 2 +- .../backend/vulkan/pipeline_cache_vk.cc | 12 ++++----- .../backend/vulkan/pipeline_cache_vk.h | 6 ++++- .../backend/vulkan/pipeline_library_vk.cc | 10 +++---- .../backend/vulkan/pipeline_library_vk.h | 2 +- impeller/renderer/backend/vulkan/queue_vk.cc | 6 ++--- .../renderer/backend/vulkan/render_pass_vk.cc | 6 ++--- .../backend/vulkan/sampler_library_vk.cc | 3 +-- .../backend/vulkan/shader_library_vk.cc | 2 +- .../backend/vulkan/swapchain_image_vk.cc | 4 +-- .../backend/vulkan/swapchain_image_vk.h | 2 +- .../backend/vulkan/swapchain_impl_vk.cc | 26 +++++++++---------- 17 files changed, 54 insertions(+), 52 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index e4afd8c8f52e4..6c60c37834ea6 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -143,8 +143,7 @@ bool CommandEncoderVK::Submit() { if (!strong_device) { return false; } - auto [fence_result, fence] = - strong_device->GetDevice()->createFenceUnique({}); + auto [fence_result, fence] = strong_device->GetDevice().createFenceUnique({}); if (fence_result != vk::Result::eSuccess) { return false; } diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index aa36fb1a7fd8a..5949ad84c4094 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -74,7 +74,7 @@ CommandPoolVK::CommandPoolVK(const ContextVK* context) pool_info.queueFamilyIndex = context->GetGraphicsQueue()->GetIndex().family; pool_info.flags = vk::CommandPoolCreateFlagBits::eTransient; - auto pool = context->GetDevice()->createCommandPoolUnique(pool_info); + auto pool = context->GetDevice().createCommandPoolUnique(pool_info); if (pool.result != vk::Result::eSuccess) { return; } @@ -118,7 +118,7 @@ vk::UniqueCommandBuffer CommandPoolVK::CreateGraphicsCommandBuffer() { alloc_info.commandBufferCount = 1u; alloc_info.level = vk::CommandBufferLevel::ePrimary; auto [result, buffers] = - strong_device->GetDevice()->allocateCommandBuffersUnique(alloc_info); + strong_device->GetDevice().allocateCommandBuffersUnique(alloc_info); if (result != vk::Result::eSuccess) { return {}; } diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 2e7f64c1a4435..7d77740694934 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -308,7 +308,7 @@ void ContextVK::Setup(Settings settings) { /// auto pipeline_library = std::shared_ptr( new PipelineLibraryVK(weak_from_this(), // - &device.get(), // + device.get(), // caps, // std::move(settings.cache_directory), // settings.worker_task_runner // @@ -423,8 +423,8 @@ vk::Instance ContextVK::GetInstance() const { return *instance_; } -const vk::Device* ContextVK::GetDevice() const { - return &device_.get(); +const vk::Device& ContextVK::GetDevice() const { + return device_.get(); } std::unique_ptr ContextVK::AcquireNextSurface() { diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 0d51d4aad5103..27d71ddeb5838 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -85,7 +85,7 @@ class ContextVK final : public Context, } template - static bool SetDebugName(const vk::Device* device, + static bool SetDebugName(const vk::Device& device, T handle, std::string_view label) { if (!HasValidationLayers()) { @@ -100,7 +100,7 @@ class ContextVK final : public Context, info.pObjectName = label.data(); info.objectHandle = reinterpret_cast(c_handle); - if (device->setDebugUtilsObjectNameEXT(info) != vk::Result::eSuccess) { + if (device.setDebugUtilsObjectNameEXT(info) != vk::Result::eSuccess) { VALIDATION_LOG << "Unable to set debug name: " << label; return false; } @@ -111,7 +111,7 @@ class ContextVK final : public Context, vk::Instance GetInstance() const; // |DeviceHolder| - const vk::Device* GetDevice() const override; + const vk::Device& GetDevice() const override; [[nodiscard]] bool SetWindowSurface(vk::UniqueSurfaceKHR surface); diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc index 61429a53e6086..7d2a6c4c5eccb 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -17,7 +17,7 @@ DescriptorPoolVK::DescriptorPoolVK( DescriptorPoolVK::~DescriptorPoolVK() = default; -static vk::UniqueDescriptorPool CreatePool(const vk::Device* device, +static vk::UniqueDescriptorPool CreatePool(const vk::Device& device, uint32_t pool_count) { TRACE_EVENT0("impeller", "CreateDescriptorPool"); std::vector pools = { @@ -32,7 +32,7 @@ static vk::UniqueDescriptorPool CreatePool(const vk::Device* device, pool_info.setMaxSets(pools.size() * pool_count); pool_info.setPoolSizes(pools); - auto [result, pool] = device->createDescriptorPoolUnique(pool_info); + auto [result, pool] = device.createDescriptorPoolUnique(pool_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Unable to create a descriptor pool"; } @@ -53,7 +53,7 @@ std::optional DescriptorPoolVK::AllocateDescriptorSet( return std::nullopt; } auto [result, sets] = - strong_device->GetDevice()->allocateDescriptorSets(set_info); + strong_device->GetDevice().allocateDescriptorSets(set_info); if (result == vk::Result::eErrorOutOfPoolMemory) { return GrowPool() ? AllocateDescriptorSet(layout) : std::nullopt; } diff --git a/impeller/renderer/backend/vulkan/device_holder.h b/impeller/renderer/backend/vulkan/device_holder.h index 9a127472eb0c6..cb9fdee248584 100644 --- a/impeller/renderer/backend/vulkan/device_holder.h +++ b/impeller/renderer/backend/vulkan/device_holder.h @@ -11,7 +11,7 @@ namespace impeller { class DeviceHolder { public: virtual ~DeviceHolder() = default; - virtual const vk::Device* GetDevice() const = 0; + virtual const vk::Device& GetDevice() const = 0; }; } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc index 159e12af44bd0..6d07881131454 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.cc @@ -54,12 +54,12 @@ static std::unique_ptr OpenCacheFile( PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, std::weak_ptr device_holder, - const vk::Device* device, + const vk::Device& device, fml::UniqueFD cache_directory) : caps_(std::move(caps)), device_holder_(device_holder), cache_directory_(std::move(cache_directory)) { - if (!caps_ || !device || !*device) { + if (!caps_ || !device) { return; } @@ -81,7 +81,7 @@ PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, cache_info.pInitialData = existing_cache_data->GetMapping(); } - auto [result, existing_cache] = device->createPipelineCacheUnique(cache_info); + auto [result, existing_cache] = device.createPipelineCacheUnique(cache_info); if (result == vk::Result::eSuccess) { cache_ = std::move(existing_cache); @@ -93,7 +93,7 @@ PipelineCacheVK::PipelineCacheVK(std::shared_ptr caps, << vk::to_string(result) << ". Starting with a fresh cache."; cache_info.pInitialData = nullptr; cache_info.initialDataSize = 0u; - auto [result2, new_cache] = device->createPipelineCacheUnique(cache_info); + auto [result2, new_cache] = device.createPipelineCacheUnique(cache_info); if (result2 == vk::Result::eSuccess) { cache_ = std::move(new_cache); } else { @@ -120,7 +120,7 @@ vk::UniquePipeline PipelineCacheVK::CreatePipeline( Lock lock(cache_mutex_); auto [result, pipeline] = - strong_device->GetDevice()->createGraphicsPipelineUnique(*cache_, info); + strong_device->GetDevice().createGraphicsPipelineUnique(*cache_, info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create graphics pipeline: " << vk::to_string(result); @@ -139,7 +139,7 @@ std::shared_ptr PipelineCacheVK::CopyPipelineCacheData() const { } Lock lock(cache_mutex_); auto [result, data] = - strong_device->GetDevice()->getPipelineCacheData(*cache_); + strong_device->GetDevice().getPipelineCacheData(*cache_); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not get pipeline cache data to persist."; return nullptr; diff --git a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h index 9c0ccefb67db8..bb90a32cfede4 100644 --- a/impeller/renderer/backend/vulkan/pipeline_cache_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_cache_vk.h @@ -15,9 +15,13 @@ namespace impeller { class PipelineCacheVK { public: + // The [device] is passed in directly so that it can be used in the + // constructor directly. The [device_holder] isn't guaranteed to be valid + // at the time of executing `PipelineCacheVK` because of how `ContextVK` does + // initialization. explicit PipelineCacheVK(std::shared_ptr caps, std::weak_ptr device_holder, - const vk::Device* device, + const vk::Device& device, fml::UniqueFD cache_directory); ~PipelineCacheVK(); diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 504cd76bd49a8..22843ee4dc251 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -20,7 +20,7 @@ namespace impeller { PipelineLibraryVK::PipelineLibraryVK( const std::weak_ptr& device_holder, - const vk::Device* device, + const vk::Device& device, std::shared_ptr caps, fml::UniqueFD cache_directory, std::shared_ptr worker_task_runner) @@ -73,7 +73,7 @@ static vk::AttachmentDescription CreatePlaceholderAttachmentDescription( /// spec: /// https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/chap8.html#renderpass-compatibility /// -static vk::UniqueRenderPass CreateRenderPass(const vk::Device* device, +static vk::UniqueRenderPass CreateRenderPass(const vk::Device& device, const PipelineDescriptor& desc) { std::vector attachments; @@ -119,7 +119,7 @@ static vk::UniqueRenderPass CreateRenderPass(const vk::Device* device, render_pass_desc.setPSubpasses(&subpass_desc); render_pass_desc.setSubpassCount(1u); - auto [result, pass] = device->createRenderPassUnique(render_pass_desc); + auto [result, pass] = device.createRenderPassUnique(render_pass_desc); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Failed to create render pass for pipeline '" << desc.GetLabel() << "'. Error: " << vk::to_string(result); @@ -286,7 +286,7 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( descs_layout_info.setBindings(desc_bindings); auto [descs_result, descs_layout] = - strong_device->GetDevice()->createDescriptorSetLayoutUnique( + strong_device->GetDevice().createDescriptorSetLayoutUnique( descs_layout_info); if (descs_result != vk::Result::eSuccess) { VALIDATION_LOG << "unable to create uniform descriptors"; @@ -301,7 +301,7 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( /// vk::PipelineLayoutCreateInfo pipeline_layout_info; pipeline_layout_info.setSetLayouts(descs_layout.get()); - auto pipeline_layout = strong_device->GetDevice()->createPipelineLayoutUnique( + auto pipeline_layout = strong_device->GetDevice().createPipelineLayoutUnique( pipeline_layout_info); if (pipeline_layout.result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create pipeline layout for pipeline " diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/impeller/renderer/backend/vulkan/pipeline_library_vk.h index 63466ca320d8e..44b87e07d9a62 100644 --- a/impeller/renderer/backend/vulkan/pipeline_library_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.h @@ -44,7 +44,7 @@ class PipelineLibraryVK final PipelineLibraryVK( const std::weak_ptr& device_holder, - const vk::Device* device, + const vk::Device& device, std::shared_ptr caps, fml::UniqueFD cache_directory, std::shared_ptr worker_task_runner); diff --git a/impeller/renderer/backend/vulkan/queue_vk.cc b/impeller/renderer/backend/vulkan/queue_vk.cc index 044b55e35d1c2..a0c82344d1fbb 100644 --- a/impeller/renderer/backend/vulkan/queue_vk.cc +++ b/impeller/renderer/backend/vulkan/queue_vk.cc @@ -45,14 +45,14 @@ QueuesVK::QueuesVK(const vk::Device& device, // Always setup the graphics queue. graphics_queue = std::make_shared(graphics, vk_graphics); - ContextVK::SetDebugName(&device, vk_graphics, "ImpellerGraphicsQ"); + ContextVK::SetDebugName(device, vk_graphics, "ImpellerGraphicsQ"); // Setup the compute queue if its different from the graphics queue. if (compute == graphics) { compute_queue = graphics_queue; } else { compute_queue = std::make_shared(compute, vk_compute); - ContextVK::SetDebugName(&device, vk_compute, "ImpellerComputeQ"); + ContextVK::SetDebugName(device, vk_compute, "ImpellerComputeQ"); } // Setup the transfer queue if its different from the graphics or compute @@ -63,7 +63,7 @@ QueuesVK::QueuesVK(const vk::Device& device, transfer_queue = compute_queue; } else { transfer_queue = std::make_shared(transfer, vk_transfer); - ContextVK::SetDebugName(&device, vk_transfer, "ImpellerTransferQ"); + ContextVK::SetDebugName(device, vk_transfer, "ImpellerTransferQ"); } } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index befbb43bb00d8..23adb1829ba33 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -127,7 +127,7 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( render_pass_desc.setSubpassCount(1u); auto [result, pass] = - context.GetDevice()->createRenderPassUnique(render_pass_desc); + context.GetDevice().createRenderPassUnique(render_pass_desc); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Failed to create render pass: " << vk::to_string(result); return {}; @@ -232,7 +232,7 @@ SharedHandleVK RenderPassVK::CreateVKFramebuffer( fb_info.setAttachments(attachments); auto [result, framebuffer] = - context.GetDevice()->createFramebufferUnique(fb_info); + context.GetDevice().createFramebufferUnique(fb_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create framebuffer: " << vk::to_string(result); @@ -390,7 +390,7 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context, return false; } - context.GetDevice()->updateDescriptorSets(writes, {}); + context.GetDevice().updateDescriptorSets(writes, {}); encoder.GetCommandBuffer().bindDescriptorSets( vk::PipelineBindPoint::eGraphics, // bind point diff --git a/impeller/renderer/backend/vulkan/sampler_library_vk.cc b/impeller/renderer/backend/vulkan/sampler_library_vk.cc index 62bb55100915e..8d4c82e39cc15 100644 --- a/impeller/renderer/backend/vulkan/sampler_library_vk.cc +++ b/impeller/renderer/backend/vulkan/sampler_library_vk.cc @@ -56,8 +56,7 @@ std::shared_ptr SamplerLibraryVK::GetSampler( } if (!desc.label.empty()) { - ContextVK::SetDebugName(&device_, sampler->GetSampler(), - desc.label.c_str()); + ContextVK::SetDebugName(device_, sampler->GetSampler(), desc.label.c_str()); } samplers_[desc] = sampler; diff --git a/impeller/renderer/backend/vulkan/shader_library_vk.cc b/impeller/renderer/backend/vulkan/shader_library_vk.cc index c258c5eae9225..dd2d4b96caa63 100644 --- a/impeller/renderer/backend/vulkan/shader_library_vk.cc +++ b/impeller/renderer/backend/vulkan/shader_library_vk.cc @@ -156,7 +156,7 @@ bool ShaderLibraryVK::RegisterFunction( const auto key_name = VKShaderNameToShaderKeyName(name, stage); vk::UniqueShaderModule shader_module = std::move(module.value); - ContextVK::SetDebugName(&device_, *shader_module, "Shader " + name); + ContextVK::SetDebugName(device_, *shader_module, "Shader " + name); WriterLock lock(functions_mutex_); functions_[ShaderKey{key_name, stage}] = std::shared_ptr( diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc index c754cd57ccd1b..392c76b645a9b 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.cc @@ -7,7 +7,7 @@ namespace impeller { SwapchainImageVK::SwapchainImageVK(TextureDescriptor desc, - const vk::Device* device, + const vk::Device& device, vk::Image image) : TextureSourceVK(desc), image_(image) { vk::ImageViewCreateInfo view_info; @@ -20,7 +20,7 @@ SwapchainImageVK::SwapchainImageVK(TextureDescriptor desc, view_info.subresourceRange.levelCount = desc.mip_count; view_info.subresourceRange.layerCount = ToArrayLayerCount(desc.type); - auto [view_result, view] = device->createImageViewUnique(view_info); + auto [view_result, view] = device.createImageViewUnique(view_info); if (view_result != vk::Result::eSuccess) { return; } diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.h b/impeller/renderer/backend/vulkan/swapchain_image_vk.h index 8693a2f172c8d..6cf1943b8729e 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h @@ -15,7 +15,7 @@ namespace impeller { class SwapchainImageVK final : public TextureSourceVK { public: SwapchainImageVK(TextureDescriptor desc, - const vk::Device* device, + const vk::Device& device, vk::Image image); // |TextureSourceVK| diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 8036262eb39b3..9e4deb598fb11 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -21,11 +21,11 @@ struct FrameSynchronizer { vk::UniqueSemaphore present_ready; bool is_valid = false; - explicit FrameSynchronizer(const vk::Device* device) { - auto acquire_res = device->createFenceUnique( + explicit FrameSynchronizer(const vk::Device& device) { + auto acquire_res = device.createFenceUnique( vk::FenceCreateInfo{vk::FenceCreateFlagBits::eSignaled}); - auto render_res = device->createSemaphoreUnique({}); - auto present_res = device->createSemaphoreUnique({}); + auto render_res = device.createSemaphoreUnique({}); + auto present_res = device.createSemaphoreUnique({}); if (acquire_res.result != vk::Result::eSuccess || render_res.result != vk::Result::eSuccess || present_res.result != vk::Result::eSuccess) { @@ -40,8 +40,8 @@ struct FrameSynchronizer { ~FrameSynchronizer() = default; - bool WaitForFence(const vk::Device* device) { - if (auto result = device->waitForFences( + bool WaitForFence(const vk::Device& device) { + if (auto result = device.waitForFences( *acquire, // fence true, // wait all std::numeric_limits::max() // timeout (ns) @@ -50,7 +50,7 @@ struct FrameSynchronizer { VALIDATION_LOG << "Fence wait failed: " << vk::to_string(result); return false; } - if (auto result = device->resetFences(*acquire); + if (auto result = device.resetFences(*acquire); result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not reset fence: " << vk::to_string(result); return false; @@ -106,7 +106,7 @@ static std::optional ChooseAlphaCompositionMode( static std::optional ChoosePresentQueue( const vk::PhysicalDevice& physical_device, - const vk::Device* device, + const vk::Device& device, const vk::SurfaceKHR& surface) { const auto families = physical_device.getQueueFamilyProperties(); for (size_t family_index = 0u; family_index < families.size(); @@ -114,7 +114,7 @@ static std::optional ChoosePresentQueue( auto [result, supported] = physical_device.getSurfaceSupportKHR(family_index, surface); if (result == vk::Result::eSuccess && supported) { - return device->getQueue(family_index, 0u); + return device.getQueue(family_index, 0u); } } return std::nullopt; @@ -207,7 +207,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, swapchain_info.oldSwapchain = old_swapchain; auto [swapchain_result, swapchain] = - vk_context.GetDevice()->createSwapchainKHRUnique(swapchain_info); + vk_context.GetDevice().createSwapchainKHRUnique(swapchain_info); if (swapchain_result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not create swapchain: " << vk::to_string(swapchain_result); @@ -215,7 +215,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, } auto [images_result, images] = - vk_context.GetDevice()->getSwapchainImagesKHR(*swapchain); + vk_context.GetDevice().getSwapchainImagesKHR(*swapchain); if (images_result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not get swapchain images."; return; @@ -284,7 +284,7 @@ bool SwapchainImplVK::IsValid() const { void SwapchainImplVK::WaitIdle() const { if (auto context = context_.lock()) { [[maybe_unused]] auto result = - ContextVK::Cast(*context).GetDevice()->waitIdle(); + ContextVK::Cast(*context).GetDevice().waitIdle(); } } @@ -329,7 +329,7 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { //---------------------------------------------------------------------------- /// Get the next image index. /// - auto [acq_result, index] = context.GetDevice()->acquireNextImageKHR( + auto [acq_result, index] = context.GetDevice().acquireNextImageKHR( *swapchain_, // swapchain std::numeric_limits::max(), // timeout (nanoseconds) *sync->render_ready, // signal semaphore