Skip to content

Commit

Permalink
Misc vulkan device API cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
bobcao3 committed Feb 18, 2023
1 parent faf5ea0 commit c5680e8
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 69 deletions.
78 changes: 33 additions & 45 deletions taichi/rhi/vulkan/vulkan_device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,10 @@ void VulkanPipeline::create_pipeline_layout() {
}

void VulkanPipeline::create_compute_pipeline(const Params &params) {
char msg_buf[512];
RHI_DEBUG_SNPRINTF(msg_buf, sizeof(msg_buf), "Compiling Vulkan pipeline %s",
std::array<char, 512> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(), "Compiling Vulkan pipeline %s",
params.name.data());
RHI_LOG_DEBUG(msg_buf);
RHI_LOG_DEBUG(msg_buf.data());
pipeline_ = vkapi::create_compute_pipeline(device_, 0, shader_stages_[0],
pipeline_layout_, params.cache);
}
Expand Down Expand Up @@ -554,11 +554,12 @@ void VulkanPipeline::create_graphics_pipeline(
if (raster_params.blending.size()) {
if (raster_params.blending.size() != color_blending.attachmentCount) {
std::array<char, 256> buf;
snprintf(buf.data(), buf.size(),
"RasterParams::blending (size=%u) must either be zero sized "
"or match the number of fragment shader outputs (size=%u).",
uint32_t(raster_params.blending.size()),
uint32_t(color_blending.attachmentCount));
RHI_DEBUG_SNPRINTF(
buf.data(), buf.size(),
"RasterParams::blending (size=%u) must either be zero sized "
"or match the number of fragment shader outputs (size=%u).",
uint32_t(raster_params.blending.size()),
uint32_t(color_blending.attachmentCount));
RHI_LOG_ERROR(buf.data());
RHI_ASSERT(false);
}
Expand Down Expand Up @@ -931,17 +932,19 @@ RhiResult VulkanCommandList::bind_shader_resources(ShaderResourceSet *res,
VulkanResourceSet &set_template = templates.at(set_index);

for (const auto &template_binding : set_template.get_bindings()) {
char msg[512];
snprintf(msg, 512, "Template binding %d: (VkDescriptorType) %d",
template_binding.first, template_binding.second.type);
RHI_LOG_ERROR(msg);
std::array<char, 256> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(),
"Template binding %d: (VkDescriptorType) %d",
template_binding.first, template_binding.second.type);
RHI_LOG_ERROR(msg_buf.data());
}

for (const auto &binding : set->get_bindings()) {
char msg[512];
snprintf(msg, 512, "Binding %d: (VkDescriptorType) %d", binding.first,
binding.second.type);
RHI_LOG_ERROR(msg);
std::array<char, 256> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(),
"Binding %d: (VkDescriptorType) %d",
binding.first, binding.second.type);
RHI_LOG_ERROR(msg_buf.data());
}

return RhiResult::invalid_usage;
Expand Down Expand Up @@ -1612,7 +1615,6 @@ VulkanDevice::~VulkanDevice() {
compute_streams_.reset();
graphics_streams_.reset();

framebuffer_pools_.clear();
renderpass_pools_.clear();
desc_set_layouts_.clear();
desc_pool_ = nullptr;
Expand Down Expand Up @@ -1826,10 +1828,11 @@ RhiResult VulkanDevice::map_internal(AllocationInternal &alloc_int,
"wrapper)");
return RhiResult::invalid_usage;
} else if (res != VK_SUCCESS) {
char msg_buf[256];
snprintf(msg_buf, sizeof(msg_buf),
"failed to map memory for unknown reasons. VkResult = %d", res);
RHI_LOG_ERROR(msg_buf);
std::array<char, 256> msg_buf;
RHI_DEBUG_SNPRINTF(
msg_buf.data(), msg_buf.size(),
"failed to map memory for unknown reasons. VkResult = %d", res);
RHI_LOG_ERROR(msg_buf.data());
return RhiResult::error;
}

Expand Down Expand Up @@ -2021,23 +2024,12 @@ StreamSemaphore VulkanStream::submit(
auto fence = vkapi::create_fence(buffer->device, 0);

// Resource tracking, check previously submitted commands
// FIXME: Figure out why it doesn't work
/*
std::remove_if(submitted_cmdbuffers_.begin(), submitted_cmdbuffers_.end(),
[&](const TrackedCmdbuf &tracked) {
// If fence is signaled, cmdbuf has completed
VkResult res =
vkGetFenceStatus(buffer->device, tracked.fence->fence);
return res == VK_SUCCESS;
});
*/

submitted_cmdbuffers_.push_back(TrackedCmdbuf{fence, buffer});

BAIL_ON_VK_BAD_RESULT_NO_RETURN(
vkQueueSubmit(queue_, /*submitCount=*/1, &submit_info,
/*fence=*/fence->fence),
"failed to submit command buffer");
"Vulkan device might be lost (vkQueueSubmit failed)");

return std::make_shared<VulkanStreamSemaphoreObject>(semaphore);
}
Expand All @@ -2053,9 +2045,6 @@ StreamSemaphore VulkanStream::submit_synced(
void VulkanStream::command_sync() {
vkQueueWaitIdle(queue_);

VkPhysicalDeviceProperties props{};
vkGetPhysicalDeviceProperties(device_.vk_physical_device(), &props);

device_.profiler_sync();

submitted_cmdbuffers_.clear();
Expand Down Expand Up @@ -2130,15 +2119,13 @@ VulkanDevice::get_vk_image(const DeviceAllocation &alloc) const {

vkapi::IVkFramebuffer VulkanDevice::get_framebuffer(
const VulkanFramebufferDesc &desc) {
if (framebuffer_pools_.find(desc) != framebuffer_pools_.end()) {
return framebuffer_pools_.at(desc);
}

// We won't pool framebuffer and resuse it, as doing so requires hashing the
// referenced IVkImageView objects, which might destruct unless we hold strong
// references. Thus doing so is way too ugly, and Vulkan is moving towards
// dynamic rendering anyways.
vkapi::IVkFramebuffer framebuffer = vkapi::create_framebuffer(
0, desc.renderpass, desc.attachments, desc.width, desc.height, 1);

framebuffer_pools_.insert({desc, framebuffer});

return framebuffer;
}

Expand Down Expand Up @@ -2688,10 +2675,11 @@ void VulkanSurface::create_swap_chain() {
std::max(capabilities.minImageExtent.height,
std::min(capabilities.maxImageExtent.height, extent.height));
{
char msg_buf[512];
RHI_DEBUG_SNPRINTF(msg_buf, sizeof(msg_buf), "Creating suface of %u x %u",
std::array<char, 512> msg_buf;
RHI_DEBUG_SNPRINTF(msg_buf.data(), msg_buf.size(),
"Creating suface of %u x %u",
extent.width, extent.height);
RHI_LOG_DEBUG(msg_buf);
RHI_LOG_DEBUG(msg_buf.data());
}
VkImageUsageFlags usage =
VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT;
Expand Down
29 changes: 5 additions & 24 deletions taichi/rhi/vulkan/vulkan_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,13 @@ struct VulkanRenderPassDesc {

struct RenderPassDescHasher {
std::size_t operator()(const VulkanRenderPassDesc &desc) const {
// TODO: Come up with a better hash
size_t hash = 0;
size_t hash = std::hash<uint64_t>()((uint64_t(desc.depth_attachment) << 1) |
uint64_t(desc.clear_depth));
for (auto &pair : desc.color_attachments) {
hash ^= (size_t(pair.first) + pair.second);
hash = (hash << 3) || (hash >> 61);
size_t hash_pair = std::hash<uint64_t>()((uint64_t(pair.first) << 1) |
uint64_t(pair.second));
rhi_impl::hash_combine(hash, hash_pair);
}
hash ^= (size_t(desc.depth_attachment) + desc.clear_depth);
hash = (hash << 3) || (hash >> 61);
return hash;
}
};
Expand All @@ -75,20 +74,6 @@ struct VulkanFramebufferDesc {
}
};

struct FramebufferDescHasher {
std::size_t operator()(const VulkanFramebufferDesc &desc) const {
size_t hash = 0;
for (auto &view : desc.attachments) {
hash ^= size_t(view->view);
hash = (hash << 3) || (hash >> 61);
}
hash ^= desc.width;
hash ^= desc.height;
hash ^= size_t(desc.renderpass->renderpass);
return hash;
}
};

class VulkanResourceSet : public ShaderResourceSet {
public:
struct Buffer {
Expand Down Expand Up @@ -808,10 +793,6 @@ class TI_DLL_EXPORT VulkanDevice : public GraphicsDevice {
vkapi::IVkRenderPass,
RenderPassDescHasher>
renderpass_pools_;
unordered_map<VulkanFramebufferDesc,
vkapi::IVkFramebuffer,
FramebufferDescHasher>
framebuffer_pools_;

// Descriptors / Layouts / Pools
unordered_map<VulkanResourceSet,
Expand Down

0 comments on commit c5680e8

Please sign in to comment.