From 6eda1777c56c58e2bb1493abeadf03f6d01a17d5 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Tue, 7 Dec 2021 02:43:14 +0530 Subject: [PATCH] Rework `TextureView` to be disconnected from `Texture` We want `TextureView`(s) to be disconnected from the backing on the host and instead represent a specific texture on the guest with a backing that can change depending on mapping of new textures which'd invalidate the backing but should now be automatically repointed to an appropriate new backing. This approach also requires locking of the backing to function as it is mutable till it has been locked or the backing has an attached `FenceCycle` that hasn't been signaled which will be added for `CommandExecutor` in a subsequent commit. --- .../gpu/interconnect/command_executor.cpp | 10 ++-- .../gpu/interconnect/command_executor.h | 4 +- .../gpu/interconnect/command_nodes.cpp | 22 ++++---- .../skyline/gpu/interconnect/command_nodes.h | 4 +- .../gpu/interconnect/graphics_context.h | 23 ++++---- .../main/cpp/skyline/gpu/texture/texture.cpp | 55 ++++++++++++++++--- .../main/cpp/skyline/gpu/texture/texture.h | 34 ++++++++++-- .../main/cpp/skyline/gpu/texture_manager.cpp | 30 +++++----- .../main/cpp/skyline/gpu/texture_manager.h | 3 +- .../hosbinder/GraphicBufferProducer.cpp | 2 +- 10 files changed, 125 insertions(+), 62 deletions(-) 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 117dd7ac..45a2cc77 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp +++ b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.cpp @@ -21,10 +21,10 @@ namespace skyline::gpu::interconnect { return newRenderPass; } - void CommandExecutor::AddSubpass(const std::function &, GPU &)> &function, vk::Rect2D renderArea, std::vector inputAttachments, std::vector colorAttachments, std::optional depthStencilAttachment) { + void CommandExecutor::AddSubpass(const std::function &, GPU &)> &function, vk::Rect2D renderArea, span inputAttachments, span colorAttachments, TextureView *depthStencilAttachment) { for (const auto &attachments : {inputAttachments, colorAttachments}) for (const auto &attachment : attachments) - syncTextures.emplace(attachment.texture.get()); + syncTextures.emplace(attachment->texture.get()); if (depthStencilAttachment) syncTextures.emplace(depthStencilAttachment->texture.get()); @@ -36,9 +36,9 @@ namespace skyline::gpu::interconnect { nodes.emplace_back(std::in_place_type_t(), function); } - void CommandExecutor::AddClearColorSubpass(TextureView attachment, const vk::ClearColorValue &value) { + void CommandExecutor::AddClearColorSubpass(TextureView *attachment, const vk::ClearColorValue &value) { bool newRenderPass{CreateRenderPass(vk::Rect2D{ - .extent = attachment.texture->dimensions, + .extent = attachment->texture->dimensions, })}; renderPass->AddSubpass({}, attachment, nullptr); @@ -46,7 +46,7 @@ namespace skyline::gpu::interconnect { if (!newRenderPass) nodes.emplace_back(std::in_place_type_t()); } else { - auto function{[scissor = attachment.texture->dimensions, value](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &, GPU &) { + auto function{[scissor = attachment->texture->dimensions, value](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &, GPU &, vk::RenderPass, u32) { commandBuffer.clearAttachments(vk::ClearAttachment{ .aspectMask = vk::ImageAspectFlagBits::eColor, .colorAttachment = 0, 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 70f9a525..ddd29e81 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h +++ b/app/src/main/cpp/skyline/gpu/interconnect/command_executor.h @@ -31,13 +31,13 @@ namespace skyline::gpu::interconnect { * @brief Adds a command that needs to be executed inside a subpass configured with certain attachments * @note Any texture supplied to this **must** be locked by the calling thread, it should also undergo no persistent layout transitions till execution */ - void AddSubpass(const std::function &, GPU &)> &function, vk::Rect2D renderArea, std::vector inputAttachments = {}, std::vector colorAttachments = {}, std::optional depthStencilAttachment = {}); + void AddSubpass(const std::function &, GPU &)> &&function, vk::Rect2D renderArea, span inputAttachments = {}, span colorAttachments = {}, TextureView *depthStencilAttachment = {}); /** * @brief Adds a subpass that clears the entirety of the specified attachment with a value, it may utilize VK_ATTACHMENT_LOAD_OP_CLEAR for a more efficient clear when possible * @note Any texture supplied to this **must** be locked by the calling thread, it should also undergo no persistent layout transitions till execution */ - void AddClearColorSubpass(TextureView attachment, const vk::ClearColorValue& value); + void AddClearColorSubpass(TextureView *attachment, const vk::ClearColorValue &value); /** * @brief Execute all the nodes and submit the resulting command buffer to the GPU diff --git a/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.cpp b/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.cpp index eb37bb83..d128f9e1 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.cpp +++ b/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.cpp @@ -13,21 +13,21 @@ namespace skyline::gpu::interconnect::node { } } - u32 RenderPassNode::AddAttachment(TextureView &view) { + u32 RenderPassNode::AddAttachment(TextureView *view) { auto &textures{storage->textures}; - auto texture{std::find(textures.begin(), textures.end(), view.texture)}; + auto texture{std::find(textures.begin(), textures.end(), view->texture)}; if (texture == textures.end()) - textures.push_back(view.texture); + textures.push_back(view->texture); - auto vkView{view.GetView()}; + auto vkView{view->GetView()}; auto attachment{std::find(attachments.begin(), attachments.end(), vkView)}; if (attachment == attachments.end()) { // If we cannot find any matches for the specified attachment, we add it as a new one attachments.push_back(vkView); attachmentDescriptions.push_back(vk::AttachmentDescription{ - .format = *view.format, - .initialLayout = view.texture->layout, - .finalLayout = view.texture->layout, + .format = *view->format, + .initialLayout = view->texture->layout, + .finalLayout = view->texture->layout, }); return static_cast(attachments.size() - 1); } else { @@ -97,14 +97,14 @@ namespace skyline::gpu::interconnect::node { } } - void RenderPassNode::AddSubpass(span inputAttachments, span colorAttachments, TextureView *depthStencilAttachment) { + void RenderPassNode::AddSubpass(span inputAttachments, span colorAttachments, TextureView *depthStencilAttachment) { attachmentReferences.reserve(attachmentReferences.size() + inputAttachments.size() + colorAttachments.size() + (depthStencilAttachment ? 1 : 0)); auto inputAttachmentsOffset{attachmentReferences.size() * sizeof(vk::AttachmentReference)}; for (auto &attachment : inputAttachments) { attachmentReferences.push_back(vk::AttachmentReference{ .attachment = AddAttachment(attachment), - .layout = attachment.texture->layout, + .layout = attachment->texture->layout, }); } @@ -112,14 +112,14 @@ namespace skyline::gpu::interconnect::node { for (auto &attachment : colorAttachments) { attachmentReferences.push_back(vk::AttachmentReference{ .attachment = AddAttachment(attachment), - .layout = attachment.texture->layout, + .layout = attachment->texture->layout, }); } auto depthStencilAttachmentOffset{attachmentReferences.size() * sizeof(vk::AttachmentReference)}; if (depthStencilAttachment) { attachmentReferences.push_back(vk::AttachmentReference{ - .attachment = AddAttachment(*depthStencilAttachment), + .attachment = AddAttachment(depthStencilAttachment), .layout = depthStencilAttachment->texture->layout, }); } diff --git a/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.h b/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.h index f7511227..b977ee2d 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.h +++ b/app/src/main/cpp/skyline/gpu/interconnect/command_nodes.h @@ -71,12 +71,12 @@ namespace skyline::gpu::interconnect::node { * @note Any preservation of attachments from previous subpasses is automatically handled by this * @return The index of the attachment in the render pass which can be utilized with VkAttachmentReference */ - u32 AddAttachment(TextureView &view); + u32 AddAttachment(TextureView *view); /** * @brief Creates a subpass with the attachments bound in the specified order */ - void AddSubpass(span inputAttachments, span colorAttachments, TextureView *depthStencilAttachment); + void AddSubpass(span inputAttachments, span colorAttachments, TextureView *depthStencilAttachment); /** * @brief Clears a color attachment in the current subpass with VK_ATTACHMENT_LOAD_OP_LOAD 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 388b4def..b27975cf 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h +++ b/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h @@ -74,7 +74,7 @@ namespace skyline::gpu::interconnect { IOVA iova; u32 widthBytes; //!< The width in bytes for linear textures GuestTexture guest; - std::optional view; + std::shared_ptr view; RenderTarget() { guest.dimensions = texture::Dimensions(1, 1, 1); // We want the depth to be 1 by default (It cannot be set by the application) @@ -233,7 +233,7 @@ namespace skyline::gpu::interconnect { renderTarget.view.reset(); } - const TextureView *GetRenderTarget(size_t index) { + TextureView *GetRenderTarget(size_t index) { auto &renderTarget{renderTargets.at(index)}; if (renderTarget.disabled) return nullptr; @@ -249,7 +249,7 @@ namespace skyline::gpu::interconnect { renderTarget.guest.type = static_cast(renderTarget.guest.dimensions.GetType()); renderTarget.view = gpu.texture.FindOrCreate(renderTarget.guest); - return &renderTarget.view.value(); + return renderTarget.view.get(); } void UpdateRenderTargetControl(maxwell3d::RenderTargetControl control) { @@ -326,10 +326,9 @@ namespace skyline::gpu::interconnect { void ClearBuffers(maxwell3d::ClearBuffers clear) { auto renderTargetIndex{renderTargetControl[clear.renderTargetId]}; - auto renderTargetPointer{GetRenderTarget(renderTargetIndex)}; - if (renderTargetPointer) { - auto renderTarget{*renderTargetPointer}; - std::lock_guard lock(*renderTarget.texture); + auto renderTarget{GetRenderTarget(renderTargetIndex)}; + if (renderTarget) { + std::lock_guard lock(*renderTarget->texture); vk::ImageAspectFlags aspect{}; if (clear.depth) @@ -338,21 +337,21 @@ namespace skyline::gpu::interconnect { aspect |= vk::ImageAspectFlagBits::eStencil; if (clear.red || clear.green || clear.blue || clear.alpha) aspect |= vk::ImageAspectFlagBits::eColor; - aspect &= renderTarget.format->vkAspect; + aspect &= renderTarget->format->vkAspect; if (aspect == vk::ImageAspectFlags{}) return; auto scissor{scissors.at(renderTargetIndex)}; - scissor.extent.width = static_cast(std::min(static_cast(renderTarget.texture->dimensions.width) - scissor.offset.x, + scissor.extent.width = static_cast(std::min(static_cast(renderTarget->texture->dimensions.width) - scissor.offset.x, static_cast(scissor.extent.width))); - scissor.extent.height = static_cast(std::min(static_cast(renderTarget.texture->dimensions.height) - scissor.offset.y, + scissor.extent.height = static_cast(std::min(static_cast(renderTarget->texture->dimensions.height) - scissor.offset.y, static_cast(scissor.extent.height))); if (scissor.extent.width == 0 || scissor.extent.height == 0) return; - if (scissor.extent.width == renderTarget.texture->dimensions.width && scissor.extent.height == renderTarget.texture->dimensions.height && renderTarget.range.baseArrayLayer == 0 && renderTarget.range.layerCount == 1 && clear.layerId == 0) { + if (scissor.extent.width == renderTarget->texture->dimensions.width && scissor.extent.height == renderTarget->texture->dimensions.height && renderTarget->range.baseArrayLayer == 0 && renderTarget->range.layerCount == 1 && clear.layerId == 0) { executor.AddClearColorSubpass(renderTarget, clearColorValue); } else { executor.AddSubpass([aspect, clearColorValue = clearColorValue, layerId = clear.layerId, scissor](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &, GPU &) { @@ -366,7 +365,7 @@ namespace skyline::gpu::interconnect { .layerCount = 1, }); }, vk::Rect2D{ - .extent = renderTarget.texture->dimensions, + .extent = renderTarget->texture->dimensions, }, {}, {renderTarget}); } } diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.cpp b/app/src/main/cpp/skyline/gpu/texture/texture.cpp index ad345799..9bac38b2 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.cpp +++ b/app/src/main/cpp/skyline/gpu/texture/texture.cpp @@ -8,6 +8,39 @@ #include "copy.h" namespace skyline::gpu { + void TextureView::lock() { + auto currentBacking{std::atomic_load(&texture)}; + while (true) { + currentBacking->lock(); + + auto newBacking{std::atomic_load(&texture)}; + if (currentBacking == newBacking) + return; + + currentBacking->unlock(); + currentBacking = newBacking; + } + } + + void TextureView::unlock() { + texture->unlock(); + } + + bool TextureView::try_lock() { + auto currentBacking{std::atomic_load(&texture)}; + while (true) { + bool success{currentBacking->try_lock()}; + + auto newBacking{std::atomic_load(&texture)}; + if (currentBacking == newBacking) + return success; + + if (success) + currentBacking->unlock(); + currentBacking = newBacking; + } + } + std::shared_ptr Texture::SynchronizeHostImpl(const std::shared_ptr &pCycle) { if (!guest) throw exception("Synchronization of host textures requires a valid guest texture to synchronize from"); @@ -130,6 +163,7 @@ namespace skyline::gpu { Texture::TextureBufferCopy::TextureBufferCopy(std::shared_ptr texture, std::shared_ptr stagingBuffer) : texture(std::move(texture)), stagingBuffer(std::move(stagingBuffer)) {} Texture::TextureBufferCopy::~TextureBufferCopy() { + TRACE_EVENT("gpu", "Texture::TextureBufferCopy"); texture->CopyToGuest(stagingBuffer ? stagingBuffer->data() : std::get(texture->backing).data()); } @@ -362,6 +396,18 @@ namespace skyline::gpu { } } + std::shared_ptr Texture::GetView(vk::ImageViewType type, vk::ImageSubresourceRange range, texture::Format pFormat, vk::ComponentMapping mapping) { + for (const auto &viewWeak : views) { + auto view{viewWeak.lock()}; + if (view && type == view->type && pFormat == view->format && range == view->range && mapping == view->mapping) + return view; + } + + auto view{std::make_shared(shared_from_this(), type, range, pFormat, mapping)}; + views.push_back(view); + return view; + } + void Texture::CopyFrom(std::shared_ptr source, const vk::ImageSubresourceRange &subresource) { WaitOnBacking(); WaitOnFence(); @@ -480,13 +526,6 @@ namespace skyline::gpu { .subresourceRange = range, }; - auto &views{texture->views}; - auto iterator{std::find_if(views.begin(), views.end(), [&](const std::pair &item) { - return item.first == createInfo; - })}; - if (iterator != views.end()) - return *iterator->second; - - return *views.emplace_back(createInfo, vk::raii::ImageView(texture->gpu.vkDevice, createInfo)).second; + return *view.emplace(texture->gpu.vkDevice, createInfo); } } diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.h b/app/src/main/cpp/skyline/gpu/texture/texture.h index 5bbf5d68..3573fe3f 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.h +++ b/app/src/main/cpp/skyline/gpu/texture/texture.h @@ -290,10 +290,12 @@ namespace skyline::gpu { /** * @brief A view into a specific subresource of a Texture + * @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 { private: - vk::raii::ImageView *view{}; + std::optional view; public: std::shared_ptr texture; @@ -308,7 +310,26 @@ namespace skyline::gpu { TextureView(std::shared_ptr texture, vk::ImageViewType type, vk::ImageSubresourceRange range, texture::Format format = {}, vk::ComponentMapping mapping = {}); /** - * @return A Vulkan Image View that corresponds to the properties of this view + * @brief Acquires an exclusive lock on the backing texture for the calling thread + * @note Naming is in accordance to the BasicLockable named requirement + */ + void lock(); + + /** + * @brief Relinquishes an existing lock on the backing texture by the calling thread + * @note Naming is in accordance to the BasicLockable named requirement + */ + void unlock(); + + /** + * @brief Attempts to acquire an exclusive lock on the backing texture but returns immediately if it's captured by another thread + * @note Naming is in accordance to the Lockable named requirement + */ + bool try_lock(); + + /** + * @return A VkImageView that corresponds to the properties of this view + * @note The texture **must** be locked prior to calling this */ vk::ImageView GetView(); @@ -329,7 +350,7 @@ namespace skyline::gpu { using BackingType = std::variant; BackingType backing; //!< The Vulkan image that backs this texture, it is nullable - std::vector> views; //!< VkImageView(s) that have been constructed from this Texture, utilized for caching + std::vector> views; //!< TextureView(s) that are backed by this Texture, used for repointing to a new Texture on deletion friend TextureManager; friend TextureView; @@ -461,8 +482,6 @@ namespace skyline::gpu { /** * @brief Synchronizes the host texture with the guest after it has been modified - * @param commandBuffer An optional command buffer that the command will be recorded into rather than creating one as necessary - * @note A command buffer **must** not be submitted if it is created just for the command as it can be more efficient to allocate one within the function as necessary which is done when one isn't passed in * @note The texture **must** be locked prior to calling this * @note The guest texture backing should exist prior to calling this */ @@ -491,6 +510,11 @@ namespace skyline::gpu { */ void SynchronizeGuestWithBuffer(const vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &cycle); + /** + * @return A cached or newly created view into this texture with the supplied attributes + */ + std::shared_ptr GetView(vk::ImageViewType type, vk::ImageSubresourceRange range, texture::Format format = {}, vk::ComponentMapping mapping = {}); + /** * @brief Copies the contents of the supplied source texture into the current texture */ diff --git a/app/src/main/cpp/skyline/gpu/texture_manager.cpp b/app/src/main/cpp/skyline/gpu/texture_manager.cpp index e679dd80..47653035 100644 --- a/app/src/main/cpp/skyline/gpu/texture_manager.cpp +++ b/app/src/main/cpp/skyline/gpu/texture_manager.cpp @@ -6,20 +6,22 @@ namespace skyline::gpu { TextureManager::TextureManager(GPU &gpu) : gpu(gpu) {} - TextureView TextureManager::FindOrCreate(const GuestTexture &guestTexture) { + std::shared_ptr TextureManager::FindOrCreate(const GuestTexture &guestTexture) { auto guestMapping{guestTexture.mappings.front()}; - // Iterate over all textures that overlap with the first mapping of the guest texture and compare the mappings: - // 1) All mappings match up perfectly, we check that the rest of the supplied mappings correspond to mappings in the texture - // 1.1) If they match as well, we check for format/dimensions/tiling config matching the texture and return or move onto (3) - // 2) Only a contiguous range of mappings match, we check for if the overlap is meaningful with layout math, it can go two ways: - // 2.1) If there is a meaningful overlap, we check for format/dimensions/tiling config compatibility and return or move onto (3) - // 2.2) If there isn't, we move onto (3) - // 3) If there's another overlap we go back to (1) with it else we go to (4) - // 4) We check all the overlapping texture for if they're in the texture pool: - // 4.1) If they are, we do nothing to them - // 4.2) If they aren't, we delete them from the map - // 5) Create a new texture and insert it in the map then return it + /* + * Iterate over all textures that overlap with the first mapping of the guest texture and compare the mappings: + * 1) All mappings match up perfectly, we check that the rest of the supplied mappings correspond to mappings in the texture + * 1.1) If they match as well, we check for format/dimensions/tiling config matching the texture and return or move onto (3) + * 2) Only a contiguous range of mappings match, we check for if the overlap is meaningful with layout math, it can go two ways: + * 2.1) If there is a meaningful overlap, we check for format/dimensions/tiling config compatibility and return or move onto (3) + * 2.2) If there isn't, we move onto (3) + * 3) If there's another overlap we go back to (1) with it else we go to (4) + * 4) We check all the overlapping texture for if they're in the texture pool: + * 4.1) If they are, we do nothing to them + * 4.2) If they aren't, we delete them from the map + * 5) Create a new texture and insert it in the map then return it + */ std::scoped_lock lock(mutex); std::shared_ptr match{}; @@ -45,7 +47,7 @@ namespace skyline::gpu { auto &matchGuestTexture{*hostMapping->texture->guest}; if (matchGuestTexture.format->IsCompatible(*guestTexture.format) && matchGuestTexture.dimensions == guestTexture.dimensions && matchGuestTexture.tileConfig == guestTexture.tileConfig) { auto &texture{hostMapping->texture}; - return TextureView(texture, static_cast(guestTexture.type), vk::ImageSubresourceRange{ + return texture->GetView(static_cast(guestTexture.type), vk::ImageSubresourceRange{ .aspectMask = guestTexture.format->vkAspect, .levelCount = texture->mipLevels, .layerCount = texture->layerCount, @@ -76,7 +78,7 @@ namespace skyline::gpu { textures.emplace(mapping, TextureMapping{texture, it, guestMapping}); } - return TextureView(texture, static_cast(guestTexture.type), vk::ImageSubresourceRange{ + return texture->GetView(static_cast(guestTexture.type), vk::ImageSubresourceRange{ .aspectMask = guestTexture.format->vkAspect, .levelCount = texture->mipLevels, .layerCount = texture->layerCount, diff --git a/app/src/main/cpp/skyline/gpu/texture_manager.h b/app/src/main/cpp/skyline/gpu/texture_manager.h index 908d5d27..dc87f6cd 100644 --- a/app/src/main/cpp/skyline/gpu/texture_manager.h +++ b/app/src/main/cpp/skyline/gpu/texture_manager.h @@ -4,7 +4,6 @@ #pragma once #include "texture/texture.h" -#include namespace skyline::gpu { /** @@ -36,6 +35,6 @@ namespace skyline::gpu { /** * @return A pre-existing or newly created Texture object which matches the specified criteria */ - TextureView FindOrCreate(const GuestTexture &guestTexture); + std::shared_ptr FindOrCreate(const GuestTexture &guestTexture); }; } diff --git a/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp b/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp index 9f1e66fa..39103d40 100644 --- a/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp +++ b/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp @@ -344,7 +344,7 @@ namespace skyline::service::hosbinder { } gpu::GuestTexture guestTexture(span(nvMapHandleObj->GetPointer() + surface.offset, surface.size), gpu::texture::Dimensions(surface.width, surface.height), format, tileConfig, gpu::texture::TextureType::e2D); - buffer.texture = state.gpu->texture.FindOrCreate(guestTexture).texture; + buffer.texture = state.gpu->texture.FindOrCreate(guestTexture)->texture; } switch (transform) {