From 47db8e8cbcdfacb32ca46cadaa4ce695e2cb3571 Mon Sep 17 00:00:00 2001
From: PixelyIon <pixelyion@protonmail.com>
Date: Thu, 7 Jul 2022 00:31:58 +0530
Subject: [PATCH] Fix GPU inline copy callback for `Buffer::Write`

The GPU inline copy callback was broken for `Buffer::Write` as it wasn't always called when it needed to be and didn't handle attaching of the buffer to the executor which would cause it to be unlocked. This commit addresses both of these issues, it introduces a `AttachLockedBuffer` method to attach an already locked buffer to the executor.
---
 app/src/main/cpp/skyline/gpu/buffer.cpp            | 10 +++++-----
 .../skyline/gpu/interconnect/command_executor.cpp  | 14 ++++++++++++++
 .../skyline/gpu/interconnect/command_executor.h    |  9 +++++++++
 .../skyline/gpu/interconnect/graphics_context.h    |  4 +++-
 4 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/app/src/main/cpp/skyline/gpu/buffer.cpp b/app/src/main/cpp/skyline/gpu/buffer.cpp
index b1b0e916..c4cdd064 100644
--- a/app/src/main/cpp/skyline/gpu/buffer.cpp
+++ b/app/src/main/cpp/skyline/gpu/buffer.cpp
@@ -210,12 +210,12 @@ namespace skyline::gpu {
 
         std::memcpy(mirror.data() + offset, data.data(), data.size()); // Always copy to mirror since any CPU side reads will need the up-to-date contents
 
-        if (PollFence())
-            // Perform a GPU-side inline update for the buffer contents if this buffer is host immutable since we can't directly modify the backing
-            gpuCopyCallback();
-        else
-            // If that's not the case we don't need to do any GPU-side sequencing here, we can write directly to the backing and the sequencing for it will be handled at usage time
+        if (!usedByContext && PollFence())
+            // We can write directly to the backing as long as this resource isn't being actively used by a past workload (in the current context or another)
             std::memcpy(backing.data() + offset, data.data(), data.size());
+        else
+            // If this buffer is host immutable, perform a GPU-side inline update for the buffer contents since we can't directly modify the backing
+            gpuCopyCallback();
     }
 
     BufferView Buffer::GetView(vk::DeviceSize offset, vk::DeviceSize size, vk::Format format) {
diff --git a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp
index 95a1ed8e..8e012ac5 100644
--- a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp
+++ b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp
@@ -135,6 +135,20 @@ namespace skyline::gpu::interconnect {
         return didLock;
     }
 
+    void CommandExecutor::AttachLockedBuffer(BufferView &view, ContextLock<BufferView> &lock) {
+        if (!bufferManagerLock)
+            // See AttachTexture(...)
+            bufferManagerLock.emplace(gpu.buffer);
+
+        if (lock.isFirst) {
+            attachedBuffers.emplace_back(view->buffer);
+            lock.isFirst = false;
+        }
+
+        if (!attachedBufferDelegates.contains(view.bufferDelegate))
+            attachedBufferDelegates.emplace(view.bufferDelegate);
+    }
+
     void CommandExecutor::AttachDependency(const std::shared_ptr<void> &dependency) {
         cycle->AttachObject(dependency);
     }
diff --git a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h
index b63ea3f8..c78fa3a4 100644
--- a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h
+++ b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h
@@ -126,6 +126,15 @@ namespace skyline::gpu::interconnect {
          */
         bool AttachBuffer(BufferView &view);
 
+        /**
+         * @brief Attach the lifetime of a buffer that's already locked to the command buffer
+         * @return If this is the first usage of the backing of this resource within this execution
+         * @note The supplied buffer **must** be locked with the executor's tag
+         * @note There must be no other external locks on the buffer aside from the supplied lock
+         * @note This'll automatically handle syncing of the buffer in the most optimal way possible
+         */
+        void AttachLockedBuffer(BufferView &view, ContextLock<BufferView>& lock);
+
         /**
          * @brief Attach the lifetime of the fence cycle dependency to the command buffer
          */
diff --git a/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h b/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h
index c072c564..323226f3 100644
--- a/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h
+++ b/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h
@@ -646,7 +646,9 @@ namespace skyline::gpu::interconnect {
                 view.Write(lock.isFirst, pExecutor.cycle, []() {
                     // TODO: see Read()
                     Logger::Warn("GPU dirty buffer reads for attached buffers are unimplemented");
-                }, [&megaBuffer, &pExecutor, srcCpuBuf, dstOffset, view = this->view]() {
+                }, [&megaBuffer, &pExecutor, srcCpuBuf, dstOffset, &view = this->view, &lock]() {
+                    pExecutor.AttachLockedBuffer(view, lock);
+
                     auto srcGpuOffset{megaBuffer.Push(srcCpuBuf)};
                     auto srcGpuBuf{megaBuffer.GetBacking()};
                     pExecutor.AddOutsideRpCommand([=](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr<FenceCycle> &, GPU &) {