From c356a20547afae97b412da36e0b0a7d51e879401 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Fri, 24 Dec 2021 12:25:46 +0200 Subject: [PATCH] Redesign asynchronous buffer uploads The Buffer class now offers an async version of sub_data, which can better account for exactly how the data needs to be uploaded. --- source/backends/opengl/buffer_backend.cpp | 11 +++++ source/backends/vulkan/buffer_backend.cpp | 57 ++++++++++++++++------- source/backends/vulkan/synchronizer.cpp | 7 ++- source/backends/vulkan/synchronizer.h | 2 +- source/core/buffer.cpp | 43 +++++++++++++++-- source/core/buffer.h | 39 ++++++++++++++++ source/core/bufferable.cpp | 15 ++---- source/core/bufferable.h | 6 +-- 8 files changed, 141 insertions(+), 39 deletions(-) diff --git a/source/backends/opengl/buffer_backend.cpp b/source/backends/opengl/buffer_backend.cpp index 6417633b..c5954083 100644 --- a/source/backends/opengl/buffer_backend.cpp +++ b/source/backends/opengl/buffer_backend.cpp @@ -132,5 +132,16 @@ void OpenGLBuffer::unbind_scratch() } } + +void Buffer::AsyncTransfer::allocate() +{ + dest_addr = buffer.map(); +} + +void Buffer::AsyncTransfer::finalize() +{ + buffer.unmap(); +} + } // namespace GL } // namespace Msp diff --git a/source/backends/vulkan/buffer_backend.cpp b/source/backends/vulkan/buffer_backend.cpp index 5439548f..aaf41570 100644 --- a/source/backends/vulkan/buffer_backend.cpp +++ b/source/backends/vulkan/buffer_backend.cpp @@ -52,24 +52,9 @@ void VulkanBuffer::allocate() void VulkanBuffer::sub_data(size_t off, size_t sz, const void *d) { - TransferQueue &tq = device.get_transfer_queue(); - void *staging = tq.prepare_transfer(this, false, sz, - [this, off, sz](){ - device.get_synchronizer().write_buffer(handle, off, sz); - }, - [this, off, sz](VkCommandBuffer cmd_buf, VkBuffer staging_buf, size_t src_off){ - const VulkanFunctions &vk = device.get_functions(); - - VkBufferCopy region = { }; - region.srcOffset = src_off; - region.dstOffset = off; - region.size = sz; - vk.CmdCopyBuffer(cmd_buf, staging_buf, handle, 1, ®ion); - }); - + Buffer::AsyncTransfer transfer(*static_cast(this), off, sz); const char *src = static_cast(d); - copy(src, src+sz, static_cast(staging)); - tq.finalize_transfer(staging); + copy(src, src+sz, static_cast(transfer.get_address())); } unsigned VulkanBuffer::get_multiplicity() const @@ -122,5 +107,43 @@ void VulkanBuffer::set_vulkan_object_name() const #endif } + +void Buffer::AsyncTransfer::allocate() +{ + if(buffer.can_map()) + dest_addr = static_cast(buffer.map())+offset; + else + { + Buffer &buf = buffer; + size_t off = offset; + size_t sz = size; + + dest_addr = buffer.device.get_transfer_queue().prepare_transfer(&buffer, false, size, + [&buf, off, sz](){ + buf.device.get_synchronizer().write_buffer(buf.handle, off, sz); + }, + [&buf, off, sz](VkCommandBuffer cmd_buf, VkBuffer staging_buf, size_t src_off){ + const VulkanFunctions &vk = buf.device.get_functions(); + + VkBufferCopy region = { }; + region.srcOffset = src_off; + region.dstOffset = off; + region.size = sz; + vk.CmdCopyBuffer(cmd_buf, staging_buf, buf.handle, 1, ®ion); + }); + } +} + +void Buffer::AsyncTransfer::finalize() +{ + if(buffer.can_map()) + { + buffer.unmap(); + buffer.device.get_synchronizer().write_buffer(buffer.handle, offset, size, true); + } + else + buffer.device.get_transfer_queue().finalize_transfer(dest_addr); +} + } // namespace GL } // namespace Msp diff --git a/source/backends/vulkan/synchronizer.cpp b/source/backends/vulkan/synchronizer.cpp index 93f48b84..9995f248 100644 --- a/source/backends/vulkan/synchronizer.cpp +++ b/source/backends/vulkan/synchronizer.cpp @@ -15,7 +15,7 @@ Synchronizer::Synchronizer(Device &d): device(d) { } -void Synchronizer::write_buffer(VkBuffer buffer, size_t offset, size_t size) +void Synchronizer::write_buffer(VkBuffer buffer, size_t offset, size_t size, bool mapped) { auto i = lower_bound_member(buffer_accesses, buffer, &BufferAccess::buffer); if(i==buffer_accesses.end() || i->buffer!=buffer) @@ -33,6 +33,8 @@ void Synchronizer::write_buffer(VkBuffer buffer, size_t offset, size_t size) i->size = end-begin; } + if(mapped) + i->was_written = true; i->pending_write = true; } @@ -131,7 +133,8 @@ void Synchronizer::barrier(VkCommandBuffer command_buffer) static constexpr VkPipelineStageFlags buffer_read_stages = VK_PIPELINE_STAGE_VERTEX_INPUT_BIT| VK_PIPELINE_STAGE_VERTEX_SHADER_BIT|VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - static constexpr VkPipelineStageFlags buffer_write_stages = VK_PIPELINE_STAGE_TRANSFER_BIT; + static constexpr VkPipelineStageFlags buffer_write_stages = VK_PIPELINE_STAGE_TRANSFER_BIT| + VK_PIPELINE_STAGE_HOST_BIT; vector buffer_barriers; buffer_barriers.reserve(buffer_accesses.size()); diff --git a/source/backends/vulkan/synchronizer.h b/source/backends/vulkan/synchronizer.h index 78c3443e..077274f8 100644 --- a/source/backends/vulkan/synchronizer.h +++ b/source/backends/vulkan/synchronizer.h @@ -36,7 +36,7 @@ private: public: Synchronizer(Device &); - void write_buffer(VkBuffer, std::size_t, std::size_t); + void write_buffer(VkBuffer, std::size_t, std::size_t, bool = false); void split_image_mipmap(VkImage, unsigned, unsigned); void change_image_layout(VkImage, unsigned, int, unsigned, bool); void reset(); diff --git a/source/core/buffer.cpp b/source/core/buffer.cpp index ed0c5523..b6f2e44b 100644 --- a/source/core/buffer.cpp +++ b/source/core/buffer.cpp @@ -31,13 +31,23 @@ void Buffer::data(const void *d) } void Buffer::sub_data(size_t off, size_t sz, const void *d) +{ + check_sub_data(off, sz, "Buffer::sub_data"); + BufferBackend::sub_data(off, sz, d); +} + +Buffer::AsyncTransfer Buffer::sub_data_async(size_t off, size_t sz) +{ + check_sub_data(off, sz, "Buffer::sub_data_async"); + return AsyncTransfer(*this, off, sz); +} + +void Buffer::check_sub_data(size_t off, size_t sz, const char *func) { if(size==0) - throw invalid_operation("Buffer::sub_data"); + throw invalid_operation(func); if(off>get_total_size() || off%size+sz>size) - throw out_of_range("Buffer::sub_data"); - - BufferBackend::sub_data(off, sz, d); + throw out_of_range(func); } void Buffer::require_size(size_t req_sz) const @@ -64,5 +74,30 @@ bool Buffer::unmap() return result; } + +Buffer::AsyncTransfer::AsyncTransfer(Buffer &b, size_t o, size_t s): + buffer(b), + offset(o), + size(s), + dest_addr(0) +{ + allocate(); +} + +Buffer::AsyncTransfer::AsyncTransfer(AsyncTransfer &&other): + buffer(other.buffer), + offset(other.offset), + size(other.size), + dest_addr(other.dest_addr) +{ + other.dest_addr = 0; +} + +Buffer::AsyncTransfer::~AsyncTransfer() +{ + if(dest_addr) + finalize(); +} + } // namespace GL } // namespace Msp diff --git a/source/core/buffer.h b/source/core/buffer.h index 8aba95b1..66eb8d41 100644 --- a/source/core/buffer.h +++ b/source/core/buffer.h @@ -39,6 +39,36 @@ class Buffer: public BufferBackend { friend BufferBackend; +public: + /** + An RAII handle for asynchronously writing data into a buffer. + */ + class AsyncTransfer: public NonCopyable + { + friend BufferBackend; + friend class Buffer; + + private: + Buffer &buffer; + std::size_t offset = 0; + std::size_t size = 0; + void *dest_addr = 0; + + AsyncTransfer(Buffer &, std::size_t, std::size_t); + public: + AsyncTransfer(AsyncTransfer &&); + ~AsyncTransfer(); + + private: + void allocate(); + void finalize(); + + public: + /** Returns an address for writing the data. It should not be used + beyond the lifetime of the object. */ + void *get_address() { return dest_addr; } + }; + private: std::size_t size = 0; BufferUsage usage = STATIC; @@ -57,6 +87,15 @@ public: The range must be fully inside the buffer. */ void sub_data(std::size_t, std::size_t, const void *); + /** Creates an asynchronous transfer for writing data to a range of bytes in + the buffer. While the transfer is pending, the state of the buffer region + is indeterminate. */ + AsyncTransfer sub_data_async(std::size_t, std::size_t); + +private: + void check_sub_data(std::size_t, std::size_t, const char *); + +public: std::size_t get_size() const { return size; } using BufferBackend::get_multiplicity; std::size_t get_total_size() const { return size*get_multiplicity(); } diff --git a/source/core/bufferable.cpp b/source/core/bufferable.cpp index 69588ddd..d8f3c9c4 100644 --- a/source/core/bufferable.cpp +++ b/source/core/bufferable.cpp @@ -1,5 +1,4 @@ #include -#include "buffer.h" #include "bufferable.h" #include "error.h" @@ -170,19 +169,13 @@ Bufferable::AsyncUpdater *Bufferable::create_async_updater() const Bufferable::AsyncUpdater::AsyncUpdater(const Bufferable &b): - bufferable(b) -{ - bufferable.buffer->require_size(bufferable.get_required_buffer_size()); - mapped_address = reinterpret_cast(bufferable.buffer->map()); -} - -Bufferable::AsyncUpdater::~AsyncUpdater() -{ - bufferable.buffer->unmap(); -} + bufferable(b), + transfer(b.buffer->sub_data_async(0, bufferable.get_required_buffer_size())) +{ } void Bufferable::AsyncUpdater::upload_data() { + char *mapped_address = static_cast(transfer.get_address()); bufferable.upload_data(0, mapped_address+bufferable.offset); // Update all bufferables in the same buffer at once for(const Bufferable *b=bufferable.prev_in_buffer; b; b=b->prev_in_buffer) diff --git a/source/core/bufferable.h b/source/core/bufferable.h index b2b25765..d49f0149 100644 --- a/source/core/bufferable.h +++ b/source/core/bufferable.h @@ -2,12 +2,11 @@ #define MSP_GL_BUFFERABLE_H_ #include +#include "buffer.h" namespace Msp { namespace GL { -class Buffer; - /** Base class for things that can store data in buffers. Multiple Bufferables may be put in the same buffer. @@ -26,11 +25,10 @@ public: { private: const Bufferable &bufferable; - char *mapped_address; + Buffer::AsyncTransfer transfer; public: AsyncUpdater(const Bufferable &); - ~AsyncUpdater(); void upload_data(); }; -- 2.43.0