From 23c9388caf30c2d18b49e659b86a339e5d3d4e90 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Wed, 4 May 2022 00:49:21 +0530 Subject: [PATCH] Fix `VK_KHR_push_descriptor`-less path for descriptor set updates Descriptor set updates were broken on the non-push-descriptor path due to lifetime issues with VkDescriptorSetLayout's usage during the execution phase which entirely broke rendering on AMD/Mali GPUs due to them not supporting `VK_KHR_push_descriptor`. This commit addresses that by moving the allocation of a descriptor set to outside the lambda and into the recording phase, it also simplifies the semantics and resources passed into the lambda by removing redundancies. --- .../gpu/interconnect/graphics_context.h | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) 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 5d91a5c4..4e2c6332 100644 --- a/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h +++ b/app/src/main/cpp/skyline/gpu/interconnect/graphics_context.h @@ -865,7 +865,7 @@ namespace skyline::gpu::interconnect { } }; - std::unique_ptr descriptorSetWrites; //!< The writes to the descriptor set that need to be done prior to executing a pipeline + DescriptorSetWrites descriptorSetWrites; //!< The writes to the descriptor set that need to be done prior to executing a pipeline }; /** @@ -985,12 +985,12 @@ namespace skyline::gpu::interconnect { } } - auto descriptorSetWrites{std::make_unique()}; - auto &descriptorWrites{**descriptorSetWrites}; + ShaderProgramState::DescriptorSetWrites descriptorSetWrites{}; + auto &descriptorWrites{*descriptorSetWrites}; descriptorWrites.reserve(PipelineDescriptorWritesReservedCount); - auto &bufferDescriptors{descriptorSetWrites->bufferDescriptors}; - auto &imageDescriptors{descriptorSetWrites->imageDescriptors}; + auto &bufferDescriptors{descriptorSetWrites.bufferDescriptors}; + auto &imageDescriptors{descriptorSetWrites.imageDescriptors}; size_t bufferCount{}, imageCount{}; for (auto &pipelineStage : pipelineStages) { if (pipelineStage.enabled) { @@ -2852,18 +2852,23 @@ namespace skyline::gpu::interconnect { // Draw Persistent Storage struct DrawStorage : FenceCycleDependency { - std::unique_ptr descriptorSetWrites; - vk::DescriptorSetLayout descriptorSetLayout; - vk::PipelineLayout pipelineLayout; - vk::Pipeline pipeline; + ShaderProgramState::DescriptorSetWrites descriptorSetWrites; + std::optional descriptorSet; - DrawStorage(std::unique_ptr &&descriptorSetWrites, vk::DescriptorSetLayout descriptorSetLayout, vk::PipelineLayout pipelineLayout, vk::Pipeline pipeline) : descriptorSetWrites(std::move(descriptorSetWrites)), descriptorSetLayout(descriptorSetLayout), pipelineLayout(pipelineLayout), pipeline(pipeline) {} + DrawStorage(ShaderProgramState::DescriptorSetWrites &&descriptorSetWrites) : descriptorSetWrites(std::move(descriptorSetWrites)) {} + + DrawStorage(ShaderProgramState::DescriptorSetWrites &&descriptorSetWrites, DescriptorAllocator::ActiveDescriptorSet &&descriptorSet) : descriptorSetWrites(std::move(descriptorSetWrites)), descriptorSet(std::move(descriptorSet)) {} }; - auto drawStorage{std::make_shared(std::move(programState.descriptorSetWrites), compiledPipeline.descriptorSetLayout, compiledPipeline.pipelineLayout, compiledPipeline.pipeline)}; + std::shared_ptr drawStorage{}; + if (gpu.traits.supportsPushDescriptors) + drawStorage = std::make_shared(std::move(programState.descriptorSetWrites)); + else { + drawStorage = std::make_shared(std::move(programState.descriptorSetWrites), gpu.descriptor.AllocateSet(compiledPipeline.descriptorSetLayout)); + } // Submit Draw - executor.AddSubpass([=, drawStorage = std::move(drawStorage), &vkDevice = gpu.vkDevice, supportsPushDescriptors = gpu.traits.supportsPushDescriptors](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &cycle, GPU &, vk::RenderPass renderPass, u32 subpassIndex) mutable { + executor.AddSubpass([=, drawStorage = std::move(drawStorage), &vkDevice = gpu.vkDevice, pipelineLayout = compiledPipeline.pipelineLayout, pipeline = compiledPipeline.pipeline, supportsPushDescriptors = gpu.traits.supportsPushDescriptors](vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &cycle, GPU &, vk::RenderPass renderPass, u32 subpassIndex) mutable { auto &vertexBufferHandles{boundVertexBuffers->handles}; for (u32 bindingIndex{}; bindingIndex != vertexBufferHandles.size(); bindingIndex++) { @@ -2879,16 +2884,15 @@ namespace skyline::gpu::interconnect { } if (supportsPushDescriptors) { - commandBuffer.pushDescriptorSetKHR(vk::PipelineBindPoint::eGraphics, drawStorage->pipelineLayout, 0, **drawStorage->descriptorSetWrites); + commandBuffer.pushDescriptorSetKHR(vk::PipelineBindPoint::eGraphics, pipelineLayout, 0, *drawStorage->descriptorSetWrites); } else { - auto descriptorSet{gpu.descriptor.AllocateSet(drawStorage->descriptorSetLayout)}; - for (auto &descriptorSetWrite : **drawStorage->descriptorSetWrites) - descriptorSetWrite.dstSet = descriptorSet; - vkDevice.updateDescriptorSets(**drawStorage->descriptorSetWrites, nullptr); - commandBuffer.bindDescriptorSets(vk::PipelineBindPoint::eGraphics, drawStorage->pipelineLayout, 0, descriptorSet, nullptr); + for (auto &descriptorSetWrite : *drawStorage->descriptorSetWrites) + descriptorSetWrite.dstSet = *drawStorage->descriptorSet; + vkDevice.updateDescriptorSets(*drawStorage->descriptorSetWrites, nullptr); + commandBuffer.bindDescriptorSets(vk::PipelineBindPoint::eGraphics, pipelineLayout, 0, *drawStorage->descriptorSet, nullptr); } - commandBuffer.bindPipeline(vk::PipelineBindPoint::eGraphics, drawStorage->pipeline); + commandBuffer.bindPipeline(vk::PipelineBindPoint::eGraphics, pipeline); if constexpr (IsIndexed) { commandBuffer.bindIndexBuffer(boundIndexBuffer->handle, boundIndexBuffer->offset, boundIndexBuffer->type);