From 47bc3b4d9966fa28425ca03b0cbd751dd66cdb73 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Sun, 1 May 2022 20:31:36 +0530 Subject: [PATCH] Fix Render Pass Cache The Vulkan render pass cache was fundamentally broken since it was designed around the Render Pass Compatibility clause due to being designed for framebuffer compatibility initially. As this scope was extended to a general render pass cache, the amount of data in the key was not extended to include everything it should have. This commit introduces the missing pieces in the RP cache and simplifies the underlying code in the process. --- app/src/main/cpp/skyline/gpu/cache/common.h | 23 --- .../cpp/skyline/gpu/cache/framebuffer_cache.h | 3 +- .../gpu/cache/graphics_pipeline_cache.h | 25 ++- .../skyline/gpu/cache/renderpass_cache.cpp | 145 +++++++++--------- .../cpp/skyline/gpu/cache/renderpass_cache.h | 31 ++-- 5 files changed, 108 insertions(+), 119 deletions(-) delete mode 100644 app/src/main/cpp/skyline/gpu/cache/common.h diff --git a/app/src/main/cpp/skyline/gpu/cache/common.h b/app/src/main/cpp/skyline/gpu/cache/common.h deleted file mode 100644 index f9b16cf9..00000000 --- a/app/src/main/cpp/skyline/gpu/cache/common.h +++ /dev/null @@ -1,23 +0,0 @@ -// SPDX-License-Identifier: MPL-2.0 -// Copyright © 2022 Skyline Team and Contributors (https://github.com/skyline-emu/) - -#pragma once - -#include -#include - -namespace skyline::gpu::cache { - /** - * @brief All unique metadata in a single attachment for a compatible render pass according to Render Pass Compatibility clause in the Vulkan specification - * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#renderpass-compatibility - * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VkAttachmentDescription.html - */ - struct AttachmentMetadata { - vk::Format format; - vk::SampleCountFlagBits sampleCount; - - constexpr AttachmentMetadata(vk::Format format, vk::SampleCountFlagBits sampleCount) : format(format), sampleCount(sampleCount) {} - - bool operator==(const AttachmentMetadata &rhs) const = default; - }; -} diff --git a/app/src/main/cpp/skyline/gpu/cache/framebuffer_cache.h b/app/src/main/cpp/skyline/gpu/cache/framebuffer_cache.h index da57b481..ad24fbce 100644 --- a/app/src/main/cpp/skyline/gpu/cache/framebuffer_cache.h +++ b/app/src/main/cpp/skyline/gpu/cache/framebuffer_cache.h @@ -3,7 +3,8 @@ #pragma once -#include "common.h" +#include +#include namespace skyline::gpu::cache { using FramebufferCreateInfo = vk::StructureChain; diff --git a/app/src/main/cpp/skyline/gpu/cache/graphics_pipeline_cache.h b/app/src/main/cpp/skyline/gpu/cache/graphics_pipeline_cache.h index 5adbbf6c..31d9a8f7 100644 --- a/app/src/main/cpp/skyline/gpu/cache/graphics_pipeline_cache.h +++ b/app/src/main/cpp/skyline/gpu/cache/graphics_pipeline_cache.h @@ -3,8 +3,8 @@ #pragma once +#include #include -#include "common.h" namespace skyline::gpu::cache { /** @@ -48,6 +48,24 @@ namespace skyline::gpu::cache { }; private: + GPU &gpu; + std::mutex mutex; //!< Synchronizes accesses to the pipeline cache + vk::raii::PipelineCache vkPipelineCache; //!< A Vulkan Pipeline Cache which stores all unique graphics pipelines + + /** + * @brief All unique metadata in a single attachment for a compatible render pass according to Render Pass Compatibility clause in the Vulkan specification + * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#renderpass-compatibility + * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VkAttachmentDescription.html + */ + struct AttachmentMetadata { + vk::Format format; + vk::SampleCountFlagBits sampleCount; + + constexpr AttachmentMetadata(vk::Format format, vk::SampleCountFlagBits sampleCount) : format(format), sampleCount(sampleCount) {} + + bool operator==(const AttachmentMetadata &rhs) const = default; + }; + /** * @brief All data in PipelineState in value form to allow cheap heterogenous lookups with reference types while still storing a value-based key in the map */ @@ -92,11 +110,6 @@ namespace skyline::gpu::cache { } }; - private: - GPU &gpu; - std::mutex mutex; //!< Synchronizes accesses to the pipeline cache - vk::raii::PipelineCache vkPipelineCache; //!< A Vulkan Pipeline Cache which stores all unique graphics pipelines - struct PipelineStateHash { using is_transparent = std::true_type; diff --git a/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.cpp b/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.cpp index c043e635..24c5c191 100644 --- a/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.cpp +++ b/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.cpp @@ -6,37 +6,24 @@ #include "renderpass_cache.h" namespace skyline::gpu::cache { - RenderPassCache::RenderPassCache(gpu::GPU &gpu) : gpu(gpu) {} + RenderPassCache::RenderPassCache(gpu::GPU &gpu) : gpu{gpu} {} - RenderPassCache::RenderPassMetadata::RenderPassMetadata(const vk::RenderPassCreateInfo &createInfo) { - for (const auto &attachment : span{createInfo.pAttachments, createInfo.attachmentCount}) - attachments.emplace_back(attachment.format, attachment.samples); + #define VEC_CPY(pointer, size) description.pointer, description.pointer + description.size - subpasses.reserve(createInfo.subpassCount); - for (const auto &subpass : span{createInfo.pSubpasses, createInfo.subpassCount}) { - auto &subpassMetadata{subpasses.emplace_back()}; + RenderPassCache::SubpassDescription::SubpassDescription(const vk::SubpassDescription &description) + : flags{description.flags}, + pipelineBindPoint{description.pipelineBindPoint}, + inputAttachments{VEC_CPY(pInputAttachments, inputAttachmentCount)}, + colorAttachments{VEC_CPY(pColorAttachments, colorAttachmentCount)}, + resolveAttachments{description.pResolveAttachments, description.pResolveAttachments + (description.pResolveAttachments ? description.colorAttachmentCount : 0)}, + depthStencilAttachment{description.pDepthStencilAttachment ? *description.pDepthStencilAttachment : std::optional{}}, + preserveAttachments{VEC_CPY(pPreserveAttachments, preserveAttachmentCount)} {} - subpassMetadata.inputAttachments.reserve(subpass.inputAttachmentCount); - for (const auto &reference : span{subpass.pInputAttachments, subpass.inputAttachmentCount}) - subpassMetadata.inputAttachments.emplace_back(reference.attachment); + #undef VEC_CPY - subpassMetadata.colorAttachments.reserve(subpass.colorAttachmentCount); - for (const auto &reference : span{subpass.pColorAttachments, subpass.colorAttachmentCount}) - subpassMetadata.colorAttachments.emplace_back(reference.attachment); - - auto resolveAttachmentCount{subpass.pResolveAttachments ? subpass.colorAttachmentCount : 0}; - subpassMetadata.resolveAttachments.reserve(resolveAttachmentCount); - for (const auto &reference : span{subpass.pResolveAttachments, resolveAttachmentCount}) - subpassMetadata.resolveAttachments.emplace_back(reference.attachment); - - if (subpass.pDepthStencilAttachment) - subpassMetadata.depthStencilAttachment.emplace(subpass.pDepthStencilAttachment->attachment); - - subpassMetadata.preserveAttachments.reserve(subpass.preserveAttachmentCount); - for (const auto &index : span{subpass.pPreserveAttachments, subpass.preserveAttachmentCount}) - subpassMetadata.resolveAttachments.emplace_back(index); - } - } + RenderPassCache::RenderPassMetadata::RenderPassMetadata(const vk::RenderPassCreateInfo &createInfo) + : attachments{createInfo.pAttachments, createInfo.pAttachments + createInfo.attachmentCount}, + subpasses{createInfo.pSubpasses, createInfo.pSubpasses + createInfo.subpassCount} {} #define HASH(x) boost::hash_combine(hash, x) @@ -45,27 +32,39 @@ namespace skyline::gpu::cache { HASH(key.attachments.size()); for (const auto &attachment : key.attachments) { + HASH(static_cast(attachment.flags)); HASH(attachment.format); - HASH(attachment.sampleCount); + HASH(attachment.samples); + HASH(attachment.loadOp); + HASH(attachment.storeOp); + HASH(attachment.stencilLoadOp); + HASH(attachment.stencilStoreOp); + HASH(attachment.initialLayout); + HASH(attachment.finalLayout); } HASH(key.subpasses.size()); for (const auto &subpass : key.subpasses) { - HASH(subpass.inputAttachments.size()); - for (const auto &reference : subpass.inputAttachments) - HASH(reference); + HASH(static_cast(subpass.flags)); + HASH(subpass.pipelineBindPoint); - HASH(subpass.colorAttachments.size()); - for (const auto &reference : subpass.colorAttachments) - HASH(reference); + auto hashReferences{[&hash](const auto &references) { + HASH(references.size()); + for (const auto &reference : references) { + HASH(reference.attachment); + HASH(reference.layout); + } + }}; - HASH(subpass.resolveAttachments.size()); - for (const auto &reference : subpass.resolveAttachments) - HASH(reference); + hashReferences(subpass.inputAttachments); + hashReferences(subpass.colorAttachments); + hashReferences(subpass.resolveAttachments); HASH(subpass.depthStencilAttachment.has_value()); - if (subpass.depthStencilAttachment) - HASH(*subpass.depthStencilAttachment); + if (subpass.depthStencilAttachment) { + HASH(subpass.depthStencilAttachment->attachment); + HASH(subpass.depthStencilAttachment->layout); + } HASH(subpass.preserveAttachments.size()); for (const auto &index : subpass.preserveAttachments) @@ -80,28 +79,40 @@ namespace skyline::gpu::cache { HASH(key.attachmentCount); for (const auto &attachment : span{key.pAttachments, key.attachmentCount}) { + HASH(static_cast(attachment.flags)); HASH(attachment.format); HASH(attachment.samples); + HASH(attachment.loadOp); + HASH(attachment.storeOp); + HASH(attachment.stencilLoadOp); + HASH(attachment.stencilStoreOp); + HASH(attachment.initialLayout); + HASH(attachment.finalLayout); } HASH(key.subpassCount); for (const auto &subpass : span{key.pSubpasses, key.subpassCount}) { - HASH(subpass.inputAttachmentCount); - for (const auto &reference : span{subpass.pInputAttachments, subpass.inputAttachmentCount}) - HASH(reference.attachment); + HASH(static_cast(subpass.flags)); + HASH(subpass.pipelineBindPoint); - HASH(subpass.colorAttachmentCount); - for (const auto &reference : span{subpass.pColorAttachments, subpass.colorAttachmentCount}) - HASH(reference.attachment); + auto hashReferences{[&hash](const vk::AttachmentReference *data, u32 count) { + HASH(count); + for (const auto &reference : span{data, count}) { + HASH(reference.attachment); + HASH(reference.layout); + } + }}; - u32 resolveAttachmentCount{subpass.pResolveAttachments ? subpass.colorAttachmentCount : 0}; - HASH(resolveAttachmentCount); - for (const auto &reference : span{subpass.pResolveAttachments, resolveAttachmentCount}) - HASH(reference.attachment); + hashReferences(subpass.pInputAttachments, subpass.inputAttachmentCount); + hashReferences(subpass.pColorAttachments, subpass.colorAttachmentCount); + if (subpass.pResolveAttachments) + hashReferences(subpass.pResolveAttachments, subpass.colorAttachmentCount); HASH(subpass.pDepthStencilAttachment != nullptr); - if (subpass.pDepthStencilAttachment) + if (subpass.pDepthStencilAttachment) { HASH(subpass.pDepthStencilAttachment->attachment); + HASH(subpass.pDepthStencilAttachment->layout); + } HASH(subpass.preserveAttachmentCount); for (const auto &index : span{subpass.pPreserveAttachments, subpass.preserveAttachmentCount}) @@ -119,41 +130,29 @@ namespace skyline::gpu::cache { bool RenderPassCache::RenderPassEqual::operator()(const RenderPassMetadata &lhs, const vk::RenderPassCreateInfo &rhs) const { #define RETF(condition) if (condition) { return false; } + #define RETARRNEQ(array, pointer, count) RETF(!std::equal(array.begin(), array.end(), pointer, pointer + count)) - RETF(lhs.attachments.size() != rhs.attachmentCount) - const vk::AttachmentDescription *vkAttachment{rhs.pAttachments}; - for (const auto &attachment : lhs.attachments) { - RETF(attachment.format != vkAttachment->format) - RETF(attachment.sampleCount != vkAttachment->samples) - vkAttachment++; - } + RETARRNEQ(lhs.attachments, rhs.pAttachments, rhs.attachmentCount) RETF(lhs.subpasses.size() != rhs.subpassCount) const vk::SubpassDescription *vkSubpass{rhs.pSubpasses}; for (const auto &subpass : lhs.subpasses) { - RETF(subpass.inputAttachments.size() != vkSubpass->inputAttachmentCount) - const vk::AttachmentReference *vkReference{vkSubpass->pInputAttachments}; - for (const auto &reference : subpass.inputAttachments) - RETF(reference != (vkReference++)->attachment) + RETF(subpass.flags != vkSubpass->flags) + RETF(subpass.pipelineBindPoint != vkSubpass->pipelineBindPoint) - RETF(subpass.colorAttachments.size() != vkSubpass->colorAttachmentCount) - vkReference = vkSubpass->pColorAttachments; - for (const auto &reference : subpass.colorAttachments) - RETF(reference != (vkReference++)->attachment) + RETARRNEQ(subpass.inputAttachments, vkSubpass->pInputAttachments, vkSubpass->inputAttachmentCount) + RETARRNEQ(subpass.colorAttachments, vkSubpass->pColorAttachments, vkSubpass->colorAttachmentCount) RETF(subpass.resolveAttachments.size() != (vkSubpass->pResolveAttachments ? vkSubpass->colorAttachmentCount : 0)) - vkReference = vkSubpass->pResolveAttachments; - for (const auto &reference : subpass.resolveAttachments) - RETF(reference != (vkReference++)->attachment) + if (!subpass.resolveAttachments.empty()) + RETARRNEQ(subpass.resolveAttachments, vkSubpass->pResolveAttachments, vkSubpass->colorAttachmentCount) RETF(subpass.depthStencilAttachment.has_value() != (vkSubpass->pDepthStencilAttachment != nullptr)) if (subpass.depthStencilAttachment) - RETF(*subpass.depthStencilAttachment != vkSubpass->pDepthStencilAttachment->attachment) + RETF(subpass.depthStencilAttachment->attachment != vkSubpass->pDepthStencilAttachment->attachment && + subpass.depthStencilAttachment->layout != vkSubpass->pDepthStencilAttachment->layout) - RETF(subpass.preserveAttachments.size() != vkSubpass->preserveAttachmentCount) - const u32 *vkIndex{vkSubpass->pPreserveAttachments}; - for (const auto &attachment : subpass.preserveAttachments) - RETF(attachment != *(vkIndex++)) + RETARRNEQ(subpass.preserveAttachments, vkSubpass->pPreserveAttachments, vkSubpass->preserveAttachmentCount) vkSubpass++; } diff --git a/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.h b/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.h index 77fca874..8a854581 100644 --- a/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.h +++ b/app/src/main/cpp/skyline/gpu/cache/renderpass_cache.h @@ -3,7 +3,8 @@ #pragma once -#include "common.h" +#include +#include namespace skyline::gpu::cache { /** @@ -14,31 +15,29 @@ namespace skyline::gpu::cache { GPU &gpu; std::mutex mutex; //!< Synchronizes access to the cache - using AttachmentReference = u32; - /** - * @brief All unique metadata in a single subpass for a compatible render pass according to Render Pass Compatibility clause in the Vulkan specification - * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#renderpass-compatibility * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VkSubpassDescription.html */ - struct SubpassMetadata { - std::vector inputAttachments; - std::vector colorAttachments; - std::vector resolveAttachments; - std::optional depthStencilAttachment; - std::vector preserveAttachments; + struct SubpassDescription { + vk::SubpassDescriptionFlags flags; + vk::PipelineBindPoint pipelineBindPoint; + std::vector inputAttachments; + std::vector colorAttachments; + std::vector resolveAttachments; + std::optional depthStencilAttachment; + std::vector preserveAttachments; - bool operator==(const SubpassMetadata &rhs) const = default; + SubpassDescription(const vk::SubpassDescription &description); + + bool operator==(const SubpassDescription &rhs) const = default; }; /** - * @brief All unique metadata in a render pass for a corresponding compatible render pass according to Render Pass Compatibility clause in the Vulkan specification - * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#renderpass-compatibility * @url https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VkRenderPassCreateInfo.html */ struct RenderPassMetadata { - std::vector attachments; - std::vector subpasses; + std::vector attachments; + std::vector subpasses; RenderPassMetadata(const vk::RenderPassCreateInfo &createInfo);