Commit c5b9c49c by Jamie Madill Committed by Commit Bot

Vulkan: Fix optimizeRenderPassForPresent regression.

gfxbench clears the depth buffer right before the swap. Even though the last draw call that presents the frame didn't ever read or write to depth, the tracking we added thought this depth write meant we had to keep the LOAD_OP as CLEAR. Instead we can refine our check to treat clears specially when enabling the depth-stencil read-only mode instead of changing how the tracking works for clears. This way the tracking can not affect other apps that don't use depth-stencil read-only loops. Also adds a regression test that counts the clears after a swap. Bug: angleproject:4959 Bug: angleproject:4979 Change-Id: I12ece6474019f7519a467f827110ad817f7d4df7 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2370364 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCharlie Lao <cclao@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org>
parent 3c2454b8
...@@ -3940,7 +3940,7 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context) ...@@ -3940,7 +3940,7 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context)
if (hasStartedRenderPass()) if (hasStartedRenderPass())
{ {
if (mRenderPassCommands->getDepthStartAccess() == vk::ResourceAccess::Write) if (mRenderPassCommands->hasDepthWriteOrClear())
{ {
ANGLE_TRY(flushCommandsAndEndRenderPass()); ANGLE_TRY(flushCommandsAndEndRenderPass());
} }
...@@ -4621,7 +4621,7 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass() ...@@ -4621,7 +4621,7 @@ angle::Result ContextVk::flushCommandsAndEndRenderPass()
addOverlayUsedBuffersCount(mRenderPassCommands); addOverlayUsedBuffersCount(mRenderPassCommands);
mRenderPassCommands->endRenderPass(); mRenderPassCommands->endRenderPass(this);
if (mRenderer->getFeatures().enableCommandProcessingThread.enabled) if (mRenderer->getFeatures().enableCommandProcessingThread.enabled)
{ {
......
...@@ -834,21 +834,6 @@ void CommandBufferHelper::beginRenderPass(const Framebuffer &framebuffer, ...@@ -834,21 +834,6 @@ void CommandBufferHelper::beginRenderPass(const Framebuffer &framebuffer,
*commandBufferOut = &mCommandBuffer; *commandBufferOut = &mCommandBuffer;
mForceIndividualBarriers = false; mForceIndividualBarriers = false;
if (mDepthStencilAttachmentIndex != vk::kInvalidAttachmentIndex)
{
if (renderPassAttachmentOps[mDepthStencilAttachmentIndex].loadOp ==
VK_ATTACHMENT_LOAD_OP_CLEAR)
{
mDepthStartAccess = ResourceAccess::Write;
}
if (renderPassAttachmentOps[mDepthStencilAttachmentIndex].stencilLoadOp ==
VK_ATTACHMENT_LOAD_OP_CLEAR)
{
mStencilStartAccess = ResourceAccess::Write;
}
}
mRenderPassStarted = true; mRenderPassStarted = true;
mCounter++; mCounter++;
} }
...@@ -868,7 +853,7 @@ void CommandBufferHelper::restartRenderPassWithReadOnlyDepth(const Framebuffer & ...@@ -868,7 +853,7 @@ void CommandBufferHelper::restartRenderPassWithReadOnlyDepth(const Framebuffer &
mForceIndividualBarriers = true; mForceIndividualBarriers = true;
} }
void CommandBufferHelper::endRenderPass() void CommandBufferHelper::endRenderPass(ContextVk *contextVk)
{ {
pauseTransformFeedbackIfStarted(); pauseTransformFeedbackIfStarted();
...@@ -877,17 +862,18 @@ void CommandBufferHelper::endRenderPass() ...@@ -877,17 +862,18 @@ void CommandBufferHelper::endRenderPass()
return; return;
} }
PackedAttachmentOpsDesc &dsOps = mAttachmentOps[mDepthStencilAttachmentIndex];
// Address invalidated depth/stencil attachments // Address invalidated depth/stencil attachments
if (mDepthInvalidatedState == Invalidated && !mDepthEnabled) if (mDepthInvalidatedState == Invalidated && !mDepthEnabled)
{ {
// The depth attachment is invalid, so don't store it // The depth attachment is invalid, so don't store it
mAttachmentOps[mDepthStencilAttachmentIndex].storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE; dsOps.storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
} }
if (mStencilInvalidatedState == Invalidated && !mStencilEnabled) if (mStencilInvalidatedState == Invalidated && !mStencilEnabled)
{ {
// The stencil attachment is invalid, so don't store it // The stencil attachment is invalid, so don't store it
mAttachmentOps[mDepthStencilAttachmentIndex].stencilStoreOp = dsOps.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
VK_ATTACHMENT_STORE_OP_DONT_CARE;
} }
if (mDepthInvalidatedState == NoLongerInvalidated || if (mDepthInvalidatedState == NoLongerInvalidated ||
mStencilInvalidatedState == NoLongerInvalidated) mStencilInvalidatedState == NoLongerInvalidated)
...@@ -905,22 +891,30 @@ void CommandBufferHelper::endRenderPass() ...@@ -905,22 +891,30 @@ void CommandBufferHelper::endRenderPass()
// buffer has not been used, and the data has also not been stored back into buffer, then // buffer has not been used, and the data has also not been stored back into buffer, then
// just skip the load/clear op. // just skip the load/clear op.
if (mDepthStartAccess == ResourceAccess::Unused && if (mDepthStartAccess == ResourceAccess::Unused &&
mAttachmentOps[mDepthStencilAttachmentIndex].storeOp == VK_ATTACHMENT_STORE_OP_DONT_CARE) dsOps.storeOp == VK_ATTACHMENT_STORE_OP_DONT_CARE)
{ {
mAttachmentOps[mDepthStencilAttachmentIndex].loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE; dsOps.loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
} }
if (mStencilStartAccess == ResourceAccess::Unused && if (mStencilStartAccess == ResourceAccess::Unused &&
mAttachmentOps[mDepthStencilAttachmentIndex].stencilStoreOp == dsOps.stencilStoreOp == VK_ATTACHMENT_STORE_OP_DONT_CARE)
VK_ATTACHMENT_STORE_OP_DONT_CARE)
{ {
mAttachmentOps[mDepthStencilAttachmentIndex].stencilLoadOp = dsOps.stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
VK_ATTACHMENT_LOAD_OP_DONT_CARE;
} }
// Ensure we don't write to a read-only RenderPass. (ReadOnly -> !Write) // Ensure we don't write to a read-only RenderPass. (ReadOnly -> !Write)
ASSERT((mRenderPassDesc.getDepthStencilAccess() != ResourceAccess::ReadOnly) || ASSERT((mRenderPassDesc.getDepthStencilAccess() != ResourceAccess::ReadOnly) ||
mDepthStartAccess != ResourceAccess::Write); mDepthStartAccess != ResourceAccess::Write);
// Fill out perf counters
PerfCounters &counters = contextVk->getPerfCounters();
counters.depthClears += dsOps.loadOp == VK_ATTACHMENT_LOAD_OP_CLEAR ? 1 : 0;
counters.depthLoads += dsOps.loadOp == VK_ATTACHMENT_LOAD_OP_LOAD ? 1 : 0;
counters.depthStores += dsOps.storeOp == VK_ATTACHMENT_STORE_OP_STORE ? 1 : 0;
counters.stencilClears += dsOps.stencilLoadOp == VK_ATTACHMENT_LOAD_OP_CLEAR ? 1 : 0;
counters.stencilLoads += dsOps.stencilLoadOp == VK_ATTACHMENT_LOAD_OP_LOAD ? 1 : 0;
counters.stencilStores += dsOps.stencilStoreOp == VK_ATTACHMENT_STORE_OP_STORE ? 1 : 0;
} }
void CommandBufferHelper::beginTransformFeedback(size_t validBufferCount, void CommandBufferHelper::beginTransformFeedback(size_t validBufferCount,
......
...@@ -952,7 +952,7 @@ class CommandBufferHelper : angle::NonCopyable ...@@ -952,7 +952,7 @@ class CommandBufferHelper : angle::NonCopyable
const ClearValuesArray &clearValues, const ClearValuesArray &clearValues,
CommandBuffer **commandBufferOut); CommandBuffer **commandBufferOut);
void endRenderPass(); void endRenderPass(ContextVk *contextVk);
void restartRenderPassWithReadOnlyDepth(const Framebuffer &framebuffer, void restartRenderPassWithReadOnlyDepth(const Framebuffer &framebuffer,
const RenderPassDesc &renderPassDesc); const RenderPassDesc &renderPassDesc);
...@@ -1031,6 +1031,12 @@ class CommandBufferHelper : angle::NonCopyable ...@@ -1031,6 +1031,12 @@ class CommandBufferHelper : angle::NonCopyable
const vk::RenderPassDesc &renderPassDesc); const vk::RenderPassDesc &renderPassDesc);
ResourceAccess getDepthStartAccess() const { return mDepthStartAccess; } ResourceAccess getDepthStartAccess() const { return mDepthStartAccess; }
bool hasDepthWriteOrClear() const
{
return mDepthStartAccess == ResourceAccess::Write ||
mAttachmentOps[mDepthStencilAttachmentIndex].loadOp == VK_ATTACHMENT_LOAD_OP_CLEAR;
}
private: private:
void addCommandDiagnostics(ContextVk *contextVk); void addCommandDiagnostics(ContextVk *contextVk);
// Allocator used by this class. Using a pool allocator per CBH to avoid threading issues // Allocator used by this class. Using a pool allocator per CBH to avoid threading issues
......
...@@ -755,6 +755,12 @@ struct PerfCounters ...@@ -755,6 +755,12 @@ struct PerfCounters
uint32_t writeDescriptorSets; uint32_t writeDescriptorSets;
uint32_t flushedOutsideRenderPassCommandBuffers; uint32_t flushedOutsideRenderPassCommandBuffers;
uint32_t resolveImageCommands; uint32_t resolveImageCommands;
uint32_t depthClears;
uint32_t depthLoads;
uint32_t depthStores;
uint32_t stencilClears;
uint32_t stencilLoads;
uint32_t stencilStores;
}; };
} // namespace vk } // namespace vk
......
...@@ -28,6 +28,17 @@ namespace ...@@ -28,6 +28,17 @@ namespace
class VulkanPerformanceCounterTest : public ANGLETest class VulkanPerformanceCounterTest : public ANGLETest
{ {
protected: protected:
VulkanPerformanceCounterTest()
{
// Depth required for SwapShouldInvalidateDepthAfterClear.
// Also RGBA8 is required to avoid the clear for emulated alpha.
setConfigRedBits(8);
setConfigGreenBits(8);
setConfigBlueBits(8);
setConfigAlphaBits(8);
setConfigDepthBits(24);
}
const rx::vk::PerfCounters &hackANGLE() const const rx::vk::PerfCounters &hackANGLE() const
{ {
// Hack the angle! // Hack the angle!
...@@ -458,6 +469,31 @@ TEST_P(VulkanPerformanceCounterTest, InvalidatingAndUsingDepthDoesNotBreakRender ...@@ -458,6 +469,31 @@ TEST_P(VulkanPerformanceCounterTest, InvalidatingAndUsingDepthDoesNotBreakRender
EXPECT_EQ(expectedRenderPassCount, actualRenderPassCount); EXPECT_EQ(expectedRenderPassCount, actualRenderPassCount);
} }
// Tests that even if the app clears depth, it should be invalidated if there is no read.
TEST_P(VulkanPerformanceCounterTest, SwapShouldInvalidateDepthAfterClear)
{
const rx::vk::PerfCounters &counters = hackANGLE();
ANGLE_GL_PROGRAM(redProgram, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
// Clear depth.
glClear(GL_DEPTH_BUFFER_BIT);
// Ensure we never read from depth.
glDisable(GL_DEPTH_TEST);
// Do one draw, then swap.
drawQuad(redProgram, essl1_shaders::PositionAttrib(), 0.5f);
ASSERT_GL_NO_ERROR();
uint32_t expectedDepthClears = counters.depthClears;
swapBuffers();
uint32_t actualDepthClears = counters.depthClears;
EXPECT_EQ(expectedDepthClears, actualDepthClears);
}
ANGLE_INSTANTIATE_TEST(VulkanPerformanceCounterTest, ES3_VULKAN()); ANGLE_INSTANTIATE_TEST(VulkanPerformanceCounterTest, ES3_VULKAN());
ANGLE_INSTANTIATE_TEST(VulkanPerformanceCounterTest_ES31, ES31_VULKAN()); ANGLE_INSTANTIATE_TEST(VulkanPerformanceCounterTest_ES31, ES31_VULKAN());
......
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