Commit f6659b3d by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Fix invalidate + deferred clear

Imagine the following scenario: 1. Clear draw framebuffer 2. Invalidate draw framebuffer 3. Copy from read framebuffer into texture that is attached to draw framebuffer 4. Draw again into draw framebuffer At step 2, FramebufferVk::syncState is called which extracts the clear and stores it in mDeferredClears, but since the framebuffer description hasn't changed, a new render pass is not started. At step 3, the texture attached to the draw framebuffer is used as copy destination, but its clear which is stored in the (apparently irrelevant) draw framebuffer is not flushed. At step 4, a new render pass is started which clears the draw framebuffer, trampling the copy in step 3. This change makes sure invalidate results in a flushDeferredClears(). With glInvalidateFramebuffer() (not glInvalidateSubFramebuffer()), the deferred clears for invalidated framebuffers are discarded first. It is unknown whether there are similar situations where syncState gathers deferred clears, but they are not flushed before the attachments are updated outside the framebuffer. To catch similar bugs in the future, assertions are added to FramebufferVk::syncState such that if a dirty bit is set for the contents of an attachment, we can make sure there are no prior deferred clears stored for that attachment. Bug: angleproject:4862 Change-Id: Idb1a08b53e7f011f0fc9a54d478289030b6d77a8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2308034Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarCourtney Goeltzenleuchter <courtneygo@google.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent c757e607
......@@ -238,6 +238,12 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
{
ContextVk *contextVk = vk::GetImpl(context);
// If there are deferred clears, flush them. syncState may have accumulated deferred clears,
// but if the framebuffer's attachments are used after this call not through the framebuffer,
// those clears wouldn't get flushed otherwise (for example as the destination of
// glCopyTex[Sub]Image, shader storage image, etc).
ANGLE_TRY(flushDeferredClears(contextVk, getRotatedCompleteRenderArea(contextVk)));
if (area.encloses(contextVk->getStartedRenderPassCommands().getRenderArea()))
{
ANGLE_TRY(invalidateImpl(contextVk, count, attachments));
......@@ -1243,6 +1249,26 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
}
}
// Remove deferred clears for the invalidated attachments.
if (invalidateDepthBuffer)
{
mDeferredClears.reset(vk::kClearValueDepthIndex);
}
if (invalidateStencilBuffer)
{
mDeferredClears.reset(vk::kClearValueStencilIndex);
}
for (size_t colorIndexGL : mState.getEnabledDrawBuffers())
{
if (invalidateColorBuffers.test(colorIndexGL))
{
mDeferredClears.reset(colorIndexGL);
}
}
// If there are still deferred clears, flush them. See relevant comment in invalidateSub.
ANGLE_TRY(flushDeferredClears(contextVk, getRotatedCompleteRenderArea(contextVk)));
const auto &colorRenderTargets = mRenderTargetCache.getColors();
RenderTargetVk *depthStencilRenderTarget = mRenderTargetCache.getDepthStencil(true);
......@@ -1427,6 +1453,10 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
gl::Rectangle scissoredRenderArea = ClipRectToScissor(context->getState(), renderArea, false);
bool deferClears = binding == GL_DRAW_FRAMEBUFFER && renderArea == scissoredRenderArea;
// If we are notified that any attachment is dirty, but we have deferred clears for them, a
// flushDeferredClears() call is missing somewhere. ASSERT this to catch these bugs.
vk::ClearValuesArray previousDeferredClears = mDeferredClears;
// For any updated attachments we'll update their Serials below
ASSERT(dirtyBits.any());
for (size_t dirtyBit : dirtyBits)
......@@ -1437,6 +1467,8 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
case gl::Framebuffer::DIRTY_BIT_DEPTH_BUFFER_CONTENTS:
case gl::Framebuffer::DIRTY_BIT_STENCIL_ATTACHMENT:
case gl::Framebuffer::DIRTY_BIT_STENCIL_BUFFER_CONTENTS:
ASSERT(!previousDeferredClears.testDepth());
ASSERT(!previousDeferredClears.testStencil());
ANGLE_TRY(updateDepthStencilAttachment(context, deferClears));
break;
case gl::Framebuffer::DIRTY_BIT_READ_BUFFER:
......@@ -1479,6 +1511,8 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
dirtyBit - gl::Framebuffer::DIRTY_BIT_COLOR_BUFFER_CONTENTS_0);
}
ASSERT(!previousDeferredClears.test(colorIndexGL));
ANGLE_TRY(updateColorAttachment(context, deferClears, colorIndexGL));
break;
}
......
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