Commit efb5a5c8 by Jamie Madill Committed by Commit Bot

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/891985Reviewed-by: 's avatarYuly Novikov <ynovikov@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent 5dd4ad89
...@@ -136,7 +136,7 @@ void CommandBufferNode::appendColorRenderTarget(Serial serial, RenderTargetVk *c ...@@ -136,7 +136,7 @@ void CommandBufferNode::appendColorRenderTarget(Serial serial, RenderTargetVk *c
{ {
// TODO(jmadill): Layout transition? // TODO(jmadill): Layout transition?
mRenderPassDesc.packColorAttachment(*colorRenderTarget->format, colorRenderTarget->samples); mRenderPassDesc.packColorAttachment(*colorRenderTarget->format, colorRenderTarget->samples);
colorRenderTarget->resource->setWriteNode(serial, this); colorRenderTarget->resource->setWriteNode(this, serial);
} }
void CommandBufferNode::appendDepthStencilRenderTarget(Serial serial, void CommandBufferNode::appendDepthStencilRenderTarget(Serial serial,
...@@ -145,7 +145,7 @@ void CommandBufferNode::appendDepthStencilRenderTarget(Serial serial, ...@@ -145,7 +145,7 @@ void CommandBufferNode::appendDepthStencilRenderTarget(Serial serial,
// TODO(jmadill): Layout transition? // TODO(jmadill): Layout transition?
mRenderPassDesc.packDepthStencilAttachment(*depthStencilRenderTarget->format, mRenderPassDesc.packDepthStencilAttachment(*depthStencilRenderTarget->format,
depthStencilRenderTarget->samples); depthStencilRenderTarget->samples);
depthStencilRenderTarget->resource->setWriteNode(serial, this); depthStencilRenderTarget->resource->setWriteNode(this, serial);
} }
void CommandBufferNode::initAttachmentDesc(VkAttachmentDescription *desc) void CommandBufferNode::initAttachmentDesc(VkAttachmentDescription *desc)
......
...@@ -223,7 +223,7 @@ gl::Error ContextVk::setupDraw(const gl::Context *context, ...@@ -223,7 +223,7 @@ gl::Error ContextVk::setupDraw(const gl::Context *context,
ASSERT(texture); ASSERT(texture);
TextureVk *textureVk = vk::GetImpl(texture); TextureVk *textureVk = vk::GetImpl(texture);
textureVk->updateDependencies(renderNode, mRenderer->getCurrentQueueSerial()); textureVk->setReadNode(renderNode, mRenderer->getCurrentQueueSerial());
} }
} }
......
...@@ -155,6 +155,8 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask) ...@@ -155,6 +155,8 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
vk::CommandBuffer *commandBuffer = nullptr; vk::CommandBuffer *commandBuffer = nullptr;
ANGLE_TRY(recordWriteCommands(renderer, &commandBuffer)); ANGLE_TRY(recordWriteCommands(renderer, &commandBuffer));
Serial currentSerial = renderer->getCurrentQueueSerial();
for (const auto &colorAttachment : mState.getColorAttachments()) for (const auto &colorAttachment : mState.getColorAttachments())
{ {
if (colorAttachment.isAttached()) if (colorAttachment.isAttached())
...@@ -162,6 +164,8 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask) ...@@ -162,6 +164,8 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
RenderTargetVk *renderTarget = nullptr; RenderTargetVk *renderTarget = nullptr;
ANGLE_TRY(colorAttachment.getRenderTarget(context, &renderTarget)); ANGLE_TRY(colorAttachment.getRenderTarget(context, &renderTarget));
renderTarget->resource->setWriteNode(getCurrentWriteNode(currentSerial), currentSerial);
renderTarget->image->changeLayoutWithStages( renderTarget->image->changeLayoutWithStages(
VK_IMAGE_ASPECT_COLOR_BIT, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, VK_IMAGE_ASPECT_COLOR_BIT, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, commandBuffer); VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, commandBuffer);
...@@ -170,6 +174,8 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask) ...@@ -170,6 +174,8 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
} }
} }
// TODO(jmadill): Depth/stencil clear.
return gl::NoError(); return gl::NoError();
} }
......
...@@ -116,14 +116,14 @@ void VertexArrayVk::updateDrawDependencies(vk::CommandBufferNode *readNode, ...@@ -116,14 +116,14 @@ void VertexArrayVk::updateDrawDependencies(vk::CommandBufferNode *readNode,
for (auto attribIndex : activeAttribsMask) for (auto attribIndex : activeAttribsMask)
{ {
ASSERT(mCurrentArrayBufferResources[attribIndex]); ASSERT(mCurrentArrayBufferResources[attribIndex]);
mCurrentArrayBufferResources[attribIndex]->updateDependencies(readNode, serial); mCurrentArrayBufferResources[attribIndex]->setReadNode(readNode, serial);
} }
// Handle the bound element array buffer. // Handle the bound element array buffer.
if (drawType == DrawType::Elements) if (drawType == DrawType::Elements)
{ {
ASSERT(mCurrentElementArrayBufferResource); ASSERT(mCurrentElementArrayBufferResource);
mCurrentElementArrayBufferResource->updateDependencies(readNode, serial); mCurrentElementArrayBufferResource->setReadNode(readNode, serial);
} }
} }
......
...@@ -1313,40 +1313,40 @@ vk::CommandBufferNode *ResourceVk::getCurrentWriteNode(Serial currentSerial) ...@@ -1313,40 +1313,40 @@ vk::CommandBufferNode *ResourceVk::getCurrentWriteNode(Serial currentSerial)
vk::CommandBufferNode *ResourceVk::getNewWriteNode(RendererVk *renderer) vk::CommandBufferNode *ResourceVk::getNewWriteNode(RendererVk *renderer)
{ {
vk::CommandBufferNode *newCommands = renderer->allocateCommandNode(); vk::CommandBufferNode *newCommands = renderer->allocateCommandNode();
setWriteNode(renderer->getCurrentQueueSerial(), newCommands); setWriteNode(newCommands, renderer->getCurrentQueueSerial());
return newCommands; return newCommands;
} }
void ResourceVk::setWriteNode(Serial serial, vk::CommandBufferNode *newCommands) vk::Error ResourceVk::recordWriteCommands(RendererVk *renderer,
vk::CommandBuffer **commandBufferOut)
{
vk::CommandBufferNode *commands = getNewWriteNode(renderer);
VkDevice device = renderer->getDevice();
ANGLE_TRY(commands->startRecording(device, renderer->getCommandPool(), commandBufferOut));
return vk::NoError();
}
void ResourceVk::setWriteNode(vk::CommandBufferNode *writeNode, Serial serial)
{ {
updateQueueSerial(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 |newCommands|.
if (!mCurrentReadNodes.empty()) if (!mCurrentReadNodes.empty())
{ {
newCommands->addDependencies(mCurrentReadNodes); writeNode->addDependencies(mCurrentReadNodes);
mCurrentReadNodes.clear(); mCurrentReadNodes.clear();
} }
if (mCurrentWriteNode) if (mCurrentWriteNode)
{ {
newCommands->addDependency(mCurrentWriteNode); writeNode->addDependency(mCurrentWriteNode);
} }
mCurrentWriteNode = newCommands; mCurrentWriteNode = writeNode;
}
vk::Error ResourceVk::recordWriteCommands(RendererVk *renderer,
vk::CommandBuffer **commandBufferOut)
{
vk::CommandBufferNode *commands = getNewWriteNode(renderer);
VkDevice device = renderer->getDevice();
ANGLE_TRY(commands->startRecording(device, renderer->getCommandPool(), commandBufferOut));
return vk::NoError();
} }
void ResourceVk::updateDependencies(vk::CommandBufferNode *readNode, Serial serial) void ResourceVk::setReadNode(vk::CommandBufferNode *readNode, Serial serial)
{ {
if (isCurrentlyRecording(serial)) if (isCurrentlyRecording(serial))
{ {
......
...@@ -687,15 +687,15 @@ class ResourceVk ...@@ -687,15 +687,15 @@ class ResourceVk
// Allocates a new write node and calls setWriteNode internally. // Allocates a new write node and calls setWriteNode internally.
vk::CommandBufferNode *getNewWriteNode(RendererVk *renderer); 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. // Allocates a write node via getNewWriteNode and returns a started command buffer.
// The started command buffer will render outside of a RenderPass. // The started command buffer will render outside of a RenderPass.
vk::Error recordWriteCommands(RendererVk *renderer, vk::CommandBuffer **commandBufferOut); 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. // 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: private:
Serial mStoredQueueSerial; Serial mStoredQueueSerial;
......
...@@ -85,6 +85,20 @@ TEST_P(SimpleOperationTest, CompileFragmentShader) ...@@ -85,6 +85,20 @@ TEST_P(SimpleOperationTest, CompileFragmentShader)
EXPECT_GL_NO_ERROR(); 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) TEST_P(SimpleOperationTest, LinkProgram)
{ {
const std::string vsSource = const std::string vsSource =
......
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