Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vulkan: implicitly free command buffers #7167

Merged
merged 2 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions filament/backend/src/vulkan/VulkanBlitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ void VulkanBlitter::blitColor(BlitArgs args) {
}
#endif
VulkanCommandBuffer& commands = mCommands->get();
VkCommandBuffer const cmdbuffer = commands.cmdbuffer;
VkCommandBuffer const cmdbuffer = commands.buffer();
commands.acquire(src.texture);
commands.acquire(dst.texture);

Expand Down Expand Up @@ -184,7 +184,7 @@ void VulkanBlitter::blitDepth(BlitArgs args) {
}

VulkanCommandBuffer& commands = mCommands->get();
VkCommandBuffer const cmdbuffer = commands.cmdbuffer;
VkCommandBuffer const cmdbuffer = commands.buffer();
commands.acquire(src.texture);
commands.acquire(dst.texture);
blitFast(cmdbuffer, aspect, args.filter, args.srcTarget->getExtent(), src, dst, args.srcRectPair,
Expand All @@ -197,13 +197,11 @@ void VulkanBlitter::terminate() noexcept {
mDepthResolveProgram = nullptr;

if (mTriangleBuffer) {
mTriangleBuffer->terminate();
delete mTriangleBuffer;
mTriangleBuffer = nullptr;
}

if (mParamsBuffer) {
mParamsBuffer->terminate();
delete mParamsBuffer;
mParamsBuffer = nullptr;
}
Expand Down Expand Up @@ -257,7 +255,7 @@ void VulkanBlitter::lazyInit() noexcept {
};

VulkanCommandBuffer& commands = mCommands->get();
VkCommandBuffer const cmdbuffer = commands.cmdbuffer;
VkCommandBuffer const cmdbuffer = commands.buffer();

mTriangleBuffer = new VulkanBuffer(mAllocator, mStagePool, VK_BUFFER_USAGE_VERTEX_BUFFER_BIT,
sizeof(kTriangleVertices));
Expand All @@ -278,7 +276,7 @@ void VulkanBlitter::blitSlowDepth(VkFilter filter, const VkExtent2D srcExtent, V
lazyInit();

VulkanCommandBuffer* commands = &mCommands->get();
VkCommandBuffer const cmdbuffer = commands->cmdbuffer;
VkCommandBuffer const cmdbuffer = commands->buffer();
commands->acquire(src.texture);
commands->acquire(dst.texture);

Expand Down
7 changes: 0 additions & 7 deletions filament/backend/src/vulkan/VulkanBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,7 @@ VulkanBuffer::VulkanBuffer(VmaAllocator allocator, VulkanStagePool& stagePool,
}

VulkanBuffer::~VulkanBuffer() {
assert_invariant(mGpuMemory == VK_NULL_HANDLE);
assert_invariant(mGpuBuffer == VK_NULL_HANDLE);
}

void VulkanBuffer::terminate() {
vmaDestroyBuffer(mAllocator, mGpuBuffer, mGpuMemory);
mGpuMemory = VK_NULL_HANDLE;
mGpuBuffer = VK_NULL_HANDLE;
}

void VulkanBuffer::loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, uint32_t byteOffset,
Expand Down
1 change: 0 additions & 1 deletion filament/backend/src/vulkan/VulkanBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ class VulkanBuffer {
VulkanBuffer(VmaAllocator allocator, VulkanStagePool& stagePool, VkBufferUsageFlags usage,
uint32_t numBytes);
~VulkanBuffer();
void terminate();
void loadFromCpu(VkCommandBuffer cmdbuf, const void* cpuData, uint32_t byteOffset,
uint32_t numBytes) const;
VkBuffer getGpuBuffer() const {
Expand Down
68 changes: 35 additions & 33 deletions filament/backend/src/vulkan/VulkanCommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,23 @@ VulkanCmdFence::~VulkanCmdFence() {
vkDestroyFence(device, fence, VKALLOC);
}

VulkanCommandBuffer::VulkanCommandBuffer(VulkanResourceAllocator* allocator, VkDevice device,
VkCommandPool pool)
: mResourceManager(allocator) {
// Create the low-level command buffer.
const VkCommandBufferAllocateInfo allocateInfo{
.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO,
.commandPool = pool,
.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY,
.commandBufferCount = 1,
};

// The buffer allocated here will be implicitly reset when vkBeginCommandBuffer is called.
vkAllocateCommandBuffers(device, &allocateInfo, &mBuffer);

// We don't need to deallocate since destroying the pool will free all of the buffers.
}

CommandBufferObserver::~CommandBufferObserver() {}

static VkCommandPool createPool(VkDevice device, uint32_t queueFamilyIndex) {
Expand Down Expand Up @@ -130,7 +147,7 @@ VulkanCommands::VulkanCommands(VkDevice device, VkQueue queue, uint32_t queueFam
}

for (size_t i = 0; i < CAPACITY; ++i) {
mStorage[i] = std::make_unique<VulkanCommandBuffer>(allocator);
mStorage[i] = std::make_unique<VulkanCommandBuffer>(allocator, mDevice, mPool);
}
}

Expand All @@ -151,7 +168,7 @@ VulkanCommandBuffer& VulkanCommands::get() {
// If we ran out of available command buffers, stall until one finishes. This is very rare.
// It occurs only when Filament invokes commit() or endFrame() a large number of times without
// presenting the swap chain or waiting on a fence.
while (mAvailableCount == 0) {
while (mAvailableBufferCount == 0) {
#if VK_REPORT_STALLS
slog.i << "VulkanCommands has stalled. "
<< "If this occurs frequently, consider increasing VK_MAX_COMMAND_BUFFERS."
Expand All @@ -165,24 +182,15 @@ VulkanCommandBuffer& VulkanCommands::get() {
// Find an available slot.
for (size_t i = 0; i < CAPACITY; ++i) {
auto wrapper = mStorage[i].get();
if (wrapper->cmdbuffer == VK_NULL_HANDLE) {
if (wrapper->buffer() == VK_NULL_HANDLE) {
mCurrentCommandBufferIndex = static_cast<int8_t>(i);
currentbuf = wrapper;
break;
}
}

assert_invariant(currentbuf);
--mAvailableCount;

// Create the low-level command buffer.
const VkCommandBufferAllocateInfo allocateInfo {
.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO,
.commandPool = mPool,
.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY,
.commandBufferCount = 1
};
vkAllocateCommandBuffers(mDevice, &allocateInfo, &currentbuf->cmdbuffer);
mAvailableBufferCount--;

// Note that the fence wrapper uses shared_ptr because a DriverAPI fence can also have ownership
// over it. The destruction of the low-level fence occurs either in VulkanCommands::gc(), or in
Expand All @@ -194,7 +202,7 @@ VulkanCommandBuffer& VulkanCommands::get() {
.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO,
.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT,
};
vkBeginCommandBuffer(currentbuf->cmdbuffer, &binfo);
vkBeginCommandBuffer(currentbuf->buffer(), &binfo);

// Notify the observer that a new command buffer has been activated.
if (mObserver) {
Expand Down Expand Up @@ -235,7 +243,7 @@ bool VulkanCommands::flush() {
VulkanCommandBuffer const* currentbuf = mStorage[index].get();
VkSemaphore const renderingFinished = mSubmissionSignals[index];

vkEndCommandBuffer(currentbuf->cmdbuffer);
vkEndCommandBuffer(currentbuf->buffer());

// If the injected semaphore is an "image available" semaphore that has not yet been signaled,
// it is sometimes fine to start executing commands anyway, as along as we stall the GPU at the
Expand All @@ -253,13 +261,15 @@ bool VulkanCommands::flush() {
VK_NULL_HANDLE,
};

VkCommandBuffer const cmdbuffer = currentbuf->buffer();

VkSubmitInfo submitInfo {
.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO,
.waitSemaphoreCount = 0,
.pWaitSemaphores = signals,
.pWaitDstStageMask = waitDestStageMasks,
.commandBufferCount = 1,
.pCommandBuffers = &currentbuf->cmdbuffer,
.pCommandBuffers = &cmdbuffer,
.signalSemaphoreCount = 1u,
.pSignalSemaphores = &renderingFinished,
};
Expand All @@ -278,7 +288,7 @@ bool VulkanCommands::flush() {
}

#if FILAMENT_VULKAN_VERBOSE
slog.i << "Submitting cmdbuffer=" << currentbuf->cmdbuffer
slog.i << "Submitting cmdbuffer=" << cmdbuffer
<< " wait=(" << signals[0] << ", " << signals[1] << ") "
<< " signal=" << renderingFinished
<< io::endl;
Expand Down Expand Up @@ -326,7 +336,7 @@ void VulkanCommands::wait() {
size_t count = 0;
for (size_t i = 0; i < CAPACITY; i++) {
auto wrapper = mStorage[i].get();
if (wrapper->cmdbuffer != VK_NULL_HANDLE
if (wrapper->buffer() != VK_NULL_HANDLE
&& mCurrentCommandBufferIndex != static_cast<int8_t>(i)) {
fences[count++] = wrapper->fence->fence;
}
Expand All @@ -337,33 +347,25 @@ void VulkanCommands::wait() {
}

void VulkanCommands::gc() {
VkCommandBuffer buffers[CAPACITY];
size_t count = 0;
for (size_t i = 0; i < CAPACITY; i++) {
auto wrapper = mStorage[i].get();
if (wrapper->cmdbuffer == VK_NULL_HANDLE) {
if (wrapper->buffer() == VK_NULL_HANDLE) {
continue;
}
VkResult const result = vkWaitForFences(mDevice, 1, &wrapper->fence->fence, VK_TRUE, 0);
if (result != VK_SUCCESS) {
continue;
}
buffers[count++] = wrapper->cmdbuffer;
wrapper->cmdbuffer = VK_NULL_HANDLE;
wrapper->fence->status.store(VK_SUCCESS);
wrapper->fence.reset();
wrapper->clearResources();
++mAvailableCount;
}
if (count > 0) {
vkFreeCommandBuffers(mDevice, mPool, count, buffers);
wrapper->reset();
mAvailableBufferCount++;
}
}

void VulkanCommands::updateFences() {
for (size_t i = 0; i < CAPACITY; i++) {
auto wrapper = mStorage[i].get();
if (wrapper->cmdbuffer != VK_NULL_HANDLE) {
if (wrapper->buffer() != VK_NULL_HANDLE) {
VulkanCmdFence* fence = wrapper->fence.get();
if (fence) {
VkResult status = vkGetFenceStatus(mDevice, fence->fence);
Expand All @@ -384,7 +386,7 @@ void VulkanCommands::pushGroupMarker(char const* str, VulkanGroupMarkers::Timest
#endif

// TODO: Add group marker color to the Driver API
const VkCommandBuffer cmdbuffer = get().cmdbuffer;
VkCommandBuffer const cmdbuffer = get().buffer();

if (!mGroupMarkers) {
mGroupMarkers = std::make_unique<VulkanGroupMarkers>();
Expand Down Expand Up @@ -412,7 +414,7 @@ void VulkanCommands::popGroupMarker() {
assert_invariant(mGroupMarkers);

if (!mGroupMarkers->empty()) {
const VkCommandBuffer cmdbuffer = get().cmdbuffer;
VkCommandBuffer const cmdbuffer = get().buffer();
#if FILAMENT_VULKAN_VERBOSE
auto const [marker, startTime] = mGroupMarkers->pop();
auto const endTime = std::chrono::high_resolution_clock::now();
Expand All @@ -437,7 +439,7 @@ void VulkanCommands::popGroupMarker() {
}

void VulkanCommands::insertEventMarker(char const* string, uint32_t len) {
VkCommandBuffer const cmdbuffer = get().cmdbuffer;
VkCommandBuffer const cmdbuffer = get().buffer();
if (mContext->isDebugUtilsSupported()) {
VkDebugUtilsLabelEXT labelInfo = {
.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT,
Expand Down
20 changes: 14 additions & 6 deletions filament/backend/src/vulkan/VulkanCommands.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,10 @@ struct VulkanCmdFence {
// DriverApi fence object and should not be destroyed until both the DriverApi object is freed and
// we're done waiting on the most recent submission of the given command buffer.
struct VulkanCommandBuffer {
VulkanCommandBuffer(VulkanResourceAllocator* allocator)
: mResourceManager(allocator) {}
VulkanCommandBuffer(VulkanResourceAllocator* allocator, VkDevice device, VkCommandPool pool);

VulkanCommandBuffer(VulkanCommandBuffer const&) = delete;
VulkanCommandBuffer& operator=(VulkanCommandBuffer const&) = delete;
VkCommandBuffer cmdbuffer = VK_NULL_HANDLE;
std::shared_ptr<VulkanCmdFence> fence;

inline void acquire(VulkanResource* resource) {
mResourceManager.acquire(resource);
Expand All @@ -87,12 +84,23 @@ struct VulkanCommandBuffer {
mResourceManager.acquire(srcResources);
}

inline void clearResources() {
inline void reset() {
fence.reset();
mResourceManager.clear();
}

inline VkCommandBuffer buffer() const {
if (fence) {
return mBuffer;
}
return VK_NULL_HANDLE;
}

std::shared_ptr<VulkanCmdFence> fence;

private:
VulkanAcquireOnlyResourceManager mResourceManager;
VkCommandBuffer mBuffer;
};

// Allows classes to be notified after a new command buffer has been activated.
Expand Down Expand Up @@ -185,7 +193,7 @@ class VulkanCommands {
VkSemaphore mInjectedSignal = {};
utils::FixedCapacityVector<std::unique_ptr<VulkanCommandBuffer>> mStorage;
VkSemaphore mSubmissionSignals[CAPACITY] = {};
size_t mAvailableCount = CAPACITY;
uint8_t mAvailableBufferCount = CAPACITY;
CommandBufferObserver* mObserver = nullptr;

std::unique_ptr<VulkanGroupMarkers> mGroupMarkers;
Expand Down
7 changes: 4 additions & 3 deletions filament/backend/src/vulkan/VulkanContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,9 @@ void VulkanTimestamps::beginQuery(VulkanCommandBuffer const* commands,
VulkanTimerQuery* query) {
uint32_t const index = query->getStartingQueryIndex();

vkCmdResetQueryPool(commands->cmdbuffer, mPool, index, 2);
vkCmdWriteTimestamp(commands->cmdbuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, mPool, index);
auto const cmdbuffer = commands->buffer();
vkCmdResetQueryPool(cmdbuffer, mPool, index, 2);
vkCmdWriteTimestamp(cmdbuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, mPool, index);

// We stash this because getResult might come before the query is actually processed.
query->setFence(commands->fence);
Expand All @@ -127,7 +128,7 @@ void VulkanTimestamps::beginQuery(VulkanCommandBuffer const* commands,
void VulkanTimestamps::endQuery(VulkanCommandBuffer const* commands,
VulkanTimerQuery const* query) {
uint32_t const index = query->getStoppingQueryIndex();
vkCmdWriteTimestamp(commands->cmdbuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, mPool, index);
vkCmdWriteTimestamp(commands->buffer(), VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, mPool, index);
}

VulkanTimestamps::QueryResult VulkanTimestamps::getResult(VulkanTimerQuery const* query) {
Expand Down
14 changes: 7 additions & 7 deletions filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ void VulkanDriver::updateIndexBuffer(Handle<HwIndexBuffer> ibh, BufferDescriptor
VulkanCommandBuffer& commands = mCommands->get();
auto ib = mResourceAllocator.handle_cast<VulkanIndexBuffer*>(ibh);
commands.acquire(ib);
ib->buffer.loadFromCpu(commands.cmdbuffer, p.buffer, byteOffset, p.size);
ib->buffer.loadFromCpu(commands.buffer(), p.buffer, byteOffset, p.size);

scheduleDestroy(std::move(p));
}
Expand All @@ -884,7 +884,7 @@ void VulkanDriver::updateBufferObject(Handle<HwBufferObject> boh, BufferDescript

auto bo = mResourceAllocator.handle_cast<VulkanBufferObject*>(boh);
commands.acquire(bo);
bo->buffer.loadFromCpu(commands.cmdbuffer, bd.buffer, byteOffset, bd.size);
bo->buffer.loadFromCpu(commands.buffer(), bd.buffer, byteOffset, bd.size);

scheduleDestroy(std::move(bd));
}
Expand All @@ -895,7 +895,7 @@ void VulkanDriver::updateBufferObjectUnsynchronized(Handle<HwBufferObject> boh,
auto bo = mResourceAllocator.handle_cast<VulkanBufferObject*>(boh);
commands.acquire(bo);
// TODO: implement unsynchronized version
bo->buffer.loadFromCpu(commands.cmdbuffer, bd.buffer, byteOffset, bd.size);
bo->buffer.loadFromCpu(commands.buffer(), bd.buffer, byteOffset, bd.size);
mResourceManager.acquire(bo);
scheduleDestroy(std::move(bd));
}
Expand Down Expand Up @@ -1021,7 +1021,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
// the non-sampling case.
bool samplingDepthAttachment = false;
VulkanCommandBuffer& commands = mCommands->get();
VkCommandBuffer const cmdbuffer = commands.cmdbuffer;
VkCommandBuffer const cmdbuffer = commands.buffer();

UTILS_NOUNROLL
for (uint8_t samplerGroupIdx = 0; samplerGroupIdx < Program::SAMPLER_BINDING_COUNT;
Expand Down Expand Up @@ -1247,7 +1247,7 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP

void VulkanDriver::endRenderPass(int) {
VulkanCommandBuffer& commands = mCommands->get();
VkCommandBuffer cmdbuffer = commands.cmdbuffer;
VkCommandBuffer cmdbuffer = commands.buffer();
vkCmdEndRenderPass(cmdbuffer);

VulkanRenderTarget* rt = mCurrentRenderPass.renderTarget;
Expand Down Expand Up @@ -1299,7 +1299,7 @@ void VulkanDriver::nextSubpass(int) {
assert_invariant(renderTarget);
assert_invariant(mCurrentRenderPass.params.subpassMask);

vkCmdNextSubpass(mCommands->get().cmdbuffer, VK_SUBPASS_CONTENTS_INLINE);
vkCmdNextSubpass(mCommands->get().buffer(), VK_SUBPASS_CONTENTS_INLINE);

mPipelineCache.bindRenderPass(mCurrentRenderPass.renderPass,
++mCurrentRenderPass.currentSubpass);
Expand Down Expand Up @@ -1484,7 +1484,7 @@ void VulkanDriver::blit(TargetBufferFlags buffers, Handle<HwRenderTarget> dst, V
void VulkanDriver::draw(PipelineState pipelineState, Handle<HwRenderPrimitive> rph,
const uint32_t instanceCount) {
VulkanCommandBuffer* commands = &mCommands->get();
VkCommandBuffer cmdbuffer = commands->cmdbuffer;
VkCommandBuffer cmdbuffer = commands->buffer();
const VulkanRenderPrimitive& prim = *mResourceAllocator.handle_cast<VulkanRenderPrimitive*>(rph);

Handle<HwProgram> programHandle = pipelineState.program;
Expand Down
Loading
Loading