Vulkan: Fix incorrect barriers for T2T copies of DS.

T2T copies for depth-stencil formats where done one by one. Because
the two per-aspect copies where submitted with no barriers in between
them, incorrect synchronization could occur, making the end state of the
destination texture incorrect.

Fix this by using the combined aspects of the texture to perform copies,
such that depth stencil are copied in a single command instead of two
commands.

Unfortunately the VVLs don't catch this issue, but the reporter of the
issue confirmed that this commit fix the dawn_end2end_tests failures
they were seeing.

Fixed: dawn:1514
Change-Id: I2e1c5f8d9aabeb0119364d26c9d66d0763cfadcf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/103421
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
This commit is contained in:
Corentin Wallez 2022-09-26 11:07:25 +00:00 committed by Dawn LUCI CQ
parent d811edd94b
commit ab882c17a5
3 changed files with 8 additions and 7 deletions

View File

@ -687,9 +687,9 @@ MaybeError CommandBuffer::RecordCommands(CommandRecordingContext* recordingConte
if (!copyUsingTemporaryBuffer) {
VkImage srcImage = ToBackend(src.texture)->GetHandle();
VkImage dstImage = ToBackend(dst.texture)->GetHandle();
Aspect aspects = ToBackend(src.texture)->GetDisjointVulkanAspects();
for (Aspect aspect : IterateEnumMask(src.texture->GetFormat().aspects)) {
ASSERT(dst.texture->GetFormat().aspects & aspect);
for (Aspect aspect : IterateEnumMask(aspects)) {
VkImageCopy region =
ComputeImageCopyRegion(src, dst, copy->copySize, aspect);

View File

@ -833,7 +833,7 @@ void Texture::TransitionEagerlyForExport(CommandRecordingContext* recordingConte
// Get any usage, ideally the last one to do nothing
ASSERT(GetNumMipLevels() == 1 && GetArrayLayers() == 1);
SubresourceRange range = {GetAllVulkanAspects(), {0, 1}, {0, 1}};
SubresourceRange range = {GetDisjointVulkanAspects(), {0, 1}, {0, 1}};
wgpu::TextureUsage usage = mSubresourceLastUsages.Get(range.aspects, 0, 0);
@ -882,7 +882,7 @@ MaybeError Texture::ExportExternalTexture(VkImageLayout desiredLayout,
mExternalState = ExternalState::Released;
ASSERT(GetNumMipLevels() == 1 && GetArrayLayers() == 1);
wgpu::TextureUsage usage = mSubresourceLastUsages.Get(GetAllVulkanAspects(), 0, 0);
wgpu::TextureUsage usage = mSubresourceLastUsages.Get(GetDisjointVulkanAspects(), 0, 0);
VkImageLayout layout = VulkanImageLayout(this, usage);
@ -971,7 +971,7 @@ void Texture::TweakTransitionForExternalUsage(CommandRecordingContext* recording
if (barriers->size() == transitionBarrierStart) {
barriers->push_back(BuildMemoryBarrier(
this, wgpu::TextureUsage::None, wgpu::TextureUsage::None,
SubresourceRange::SingleMipAndLayer(0, 0, GetAllVulkanAspects())));
SubresourceRange::SingleMipAndLayer(0, 0, GetDisjointVulkanAspects())));
}
VkImageMemoryBarrier* barrier = &(*barriers)[transitionBarrierStart];
@ -1348,7 +1348,7 @@ bool Texture::UseCombinedAspects() const {
return mCombinedAspect != Aspect::None;
}
Aspect Texture::GetAllVulkanAspects() const {
Aspect Texture::GetDisjointVulkanAspects() const {
if (UseCombinedAspects()) {
return mCombinedAspect;
}

View File

@ -65,6 +65,8 @@ class Texture final : public TextureBase {
VkImage nativeImage);
VkImage GetHandle() const;
// Returns the aspects used for tracking of Vulkan state. These can be the combined aspects.
Aspect GetDisjointVulkanAspects() const;
// Transitions the texture to be used as `usage`, recording any necessary barrier in
// `commands`.
@ -183,7 +185,6 @@ class Texture final : public TextureBase {
SubresourceStorage<wgpu::TextureUsage> mSubresourceLastUsages;
bool UseCombinedAspects() const;
Aspect GetAllVulkanAspects() const;
};
class TextureView final : public TextureViewBase {