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.

diff --git a/source/backends/opengl/buffer_backend.cpp b/source/backends/opengl/buffer_backend.cpp
index 6417633b72a34a6866ff4b73d761fa56bd697dc5..c59540834615c61062ccd81ad33428fe90c764c4 100644 (file)
--- 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 5439548f886fd1d81ba0ac16181f12c4a213f28f..aaf415703db8158e5cba7cacd4b6d93ee294f769 100644 (file)
--- 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, &region);
-               });
-
+       Buffer::AsyncTransfer transfer(*static_cast<Buffer *>(this), off, sz);
        const char *src = static_cast<const char *>(d);
-       copy(src, src+sz, static_cast<char *>(staging));
-       tq.finalize_transfer(staging);
+       copy(src, src+sz, static_cast<char *>(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<char *>(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, &region);
+                       });
+       }
+}
+
+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 93f48b847f9290cc456d06fe4d108bea1068bf2b..9995f248c89d06503dc00a507b2748e08f3cb964 100644 (file)
--- 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<VkBufferMemoryBarrier> buffer_barriers;
        buffer_barriers.reserve(buffer_accesses.size());

diff --git a/source/backends/vulkan/synchronizer.h b/source/backends/vulkan/synchronizer.h
index 78c3443ed0761d32e19326b6fd7473602a832e6d..077274f8200a0fd1fad5417dcde7d92ba9995b4e 100644 (file)
--- 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 ed0c552360ac8fad2345236e2024d17455200334..b6f2e44b9e55555bebcfa08b80bf1ffeadb4a3c5 100644 (file)
--- 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 8aba95b10f08176be725bbafb0aa4b5771e6e8c8..66eb8d41fc44e76c91eddc4865b2a2f1dcb43c9d 100644 (file)
--- 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 69588dddef301d72f0eda7128209a5fba632cc63..d8f3c9c43f9622602851e03f38dd72221031d0b3 100644 (file)
--- a/source/core/bufferable.cpp
+++ b/source/core/bufferable.cpp
@@ -1,5 +1,4 @@
 #include <stdexcept>
-#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<char *>(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<char *>(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 b2b25765af2ef67b88c3c9e91bb7e785ee97b1b5..d49f0149ca4cb79bd659d8abeccdf7ce579cdf13 100644 (file)
--- a/source/core/bufferable.h
+++ b/source/core/bufferable.h
@@ -2,12 +2,11 @@
 #define MSP_GL_BUFFERABLE_H_
 
 #include <msp/core/noncopyable.h>
+#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();
        };