Commit da904484 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Implement glInvalidate[Sub]Framebuffer

Additionally, fixes an issue where the read framebuffer was affecting the render pass desc given to the pipeline. This fix is included with this CL as its test depends on glInvalidateFramebuffer. This issue was revealed by 071d2a44 changing the order in which read and draw framebuffers were synced. Previously, read was synced first, dirtying the pipeline and then draw was synced fixing it. With the order reversed, the read framebuffer is the last to changes the pipeline, leaving it in an invalid state. Bug: angleproject:3201 Bug: angleproject:3202 Change-Id: Ibebf732a3e3cc081e4865f79dcbaedb467fd9038 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1682468Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent a845b599
...@@ -151,6 +151,17 @@ const char *GetLoadOpShorthand(uint32_t loadOp) ...@@ -151,6 +151,17 @@ const char *GetLoadOpShorthand(uint32_t loadOp)
} }
} }
const char *GetStoreOpShorthand(uint32_t storeOp)
{
switch (storeOp)
{
case VK_ATTACHMENT_STORE_OP_STORE:
return "S";
default:
return "D";
}
}
void MakeDebugUtilsLabel(GLenum source, const char *marker, VkDebugUtilsLabelEXT *label) void MakeDebugUtilsLabel(GLenum source, const char *marker, VkDebugUtilsLabelEXT *label)
{ {
static constexpr angle::ColorF kLabelColors[6] = { static constexpr angle::ColorF kLabelColors[6] = {
...@@ -721,13 +732,17 @@ std::string CommandGraphNode::dumpCommandsForDiagnostics(const char *separator) ...@@ -721,13 +732,17 @@ std::string CommandGraphNode::dumpCommandsForDiagnostics(const char *separator)
size_t depthStencilAttachmentCount = mRenderPassDesc.hasDepthStencilAttachment(); size_t depthStencilAttachmentCount = mRenderPassDesc.hasDepthStencilAttachment();
size_t colorAttachmentCount = attachmentCount - depthStencilAttachmentCount; size_t colorAttachmentCount = attachmentCount - depthStencilAttachmentCount;
std::string loadOps, storeOps;
if (colorAttachmentCount > 0) if (colorAttachmentCount > 0)
{ {
result += " Color: "; loadOps += " Color: ";
storeOps += " Color: ";
for (size_t i = 0; i < colorAttachmentCount; ++i) for (size_t i = 0; i < colorAttachmentCount; ++i)
{ {
result += GetLoadOpShorthand(mRenderPassAttachmentOps[i].loadOp); loadOps += GetLoadOpShorthand(mRenderPassAttachmentOps[i].loadOp);
storeOps += GetStoreOpShorthand(mRenderPassAttachmentOps[i].storeOp);
} }
} }
...@@ -735,11 +750,22 @@ std::string CommandGraphNode::dumpCommandsForDiagnostics(const char *separator) ...@@ -735,11 +750,22 @@ std::string CommandGraphNode::dumpCommandsForDiagnostics(const char *separator)
{ {
ASSERT(depthStencilAttachmentCount == 1); ASSERT(depthStencilAttachmentCount == 1);
result += " Depth/Stencil: "; loadOps += " Depth/Stencil: ";
storeOps += " Depth/Stencil: ";
size_t dsIndex = colorAttachmentCount; size_t dsIndex = colorAttachmentCount;
result += GetLoadOpShorthand(mRenderPassAttachmentOps[dsIndex].loadOp); loadOps += GetLoadOpShorthand(mRenderPassAttachmentOps[dsIndex].loadOp);
result += GetLoadOpShorthand(mRenderPassAttachmentOps[dsIndex].stencilLoadOp); loadOps += GetLoadOpShorthand(mRenderPassAttachmentOps[dsIndex].stencilLoadOp);
storeOps += GetStoreOpShorthand(mRenderPassAttachmentOps[dsIndex].storeOp);
storeOps += GetStoreOpShorthand(mRenderPassAttachmentOps[dsIndex].stencilStoreOp);
}
if (attachmentCount > 0)
{
result += " LoadOp: " + loadOps;
result += separator;
result += "------------ StoreOp: " + storeOps;
} }
result += DumpCommands(mInsideRenderPassCommands, separator); result += DumpCommands(mInsideRenderPassCommands, separator);
......
...@@ -126,6 +126,21 @@ class CommandGraphNode final : angle::NonCopyable ...@@ -126,6 +126,21 @@ class CommandGraphNode final : angle::NonCopyable
mRenderPassClearValues[attachmentIndex].depthStencil.stencil = stencil; mRenderPassClearValues[attachmentIndex].depthStencil.stencil = stencil;
} }
void invalidateRenderPassColorAttachment(size_t attachmentIndex)
{
mRenderPassAttachmentOps[attachmentIndex].storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
}
void invalidateRenderPassDepthAttachment(size_t attachmentIndex)
{
mRenderPassAttachmentOps[attachmentIndex].storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
}
void invalidateRenderPassStencilAttachment(size_t attachmentIndex)
{
mRenderPassAttachmentOps[attachmentIndex].stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
}
// Dependency commands order node execution in the command graph. // Dependency commands order node execution in the command graph.
// Once a node has commands that must happen after it, recording is stopped and the node is // Once a node has commands that must happen after it, recording is stopped and the node is
// frozen forever. // frozen forever.
...@@ -356,6 +371,24 @@ class CommandGraphResource : angle::NonCopyable ...@@ -356,6 +371,24 @@ class CommandGraphResource : angle::NonCopyable
mCurrentWritingNode->clearRenderPassStencilAttachment(attachmentIndex, stencil); mCurrentWritingNode->clearRenderPassStencilAttachment(attachmentIndex, stencil);
} }
void invalidateRenderPassColorAttachment(size_t attachmentIndex)
{
ASSERT(hasStartedRenderPass());
mCurrentWritingNode->invalidateRenderPassColorAttachment(attachmentIndex);
}
void invalidateRenderPassDepthAttachment(size_t attachmentIndex)
{
ASSERT(hasStartedRenderPass());
mCurrentWritingNode->invalidateRenderPassDepthAttachment(attachmentIndex);
}
void invalidateRenderPassStencilAttachment(size_t attachmentIndex)
{
ASSERT(hasStartedRenderPass());
mCurrentWritingNode->invalidateRenderPassStencilAttachment(attachmentIndex);
}
// Accessor for RenderPass RenderArea. // Accessor for RenderPass RenderArea.
const gl::Rectangle &getRenderPassRenderArea() const const gl::Rectangle &getRenderPassRenderArea() const
{ {
......
...@@ -1896,8 +1896,10 @@ void ContextVk::invalidateDriverUniforms() ...@@ -1896,8 +1896,10 @@ void ContextVk::invalidateDriverUniforms()
mDirtyBits.set(DIRTY_BIT_DESCRIPTOR_SETS); mDirtyBits.set(DIRTY_BIT_DESCRIPTOR_SETS);
} }
void ContextVk::onFramebufferChange(const vk::RenderPassDesc &renderPassDesc) void ContextVk::onDrawFramebufferChange(FramebufferVk *framebufferVk)
{ {
const vk::RenderPassDesc &renderPassDesc = framebufferVk->getRenderPassDesc();
// Ensure that the RenderPass description is updated. // Ensure that the RenderPass description is updated.
invalidateCurrentPipeline(); invalidateCurrentPipeline();
mGraphicsPipelineDesc->updateRenderPassDesc(&mGraphicsPipelineTransition, renderPassDesc); mGraphicsPipelineDesc->updateRenderPassDesc(&mGraphicsPipelineTransition, renderPassDesc);
......
...@@ -212,7 +212,7 @@ class ContextVk : public ContextImpl, public vk::Context, public vk::CommandBuff ...@@ -212,7 +212,7 @@ class ContextVk : public ContextImpl, public vk::Context, public vk::CommandBuff
void invalidateDefaultAttribute(size_t attribIndex); void invalidateDefaultAttribute(size_t attribIndex);
void invalidateDefaultAttributes(const gl::AttributesMask &dirtyMask); void invalidateDefaultAttributes(const gl::AttributesMask &dirtyMask);
void onFramebufferChange(const vk::RenderPassDesc &renderPassDesc); void onDrawFramebufferChange(FramebufferVk *framebufferVk);
void onHostVisibleBufferWrite() { mIsAnyHostVisibleBufferWritten = true; } void onHostVisibleBufferWrite() { mIsAnyHostVisibleBufferWritten = true; }
void invalidateCurrentTransformFeedbackBuffers(); void invalidateCurrentTransformFeedbackBuffers();
......
...@@ -190,8 +190,14 @@ angle::Result FramebufferVk::invalidate(const gl::Context *context, ...@@ -190,8 +190,14 @@ angle::Result FramebufferVk::invalidate(const gl::Context *context,
size_t count, size_t count,
const GLenum *attachments) const GLenum *attachments)
{ {
ANGLE_VK_UNREACHABLE(vk::GetImpl(context)); mFramebuffer.updateQueueSerial(vk::GetImpl(context)->getCurrentQueueSerial());
return angle::Result::Stop;
if (mFramebuffer.valid() && mFramebuffer.hasStartedRenderPass())
{
invalidateImpl(vk::GetImpl(context), count, attachments);
}
return angle::Result::Continue;
} }
angle::Result FramebufferVk::invalidateSub(const gl::Context *context, angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
...@@ -199,8 +205,17 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context, ...@@ -199,8 +205,17 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
const GLenum *attachments, const GLenum *attachments,
const gl::Rectangle &area) const gl::Rectangle &area)
{ {
ANGLE_VK_UNREACHABLE(vk::GetImpl(context)); mFramebuffer.updateQueueSerial(vk::GetImpl(context)->getCurrentQueueSerial());
return angle::Result::Stop;
// RenderPass' storeOp cannot be made conditional to a specific region, so we only apply this
// hint if the requested area encompasses the render area.
if (mFramebuffer.valid() && mFramebuffer.hasStartedRenderPass() &&
area.encloses(mFramebuffer.getRenderPassRenderArea()))
{
invalidateImpl(vk::GetImpl(context), count, attachments);
}
return angle::Result::Continue;
} }
angle::Result FramebufferVk::clear(const gl::Context *context, GLbitfield mask) angle::Result FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
...@@ -963,6 +978,83 @@ angle::Result FramebufferVk::updateColorAttachment(const gl::Context *context, s ...@@ -963,6 +978,83 @@ angle::Result FramebufferVk::updateColorAttachment(const gl::Context *context, s
return angle::Result::Continue; return angle::Result::Continue;
} }
void FramebufferVk::invalidateImpl(ContextVk *contextVk, size_t count, const GLenum *attachments)
{
ASSERT(mFramebuffer.hasStartedRenderPass());
gl::DrawBufferMask invalidateColorBuffers;
bool invalidateDepthBuffer = false;
bool invalidateStencilBuffer = false;
for (size_t i = 0; i < count; ++i)
{
const GLenum attachment = attachments[i];
switch (attachment)
{
case GL_DEPTH:
case GL_DEPTH_ATTACHMENT:
invalidateDepthBuffer = true;
break;
case GL_STENCIL:
case GL_STENCIL_ATTACHMENT:
invalidateStencilBuffer = true;
break;
case GL_DEPTH_STENCIL_ATTACHMENT:
invalidateDepthBuffer = true;
invalidateStencilBuffer = true;
break;
default:
ASSERT(
(attachment >= GL_COLOR_ATTACHMENT0 && attachment <= GL_COLOR_ATTACHMENT15) ||
(attachment == GL_COLOR));
invalidateColorBuffers.set(
attachment == GL_COLOR ? 0u : (attachment - GL_COLOR_ATTACHMENT0));
}
}
// Set the appropriate storeOp for attachments.
size_t attachmentIndexVk = 0;
for (size_t colorIndexGL : mState.getEnabledDrawBuffers())
{
if (invalidateColorBuffers.test(colorIndexGL))
{
mFramebuffer.invalidateRenderPassColorAttachment(attachmentIndexVk);
}
++attachmentIndexVk;
}
RenderTargetVk *depthStencilRenderTarget = mRenderTargetCache.getDepthStencil();
if (depthStencilRenderTarget)
{
if (invalidateDepthBuffer)
{
mFramebuffer.invalidateRenderPassDepthAttachment(attachmentIndexVk);
}
if (invalidateStencilBuffer)
{
mFramebuffer.invalidateRenderPassStencilAttachment(attachmentIndexVk);
}
}
// NOTE: Possible future optimization is to delay setting the storeOp and only do so if the
// render pass is closed by itself before another draw call. Otherwise, in a situation like
// this:
//
// draw()
// invalidate()
// draw()
//
// We would be discarding the attachments only to load them for the next draw (which is less
// efficient than keeping the render pass open and not do the discard at all). While dEQP tests
// this pattern, this optimization may not be necessary if no application does this. It is
// expected that an application would invalidate() when it's done with the framebuffer, so the
// render pass would have closed either way.
mFramebuffer.finishCurrentCommands(contextVk);
}
angle::Result FramebufferVk::syncState(const gl::Context *context, angle::Result FramebufferVk::syncState(const gl::Context *context,
const gl::Framebuffer::DirtyBits &dirtyBits) const gl::Framebuffer::DirtyBits &dirtyBits)
{ {
...@@ -1023,7 +1115,12 @@ angle::Result FramebufferVk::syncState(const gl::Context *context, ...@@ -1023,7 +1115,12 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
// Notify the ContextVk to update the pipeline desc. // Notify the ContextVk to update the pipeline desc.
updateRenderPassDesc(); updateRenderPassDesc();
contextVk->onFramebufferChange(mRenderPassDesc);
FramebufferVk *currentDrawFramebuffer = vk::GetImpl(context->getState().getDrawFramebuffer());
if (currentDrawFramebuffer == this)
{
contextVk->onDrawFramebufferChange(this);
}
return angle::Result::Continue; return angle::Result::Continue;
} }
......
...@@ -179,6 +179,7 @@ class FramebufferVk : public FramebufferImpl ...@@ -179,6 +179,7 @@ class FramebufferVk : public FramebufferImpl
void updateActiveColorMasks(size_t colorIndex, bool r, bool g, bool b, bool a); void updateActiveColorMasks(size_t colorIndex, bool r, bool g, bool b, bool a);
void updateRenderPassDesc(); void updateRenderPassDesc();
angle::Result updateColorAttachment(const gl::Context *context, size_t colorIndex); angle::Result updateColorAttachment(const gl::Context *context, size_t colorIndex);
void invalidateImpl(ContextVk *contextVk, size_t count, const GLenum *attachments);
WindowSurfaceVk *mBackbuffer; WindowSurfaceVk *mBackbuffer;
......
...@@ -53,6 +53,7 @@ void RendererVk::ensureCapsInitialized() const ...@@ -53,6 +53,7 @@ void RendererVk::ensureCapsInitialized() const
mNativeExtensions.robustness = true; mNativeExtensions.robustness = true;
mNativeExtensions.textureBorderClamp = false; // not implemented yet mNativeExtensions.textureBorderClamp = false; // not implemented yet
mNativeExtensions.translatedShaderSource = true; mNativeExtensions.translatedShaderSource = true;
mNativeExtensions.discardFramebuffer = true;
// Enable EXT_blend_minmax // Enable EXT_blend_minmax
mNativeExtensions.blendMinMax = true; mNativeExtensions.blendMinMax = true;
......
...@@ -703,9 +703,6 @@ ...@@ -703,9 +703,6 @@
// Misc unimplemented: // Misc unimplemented:
// - FramebufferVk::invalidate*:
2950 VULKAN : dEQP-GLES3.functional.fbo.invalidate.* = SKIP
// - Primitive restart with line loops: // - Primitive restart with line loops:
2672 VULKAN : dEQP-GLES3.functional.primitive_restart.*.line_loop.*instanced = SKIP 2672 VULKAN : dEQP-GLES3.functional.primitive_restart.*.line_loop.*instanced = SKIP
......
...@@ -2238,6 +2238,62 @@ TEST_P(SimpleStateChangeTest, CopyTexSubImageOnTextureBoundToFrambuffer) ...@@ -2238,6 +2238,62 @@ TEST_P(SimpleStateChangeTest, CopyTexSubImageOnTextureBoundToFrambuffer)
updateTextureBoundToFramebufferHelper(updateFunc); updateTextureBoundToFramebufferHelper(updateFunc);
} }
// Tests that the read framebuffer doesn't affect what the draw call thinks the attachments are
// (which is what the draw framebuffer dictates) when a command is issued with the GL_FRAMEBUFFER
// target.
TEST_P(SimpleStateChangeTestES3, ReadFramebufferDrawFramebufferDifferentAttachments)
{
GLRenderbuffer drawColorBuffer;
glBindRenderbuffer(GL_RENDERBUFFER, drawColorBuffer);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, 1, 1);
GLRenderbuffer drawDepthBuffer;
glBindRenderbuffer(GL_RENDERBUFFER, drawDepthBuffer);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, 1, 1);
GLRenderbuffer readColorBuffer;
glBindRenderbuffer(GL_RENDERBUFFER, readColorBuffer);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, 1, 1);
GLFramebuffer drawFBO;
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, drawFBO);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER,
drawColorBuffer);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER,
drawDepthBuffer);
ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_DRAW_FRAMEBUFFER);
GLFramebuffer readFBO;
glBindFramebuffer(GL_READ_FRAMEBUFFER, readFBO);
glFramebufferRenderbuffer(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER,
readColorBuffer);
ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_READ_FRAMEBUFFER);
EXPECT_GL_NO_ERROR();
glClearDepthf(1.0f);
glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
// A handful of non-draw calls can sync framebuffer state, such as discard, invalidate,
// invalidateSub and multisamplefv. The trick here is to give GL_FRAMEBUFFER as target, which
// includes both the read and draw framebuffers. The test is to make sure syncing the read
// framebuffer doesn't affect the draw call.
GLenum invalidateAttachment = GL_COLOR_ATTACHMENT0;
glInvalidateFramebuffer(GL_FRAMEBUFFER, 1, &invalidateAttachment);
EXPECT_GL_NO_ERROR();
glEnable(GL_DEPTH_TEST);
glDepthFunc(GL_EQUAL);
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Passthrough(), essl1_shaders::fs::Blue());
drawQuad(program, essl1_shaders::PositionAttrib(), 1.0f);
EXPECT_GL_NO_ERROR();
glBindFramebuffer(GL_READ_FRAMEBUFFER, drawFBO);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::blue);
}
// Tests deleting a Framebuffer that is in use. // Tests deleting a Framebuffer that is in use.
TEST_P(SimpleStateChangeTest, DeleteFramebufferInUse) TEST_P(SimpleStateChangeTest, DeleteFramebufferInUse)
{ {
......
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