🐛 Fix present semaphore sync

Discovered on SDK 1.4.313

Author: karnkaul

.github/workflows/ci.yml

@@ -1,4 +1,4 @@
-name: ci-push
+name: ci-pr
 on: 
   pull_request:
     branches-ignore:

guide/src/rendering/render_sync.md

@@ -17,8 +17,6 @@ Add a private `struct RenderSync` to `App`:
 struct RenderSync {
   // signaled when Swapchain image has been acquired.
   vk::UniqueSemaphore draw{};
-  // signaled when image is ready to be presented.
-  vk::UniqueSemaphore present{};
   // signaled with present Semaphore, waited on before next render.
   vk::UniqueFence drawn{};
   // used to record rendering commands.
@@ -68,7 +66,6 @@ void App::create_render_sync() {
      std::views::zip(m_render_sync, command_buffers)) {
     sync.command_buffer = command_buffer;
     sync.draw = m_device->createSemaphoreUnique({});
-    sync.present = m_device->createSemaphoreUnique({});
     sync.drawn = m_device->createFenceUnique(fence_create_info_v);
   }
 }

guide/src/rendering/swapchain_loop.md

@@ -18,7 +18,9 @@ Additionally, the number of swapchain images can vary, whereas the engine should
 
 ## Virtual Frames
 
