Fix data transfer time ordering

Author: floppyhammer

demo/android/app/src/main/cpp/native_engine.cpp

@@ -30,6 +30,8 @@ void NativeEngine::draw_frame() {
 
     auto surface = pf_swapchain->get_surface_texture();
 
+    pf_blit->update_uniform(encoder);
+
     // Swap chain render pass.
     {
         encoder->begin_render_pass(pf_swapchain->get_render_pass(), surface, Pathfinder::ColorF(0.2, 0.2, 0.2, 1.0));

demo/native/main.cpp

@@ -65,6 +65,8 @@ int main() {
 
         auto encoder = device->create_command_encoder("main encoder");
 
+        blit->update_uniform(encoder);
+
         // Swap chain render pass.
         {
             auto surface_texture = swap_chain->get_surface_texture();

pathfinder/core/d3d9/renderer.cpp

@@ -581,13 +581,12 @@ void RendererD3D9::draw_tiles(uint64_t tile_vertex_buffer_id,
                               const std::shared_ptr<DescriptorSet> &tile_descriptor_set,
                               uint64_t tile_uniform_offset) {
     std::shared_ptr<Texture> target_texture;
+    std::shared_ptr<RenderPass> render_pass;
 
     // If no specific RenderTarget is given, we render to the dst framebuffer.
     if (render_target_id == nullptr) {
         // Check if we should clear the dst framebuffer.
-        encoder->begin_render_pass(clear_dest_texture ? dest_render_pass_clear : dest_render_pass_load,
-                                   dest_texture,
-                                   ColorF());
+        render_pass = clear_dest_texture ? dest_render_pass_clear : dest_render_pass_load;
 
         target_texture = dest_texture;
 
@@ -596,20 +595,15 @@ void RendererD3D9::draw_tiles(uint64_t tile_vertex_buffer_id,
     }
     // Otherwise, we render to the given render target.
     else {
-        auto render_target = get_render_target(*render_target_id);
-
-        auto texture = render_target.texture;
-
         // We always clear a render target.
-        encoder->begin_render_pass(dest_render_pass_clear, texture, ColorF());
+        render_pass = dest_render_pass_clear;
 
-        target_texture = texture;
+        auto render_target = get_render_target(*render_target_id);
+        target_texture = render_target.texture;
     }
 
     Vec2F target_texture_size = target_texture->get_size().to_f32();
 
-    encoder->set_viewport({{0, 0}, target_texture_size.to_i32()});
-
     auto z_buffer_texture = allocator->get_texture(z_buffer_texture_id);
     auto color_texture = allocator->get_texture(dummy_texture_id);
 
@@ -670,6 +664,10 @@ void RendererD3D9::draw_tiles(uint64_t tile_vertex_buffer_id,
         Descriptor::sampled(4, allocator->get_texture(*mask_storage.texture_id), get_default_sampler()),
     });
 
+    encoder->begin_render_pass(render_pass, target_texture, ColorF());
+
+    encoder->set_viewport({{0, 0}, target_texture_size.to_i32()});
+
     encoder->bind_render_pipeline(tile_pipeline);
 
     encoder->bind_vertex_buffers(

pathfinder/gpu/command_encoder.cpp

@@ -209,11 +209,12 @@ void CommandEncoder::write_buffer(const std::shared_ptr<Buffer> &buffer,
         return;
     }
 
-    // Write buffer by memory mapping.
-    if (buffer->get_memory_property() == MemoryProperty::HostVisibleAndCoherent) {
-        buffer->upload_via_mapping(data_size, offset, data);
-        return;
-    }
+    auto device = device_.lock();
+    auto allocation = device->allocate_staging(data_size);
+
+    void *mapped_ptr = device->map_staging(allocation);
+    memcpy(mapped_ptr, data, data_size);
+    device->unmap_staging(allocation);
 
     Command cmd{};
     cmd.type = CommandType::WriteBuffer;
@@ -222,9 +223,12 @@ void CommandEncoder::write_buffer(const std::shared_ptr<Buffer> &buffer,
     args.buffer = buffer.get();
     args.offset = offset;
     args.data_size = data_size;
-    args.data = data;
+    args.staging_buffer = allocation.buffer.get();
+    args.staging_offset = (uint32_t)allocation.offset;
 
     commands_.push_back(cmd);
+
+    track_temporary_resource(allocation.buffer);
 }
 
 void CommandEncoder::read_buffer(const std::shared_ptr<Buffer> &buffer,
@@ -238,15 +242,12 @@ void CommandEncoder::read_buffer(const std::shared_ptr<Buffer> &buffer,
 
     if (data_size == 0 || data == nullptr) {
         Logger::error("Tried to read invalid data from buffer!");
-    }
-
-    // Read buffer by memory mapping.
-    if (buffer->get_memory_property() == MemoryProperty::HostVisibleAndCoherent) {
-        Logger::error("You're trying to read a mappable buffer through a command. It may indicate some bug.");
-        buffer->download_via_mapping(data_size, offset, data);
         return;
     }
 
+    auto device = device_.lock();
+    auto allocation = device->allocate_staging(data_size);
+
     Command cmd{};
     cmd.type = CommandType::ReadBuffer;
 
@@ -255,22 +256,40 @@ void CommandEncoder::read_buffer(const std::shared_ptr<Buffer> &buffer,
     args.offset = offset;
     args.data_size = data_size;
     args.data = data;
+    args.staging_buffer = allocation.buffer.get();
+    args.staging_offset = (uint32_t)allocation.offset;
 
     commands_.push_back(cmd);
+
+    add_callback([allocation, data, data_size, device] {
+        void *mapped_ptr = device->map_staging(allocation);
+        memcpy(data, mapped_ptr, data_size);
+        device->unmap_staging(allocation);
+    });
+
+    track_temporary_resource(allocation.buffer);
 }
 
 void CommandEncoder::write_texture(const std::shared_ptr<Texture> &texture, RectI region, const void *src) {
     auto whole_region = RectI({0, 0}, {texture->get_size()});
 
-    // Invalid region represents the whole texture.
     auto effective_region = region.is_valid() ? region : whole_region;
 
-    // Check if the region is a subset of the whole texture region.
     if (!effective_region.union_rect(whole_region).is_valid() || effective_region.area() == 0) {
         Logger::error("Tried to write invalid region of a texture!");
         return;
     }
 
+    auto pixel_size = get_pixel_size(texture->get_format());
+    size_t data_size = (size_t)effective_region.width() * effective_region.height() * pixel_size;
+
+    auto device = device_.lock();
+    auto allocation = device->allocate_staging(data_size);
+
+    void *mapped_ptr = device->map_staging(allocation);
+    memcpy(mapped_ptr, src, data_size);
+    device->unmap_staging(allocation);
+
     Command cmd{};
     cmd.type = CommandType::WriteTexture;
 
@@ -280,23 +299,30 @@ void CommandEncoder::write_texture(const std::shared_ptr<Texture> &texture, Rect
     args.offset_y = effective_region.top;
     args.width = effective_region.width();
     args.height = effective_region.height();
-    args.data = src;
+    args.staging_buffer = allocation.buffer.get();
+    args.staging_offset = (uint32_t)allocation.offset;
 
     commands_.push_back(cmd);
+
+    track_temporary_resource(allocation.buffer);
 }
 
 void CommandEncoder::read_texture(const std::shared_ptr<Texture> &texture, RectI region, void *dst) {
     auto whole_region = RectI({0, 0}, {texture->get_size()});
 
-    // Invalid region represents the whole texture.
     auto effective_region = region.is_valid() ? region : whole_region;
 
-    // Check if the region is a subset of the whole texture region.
     if (!effective_region.union_rect(whole_region).is_valid() || effective_region.area() == 0) {
         Logger::error("Tried to write invalid region of a texture!");
         return;
     }
 
+    auto pixel_size = get_pixel_size(texture->get_format());
+    size_t data_size = (size_t)effective_region.width() * effective_region.height() * pixel_size;
+
+    auto device = device_.lock();
+    auto allocation = device->allocate_staging(data_size);
+
     Command cmd{};
     cmd.type = CommandType::ReadTexture;
 
@@ -307,8 +333,18 @@ void CommandEncoder::read_texture(const std::shared_ptr<Texture> &texture, RectI
     args.width = effective_region.width();
     args.height = effective_region.height();
     args.data = dst;
+    args.staging_buffer = allocation.buffer.get();
+    args.staging_offset = (uint32_t)allocation.offset;
 
     commands_.push_back(cmd);
+
+    add_callback([allocation, dst, data_size, device] {
+        void *mapped_ptr = device->map_staging(allocation);
+        memcpy(dst, mapped_ptr, data_size);
+        device->unmap_staging(allocation);
+    });
+
+    track_temporary_resource(allocation.buffer);
 }
 
 } // namespace Pathfinder

pathfinder/gpu/command_encoder.h

@@ -52,6 +52,12 @@ enum class CommandType {
     Max,
 };
 
+struct StagingAllocation {
+    std::shared_ptr<Buffer> buffer;
+    size_t offset = 0;
+    void *mapped_ptr = nullptr;
+};
+
 struct Command {
     CommandType type = CommandType::Max;
 
@@ -111,12 +117,16 @@ struct Command {
             uint32_t offset;
             uint32_t data_size;
             const void *data;
+            Buffer *staging_buffer;
+            uint32_t staging_offset;
         } write_buffer;
         struct {
             Buffer *buffer;
             uint32_t offset;
             uint32_t data_size;
             void *data;
+            Buffer *staging_buffer;
+            uint32_t staging_offset;
         } read_buffer;
         struct {
             Texture *texture;
@@ -125,6 +135,8 @@ struct Command {
             uint32_t width;
             uint32_t height;
             const void *data;
+            Buffer *staging_buffer;
+            uint32_t staging_offset;
         } write_texture;
         struct {
             Texture *texture;
@@ -133,6 +145,8 @@ struct Command {
             uint32_t width;
             uint32_t height;
             void *data;
+            Buffer *staging_buffer;
+            uint32_t staging_offset;
         } read_texture;
     } args;
 };
@@ -194,6 +208,7 @@ class CommandEncoder {
 
     // DATA TRANSFER
 
+    /// fixme: should set up a staging buffer every time we need to write to a buffer.
     /**
      * Upload to buffer.
      * @param buffer
@@ -206,6 +221,7 @@ class CommandEncoder {
                               uint32_t data_size,
                               const void *data);
 
+    /// fixme: should set up a staging buffer every time we need to read from a buffer.
     void read_buffer(const std::shared_ptr<Buffer> &buffer, uint32_t offset, uint32_t data_size, void *data);
 
     void write_texture(const std::shared_ptr<Texture> &texture, RectI region, const void *data);
@@ -216,12 +232,17 @@ class CommandEncoder {
         callbacks_.push_back(callback);
     }
 
+    void track_temporary_resource(const std::shared_ptr<Buffer> &buffer) {
+        temp_buffers_.push_back(buffer);
+    }
+
     void invoke_callbacks() {
         for (auto &callback : callbacks_) {
             callback();
         }
 
         callbacks_.clear();
+        temp_buffers_.clear();
     }
 
 protected:
@@ -245,6 +266,9 @@ class CommandEncoder {
     /// Callbacks after the commands are finished (not only submitted) on GPU.
     std::vector<std::function<void()>> callbacks_;
 
+    /// Temporary resources that should be kept alive until the command buffer is finished.
+    std::vector<std::shared_ptr<Buffer>> temp_buffers_;
+
     /// Currently bound pipeline.
     RenderPipeline *render_pipeline_{};
     ComputePipeline *compute_pipeline_{};

pathfinder/gpu/device.h

@@ -72,6 +72,14 @@ class Device : public std::enable_shared_from_this<Device> {
 
     virtual std::shared_ptr<Fence> create_fence(const std::string &label) = 0;
 
+    virtual StagingAllocation allocate_staging(size_t size) = 0;
+
+    virtual void *map_staging(const StagingAllocation &allocation) { return nullptr; }
+
+    virtual void unmap_staging(const StagingAllocation &allocation) {}
+
+    virtual void reset_staging() {}
+
     virtual size_t get_aligned_uniform_size(size_t original_size) = 0;
 
     BackendType get_backend_type() const {