From e0e2c76d2ffb7acea3569296ef115abee4c744e6 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 7 Jun 2023 11:05:47 -0700 Subject: [PATCH] vulkan: minor fixes (#6874) - flush() and wait() before destroying a swapchain - Make sure the debug marker extension is enabled under correct circumstances. - Change shared_ptrs to unique_ptrs and raw pointers. - Rename most teardown methods to terminate() --- filament/backend/src/vulkan/VulkanBlitter.cpp | 8 +-- filament/backend/src/vulkan/VulkanBlitter.h | 9 ++- filament/backend/src/vulkan/VulkanBuffer.cpp | 3 +- filament/backend/src/vulkan/VulkanBuffer.h | 6 +- filament/backend/src/vulkan/VulkanContext.h | 67 +------------------ .../backend/src/vulkan/VulkanDisposer.cpp | 2 +- filament/backend/src/vulkan/VulkanDisposer.h | 2 +- filament/backend/src/vulkan/VulkanDriver.cpp | 63 ++++++++--------- filament/backend/src/vulkan/VulkanDriver.h | 4 +- .../backend/src/vulkan/VulkanFboCache.cpp | 2 +- filament/backend/src/vulkan/VulkanHandles.cpp | 18 ++--- filament/backend/src/vulkan/VulkanHandles.h | 6 +- .../src/vulkan/VulkanPipelineCache.cpp | 4 +- .../backend/src/vulkan/VulkanPipelineCache.h | 2 +- .../backend/src/vulkan/VulkanSamplerCache.cpp | 2 +- .../backend/src/vulkan/VulkanSamplerCache.h | 2 +- .../backend/src/vulkan/VulkanStagePool.cpp | 7 +- filament/backend/src/vulkan/VulkanStagePool.h | 6 +- .../backend/src/vulkan/VulkanSwapChain.cpp | 13 ++-- filament/backend/src/vulkan/VulkanSwapChain.h | 19 +++--- filament/backend/src/vulkan/VulkanTexture.cpp | 4 +- filament/backend/src/vulkan/VulkanTexture.h | 27 ++++---- .../src/vulkan/platform/VulkanPlatform.cpp | 9 ++- 23 files changed, 111 insertions(+), 174 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBlitter.cpp b/filament/backend/src/vulkan/VulkanBlitter.cpp index 7302f76db6..a67c75a86c 100644 --- a/filament/backend/src/vulkan/VulkanBlitter.cpp +++ b/filament/backend/src/vulkan/VulkanBlitter.cpp @@ -116,8 +116,7 @@ VulkanBlitter::VulkanBlitter(VulkanStagePool& stagePool, VulkanPipelineCache& pi mSamplerCache(samplerCache) {} void VulkanBlitter::initialize(VkPhysicalDevice physicalDevice, VkDevice device, - VmaAllocator allocator, std::shared_ptr commands, - std::shared_ptr emptyTexture) noexcept { + VmaAllocator allocator, VulkanCommands* commands, VulkanTexture* emptyTexture) noexcept { mPhysicalDevice = physicalDevice; mDevice = device; mAllocator = allocator; @@ -181,7 +180,7 @@ void VulkanBlitter::blitDepth(BlitArgs args) { args.dstRectPair); } -void VulkanBlitter::shutdown() noexcept { +void VulkanBlitter::terminate() noexcept { if (mDevice) { delete mDepthResolveProgram; mDepthResolveProgram = nullptr; @@ -198,9 +197,6 @@ void VulkanBlitter::shutdown() noexcept { mParamsBuffer = nullptr; } } - - mCommands.reset(); - mEmptyTexture.reset(); } // If we created these shader modules in the constructor, the device might not be ready yet. diff --git a/filament/backend/src/vulkan/VulkanBlitter.h b/filament/backend/src/vulkan/VulkanBlitter.h index 18527770e2..2c2b70bc68 100644 --- a/filament/backend/src/vulkan/VulkanBlitter.h +++ b/filament/backend/src/vulkan/VulkanBlitter.h @@ -36,8 +36,7 @@ public: VulkanFboCache& fboCache, VulkanSamplerCache& samplerCache) noexcept; void initialize(VkPhysicalDevice physicalDevice, VkDevice device, VmaAllocator allocator, - std::shared_ptr commands, - std::shared_ptr emptyTexture) noexcept; + VulkanCommands* commands, VulkanTexture* emptyTexture) noexcept; struct BlitArgs { const VulkanRenderTarget* dstTarget; @@ -51,7 +50,7 @@ public: void blitColor(BlitArgs args); void blitDepth(BlitArgs args); - void shutdown() noexcept; + void terminate() noexcept; private: void lazyInit() noexcept; @@ -67,8 +66,8 @@ private: UTILS_UNUSED VkPhysicalDevice mPhysicalDevice; VkDevice mDevice; VmaAllocator mAllocator; - std::shared_ptr mCommands; - std::shared_ptr mEmptyTexture; + VulkanCommands* mCommands; + VulkanTexture* mEmptyTexture; VulkanStagePool& mStagePool; VulkanPipelineCache& mPipelineCache; diff --git a/filament/backend/src/vulkan/VulkanBuffer.cpp b/filament/backend/src/vulkan/VulkanBuffer.cpp index 7d9ba47910..bf60ee51cd 100644 --- a/filament/backend/src/vulkan/VulkanBuffer.cpp +++ b/filament/backend/src/vulkan/VulkanBuffer.cpp @@ -23,7 +23,7 @@ using namespace bluevk; namespace filament::backend { -VulkanBuffer::VulkanBuffer(VmaAllocator allocator, std::shared_ptr commands, +VulkanBuffer::VulkanBuffer(VmaAllocator allocator, VulkanCommands* commands, VulkanStagePool& stagePool, VkBufferUsageFlags usage, uint32_t numBytes) : mAllocator(allocator), mCommands(commands), mStagePool(stagePool), mUsage(usage) { @@ -51,7 +51,6 @@ void VulkanBuffer::terminate() { vmaDestroyBuffer(mAllocator, mGpuBuffer, mGpuMemory); mGpuMemory = VK_NULL_HANDLE; mGpuBuffer = VK_NULL_HANDLE; - mCommands.reset(); } void VulkanBuffer::loadFromCpu(const void* cpuData, uint32_t byteOffset, uint32_t numBytes) const { diff --git a/filament/backend/src/vulkan/VulkanBuffer.h b/filament/backend/src/vulkan/VulkanBuffer.h index a5cef97cc9..4d80e56a4a 100644 --- a/filament/backend/src/vulkan/VulkanBuffer.h +++ b/filament/backend/src/vulkan/VulkanBuffer.h @@ -25,15 +25,15 @@ namespace filament::backend { // Encapsulates a Vulkan buffer, its attached DeviceMemory and a staging area. class VulkanBuffer { public: - VulkanBuffer(VmaAllocator allocator, std::shared_ptr commands, - VulkanStagePool& stagePool, VkBufferUsageFlags usage, uint32_t numBytes); + VulkanBuffer(VmaAllocator allocator, VulkanCommands* commands, VulkanStagePool& stagePool, + VkBufferUsageFlags usage, uint32_t numBytes); ~VulkanBuffer(); void terminate(); void loadFromCpu(const void* cpuData, uint32_t byteOffset, uint32_t numBytes) const; VkBuffer getGpuBuffer() const { return mGpuBuffer; } private: VmaAllocator mAllocator; - std::shared_ptr mCommands; + VulkanCommands* mCommands; VulkanStagePool& mStagePool; VmaAllocation mGpuMemory = VK_NULL_HANDLE; diff --git a/filament/backend/src/vulkan/VulkanContext.h b/filament/backend/src/vulkan/VulkanContext.h index 0fbed1154b..713abdc8fc 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/filament/backend/src/vulkan/VulkanContext.h @@ -39,73 +39,8 @@ struct VulkanTexture; class VulkanStagePool; struct VulkanTimerQuery; -// TODO: We used std::shared_ptr in various places across the vulkan backend, but VulkanTexture* -// also maps to HwTexture so it's not possible to switch in VulkanAttachment. Hence we introduce -// this temporary class. We need to revisit the ownership pattern and use of smart pointer in the -// vulkan backend, in particular std::shared_ptr uses atomic, which is overhead we do not need. -struct TexturePointer { - TexturePointer() = default; - explicit TexturePointer(VulkanTexture* tex) :mTexture(tex) {} - explicit TexturePointer(std::shared_ptr tex) :mTexture(tex) {} - - inline TexturePointer& operator=(VulkanTexture* tex) { - mTexture = tex; - return *this; - } - - inline TexturePointer& operator=(std::shared_ptr tex) { - mTexture = tex; - return *this; - } - - // Be careful not to leak here. Should only be used in local scope. - explicit operator VulkanTexture*() const { - if (mTexture.index() == 0) { - return std::get<0>(mTexture); - } - return std::get<1>(mTexture).get(); - } - - // Be careful not to leak here. Should only be used in local scope. - explicit operator VulkanTexture const*() const { - if (mTexture.index() == 0) { - return std::get<0>(mTexture); - } - return std::get<1>(mTexture).get(); - } - - explicit operator std::shared_ptr() const { - assert_invariant(mTexture.index() == 1); - return std::get<1>(mTexture); - } - - inline operator bool() const { - if (mTexture.index() == 0) { - return std::get<0>(mTexture) != nullptr; - } - return (bool) std::get<1>(mTexture); - } - - inline VulkanTexture* operator->() { - if (mTexture.index() == 0) { - return std::get<0>(mTexture); - } - return std::get<1>(mTexture).get(); - } - - inline VulkanTexture const* operator->() const { - if (mTexture.index() == 0) { - return std::get<0>(mTexture); - } - return std::get<1>(mTexture).get(); - } - -private: - std::variant> mTexture; -}; - struct VulkanAttachment { - TexturePointer texture; + VulkanTexture* texture; uint8_t level = 0; uint16_t layer = 0; VkImage getImage() const; diff --git a/filament/backend/src/vulkan/VulkanDisposer.cpp b/filament/backend/src/vulkan/VulkanDisposer.cpp index 12774cc23c..d0cbf93ae5 100644 --- a/filament/backend/src/vulkan/VulkanDisposer.cpp +++ b/filament/backend/src/vulkan/VulkanDisposer.cpp @@ -84,7 +84,7 @@ void VulkanDisposer::gc() noexcept { disposables.swap(mDisposables); } -void VulkanDisposer::reset() noexcept { +void VulkanDisposer::terminate() noexcept { #ifndef NDEBUG utils::slog.i << mDisposables.size() << " disposables are outstanding." << utils::io::endl; #endif diff --git a/filament/backend/src/vulkan/VulkanDisposer.h b/filament/backend/src/vulkan/VulkanDisposer.h index a8ac5b1968..ea6f39defb 100644 --- a/filament/backend/src/vulkan/VulkanDisposer.h +++ b/filament/backend/src/vulkan/VulkanDisposer.h @@ -44,7 +44,7 @@ public: void gc() noexcept; // Invokes the destructor function for all disposables, regardless of reference count. - void reset() noexcept; + void terminate() noexcept; private: struct Disposable { diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 9d3f42bb27..05a866b4ae 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -92,12 +92,11 @@ VmaAllocator createAllocator(VkInstance instance, VkPhysicalDevice physicalDevic return allocator; } -std::shared_ptr createEmptyTexture(VkDevice device, VkPhysicalDevice physicalDevice, - VulkanContext const& context, VmaAllocator allocator, - std::shared_ptr commands, VulkanStagePool& stagePool) { - std::shared_ptr emptyTexture = std::make_shared(device, - physicalDevice, context, allocator, commands, SamplerType::SAMPLER_2D, 1, - TextureFormat::RGBA8, 1, 1, 1, 1, +VulkanTexture* createEmptyTexture(VkDevice device, VkPhysicalDevice physicalDevice, + VulkanContext const& context, VmaAllocator allocator, VulkanCommands* commands, + VulkanStagePool& stagePool) { + VulkanTexture* emptyTexture = new VulkanTexture(device, physicalDevice, context, allocator, + commands, SamplerType::SAMPLER_2D, 1, TextureFormat::RGBA8, 1, 1, 1, 1, TextureUsage::DEFAULT | TextureUsage::COLOR_ATTACHMENT | TextureUsage::SUBPASS_INPUT, stagePool); uint32_t black = 0; @@ -186,22 +185,22 @@ VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& contex } #endif mTimestamps = std::make_unique(mPlatform->getDevice()); - mCommands = std::make_shared(mPlatform->getDevice(), + mCommands = std::make_unique(mPlatform->getDevice(), mPlatform->getGraphicsQueue(), mPlatform->getGraphicsQueueFamilyIndex()); mCommands->setObserver(&mPipelineCache); mPipelineCache.setDevice(mPlatform->getDevice(), mAllocator); // TOOD: move them all to be initialized by constructor - mStagePool.initialize(mAllocator, mCommands); + mStagePool.initialize(mAllocator, mCommands.get()); mFramebufferCache.initialize(mPlatform->getDevice()); mSamplerCache.initialize(mPlatform->getDevice()); - mEmptyTexture = createEmptyTexture(mPlatform->getDevice(), mPlatform->getPhysicalDevice(), - mContext, mAllocator, mCommands, mStagePool); + mEmptyTexture.reset(createEmptyTexture(mPlatform->getDevice(), mPlatform->getPhysicalDevice(), + mContext, mAllocator, mCommands.get(), mStagePool)); mPipelineCache.setDummyTexture(mEmptyTexture->getPrimaryImageView()); mBlitter.initialize(mPlatform->getPhysicalDevice(), mPlatform->getDevice(), mAllocator, - mCommands, mEmptyTexture); + mCommands.get(), mEmptyTexture.get()); } VulkanDriver::~VulkanDriver() noexcept = default; @@ -225,20 +224,22 @@ ShaderModel VulkanDriver::getShaderModel() const noexcept { } void VulkanDriver::terminate() { - mEmptyTexture.reset(); + // Command buffers should come first since it might have commands depending on resources that + // are about to be destroyed. mCommands.reset(); + mEmptyTexture.reset(); mTimestamps.reset(); - mBlitter.shutdown(); + mBlitter.terminate(); // Allow the stage pool and disposer to clean up. mStagePool.gc(); - mDisposer.reset(); + mDisposer.terminate(); - mStagePool.reset(); - mPipelineCache.destroyCache(); + mStagePool.terminate(); + mPipelineCache.terminate(); mFramebufferCache.reset(); - mSamplerCache.reset(); + mSamplerCache.terminate(); vmaDestroyAllocator(mAllocator); @@ -335,7 +336,7 @@ void VulkanDriver::destroyVertexBuffer(Handle vbh) { void VulkanDriver::createIndexBufferR(Handle ibh, ElementType elementType, uint32_t indexCount, BufferUsage usage) { auto elementSize = (uint8_t) getElementTypeSize(elementType); - auto indexBuffer = construct(ibh, mAllocator, mCommands, mStagePool, + auto indexBuffer = construct(ibh, mAllocator, mCommands.get(), mStagePool, elementSize, indexCount); mDisposer.createDisposable(indexBuffer, [this, ibh]() { destructBuffer(ibh); }); @@ -350,7 +351,7 @@ void VulkanDriver::destroyIndexBuffer(Handle ibh) { void VulkanDriver::createBufferObjectR(Handle boh, uint32_t byteCount, BufferObjectBinding bindingType, BufferUsage usage) { - auto bufferObject = construct(boh, mAllocator, mCommands, mStagePool, + auto bufferObject = construct(boh, mAllocator, mCommands.get(), mStagePool, byteCount, bindingType, usage); mDisposer.createDisposable(bufferObject, [this, boh]() { destructBuffer(boh); }); @@ -374,8 +375,8 @@ void VulkanDriver::createTextureR(Handle th, SamplerType target, uint TextureFormat format, uint8_t samples, uint32_t w, uint32_t h, uint32_t depth, TextureUsage usage) { auto vktexture = construct(th, mPlatform->getDevice(), - mPlatform->getPhysicalDevice(), mContext, mAllocator, mCommands, target, levels, format, - samples, w, h, depth, usage, mStagePool); + mPlatform->getPhysicalDevice(), mContext, mAllocator, mCommands.get(), target, levels, + format, samples, w, h, depth, usage, mStagePool); mDisposer.createDisposable(vktexture, [this, th]() { destruct(th); }); } @@ -386,8 +387,8 @@ void VulkanDriver::createTextureSwizzledR(Handle th, SamplerType targ TextureSwizzle swizzleArray[] = {r, g, b, a}; const VkComponentMapping swizzleMap = getSwizzleMap(swizzleArray); auto vktexture = construct(th, mPlatform->getDevice(), - mPlatform->getPhysicalDevice(), mContext, mAllocator, mCommands, target, levels, format, - samples, w, h, depth, usage, mStagePool, swizzleMap); + mPlatform->getPhysicalDevice(), mContext, mAllocator, mCommands.get(), target, levels, + format, samples, w, h, depth, usage, mStagePool, swizzleMap); mDisposer.createDisposable(vktexture, [this, th]() { destruct(th); }); @@ -441,7 +442,7 @@ void VulkanDriver::createRenderTargetR(Handle rth, for (int i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) { if (color[i].handle) { colorTargets[i] = { - .texture = TexturePointer(handle_cast(color[i].handle)), + .texture = handle_cast(color[i].handle), .level = color[i].level, .layer = color[i].layer, }; @@ -455,7 +456,7 @@ void VulkanDriver::createRenderTargetR(Handle rth, VulkanAttachment depthStencil[2] = {}; if (depth.handle) { depthStencil[0] = { - .texture = TexturePointer(handle_cast(depth.handle)), + .texture = handle_cast(depth.handle), .level = depth.level, .layer = depth.layer, }; @@ -467,7 +468,7 @@ void VulkanDriver::createRenderTargetR(Handle rth, if (stencil.handle) { depthStencil[1] = { - .texture = TexturePointer(handle_cast(stencil.handle)), + .texture = handle_cast(stencil.handle), .level = stencil.level, .layer = stencil.layer, }; @@ -484,8 +485,8 @@ void VulkanDriver::createRenderTargetR(Handle rth, assert_invariant(tmin.x >= width && tmin.y >= height); auto renderTarget = construct(rth, mPlatform->getDevice(), - mPlatform->getPhysicalDevice(), mContext, mAllocator, mCommands, width, height, samples, - colorTargets, depthStencil, mStagePool); + mPlatform->getPhysicalDevice(), mContext, mAllocator, mCommands.get(), width, height, + samples, colorTargets, depthStencil, mStagePool); mDisposer.createDisposable(renderTarget, [this, rth]() { destruct(rth); }); } @@ -510,15 +511,15 @@ void VulkanDriver::createSyncR(Handle sh, int) { } void VulkanDriver::createSwapChainR(Handle sch, void* nativeWindow, uint64_t flags) { - construct(sch, mPlatform, mContext, mAllocator, mCommands, mStagePool, + construct(sch, mPlatform, mContext, mAllocator, mCommands.get(), mStagePool, nativeWindow, flags); } void VulkanDriver::createSwapChainHeadlessR(Handle sch, uint32_t width, uint32_t height, uint64_t flags) { assert_invariant(width > 0 && height > 0 && "Vulkan requires non-zero swap chain dimensions."); - construct(sch, mPlatform, mContext, mAllocator, mCommands, mStagePool, nullptr, - flags, VkExtent2D{width, height}); + construct(sch, mPlatform, mContext, mAllocator, mCommands.get(), mStagePool, + nullptr, flags, VkExtent2D{width, height}); } void VulkanDriver::createTimerQueryR(Handle tqh, int) { diff --git a/filament/backend/src/vulkan/VulkanDriver.h b/filament/backend/src/vulkan/VulkanDriver.h index 5d1b4e19f6..f6dae95e4b 100644 --- a/filament/backend/src/vulkan/VulkanDriver.h +++ b/filament/backend/src/vulkan/VulkanDriver.h @@ -138,9 +138,9 @@ private: void collectGarbage(); VulkanPlatform* mPlatform = nullptr; - std::shared_ptr mCommands; + std::unique_ptr mCommands; std::unique_ptr mTimestamps; - std::shared_ptr mEmptyTexture; + std::unique_ptr mEmptyTexture; VulkanSwapChain* mCurrentSwapChain = nullptr; VulkanRenderTarget* mDefaultRenderTarget = nullptr; diff --git a/filament/backend/src/vulkan/VulkanFboCache.cpp b/filament/backend/src/vulkan/VulkanFboCache.cpp index c01e6b5753..1943dce87a 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.cpp +++ b/filament/backend/src/vulkan/VulkanFboCache.cpp @@ -68,7 +68,7 @@ void VulkanFboCache::initialize(VkDevice device) noexcept { mDevice = device; } VulkanFboCache::~VulkanFboCache() { ASSERT_POSTCONDITION(mFramebufferCache.empty() && mRenderPassCache.empty(), - "Please explicitly call reset() while the VkDevice is still alive."); + "Please explicitly call terminate() while the VkDevice is still alive."); } VkFramebuffer VulkanFboCache::getFramebuffer(FboKey config) noexcept { diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index d67a9113eb..e68183d068 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -132,15 +132,15 @@ VulkanRenderTarget::VulkanRenderTarget() : HwRenderTarget(0, 0), mOffscreen(fals void VulkanRenderTarget::bindToSwapChain(VulkanSwapChain& swapChain) { assert_invariant(!mOffscreen); VkExtent2D const extent = swapChain.getExtent(); - mColor[0] = { .texture = TexturePointer(swapChain.getCurrentColor()) }; - mDepth = { .texture = TexturePointer(swapChain.getDepth()) }; + mColor[0] = { .texture = swapChain.getCurrentColor() }; + mDepth = { .texture = swapChain.getDepth() }; width = extent.width; height = extent.height; } VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, VmaAllocator allocator, - std::shared_ptr commands, uint32_t width, uint32_t height, uint8_t samples, + VulkanCommands* commands, uint32_t width, uint32_t height, uint8_t samples, VulkanAttachment color[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT], VulkanAttachment depthStencil[2], VulkanStagePool& stagePool) : HwRenderTarget(width, height), mOffscreen(true), mSamples(samples) { @@ -167,13 +167,13 @@ VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physica if (texture && texture->samples == 1) { auto msTexture = texture->getSidecar(); if (UTILS_UNLIKELY(!msTexture)) { - msTexture = std::make_shared(device, physicalDevice, context, + msTexture = new VulkanTexture(device, physicalDevice, context, allocator, commands, texture->target, ((VulkanTexture const*) texture)->levels, texture->format, samples, texture->width, texture->height, texture->depth, texture->usage, stagePool); texture->setSidecar(msTexture); } - mMsaaAttachments[index] = {.texture = TexturePointer(msTexture)}; + mMsaaAttachments[index] = {.texture = msTexture}; } if (texture && texture->samples > 1) { mMsaaAttachments[index] = mColor[index]; @@ -194,9 +194,9 @@ VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physica uint8_t const msLevel = 1; // Create sidecar MSAA texture for the depth attachment if it does not already exist. - std::shared_ptr msTexture = depthTexture->getSidecar(); + VulkanTexture* msTexture = depthTexture->getSidecar(); if (UTILS_UNLIKELY(!msTexture)) { - msTexture = std::make_shared(device, physicalDevice, context, allocator, + msTexture = new VulkanTexture(device, physicalDevice, context, allocator, commands, depthTexture->target, msLevel, depthTexture->format, samples, depthTexture->width, depthTexture->height, depthTexture->depth, depthTexture->usage, stagePool); @@ -204,7 +204,7 @@ VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physica } mMsaaDepthAttachment = { - .texture = TexturePointer(msTexture), + .texture = msTexture, .level = msLevel, .layer = mDepth.layer, }; @@ -264,7 +264,7 @@ VulkanVertexBuffer::VulkanVertexBuffer(VulkanContext& context, VulkanStagePool& buffers(bufferCount, nullptr) {} VulkanBufferObject::VulkanBufferObject(VmaAllocator allocator, - std::shared_ptr commands, VulkanStagePool& stagePool, uint32_t byteCount, + VulkanCommands* commands, VulkanStagePool& stagePool, uint32_t byteCount, BufferObjectBinding bindingType, BufferUsage usage) : HwBufferObject(byteCount), buffer(allocator, commands, stagePool, getBufferObjectUsage(bindingType), byteCount), diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index 40a7a3ccfb..751c4a2c91 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -53,7 +53,7 @@ struct VulkanRenderTarget : private HwRenderTarget { // Creates an offscreen render target. VulkanRenderTarget(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, VmaAllocator allocator, - std::shared_ptr commands, uint32_t width, uint32_t height, + VulkanCommands* commands, uint32_t width, uint32_t height, uint8_t samples, VulkanAttachment color[MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT], VulkanAttachment depthStencil[2], VulkanStagePool& stagePool); @@ -90,7 +90,7 @@ struct VulkanVertexBuffer : public HwVertexBuffer { }; struct VulkanIndexBuffer : public HwIndexBuffer { - VulkanIndexBuffer(VmaAllocator allocator, std::shared_ptr commands, + VulkanIndexBuffer(VmaAllocator allocator, VulkanCommands* commands, VulkanStagePool& stagePool, uint8_t elementSize, uint32_t indexCount) : HwIndexBuffer(elementSize, indexCount), buffer(allocator, commands, stagePool, VK_BUFFER_USAGE_INDEX_BUFFER_BIT, @@ -102,7 +102,7 @@ struct VulkanIndexBuffer : public HwIndexBuffer { }; struct VulkanBufferObject : public HwBufferObject { - VulkanBufferObject(VmaAllocator allocator, std::shared_ptr commands, + VulkanBufferObject(VmaAllocator allocator, VulkanCommands* commands, VulkanStagePool& stagePool, uint32_t byteCount, BufferObjectBinding bindingType, BufferUsage usage); void terminate() { diff --git a/filament/backend/src/vulkan/VulkanPipelineCache.cpp b/filament/backend/src/vulkan/VulkanPipelineCache.cpp index 3ea2fa9a48..27091c6b5a 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.cpp +++ b/filament/backend/src/vulkan/VulkanPipelineCache.cpp @@ -86,7 +86,7 @@ VulkanPipelineCache::VulkanPipelineCache() : mCurrentRasterState(createDefaultRa } VulkanPipelineCache::~VulkanPipelineCache() { - // This does nothing because VulkanDriver::terminate() calls destroyCache() in order to + // This does nothing because VulkanDriver::terminate() calls terminate() in order to // be explicit about teardown order of various components. } @@ -639,7 +639,7 @@ void VulkanPipelineCache::bindInputAttachment(uint32_t bindingIndex, mDescriptorRequirements.inputAttachments[bindingIndex] = targetInfo; } -void VulkanPipelineCache::destroyCache() noexcept { +void VulkanPipelineCache::terminate() noexcept { // Symmetric to createLayoutsAndDescriptors. destroyLayoutsAndDescriptors(); for (auto& iter : mPipelines) { diff --git a/filament/backend/src/vulkan/VulkanPipelineCache.h b/filament/backend/src/vulkan/VulkanPipelineCache.h index ed203c9c56..2c1e63a9c5 100644 --- a/filament/backend/src/vulkan/VulkanPipelineCache.h +++ b/filament/backend/src/vulkan/VulkanPipelineCache.h @@ -169,7 +169,7 @@ public: // NOTE: In theory we should proffer "unbindSampler" but in practice we never destroy samplers. // Destroys all managed Vulkan objects. This should be called before changing the VkDevice. - void destroyCache() noexcept; + void terminate() noexcept; // vkCmdBindPipeline and vkCmdBindDescriptorSets establish bindings to a specific command // buffer; they are not global to the device. Therefore we need to be notified when a diff --git a/filament/backend/src/vulkan/VulkanSamplerCache.cpp b/filament/backend/src/vulkan/VulkanSamplerCache.cpp index 6ebe235640..b57b739d3a 100644 --- a/filament/backend/src/vulkan/VulkanSamplerCache.cpp +++ b/filament/backend/src/vulkan/VulkanSamplerCache.cpp @@ -127,7 +127,7 @@ VkSampler VulkanSamplerCache::getSampler(SamplerParams params) noexcept { return sampler; } -void VulkanSamplerCache::reset() noexcept { +void VulkanSamplerCache::terminate() noexcept { for (auto pair : mCache) { vkDestroySampler(mDevice, pair.second, VKALLOC); } diff --git a/filament/backend/src/vulkan/VulkanSamplerCache.h b/filament/backend/src/vulkan/VulkanSamplerCache.h index 5ce7ad2dc1..f85592117f 100644 --- a/filament/backend/src/vulkan/VulkanSamplerCache.h +++ b/filament/backend/src/vulkan/VulkanSamplerCache.h @@ -29,7 +29,7 @@ class VulkanSamplerCache { public: void initialize(VkDevice device); VkSampler getSampler(SamplerParams params) noexcept; - void reset() noexcept; + void terminate() noexcept; private: VkDevice mDevice; tsl::robin_map mCache; diff --git a/filament/backend/src/vulkan/VulkanStagePool.cpp b/filament/backend/src/vulkan/VulkanStagePool.cpp index 3a136852d7..cb0f79b7e7 100644 --- a/filament/backend/src/vulkan/VulkanStagePool.cpp +++ b/filament/backend/src/vulkan/VulkanStagePool.cpp @@ -26,8 +26,7 @@ static constexpr uint32_t TIME_BEFORE_EVICTION = VK_MAX_COMMAND_BUFFERS; namespace filament::backend { -void VulkanStagePool::initialize(VmaAllocator allocator, - std::shared_ptr commands) noexcept { +void VulkanStagePool::initialize(VmaAllocator allocator, VulkanCommands* commands) noexcept { mAllocator = allocator; mCommands = commands; } @@ -185,7 +184,7 @@ void VulkanStagePool::gc() noexcept { } } -void VulkanStagePool::reset() noexcept { +void VulkanStagePool::terminate() noexcept { for (auto stage : mUsedStages) { vmaDestroyBuffer(mAllocator, stage->buffer, stage->memory); delete stage; @@ -209,8 +208,6 @@ void VulkanStagePool::reset() noexcept { delete image; } mFreeStages.clear(); - - mCommands.reset(); } } // namespace filament::backend diff --git a/filament/backend/src/vulkan/VulkanStagePool.h b/filament/backend/src/vulkan/VulkanStagePool.h index 95f32f7e02..65a67b0ce7 100644 --- a/filament/backend/src/vulkan/VulkanStagePool.h +++ b/filament/backend/src/vulkan/VulkanStagePool.h @@ -45,7 +45,7 @@ struct VulkanStageImage { // This class manages two types of host-mappable staging areas: buffer stages and image stages. class VulkanStagePool { public: - void initialize(VmaAllocator allocator, std::shared_ptr commands) noexcept; + void initialize(VmaAllocator allocator, VulkanCommands* commands) noexcept; // Finds or creates a stage whose capacity is at least the given number of bytes. // The stage is automatically released back to the pool after TIME_BEFORE_EVICTION frames. @@ -60,11 +60,11 @@ public: // Destroys all unused stages and asserts that there are no stages currently in use. // This should be called while the context's VkDevice is still alive. - void reset() noexcept; + void terminate() noexcept; private: VmaAllocator mAllocator; - std::shared_ptr mCommands; + VulkanCommands* mCommands; // Use an ordered multimap for quick (capacity => stage) lookups using lower_bound(). std::multimap mFreeStages; diff --git a/filament/backend/src/vulkan/VulkanSwapChain.cpp b/filament/backend/src/vulkan/VulkanSwapChain.cpp index f563f61514..cd996c0de5 100644 --- a/filament/backend/src/vulkan/VulkanSwapChain.cpp +++ b/filament/backend/src/vulkan/VulkanSwapChain.cpp @@ -26,8 +26,8 @@ using namespace utils; namespace filament::backend { VulkanSwapChain::VulkanSwapChain(VulkanPlatform* platform, VulkanContext const& context, - VmaAllocator allocator, std::shared_ptr commands, - VulkanStagePool& stagePool, void* nativeWindow, uint64_t flags, VkExtent2D extent) + VmaAllocator allocator, VulkanCommands* commands, VulkanStagePool& stagePool, + void* nativeWindow, uint64_t flags, VkExtent2D extent) : mPlatform(platform), mCommands(commands), mAllocator(allocator), @@ -48,6 +48,11 @@ VulkanSwapChain::VulkanSwapChain(VulkanPlatform* platform, VulkanContext const& } VulkanSwapChain::~VulkanSwapChain() { + // Must wait for the inflight command buffers to finish since they might contain the images + // we're about to destroy. + mCommands->flush(); + mCommands->wait(); + mPlatform->destroy(swapChain); vkDestroySemaphore(mPlatform->getDevice(), mImageReady, VKALLOC); } @@ -60,11 +65,11 @@ void VulkanSwapChain::update() { VkDevice const device = mPlatform->getDevice(); for (auto const color: bundle.colors) { - mColors.push_back(std::make_shared(device, mAllocator, mCommands, color, + mColors.push_back(std::make_unique(device, mAllocator, mCommands, color, bundle.colorFormat, 1, bundle.extent.width, bundle.extent.height, TextureUsage::COLOR_ATTACHMENT, mStagePool)); } - mDepth = std::make_shared(device, mAllocator, mCommands, bundle.depth, + mDepth = std::make_unique(device, mAllocator, mCommands, bundle.depth, bundle.depthFormat, 1, bundle.extent.width, bundle.extent.height, TextureUsage::DEPTH_ATTACHMENT, mStagePool); diff --git a/filament/backend/src/vulkan/VulkanSwapChain.h b/filament/backend/src/vulkan/VulkanSwapChain.h index 4e593fc759..e89d75f598 100644 --- a/filament/backend/src/vulkan/VulkanSwapChain.h +++ b/filament/backend/src/vulkan/VulkanSwapChain.h @@ -37,7 +37,7 @@ struct VulkanSurfaceSwapChain; // A wrapper around the platform implementation of swapchain. struct VulkanSwapChain : public HwSwapChain { VulkanSwapChain(VulkanPlatform* platform, VulkanContext const& context, VmaAllocator allocator, - std::shared_ptr commands, VulkanStagePool& stagePool, + VulkanCommands* commands, VulkanStagePool& stagePool, void* nativeWindow, uint64_t flags, VkExtent2D extent = {0, 0}); ~VulkanSwapChain(); @@ -46,12 +46,12 @@ struct VulkanSwapChain : public HwSwapChain { void acquire(bool& reized); - inline std::shared_ptr getCurrentColor() const noexcept { - return mColors[mCurrentSwapIndex]; + inline VulkanTexture* getCurrentColor() const noexcept { + return mColors[mCurrentSwapIndex].get(); } - inline std::shared_ptr getDepth() const noexcept { - return mDepth; + inline VulkanTexture* getDepth() const noexcept { + return mDepth.get(); } inline bool isFirstRenderPass() const noexcept { @@ -70,16 +70,15 @@ private: void update(); VulkanPlatform* mPlatform; - std::shared_ptr mCommands; + VulkanCommands* mCommands; VmaAllocator mAllocator; VulkanStagePool& mStagePool; bool const mHeadless; // We create VulkanTextures based on VkImages. VulkanTexture has facilities for doing layout - // transitions, which are useful here. We use std::shared_ptr because they will be shared with - // VulkanRenderTarget. - utils::FixedCapacityVector> mColors; - std::shared_ptr mDepth; + // transitions, which are useful here. + utils::FixedCapacityVector> mColors; + std::unique_ptr mDepth; VkExtent2D mExtent; VkSemaphore mImageReady; uint32_t mCurrentSwapIndex; diff --git a/filament/backend/src/vulkan/VulkanTexture.cpp b/filament/backend/src/vulkan/VulkanTexture.cpp index c0bdf45ec6..98ec6d739f 100644 --- a/filament/backend/src/vulkan/VulkanTexture.cpp +++ b/filament/backend/src/vulkan/VulkanTexture.cpp @@ -30,7 +30,7 @@ namespace filament::backend { using ImgUtil = VulkanImageUtility; VulkanTexture::VulkanTexture(VkDevice device, VmaAllocator allocator, - std::shared_ptr commands, VkImage image, VkFormat format, uint8_t samples, + VulkanCommands* commands, VkImage image, VkFormat format, uint8_t samples, uint32_t width, uint32_t height, TextureUsage tusage, VulkanStagePool& stagePool) : HwTexture(SamplerType::SAMPLER_2D, 1, samples, width, height, 1, TextureFormat::UNUSED, tusage), @@ -47,7 +47,7 @@ VulkanTexture::VulkanTexture(VkDevice device, VmaAllocator allocator, VulkanTexture::VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, VmaAllocator allocator, - std::shared_ptr commands, SamplerType target, uint8_t levels, + VulkanCommands* commands, SamplerType target, uint8_t levels, TextureFormat tformat, uint8_t samples, uint32_t w, uint32_t h, uint32_t depth, TextureUsage tusage, VulkanStagePool& stagePool, VkComponentMapping swizzle) : HwTexture(target, levels, samples, w, h, depth, tformat, tusage), diff --git a/filament/backend/src/vulkan/VulkanTexture.h b/filament/backend/src/vulkan/VulkanTexture.h index 1d8f338c0a..05eafae6b5 100644 --- a/filament/backend/src/vulkan/VulkanTexture.h +++ b/filament/backend/src/vulkan/VulkanTexture.h @@ -28,17 +28,15 @@ namespace filament::backend { struct VulkanTexture : public HwTexture { // Standard constructor for user-facing textures. VulkanTexture(VkDevice device, VkPhysicalDevice physicalDevice, VulkanContext const& context, - VmaAllocator allocator, std::shared_ptr commands, SamplerType target, - uint8_t levels, TextureFormat tformat, uint8_t samples, uint32_t w, uint32_t h, - uint32_t depth, TextureUsage tusage, VulkanStagePool& stagePool, - VkComponentMapping swizzle={}); + VmaAllocator allocator, VulkanCommands* commands, SamplerType target, uint8_t levels, + TextureFormat tformat, uint8_t samples, uint32_t w, uint32_t h, uint32_t depth, + TextureUsage tusage, VulkanStagePool& stagePool, VkComponentMapping swizzle = {}); // Specialized constructor for internally created textures (e.g. from a swap chain) // The texture will never destroy the given VkImage, but it does manages its subresources. - VulkanTexture(VkDevice device, - VmaAllocator allocator, std::shared_ptr commands, VkImage image, - VkFormat format, uint8_t samples, uint32_t width, uint32_t height, TextureUsage tusage, - VulkanStagePool& stagePool); + VulkanTexture(VkDevice device, VmaAllocator allocator, VulkanCommands* commands, VkImage image, + VkFormat format, uint8_t samples, uint32_t width, uint32_t height, TextureUsage tusage, + VulkanStagePool& stagePool); ~VulkanTexture(); @@ -68,12 +66,12 @@ struct VulkanTexture : public HwTexture { VulkanLayout getLayout(uint32_t layer, uint32_t level) const; - void setSidecar(std::shared_ptr sidecar) { - mSidecarMSAA = sidecar; + void setSidecar(VulkanTexture* sidecar) { + mSidecarMSAA.reset(sidecar); } - std::shared_ptr getSidecar() const { - return mSidecarMSAA; + VulkanTexture* getSidecar() const { + return mSidecarMSAA.get(); } void transitionLayout(VkCommandBuffer commands, const VkImageSubresourceRange& range, @@ -96,7 +94,8 @@ private: void updateImageWithBlit(const PixelBufferDescriptor& hostData, uint32_t width, uint32_t height, uint32_t depth, uint32_t miplevel); - std::shared_ptr mSidecarMSAA; + // The texture with the sidecar owns the sidecar. + std::unique_ptr mSidecarMSAA; const VkFormat mVkFormat; const VkImageViewType mViewType; const VkComponentMapping mSwizzle; @@ -114,7 +113,7 @@ private: VulkanStagePool& mStagePool; VkDevice mDevice; VmaAllocator mAllocator; - std::shared_ptr mCommands; + VulkanCommands* mCommands; }; } // namespace filament::backend diff --git a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp index 1373620112..9a8e36669d 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp @@ -198,7 +198,7 @@ ExtensionSet getDeviceExtensions(VkPhysicalDevice device) { return exts; } -VkInstance createInstance(const ExtensionSet& requiredExts) { +VkInstance createInstance(ExtensionSet const& requiredExts) { VkInstance instance; VkInstanceCreateInfo instanceCreateInfo = {}; bool validationFeaturesSupported = false; @@ -363,6 +363,13 @@ std::tuple pruneExtensions(VkPhysicalDevice device, && newDeviceExts.find(VK_EXT_DEBUG_MARKER_EXTENSION_NAME) != newDeviceExts.end()) { newDeviceExts.erase(VK_EXT_DEBUG_MARKER_EXTENSION_NAME); } + + // debugMarker must also request debugReport the instance extension. So check if that's present. + if (newDeviceExts.find(VK_EXT_DEBUG_MARKER_EXTENSION_NAME) != newDeviceExts.end() + && newInstExts.find(VK_EXT_DEBUG_REPORT_EXTENSION_NAME) == newInstExts.end()) { + newDeviceExts.erase(VK_EXT_DEBUG_MARKER_EXTENSION_NAME); + } + return std::tuple(newInstExts, newDeviceExts); }