Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

vk: VulkanBuffer cannot skip staging buffer after a read (#8946)

Browse Source 
The previous introduction of VulkanBufferProxy uses the refcount of
the backing VulkanBuffer as an indication whether there's a write
in progress or not. But we also want to make sure there is no
read in progress (i.e. not referenced in a descriptor set). This
commit fixes the bookkeeping while trying to clean up the relevant
structs.

We add logic to track whether a UBO is actively been read (i.e.
bound to a descriptor set that is itself bound to a command buffer).
This commit is contained in:
Powei Feng
2025-07-22 21:47:45 +00:00
committed by GitHub
parent 13629ed93a
commit 3a5f558874
9 changed files with 124 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
filament/backend/src/vulkan/VulkanBufferProxy.cpp
+18 -18
View File
@@ -20,6 +20,7 @@
#include "VulkanCommands.h"
#include "VulkanContext.h"
#include "VulkanMemory.h"
#include "VulkanHandles.h"
using namespace bluevk;
@@ -33,22 +34,16 @@ VulkanBufferProxy::VulkanBufferProxy(VulkanContext const& context, VmaAllocator
mStagePool(stagePool),
mBufferCache(bufferCache),
mBuffer(mBufferCache.acquire(usage, numBytes)),
mUpdatedOffset(0),
mUpdatedBytes(0) {}
mLastReadAge(0) {}
void VulkanBufferProxy::loadFromCpu(VulkanCommandBuffer& commands, const void* cpuData,
uint32_t byteOffset, uint32_t numBytes) {
// The VulkanBuffer is available if the only object reference is hold by the
// `VulkanBufferProxy`. This means that the buffer is not currently referenced by a
// `VulkanCommandBuffer`.
bool const isAvailable = mBuffer->getCount() == 1;
if (isAvailable) {
// We are up to date with all the update operations, so no need to synchronize with previous
// updates.
mUpdatedBytes = 0;
mUpdatedOffset = 0;
}
// This means that we're recording a write into a command buffer without a previous read, so it
// should be safe to
// 1) Do a direct memcpy in UMA mode
// 2) Skip adding a barrier (to protect the write from writing over a read).
bool const isAvailable = commands.age() != mLastReadAge;
// Keep track of the VulkanBuffer usage
commands.acquire(mBuffer);
@@ -67,6 +62,10 @@ void VulkanBufferProxy::loadFromCpu(VulkanCommandBuffer& commands, const void* c
vmaFlushAllocation(mAllocator, mBuffer->getGpuBuffer()->vmaAllocation, byteOffset,
numBytes);
return;
// TODO: to properly bypass staging buffer, we'd need to be able to swap out a VulkanBuffer,
// which represents a VkBuffer. This means that the corresponding descriptor sets also have
// to be updated.
}
// Note: this should be stored within the command buffer before going out of
@@ -77,10 +76,9 @@ void VulkanBufferProxy::loadFromCpu(VulkanCommandBuffer& commands, const void* c
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
// If there was a previous read, then we need to make sure the following write is properly
// synced with the previous read.
if (mUpdatedBytes > 0 &&
(byteOffset >= mUpdatedOffset && byteOffset <= (mUpdatedOffset + mUpdatedBytes))) {
if (!isAvailable) {
VkAccessFlags srcAccess = 0;
VkPipelineStageFlags srcStage = 0;
if (getUsage() == VulkanBufferUsage::UNIFORM) {
@@ -115,9 +113,6 @@ void VulkanBufferProxy::loadFromCpu(VulkanCommandBuffer& commands, const void* c
};
vkCmdCopyBuffer(commands.buffer(), stage->buffer(), getVkBuffer(), 1, &region);
mUpdatedOffset = byteOffset;
mUpdatedBytes = numBytes;
// Firstly, ensure that the copy finishes before the next draw call.
// Secondly, in case the user decides to upload another chunk (without ever using the first one)
// we need to ensure that this upload completes first (hence
@@ -161,4 +156,9 @@ VulkanBufferUsage VulkanBufferProxy::getUsage() const noexcept {
return mBuffer->getGpuBuffer()->usage;
}
void VulkanBufferProxy::referencedBy(VulkanCommandBuffer& commands) {
commands.acquire(mBuffer);
mLastReadAge = commands.age();
}
} // namespace filament::backend
filament/backend/src/vulkan/VulkanBufferProxy.h
+8 -3
View File
@@ -25,6 +25,9 @@
namespace filament::backend {
struct VulkanDescriptorSet;
struct VulkanCommandBuffer;
// This class acts as a dynamic wrapper for a `VulkanBuffer`. It allows you to modify the
// `VulkanBuffer` it references at runtime, wihtout affecting any external objects.
class VulkanBufferProxy {
@@ -38,17 +41,19 @@ public:
VkBuffer getVkBuffer() const noexcept;
VulkanBufferUsage getUsage() const noexcept;
void referencedBy(VulkanCommandBuffer& commands);
private:
VulkanBufferUsage getUsage() const noexcept;
bool const mStagingBufferBypassEnabled;
VmaAllocator mAllocator;
VulkanStagePool& mStagePool;
VulkanBufferCache& mBufferCache;
fvkmemory::resource_ptr<VulkanBuffer> mBuffer;
uint32_t mUpdatedOffset = 0;
uint32_t mUpdatedBytes = 0;
uint32_t mLastReadAge;
};
} // namespace filament::backend
filament/backend/src/vulkan/VulkanCommands.cpp
+5 -1
View File
@@ -86,6 +86,8 @@ bool VulkanGroupMarkers::empty() const noexcept {
}
#endif // FVK_DEBUG_GROUP_MARKERS
uint32_t VulkanCommandBuffer::sAgeCounter = 0;
VulkanCommandBuffer::VulkanCommandBuffer(VulkanContext const& context, VkDevice device,
VkQueue queue, VkCommandPool pool, bool isProtected)
: mContext(context),
@@ -94,7 +96,8 @@ VulkanCommandBuffer::VulkanCommandBuffer(VulkanContext const& context, VkDevice
mDevice(device),
mQueue(queue),
mBuffer(createCommandBuffer(device, pool)),
mFenceStatus(std::make_shared<VulkanCmdFence>(VK_INCOMPLETE)) {
mFenceStatus(std::make_shared<VulkanCmdFence>(VK_INCOMPLETE)),
mAge(++sAgeCounter) {
VkSemaphoreCreateInfo sci{.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO};
vkCreateSemaphore(mDevice, &sci, VKALLOC, &mSubmission);
@@ -111,6 +114,7 @@ void VulkanCommandBuffer::reset() noexcept {
mMarkerCount = 0;
mResources.clear();
mWaitSemaphores.clear();
mAge = ++sAgeCounter;
// Internally we use the VK_INCOMPLETE status to mean "not yet submitted". When this fence
// gets, gets submitted, its status changes to VK_NOT_READY. Finally, when the GPU actually
filament/backend/src/vulkan/VulkanCommands.h
+7
View File
@@ -108,7 +108,13 @@ struct VulkanCommandBuffer {
return mBuffer;
}
uint32_t age() const {
return mAge;
}
private:
static uint32_t sAgeCounter;
VulkanContext const& mContext;
uint8_t mMarkerCount;
bool const isProtected;
@@ -120,6 +126,7 @@ private:
VkFence mFence;
std::shared_ptr<VulkanCmdFence> mFenceStatus;
std::vector<fvkmemory::resource_ptr<Resource>> mResources;
uint32_t mAge;
};
struct CommandBufferPool {
filament/backend/src/vulkan/VulkanDescriptorSetCache.cpp
+7 -6
View File
@@ -290,7 +290,9 @@ void VulkanDescriptorSetCache::commit(VulkanCommandBuffer* commands,
if (mLastBoundInfo.pipelineLayout == pipelineLayout) {
auto& lastBoundSets = mLastBoundInfo.boundSets;
curMask.forEachSetBit([&](size_t index) {
if (updateSets[index] == lastBoundSets[index] && !useExternalSamplers[index]) {
auto& set = updateSets[index];
if (set == lastBoundSets[index] && !useExternalSamplers[index] &&
set->uniqueDynamicUboCount == 0) {
curMask.unset(index);
}
});
@@ -302,25 +304,24 @@ void VulkanDescriptorSetCache::commit(VulkanCommandBuffer* commands,
VkCommandBuffer const cmdbuffer = commands->buffer();
VkDescriptorSet vkset = useExternalSamplers[index] ? set->getExternalSamplerVkSet() :
set->getVkSet();
commands->acquire(set);
vkCmdBindDescriptorSets(cmdbuffer, VK_PIPELINE_BIND_POINT_GRAPHICS, pipelineLayout, index,
1, &vkset, set->uniqueDynamicUboCount, set->getOffsets()->data());
commands->acquire(set);
set->referencedBy(*commands);
});
mStashedSets = {};
mLastBoundInfo = {
pipelineLayout,
setMask,
updateSets,
};
mStashedSets = {};
}
void VulkanDescriptorSetCache::updateBuffer(fvkmemory::resource_ptr<VulkanDescriptorSet> set,
uint8_t binding, fvkmemory::resource_ptr<VulkanBufferObject> bufferObject,
VkDeviceSize offset, VkDeviceSize size) noexcept {
VkDescriptorBufferInfo const info = {
.buffer = bufferObject->buffer.getVkBuffer(),
.buffer = bufferObject->getVkBuffer(),
.offset = offset,
.range = size,
};
filament/backend/src/vulkan/VulkanDriver.cpp
+4 -4
View File
@@ -1239,7 +1239,7 @@ void VulkanDriver::updateIndexBuffer(Handle<HwIndexBuffer> ibh, BufferDescriptor
VulkanCommandBuffer& commands = mCommands.get();
auto ib = resource_ptr<VulkanIndexBuffer>::cast(&mResourceManager, ibh);
commands.acquire(ib);
ib->buffer.loadFromCpu(commands, p.buffer, byteOffset, p.size);
ib->loadFromCpu(commands, p.buffer, byteOffset, p.size);
scheduleDestroy(std::move(p));
}
@@ -1255,7 +1255,7 @@ void VulkanDriver::updateBufferObject(Handle<HwBufferObject> boh, BufferDescript
auto bo = resource_ptr<VulkanBufferObject>::cast(&mResourceManager, boh);
commands.acquire(bo);
bo->buffer.loadFromCpu(commands, bd.buffer, byteOffset, bd.size);
bo->loadFromCpu(commands, bd.buffer, byteOffset, bd.size);
scheduleDestroy(std::move(bd));
}
@@ -1266,7 +1266,7 @@ void VulkanDriver::updateBufferObjectUnsynchronized(Handle<HwBufferObject> boh,
auto bo = resource_ptr<VulkanBufferObject>::cast(&mResourceManager, boh);
commands.acquire(bo);
// TODO: implement unsynchronized version
bo->buffer.loadFromCpu(commands, bd.buffer, byteOffset, bd.size);
bo->loadFromCpu(commands, bd.buffer, byteOffset, bd.size);
scheduleDestroy(std::move(bd));
}
@@ -1915,7 +1915,7 @@ void VulkanDriver::bindRenderPrimitive(Handle<HwRenderPrimitive> rph) {
// avoid rebinding these if they are already bound, but since we do not (yet) support subranges
// it would be rare for a client to make consecutive draw calls with the same render primitive.
vkCmdBindVertexBuffers(cmdbuffer, 0, bufferCount, buffers, offsets);
vkCmdBindIndexBuffer(cmdbuffer, prim->indexBuffer->buffer.getVkBuffer(), 0,
vkCmdBindIndexBuffer(cmdbuffer, prim->indexBuffer->getVkBuffer(), 0,
prim->indexBuffer->indexType);
}
filament/backend/src/vulkan/VulkanHandles.cpp
+28 -12
View File
@@ -39,6 +39,20 @@ namespace filament::backend {
namespace {
inline VulkanBufferUsage getBufferObjectUsage(BufferObjectBinding bindingType) noexcept {
switch (bindingType) {
case BufferObjectBinding::VERTEX:
return VulkanBufferUsage::VERTEX;
case BufferObjectBinding::UNIFORM:
return VulkanBufferUsage::UNIFORM;
case BufferObjectBinding::SHADER_STORAGE:
return VulkanBufferUsage::SHADER_STORAGE;
// when adding more buffer-types here, make sure to update VulkanBuffer::loadFromCpu()
// if necessary.
}
return VulkanBufferUsage::UNKNOWN;
}
void flipVertically(VkViewport* rect, uint32_t framebufferHeight) {
rect->y = framebufferHeight - rect->y - rect->height;
}
@@ -166,14 +180,6 @@ VulkanAttachment createSwapchainAttachment(const fvkmemory::resource_ptr<VulkanT
} // anonymous namespace
void VulkanDescriptorSet::acquire(fvkmemory::resource_ptr<VulkanTexture> texture) {
mResources.push_back(texture);
}
void VulkanDescriptorSet::acquire(fvkmemory::resource_ptr<VulkanBufferObject> obj) {
mResources.push_back(obj);
}
VulkanDescriptorSetLayout::VulkanDescriptorSetLayout(DescriptorSetLayout&& layout,
VkDescriptorSetLayout vkLayout)
: bitmask(fromBackendLayout(layout)),
@@ -185,6 +191,17 @@ VulkanDescriptorSetLayout::Bitmask VulkanDescriptorSetLayout::Bitmask::fromLayou
return fromBackendLayout(layout);
}
// This method will store an age associated with this command buffer into the VulkanBuffer, which
// will allow us to determine whether a barrier is necessary or not.
void VulkanDescriptorSet::referencedBy(VulkanCommandBuffer& commands) {
mUboMask.forEachSetBit([this, &commands](size_t index) {
auto& res = mResources[index];
fvkmemory::resource_ptr<VulkanBufferObject> bo =
fvkmemory::resource_ptr<VulkanBufferObject>::cast((VulkanBufferObject*) res.get());
bo->referencedBy(commands);
});
}
PushConstantDescription::PushConstantDescription(backend::Program const& program) {
mRangeCount = 0;
for (auto stage : { ShaderStage::VERTEX, ShaderStage::FRAGMENT, ShaderStage::COMPUTE }) {
@@ -588,7 +605,7 @@ void VulkanVertexBuffer::setBuffer(fvkmemory::resource_ptr<VulkanBufferObject> b
int8_t const* const attribToBuffer = vbi->getAttributeToBuffer();
for (uint8_t attribIndex = 0; attribIndex < count; attribIndex++) {
if (attribToBuffer[attribIndex] == static_cast<int8_t>(index)) {
vkbuffers[attribIndex] = bufferObject->buffer.getVkBuffer();
vkbuffers[attribIndex] = bufferObject->getVkBuffer();
}
}
mResources.push_back(bufferObject);
@@ -598,9 +615,8 @@ VulkanBufferObject::VulkanBufferObject(VulkanContext const& context, VmaAllocato
VulkanStagePool& stagePool, VulkanBufferCache& bufferCache, uint32_t byteCount,
BufferObjectBinding bindingType)
: HwBufferObject(byteCount),
buffer(context, allocator, stagePool, bufferCache, getBufferObjectUsage(bindingType),
byteCount),
bindingType(bindingType) {}
bindingType(bindingType),
mBuffer(context, allocator, stagePool, bufferCache, getBufferObjectUsage(bindingType), byteCount) {}
VulkanRenderPrimitive::VulkanRenderPrimitive(PrimitiveType pt,
fvkmemory::resource_ptr<VulkanVertexBuffer> vb,
filament/backend/src/vulkan/VulkanHandles.h
+43 -24
View File
@@ -26,6 +26,7 @@
#include "VulkanFboCache.h"
#include "VulkanSwapChain.h"
#include "VulkanTexture.h"
#include "vulkan/VulkanCommands.h"
#include "vulkan/memory/Resource.h"
#include "vulkan/utils/Definitions.h"
#include "vulkan/utils/StaticVector.h"
@@ -200,8 +201,15 @@ public:
return &mOffsets;
}
void acquire(fvkmemory::resource_ptr<VulkanTexture> texture);
void acquire(fvkmemory::resource_ptr<VulkanBufferObject> buffer);
template<typename Resource>
void acquire(fvkmemory::resource_ptr<Resource> res) {
if (res->template isType<VulkanBufferObject>()) {
mUboMask.set(mResources.size());
}
mResources.push_back(res);
}
void referencedBy(VulkanCommandBuffer& commands);
fvkutils::UniformBufferBitmask const dynamicUboMask;
uint8_t const uniqueDynamicUboCount;
@@ -214,6 +222,7 @@ private:
std::vector<fvkmemory::resource_ptr<fvkmemory::Resource>> mResources;
OnRecycle mOnRecycleFn;
OnRecycle mOnRecycleExternalSamplerFn;
fvkutils::UniformBufferBitmask mUboMask;
};
using PushConstantNameArray = utils::FixedCapacityVector<char const*>;
@@ -419,6 +428,9 @@ struct VulkanVertexBuffer : public HwVertexBuffer, fvkmemory::Resource {
fvkmemory::resource_ptr<VulkanVertexBufferInfo> vbi);
void setBuffer(fvkmemory::resource_ptr<VulkanBufferObject> bufferObject, uint32_t index);
// TODO: because VulkanBufferObject is backed by VulkanBufferProxy, which could switch out the
// backing VkBuffer, we cannot store the VkBuffers for optimization here. We could store a dirty
// bit to indicate if a VkBuffer has changed maybe.
inline VkBuffer const* getVkBuffers() const { return mBuffers.data(); }
inline VkBuffer* getVkBuffers() { return mBuffers.data(); }
fvkmemory::resource_ptr<VulkanVertexBufferInfo> vbi;
@@ -433,12 +445,21 @@ struct VulkanIndexBuffer : public HwIndexBuffer, fvkmemory::Resource {
VulkanStagePool& stagePool, VulkanBufferCache& bufferCache, uint8_t elementSize,
uint32_t indexCount)
: HwIndexBuffer(elementSize, indexCount),
buffer(context, allocator, stagePool, bufferCache, VulkanBufferUsage::INDEX,
elementSize * indexCount),
indexType(elementSize == 2 ? VK_INDEX_TYPE_UINT16 : VK_INDEX_TYPE_UINT32) {}
indexType(elementSize == 2 ? VK_INDEX_TYPE_UINT16 : VK_INDEX_TYPE_UINT32),
mBuffer(context, allocator, stagePool, bufferCache, VulkanBufferUsage::INDEX,
elementSize * indexCount) {}
VulkanBufferProxy buffer;
const VkIndexType indexType;
inline void loadFromCpu(VulkanCommandBuffer& commands, const void* cpuData, uint32_t byteOffset,
uint32_t numBytes) {
mBuffer.loadFromCpu(commands, cpuData, byteOffset, numBytes);
}
inline VkBuffer getVkBuffer() const noexcept { return mBuffer.getVkBuffer(); }
VkIndexType const indexType;
private:
VulkanBufferProxy mBuffer;
};
struct VulkanBufferObject : public HwBufferObject, fvkmemory::Resource {
@@ -446,8 +467,21 @@ struct VulkanBufferObject : public HwBufferObject, fvkmemory::Resource {
VulkanStagePool& stagePool, VulkanBufferCache& bufferCache, uint32_t byteCount,
BufferObjectBinding bindingType);
VulkanBufferProxy buffer;
const BufferObjectBinding bindingType;
inline void loadFromCpu(VulkanCommandBuffer& commands, const void* cpuData, uint32_t byteOffset,
uint32_t numBytes) {
mBuffer.loadFromCpu(commands, cpuData, byteOffset, numBytes);
}
inline VkBuffer getVkBuffer() const noexcept { return mBuffer.getVkBuffer(); }
inline void referencedBy(VulkanCommandBuffer& commands) {
mBuffer.referencedBy(commands);
}
BufferObjectBinding const bindingType;
private:
VulkanBufferProxy mBuffer;
};
struct VulkanRenderPrimitive : public HwRenderPrimitive, fvkmemory::Resource {
@@ -459,21 +493,6 @@ struct VulkanRenderPrimitive : public HwRenderPrimitive, fvkmemory::Resource {
fvkmemory::resource_ptr<VulkanIndexBuffer> indexBuffer;
};
inline constexpr VulkanBufferUsage getBufferObjectUsage(BufferObjectBinding bindingType) noexcept {
switch (bindingType) {
case BufferObjectBinding::VERTEX:
return VulkanBufferUsage::VERTEX;
case BufferObjectBinding::UNIFORM:
return VulkanBufferUsage::UNIFORM;
case BufferObjectBinding::SHADER_STORAGE:
return VulkanBufferUsage::SHADER_STORAGE;
// when adding more buffer-types here, make sure to update VulkanBuffer::loadFromCpu()
// if necessary.
}
return VulkanBufferUsage::UNKNOWN;
}
} // namespace filament::backend
#endif // TNT_FILAMENT_BACKEND_VULKANHANDLES_H
filament/backend/src/vulkan/memory/Resource.h
+4 -1
View File
@@ -72,7 +72,10 @@ struct Resource {
restype(ResourceType::UNDEFINED_TYPE),
mHandleConsideredDestroyed(false) {}
uint32_t getCount() const { return mCount; }
template<typename D>
bool isType() const {
return getTypeEnum<D>() == restype;
}
private:
inline void inc() noexcept {