From 08f29f7da42edead2e15a71f30ed35d37dcb2a59 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Tue, 4 Jan 2022 13:48:05 +0530 Subject: [PATCH] Make `ActiveDescriptorSet` movable and non-copyable There should only ever be a single instance of a `ActiveDescriptorSet` that tracks the lifetime of a descriptor set as the destructor is responsible for freeing the descriptor set. There are cases where a new object inheriting the descriptor set needs to be created in these cases we need to have move semantics and make the destructor of the prior object inert, this allows for moving to the new object without any side effects. If the copy constructor was used in these cases the older object would free the set on its destruction which would lead to the set being invalid on existing instances which is incorrect behavior and would likely lead to driver crashes. --- .../main/cpp/skyline/gpu/descriptor_allocator.cpp | 13 ++++++++++--- app/src/main/cpp/skyline/gpu/descriptor_allocator.h | 7 +++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/src/main/cpp/skyline/gpu/descriptor_allocator.cpp b/app/src/main/cpp/skyline/gpu/descriptor_allocator.cpp index caf7eec9..152a15d1 100644 --- a/app/src/main/cpp/skyline/gpu/descriptor_allocator.cpp +++ b/app/src/main/cpp/skyline/gpu/descriptor_allocator.cpp @@ -38,10 +38,17 @@ namespace skyline::gpu { pool->freeSetCount--; } + DescriptorAllocator::ActiveDescriptorSet::ActiveDescriptorSet(DescriptorAllocator::ActiveDescriptorSet &&other) noexcept { + pool = std::move(other.pool); + static_cast(*this) = std::exchange(static_cast(other), vk::DescriptorSet{}); + } + DescriptorAllocator::ActiveDescriptorSet::~ActiveDescriptorSet() { - std::scoped_lock lock(*pool); - pool->getDevice().freeDescriptorSets(**pool, 1, this, *pool->getDispatcher()); - pool->freeSetCount++; + if (static_cast(*this)) { + std::scoped_lock lock(*pool); + pool->getDevice().freeDescriptorSets(**pool, 1, this, *pool->getDispatcher()); + pool->freeSetCount++; + } } DescriptorAllocator::DescriptorAllocator(GPU &gpu) : gpu(gpu) { diff --git a/app/src/main/cpp/skyline/gpu/descriptor_allocator.h b/app/src/main/cpp/skyline/gpu/descriptor_allocator.h index 9e83e619..e7b574e2 100644 --- a/app/src/main/cpp/skyline/gpu/descriptor_allocator.h +++ b/app/src/main/cpp/skyline/gpu/descriptor_allocator.h @@ -50,6 +50,13 @@ namespace skyline::gpu { ActiveDescriptorSet(std::shared_ptr pool, vk::DescriptorSet set); public: + ActiveDescriptorSet(ActiveDescriptorSet &&other) noexcept; + + /* Delete the move constructor to prevent early freeing of the descriptor set */ + ActiveDescriptorSet(const ActiveDescriptorSet &) = delete; + + ActiveDescriptorSet &operator=(const ActiveDescriptorSet &) = delete; + ~ActiveDescriptorSet(); };