Commit 243d0f89 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Avoid content restore by detecting no-op stencil

Previously, as long as stencil was enabled, it was considered that it is also being modified. This caused stencil invalidate to be undone in a number of situations, such as: - glEnable(GL_STENCIL_TEST); // with func/ops default - glDrawArrays(); - glInvalidateFramebuffer([GL_STENCIL_ATTACHMENT]); - glClear(GL_DEPTH_BUFFER_BIT); - Close render pass In the above scenario, invalidation of stencil was undone at the end of render pass. In this change, the following cases are considered read-only stencil: - Func = GL_NEVER, stencilFail = GL_KEEP - Func = GL_ALWAYS, stencilPassDepth* = GL_KEEP - stencilFail = GL_KEEP, stencilPassDepth* = GL_KEEP Note that while the above scenario is fixed for no-op stencil, a similar issue persists if stencil was not no-op. The reason stencil invalidate is undone in that case is due to the fact that it's assumed any command after the invalidate call will be a draw call that outputs to stencil, but that is not the case with the glClear call in this example. Bug: angleproject:4836 Change-Id: Ie2ea2d52b7c8ee2394f5456773a7ef434e2b2b16 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2461465 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarIan Elliott <ianelliott@google.com>
parent 6c1c3bd9
...@@ -14,6 +14,23 @@ ...@@ -14,6 +14,23 @@
namespace gl namespace gl
{ {
namespace
{
bool IsStencilNoOp(GLenum stencilFunc,
GLenum stencilFail,
GLenum stencilPassDepthFail,
GLenum stencilPassDepthPass)
{
const bool isNeverAndKeep = stencilFunc == GL_NEVER && stencilFail == GL_KEEP;
const bool isAlwaysAndKeepOrAllKeep = (stencilFunc == GL_ALWAYS || stencilFail == GL_KEEP) &&
stencilPassDepthFail == GL_KEEP &&
stencilPassDepthPass == GL_KEEP;
return isNeverAndKeep || isAlwaysAndKeepOrAllKeep;
}
} // anonymous namespace
RasterizerState::RasterizerState() RasterizerState::RasterizerState()
{ {
memset(this, 0, sizeof(RasterizerState)); memset(this, 0, sizeof(RasterizerState));
...@@ -114,6 +131,20 @@ bool DepthStencilState::isStencilMaskedOut() const ...@@ -114,6 +131,20 @@ bool DepthStencilState::isStencilMaskedOut() const
return (stencilMask & stencilWritemask) == 0; return (stencilMask & stencilWritemask) == 0;
} }
bool DepthStencilState::isStencilNoOp() const
{
return isStencilMaskedOut() ||
IsStencilNoOp(stencilFunc, stencilFail, stencilPassDepthFail, stencilPassDepthPass);
}
bool DepthStencilState::isStencilBackNoOp() const
{
const bool isStencilBackMaskedOut = (stencilBackMask & stencilBackWritemask) == 0;
return isStencilBackMaskedOut ||
IsStencilNoOp(stencilBackFunc, stencilBackFail, stencilBackPassDepthFail,
stencilBackPassDepthPass);
}
bool operator==(const DepthStencilState &a, const DepthStencilState &b) bool operator==(const DepthStencilState &a, const DepthStencilState &b)
{ {
return memcmp(&a, &b, sizeof(DepthStencilState)) == 0; return memcmp(&a, &b, sizeof(DepthStencilState)) == 0;
......
...@@ -199,6 +199,8 @@ struct DepthStencilState final ...@@ -199,6 +199,8 @@ struct DepthStencilState final
bool isDepthMaskedOut() const; bool isDepthMaskedOut() const;
bool isStencilMaskedOut() const; bool isStencilMaskedOut() const;
bool isStencilNoOp() const;
bool isStencilBackNoOp() const;
bool depthTest; bool depthTest;
GLenum depthFunc; GLenum depthFunc;
......
...@@ -300,7 +300,7 @@ vk::ResourceAccess GetDepthAccess(const gl::DepthStencilState &dsState) ...@@ -300,7 +300,7 @@ vk::ResourceAccess GetDepthAccess(const gl::DepthStencilState &dsState)
{ {
return vk::ResourceAccess::Unused; return vk::ResourceAccess::Unused;
} }
return dsState.depthMask ? vk::ResourceAccess::Write : vk::ResourceAccess::ReadOnly; return dsState.isDepthMaskedOut() ? vk::ResourceAccess::ReadOnly : vk::ResourceAccess::Write;
} }
vk::ResourceAccess GetStencilAccess(const gl::DepthStencilState &dsState) vk::ResourceAccess GetStencilAccess(const gl::DepthStencilState &dsState)
...@@ -309,8 +309,9 @@ vk::ResourceAccess GetStencilAccess(const gl::DepthStencilState &dsState) ...@@ -309,8 +309,9 @@ vk::ResourceAccess GetStencilAccess(const gl::DepthStencilState &dsState)
{ {
return vk::ResourceAccess::Unused; return vk::ResourceAccess::Unused;
} }
// Simplify this check by returning write instead of checking the mask.
return vk::ResourceAccess::Write; return dsState.isStencilNoOp() && dsState.isStencilBackNoOp() ? vk::ResourceAccess::ReadOnly
: vk::ResourceAccess::Write;
} }
} // anonymous namespace } // anonymous namespace
......
...@@ -1034,9 +1034,10 @@ class CommandBufferHelper : angle::NonCopyable ...@@ -1034,9 +1034,10 @@ class CommandBufferHelper : angle::NonCopyable
// Keep track of the size of commands in the command buffer. If the size grows in the // Keep track of the size of commands in the command buffer. If the size grows in the
// future, that implies that drawing occured since invalidated. // future, that implies that drawing occured since invalidated.
mDepthCmdSizeInvalidated = mCommandBuffer.getCommandSize(); mDepthCmdSizeInvalidated = mCommandBuffer.getCommandSize();
// Also track the size if the attachment is currently disabled. // Also track the size if the attachment is currently disabled.
mDepthCmdSizeDisabled = const bool isDepthWriteEnabled = dsState.depthTest && dsState.depthMask;
(dsState.depthTest && dsState.depthMask) ? kInfiniteCmdSize : mDepthCmdSizeInvalidated; mDepthCmdSizeDisabled = isDepthWriteEnabled ? kInfiniteCmdSize : mDepthCmdSizeInvalidated;
} }
void invalidateRenderPassStencilAttachment(const gl::DepthStencilState &dsState) void invalidateRenderPassStencilAttachment(const gl::DepthStencilState &dsState)
...@@ -1045,9 +1046,12 @@ class CommandBufferHelper : angle::NonCopyable ...@@ -1045,9 +1046,12 @@ class CommandBufferHelper : angle::NonCopyable
// Keep track of the size of commands in the command buffer. If the size grows in the // Keep track of the size of commands in the command buffer. If the size grows in the
// future, that implies that drawing occured since invalidated. // future, that implies that drawing occured since invalidated.
mStencilCmdSizeInvalidated = mCommandBuffer.getCommandSize(); mStencilCmdSizeInvalidated = mCommandBuffer.getCommandSize();
// Also track the size if the attachment is currently disabled. // Also track the size if the attachment is currently disabled.
const bool isStencilWriteEnabled =
dsState.stencilTest && (!dsState.isStencilNoOp() || !dsState.isStencilBackNoOp());
mStencilCmdSizeDisabled = mStencilCmdSizeDisabled =
dsState.stencilTest ? kInfiniteCmdSize : mStencilCmdSizeInvalidated; isStencilWriteEnabled ? kInfiniteCmdSize : mStencilCmdSizeInvalidated;
} }
bool hasWriteAfterInvalidate(uint32_t cmdCountInvalidated, uint32_t cmdCountDisabled) bool hasWriteAfterInvalidate(uint32_t cmdCountInvalidated, uint32_t cmdCountDisabled)
......
...@@ -737,8 +737,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDraw) ...@@ -737,8 +737,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDraw)
const rx::vk::PerfCounters &counters = hackANGLE(); const rx::vk::PerfCounters &counters = hackANGLE();
rx::vk::PerfCounters expected; rx::vk::PerfCounters expected;
// Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+1) // Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+0)
setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 1, &expected); setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 0, &expected);
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red()); ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
GLFramebuffer framebuffer; GLFramebuffer framebuffer;
...@@ -765,7 +765,7 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDraw) ...@@ -765,7 +765,7 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDraw)
compareDepthStencilCountersForInvalidateTest(counters, expected); compareDepthStencilCountersForInvalidateTest(counters, expected);
// Start and end another render pass, to check that the load ops are as expected // Start and end another render pass, to check that the load ops are as expected
setAndIncrementLoadCountersForInvalidateTest(counters, 1, 1, &expected); setAndIncrementLoadCountersForInvalidateTest(counters, 1, 0, &expected);
drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f); drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR();
swapBuffers(); swapBuffers();
...@@ -1052,8 +1052,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDrawDisableEnableInvalidateDraw) ...@@ -1052,8 +1052,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDrawDisableEnableInvalidateDraw)
const rx::vk::PerfCounters &counters = hackANGLE(); const rx::vk::PerfCounters &counters = hackANGLE();
rx::vk::PerfCounters expected; rx::vk::PerfCounters expected;
// Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+1) // Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+0)
setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 1, &expected); setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 0, &expected);
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red()); ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
GLFramebuffer framebuffer; GLFramebuffer framebuffer;
...@@ -1099,7 +1099,7 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDrawDisableEnableInvalidateDraw) ...@@ -1099,7 +1099,7 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDrawDisableEnableInvalidateDraw)
compareDepthStencilCountersForInvalidateTest(counters, expected); compareDepthStencilCountersForInvalidateTest(counters, expected);
// Start and end another render pass, to check that the load ops are as expected // Start and end another render pass, to check that the load ops are as expected
setAndIncrementLoadCountersForInvalidateTest(counters, 1, 1, &expected); setAndIncrementLoadCountersForInvalidateTest(counters, 1, 0, &expected);
drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f); drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR();
swapBuffers(); swapBuffers();
...@@ -1166,8 +1166,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateAndClear) ...@@ -1166,8 +1166,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateAndClear)
const rx::vk::PerfCounters &counters = hackANGLE(); const rx::vk::PerfCounters &counters = hackANGLE();
rx::vk::PerfCounters expected; rx::vk::PerfCounters expected;
// Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+1) // Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+0)
setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 1, &expected); setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 0, &expected);
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red()); ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
GLFramebuffer framebuffer; GLFramebuffer framebuffer;
...@@ -1193,8 +1193,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateAndClear) ...@@ -1193,8 +1193,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateAndClear)
swapBuffers(); swapBuffers();
compareDepthStencilCountersForInvalidateTest(counters, expected); compareDepthStencilCountersForInvalidateTest(counters, expected);
// Expect rpCount+1, depth(Clears+0, Loads+1, Stores+1), stencil(Clears+0, Load+1, Stores+1) // Expect rpCount+1, depth(Clears+0, Loads+1, Stores+1), stencil(Clears+0, Load+0, Stores+1)
setExpectedCountersForInvalidateTest(counters, 0, 0, 1, 1, 0, 1, 1, &expected); setExpectedCountersForInvalidateTest(counters, 0, 0, 1, 1, 0, 0, 1, &expected);
// Bind FBO again and try to use the depth buffer without clear. This should result in // Bind FBO again and try to use the depth buffer without clear. This should result in
// loadOp=LOAD and StoreOP=STORE // loadOp=LOAD and StoreOP=STORE
...@@ -1367,8 +1367,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDrawAndDeleteRenderbuffer) ...@@ -1367,8 +1367,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDrawAndDeleteRenderbuffer)
const rx::vk::PerfCounters &counters = hackANGLE(); const rx::vk::PerfCounters &counters = hackANGLE();
rx::vk::PerfCounters expected; rx::vk::PerfCounters expected;
// Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+1) // Expect rpCount+1, depth(Clears+1, Loads+0, Stores+1), stencil(Clears+0, Load+0, Stores+0)
setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 1, &expected); setExpectedCountersForInvalidateTest(counters, 1, 1, 0, 1, 0, 0, 0, &expected);
ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red()); ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
GLFramebuffer framebuffer; GLFramebuffer framebuffer;
......
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