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 <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>
Reviewed-by: Ian Elliott <[email protected]>

Author: cclao

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -284,7 +284,7 @@ EventName GetTraceEventName(const char *title, uint32_t counter)
 }  // anonymous namespace
 
 ContextVk::DriverUniformsDescriptorSet::DriverUniformsDescriptorSet()
-    : descriptorSet(VK_NULL_HANDLE), dynamicOffset(0)
+    : descriptorSet(VK_NULL_HANDLE), dynamicOffset(0), referenced(false)
 {}
 
 ContextVk::DriverUniformsDescriptorSet::~DriverUniformsDescriptorSet() = default;
@@ -914,6 +914,9 @@ angle::Result ContextVk::setupDraw(const gl::Context *context,
                                    DirtyBits dirtyBitMask,
                                    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.
     if (mode != mCurrentDrawMode)
     {
@@ -1175,6 +1178,9 @@ angle::Result ContextVk::setupLineLoopDraw(const gl::Context *context,
 angle::Result ContextVk::setupDispatch(const gl::Context *context,
                                        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
     // 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.
@@ -3661,9 +3667,6 @@ angle::Result ContextVk::allocateDriverUniforms(size_t driverUniformsSize,
                                                 uint8_t **ptrOut,
                                                 bool *newBufferOut)
 {
-    // Release any previously retained buffers.
-    driverUniforms->dynamicBuffer.releaseInFlightBuffers(this);
-
     // Allocate a new region in the dynamic buffer.
     VkDeviceSize offset;
     ANGLE_TRY(driverUniforms->dynamicBuffer.allocate(this, driverUniformsSize, ptrOut, bufferOut,
@@ -3913,6 +3916,26 @@ angle::Result ContextVk::flushImpl(const vk::Semaphore *signalSemaphore)
     }
     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)
     {
         // Worker thread must complete adding any commands that were just flushed above to the

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -615,6 +615,7 @@ class ContextVk : public ContextImpl, public vk::Context
         vk::DynamicBuffer dynamicBuffer;
         VkDescriptorSet descriptorSet;
         uint32_t dynamicOffset;
+        bool referenced;  // True if it has been used by current submission
         vk::BindingPointer<vk::DescriptorSetLayout> descriptorSetLayout;
         vk::RefCountedDescriptorPoolBinding descriptorPoolBinding;
 

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -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)
 {
     for (BufferHelper *toRelease : mInFlightBuffers)

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -102,6 +102,9 @@ class DynamicBuffer : angle::NonCopyable
     // This releases all the buffers that have been allocated since this was last called.
     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.
     void destroy(RendererVk *renderer);