From 88a06ec8e7db641e8aaa7854fd4802cc7c63bfe8 Mon Sep 17 00:00:00 2001 From: Anish Goyal Date: Thu, 5 Jun 2025 12:15:26 -0400 Subject: [PATCH] Switch to block-based stage-pool for Vulkan (#8742) * Switch to block-based stage-pool for Vulkan Instead of allocating a staging buffer every time one is needed, allocate a large (8mb) block of memory, and divvy it up as needed. We will make this configurable in the future, to allow for tuning for different apps as needed. * Address PR comments: use fvkmemory::Resource Instead of having the child block be a unique_ptr that we create a separate container for within the command buffers, just have the stage block segments be fvkmemory::Resource instances. * Address PR comments for staging buff change - As per discussion with @poweifeng, change the name of a variable called "stage" to "stageSegment" for clarity - As per discussion with @rafadevai, change the order of terminate calls in VulkanDriver to better reflect cleanup order of some objects. * Align stage pool to nonCoherentAtomSize In order to prevent flushing more atoms than were modified when writing data to host-mapped memory in a staging buffer, ensure that all segments allocated are aligned to nonCoherentAtomSize. Also - fix merge conflict compile errors. --------- Co-authored-by: Serge Metral --- .../backend/src/vulkan/VulkanBufferProxy.cpp | 29 +-- .../backend/src/vulkan/VulkanBufferProxy.h | 3 +- filament/backend/src/vulkan/VulkanDriver.cpp | 13 +- .../backend/src/vulkan/VulkanStagePool.cpp | 175 ++++++++++++------ filament/backend/src/vulkan/VulkanStagePool.h | 141 ++++++++++++-- filament/backend/src/vulkan/VulkanTexture.cpp | 32 ++-- .../backend/src/vulkan/memory/Resource.cpp | 6 + filament/backend/src/vulkan/memory/Resource.h | 3 +- .../src/vulkan/memory/ResourceManager.cpp | 3 + 9 files changed, 299 insertions(+), 106 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBufferProxy.cpp b/filament/backend/src/vulkan/VulkanBufferProxy.cpp index b31e0cb008..f1f583060c 100644 --- a/filament/backend/src/vulkan/VulkanBufferProxy.cpp +++ b/filament/backend/src/vulkan/VulkanBufferProxy.cpp @@ -15,6 +15,8 @@ */ #include "VulkanBufferProxy.h" +#include "VulkanCommands.h" +#include "VulkanMemory.h" #include "VulkanBufferCache.h" #include "VulkanMemory.h" @@ -32,14 +34,15 @@ VulkanBufferProxy::VulkanBufferProxy(VmaAllocator allocator, VulkanStagePool& st mUpdatedOffset(0), mUpdatedBytes(0) {} -void VulkanBufferProxy::loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, +void VulkanBufferProxy::loadFromCpu(VulkanCommandBuffer& commands, const void* cpuData, uint32_t byteOffset, uint32_t numBytes) { - VulkanStage const* stage = mStagePool.acquireStage(numBytes); - void* mapped; - vmaMapMemory(mAllocator, stage->memory, &mapped); - memcpy(mapped, cpuData, numBytes); - vmaUnmapMemory(mAllocator, stage->memory); - vmaFlushAllocation(mAllocator, stage->memory, 0, numBytes); + // Note: this should be stored within the command buffer before going out of + // scope, so that the command buffer can manage its lifecycle. + fvkmemory::resource_ptr stage = mStagePool.acquireStage(numBytes); + assert_invariant(stage->memory()); + commands.acquire(stage); + memcpy(stage->mapping(), cpuData, numBytes); + vmaFlushAllocation(mAllocator, stage->memory(), stage->offset(), numBytes); // If there was a previous update, then we need to make sure the following write is properly // synced with the previous read. @@ -68,16 +71,16 @@ void VulkanBufferProxy::loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, .offset = byteOffset, .size = numBytes, }; - vkCmdPipelineBarrier(cmdbuf, srcStage, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, nullptr, 1, - &barrier, 0, nullptr); + vkCmdPipelineBarrier(commands.buffer(), srcStage, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, + nullptr, 1, &barrier, 0, nullptr); } VkBufferCopy region = { - .srcOffset = 0, + .srcOffset = stage->offset(), .dstOffset = byteOffset, .size = numBytes, }; - vkCmdCopyBuffer(cmdbuf, stage->buffer, getVkBuffer(), 1, ®ion); + vkCmdCopyBuffer(commands.buffer(), stage->buffer(), getVkBuffer(), 1, ®ion); mUpdatedOffset = byteOffset; mUpdatedBytes = numBytes; @@ -113,8 +116,8 @@ void VulkanBufferProxy::loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, .size = numBytes, }; - vkCmdPipelineBarrier(cmdbuf, VK_PIPELINE_STAGE_TRANSFER_BIT, dstStageMask, 0, 0, nullptr, 1, - &barrier, 0, nullptr); + vkCmdPipelineBarrier(commands.buffer(), VK_PIPELINE_STAGE_TRANSFER_BIT, dstStageMask, 0, 0, + nullptr, 1, &barrier, 0, nullptr); } VkBuffer VulkanBufferProxy::getVkBuffer() const noexcept { diff --git a/filament/backend/src/vulkan/VulkanBufferProxy.h b/filament/backend/src/vulkan/VulkanBufferProxy.h index e6f916ea68..3f35836925 100644 --- a/filament/backend/src/vulkan/VulkanBufferProxy.h +++ b/filament/backend/src/vulkan/VulkanBufferProxy.h @@ -18,6 +18,7 @@ #define TNT_FILAMENT_BACKEND_VULKANBUFFERPROXY_H #include "VulkanBufferCache.h" +#include "VulkanCommands.h" #include "VulkanContext.h" #include "VulkanMemory.h" #include "VulkanStagePool.h" @@ -31,7 +32,7 @@ public: VulkanBufferProxy(VmaAllocator allocator, VulkanStagePool& stagePool, VulkanBufferCache& bufferCache, VulkanBufferUsage usage, uint32_t numBytes); - void loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, uint32_t byteOffset, + void loadFromCpu(VulkanCommandBuffer& commands, const void* cpuData, uint32_t byteOffset, uint32_t numBytes); VkBuffer getVkBuffer() const noexcept; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index f27d9545f3..d3576fad01 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -210,7 +210,7 @@ VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& contex mPlatform->getProtectedGraphicsQueueFamilyIndex(), &mContext), mPipelineLayoutCache(mPlatform->getDevice()), mPipelineCache(mPlatform->getDevice()), - mStagePool(mAllocator, &mCommands), + mStagePool(mAllocator, &mResourceManager, &mCommands, &mContext.getPhysicalDeviceLimits()), mBufferCache(context, mResourceManager, mAllocator), mFramebufferCache(mPlatform->getDevice()), mYcbcrConversionCache(mPlatform->getDevice()), @@ -330,7 +330,6 @@ void VulkanDriver::terminate() { // descriptorSetLayoutCache mExternalImageManager.terminate(); - mStagePool.terminate(); mPipelineCache.terminate(); mFramebufferCache.terminate(); mSamplerCache.terminate(); @@ -346,6 +345,10 @@ void VulkanDriver::terminate() { // back to the pool. mBufferCache.terminate(); + // Before terminating stagePool, we need all resources to have been + // reclaimed, as they perform cleanup within the stage pool. + mStagePool.terminate(); + #if FVK_ENABLED(FVK_DEBUG_RESOURCE_LEAK) mResourceManager.print(); #endif @@ -1231,7 +1234,7 @@ void VulkanDriver::updateIndexBuffer(Handle ibh, BufferDescriptor VulkanCommandBuffer& commands = mCommands.get(); auto ib = resource_ptr::cast(&mResourceManager, ibh); commands.acquire(ib); - ib->buffer.loadFromCpu(commands.buffer(), p.buffer, byteOffset, p.size); + ib->buffer.loadFromCpu(commands, p.buffer, byteOffset, p.size); scheduleDestroy(std::move(p)); } @@ -1246,7 +1249,7 @@ void VulkanDriver::updateBufferObject(Handle boh, BufferDescript auto bo = resource_ptr::cast(&mResourceManager, boh); commands.acquire(bo); - bo->buffer.loadFromCpu(commands.buffer(), bd.buffer, byteOffset, bd.size); + bo->buffer.loadFromCpu(commands, bd.buffer, byteOffset, bd.size); scheduleDestroy(std::move(bd)); } @@ -1257,7 +1260,7 @@ void VulkanDriver::updateBufferObjectUnsynchronized(Handle boh, auto bo = resource_ptr::cast(&mResourceManager, boh); commands.acquire(bo); // TODO: implement unsynchronized version - bo->buffer.loadFromCpu(commands.buffer(), bd.buffer, byteOffset, bd.size); + bo->buffer.loadFromCpu(commands, bd.buffer, byteOffset, bd.size); scheduleDestroy(std::move(bd)); } diff --git a/filament/backend/src/vulkan/VulkanStagePool.cpp b/filament/backend/src/vulkan/VulkanStagePool.cpp index 9a7a6ff5bd..5be2bf728a 100644 --- a/filament/backend/src/vulkan/VulkanStagePool.cpp +++ b/filament/backend/src/vulkan/VulkanStagePool.cpp @@ -28,46 +28,111 @@ static constexpr uint32_t TIME_BEFORE_EVICTION = 3; namespace filament::backend { -VulkanStagePool::VulkanStagePool(VmaAllocator allocator, VulkanCommands* commands) - : mAllocator(allocator), - mCommands(commands) {} +namespace { -VulkanStage const* VulkanStagePool::acquireStage(uint32_t numBytes) { - // First check if a stage exists whose capacity is greater than or equal to the requested size. - auto iter = mFreeStages.lower_bound(numBytes); - if (iter != mFreeStages.end()) { - auto stage = iter->second; - mFreeStages.erase(iter); - stage->lastAccessed = mCurrentFrame; - mUsedStages.push_back(stage); - return stage; - } - // We were not able to find a sufficiently large stage, so create a new one. - VulkanStage* stage = new VulkanStage({ - .memory = VK_NULL_HANDLE, - .buffer = VK_NULL_HANDLE, - .capacity = numBytes, - .lastAccessed = mCurrentFrame, +// Note: these are temporary values, they will be configurable. +static constexpr uint32_t MAX_EMPTY_STAGES_TO_RETAIN = 1; +constexpr uint32_t STAGE_SIZE = 1048576; + +}// namespace + +fvkmemory::resource_ptr VulkanStage::acquireSegment( + fvkmemory::ResourceManager* resManager, uint32_t numBytes) { + auto segment = fvkmemory::resource_ptr::construct( + resManager, this, numBytes, mCurrentOffset, [this](uint32_t offset) { + mSegments.erase(offset); }); + mSegments.insert({mCurrentOffset, segment.get()}); + mCurrentOffset += numBytes; + return segment; +} - // Create the VkBuffer. - mUsedStages.push_back(stage); - VkBufferCreateInfo bufferInfo { +VulkanStagePool::VulkanStagePool(VmaAllocator allocator, fvkmemory::ResourceManager* resManager, + VulkanCommands* commands, const VkPhysicalDeviceLimits* deviceLimits) + : mAllocator(allocator), + mResManager(resManager), + mCommands(commands), + mDeviceLimits(deviceLimits) {} + +fvkmemory::resource_ptr VulkanStagePool::acquireStage(uint32_t numBytes) { + // Apply alignment to the byte count to ensure that, when we later flush + // data written by the host, we only flush the atoms that we modified, and + // no adjacent atoms. + numBytes = alignToNonCoherentAtomSize(numBytes); + + // First check if a stage segment exists whose capacity is greater than or + // equal to the requested size. + auto iter = mStages.lower_bound(numBytes); + + VulkanStage* pStage; + if (iter != mStages.end()) { + pStage = iter->second; + mStages.erase(iter); + } else { + pStage = allocateNewStage(std::max(numBytes, STAGE_SIZE)); + } + + // Note: this allocation updates `currentOffset` and `segments` within + // the parent stage. When destroyed, it will update `segments`. + fvkmemory::resource_ptr pSegment = pStage->acquireSegment(mResManager, numBytes); + + // Update the stage's metadata, and reinsert it with the remaining segment + // capacity. + uint32_t spaceRemaining = pStage->capacity() - pStage->currentOffset(); + mStages.insert({ spaceRemaining, pStage }); + + return pSegment; +} + +uint32_t VulkanStagePool::alignToNonCoherentAtomSize(uint32_t bytes) { + VkDeviceSize alignment = mDeviceLimits->nonCoherentAtomSize; + if (alignment == 0) { + return bytes; + } + + uint32_t remainder = bytes % alignment; + return remainder == 0 ? bytes : bytes + (alignment - remainder); +} + +VulkanStage* VulkanStagePool::allocateNewStage(uint32_t capacity) { + VkBufferCreateInfo bufferInfo{ .sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, - .size = numBytes, + .size = alignToNonCoherentAtomSize(capacity), .usage = VK_BUFFER_USAGE_TRANSFER_SRC_BIT, }; VmaAllocationCreateInfo allocInfo { .usage = VMA_MEMORY_USAGE_CPU_ONLY }; - UTILS_UNUSED_IN_RELEASE VkResult result = vmaCreateBuffer(mAllocator, &bufferInfo, - &allocInfo, &stage->buffer, &stage->memory, nullptr); + VkBuffer buffer; + VmaAllocation memory; + VkResult result = + vmaCreateBuffer(mAllocator, &bufferInfo, &allocInfo, &buffer, &memory, nullptr); #if FVK_ENABLED(FVK_DEBUG_STAGING_ALLOCATION) if (result != VK_SUCCESS) { FVK_LOGE << "Allocation error: " << result << utils::io::endl; + } else { + FVK_LOGD << "Allocated stage with hndl " << buffer << utils::io::endl; } #endif - return stage; + void* pMapping = nullptr; + if (result == VK_SUCCESS) { + result = vmaMapMemory(mAllocator, memory, &pMapping); + +#if FVK_ENABLED(FVK_DEBUG_STAGING_ALLOCATION) + if (result != VK_SUCCESS) { + FVK_LOGE << "Memory mapping erryr: " << result << utils::io::endl; + } +#endif + } + + return new VulkanStage(memory, buffer, capacity, pMapping); +} + +void VulkanStagePool::destroyStage(VulkanStage const*&& stage) { + assert(stage->isSafeToReset()); // Ensure all segments have been reset already. + vmaUnmapMemory(mAllocator, stage->memory()); + vmaDestroyBuffer(mAllocator, stage->buffer(), stage->memory()); + delete stage; } VulkanStageImage const* VulkanStagePool::acquireImage(PixelDataFormat format, PixelDataType type, @@ -141,27 +206,34 @@ void VulkanStagePool::gc() noexcept { } const uint64_t evictionTime = mCurrentFrame - TIME_BEFORE_EVICTION; - // Destroy buffers that have not been used for several frames. - decltype(mFreeStages) freeStages; - freeStages.swap(mFreeStages); - for (auto pair : freeStages) { - if (pair.second->lastAccessed < evictionTime) { - vmaDestroyBuffer(mAllocator, pair.second->buffer, pair.second->memory); - delete pair.second; - } else { - mFreeStages.insert(pair); - } - } + decltype(mStages) freeStages; + freeStages.swap(mStages); + uint8_t freeStageCount = 0; // Assuming we'll never have > 255 free stages + for (auto& pair : freeStages) { + // First, find any stages that have no segments within them. + if (pair.second->isSafeToReset()) { + if (++freeStageCount > MAX_EMPTY_STAGES_TO_RETAIN) { +#if FVK_ENABLED(FVK_DEBUG_STAGING_ALLOCATION) + FVK_LOGD << "Destroying a staging buffer with hndl " << pair.second->buffer() + << utils::io::endl; +#endif + destroyStage(std::move(pair.second)); + continue; + } - // Reclaim buffers that are no longer being used by any command buffer. - decltype(mUsedStages) usedStages; - usedStages.swap(mUsedStages); - for (auto stage : usedStages) { - if (stage->lastAccessed < evictionTime) { - stage->lastAccessed = mCurrentFrame; - mFreeStages.insert(std::make_pair(stage->capacity, stage)); +#if FVK_ENABLED(FVK_DEBUG_STAGING_ALLOCATION) + if (pair.first == 0) { + FVK_LOGD << "Recycling an unused staging buffer with hndl " << pair.second->buffer() + << utils::io::endl; + } +#endif + + // Note - this segment is free, make sure the structure is cleared + // and reinsert it into our free stage list. + pair.second->reset(); + mStages.insert({ pair.second->capacity(), pair.second }); } else { - mUsedStages.push_back(stage); + mStages.insert(pair); } } @@ -192,17 +264,10 @@ void VulkanStagePool::gc() noexcept { } void VulkanStagePool::terminate() noexcept { - for (auto stage : mUsedStages) { - vmaDestroyBuffer(mAllocator, stage->buffer, stage->memory); - delete stage; + for (auto& pair : mStages) { + destroyStage(std::move(pair.second)); } - mUsedStages.clear(); - - for (auto pair : mFreeStages) { - vmaDestroyBuffer(mAllocator, pair.second->buffer, pair.second->memory); - delete pair.second; - } - mFreeStages.clear(); + mStages.clear(); for (auto image : mUsedImages) { vmaDestroyImage(mAllocator, image->image, image->memory); diff --git a/filament/backend/src/vulkan/VulkanStagePool.h b/filament/backend/src/vulkan/VulkanStagePool.h index 155a3188e9..1de5ffb4a4 100644 --- a/filament/backend/src/vulkan/VulkanStagePool.h +++ b/filament/backend/src/vulkan/VulkanStagePool.h @@ -17,8 +17,11 @@ #ifndef TNT_FILAMENT_BACKEND_VULKANSTAGEPOOL_H #define TNT_FILAMENT_BACKEND_VULKANSTAGEPOOL_H -#include "backend/DriverEnums.h" #include "VulkanMemory.h" +#include "backend/DriverEnums.h" +#include "vulkan/memory/Resource.h" +#include "vulkan/memory/ResourceManager.h" +#include "vulkan/memory/ResourcePointer.h" #include #include @@ -28,12 +31,96 @@ namespace filament::backend { class VulkanCommands; -// Immutable POD representing a shared CPU-GPU staging area. -struct VulkanStage { - VmaAllocation memory; - VkBuffer buffer; - uint32_t capacity; - mutable uint64_t lastAccessed; +// Object representing a shared CPU-GPU staging area, which can be subdivided +// into smaller buffers as needed. +class VulkanStage { +public: + VulkanStage(VmaAllocation memory, VkBuffer buffer, uint32_t capacity, void* mapping) + : mMemory(memory), + mBuffer(buffer), + mCapacity(capacity), + mMapping(mapping) {} + + ~VulkanStage() = default; + VulkanStage(const VulkanStage& other) = delete; + VulkanStage(VulkanStage&& other) = delete; + VulkanStage& operator=(const VulkanStage& other) = delete; + VulkanStage& operator=(VulkanStage&& other) = delete; + + class Segment : public fvkmemory::Resource { + public: + using OnRecycle = std::function; + + Segment(VulkanStage* parentStage, uint32_t capacity, uint32_t offset, + OnRecycle&& onRecycleFn) + : mParentStage(parentStage), + mCapacity(capacity), + mOffset(offset), + mOnRecycleFn(onRecycleFn) {} + + ~Segment() { + if (mOnRecycleFn) { + mOnRecycleFn(offset()); + } + } + + // Should not be copying this around. + Segment(const Segment& other) = delete; + Segment(Segment&& other) = delete; + Segment& operator=(const Segment& other) = delete; + Segment& operator=(Segment&& other) = delete; + + inline VulkanStage* parentStage() const { return mParentStage; } + inline VkBuffer buffer() const { return parentStage()->buffer(); } + inline VmaAllocation memory() const { return parentStage()->memory(); } + inline uint32_t capacity() const { return mCapacity; } + inline uint32_t offset() const { return mOffset; } + + inline void* mapping() const { + return reinterpret_cast( + reinterpret_cast(mParentStage->mapping()) + offset()); + } + + private: + // Ensure parent class can access the terminate method. + friend class VulkanStage; + + VulkanStage* const mParentStage; + const uint32_t mCapacity; + const uint32_t mOffset; + OnRecycle mOnRecycleFn; + }; + + inline VmaAllocation memory() const { return mMemory; } + inline VkBuffer buffer() const { return mBuffer; } + inline uint32_t capacity() const { return mCapacity; } + inline void* mapping() const { return mMapping; } + + inline uint32_t currentOffset() { return mCurrentOffset; } + + inline bool isSafeToReset() const { return mSegments.empty(); } + + inline void reset() { mCurrentOffset = 0; } + + // Marks a region of the block as "in-use", and provides information about + // the allocated region to the caller. Note: this assumes that numBytes + // is aligned to the physical device's nonCoherentAtomSize. + fvkmemory::resource_ptr acquireSegment(fvkmemory::ResourceManager* resManager, + uint32_t numBytes); + +private: + const VmaAllocation mMemory; + const VkBuffer mBuffer; + const uint32_t mCapacity; + + void* mMapping; + + uint32_t mCurrentOffset = 0; + + // Maps the start offset of a vulkan stage block to the stage block, + // for easy deletions later. This is managed by the blocks themselves, in an + // RAII pattern, during construction and destruction. + std::unordered_map mSegments; }; struct VulkanStageImage { @@ -49,11 +136,15 @@ struct VulkanStageImage { // This class manages two types of host-mappable staging areas: buffer stages and image stages. class VulkanStagePool { public: - VulkanStagePool(VmaAllocator allocator, VulkanCommands* commands); + VulkanStagePool(VmaAllocator allocator, fvkmemory::ResourceManager* resManager, + VulkanCommands* commands, const VkPhysicalDeviceLimits* deviceLimits); - // 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. - VulkanStage const* acquireStage(uint32_t numBytes); + // Finds or creates a stage block whose capacity is at least the given + // number of bytes. Internally, creates and manages and subdivides large + // buffers so that we have less objects around that we have to keep track + // of. + // This function is NOT thread-safe. + fvkmemory::resource_ptr acquireStage(uint32_t numBytes); // Images have VK_IMAGE_LAYOUT_GENERAL and must not be transitioned to any other layout VulkanStageImage const* acquireImage(PixelDataFormat format, PixelDataType type, @@ -64,17 +155,37 @@ 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. + // Note: it is expected that all resources have been reclaimed before this + // is called. It is also expected that this stage pool does not hold any + // resource_ptrs, as this would lead to undefined behavior. void terminate() noexcept; private: VmaAllocator mAllocator; + fvkmemory::ResourceManager* mResManager; VulkanCommands* mCommands; + const VkPhysicalDeviceLimits* mDeviceLimits; + + // Takes a number of bytes, and aligns it to the non-coherent atom size. + // This allows us to ensure that when we flush buffers from the host, we + // never flush more atoms than we need to. + uint32_t alignToNonCoherentAtomSize(uint32_t numBytes); + + // Allocates a new stage buffer, and optionally subdivides it into stage + // blocks. If subdivideBlocks is true, predefined divisions will be used. + // Otherwise, it's expected that capacity is defined to a value, and that + // is the size that will be used for the buffer (as well as the only block + // being created). + VulkanStage* allocateNewStage(uint32_t capacity); + + // Performs any bookkeeping required to delete a VulkanStage object; namely, + // unmapping memory, freeing the allocation, and deleting the VulkanStage + // object. Note: takes an r-value because after this call, `stage` won't + // exist. + void destroyStage(VulkanStage const*&& stage); // Use an ordered multimap for quick (capacity => stage) lookups using lower_bound(). - std::multimap mFreeStages; - - // Simple unordered set for stashing a list of in-use stages that can be reclaimed later. - std::vector mUsedStages; + std::multimap mStages; std::unordered_set mFreeImages; std::vector mUsedImages; diff --git a/filament/backend/src/vulkan/VulkanTexture.cpp b/filament/backend/src/vulkan/VulkanTexture.cpp index 5e4f16685c..1d0fd8b611 100644 --- a/filament/backend/src/vulkan/VulkanTexture.cpp +++ b/filament/backend/src/vulkan/VulkanTexture.cpp @@ -480,31 +480,30 @@ void VulkanTexture::updateImage(const PixelBufferDescriptor& data, uint32_t widt assert_invariant(hostData->size > 0 && "Data is empty"); // Otherwise, use vkCmdCopyBufferToImage. - void* mapped = nullptr; - VulkanStage const* stage = mState->mStagePool.acquireStage(hostData->size); - assert_invariant(stage->memory); - vmaMapMemory(mState->mAllocator, stage->memory, &mapped); - memcpy(mapped, hostData->buffer, hostData->size); - vmaUnmapMemory(mState->mAllocator, stage->memory); - vmaFlushAllocation(mState->mAllocator, stage->memory, 0, hostData->size); + // Note: the following stageSegment must be stored within the command buffer + // before going out of scope, to ensure proper bookkeeping within the + // staging buffer pool. + fvkmemory::resource_ptr stageSegment = + mState->mStagePool.acquireStage(hostData->size); + assert_invariant(stageSegment->memory()); + memcpy(stageSegment->mapping(), hostData->buffer, hostData->size); + vmaFlushAllocation(mState->mAllocator, stageSegment->memory(), stageSegment->offset(), + hostData->size); VulkanCommandBuffer& commands = mState->mCommands->get(); VkCommandBuffer const cmdbuf = commands.buffer(); + commands.acquire(stageSegment); commands.acquire(fvkmemory::resource_ptr::cast(this)); - VkBufferImageCopy copyRegion = { - .bufferOffset = {}, + VkBufferImageCopy copyRegion = { .bufferOffset = stageSegment->offset(), .bufferRowLength = {}, .bufferImageHeight = {}, - .imageSubresource = { - .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, + .imageSubresource = { .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, .mipLevel = miplevel, .baseArrayLayer = 0, - .layerCount = 1 - }, + .layerCount = 1 }, .imageOffset = { int32_t(xoffset), int32_t(yoffset), int32_t(zoffset) }, - .imageExtent = { width, height, depth } - }; + .imageExtent = { width, height, depth } }; VkImageSubresourceRange transitionRange = { .aspectMask = getImageAspect(), @@ -536,7 +535,8 @@ void VulkanTexture::updateImage(const PixelBufferDescriptor& data, uint32_t widt transitionLayout(&commands, transitionRange, newLayout); - vkCmdCopyBufferToImage(cmdbuf, stage->buffer, mState->mTextureImage, newVkLayout, 1, ©Region); + vkCmdCopyBufferToImage(cmdbuf, stageSegment->buffer(), mState->mTextureImage, newVkLayout, 1, + ©Region); transitionLayout(&commands, transitionRange, nextLayout); } diff --git a/filament/backend/src/vulkan/memory/Resource.cpp b/filament/backend/src/vulkan/memory/Resource.cpp index e11d69d937..ac57c9acb6 100644 --- a/filament/backend/src/vulkan/memory/Resource.cpp +++ b/filament/backend/src/vulkan/memory/Resource.cpp @@ -26,6 +26,7 @@ template ResourceType getTypeEnum() noexcept; template ResourceType getTypeEnum() noexcept; template ResourceType getTypeEnum() noexcept; template ResourceType getTypeEnum() noexcept; +template ResourceType getTypeEnum() noexcept; template ResourceType getTypeEnum() noexcept; template ResourceType getTypeEnum() noexcept; template ResourceType getTypeEnum() noexcept; @@ -54,6 +55,9 @@ ResourceType getTypeEnum() noexcept { if constexpr (std::is_same_v) { return ResourceType::SWAP_CHAIN; } + if constexpr (std::is_same_v) { + return ResourceType::STAGE_SEGMENT; + } if constexpr (std::is_same_v) { return ResourceType::RENDER_PRIMITIVE; } @@ -99,6 +103,8 @@ std::string getTypeStr(ResourceType type) { return "RenderTarget"; case ResourceType::SWAP_CHAIN: return "SwapChain"; + case ResourceType::STAGE_SEGMENT: + return "Stage::Segment"; case ResourceType::RENDER_PRIMITIVE: return "RenderPrimitive"; case ResourceType::TEXTURE: diff --git a/filament/backend/src/vulkan/memory/Resource.h b/filament/backend/src/vulkan/memory/Resource.h index f42d7037f5..c1444ea415 100644 --- a/filament/backend/src/vulkan/memory/Resource.h +++ b/filament/backend/src/vulkan/memory/Resource.h @@ -50,7 +50,8 @@ enum class ResourceType : uint8_t { DESCRIPTOR_SET = 12, FENCE = 13, VULKAN_BUFFER = 14, - UNDEFINED_TYPE = 15, // Must be the last enum because we use it for iterating over the enums. + STAGE_SEGMENT = 15, + UNDEFINED_TYPE = 16, // Must be the last enum because we use it for iterating over the enums. }; template diff --git a/filament/backend/src/vulkan/memory/ResourceManager.cpp b/filament/backend/src/vulkan/memory/ResourceManager.cpp index 2681b508af..aa2087d9f0 100644 --- a/filament/backend/src/vulkan/memory/ResourceManager.cpp +++ b/filament/backend/src/vulkan/memory/ResourceManager.cpp @@ -77,6 +77,9 @@ void ResourceManager::destroyWithType(ResourceType type, HandleId id) { case ResourceType::SWAP_CHAIN: destruct(Handle(id)); break; + case ResourceType::STAGE_SEGMENT: + destruct(Handle(id)); + break; case ResourceType::RENDER_PRIMITIVE: destruct(Handle(id)); break;