Fixed uniform buffer alignment

CMakeLists.txt

@@ -19,6 +19,7 @@ add_executable(progressia
     desktop/graphics/vulkan_texture_descriptors.cpp
     desktop/graphics/vulkan_adapter.cpp
     desktop/graphics/vulkan_swap_chain.cpp
+    desktop/graphics/vulkan_physical_device.cpp
 
     main/game.cpp
     main/logging.cpp

desktop/graphics/vulkan_common.cpp

@@ -3,6 +3,7 @@
 #include "../config.h"
 #include "vulkan_adapter.h"
 #include "vulkan_frame.h"
+#include "vulkan_physical_device.h"
 #include "vulkan_pick_device.h"
 #include "vulkan_pipeline.h"
 #include "vulkan_render_pass.h"
@@ -150,31 +151,24 @@ Vulkan::Vulkan(std::vector<const char *> instanceExtensions,
             exit(1);
         }
 
-        std::vector<VkPhysicalDevice> devices(deviceCount);
+        std::vector<VkPhysicalDevice> vkDevices(deviceCount);
-        vkEnumeratePhysicalDevices(instance, &deviceCount, devices.data());
+        vkEnumeratePhysicalDevices(instance, &deviceCount, vkDevices.data());
 
-        std::vector<PhysicalDeviceData> choices;
+        std::vector<PhysicalDevice> choices;
-
+        for (const auto &vkDevice : vkDevices) {
-        for (const auto &device : devices) {
+            choices.push_back(PhysicalDevice(vkDevice));
-            PhysicalDeviceData data = {};
-            data.device = device;
-
-            vkGetPhysicalDeviceProperties(device, &data.properties);
-            vkGetPhysicalDeviceFeatures(device, &data.features);
-
-            choices.push_back(data);
         }
 
         const auto &result =
             pickPhysicalDevice(choices, *this, deviceExtensions);
-        physicalDevice = result.device;
+        physicalDevice = std::make_unique<PhysicalDevice>(result);
     }
 
     /*
      * Setup queues
      */
 
-    queues = std::make_unique<Queues>(physicalDevice, *this);
+    queues = std::make_unique<Queues>(physicalDevice->getVk(), *this);
 
     /*
      * Create logical device
@@ -207,9 +201,9 @@ Vulkan::Vulkan(std::vector<const char *> instanceExtensions,
 
         // Create logical device
 
-        handleVkResult(
+        handleVkResult("Could not create logical device",
-            "Could not create logical device",
+                       vkCreateDevice(physicalDevice->getVk(), &createInfo,
-            vkCreateDevice(physicalDevice, &createInfo, nullptr, &device));
+                                      nullptr, &device));
 
         // Store queue handles
 
@@ -275,13 +269,16 @@ Vulkan::~Vulkan() {
     commandPool.reset();
     vkDestroyDevice(device, nullptr);
     surface.reset();
+    physicalDevice.reset();
     errorHandler.reset();
     vkDestroyInstance(instance, nullptr);
 }
 
 VkInstance Vulkan::getInstance() const { return instance; }
 
-VkPhysicalDevice Vulkan::getPhysicalDevice() const { return physicalDevice; }
+const PhysicalDevice &Vulkan::getPhysicalDevice() const {
+    return *physicalDevice;
+}
 
 VkDevice Vulkan::getDevice() const { return device; }
 
@@ -333,7 +330,8 @@ VkFormat Vulkan::findSupportedFormat(const std::vector<VkFormat> &candidates,
 
     for (VkFormat format : candidates) {
         VkFormatProperties props;
-        vkGetPhysicalDeviceFormatProperties(physicalDevice, format, &props);
+        vkGetPhysicalDeviceFormatProperties(physicalDevice->getVk(), format,
+                                            &props);
 
         if (tiling == VK_IMAGE_TILING_LINEAR &&
             (props.linearTilingFeatures & features) == features) {
@@ -351,8 +349,7 @@ VkFormat Vulkan::findSupportedFormat(const std::vector<VkFormat> &candidates,
 
 uint32_t Vulkan::findMemoryType(uint32_t allowedByDevice,
                                 VkMemoryPropertyFlags desiredProperties) {
-    VkPhysicalDeviceMemoryProperties memProperties;
+    auto memProperties = physicalDevice->getMemory();
-    vkGetPhysicalDeviceMemoryProperties(physicalDevice, &memProperties);
 
     for (uint32_t i = 0; i < memProperties.memoryTypeCount; i++) {
         if (((1 << i) & allowedByDevice) == 0) {

desktop/graphics/vulkan_common.h

@@ -61,6 +61,7 @@ class VkObjectWrapper : private boost::noncopyable {
 constexpr std::size_t MAX_FRAMES_IN_FLIGHT = 2;
 
 class VulkanErrorHandler;
+class PhysicalDevice;
 class Surface;
 class Queue;
 class Queues;
@@ -75,10 +76,10 @@ class Frame;
 class Vulkan : public VkObjectWrapper {
   private:
     VkInstance instance = VK_NULL_HANDLE;
-    VkPhysicalDevice physicalDevice = VK_NULL_HANDLE;
     VkDevice device = VK_NULL_HANDLE;
 
     std::unique_ptr<VulkanErrorHandler> errorHandler;
+    std::unique_ptr<PhysicalDevice> physicalDevice;
     std::unique_ptr<Surface> surface;
     std::unique_ptr<Queues> queues;
     std::unique_ptr<CommandPool> commandPool;
@@ -103,9 +104,9 @@ class Vulkan : public VkObjectWrapper {
     ~Vulkan();
 
     VkInstance getInstance() const;
-    VkPhysicalDevice getPhysicalDevice() const;
     VkDevice getDevice() const;
 
+    const PhysicalDevice &getPhysicalDevice() const;
     Surface &getSurface();
     const Surface &getSurface() const;
     Queues &getQueues();

desktop/graphics/vulkan_physical_device.cpp

@@ -0,0 +1,51 @@
+#include "vulkan_physical_device.h"
+
+namespace progressia {
+namespace desktop {
+
+PhysicalDevice::PhysicalDevice(VkPhysicalDevice vk) : vk(vk) {
+    vkGetPhysicalDeviceProperties(vk, &properties);
+    vkGetPhysicalDeviceFeatures(vk, &features);
+    vkGetPhysicalDeviceMemoryProperties(vk, &memory);
+}
+
+bool PhysicalDevice::isSuitable() const {
+    // Add feature, limit, etc. checks here.
+    // Return false and debug() if problems arise.
+    return true;
+}
+
+VkPhysicalDevice PhysicalDevice::getVk() const { return vk; }
+
+const VkPhysicalDeviceProperties &PhysicalDevice::getProperties() const {
+    return properties;
+}
+
+const VkPhysicalDeviceFeatures &PhysicalDevice::getFeatures() const {
+    return features;
+}
+
+const VkPhysicalDeviceLimits &PhysicalDevice::getLimits() const {
+    return properties.limits;
+}
+
+const VkPhysicalDeviceMemoryProperties &PhysicalDevice::getMemory() const {
+    return memory;
+}
+
+VkPhysicalDeviceType PhysicalDevice::getType() const {
+    return properties.deviceType;
+}
+
+const char *PhysicalDevice::getName() const { return properties.deviceName; }
+
+VkDeviceSize PhysicalDevice::getMinUniformOffset() const {
+    return getLimits().minUniformBufferOffsetAlignment;
+}
+
+uint32_t PhysicalDevice::getMaxTextureSize() const {
+    return getLimits().maxImageDimension2D;
+}
+
+} // namespace desktop
+} // namespace progressia

desktop/graphics/vulkan_physical_device.h

@@ -0,0 +1,35 @@
+#pragma once
+
+#include "vulkan_common.h"
+
+namespace progressia {
+namespace desktop {
+
+class PhysicalDevice {
+
+  private:
+    VkPhysicalDevice vk;
+    VkPhysicalDeviceProperties properties;
+    VkPhysicalDeviceFeatures features;
+    VkPhysicalDeviceMemoryProperties memory;
+
+  public:
+    PhysicalDevice(VkPhysicalDevice vk);
+
+    bool isSuitable() const;
+
+    VkPhysicalDevice getVk() const;
+    const VkPhysicalDeviceProperties &getProperties() const;
+    const VkPhysicalDeviceFeatures &getFeatures() const;
+    const VkPhysicalDeviceLimits &getLimits() const;
+    const VkPhysicalDeviceMemoryProperties &getMemory() const;
+
+    VkPhysicalDeviceType getType() const;
+    const char *getName() const;
+
+    VkDeviceSize getMinUniformOffset() const;
+    uint32_t getMaxTextureSize() const;
+};
+
+} // namespace desktop
+} // namespace progressia

desktop/graphics/vulkan_pick_device.cpp

@@ -29,20 +29,24 @@ bool checkDeviceExtensions(VkPhysicalDevice device,
     return toFind.empty();
 }
 
-bool isDeviceSuitable(const PhysicalDeviceData &data, Vulkan &vulkan,
+bool isDeviceSuitable(const PhysicalDevice &data, Vulkan &vulkan,
                       const std::vector<const char *> &deviceExtensions) {
 
-    if (!Queues(data.device, vulkan).isComplete()) {
+    if (!data.isSuitable()) {
         return false;
     }
 
-    if (!checkDeviceExtensions(data.device, deviceExtensions)) {
+    if (!Queues(data.getVk(), vulkan).isComplete()) {
+        return false;
+    }
+
+    if (!checkDeviceExtensions(data.getVk(), deviceExtensions)) {
         return false;
     }
 
     // Check requires that the swap chain extension is present
     if (!SwapChain::isSwapChainSuitable(
-            SwapChain::querySwapChainSupport(data.device, vulkan))) {
+            SwapChain::querySwapChainSupport(data.getVk(), vulkan))) {
         return false;
     }
 
@@ -51,8 +55,8 @@ bool isDeviceSuitable(const PhysicalDeviceData &data, Vulkan &vulkan,
 
 } // namespace
 
-const PhysicalDeviceData &
+const PhysicalDevice &
-pickPhysicalDevice(std::vector<PhysicalDeviceData> &choices, Vulkan &vulkan,
+pickPhysicalDevice(std::vector<PhysicalDevice> &choices, Vulkan &vulkan,
                    const std::vector<const char *> &deviceExtensions) {
 
     // Remove unsuitable devices
@@ -82,18 +86,16 @@ pickPhysicalDevice(std::vector<PhysicalDeviceData> &choices, Vulkan &vulkan,
                         {"Virtual GPU", +1},
                         {"CPU", -1}};
 
-        auto type = option.properties.deviceType;
+        auto type = option.getType();
-        m << "\n\t- " << opinions[type].description << " "
+        m << "\n\t- " << opinions[type].description << " " << option.getName();
-          << option.properties.deviceName;
 
-        if (opinions[pick->properties.deviceType].value <
+        if (opinions[pick->getType()].value < opinions[type].value) {
-            opinions[type].value) {
             pick = &option;
         }
     }
     m << "\n";
 
-    m << "Picked device " << pick->properties.deviceName;
+    m << "Picked device " << pick->getName();
     return *pick;
 }
 

desktop/graphics/vulkan_pick_device.h

@@ -1,20 +1,15 @@
 #pragma once
 
 #include "vulkan_common.h"
+#include "vulkan_physical_device.h"
 
 #include <vector>
 
 namespace progressia {
 namespace desktop {
 
-struct PhysicalDeviceData {
+const PhysicalDevice &
-    VkPhysicalDevice device;
+pickPhysicalDevice(std::vector<PhysicalDevice> &, Vulkan &,
-    VkPhysicalDeviceProperties properties;
-    VkPhysicalDeviceFeatures features;
-};
-
-const PhysicalDeviceData &
-pickPhysicalDevice(std::vector<PhysicalDeviceData> &, Vulkan &,
                    const std::vector<const char *> &deviceExtensions);
 
 } // namespace desktop

desktop/graphics/vulkan_swap_chain.cpp

@@ -7,6 +7,7 @@
 #include "glfw_mgmt_details.h"
 #include "vulkan_adapter.h"
 #include "vulkan_common.h"
+#include "vulkan_physical_device.h"
 #include "vulkan_render_pass.h"
 
 #include "../../main/logging.h"
@@ -51,7 +52,8 @@ bool SwapChain::isSwapChainSuitable(const SupportDetails &details) {
 }
 
 void SwapChain::create() {
-    auto details = querySwapChainSupport(vulkan.getPhysicalDevice(), vulkan);
+    auto details =
+        querySwapChainSupport(vulkan.getPhysicalDevice().getVk(), vulkan);
     auto surfaceFormat = chooseSurfaceFormat(details.formats);
     auto presentMode = choosePresentMode(details.presentModes, true);
     this->extent = chooseExtent(details.capabilities);
@@ -274,7 +276,8 @@ SwapChain::SwapChain(Vulkan &vulkan)
     : vk(VK_NULL_HANDLE), colorBuffer(nullptr),
       colorBufferViews(), extent{0, 0}, depthBuffer(nullptr), framebuffers(),
       vulkan(vulkan) {
-    auto details = querySwapChainSupport(vulkan.getPhysicalDevice(), vulkan);
+    auto details =
+        querySwapChainSupport(vulkan.getPhysicalDevice().getVk(), vulkan);
     auto surfaceFormat = chooseSurfaceFormat(details.formats);
 
     vulkan.getAdapter().getAttachments().push_back(

desktop/graphics/vulkan_uniform.inl

@@ -5,14 +5,30 @@
 #include "../../main/util.h"
 #include "vulkan_frame.h"
 #include "vulkan_pipeline.h"
+#include "vulkan_physical_device.h"
 
 namespace progressia {
 namespace desktop {
 
+namespace detail {
+
+    template <typename T>
+    std::size_t offsetOf(Vulkan &vulkan) {
+        auto step = vulkan.getPhysicalDevice().getMinUniformOffset();
+        return ((sizeof(T) - 1) / step + 1) * step; // Round up to multiple
+    }
+
+    template <typename T>
+    std::size_t offsetOf(Vulkan &vulkan, const T&) {
+        return offsetOf<T>(vulkan);
+    }
+
+}
+
 template <typename... Entries>
 Uniform<Entries...>::StateImpl::Set::Set(VkDescriptorSet vk, Vulkan &vulkan)
     : vk(vk),
-      contents((sizeof(Entries) + ...), VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT,
+      contents((detail::offsetOf<Entries>(vulkan) + ...), VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT,
                VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT |
                    VK_MEMORY_PROPERTY_HOST_COHERENT_BIT,
                vulkan) {}
@@ -48,7 +64,7 @@ Uniform<Entries...>::StateImpl::StateImpl(
             writes[index].descriptorCount = 1;
             writes[index].pBufferInfo = &bufferInfos[index];
 
-            offset += sizeof(Entry);
+            offset += detail::offsetOf<Entry>(vulkan);
             index++;
         })
     }
@@ -71,7 +87,7 @@ void Uniform<Entries...>::State::update(const Entries &...entries) {
     auto *dst = state.newContents.data();
     FOR_PACK(Entries, entries, e, {
         std::memcpy(dst, &e, sizeof(e));
-        dst += sizeof(e);
+        dst += detail::offsetOf(uniform->getVulkan(), e);
     })
     state.setsToUpdate = state.sets.size();
 }

tools/cppcheck/suppressions.txt

@@ -7,6 +7,9 @@ noExplicitConstructor:*
 # In most cases using STL algorithm functions causes unnecessary code bloat.
 useStlAlgorithm:*
 
+# Non-static non-virtual stubs are often useful for establishing code structure.
+functionStatic:*
+
 # cppcheck trips on #include <embedded_resources.h> and there's no way to
 #   suppress that exlusively
 missingInclude:*