From cfeb8098dbff4c68265542214c0ce42ec0eea908 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Wed, 8 Dec 2021 14:13:10 +0530 Subject: [PATCH] Attach `TextureView`/`BufferView` Lifetime to `FenceCycle` The lifetime of a texture and buffer view is now bound by the `FenceCycle` in `CommandExecutor`, this ensures that a `VkImageView` isn't destroyed prior to usage leading to UB. --- app/src/main/cpp/skyline/gpu/buffer.h | 4 +-- .../gpu/interconnect/command_executor.cpp | 26 ++++++++++--------- .../gpu/interconnect/command_executor.h | 6 +++-- .../gpu/interconnect/graphics_context.h | 2 +- .../main/cpp/skyline/gpu/texture/texture.h | 2 +- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/app/src/main/cpp/skyline/gpu/buffer.h b/app/src/main/cpp/skyline/gpu/buffer.h index 5f9fdf95..a663492c 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.h +++ b/app/src/main/cpp/skyline/gpu/buffer.h @@ -10,7 +10,7 @@ namespace skyline::gpu { * @brief A descriptor for a GPU buffer on the guest */ struct GuestBuffer { - using Mappings = boost::container::small_vector, 3>; + using Mappings = boost::container::small_vector, 3>; Mappings mappings; //!< Spans to CPU memory for the underlying data backing this buffer /** @@ -114,7 +114,7 @@ namespace skyline::gpu { * @note The object **must** be locked prior to accessing any members as values will be mutated * @note This class conforms to the Lockable and BasicLockable C++ named requirements */ - struct BufferView { + struct BufferView : public FenceCycleDependency, public std::enable_shared_from_this { std::shared_ptr buffer; vk::DeviceSize offset; vk::DeviceSize range; 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 522b6eed..190f3100 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp +++ b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp @@ -25,30 +25,32 @@ namespace skyline::gpu::interconnect { return newRenderPass; } - void CommandExecutor::AttachTexture(const std::shared_ptr &texture) { - if (!syncTextures.contains(texture.get())) { + void CommandExecutor::AttachTexture(TextureView *view) { + auto texture{view->texture.get()}; + if (!syncTextures.contains(texture)) { texture->WaitOnFence(); texture->cycle = cycle; - cycle->AttachObject(texture); - syncTextures.emplace(texture.get()); + cycle->AttachObject(view->shared_from_this()); + syncTextures.emplace(texture); } } - void CommandExecutor::AttachBuffer(const std::shared_ptr &buffer) { - if (!syncBuffers.contains(buffer.get())) { + void CommandExecutor::AttachBuffer(BufferView *view) { + auto buffer{view->buffer.get()}; + if (!syncBuffers.contains(buffer)) { buffer->WaitOnFence(); buffer->cycle = cycle; - cycle->AttachObject(buffer); - syncBuffers.emplace(buffer.get()); + cycle->AttachObject(view->shared_from_this()); + syncBuffers.emplace(buffer); } } void CommandExecutor::AddSubpass(std::function &, GPU &, vk::RenderPass, u32)> &&function, vk::Rect2D renderArea, span inputAttachments, span colorAttachments, TextureView *depthStencilAttachment) { for (const auto &attachments : {inputAttachments, colorAttachments}) for (const auto &attachment : attachments) - AttachTexture(attachment->texture); + AttachTexture(attachment); if (depthStencilAttachment) - AttachTexture(depthStencilAttachment->texture); + AttachTexture(depthStencilAttachment); bool newRenderPass{CreateRenderPass(renderArea)}; renderPass->AddSubpass(inputAttachments, colorAttachments, depthStencilAttachment ? &*depthStencilAttachment : nullptr); @@ -59,7 +61,7 @@ namespace skyline::gpu::interconnect { } void CommandExecutor::AddClearColorSubpass(TextureView *attachment, const vk::ClearColorValue &value) { - AttachTexture(attachment->texture); + AttachTexture(attachment); bool newRenderPass{CreateRenderPass(vk::Rect2D{ .extent = attachment->texture->dimensions, @@ -90,7 +92,7 @@ namespace skyline::gpu::interconnect { } void CommandExecutor::AddClearDepthStencilSubpass(TextureView *attachment, const vk::ClearDepthStencilValue &value) { - AttachTexture(attachment->texture); + AttachTexture(attachment); bool newRenderPass{CreateRenderPass(vk::Rect2D{ .extent = attachment->texture->dimensions, 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 b430983e..6fef8650 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h +++ b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h @@ -34,15 +34,17 @@ namespace skyline::gpu::interconnect { /** * @brief Attach the lifetime of the texture to the command buffer + * @note The supplied texture **must** be locked by the calling thread * @note This'll automatically handle syncing of the texture in the most optimal way possible */ - void AttachTexture(const std::shared_ptr &texture); + void AttachTexture(TextureView *view); /** * @brief Attach the lifetime of a buffer to the command buffer + * @note The supplied buffer **must** be locked by the calling thread * @note This'll automatically handle syncing of the buffer in the most optimal way possible */ - void AttachBuffer(const std::shared_ptr &buffer); + void AttachBuffer(BufferView *view); /** * @brief Adds a command that needs to be executed inside a subpass configured with certain attachments 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 cfb2a3bb..def59b06 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h +++ b/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h @@ -1308,7 +1308,7 @@ namespace skyline::gpu::interconnect { vertexBindingDivisorsDescriptions.push_back(vertexBuffer.bindingDivisorDescription); vertexBufferLocks.emplace_back(*vertexBufferView); - executor.AttachBuffer(vertexBufferView->buffer); + executor.AttachBuffer(vertexBufferView); vertexBufferHandles[index] = vertexBufferView->buffer->GetBacking(); vertexBufferOffsets[index] = vertexBufferView->offset; } diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.h b/app/src/main/cpp/skyline/gpu/texture/texture.h index 3573fe3f..08fdb6b6 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.h +++ b/app/src/main/cpp/skyline/gpu/texture/texture.h @@ -293,7 +293,7 @@ namespace skyline::gpu { * @note The object **must** be locked prior to accessing any members as values will be mutated * @note This class conforms to the Lockable and BasicLockable C++ named requirements */ - class TextureView { + class TextureView : public FenceCycleDependency, public std::enable_shared_from_this { private: std::optional view;