Commit 68791f89 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Fix sub invalidate marking render targets undefined

When glInvalidateSubFramebuffer is called, the framebuffer is only partially invalidated. FramebufferVk::invalidateImpl was nevertheless marking the render targets as undefined, which would lead the subsequent render pass have loadOp=DONT_CARE. This is not correct, as the rest of the framebuffer is expected to still be valid. Bug: angleproject:4859 Change-Id: I2e64baa32b1fc84beb8008411b564cd7619af962 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2309111 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com>
parent d1afcd96
...@@ -2409,7 +2409,7 @@ void ContextVk::optimizeRenderPassForPresent(VkFramebuffer framebufferHandle) ...@@ -2409,7 +2409,7 @@ void ContextVk::optimizeRenderPassForPresent(VkFramebuffer framebufferHandle)
mRenderPassCommands->invalidateRenderPassStencilAttachment(depthStencilAttachmentIndexVk); mRenderPassCommands->invalidateRenderPassStencilAttachment(depthStencilAttachmentIndexVk);
mRenderPassCommands->invalidateRenderPassDepthAttachment(depthStencilAttachmentIndexVk); mRenderPassCommands->invalidateRenderPassDepthAttachment(depthStencilAttachmentIndexVk);
// Mark content as invalid so that we will not load them in next renderpass // Mark content as invalid so that we will not load them in next renderpass
depthStencilRenderTarget->invalidateContent(); depthStencilRenderTarget->invalidateEntireContent();
} }
// Use finalLayout instead of extra barrier for layout change to present // Use finalLayout instead of extra barrier for layout change to present
......
...@@ -227,7 +227,7 @@ angle::Result FramebufferVk::invalidate(const gl::Context *context, ...@@ -227,7 +227,7 @@ angle::Result FramebufferVk::invalidate(const gl::Context *context,
{ {
ContextVk *contextVk = vk::GetImpl(context); ContextVk *contextVk = vk::GetImpl(context);
ANGLE_TRY(invalidateImpl(contextVk, count, attachments)); ANGLE_TRY(invalidateImpl(contextVk, count, attachments, false));
return angle::Result::Continue; return angle::Result::Continue;
} }
...@@ -246,7 +246,7 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context, ...@@ -246,7 +246,7 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
if (area.encloses(contextVk->getStartedRenderPassCommands().getRenderArea())) if (area.encloses(contextVk->getStartedRenderPassCommands().getRenderArea()))
{ {
ANGLE_TRY(invalidateImpl(contextVk, count, attachments)); ANGLE_TRY(invalidateImpl(contextVk, count, attachments, true));
} }
return angle::Result::Continue; return angle::Result::Continue;
...@@ -1215,7 +1215,8 @@ bool FramebufferVk::checkStatus(const gl::Context *context) const ...@@ -1215,7 +1215,8 @@ bool FramebufferVk::checkStatus(const gl::Context *context) const
angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk, angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
size_t count, size_t count,
const GLenum *attachments) const GLenum *attachments,
bool isSubInvalidate)
{ {
gl::DrawBufferMask invalidateColorBuffers; gl::DrawBufferMask invalidateColorBuffers;
bool invalidateDepthBuffer = false; bool invalidateDepthBuffer = false;
...@@ -1249,6 +1250,9 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk, ...@@ -1249,6 +1250,9 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
} }
} }
// Shouldn't try to issue deferred clears if invalidating sub framebuffer.
ASSERT(mDeferredClears.empty() || !isSubInvalidate);
// Remove deferred clears for the invalidated attachments. // Remove deferred clears for the invalidated attachments.
if (invalidateDepthBuffer) if (invalidateDepthBuffer)
{ {
...@@ -1327,21 +1331,26 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk, ...@@ -1327,21 +1331,26 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
ANGLE_TRY(contextVk->endRenderPass()); ANGLE_TRY(contextVk->endRenderPass());
} }
for (size_t colorIndexGL : mState.getEnabledDrawBuffers()) // If not a partial invalidate, mark the contents of the invalidated attachments as undefined,
// so their loadOp can be set to DONT_CARE in the following render pass.
if (!isSubInvalidate)
{ {
if (invalidateColorBuffers.test(colorIndexGL)) for (size_t colorIndexGL : mState.getEnabledDrawBuffers())
{ {
RenderTargetVk *colorRenderTarget = colorRenderTargets[colorIndexGL]; if (invalidateColorBuffers.test(colorIndexGL))
ASSERT(colorRenderTarget); {
colorRenderTarget->invalidateContent(); RenderTargetVk *colorRenderTarget = colorRenderTargets[colorIndexGL];
ASSERT(colorRenderTarget);
colorRenderTarget->invalidateEntireContent();
}
} }
}
// If we have a depth / stencil render target AND we invalidate both we'll mark it as // If we have a depth / stencil render target AND we invalidate both we'll mark it as
// invalid. Maybe in the future add separate depth & stencil invalid flags. // invalid. Maybe in the future add separate depth & stencil invalid flags.
if (depthStencilRenderTarget && invalidateDepthBuffer && invalidateStencilBuffer) if (depthStencilRenderTarget && invalidateDepthBuffer && invalidateStencilBuffer)
{ {
depthStencilRenderTarget->invalidateContent(); depthStencilRenderTarget->invalidateEntireContent();
}
} }
return angle::Result::Continue; return angle::Result::Continue;
......
...@@ -196,7 +196,10 @@ class FramebufferVk : public FramebufferImpl ...@@ -196,7 +196,10 @@ class FramebufferVk : public FramebufferImpl
angle::Result updateColorAttachment(const gl::Context *context, angle::Result updateColorAttachment(const gl::Context *context,
bool deferClears, bool deferClears,
uint32_t colorIndex); uint32_t colorIndex);
angle::Result invalidateImpl(ContextVk *contextVk, size_t count, const GLenum *attachments); angle::Result invalidateImpl(ContextVk *contextVk,
size_t count,
const GLenum *attachments,
bool isSubInvalidate);
// Release all FramebufferVk objects in the cache and clear cache // Release all FramebufferVk objects in the cache and clear cache
void clearCache(ContextVk *contextVk); void clearCache(ContextVk *contextVk);
angle::Result updateDepthStencilAttachment(const gl::Context *context, bool deferClears); angle::Result updateDepthStencilAttachment(const gl::Context *context, bool deferClears);
......
...@@ -86,9 +86,9 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget ...@@ -86,9 +86,9 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
void retainImageViews(ContextVk *contextVk) const; void retainImageViews(ContextVk *contextVk) const;
bool hasDefinedContent() const { return mContentDefined; } bool hasDefinedContent() const { return mContentDefined; }
// mark content as undefined so that certain optimizations are possible such as using DONT_CARE // Mark content as undefined so that certain optimizations are possible such as using DONT_CARE
// as loadOp of the render target in the next renderpass. // as loadOp of the render target in the next renderpass.
void invalidateContent() { mContentDefined = false; } void invalidateEntireContent() { mContentDefined = false; }
private: private:
vk::ImageHelper *mImage; vk::ImageHelper *mImage;
......
...@@ -2803,6 +2803,47 @@ void main() ...@@ -2803,6 +2803,47 @@ void main()
blendAndVerifyColor(GLColor32F(1.0f, 0.0f, 0.0f, 0.5f), GLColor(127, 127, 127, 191)); blendAndVerifyColor(GLColor32F(1.0f, 0.0f, 0.0f, 0.5f), GLColor(127, 127, 127, 191));
} }
// Tests that sub-invalidate then draw works.
TEST_P(SimpleStateChangeTestES3, SubInvalidateThenDraw)
{
// Fails on AMD OpenGL Windows. This configuration isn't maintained.
ANGLE_SKIP_TEST_IF(IsWindows() && IsAMD() && IsOpenGL());
// Create the framebuffer that will be invalidated
GLTexture renderTarget;
glBindTexture(GL_TEXTURE_2D, renderTarget);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
GLFramebuffer drawFBO;
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, drawFBO);
glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, renderTarget,
0);
ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_DRAW_FRAMEBUFFER);
EXPECT_GL_NO_ERROR();
// Clear the framebuffer.
glClearColor(1.0f, 1.0f, 0.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
// Draw into a quarter of the framebuffer, then invalidate that same region.
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
glEnable(GL_SCISSOR_TEST);
glScissor(1, 1, 1, 1);
drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
// Only invalidate a quarter of the framebuffer.
GLenum invalidateAttachment = GL_COLOR_ATTACHMENT0;
glInvalidateSubFramebuffer(GL_DRAW_FRAMEBUFFER, 1, &invalidateAttachment, 1, 1, 1, 1);
EXPECT_GL_NO_ERROR();
glDisable(GL_SCISSOR_TEST);
// Blend into the framebuffer, then verify that the framebuffer should have had cyan.
glBindFramebuffer(GL_READ_FRAMEBUFFER, drawFBO);
blendAndVerifyColor(GLColor32F(0.0f, 0.0f, 1.0f, 0.5f), GLColor(127, 127, 127, 191));
}
// 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