Commit 46a139ad by Charlie Lao Committed by Commit Bot

Vulkan: set DS layout before using it in the endRenderPass

In CommandBufferHelper::endRenderPass(), we are checking depth stencil's initialLayout to change storeOp to None if the layout is read only. But the layout was set after that check, which essentially voids the optimization. This CL moves the finalizeDepthStencilImageLayout() call before the layout is used. This CL also moves the depth stencil loadOp/storeOp to a new function finalizeDepthStencilLoadStoreOp(). When depthImage gets deleted before renderpass ends, we could also apply the same load/store optimization just like we did at endRenderPass() time. Bug: b/187425444 Change-Id: I89814274352f09cbf1f7b58a91bbaf131b983fb1 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2877933Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Commit-Queue: Charlie Lao <cclao@google.com>
parent 6e3c5321
...@@ -1388,30 +1388,6 @@ void CommandBufferHelper::finalizeDepthStencilImageLayout(Context *context) ...@@ -1388,30 +1388,6 @@ void CommandBufferHelper::finalizeDepthStencilImageLayout(Context *context)
VkImageAspectFlags aspectFlags = GetDepthStencilAspectFlags(format); VkImageAspectFlags aspectFlags = GetDepthStencilAspectFlags(format);
updateImageLayoutAndBarrier(context, mDepthStencilImage, aspectFlags, imageLayout); updateImageLayoutAndBarrier(context, mDepthStencilImage, aspectFlags, imageLayout);
} }
if (!mDepthStencilImage->hasRenderPassUsageFlag(RenderPassUsage::ReadOnlyAttachment))
{
ASSERT(mDepthStencilAttachmentIndex != kAttachmentIndexInvalid);
const PackedAttachmentOpsDesc &dsOps = mAttachmentOps[mDepthStencilAttachmentIndex];
// If the image is being written to, mark its contents defined.
VkImageAspectFlags definedAspects = 0;
if (dsOps.storeOp == VK_ATTACHMENT_STORE_OP_STORE)
{
definedAspects |= VK_IMAGE_ASPECT_DEPTH_BIT;
}
if (dsOps.stencilStoreOp == VK_ATTACHMENT_STORE_OP_STORE)
{
definedAspects |= VK_IMAGE_ASPECT_STENCIL_BIT;
}
if (definedAspects != 0)
{
mDepthStencilImage->onWrite(mDepthStencilLevelIndex, 1, mDepthStencilLayerIndex,
mDepthStencilLayerCount, definedAspects);
}
}
mDepthStencilImage->resetRenderPassUsageFlags();
} }
void CommandBufferHelper::finalizeDepthStencilResolveImageLayout(Context *context) void CommandBufferHelper::finalizeDepthStencilResolveImageLayout(Context *context)
...@@ -1475,7 +1451,7 @@ void CommandBufferHelper::finalizeImageLayout(Context *context, const ImageHelpe ...@@ -1475,7 +1451,7 @@ void CommandBufferHelper::finalizeImageLayout(Context *context, const ImageHelpe
if (mDepthStencilImage == image) if (mDepthStencilImage == image)
{ {
finalizeDepthStencilImageLayout(context); finalizeDepthStencilImageLayoutAndLoadStore(context);
mDepthStencilImage = nullptr; mDepthStencilImage = nullptr;
} }
...@@ -1486,52 +1462,18 @@ void CommandBufferHelper::finalizeImageLayout(Context *context, const ImageHelpe ...@@ -1486,52 +1462,18 @@ void CommandBufferHelper::finalizeImageLayout(Context *context, const ImageHelpe
} }
} }
void CommandBufferHelper::beginRenderPass(const Framebuffer &framebuffer, void CommandBufferHelper::finalizeDepthStencilLoadStore(Context *context)
const gl::Rectangle &renderArea,
const RenderPassDesc &renderPassDesc,
const AttachmentOpsArray &renderPassAttachmentOps,
const vk::PackedAttachmentCount colorAttachmentCount,
const PackedAttachmentIndex depthStencilAttachmentIndex,
const PackedClearValuesArray &clearValues,
CommandBuffer **commandBufferOut)
{ {
ASSERT(mIsRenderPassCommandBuffer); ASSERT(mDepthStencilAttachmentIndex != kAttachmentIndexInvalid);
ASSERT(empty());
mRenderPassDesc = renderPassDesc;
mAttachmentOps = renderPassAttachmentOps;
mDepthStencilAttachmentIndex = depthStencilAttachmentIndex;
mColorImagesCount = colorAttachmentCount;
mFramebuffer.setHandle(framebuffer.getHandle());
mRenderArea = renderArea;
mClearValues = clearValues;
*commandBufferOut = &mCommandBuffer;
mRenderPassStarted = true;
mCounter++;
}
void CommandBufferHelper::endRenderPass(ContextVk *contextVk) PackedAttachmentOpsDesc &dsOps = mAttachmentOps[mDepthStencilAttachmentIndex];
{
for (PackedAttachmentIndex index = kAttachmentIndexZero; index < mColorImagesCount; ++index)
{
if (mColorImages[index])
{
finalizeColorImageLayout(contextVk, mColorImages[index], index, false);
}
if (mColorResolveImages[index])
{
finalizeColorImageLayout(contextVk, mColorResolveImages[index], index, true);
}
}
if (mDepthStencilAttachmentIndex == kAttachmentIndexInvalid) // This has to be called after layout been finalized
{ ASSERT(dsOps.initialLayout != static_cast<uint16_t>(ImageLayout::Undefined));
return;
}
PackedAttachmentOpsDesc &dsOps = mAttachmentOps[mDepthStencilAttachmentIndex]; // Ensure we don't write to a read-only RenderPass. (ReadOnly -> !Write)
// Depth/Stencil buffer optimizations: ASSERT(dsOps.initialLayout != static_cast<uint16_t>(ImageLayout::DepthStencilReadOnly) ||
(mDepthAccess != ResourceAccess::Write && mStencilAccess != ResourceAccess::Write));
// If the attachment is invalidated, skip the store op. If we are not loading or clearing the // If the attachment is invalidated, skip the store op. If we are not loading or clearing the
// attachment and the attachment has not been used, auto-invalidate it. // attachment and the attachment has not been used, auto-invalidate it.
...@@ -1567,7 +1509,7 @@ void CommandBufferHelper::endRenderPass(ContextVk *contextVk) ...@@ -1567,7 +1509,7 @@ void CommandBufferHelper::endRenderPass(ContextVk *contextVk)
// For read only depth stencil, we can use StoreOpNone if available. DONT_CARE is still // For read only depth stencil, we can use StoreOpNone if available. DONT_CARE is still
// preferred, so do this after finish the DONT_CARE handling. // preferred, so do this after finish the DONT_CARE handling.
if (dsOps.initialLayout == static_cast<uint16_t>(ImageLayout::DepthStencilReadOnly) && if (dsOps.initialLayout == static_cast<uint16_t>(ImageLayout::DepthStencilReadOnly) &&
contextVk->getFeatures().supportsRenderPassStoreOpNoneQCOM.enabled) context->getRenderer()->getFeatures().supportsRenderPassStoreOpNoneQCOM.enabled)
{ {
if (dsOps.storeOp == RenderPassStoreOp::Store) if (dsOps.storeOp == RenderPassStoreOp::Store)
{ {
...@@ -1592,14 +1534,82 @@ void CommandBufferHelper::endRenderPass(ContextVk *contextVk) ...@@ -1592,14 +1534,82 @@ void CommandBufferHelper::endRenderPass(ContextVk *contextVk)
dsOps.stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE; dsOps.stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
} }
// Ensure we don't write to a read-only RenderPass. (ReadOnly -> !Write) // This has to be done after storeOp has been finalized.
ASSERT(dsOps.initialLayout != static_cast<uint16_t>(ImageLayout::DepthStencilReadOnly) || if (!mDepthStencilImage->hasRenderPassUsageFlag(RenderPassUsage::ReadOnlyAttachment))
(mDepthAccess != ResourceAccess::Write && mStencilAccess != ResourceAccess::Write)); {
// If the image is being written to, mark its contents defined.
VkImageAspectFlags definedAspects = 0;
if (dsOps.storeOp == VK_ATTACHMENT_STORE_OP_STORE)
{
definedAspects |= VK_IMAGE_ASPECT_DEPTH_BIT;
}
if (dsOps.stencilStoreOp == VK_ATTACHMENT_STORE_OP_STORE)
{
definedAspects |= VK_IMAGE_ASPECT_STENCIL_BIT;
}
if (definedAspects != 0)
{
mDepthStencilImage->onWrite(mDepthStencilLevelIndex, 1, mDepthStencilLayerIndex,
mDepthStencilLayerCount, definedAspects);
}
}
}
// Do depth stencil layout change. void CommandBufferHelper::finalizeDepthStencilImageLayoutAndLoadStore(Context *context)
{
finalizeDepthStencilImageLayout(context);
finalizeDepthStencilLoadStore(context);
mDepthStencilImage->resetRenderPassUsageFlags();
}
void CommandBufferHelper::beginRenderPass(const Framebuffer &framebuffer,
const gl::Rectangle &renderArea,
const RenderPassDesc &renderPassDesc,
const AttachmentOpsArray &renderPassAttachmentOps,
const vk::PackedAttachmentCount colorAttachmentCount,
const PackedAttachmentIndex depthStencilAttachmentIndex,
const PackedClearValuesArray &clearValues,
CommandBuffer **commandBufferOut)
{
ASSERT(mIsRenderPassCommandBuffer);
ASSERT(empty());
mRenderPassDesc = renderPassDesc;
mAttachmentOps = renderPassAttachmentOps;
mDepthStencilAttachmentIndex = depthStencilAttachmentIndex;
mColorImagesCount = colorAttachmentCount;
mFramebuffer.setHandle(framebuffer.getHandle());
mRenderArea = renderArea;
mClearValues = clearValues;
*commandBufferOut = &mCommandBuffer;
mRenderPassStarted = true;
mCounter++;
}
void CommandBufferHelper::endRenderPass(ContextVk *contextVk)
{
for (PackedAttachmentIndex index = kAttachmentIndexZero; index < mColorImagesCount; ++index)
{
if (mColorImages[index])
{
finalizeColorImageLayout(contextVk, mColorImages[index], index, false);
}
if (mColorResolveImages[index])
{
finalizeColorImageLayout(contextVk, mColorResolveImages[index], index, true);
}
}
if (mDepthStencilAttachmentIndex == kAttachmentIndexInvalid)
{
return;
}
// Do depth stencil layout change and load store optimization.
if (mDepthStencilImage) if (mDepthStencilImage)
{ {
finalizeDepthStencilImageLayout(contextVk); finalizeDepthStencilImageLayoutAndLoadStore(contextVk);
} }
if (mDepthStencilResolveImage) if (mDepthStencilResolveImage)
{ {
......
...@@ -1221,6 +1221,8 @@ class CommandBufferHelper : angle::NonCopyable ...@@ -1221,6 +1221,8 @@ class CommandBufferHelper : angle::NonCopyable
bool isResolveImage); bool isResolveImage);
void finalizeDepthStencilImageLayout(Context *context); void finalizeDepthStencilImageLayout(Context *context);
void finalizeDepthStencilResolveImageLayout(Context *context); void finalizeDepthStencilResolveImageLayout(Context *context);
void finalizeDepthStencilLoadStore(Context *context);
void finalizeDepthStencilImageLayoutAndLoadStore(Context *context);
void updateImageLayoutAndBarrier(Context *context, void updateImageLayoutAndBarrier(Context *context,
ImageHelper *image, ImageHelper *image,
......
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