Vulkan: Fix swaps done right after a clear.
We were missing a dependency insertion between the Framebuffer and
its attachments, only during clear operations. Also renames a few
methods to make them more consistent.
Bug: angleproject:2264
Change-Id: Ic3af5b34b6de900ea2cc1b765f8d3d69f7f9a131
Reviewed-on: https://chromium-review.googlesource.com/891985
Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/renderer/vulkan/CommandBufferNode.cpp b/src/libANGLE/renderer/vulkan/CommandBufferNode.cpp
index 5dbcc2d..45effd3 100644
--- a/src/libANGLE/renderer/vulkan/CommandBufferNode.cpp
+++ b/src/libANGLE/renderer/vulkan/CommandBufferNode.cpp
@@ -136,7 +136,7 @@
{
// TODO(jmadill): Layout transition?
mRenderPassDesc.packColorAttachment(*colorRenderTarget->format, colorRenderTarget->samples);
- colorRenderTarget->resource->setWriteNode(serial, this);
+ colorRenderTarget->resource->setWriteNode(this, serial);
}
void CommandBufferNode::appendDepthStencilRenderTarget(Serial serial,
@@ -145,7 +145,7 @@
// TODO(jmadill): Layout transition?
mRenderPassDesc.packDepthStencilAttachment(*depthStencilRenderTarget->format,
depthStencilRenderTarget->samples);
- depthStencilRenderTarget->resource->setWriteNode(serial, this);
+ depthStencilRenderTarget->resource->setWriteNode(this, serial);
}
void CommandBufferNode::initAttachmentDesc(VkAttachmentDescription *desc)
diff --git a/src/libANGLE/renderer/vulkan/ContextVk.cpp b/src/libANGLE/renderer/vulkan/ContextVk.cpp
index 7c307b0..67db387 100644
--- a/src/libANGLE/renderer/vulkan/ContextVk.cpp
+++ b/src/libANGLE/renderer/vulkan/ContextVk.cpp
@@ -223,7 +223,7 @@
ASSERT(texture);
TextureVk *textureVk = vk::GetImpl(texture);
- textureVk->updateDependencies(renderNode, mRenderer->getCurrentQueueSerial());
+ textureVk->setReadNode(renderNode, mRenderer->getCurrentQueueSerial());
}
}
diff --git a/src/libANGLE/renderer/vulkan/FramebufferVk.cpp b/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
index e96dfe2..e4c12d2 100644
--- a/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
+++ b/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
@@ -155,6 +155,8 @@
vk::CommandBuffer *commandBuffer = nullptr;
ANGLE_TRY(recordWriteCommands(renderer, &commandBuffer));
+ Serial currentSerial = renderer->getCurrentQueueSerial();
+
for (const auto &colorAttachment : mState.getColorAttachments())
{
if (colorAttachment.isAttached())
@@ -162,6 +164,8 @@
RenderTargetVk *renderTarget = nullptr;
ANGLE_TRY(colorAttachment.getRenderTarget(context, &renderTarget));
+ renderTarget->resource->setWriteNode(getCurrentWriteNode(currentSerial), currentSerial);
+
renderTarget->image->changeLayoutWithStages(
VK_IMAGE_ASPECT_COLOR_BIT, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, commandBuffer);
@@ -170,6 +174,8 @@
}
}
+ // TODO(jmadill): Depth/stencil clear.
+
return gl::NoError();
}
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
index c74f62c..dcefe8b 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
@@ -116,14 +116,14 @@
for (auto attribIndex : activeAttribsMask)
{
ASSERT(mCurrentArrayBufferResources[attribIndex]);
- mCurrentArrayBufferResources[attribIndex]->updateDependencies(readNode, serial);
+ mCurrentArrayBufferResources[attribIndex]->setReadNode(readNode, serial);
}
// Handle the bound element array buffer.
if (drawType == DrawType::Elements)
{
ASSERT(mCurrentElementArrayBufferResource);
- mCurrentElementArrayBufferResource->updateDependencies(readNode, serial);
+ mCurrentElementArrayBufferResource->setReadNode(readNode, serial);
}
}
diff --git a/src/libANGLE/renderer/vulkan/vk_utils.cpp b/src/libANGLE/renderer/vulkan/vk_utils.cpp
index 5ecd0d5..788ffbf 100644
--- a/src/libANGLE/renderer/vulkan/vk_utils.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_utils.cpp
@@ -1313,29 +1313,10 @@
vk::CommandBufferNode *ResourceVk::getNewWriteNode(RendererVk *renderer)
{
vk::CommandBufferNode *newCommands = renderer->allocateCommandNode();
- setWriteNode(renderer->getCurrentQueueSerial(), newCommands);
+ setWriteNode(newCommands, renderer->getCurrentQueueSerial());
return newCommands;
}
-void ResourceVk::setWriteNode(Serial serial, vk::CommandBufferNode *newCommands)
-{
- updateQueueSerial(serial);
-
- // Make sure any open reads and writes finish before we execute |newCommands|.
- if (!mCurrentReadNodes.empty())
- {
- newCommands->addDependencies(mCurrentReadNodes);
- mCurrentReadNodes.clear();
- }
-
- if (mCurrentWriteNode)
- {
- newCommands->addDependency(mCurrentWriteNode);
- }
-
- mCurrentWriteNode = newCommands;
-}
-
vk::Error ResourceVk::recordWriteCommands(RendererVk *renderer,
vk::CommandBuffer **commandBufferOut)
{
@@ -1346,7 +1327,26 @@
return vk::NoError();
}
-void ResourceVk::updateDependencies(vk::CommandBufferNode *readNode, Serial serial)
+void ResourceVk::setWriteNode(vk::CommandBufferNode *writeNode, Serial serial)
+{
+ updateQueueSerial(serial);
+
+ // Make sure any open reads and writes finish before we execute |newCommands|.
+ if (!mCurrentReadNodes.empty())
+ {
+ writeNode->addDependencies(mCurrentReadNodes);
+ mCurrentReadNodes.clear();
+ }
+
+ if (mCurrentWriteNode)
+ {
+ writeNode->addDependency(mCurrentWriteNode);
+ }
+
+ mCurrentWriteNode = writeNode;
+}
+
+void ResourceVk::setReadNode(vk::CommandBufferNode *readNode, Serial serial)
{
if (isCurrentlyRecording(serial))
{
diff --git a/src/libANGLE/renderer/vulkan/vk_utils.h b/src/libANGLE/renderer/vulkan/vk_utils.h
index 539f1b2..8ce2799 100644
--- a/src/libANGLE/renderer/vulkan/vk_utils.h
+++ b/src/libANGLE/renderer/vulkan/vk_utils.h
@@ -687,15 +687,15 @@
// Allocates a new write node and calls setWriteNode internally.
vk::CommandBufferNode *getNewWriteNode(RendererVk *renderer);
- // Called on an operation that will modify this ResourceVk.
- void setWriteNode(Serial serial, vk::CommandBufferNode *newCommands);
-
// Allocates a write node via getNewWriteNode and returns a started command buffer.
// The started command buffer will render outside of a RenderPass.
vk::Error recordWriteCommands(RendererVk *renderer, vk::CommandBuffer **commandBufferOut);
+ // Called on an operation that will modify this ResourceVk.
+ void setWriteNode(vk::CommandBufferNode *newCommands, Serial serial);
+
// Sets up the dependency relations. |readNode| has the commands that read from this object.
- void updateDependencies(vk::CommandBufferNode *readNode, Serial serial);
+ void setReadNode(vk::CommandBufferNode *readNode, Serial serial);
private:
Serial mStoredQueueSerial;
diff --git a/src/tests/gl_tests/SimpleOperationTest.cpp b/src/tests/gl_tests/SimpleOperationTest.cpp
index 3a583d2..19a0f05 100644
--- a/src/tests/gl_tests/SimpleOperationTest.cpp
+++ b/src/tests/gl_tests/SimpleOperationTest.cpp
@@ -85,6 +85,20 @@
EXPECT_GL_NO_ERROR();
}
+// Covers a simple bug in Vulkan to do with dependencies between the Surface and the default
+// Framebuffer.
+TEST_P(SimpleOperationTest, ClearAndSwap)
+{
+ glClearColor(1.0, 0.0, 0.0, 1.0);
+ glClear(GL_COLOR_BUFFER_BIT);
+ swapBuffers();
+
+ // Can't check the pixel result after the swap, and checking the pixel result affects the
+ // behaviour of the test on the Vulkan back-end, so don't bother checking correctness.
+ EXPECT_GL_NO_ERROR();
+ EXPECT_EGL_SUCCESS();
+}
+
TEST_P(SimpleOperationTest, LinkProgram)
{
const std::string vsSource =
