Skip to content

Commit

Permalink
Document and provide small cleanups to CommandPoolVk. (flutter#45558)
Browse files Browse the repository at this point in the history
I was reviewing this code for the first time before implementing
flutter/flutter#133198.

- Removed unused public method `GetGraphicsCommandPool()`.
- Removed forward references by including the right references [^1].
- Tried to document the contracts of various methods that return invalid
or disconnected objects.

If I made any mistakes, please feel free to point them out as this is
how I'm learning how this stuff works 😄

/cc @jonahwilliams 

[^1]: [Avoid using forward declarations where
possible](https://google.github.io/styleguide/cppguide.html#Forward_Declarations).
Instead, include the headers you need.
  • Loading branch information
matanlurey authored Sep 8, 2023
1 parent 47a7930 commit 3aa3880
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 14 deletions.
4 changes: 0 additions & 4 deletions impeller/renderer/backend/vulkan/command_pool_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ void CommandPoolVK::Reset() {
is_valid_ = false;
}

vk::CommandPool CommandPoolVK::GetGraphicsCommandPool() const {
return graphics_pool_.get();
}

vk::UniqueCommandBuffer CommandPoolVK::CreateGraphicsCommandBuffer() {
std::shared_ptr<const DeviceHolder> strong_device = device_holder_.lock();
if (!strong_device) {
Expand Down
76 changes: 66 additions & 10 deletions impeller/renderer/backend/vulkan/command_pool_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,80 @@
#pragma once

#include <memory>
#include <set>
#include <thread>

#include "flutter/fml/macros.h"
#include "impeller/base/thread.h"
#include "impeller/renderer/backend/vulkan/context_vk.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"

namespace impeller {

class ContextVK;

class CommandPoolVK {
//------------------------------------------------------------------------------
/// @brief An opaque object that provides |vk::CommandBuffer| objects.
///
/// A best practice is to create a |CommandPoolVK| for each thread that will
/// submit commands to the GPU, and to recycle (reuse) |vk::CommandBuffer|s by
/// resetting them in a background thread.
///
/// @see
/// https://arm-software.github.io/vulkan_best_practice_for_mobile_developers/samples/performance/command_buffer_usage/command_buffer_usage_tutorial.html#resetting-the-command-pool
class CommandPoolVK final {
public:
/// @brief Gets the |CommandPoolVK| for the current thread.
///
/// @param[in] context The |ContextVK| to use.
///
/// If the current thread does not have a |CommandPoolVK|, one will be created
/// and returned. If the current thread already has a |CommandPoolVK|, it will
/// be returned.
///
/// @return The |CommandPoolVK| for the current thread, or |nullptr| if
/// either the |ContextVK| is invalid, or a pool could not be
/// created for any reason.
///
/// In other words an invalid command pool will never be returned.
static std::shared_ptr<CommandPoolVK> GetThreadLocal(
const ContextVK* context);

/// @brief Clears all |CommandPoolVK|s for the given |ContextVK|.
///
/// @param[in] context The |ContextVK| to clear.
///
/// Every |CommandPoolVK| that was created for every thread that has ever
/// called |GetThreadLocal| with the given |ContextVK| will be cleared.
///
/// @note Should only be called when the |ContextVK| is being destroyed.
static void ClearAllPools(const ContextVK* context);

~CommandPoolVK();

/// @brief Whether or not this |CommandPoolVK| is valid.
///
/// A command pool is no longer when valid once it's been |Reset|.
bool IsValid() const;

void Reset();

vk::CommandPool GetGraphicsCommandPool() const;

/// @brief Creates and returns a new |vk::CommandBuffer|.
///
/// An attempt is made to reuse existing buffers (instead of creating new
/// ones) by recycling buffers that have been collected by
/// |CollectGraphicsCommandBuffer|.
///
/// @return Always returns a new |vk::CommandBuffer|, but if for any
/// reason a valid command buffer could not be created, it will be
/// a `{}` default instance (i.e. while being torn down).
vk::UniqueCommandBuffer CreateGraphicsCommandBuffer();

/// @brief Collects the given |vk::CommandBuffer| for recycling.
///
/// The given |vk::CommandBuffer| will be recycled (reused) in the future when
/// |CreateGraphicsCommandBuffer| is called.
///
/// @param[in] buffer The |vk::CommandBuffer| to collect.
///
/// @note This method is a noop if a different thread created the pool.
///
/// @see |GarbageCollectBuffersIfAble|
void CollectGraphicsCommandBuffer(vk::UniqueCommandBuffer buffer);

private:
Expand All @@ -47,8 +91,20 @@ class CommandPoolVK {
std::vector<vk::UniqueCommandBuffer> recycled_buffers_;
bool is_valid_ = false;

/// @brief Resets, releasing all |vk::CommandBuffer|s.
///
/// @note "All" includes active and recycled buffers.
void Reset();

explicit CommandPoolVK(const ContextVK* context);

/// @brief Collects buffers for recycling if able.
///
/// If any buffers have been returned through |CollectGraphicsCommandBuffer|,
/// then they are reset and made available to future calls to
/// |CreateGraphicsCommandBuffer|.
///
/// @note This method is a noop if a different thread created the pool.
void GarbageCollectBuffersIfAble() IPLR_REQUIRES(buffers_to_collect_mutex_);

FML_DISALLOW_COPY_AND_ASSIGN(CommandPoolVK);
Expand Down

0 comments on commit 3aa3880

Please sign in to comment.