vulkan: Fix synchronization with acquire swapchain images.
The vkAcquireNextImageKHR method is a read operation in Vulkan which completes
before the semaphore/fence out parameters are signaled. This means that future uses
of the texture must performs a memory barriers that synchronizes with that
semaphore/barrier. Dawn uses the ALL_COMMANDS_BIT stage for the semaphore, however
such a semaphore doesn't synchronize with a subsequent BOTTOM_OF_PIPE or NONE
srcStage vkPipelineBarrier because there are no common stages. Instead we also use
VK_PIPELINE_STAGE_ALL_COMMANDS_BIT for srcStage for presentable images, ensuring
correct ordering.
The kPresentTextureUsage is split in two different usages "Acquire" and
"Release". When acquiring the stage mask is set to ALL_COMMANDS_BIT and
the layout to UNDEFINED (because we will initialize the texture anyway).
When release the stage mask is 0 as noted in the Vulkan spec, but the
layout set to PRESENT_KHR.
Bug: 41497359
Change-Id: I158d3071602ee598e661fca7993865c0881d4f05
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/173463
Reviewed-by: Loko Kung <lokokung@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/dawn/native/d3d12/SwapChainD3D12.cpp b/src/dawn/native/d3d12/SwapChainD3D12.cpp
index 993efab..fdd68a7 100644
--- a/src/dawn/native/d3d12/SwapChainD3D12.cpp
+++ b/src/dawn/native/d3d12/SwapChainD3D12.cpp
@@ -90,7 +90,7 @@
// presentable texture to present at the end of submits that use them.
CommandRecordingContext* commandContext;
DAWN_TRY_ASSIGN(commandContext, queue->GetPendingCommandContext());
- mApiTexture->TrackUsageAndTransitionNow(commandContext, kPresentTextureUsage,
+ mApiTexture->TrackUsageAndTransitionNow(commandContext, kPresentReleaseTextureUsage,
mApiTexture->GetAllSubresources());
DAWN_TRY(queue->SubmitPendingCommands());
diff --git a/src/dawn/native/d3d12/TextureD3D12.cpp b/src/dawn/native/d3d12/TextureD3D12.cpp
index 1847a02..4203c6f 100644
--- a/src/dawn/native/d3d12/TextureD3D12.cpp
+++ b/src/dawn/native/d3d12/TextureD3D12.cpp
@@ -61,10 +61,12 @@
D3D12_RESOURCE_STATES D3D12TextureUsage(wgpu::TextureUsage usage, const Format& format) {
D3D12_RESOURCE_STATES resourceState = D3D12_RESOURCE_STATE_COMMON;
- if (usage & kPresentTextureUsage) {
+ // D3D12 doesn't need special acquire operations for presentable textures.
+ DAWN_ASSERT(!(usage & kPresentAcquireTextureUsage));
+ if (usage & kPresentReleaseTextureUsage) {
// The present usage is only used internally by the swapchain and is never used in
// combination with other usages.
- DAWN_ASSERT(usage == kPresentTextureUsage);
+ DAWN_ASSERT(usage == kPresentReleaseTextureUsage);
return D3D12_RESOURCE_STATE_PRESENT;
}
diff --git a/src/dawn/native/dawn_platform.h b/src/dawn/native/dawn_platform.h
index 5aff6f5..cf1fffb 100644
--- a/src/dawn/native/dawn_platform.h
+++ b/src/dawn/native/dawn_platform.h
@@ -61,27 +61,30 @@
static constexpr wgpu::TextureUsage kReservedTextureUsage =
static_cast<wgpu::TextureUsage>(1u << 31);
-// Add an extra texture usage for textures that will be presented, for use in backends
-// that needs to transition to present usage.
-static constexpr wgpu::TextureUsage kPresentTextureUsage =
+// Extrea texture usages for textures that are used with the presentation engine.
+// Acquire is that Dawn is acquiring the texture from the presentation engine while Release is Dawn
+// releasing is to the presentation engine.
+static constexpr wgpu::TextureUsage kPresentAcquireTextureUsage =
static_cast<wgpu::TextureUsage>(1u << 30);
+static constexpr wgpu::TextureUsage kPresentReleaseTextureUsage =
+ static_cast<wgpu::TextureUsage>(1u << 29);
// Add an extra texture usage (readonly render attachment usage) for render pass resource
// tracking
static constexpr wgpu::TextureUsage kReadOnlyRenderAttachment =
- static_cast<wgpu::TextureUsage>(1u << 29);
+ static_cast<wgpu::TextureUsage>(1u << 28);
// Add an extra texture usage (readonly storage texture usage) for resource tracking
static constexpr wgpu::TextureUsage kReadOnlyStorageTexture =
- static_cast<wgpu::TextureUsage>(1u << 28);
+ static_cast<wgpu::TextureUsage>(1u << 27);
// Add an extra texture usage (writeonly storage texture usage) for resource tracking
static constexpr wgpu::TextureUsage kWriteOnlyStorageTexture =
- static_cast<wgpu::TextureUsage>(1u << 27);
+ static_cast<wgpu::TextureUsage>(1u << 26);
// Add an extra texture usage (load resolve texture to MSAA) for render pass resource tracking
static constexpr wgpu::TextureUsage kResolveAttachmentLoadingUsage =
- static_cast<wgpu::TextureUsage>(1u << 26);
+ static_cast<wgpu::TextureUsage>(1u << 25);
// Extra BufferBindingType for internal storage buffer binding.
static constexpr wgpu::BufferBindingType kInternalStorageBufferBinding =
diff --git a/src/dawn/native/vulkan/BackendVk.cpp b/src/dawn/native/vulkan/BackendVk.cpp
index efa1cd5c..fbc0b2f 100644
--- a/src/dawn/native/vulkan/BackendVk.cpp
+++ b/src/dawn/native/vulkan/BackendVk.cpp
@@ -137,11 +137,6 @@
"Access info (usage: SYNC_IMAGE_LAYOUT_TRANSITION, prior_usage: SYNC_COPY_TRANSFER_WRITE, "
"write_barriers: 0, command: vkCmdCopyBufferToImage"},
- // http://crbug.com/1524408
- {"SYNC-HAZARD-WRITE-AFTER-READ",
- "Access info (prior_usage: SYNC_PRESENT_ENGINE_SYNCVAL_PRESENT_ACQUIRE_READ_SYNCVAL, "
- "read_barriers: VK_PIPELINE_STAGE_2_TOP_OF_PIPE_BIT"},
-
// http://anglebug.com/7513
{"VUID-VkGraphicsPipelineCreateInfo-pStages-06896",
"contains fragment shader state, but stages"},
diff --git a/src/dawn/native/vulkan/SwapChainVk.cpp b/src/dawn/native/vulkan/SwapChainVk.cpp
index adfcc0e..b33764e 100644
--- a/src/dawn/native/vulkan/SwapChainVk.cpp
+++ b/src/dawn/native/vulkan/SwapChainVk.cpp
@@ -591,8 +591,8 @@
// TODO(crbug.com/dawn/269): Remove the need for this by eagerly transitioning the
// presentable texture to present at the end of submits that use them and ideally even
// folding that in the free layout transition at the end of render passes.
- mTexture->TransitionUsageNow(recordingContext, kPresentTextureUsage, wgpu::ShaderStage::None,
- mTexture->GetAllSubresources());
+ mTexture->TransitionUsageNow(recordingContext, kPresentReleaseTextureUsage,
+ wgpu::ShaderStage::None, mTexture->GetAllSubresources());
// Use a semaphore to make sure all rendering has finished before presenting.
VkSemaphore currentSemaphore = mSwapChainSemaphores[mLastImageIndex];
diff --git a/src/dawn/native/vulkan/TextureVk.cpp b/src/dawn/native/vulkan/TextureVk.cpp
index dc90fc8..5d81718 100644
--- a/src/dawn/native/vulkan/TextureVk.cpp
+++ b/src/dawn/native/vulkan/TextureVk.cpp
@@ -153,10 +153,31 @@
if (usage & kReadOnlyRenderAttachment) {
flags |= VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT;
}
- if (usage & kPresentTextureUsage) {
- // The present usage is only used internally by the swapchain and is never used in
+
+ if (usage & kPresentAcquireTextureUsage) {
+ // The present acquire usage is only used internally by the swapchain and is never used in
// combination with other usages.
- DAWN_ASSERT(usage == kPresentTextureUsage);
+ DAWN_ASSERT(usage == kPresentAcquireTextureUsage);
+ // The Vulkan spec has the following note:
+ //
+ // When the presentable image will be accessed by some stage S, the recommended idiom
+ // for ensuring correct synchronization is:
+ //
+ // The VkSubmitInfo used to submit the image layout transition for execution includes
+ // vkAcquireNextImageKHR::semaphore in its pWaitSemaphores member, with the
+ // corresponding element of pWaitDstStageMask including S.
+ //
+ // The synchronization command that performs any necessary image layout transition
+ // includes S in both the srcStageMask and dstStageMask.
+ //
+ // There is no mention of an access flag because there is no access flag associated with
+ // the presentation engine, so we leave it to 0.
+ flags |= 0;
+ }
+ if (usage & kPresentReleaseTextureUsage) {
+ // The present release usage is only used internally by the swapchain and is never used in
+ // combination with other usages.
+ DAWN_ASSERT(usage == kPresentReleaseTextureUsage);
// The Vulkan spec has the following note:
//
// When transitioning the image to VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR or
@@ -221,10 +242,35 @@
flags |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
}
}
- if (usage & kPresentTextureUsage) {
- // The present usage is only used internally by the swapchain and is never used in
+
+ if (usage & kPresentAcquireTextureUsage) {
+ // The present acquire usage is only used internally by the swapchain and is never used in
// combination with other usages.
- DAWN_ASSERT(usage == kPresentTextureUsage);
+ DAWN_ASSERT(usage == kPresentAcquireTextureUsage);
+ // The vkAcquireNextImageKHR method is a read operation in Vulkan which completes
+ // before the semaphore/fence out parameters are signaled. This means that future uses
+ // of the texture must performs a memory barriers that synchronizes with that
+ // semaphore/barrier. Dawn uses the ALL_COMMANDS_BIT stage for the semaphore, however
+ // such a semaphore doesn't synchronize with a subsequent BOTTOM_OF_PIPE or NONE
+ // srcStage vkPipelineBarrier because there are no common stages. Instead we also use
+ // VK_PIPELINE_STAGE_ALL_COMMANDS_BIT for srcStage for presentable images, ensuring
+ // correct ordering. This explains the idiom noted in the Vulkan spec:
+ //
+ // When the presentable image will be accessed by some stage S, the recommended idiom
+ // for ensuring correct synchronization is:
+ //
+ // The VkSubmitInfo used to submit the image layout transition for execution includes
+ // vkAcquireNextImageKHR::semaphore in its pWaitSemaphores member, with the
+ // corresponding element of pWaitDstStageMask including S.
+ //
+ // The synchronization command that performs any necessary image layout transition
+ // includes S in both the srcStageMask and dstStageMask.
+ flags |= VK_PIPELINE_STAGE_ALL_COMMANDS_BIT;
+ }
+ if (usage & kPresentReleaseTextureUsage) {
+ // The present release usage is only used internally by the swapchain and is never used in
+ // combination with other usages.
+ DAWN_ASSERT(usage == kPresentReleaseTextureUsage);
// The Vulkan spec has the following note:
//
// When transitioning the image to VK_IMAGE_LAYOUT_SHARED_PRESENT_KHR or
@@ -611,8 +657,12 @@
case kReadOnlyRenderAttachment:
return VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL;
- case kPresentTextureUsage:
+ case kPresentReleaseTextureUsage:
return VK_IMAGE_LAYOUT_PRESENT_SRC_KHR;
+ case kPresentAcquireTextureUsage:
+ // We always consider images being acquired from the swapchain as uninitialized,
+ // so we can use the UNDEFINED Vulkan image layout.
+ return VK_IMAGE_LAYOUT_UNDEFINED;
case wgpu::TextureUsage::TransientAttachment:
// Will be covered by RenderAttachment above, as specification of
@@ -949,6 +999,7 @@
void Texture::InitializeForSwapChain(VkImage nativeImage) {
mHandle = nativeImage;
+ mSubresourceLastSyncInfos.Fill({kPresentAcquireTextureUsage, wgpu::ShaderStage::None});
SetLabelHelper("Dawn_SwapChainTexture");
}
