From c5233a6bf0eb48ed1c4270e8981765f95b4c05a8 Mon Sep 17 00:00:00 2001 From: OLEGSHA Date: Mon, 7 Nov 2022 11:33:19 +0300 Subject: [PATCH] Fixed uniform buffer alignment --- CMakeLists.txt | 1 + desktop/graphics/vulkan_common.cpp | 39 ++++++++-------- desktop/graphics/vulkan_common.h | 5 +- desktop/graphics/vulkan_physical_device.cpp | 51 +++++++++++++++++++++ desktop/graphics/vulkan_physical_device.h | 35 ++++++++++++++ desktop/graphics/vulkan_pick_device.cpp | 26 ++++++----- desktop/graphics/vulkan_pick_device.h | 11 ++--- desktop/graphics/vulkan_swap_chain.cpp | 7 ++- desktop/graphics/vulkan_uniform.inl | 22 +++++++-- tools/cppcheck/suppressions.txt | 3 ++ 10 files changed, 152 insertions(+), 48 deletions(-) create mode 100644 desktop/graphics/vulkan_physical_device.cpp create mode 100644 desktop/graphics/vulkan_physical_device.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 273763f..2ea62d6 100644 --- a/CMakeLists.txt +++ b/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 diff --git a/desktop/graphics/vulkan_common.cpp b/desktop/graphics/vulkan_common.cpp index 2658d4a..82c2375 100644 --- a/desktop/graphics/vulkan_common.cpp +++ b/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 instanceExtensions, exit(1); } - std::vector devices(deviceCount); - vkEnumeratePhysicalDevices(instance, &deviceCount, devices.data()); + std::vector vkDevices(deviceCount); + vkEnumeratePhysicalDevices(instance, &deviceCount, vkDevices.data()); - std::vector choices; - - for (const auto &device : devices) { - PhysicalDeviceData data = {}; - data.device = device; - - vkGetPhysicalDeviceProperties(device, &data.properties); - vkGetPhysicalDeviceFeatures(device, &data.features); - - choices.push_back(data); + std::vector choices; + for (const auto &vkDevice : vkDevices) { + choices.push_back(PhysicalDevice(vkDevice)); } const auto &result = pickPhysicalDevice(choices, *this, deviceExtensions); - physicalDevice = result.device; + physicalDevice = std::make_unique(result); } /* * Setup queues */ - queues = std::make_unique(physicalDevice, *this); + queues = std::make_unique(physicalDevice->getVk(), *this); /* * Create logical device @@ -207,9 +201,9 @@ Vulkan::Vulkan(std::vector instanceExtensions, // Create logical device - handleVkResult( - "Could not create logical device", - vkCreateDevice(physicalDevice, &createInfo, nullptr, &device)); + handleVkResult("Could not create logical device", + vkCreateDevice(physicalDevice->getVk(), &createInfo, + 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 &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 &candidates, uint32_t Vulkan::findMemoryType(uint32_t allowedByDevice, VkMemoryPropertyFlags desiredProperties) { - VkPhysicalDeviceMemoryProperties memProperties; - vkGetPhysicalDeviceMemoryProperties(physicalDevice, &memProperties); + auto memProperties = physicalDevice->getMemory(); for (uint32_t i = 0; i < memProperties.memoryTypeCount; i++) { if (((1 << i) & allowedByDevice) == 0) { diff --git a/desktop/graphics/vulkan_common.h b/desktop/graphics/vulkan_common.h index acdfbc2..1921b06 100644 --- a/desktop/graphics/vulkan_common.h +++ b/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 errorHandler; + std::unique_ptr physicalDevice; std::unique_ptr surface; std::unique_ptr queues; std::unique_ptr 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(); diff --git a/desktop/graphics/vulkan_physical_device.cpp b/desktop/graphics/vulkan_physical_device.cpp new file mode 100644 index 0000000..43676a9 --- /dev/null +++ b/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 diff --git a/desktop/graphics/vulkan_physical_device.h b/desktop/graphics/vulkan_physical_device.h new file mode 100644 index 0000000..c229fb1 --- /dev/null +++ b/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 diff --git a/desktop/graphics/vulkan_pick_device.cpp b/desktop/graphics/vulkan_pick_device.cpp index 21c19e5..fe20109 100644 --- a/desktop/graphics/vulkan_pick_device.cpp +++ b/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 &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 & -pickPhysicalDevice(std::vector &choices, Vulkan &vulkan, +const PhysicalDevice & +pickPhysicalDevice(std::vector &choices, Vulkan &vulkan, const std::vector &deviceExtensions) { // Remove unsuitable devices @@ -82,18 +86,16 @@ pickPhysicalDevice(std::vector &choices, Vulkan &vulkan, {"Virtual GPU", +1}, {"CPU", -1}}; - auto type = option.properties.deviceType; - m << "\n\t- " << opinions[type].description << " " - << option.properties.deviceName; + auto type = option.getType(); + m << "\n\t- " << opinions[type].description << " " << option.getName(); - if (opinions[pick->properties.deviceType].value < - opinions[type].value) { + if (opinions[pick->getType()].value < opinions[type].value) { pick = &option; } } m << "\n"; - m << "Picked device " << pick->properties.deviceName; + m << "Picked device " << pick->getName(); return *pick; } diff --git a/desktop/graphics/vulkan_pick_device.h b/desktop/graphics/vulkan_pick_device.h index 2e7998f..63d21ff 100644 --- a/desktop/graphics/vulkan_pick_device.h +++ b/desktop/graphics/vulkan_pick_device.h @@ -1,20 +1,15 @@ #pragma once #include "vulkan_common.h" +#include "vulkan_physical_device.h" #include namespace progressia { namespace desktop { -struct PhysicalDeviceData { - VkPhysicalDevice device; - VkPhysicalDeviceProperties properties; - VkPhysicalDeviceFeatures features; -}; - -const PhysicalDeviceData & -pickPhysicalDevice(std::vector &, Vulkan &, +const PhysicalDevice & +pickPhysicalDevice(std::vector &, Vulkan &, const std::vector &deviceExtensions); } // namespace desktop diff --git a/desktop/graphics/vulkan_swap_chain.cpp b/desktop/graphics/vulkan_swap_chain.cpp index 5ca1ee3..47d8bb0 100644 --- a/desktop/graphics/vulkan_swap_chain.cpp +++ b/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( diff --git a/desktop/graphics/vulkan_uniform.inl b/desktop/graphics/vulkan_uniform.inl index 79e99b6..46f884b 100644 --- a/desktop/graphics/vulkan_uniform.inl +++ b/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 + std::size_t offsetOf(Vulkan &vulkan) { + auto step = vulkan.getPhysicalDevice().getMinUniformOffset(); + return ((sizeof(T) - 1) / step + 1) * step; // Round up to multiple + } + + template + std::size_t offsetOf(Vulkan &vulkan, const T&) { + return offsetOf(vulkan); + } + +} + template Uniform::StateImpl::Set::Set(VkDescriptorSet vk, Vulkan &vulkan) : vk(vk), - contents((sizeof(Entries) + ...), VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, + contents((detail::offsetOf(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::StateImpl::StateImpl( writes[index].descriptorCount = 1; writes[index].pBufferInfo = &bufferInfos[index]; - offset += sizeof(Entry); + offset += detail::offsetOf(vulkan); index++; }) } @@ -71,7 +87,7 @@ void Uniform::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(); } diff --git a/tools/cppcheck/suppressions.txt b/tools/cppcheck/suppressions.txt index 5506e95..25b94a4 100644 --- a/tools/cppcheck/suppressions.txt +++ b/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 and there's no way to # suppress that exlusively missingInclude:*