fix: small edge case bugs

Author: alexcu2718

include/quark/vk/presentation/details/swapchain.hpp

@@ -2,7 +2,7 @@
 
 #include <cstdint>
 #include <quark/platform/window/IWindow.hpp>
-#include <quark/utils/raii.hpp>
+#include <quark/utils/result.hpp>
 #include <vector>
 #include <vulkan/vulkan_core.h>
 
@@ -30,11 +30,12 @@ class Swapchain final {
   Swapchain() = default;
   ~Swapchain() { reset(); }
 
-  QUARK_NO_COPY(Swapchain);
+  Swapchain(const Swapchain &) = delete;
+  Swapchain &operator=(const Swapchain &) = delete;
   Swapchain(Swapchain &&other) noexcept;
   Swapchain &operator=(Swapchain &&other) noexcept;
 
-  void create(const CreateInfo &ci);
+  [[nodiscard]] util::Status create(const CreateInfo &ci);
   void reset() noexcept;
 
   [[nodiscard]] VkSwapchainKHR handle() const noexcept { return swapchain_; }

include/quark/vk/presentation/presenter_bundle.hpp

@@ -45,9 +45,6 @@ class PresenterBundle final {
   [[nodiscard]] Swapchain &swapchain() noexcept { return swapchain_; }
 
 private:
-  [[nodiscard]] util::Result<Swapchain>
-  create_swapchain_(VkSwapchainKHR old_swapchain) const;
-
   CreateInfo create_info_{};
   VkSurfaceKHR surface_{VK_NULL_HANDLE};
   Swapchain swapchain_;

src/backend/vulkan/device/device.cpp

@@ -248,7 +248,9 @@ Device::build_device_extensions_(VkPhysicalDevice physical_device,
                  QUARK_ERR(util::Errc::Unsupported,
                            "Required device extension unsupported {}", name));
 
-    if (!std::ranges::contains(enabled, name)) {
+    if (std::ranges::none_of(enabled, [name](const char *e) {
+          return std::strcmp(e, name) == 0;
+        })) {
       enabled.push_back(name);
       QUARK_LOG_INFO("device extension enabled: '{}'", name);
     }
@@ -263,7 +265,9 @@ Device::build_device_extensions_(VkPhysicalDevice physical_device,
   // MoltenVK usually needs portability subset;
   constexpr const char *kPortabilitySubset = "VK_KHR_portability_subset";
   if (has_device_extension_props_(props, kPortabilitySubset) &&
-      !std::ranges::contains(enabled, kPortabilitySubset)) {
+      std::ranges::none_of(enabled, [](const char *e) {
+        return std::strcmp(e, kPortabilitySubset) == 0;
+      })) {
     enabled.push_back(kPortabilitySubset);
     QUARK_LOG_INFO("portability subset: enabled");
   }

src/backend/vulkan/presentation/presenter_bundle.cpp

@@ -3,8 +3,6 @@
 #include <quark/vk/presentation/presenter_bundle.hpp>
 #include <quark/vk/surface_source.hpp>
 
-#include <exception>
-
 namespace quark::vk {
 
 namespace {
@@ -53,52 +51,43 @@ util::Status PresenterBundle::create(const CreateInfo &ci) {
 
   QUARK_TRY_ASSIGN(surface_, create_surface(ci.instance, ci.window));
 
-  auto swapchain_res = create_swapchain_(VK_NULL_HANDLE);
-  if (!swapchain_res) {
+  Swapchain::CreateInfo sci{};
+  sci.physical_device = ci.physical_device;
+  sci.device = ci.device;
+  sci.surface = surface_;
+  sci.graphics_queue_family_index = ci.graphics_queue_family_index;
+  sci.present_queue_family_index = ci.present_queue_family_index;
+  sci.window = ci.window;
+  sci.old_swapchain = VK_NULL_HANDLE;
+  sci.preferred_present_mode = ci.preferred_present_mode;
+  sci.preferred_format = ci.preferred_format;
+  sci.preferred_color_space = ci.preferred_color_space;
+
+  if (auto res = swapchain_.create(sci); !res) {
     destroy_surface(ci.instance, surface_);
-    return util::unexpected(std::move(swapchain_res.error()));
+    return util::unexpected(std::move(res.error()));
   }
 
-  swapchain_ = std::move(swapchain_res.value());
   QUARK_OK();
 }
 
-util::Result<Swapchain>
-PresenterBundle::create_swapchain_(VkSwapchainKHR old_swapchain) const {
-  QUARK_ENSURE(
-      surface_ != VK_NULL_HANDLE,
-      QUARK_ERR(util::Errc::InvalidState, "presenter surface is null"));
-
-  Swapchain swapchain;
-  Swapchain::CreateInfo ci{};
-  ci.physical_device = create_info_.physical_device;
-  ci.device = create_info_.device;
-  ci.surface = surface_;
-  ci.graphics_queue_family_index = create_info_.graphics_queue_family_index;
-  ci.present_queue_family_index = create_info_.present_queue_family_index;
-  ci.window = create_info_.window;
-  ci.old_swapchain = old_swapchain;
-  ci.preferred_present_mode = create_info_.preferred_present_mode;
-  ci.preferred_format = create_info_.preferred_format;
-  ci.preferred_color_space = create_info_.preferred_color_space;
-
-  try {
-    swapchain.create(ci);
-  } catch (const std::exception &e) {
-    QUARK_FAIL(QUARK_ERR(util::Errc::ApiError,
-                         "PresenterBundle::create_swapchain_ failed: {}",
-                         e.what()));
-  }
-
-  return swapchain;
-}
-
 util::Status PresenterBundle::recreate_swapchain() {
-  auto *old_swapchain = swapchain_.handle();
+  VkSwapchainKHR old_handle = swapchain_.handle();
+
+  Swapchain::CreateInfo sci{};
+  sci.physical_device = create_info_.physical_device;
+  sci.device = create_info_.device;
+  sci.surface = surface_;
+  sci.graphics_queue_family_index = create_info_.graphics_queue_family_index;
+  sci.present_queue_family_index = create_info_.present_queue_family_index;
+  sci.window = create_info_.window;
+  sci.old_swapchain = old_handle;
+  sci.preferred_present_mode = create_info_.preferred_present_mode;
+  sci.preferred_format = create_info_.preferred_format;
+  sci.preferred_color_space = create_info_.preferred_color_space;
 
   Swapchain next_swapchain;
-  QUARK_TRY_ASSIGN(next_swapchain, create_swapchain_(old_swapchain));
-
+  QUARK_TRY_STATUS(next_swapchain.create(sci));
   swapchain_ = std::move(next_swapchain);
   QUARK_OK();
 }

src/backend/vulkan/presentation/swapchain.cpp

@@ -2,8 +2,8 @@
 #include <array>
 #include <limits>
 #include <quark/platform/window/IWindow.hpp>
+#include <quark/utils/diagnostic.hpp>
 #include <quark/vk/presentation/details/swapchain.hpp>
-#include <stdexcept>
 #include <utility>
 #include <vector>
 #include <vulkan/vulkan_core.h>
@@ -122,33 +122,30 @@ Swapchain &Swapchain::operator=(Swapchain &&other) noexcept {
   return *this;
 }
 
-void Swapchain::create(const CreateInfo &ci) {
+util::Status Swapchain::create(const CreateInfo &ci) {
   reset();
 
   // TODO: Replace with device valid()
-  if (ci.physical_device == VK_NULL_HANDLE) {
-    throw std::runtime_error("Swapchain::create: physical_device is null");
-  }
-
-  if (ci.device == VK_NULL_HANDLE) {
-    throw std::runtime_error("Swapchain::create: device is null");
-  }
-
-  if (ci.surface == VK_NULL_HANDLE) {
-    throw std::runtime_error("Swapchain::create: surface is null");
-  }
-
+  QUARK_ENSURE(ci.physical_device != VK_NULL_HANDLE,
+               QUARK_ERR(util::Errc::InvalidArg,
+                         "Swapchain::create: physical_device is null"));
+  QUARK_ENSURE(
+      ci.device != VK_NULL_HANDLE,
+      QUARK_ERR(util::Errc::InvalidArg, "Swapchain::create: device is null"));
+  QUARK_ENSURE(
+      ci.surface != VK_NULL_HANDLE,
+      QUARK_ERR(util::Errc::InvalidArg, "Swapchain::create: surface is null"));
   // TODO: Replace with window valid
-  if (ci.window == nullptr) {
-    throw std::runtime_error("Swapchain::create: window is null");
-  }
+  QUARK_ENSURE(
+      ci.window != nullptr,
+      QUARK_ERR(util::Errc::InvalidArg, "Swapchain::create: window is null"));
 
   device_ = ci.device;
 
   const SupportDetails support = query_support(ci.physical_device, ci.surface);
   if (support.formats.empty() || support.present_modes.empty()) {
-    throw std::runtime_error(
-        "Swapchain::create: swapchain support is incomplete");
+    QUARK_FAIL(QUARK_ERR(util::Errc::Unsupported,
+                         "Swapchain::create: swapchain support is incomplete"));
   }
 
   const VkSurfaceFormatKHR surface_format = choose_format(
@@ -191,9 +188,8 @@ void Swapchain::create(const CreateInfo &ci) {
 
   VkResult r = vkCreateSwapchainKHR(ci.device, &sci, /*pAllocator=*/nullptr,
                                     &swapchain_);
-  if (r != VK_SUCCESS) {
-    throw std::runtime_error("vkCreateSwapchainKHR failed");
-  }
+  QUARK_ENSURE(r == VK_SUCCESS,
+               QUARK_ERR(util::Errc::ApiError, "vkCreateSwapchainKHR failed"));
 
   vkGetSwapchainImagesKHR(ci.device, swapchain_, &image_count,
                           /*pSwapchainImages=*/nullptr);
@@ -218,10 +214,11 @@ void Swapchain::create(const CreateInfo &ci) {
 
     r = vkCreateImageView(ci.device, &ivci, /*pAllocator=*/nullptr,
                           &image_views_[index]);
-    if (r != VK_SUCCESS) {
-      throw std::runtime_error("vkCreateImageView failed");
-    }
+    QUARK_ENSURE(r == VK_SUCCESS,
+                 QUARK_ERR(util::Errc::ApiError, "vkCreateImageView failed"));
   }
+
+  QUARK_OK();
 }
 
 void Swapchain::reset() noexcept {

src/utils/diagnostics/diagnostic.cpp

@@ -15,12 +15,18 @@ std::shared_ptr<const SinkList> g_sinks;
 
 void set_diagnostic_sinks(std::span<const DiagnosticSink> sinks) noexcept {
   auto list = std::make_shared<SinkList>(sinks.begin(), sinks.end());
-  std::atomic_store_explicit(&g_sinks,
-                             std::shared_ptr<const SinkList>(std::move(list)),
-                             std::memory_order_release);
+  std::atomic_store_explicit( // NOLINT deprecated-declarations
+      &g_sinks,
+      // the suggestion to use std::atomic::<std::shared_ptr<T>>
+      std::shared_ptr<const SinkList>(
+          std::move(list)), // This may not require a move
+      std::memory_order_release);
+  // lint from clang tidy does not work on msvc(it works on GCC, at least!),  we
+  // should look into this more! TODO
 }
 
 std::shared_ptr<const SinkList> diagnostic_sinks_snapshot() noexcept {
+  // NOLINTNEXTLINE (same as above)
   return std::atomic_load_explicit(std::addressof(g_sinks),
                                    std::memory_order_acquire);
 }