Fixed
Status Update
No update yet.
Comments
ap...@google.com
Project: angle/angle
Branch: master
commit 46a139ad5d7b1d9fbd6779a45c6c77b4afce4767
Author: Charlie Lao <cclao@google.com>
Date: Thu May 06 11:20:51 2021
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:
Change-Id: I89814274352f09cbf1f7b58a91bbaf131b983fb1
Reviewed-on:https://chromium-review.googlesource.com/c/angle/angle/+/2877933
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
M src/libANGLE/renderer/vulkan/vk_helpers.cpp
M src/libANGLE/renderer/vulkan/vk_helpers.h
https://chromium-review.googlesource.com/2877933
Branch: master
commit 46a139ad5d7b1d9fbd6779a45c6c77b4afce4767
Author: Charlie Lao <cclao@google.com>
Date: Thu May 06 11:20:51 2021
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:
Change-Id: I89814274352f09cbf1f7b58a91bbaf131b983fb1
Reviewed-on:
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
M src/libANGLE/renderer/vulkan/vk_helpers.cpp
M src/libANGLE/renderer/vulkan/vk_helpers.h
Description
In CommandBufferHelper::endRenderPass(), we are using dsOps.initialLayout to determine if we could change storeOps to None or not
line 1569:
// 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.
if (dsOps.initialLayout == static_cast<uint16_t>(ImageLayout::DepthStencilReadOnly) &&
contextVk->getFeatures().supportsRenderPassStoreOpNoneQCOM.enabled)
{
if (dsOps.storeOp == RenderPassStoreOp::Store)
{
dsOps.storeOp = RenderPassStoreOp::NoneQCOM;
}
if (dsOps.stencilStoreOp == RenderPassStoreOp::Store)
{
dsOps.stencilStoreOp = RenderPassStoreOp::NoneQCOM;
}
}
But the initialLayout is set at the end of function, at line 1602
line 1602:
// Do depth stencil layout change.
if (mDepthStencilImage)
{
finalizeDepthStencilImageLayout(contextVk);
}
This will void the optimization for storeOp.
This does not have material effect on QC right now because the current driver does not support RenderPassStoreOp::NoneQCOM. But with futyure QC driver, this may help reduce the unnecessary depth store.
It should not affect ARM since ARM is using the read only layout to automatically change storeOP to Preserve.