TMP / Made {GLFW,Vulkan}Managers more OO

desktop/graphics/glfw_mgmt.cpp

@@ -8,18 +8,25 @@
 
 #include "../../main/logging.h"
 #include "../../main/meta.h"
-#include "vulkan_mgmt.h"
+#include "../../main/util.h"
+
 using namespace progressia::main::logging;
 
 namespace progressia::desktop {
 
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables): global variable required for GLFW callbacks
-static GLFWwindow *window = nullptr;
-
 static void onGlfwError(int errorCode, const char *description);
 static void onWindowGeometryChange(GLFWwindow *window, int width, int height);
 
-void initializeGlfw() {
+class GlfwManagerImpl : public GlfwManager {
+  private:
+    GLFWwindow *window = nullptr;
+    std::function<void()> onScreenResize = nullptr;
+
+  public:
+    DISABLE_COPYING(GlfwManagerImpl)
+    DISABLE_MOVING(GlfwManagerImpl)
+
+    GlfwManagerImpl() {
         debug("Beginning GLFW init");
 
         glfwSetErrorCallback(onGlfwError);
@@ -44,24 +51,51 @@ void initializeGlfw() {
         }
 
         constexpr auto windowDimensions = 800;
-    window = glfwCreateWindow(windowDimensions, windowDimensions, title.c_str(),
+        window = glfwCreateWindow(windowDimensions, windowDimensions,
-                              nullptr, nullptr);
+                                  title.c_str(), nullptr, nullptr);
 
         glfwSetWindowSizeCallback(window, onWindowGeometryChange);
 
         debug("GLFW init complete");
     }
 
-void showWindow() {
+    ~GlfwManagerImpl() override { glfwTerminate(); }
+
+    void setOnScreenResize(std::function<void()> hook) override {
+        onScreenResize = hook;
+    }
+
+    void showWindow() override {
         glfwShowWindow(window);
         debug("Window now visible");
     }
 
-bool shouldRun() { return !glfwWindowShouldClose(window); }
+    bool shouldRun() override { return !glfwWindowShouldClose(window); }
 
-void doGlfwRoutine() { glfwPollEvents(); }
+    void doGlfwRoutine() override { glfwPollEvents(); }
 
-void shutdownGlfw() { glfwTerminate(); }
+    friend GLFWwindow *getGLFWWindowHandle();
+    friend void onWindowGeometryChange(GLFWwindow *, int, int);
+};
+
+namespace {
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables): global variables required by GLFW C callbacks
+std::weak_ptr<GlfwManagerImpl> theGlfwManager;
+} // namespace
+
+std::shared_ptr<GlfwManager> makeGlfwManager() {
+    if (!theGlfwManager.expired()) {
+        fatal("GlfwManager already exists");
+        // REPORT_ERROR
+        exit(1);
+    }
+
+    std::shared_ptr<GlfwManagerImpl> aGlfwManager =
+        std::make_shared<GlfwManagerImpl>();
+
+    theGlfwManager = aGlfwManager;
+    return aGlfwManager;
+}
 
 void onGlfwError(int errorCode, const char *description) {
     fatal() << "[GLFW] " << description << " (" << errorCode << ")";
@@ -71,13 +105,25 @@ void onGlfwError(int errorCode, const char *description) {
 
 void onWindowGeometryChange(GLFWwindow *window, [[maybe_unused]] int width,
                             [[maybe_unused]] int height) {
-    if (window != progressia::desktop::window) {
+    if (auto manager = theGlfwManager.lock()) {
+        if (manager->window != window) {
             return;
         }
 
-    resizeVulkanSurface();
+        if (manager->onScreenResize != nullptr) {
+            manager->onScreenResize();
+        }
+    } else {
+        return;
+    }
 }
 
-GLFWwindow *getGLFWWindowHandle() { return window; }
+GLFWwindow *getGLFWWindowHandle() {
+    if (auto manager = theGlfwManager.lock()) {
+        return manager->window;
+    }
+
+    return nullptr;
+}
 
 } // namespace progressia::desktop

desktop/graphics/glfw_mgmt.h

@@ -1,11 +1,22 @@
 #pragma once
 
+#include <functional>
+#include <memory>
+
 namespace progressia::desktop {
 
-void initializeGlfw();
+class GlfwManager {
-void showWindow();
+
-void shutdownGlfw();
+  public:
-bool shouldRun();
+    virtual ~GlfwManager(){};
-void doGlfwRoutine();
+
+    virtual void setOnScreenResize(std::function<void()>) = 0;
+
+    virtual void showWindow() = 0;
+    virtual bool shouldRun() = 0;
+    virtual void doGlfwRoutine() = 0;
+};
+
+std::shared_ptr<GlfwManager> makeGlfwManager();
 
 } // namespace progressia::desktop

desktop/graphics/glfw_mgmt_details.h

@@ -7,6 +7,7 @@
 
 namespace progressia::desktop {
 
+// TODO refactor into OOP
 GLFWwindow *getGLFWWindowHandle();
 
 } // namespace progressia::desktop

desktop/graphics/vulkan_image.cpp

@@ -20,9 +20,7 @@ Image::Image(VkImage vk, VkImageView view, VkFormat format)
     // do nothing
 }
 
-Image::~Image() {
+Image::~Image() = default;
-    // do nothing
-}
 
 /*
  * ManagedImage
@@ -33,7 +31,7 @@ ManagedImage::ManagedImage(std::size_t width, std::size_t height,
                            VkImageUsageFlags usage, Vulkan &vulkan)
     :
 
-      Image(VK_NULL_HANDLE, VK_NULL_HANDLE, format), vulkan(vulkan),
+      Image(VK_NULL_HANDLE, VK_NULL_HANDLE, format), memory(), vulkan(vulkan),
 
       state{VK_IMAGE_LAYOUT_UNDEFINED, 0, VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT} {
 
@@ -146,7 +144,8 @@ Texture::Texture(const progressia::main::Image &src, Vulkan &vulkan)
       ManagedImage(src.width, src.height, VK_FORMAT_R8G8B8A8_SRGB,
                    VK_IMAGE_ASPECT_COLOR_BIT,
                    VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT,
-                   vulkan) {
+                   vulkan),
+      sampler() {
 
     /*
      * Create a staging buffer
@@ -211,9 +210,9 @@ Texture::Texture(const progressia::main::Image &src, Vulkan &vulkan)
     samplerInfo.compareEnable = VK_FALSE;
     samplerInfo.compareOp = VK_COMPARE_OP_ALWAYS;
     samplerInfo.mipmapMode = VK_SAMPLER_MIPMAP_MODE_LINEAR;
-    samplerInfo.mipLodBias = 0.0f;
+    samplerInfo.mipLodBias = 0.0F;
-    samplerInfo.minLod = 0.0f;
+    samplerInfo.minLod = 0.0F;
-    samplerInfo.maxLod = 0.0f;
+    samplerInfo.maxLod = 0.0F;
 
     vulkan.handleVkResult(
         "Could not create texture sampler",
@@ -223,6 +222,7 @@ Texture::Texture(const progressia::main::Image &src, Vulkan &vulkan)
      * Create descriptor set
      */
 
+    // NOLINTNEXTLINE(cppcoreguidelines-prefer-member-initializer): sampler must be set using vkCreateSampler first
     descriptorSet = vulkan.getTextureDescriptors().addTexture(view, sampler);
 }
 
@@ -233,8 +233,8 @@ Texture::~Texture() {
 
 void Texture::bind() {
     // REPORT_ERROR if getCurrentFrame() == nullptr
-    auto commandBuffer = vulkan.getCurrentFrame()->getCommandBuffer();
+    auto *commandBuffer = vulkan.getCurrentFrame()->getCommandBuffer();
-    auto pipelineLayout = vulkan.getPipeline().getLayout();
+    auto *pipelineLayout = vulkan.getPipeline().getLayout();
 
     vkCmdBindDescriptorSets(commandBuffer, VK_PIPELINE_BIND_POINT_GRAPHICS,
                             pipelineLayout,

desktop/graphics/vulkan_image.h

@@ -35,8 +35,9 @@ class ManagedImage : public Image {
     State state;
 
   public:
-    ManagedImage(std::size_t width, std::size_t height, VkFormat,
+    ManagedImage(std::size_t width, std::size_t height, VkFormat format,
-                 VkImageAspectFlags, VkImageUsageFlags, Vulkan &);
+                 VkImageAspectFlags aspect, VkImageUsageFlags usage,
+                 Vulkan &vulkan);
     ~ManagedImage();
 
     void transition(State);
@@ -48,7 +49,7 @@ class Texture : public ManagedImage {
     VkSampler sampler;
     VkDescriptorSet descriptorSet;
 
-    Texture(const progressia::main::Image &, Vulkan &vulkan);
+    Texture(const main::Image &src, Vulkan &vulkan);
     ~Texture();
 
     void bind();

desktop/graphics/vulkan_mgmt.cpp

@@ -8,25 +8,25 @@ using namespace progressia::main::logging;
 
 namespace progressia::desktop {
 
-Vulkan *vulkan;
+VulkanManager::VulkanManager() {
-
-void initializeVulkan() {
     debug("Vulkan initializing");
 
     // Instance extensions
 
     std::vector<const char *> instanceExtensions;
     {
-        uint32_t glfwExtensionCount;
+        uint32_t glfwExtensionCount = 0;
-        const char **glfwExtensions;
+        const char **glfwExtensions =
-        glfwExtensions = glfwGetRequiredInstanceExtensions(&glfwExtensionCount);
+            glfwGetRequiredInstanceExtensions(&glfwExtensionCount);
 
+        instanceExtensions.reserve(instanceExtensions.size() +
+                                   glfwExtensionCount);
         for (std::size_t i = 0; i < glfwExtensionCount; i++) {
-            instanceExtensions.push_back(glfwExtensions[i]);
+            instanceExtensions.emplace_back(glfwExtensions[i]);
         }
 
 #ifdef VULKAN_ERROR_CHECKING
-        instanceExtensions.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
+        instanceExtensions.emplace_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
 #endif
     }
 
@@ -42,28 +42,21 @@ void initializeVulkan() {
 #endif
     };
 
-    vulkan = new Vulkan(instanceExtensions, deviceExtensions, validationLayers);
+    vulkan = std::make_unique<Vulkan>(instanceExtensions, deviceExtensions,
+                                      validationLayers);
 
     debug("Vulkan initialized");
 }
 
-Vulkan *getVulkan() { return vulkan; }
+VulkanManager::~VulkanManager() { debug("Vulkan terminating"); }
 
-bool startRender() { return vulkan->startRender(); }
+Vulkan *VulkanManager::getVulkan() { return vulkan.get(); }
+const Vulkan *VulkanManager::getVulkan() const { return vulkan.get(); }
 
-void endRender() { return vulkan->endRender(); }
+bool VulkanManager::startRender() { return vulkan->startRender(); }
 
-void resizeVulkanSurface() { vulkan->getSwapChain().recreate(); }
+void VulkanManager::endRender() { return vulkan->endRender(); }
 
-void shutdownVulkan() {
+void VulkanManager::resizeSurface() { vulkan->getSwapChain().recreate(); }
-    debug("Vulkan terminating");
-
-    if (vulkan != nullptr) {
-        delete vulkan;
-        vulkan = nullptr;
-    }
-
-    debug("Vulkan terminated");
-}
 
 } // namespace progressia::desktop

desktop/graphics/vulkan_mgmt.h

@@ -4,18 +4,25 @@
 
 namespace progressia::desktop {
 
-void initializeVulkan();
+class VulkanManager {
+
+  private:
+    std::unique_ptr<Vulkan> vulkan;
+
+  public:
+    VulkanManager();
+    ~VulkanManager();
 
     Vulkan *getVulkan();
+    const Vulkan *getVulkan() const;
 
-void resizeVulkanSurface();
+    void resizeSurface();
 
     /*
      * Returns false when the frame should be skipped
      */
     bool startRender();
     void endRender();
-
+};
-void shutdownVulkan();
 
 } // namespace progressia::desktop

desktop/main.cpp

@@ -13,7 +13,6 @@ int main(int argc, char *argv[]) {
     using namespace progressia;
 
     for (int i = 1; i < argc; i++) {
-        // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic): infeasible to avoid in C++17
         char *arg = argv[i];
         if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) {
             std::cout << main::meta::NAME << " " << main::meta::VERSION << "+"
@@ -28,30 +27,29 @@ int main(int argc, char *argv[]) {
            << main::meta::VERSION_NUMBER << ")";
     debug("Debug is enabled");
 
-    desktop::initializeGlfw();
+    auto glfwManager = desktop::makeGlfwManager();
-    desktop::initializeVulkan();
+    desktop::VulkanManager vulkanManager;
-    desktop::showWindow();
+    glfwManager->setOnScreenResize([&]() { vulkanManager.resizeSurface(); });
+    glfwManager->showWindow();
 
-    main::initialize(desktop::getVulkan()->getGint());
+    main::initialize(vulkanManager.getVulkan()->getGint());
 
     info("Loading complete");
-    while (desktop::shouldRun()) {
+    while (glfwManager->shouldRun()) {
-        bool abortFrame = !desktop::startRender();
+        bool abortFrame = !vulkanManager.startRender();
         if (abortFrame) {
             continue;
         }
 
         main::renderTick();
 
-        desktop::endRender();
+        vulkanManager.endRender();
-        desktop::doGlfwRoutine();
+        glfwManager->doGlfwRoutine();
     }
     info("Shutting down");
 
-    desktop::getVulkan()->waitIdle();
+    vulkanManager.getVulkan()->waitIdle();
     main::shutdown();
-    desktop::shutdownVulkan();
-    desktop::shutdownGlfw();
 
     return 0;
 }

main/util.h

@@ -29,6 +29,18 @@
 }
 // clang-format on
 
+// clang-format off
+#define DISABLE_MOVING(CLASS)            \
+    CLASS &operator=(CLASS &&) = delete; \
+    CLASS(CLASS &&) = delete;            \
+// clang-format on
+
+// clang-format off
+#define DISABLE_COPYING(CLASS)                \
+    CLASS &operator=(const CLASS &) = delete; \
+    CLASS(const CLASS &) = delete;            \
+// clang-format on
+
 namespace progressia::main {
 
 struct NonCopyable {

tools/clang-tidy/clang-tidy.yml

@@ -13,7 +13,8 @@ Checks: "-*,\
   -*-avoid-c-arrays,\
   -readability-else-after-return,\
   -readability-named-parameter,\
-  -readability-use-anyofallof"
+  -readability-use-anyofallof,\
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic"
 
 # modernize-use-trailing-return-type
 #   ignore reason: reduces readability
@@ -43,3 +44,6 @@ Checks: "-*,\
 # readability-use-anyofallof
 #   ignore reason: these relatively obscure functions reduce readability by
 #   increasing code complexity and verbosity
+
+# cppcoreguidelines-pro-bounds-pointer-arithmetic
+#   ignore reason: infeasible to avoid in C++17