From 50791cb974e462c97dc133a619ddc2066961b526 Mon Sep 17 00:00:00 2001 From: GPUCode Date: Sat, 8 Apr 2023 21:53:13 +0300 Subject: [PATCH] renderer_vulkan: Fix crashing when updating descriptors * During pipeline configure the function would acquire some payload space from the descriptor update queue, write the descriptor data on the GPU thread and give the scheduler a pointer to the beginning of said space to update it later. TickFrame resets the payload cursor, used to track acquires, back to the beginning of the buffer. This wasn't a problem before since WaitWorker was called at the end of the frame but now it is. If a frame writes to a cursor before the scheduler catches up, it will crash * To fix this the payload buffer has been increased to account for the in flight frames that are allowed to exist now. TickFrame will switch between the payload spaces instead of resetting --- .../renderer_vulkan/vk_update_descriptor.cpp | 11 ++++++++--- src/video_core/renderer_vulkan/vk_update_descriptor.h | 10 +++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/video_core/renderer_vulkan/vk_update_descriptor.cpp b/src/video_core/renderer_vulkan/vk_update_descriptor.cpp index 009dab0b6..0630ebda5 100644 --- a/src/video_core/renderer_vulkan/vk_update_descriptor.cpp +++ b/src/video_core/renderer_vulkan/vk_update_descriptor.cpp @@ -14,13 +14,18 @@ namespace Vulkan { UpdateDescriptorQueue::UpdateDescriptorQueue(const Device& device_, Scheduler& scheduler_) : device{device_}, scheduler{scheduler_} { + payload_start = payload.data(); payload_cursor = payload.data(); } UpdateDescriptorQueue::~UpdateDescriptorQueue() = default; void UpdateDescriptorQueue::TickFrame() { - payload_cursor = payload.data(); + if (++frame_index >= FRAMES_IN_FLIGHT) { + frame_index = 0; + } + payload_start = payload.data() + frame_index * FRAME_PAYLOAD_SIZE; + payload_cursor = payload_start; } void UpdateDescriptorQueue::Acquire() { @@ -28,10 +33,10 @@ void UpdateDescriptorQueue::Acquire() { // This is the maximum number of entries a single draw call might use. static constexpr size_t MIN_ENTRIES = 0x400; - if (std::distance(payload.data(), payload_cursor) + MIN_ENTRIES >= payload.max_size()) { + if (std::distance(payload_start, payload_cursor) + MIN_ENTRIES >= FRAME_PAYLOAD_SIZE) { LOG_WARNING(Render_Vulkan, "Payload overflow, waiting for worker thread"); scheduler.WaitWorker(); - payload_cursor = payload.data(); + payload_cursor = payload_start; } upload_start = payload_cursor; } diff --git a/src/video_core/renderer_vulkan/vk_update_descriptor.h b/src/video_core/renderer_vulkan/vk_update_descriptor.h index 625bcc809..1c1a7020b 100644 --- a/src/video_core/renderer_vulkan/vk_update_descriptor.h +++ b/src/video_core/renderer_vulkan/vk_update_descriptor.h @@ -29,6 +29,12 @@ struct DescriptorUpdateEntry { }; class UpdateDescriptorQueue final { + // This should be plenty for the vast majority of cases. Most desktop platforms only + // provide up to 3 swapchain images. + static constexpr size_t FRAMES_IN_FLIGHT = 5; + static constexpr size_t FRAME_PAYLOAD_SIZE = 0x10000; + static constexpr size_t PAYLOAD_SIZE = FRAME_PAYLOAD_SIZE * FRAMES_IN_FLIGHT; + public: explicit UpdateDescriptorQueue(const Device& device_, Scheduler& scheduler_); ~UpdateDescriptorQueue(); @@ -73,9 +79,11 @@ private: const Device& device; Scheduler& scheduler; + size_t frame_index{0}; DescriptorUpdateEntry* payload_cursor = nullptr; + DescriptorUpdateEntry* payload_start = nullptr; const DescriptorUpdateEntry* upload_start = nullptr; - std::array payload; + std::array payload; }; } // namespace Vulkan