-All the dynamic resources used during the rendering of a frame comprise a virtual frame. The application has a fixed number of virtual frames which it cycles through on each render pass. Each frame will be associated with a `vk::Fence` which will be waited on before rendering to it again. It will also have a pair of `vk::Semaphore`s to synchronize the acquire, render, and present calls on the GPU (we don't need to wait for them in the code). Lastly, there will be a Command Buffer per virtual frame, where all rendering commands for that frame (including layout transitions) will be recorded.
+All the dynamic resources used during the rendering of a frame comprise a virtual frame. The application has a fixed number of virtual frames which it cycles through on each render pass. Each frame will be associated with a `vk::Fence` which will be waited on before rendering to it again. It will also have a `vk::Semaphore` to synchronize the acquire and render calls on the GPU (we don't need to wait for them in the code). Lastly, there will be a Command Buffer per virtual frame, where all rendering commands for that frame (including layout transitions) will be recorded.
+
+Presentation will require a semaphore for synchronization too, but since the swapchain loop waits on the _drawn_ fence, which will be pre-signaled for each virtual frame on first use, present semaphores cannot be part of the virtual frame. It is possible to acquire another image and submit commands with a present semaphore that has not been signaled yet - this is invalid. Thus, these semaphores will be tied to the swapchain images (associated with their indices), and recreated with it.
 
 ## Image Layouts
 

guide/src/rendering/swapchain_update.md

@@ -1,5 +1,41 @@
 # Swapchain Update
 
+Add a vector of semaphores and populate them in `recreate()`:
+
+```cpp
+void create_present_semaphores();
+
+// ...
+// signaled when image is ready to be presented.
+std::vector<vk::UniqueSemaphore> m_present_semaphores{};
+
+// ...
+auto Swapchain::recreate(glm::ivec2 size) -> bool {
+  // ...
+  populate_images();
+  create_image_views();
+  // recreate present semaphores as the image count might have changed.
+  create_present_semaphores();
+  // ...
+}
+
+void Swapchain::create_present_semaphores() {
+  m_present_semaphores.clear();
+  m_present_semaphores.resize(m_images.size());
+  for (auto& semaphore : m_present_semaphores) {
+    semaphore = m_device.createSemaphoreUnique({});
+  }
+}
+```
+
+Add a function to get the present semaphore corresponding to the acquired image, this will be signaled by render command submission:
+
+```cpp
+auto Swapchain::get_present_semaphore() const -> vk::Semaphore {
+  return *m_present_semaphores.at(m_image_index.value());
+}
+```
+
 Swapchain acquire/present operations can have various results. We constrain ourselves to the following:
 
 - `eSuccess`: all good
@@ -59,13 +95,15 @@ auto Swapchain::acquire_next_image(vk::Semaphore const to_signal)
 Similarly, present:
 
 ```cpp
-auto Swapchain::present(vk::Queue const queue, vk::Semaphore const to_wait)
+auto Swapchain::present(vk::Queue const queue)
   -> bool {
   auto const image_index = static_cast<std::uint32_t>(m_image_index.value());
+  auto const wait_semaphore =
+    *m_present_semaphores.at(static_cast<std::size_t>(image_index));
   auto present_info = vk::PresentInfoKHR{};
   present_info.setSwapchains(*m_swapchain)
     .setImageIndices(image_index)
-    .setWaitSemaphores(to_wait);
+    .setWaitSemaphores(wait_semaphore);
   // avoid VulkanHPP ErrorOutOfDateKHR exceptions by using alternate API.
   auto const result = queue.presentKHR(&present_info);
   m_image_index.reset();

src/app.cpp

@@ -132,7 +132,6 @@ void App::create_render_sync() {
 		 std::views::zip(m_render_sync, command_buffers)) {
 		sync.command_buffer = command_buffer;
 		sync.draw = m_device->createSemaphoreUnique({});
-		sync.present = m_device->createSemaphoreUnique({});
 		sync.drawn = m_device->createFenceUnique(fence_create_info_v);
 	}
 }
@@ -256,7 +255,7 @@ void App::submit_and_present() {
 	wait_semaphore_info.setSemaphore(*render_sync.draw)
 		.setStageMask(vk::PipelineStageFlagBits2::eColorAttachmentOutput);
 	auto signal_semaphore_info = vk::SemaphoreSubmitInfo{};
-	signal_semaphore_info.setSemaphore(*render_sync.present)
+	signal_semaphore_info.setSemaphore(m_swapchain->get_present_semaphore())
 		.setStageMask(vk::PipelineStageFlagBits2::eColorAttachmentOutput);
 	submit_info.setCommandBufferInfos(command_buffer_info)
 		.setWaitSemaphoreInfos(wait_semaphore_info)
@@ -270,8 +269,7 @@ void App::submit_and_present() {
 	// framebuffer size does not match the Swapchain image size, check it
 	// explicitly.
 	auto const fb_size_changed = m_framebuffer_size != m_swapchain->get_size();
-	auto const out_of_date =
-		!m_swapchain->present(m_queue, *render_sync.present);
+	auto const out_of_date = !m_swapchain->present(m_queue);
 	if (fb_size_changed || out_of_date) {
 		m_swapchain->recreate(m_framebuffer_size);
 	}

src/app.hpp

@@ -13,11 +13,9 @@ class App {
 
   private:
 	struct RenderSync {
-		// signalled when Swapchain image has been acquired.
+		// signaled when Swapchain image has been acquired.
 		vk::UniqueSemaphore draw{};
-		// signalled when image is ready to be presented.
-		vk::UniqueSemaphore present{};
-		// signalled with present Semaphore, waited on before next render.
+		// signaled with present Semaphore, waited on before next render.
 		vk::UniqueFence drawn{};
 		// used to record rendering commands.
 		vk::CommandBuffer command_buffer{};

src/swapchain.cpp

@@ -120,6 +120,8 @@ auto Swapchain::recreate(glm::ivec2 size) -> bool {
 
 	populate_images();
 	create_image_views();
+	// recreate present semaphores as the image count might have changed.
+	create_present_semaphores();
 
 	size = get_size();
 	std::println("[lvk] Swapchain [{}x{}]", size.x, size.y);
@@ -155,13 +157,18 @@ auto Swapchain::base_barrier() const -> vk::ImageMemoryBarrier2 {
 	return ret;
 }
 
-auto Swapchain::present(vk::Queue const queue, vk::Semaphore const to_wait)
-	-> bool {
+auto Swapchain::get_present_semaphore() const -> vk::Semaphore {
+	return *m_present_semaphores.at(m_image_index.value());
+}
+
+auto Swapchain::present(vk::Queue const queue) -> bool {
 	auto const image_index = static_cast<std::uint32_t>(m_image_index.value());
+	auto const wait_semaphore =
+		*m_present_semaphores.at(static_cast<std::size_t>(image_index));
 	auto present_info = vk::PresentInfoKHR{};
 	present_info.setSwapchains(*m_swapchain)
 		.setImageIndices(image_index)
-		.setWaitSemaphores(to_wait);
+		.setWaitSemaphores(wait_semaphore);
 	// avoid VulkanHPP ErrorOutOfDateKHR exceptions by using alternate API.
 	auto const result = queue.presentKHR(&present_info);
 	m_image_index.reset();
@@ -195,4 +202,12 @@ void Swapchain::create_image_views() {
 		m_image_views.push_back(m_device.createImageViewUnique(image_view_ci));
 	}
 }
+
+void Swapchain::create_present_semaphores() {
+	m_present_semaphores.clear();
+	m_present_semaphores.resize(m_images.size());
+	for (auto& semaphore : m_present_semaphores) {
+		semaphore = m_device.createSemaphoreUnique({});
+	}
+}
 } // namespace lvk

src/swapchain.hpp

@@ -22,11 +22,13 @@ class Swapchain {
 
 	[[nodiscard]] auto base_barrier() const -> vk::ImageMemoryBarrier2;
 
-	[[nodiscard]] auto present(vk::Queue queue, vk::Semaphore to_wait) -> bool;
+	[[nodiscard]] auto get_present_semaphore() const -> vk::Semaphore;
+	[[nodiscard]] auto present(vk::Queue queue) -> bool;
 
   private:
 	void populate_images();
 	void create_image_views();
+	void create_present_semaphores();
 
 	vk::Device m_device{};
 	Gpu m_gpu{};
@@ -35,6 +37,8 @@ class Swapchain {
 	vk::UniqueSwapchainKHR m_swapchain{};
 	std::vector<vk::Image> m_images{};
 	std::vector<vk::UniqueImageView> m_image_views{};
+	// signaled when image is ready to be presented.
+	std::vector<vk::UniqueSemaphore> m_present_semaphores{};
 	std::optional<std::size_t> m_image_index{};
 };
 } // namespace lvk