From c5680e878937ad2c158bceb848db4b31495fb7ba Mon Sep 17 00:00:00 2001 From: Cheng Cao Date: Sat, 18 Feb 2023 03:44:20 -0800 Subject: [PATCH] Misc vulkan device API cleanup --- taichi/rhi/vulkan/vulkan_device.cpp | 78 ++++++++++++----------------- taichi/rhi/vulkan/vulkan_device.h | 29 ++--------- 2 files changed, 38 insertions(+), 69 deletions(-) diff --git a/taichi/rhi/vulkan/vulkan_device.cpp b/taichi/rhi/vulkan/vulkan_device.cpp index 4594cf74942f9..c1fd9a7fbe297 100644 --- a/taichi/rhi/vulkan/vulkan_device.cpp +++ b/taichi/rhi/vulkan/vulkan_device.cpp @@ -416,10 +416,10 @@ void VulkanPipeline::create_pipeline_layout() { } void VulkanPipeline::create_compute_pipeline(const Params ¶ms) { - char msg_buf[512]; - RHI_DEBUG_SNPRINTF(msg_buf, sizeof(msg_buf), "Compiling Vulkan pipeline %s", + std::array 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); } @@ -554,11 +554,12 @@ void VulkanPipeline::create_graphics_pipeline( if (raster_params.blending.size()) { if (raster_params.blending.size() != color_blending.attachmentCount) { std::array 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); } @@ -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 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 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; @@ -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; @@ -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 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; } @@ -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(semaphore); } @@ -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(); @@ -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; } @@ -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 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; diff --git a/taichi/rhi/vulkan/vulkan_device.h b/taichi/rhi/vulkan/vulkan_device.h index 167357c3bdb6e..c2fc1e689824f 100644 --- a/taichi/rhi/vulkan/vulkan_device.h +++ b/taichi/rhi/vulkan/vulkan_device.h @@ -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(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(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; } }; @@ -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 { @@ -808,10 +793,6 @@ class TI_DLL_EXPORT VulkanDevice : public GraphicsDevice { vkapi::IVkRenderPass, RenderPassDescHasher> renderpass_pools_; - unordered_map - framebuffer_pools_; // Descriptors / Layouts / Pools unordered_map