Commit fe36a647 by Charlie Lao Committed by Commit Bot

Vulkan: Add driverUniform's buffer to mResourceUseList

There is a bug with mDriverUniforms' dynamicBuffer that we never add it to context's mResourceUseList to let it hold onto it until GPU completes. This causes old buffer gets recycled prematurely and gets overwritten, result in rendering corruption. This CL adds current buffer to the mResourceUseList so that it will be waited properly before gets recycled. Bug: b/160777679 Change-Id: I7707442e0f5ba408f5f28337422274e0c23b6bfb Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2288325 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarIan Elliott <ianelliott@google.com>
parent f61272fb
...@@ -284,7 +284,7 @@ EventName GetTraceEventName(const char *title, uint32_t counter) ...@@ -284,7 +284,7 @@ EventName GetTraceEventName(const char *title, uint32_t counter)
} // anonymous namespace } // anonymous namespace
ContextVk::DriverUniformsDescriptorSet::DriverUniformsDescriptorSet() ContextVk::DriverUniformsDescriptorSet::DriverUniformsDescriptorSet()
: descriptorSet(VK_NULL_HANDLE), dynamicOffset(0) : descriptorSet(VK_NULL_HANDLE), dynamicOffset(0), referenced(false)
{} {}
ContextVk::DriverUniformsDescriptorSet::~DriverUniformsDescriptorSet() = default; ContextVk::DriverUniformsDescriptorSet::~DriverUniformsDescriptorSet() = default;
...@@ -914,6 +914,9 @@ angle::Result ContextVk::setupDraw(const gl::Context *context, ...@@ -914,6 +914,9 @@ angle::Result ContextVk::setupDraw(const gl::Context *context,
DirtyBits dirtyBitMask, DirtyBits dirtyBitMask,
vk::CommandBuffer **commandBufferOut) vk::CommandBuffer **commandBufferOut)
{ {
// Mark it as been used so that we will add current buffer to mResourceUseList
mDriverUniforms[PipelineType::Graphics].referenced = true;
// Set any dirty bits that depend on draw call parameters or other objects. // Set any dirty bits that depend on draw call parameters or other objects.
if (mode != mCurrentDrawMode) if (mode != mCurrentDrawMode)
{ {
...@@ -1175,6 +1178,9 @@ angle::Result ContextVk::setupLineLoopDraw(const gl::Context *context, ...@@ -1175,6 +1178,9 @@ angle::Result ContextVk::setupLineLoopDraw(const gl::Context *context,
angle::Result ContextVk::setupDispatch(const gl::Context *context, angle::Result ContextVk::setupDispatch(const gl::Context *context,
vk::CommandBuffer **commandBufferOut) vk::CommandBuffer **commandBufferOut)
{ {
// Mark it as been used so that we will add current buffer to mResourceUseList
mDriverUniforms[PipelineType::Compute].referenced = true;
// |setupDispatch| and |setupDraw| are special in that they flush dirty bits. Therefore they // |setupDispatch| and |setupDraw| are special in that they flush dirty bits. Therefore they
// don't use the same APIs to record commands as the functions outside ContextVk. // don't use the same APIs to record commands as the functions outside ContextVk.
// The following ensures prior commands are flushed before we start processing dirty bits. // The following ensures prior commands are flushed before we start processing dirty bits.
...@@ -3661,9 +3667,6 @@ angle::Result ContextVk::allocateDriverUniforms(size_t driverUniformsSize, ...@@ -3661,9 +3667,6 @@ angle::Result ContextVk::allocateDriverUniforms(size_t driverUniformsSize,
uint8_t **ptrOut, uint8_t **ptrOut,
bool *newBufferOut) bool *newBufferOut)
{ {
// Release any previously retained buffers.
driverUniforms->dynamicBuffer.releaseInFlightBuffers(this);
// Allocate a new region in the dynamic buffer. // Allocate a new region in the dynamic buffer.
VkDeviceSize offset; VkDeviceSize offset;
ANGLE_TRY(driverUniforms->dynamicBuffer.allocate(this, driverUniformsSize, ptrOut, bufferOut, ANGLE_TRY(driverUniforms->dynamicBuffer.allocate(this, driverUniformsSize, ptrOut, bufferOut,
...@@ -3913,6 +3916,26 @@ angle::Result ContextVk::flushImpl(const vk::Semaphore *signalSemaphore) ...@@ -3913,6 +3916,26 @@ angle::Result ContextVk::flushImpl(const vk::Semaphore *signalSemaphore)
} }
ANGLE_TRY(flushOutsideRenderPassCommands()); ANGLE_TRY(flushOutsideRenderPassCommands());
// We must add the per context dynamic buffers into mResourceUseList before submission so that
// they get retained properly until GPU completes
for (DriverUniformsDescriptorSet &driverUniform : mDriverUniforms)
{
driverUniform.dynamicBuffer.releaseInFlightBuffersToResourceUseList(this);
if (driverUniform.referenced)
{
// We always have to retain the current buffer. Even if data has not changed, current
// buffer may still be referenced by this command buffer.
vk::BufferHelper *currentBuffer = driverUniform.dynamicBuffer.getCurrentBuffer();
if (currentBuffer)
{
currentBuffer->retain(&mResourceUseList);
}
driverUniform.referenced = false;
}
}
if (mRenderer->getFeatures().enableCommandProcessingThread.enabled) if (mRenderer->getFeatures().enableCommandProcessingThread.enabled)
{ {
// Worker thread must complete adding any commands that were just flushed above to the // Worker thread must complete adding any commands that were just flushed above to the
......
...@@ -615,6 +615,7 @@ class ContextVk : public ContextImpl, public vk::Context ...@@ -615,6 +615,7 @@ class ContextVk : public ContextImpl, public vk::Context
vk::DynamicBuffer dynamicBuffer; vk::DynamicBuffer dynamicBuffer;
VkDescriptorSet descriptorSet; VkDescriptorSet descriptorSet;
uint32_t dynamicOffset; uint32_t dynamicOffset;
bool referenced; // True if it has been used by current submission
vk::BindingPointer<vk::DescriptorSetLayout> descriptorSetLayout; vk::BindingPointer<vk::DescriptorSetLayout> descriptorSetLayout;
vk::RefCountedDescriptorPoolBinding descriptorPoolBinding; vk::RefCountedDescriptorPoolBinding descriptorPoolBinding;
......
...@@ -1121,6 +1121,26 @@ void DynamicBuffer::release(RendererVk *renderer) ...@@ -1121,6 +1121,26 @@ void DynamicBuffer::release(RendererVk *renderer)
} }
} }
void DynamicBuffer::releaseInFlightBuffersToResourceUseList(ContextVk *contextVk)
{
ResourceUseList *resourceUseList = &contextVk->getResourceUseList();
for (BufferHelper *bufferHelper : mInFlightBuffers)
{
bufferHelper->retain(resourceUseList);
// If the dynamic buffer was resized we cannot reuse the retained buffer.
if (bufferHelper->getSize() < mSize)
{
bufferHelper->release(contextVk->getRenderer());
}
else
{
mBufferFreeList.push_back(bufferHelper);
}
}
mInFlightBuffers.clear();
}
void DynamicBuffer::releaseInFlightBuffers(ContextVk *contextVk) void DynamicBuffer::releaseInFlightBuffers(ContextVk *contextVk)
{ {
for (BufferHelper *toRelease : mInFlightBuffers) for (BufferHelper *toRelease : mInFlightBuffers)
......
...@@ -102,6 +102,9 @@ class DynamicBuffer : angle::NonCopyable ...@@ -102,6 +102,9 @@ class DynamicBuffer : angle::NonCopyable
// This releases all the buffers that have been allocated since this was last called. // This releases all the buffers that have been allocated since this was last called.
void releaseInFlightBuffers(ContextVk *contextVk); void releaseInFlightBuffers(ContextVk *contextVk);
// This adds inflight buffers to the context's mResourceUseList and then releases them
void releaseInFlightBuffersToResourceUseList(ContextVk *contextVk);
// This frees resources immediately. // This frees resources immediately.
void destroy(RendererVk *renderer); void destroy(RendererVk *renderer);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment