Skip to content

Commit fd7a1fd

Browse files
JoseEmilio-ARMJoseEmilio-ARM
authored
Fix synchronization in Subpasses sample (KhronosGroup#1341)
* Fix synchronization in Subpasses sample * Fix mask * Clang-format * Review fixes * Review fixes
1 parent 8e20b2b commit fd7a1fd

3 files changed

Lines changed: 87 additions & 19 deletions

File tree

‎framework/core/command_buffer.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ class CommandBuffer
185185
std::vector<LoadStoreInfoType> const &load_store_infos,
186186
std::vector<std::unique_ptr<vkb::rendering::Subpass<bindingType>>> const &subpasses);
187187
void image_memory_barrier(ImageViewType const &image_view, ImageMemoryBarrierType const &memory_barrier) const;
188+
void image_memory_barrier(RenderTargetType &render_target, uint32_t view_index, ImageMemoryBarrierType const &memory_barrier) const;
188189
void next_subpass();
189190

190191
/**
@@ -929,6 +930,24 @@ inline vkb::core::HPPRenderPass &
929930
return device.get_resource_cache().request_render_pass(render_target.get_attachments(), load_store_infos, subpass_infos);
930931
}
931932

933+
template <vkb::BindingType bindingType>
934+
inline void CommandBuffer<bindingType>::image_memory_barrier(RenderTargetType &render_target, uint32_t view_index, ImageMemoryBarrierType const &memory_barrier) const
935+
{
936+
auto const &image_view = render_target.get_views()[view_index];
937+
938+
if constexpr (bindingType == vkb::BindingType::Cpp)
939+
{
940+
image_memory_barrier_impl(image_view, memory_barrier);
941+
}
942+
else
943+
{
944+
image_memory_barrier_impl(reinterpret_cast<vkb::core::HPPImageView const &>(image_view),
945+
reinterpret_cast<vkb::common::HPPImageMemoryBarrier const &>(memory_barrier));
946+
}
947+
948+
render_target.set_layout(view_index, memory_barrier.new_layout);
949+
}
950+
932951
template <vkb::BindingType bindingType>
933952
inline void CommandBuffer<bindingType>::image_memory_barrier(ImageViewType const &image_view, ImageMemoryBarrierType const &memory_barrier) const
934953
{

‎framework/core/render_pass.cpp‎

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "render_pass.h"
1919

2020
#include <numeric>
21+
#include <span>
2122

2223
#include "device.h"
2324
#include "rendering/render_target.h"
@@ -244,23 +245,66 @@ void set_attachment_layouts(std::vector<T_SubpassDescription> &subpass_descripti
244245
}
245246
}
246247

248+
/**
249+
* @brief Assuming there is only one depth attachment
250+
*/
251+
template <typename T_SubpassDescription, typename T_AttachmentDescription>
252+
bool is_depth_a_dependency(std::vector<T_SubpassDescription> &subpass_descriptions, std::vector<T_AttachmentDescription> &attachment_descriptions)
253+
{
254+
// More than 1 subpass uses depth
255+
if (std::ranges::count_if(subpass_descriptions,
256+
[](auto const &subpass) {
257+
return subpass.pDepthStencilAttachment != nullptr;
258+
}) > 1)
259+
{
260+
return true;
261+
}
262+
263+
// Otherwise check if any uses depth as an input
264+
return std::ranges::any_of(
265+
subpass_descriptions,
266+
[&attachment_descriptions](auto const &subpass) {
267+
return std::ranges::any_of(
268+
std::span{subpass.pInputAttachments, subpass.inputAttachmentCount},
269+
[&attachment_descriptions](auto const &reference) {
270+
return vkb::is_depth_format(attachment_descriptions[reference.attachment].format);
271+
});
272+
});
273+
274+
return false;
275+
}
276+
247277
template <typename T>
248-
std::vector<T> get_subpass_dependencies(const size_t subpass_count)
278+
std::vector<T> get_subpass_dependencies(const size_t subpass_count, bool depth_stencil_dependency)
249279
{
250-
std::vector<T> dependencies(subpass_count - 1);
280+
std::vector<T> dependencies{};
251281

252282
if (subpass_count > 1)
253283
{
254-
for (uint32_t i = 0; i < to_u32(dependencies.size()); ++i)
284+
for (uint32_t subpass_id = 0; subpass_id < to_u32(subpass_count - 1); ++subpass_id)
255285
{
256-
// Transition input attachments from color attachment to shader read
257-
dependencies[i].srcSubpass = i;
258-
dependencies[i].dstSubpass = i + 1;
259-
dependencies[i].srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
260-
dependencies[i].dstStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT;
261-
dependencies[i].srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
262-
dependencies[i].dstAccessMask = VK_ACCESS_INPUT_ATTACHMENT_READ_BIT;
263-
dependencies[i].dependencyFlags = VK_DEPENDENCY_BY_REGION_BIT;
286+
T color_dep{};
287+
color_dep.srcSubpass = subpass_id;
288+
color_dep.dstSubpass = subpass_id + 1;
289+
color_dep.srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
290+
color_dep.dstStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT;
291+
color_dep.srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
292+
color_dep.dstAccessMask = VK_ACCESS_INPUT_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
293+
color_dep.dependencyFlags = VK_DEPENDENCY_BY_REGION_BIT;
294+
dependencies.push_back(color_dep);
295+
296+
if (depth_stencil_dependency)
297+
{
298+
T depth_dep{};
299+
depth_dep.srcSubpass = subpass_id;
300+
depth_dep.dstSubpass = subpass_id + 1;
301+
depth_dep.srcStageMask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT;
302+
depth_dep.dstStageMask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT;
303+
depth_dep.srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
304+
depth_dep.dstAccessMask = VK_ACCESS_INPUT_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
305+
depth_dep.dependencyFlags = VK_DEPENDENCY_BY_REGION_BIT;
306+
dependencies.push_back(depth_dep);
307+
}
264308
}
265309
}
266310

@@ -282,6 +326,11 @@ T get_attachment_reference(const uint32_t attachment, const VkImageLayout layout
282326
template <typename T_SubpassDescription, typename T_AttachmentDescription, typename T_AttachmentReference, typename T_SubpassDependency, typename T_RenderPassCreateInfo>
283327
void RenderPass::create_renderpass(const std::vector<Attachment> &attachments, const std::vector<LoadStoreInfo> &load_store_infos, const std::vector<SubpassInfo> &subpasses)
284328
{
329+
if (attachments.size() != load_store_infos.size())
330+
{
331+
LOGW("Render Pass creation: size of attachment list and load/store info list does not match: {} vs {}", attachments.size(), load_store_infos.size());
332+
}
333+
285334
auto attachment_descriptions = get_attachment_descriptions<T_AttachmentDescription>(attachments, load_store_infos);
286335

287336
// Store attachments for every subpass
@@ -321,8 +370,7 @@ void RenderPass::create_renderpass(const std::vector<Attachment> &attachments, c
321370
// Fill input attachments references
322371
for (auto i_attachment : subpass.input_attachments)
323372
{
324-
auto default_layout = vkb::is_depth_format(attachment_descriptions[i_attachment].format) ? VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL : VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
325-
auto initial_layout = attachments[i_attachment].initial_layout == VK_IMAGE_LAYOUT_UNDEFINED ? default_layout : attachments[i_attachment].initial_layout;
373+
auto initial_layout = vkb::is_depth_format(attachments[i_attachment].format) ? VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL : VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
326374
input_attachments[i].push_back(get_attachment_reference<T_AttachmentReference>(i_attachment, initial_layout));
327375
}
328376

@@ -438,7 +486,7 @@ void RenderPass::create_renderpass(const std::vector<Attachment> &attachments, c
438486
color_output_count.push_back(to_u32(color_attachments[i].size()));
439487
}
440488

441-
const auto &subpass_dependencies = get_subpass_dependencies<T_SubpassDependency>(subpass_count);
489+
const auto &subpass_dependencies = get_subpass_dependencies<T_SubpassDependency>(subpass_count, is_depth_a_dependency(subpass_descriptions, attachment_descriptions));
442490

443491
T_RenderPassCreateInfo create_info{};
444492
set_structure_type(create_info);

‎samples/performance/subpasses/subpasses.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -403,27 +403,28 @@ void Subpasses::draw_renderpasses(vkb::core::CommandBufferC &command_buffer, vkb
403403

404404
vkb::ImageMemoryBarrier barrier;
405405

406-
if (i == 1)
406+
if (vkb::is_depth_format(view.get_format()))
407407
{
408408
barrier.old_layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
409409
barrier.new_layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL;
410410

411411
barrier.src_stage_mask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT;
412+
barrier.dst_stage_mask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT;
412413
barrier.src_access_mask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
414+
barrier.dst_access_mask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT;
413415
}
414416
else
415417
{
416418
barrier.old_layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
417419
barrier.new_layout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
418420

419421
barrier.src_stage_mask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
422+
barrier.dst_stage_mask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT;
420423
barrier.src_access_mask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
424+
barrier.dst_access_mask = VK_ACCESS_COLOR_ATTACHMENT_READ_BIT;
421425
}
422426

423-
barrier.dst_stage_mask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT;
424-
barrier.dst_access_mask = VK_ACCESS_INPUT_ATTACHMENT_READ_BIT;
425-
426-
command_buffer.image_memory_barrier(view, barrier);
427+
command_buffer.image_memory_barrier(render_target, i, barrier);
427428
}
428429

429430
// Second render pass

0 commit comments

Comments
 (0)