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

[Impeller] Allocate buffers out of a pool on the raster thread. #43564

Merged
auto-submit merged 9 commits into from
Jul 12, 2023
Merged

[Impeller] Allocate buffers out of a pool on the raster thread. #43564

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8590d30
[Impeller] allocate buffers out of a pool on the raster thread.
Jul 11, 2023
866761f
fall back to host visible only
Jul 11, 2023
1a01926
wip testing
Jul 11, 2023
c85a337
testing
Jul 11, 2023
3947f81
++
Jul 11, 2023
b8bee47
++
Jul 11, 2023
9d4d44a
++
Jul 11, 2023
4efb6a8
use context didAcquireSurfaceFrame
Jul 12, 2023
10d29cc
use c++ types
Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
++
  • Loading branch information
jonahwilliams committed Jul 11, 2023
commit b8bee473817551c9b45ef0f553344736e4706cba
16 changes: 8 additions & 8 deletions impeller/renderer/backend/vulkan/allocator_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,8 @@ AllocatorVK::AllocatorVK(std::weak_ptr<Context> context,
return;
}
for (auto i = 0u; i < kPoolCount; i++) {
if (!CreateBufferPool(allocator, &staging_buffer_pools_[i])) {
return;
}
created_buffer_pools_ &=
Copy link
Contributor Author

@jonahwilliams jonahwilliams Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this optional for now as the host unittests don't seem to be able to create the same buffer pools that real devices do.

CreateBufferPool(allocator, &staging_buffer_pools_[i]);
}
allocator_ = allocator;
supports_memoryless_textures_ = capabilities.SupportsMemorylessTextures();
Expand Down Expand Up @@ -192,7 +191,9 @@ static constexpr VkMemoryPropertyFlags ToVKTextureMemoryPropertyFlags(
bool supports_memoryless_textures) {
switch (mode) {
case StorageMode::kHostVisible:
return VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT;
return VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT |
Copy link
Member

@chinmaygarde chinmaygarde Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is just modifying existing code, but can we use vk::MemoryPropertyFlagBits here (the C++ variant)?

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do I convert the vk::Flag type back to the C bits for VMA?

Copy link
Member

@chinmaygarde chinmaygarde Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just static cast it to the underlying type. The header says enum class MemoryPropertyFlagBits : VkMemoryPropertyFlags. You can even use decltype for it. But your call as this isn't how the code works now.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh makes sense

VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT |
VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
case StorageMode::kDevicePrivate:
return VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
case StorageMode::kDeviceTransient:
Expand Down Expand Up @@ -438,7 +439,7 @@ std::shared_ptr<DeviceBuffer> AllocatorVK::OnCreateBuffer(
allocation_info.preferredFlags =
ToVKBufferMemoryPropertyFlags(desc.storage_mode);
allocation_info.flags = ToVmaAllocationBufferCreateFlags(desc.storage_mode);
if (desc.storage_mode == StorageMode::kHostVisible &&
if (created_buffer_pools_ && desc.storage_mode == StorageMode::kHostVisible &&
raster_thread_id_ == std::this_thread::get_id()) {
allocation_info.pool = staging_buffer_pools_[frame_count_ % kPoolCount];
}
Expand Down Expand Up @@ -487,9 +488,8 @@ bool AllocatorVK::CreateBufferPool(VmaAllocator allocator, VmaPool* pool) {
allocation_info.usage = VMA_MEMORY_USAGE_AUTO;
allocation_info.preferredFlags =
ToVKBufferMemoryPropertyFlags(StorageMode::kHostVisible);
// TESTING
// allocation_info.flags =
// ToVmaAllocationBufferCreateFlags(StorageMode::kHostVisible);
allocation_info.flags =
ToVmaAllocationBufferCreateFlags(StorageMode::kHostVisible);

uint32_t memTypeIndex;
auto result = vk::Result{vmaFindMemoryTypeIndexForBufferInfo(
Expand Down
2 changes: 2 additions & 0 deletions impeller/renderer/backend/vulkan/allocator_vk.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class AllocatorVK final : public Allocator {
ISize max_texture_size_;
bool is_valid_ = false;
bool supports_memoryless_textures_ = false;
// TODO(jonahwilliams): figure out why CI can't create these buffer pools.
bool created_buffer_pools_ = true;
uint32_t frame_count_ = 0;
std::thread::id raster_thread_id_;

Expand Down