Vulkan: Update resource dependency semantics.

src/libANGLE/renderer/vulkan/BufferVk.cpp

@@ -190,7 +190,7 @@ vk::Error BufferVk::setDataImpl(ContextVk *contextVk,
     VkDevice device      = contextVk->getDevice();
 
     // Use map when available.
-    if (renderer->isSerialInUse(getQueueSerial()))
+    if (checkResourceInUseAndRefreshDeps(renderer))
     {
         vk::StagingBuffer stagingBuffer;
         ANGLE_TRY(stagingBuffer.init(contextVk, static_cast<VkDeviceSize>(size),

src/libANGLE/renderer/vulkan/CommandGraph.cpp

@@ -77,15 +77,13 @@ Serial CommandGraphResource::getQueueSerial() const
     return mStoredQueueSerial;
 }
 
-bool CommandGraphResource::hasCurrentWritingNode(Serial currentSerial) const
+bool CommandGraphResource::hasChildlessWritingNode() const
 {
-    return (mStoredQueueSerial == currentSerial && mCurrentWritingNode != nullptr &&
-            !mCurrentWritingNode->hasChildren());
+    return (mCurrentWritingNode != nullptr && !mCurrentWritingNode->hasChildren());
 }
 
-CommandGraphNode *CommandGraphResource::getCurrentWritingNode(Serial currentSerial)
+CommandGraphNode *CommandGraphResource::getCurrentWritingNode()
 {
-    ASSERT(currentSerial == mStoredQueueSerial);
     return mCurrentWritingNode;
 }
 
@@ -111,7 +109,7 @@ void CommandGraphResource::onWriteResource(CommandGraphNode *writingNode, Serial
 {
     updateQueueSerial(serial);
 
-    // Make sure any open reads and writes finish before we execute 'newCommands'.
+    // Make sure any open reads and writes finish before we execute 'writingNode'.
     if (!mCurrentReadingNodes.empty())
     {
         CommandGraphNode::SetHappensBeforeDependencies(mCurrentReadingNodes, writingNode);
@@ -128,19 +126,32 @@ void CommandGraphResource::onWriteResource(CommandGraphNode *writingNode, Serial
 
 void CommandGraphResource::onReadResource(CommandGraphNode *readingNode, Serial serial)
 {
-    if (hasCurrentWritingNode(serial))
+    updateQueueSerial(serial);
+
+    if (hasChildlessWritingNode())
     {
-        // Ensure 'readOperation' happens after the current write commands.
-        CommandGraphNode::SetHappensBeforeDependency(getCurrentWritingNode(serial), readingNode);
         ASSERT(mStoredQueueSerial == serial);
+
+        // Ensure 'readingNode' happens after the current writing node.
+        CommandGraphNode::SetHappensBeforeDependency(mCurrentWritingNode, readingNode);
+    }
+
+    // Add the read node to the list of nodes currently reading this resource.
+    mCurrentReadingNodes.push_back(readingNode);
+}
+
+bool CommandGraphResource::checkResourceInUseAndRefreshDeps(RendererVk *renderer)
+{
+    if (!renderer->isResourceInUse(*this))
+    {
+        mCurrentReadingNodes.clear();
+        mCurrentWritingNode = nullptr;
+        return false;
     }
     else
     {
-        updateQueueSerial(serial);
+        return true;
     }
-
-    // Add the read operation to the list of nodes currently reading this resource.
-    mCurrentReadingNodes.push_back(readingNode);
 }
 
 // CommandGraphNode implementation.
@@ -247,9 +258,9 @@ void CommandGraphNode::appendDepthStencilRenderTarget(Serial serial,
 void CommandGraphNode::SetHappensBeforeDependency(CommandGraphNode *beforeNode,
                                                   CommandGraphNode *afterNode)
 {
+    ASSERT(beforeNode != afterNode && !beforeNode->isChildOf(afterNode));
     afterNode->mParents.emplace_back(beforeNode);
     beforeNode->setHasChildren();
-    ASSERT(beforeNode != afterNode && !beforeNode->isChildOf(afterNode));
 }
 
 // static

src/libANGLE/renderer/vulkan/CommandGraph.h

@@ -33,11 +33,11 @@ class CommandGraphResource
     void updateQueueSerial(Serial queueSerial);
     Serial getQueueSerial() const;
 
-    // Returns true if any tracked read or write nodes match 'currentSerial'.
-    bool hasCurrentWritingNode(Serial currentSerial) const;
+    // Returns true if this node has a current writing node with no children.
+    bool hasChildlessWritingNode() const;
 
-    // Returns the active write node, and asserts 'currentSerial' matches the stored serial.
-    CommandGraphNode *getCurrentWritingNode(Serial currentSerial);
+    // Returns the active write node.
+    CommandGraphNode *getCurrentWritingNode();
 
     // Allocates a new write node and calls onWriteResource internally.
     CommandGraphNode *getNewWritingNode(RendererVk *renderer);
@@ -52,6 +52,9 @@ class CommandGraphResource
     // Sets up dependency relations. 'readingNode' will read from 'this' ResourceVk.
     void onReadResource(CommandGraphNode *readingNode, Serial serial);
 
+    // Returns false if the resource is not in use, and clears any current read/write nodes.
+    bool checkResourceInUseAndRefreshDeps(RendererVk *renderer);
+
   private:
     Serial mStoredQueueSerial;
     std::vector<CommandGraphNode *> mCurrentReadingNodes;

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -54,20 +54,12 @@ FramebufferVk *FramebufferVk::CreateDefaultFBO(const gl::FramebufferState &state
 }
 
 FramebufferVk::FramebufferVk(const gl::FramebufferState &state)
-    : FramebufferImpl(state),
-      mBackbuffer(nullptr),
-      mRenderPassDesc(),
-      mFramebuffer(),
-      mLastRenderNodeSerial()
+    : FramebufferImpl(state), mBackbuffer(nullptr), mRenderPassDesc(), mFramebuffer()
 {
 }
 
 FramebufferVk::FramebufferVk(const gl::FramebufferState &state, WindowSurfaceVk *backbuffer)
-    : FramebufferImpl(state),
-      mBackbuffer(backbuffer),
-      mRenderPassDesc(),
-      mFramebuffer(),
-      mLastRenderNodeSerial()
+    : FramebufferImpl(state), mBackbuffer(backbuffer), mRenderPassDesc(), mFramebuffer()
 {
 }
 
@@ -154,7 +146,7 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
     if (clearDepth || clearStencil)
     {
         ANGLE_TRY(beginWriteResource(renderer, &commandBuffer));
-        writingNode = getCurrentWritingNode(currentSerial);
+        writingNode = getCurrentWritingNode();
 
         const VkClearDepthStencilValue &clearDepthStencilValue =
             contextVk->getClearDepthStencilValue().depthStencil;
@@ -183,7 +175,7 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
     if (!commandBuffer)
     {
         ANGLE_TRY(beginWriteResource(renderer, &commandBuffer));
-        writingNode = getCurrentWritingNode(currentSerial);
+        writingNode = getCurrentWritingNode();
     }
 
     // TODO(jmadill): Support gaps in RenderTargets. http://anglebug.com/2394
@@ -340,8 +332,8 @@ gl::Error FramebufferVk::syncState(const gl::Context *context,
     mRenderPassDesc.reset();
     renderer->releaseResource(*this, &mFramebuffer);
 
-    // Trigger a new set of secondary commands next time we render to this FBO,.
-    mLastRenderNodeSerial = Serial();
+    // Trigger a new set of secondary commands next time we render to this FBO.
+    getNewWritingNode(renderer);
 
     contextVk->invalidateCurrentPipeline();
 
@@ -538,14 +530,22 @@ gl::Error FramebufferVk::getCommandGraphNodeForDraw(const gl::Context *context,
     RendererVk *renderer = contextVk->getRenderer();
     Serial currentSerial = renderer->getCurrentQueueSerial();
 
-    if (hasCurrentWritingNode(currentSerial) && mLastRenderNodeSerial == currentSerial)
+    // This will reset the current writing node if it has been completed.
+    updateQueueSerial(currentSerial);
+
+    if (hasChildlessWritingNode())
     {
-        *nodeOut = getCurrentWritingNode(currentSerial);
-        ASSERT((*nodeOut)->getInsideRenderPassCommands()->valid());
-        return gl::NoError();
+        *nodeOut = getCurrentWritingNode();
+    }
+    else
+    {
+        *nodeOut = getNewWritingNode(renderer);
     }
 
-    vk::CommandGraphNode *node = getNewWritingNode(renderer);
+    if ((*nodeOut)->getInsideRenderPassCommands()->valid())
+    {
+        return gl::NoError();
+    }
 
     vk::Framebuffer *framebuffer = nullptr;
     ANGLE_TRY_RESULT(getFramebuffer(context, renderer), framebuffer);
@@ -553,8 +553,15 @@ gl::Error FramebufferVk::getCommandGraphNodeForDraw(const gl::Context *context,
     std::vector<VkClearValue> attachmentClearValues;
 
     vk::CommandBuffer *commandBuffer = nullptr;
-    ANGLE_TRY(node->beginOutsideRenderPassRecording(renderer->getDevice(),
-                                                    renderer->getCommandPool(), &commandBuffer));
+    if (!(*nodeOut)->getOutsideRenderPassCommands()->valid())
+    {
+        ANGLE_TRY((*nodeOut)->beginOutsideRenderPassRecording(
+            renderer->getDevice(), renderer->getCommandPool(), &commandBuffer));
+    }
+    else
+    {
+        commandBuffer = (*nodeOut)->getOutsideRenderPassCommands();
+    }
 
     // Initialize RenderPass info.
     // TODO(jmadill): Support gaps in RenderTargets. http://anglebug.com/2394
@@ -569,7 +576,7 @@ gl::Error FramebufferVk::getCommandGraphNodeForDraw(const gl::Context *context,
             VK_IMAGE_ASPECT_COLOR_BIT, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
             VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
             commandBuffer);
-        node->appendColorRenderTarget(currentSerial, colorRenderTarget);
+        (*nodeOut)->appendColorRenderTarget(currentSerial, colorRenderTarget);
         attachmentClearValues.emplace_back(contextVk->getClearColorValue());
     }
 
@@ -585,17 +592,15 @@ gl::Error FramebufferVk::getCommandGraphNodeForDraw(const gl::Context *context,
             aspectFlags, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL,
             VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT,
             commandBuffer);
-        node->appendDepthStencilRenderTarget(currentSerial, depthStencilRenderTarget);
+        (*nodeOut)->appendDepthStencilRenderTarget(currentSerial, depthStencilRenderTarget);
         attachmentClearValues.emplace_back(contextVk->getClearDepthStencilValue());
     }
 
     // Hard-code RenderPass to clear the first render target to the current clear value.
     // TODO(jmadill): Proper clear value implementation. http://anglebug.com/2361
     const gl::State &glState = context->getGLState();
-    node->storeRenderPassInfo(*framebuffer, glState.getViewport(), attachmentClearValues);
-    mLastRenderNodeSerial = currentSerial;
+    (*nodeOut)->storeRenderPassInfo(*framebuffer, glState.getViewport(), attachmentClearValues);
 
-    *nodeOut = node;
     return gl::NoError();
 }
 

src/libANGLE/renderer/vulkan/FramebufferVk.h

@@ -106,7 +106,6 @@ class FramebufferVk : public FramebufferImpl, public vk::CommandGraphResource
 
     Optional<vk::RenderPassDesc> mRenderPassDesc;
     vk::Framebuffer mFramebuffer;
-    Serial mLastRenderNodeSerial;
     RenderTargetCache<RenderTargetVk> mRenderTargetCache;
 };
 

src/libANGLE/renderer/vulkan/VertexArrayVk.cpp

@@ -255,7 +255,7 @@ const gl::AttribArray<VkDeviceSize> &VertexArrayVk::getCurrentArrayBufferOffsets
     return mCurrentArrayBufferOffsets;
 }
 
-void VertexArrayVk::updateArrayBufferReadDependencies(vk::CommandGraphNode *drawNode,
+void VertexArrayVk::updateArrayBufferReadDependencies(vk::CommandGraphNode *readingNode,
                                                       const gl::AttributesMask &activeAttribsMask,
                                                       Serial serial)
 {
@@ -263,17 +263,17 @@ void VertexArrayVk::updateArrayBufferReadDependencies(vk::CommandGraphNode *draw
     for (size_t attribIndex : activeAttribsMask)
     {
         if (mCurrentArrayBufferResources[attribIndex])
-            mCurrentArrayBufferResources[attribIndex]->onReadResource(drawNode, serial);
+            mCurrentArrayBufferResources[attribIndex]->onReadResource(readingNode, serial);
     }
 }
 
-void VertexArrayVk::updateElementArrayBufferReadDependency(vk::CommandGraphNode *drawNode,
+void VertexArrayVk::updateElementArrayBufferReadDependency(vk::CommandGraphNode *readingNode,
                                                            Serial serial)
 {
     // Handle the bound element array buffer.
     if (mCurrentElementArrayBufferResource)
     {
-        mCurrentElementArrayBufferResource->onReadResource(drawNode, serial);
+        mCurrentElementArrayBufferResource->onReadResource(readingNode, serial);
     }
 }
 

src/libANGLE/renderer/vulkan/VertexArrayVk.h

@@ -45,7 +45,7 @@ class VertexArrayVk : public VertexArrayImpl
     const gl::AttribArray<VkBuffer> &getCurrentArrayBufferHandles() const;
     const gl::AttribArray<VkDeviceSize> &getCurrentArrayBufferOffsets() const;
 
-    void updateDrawDependencies(vk::CommandGraphNode *readNode,
+    void updateDrawDependencies(vk::CommandGraphNode *readingNode,
                                 const gl::AttributesMask &activeAttribsMask,
                                 vk::CommandGraphResource *elementArrayBufferOverride,
                                 Serial serial,

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -365,7 +365,7 @@ gl::Error LineLoopHandler::createIndexBufferFromElementArrayBuffer(RendererVk *r
     beginWriteResource(renderer, &commandBuffer);
 
     Serial currentSerial = renderer->getCurrentQueueSerial();
-    elementArrayBufferVk->onReadResource(getCurrentWritingNode(currentSerial), currentSerial);
+    elementArrayBufferVk->onReadResource(getCurrentWritingNode(), currentSerial);
     commandBuffer->copyBuffer(elementArrayBufferVk->getVkBuffer().getHandle(), *bufferHandleOut, 2,
                               copies.data());
 

src/tests/gl_tests/SimpleOperationTest.cpp

@@ -523,7 +523,9 @@ TEST_P(SimpleOperationTest, DrawIndexedQuadAndSwap)
 {
     ANGLE_GL_PROGRAM(program, kBasicVertexShader, kGreenFragmentShader);
 
-    for (int i = 0; i < 8; ++i)
+    // 32 iterations is an arbitrary number. The more iterations, the more flaky syncronization
+    // issues will reproduce consistently.
+    for (int i = 0; i < 32; ++i)
     {
         drawIndexedQuad(program.get(), "position", 0.5f, 1.0f, true);
         ASSERT_GL_NO_ERROR();