Unify vkb::allocated::Allocated and vkb::allocated::HPPAllocated into vkb::allocated::Allocated<bindingType>

framework/CMakeLists.txt

@@ -251,7 +251,6 @@ set(CORE_FILES
     core/render_pass.h
     core/query_pool.h
     core/acceleration_structure.h
-    core/hpp_allocated.h
     core/hpp_command_buffer.h
     core/hpp_command_pool.h
     core/hpp_debug.h

framework/builder_base.h

@@ -24,6 +24,54 @@
 
 namespace vkb
 {
+
+namespace allocated
+{
+
+/**
+ * @brief Many Vulkan resource types (most notably Images and to a lesser extent Buffers)
+ * and and their corresponding memory allocations have many parameters that need to be setup
+ * when creating them.  Although many of these have reasonable defaults, constructors with
+ * numerous arguments, some or all of which may have default arguments, aren't well suited
+ * to partial customization.  This is a common failing of languages that don't support named
+ * arguments and has led to the common use of the [builder pattern](https://en.wikipedia.org/wiki/Builder_pattern),
+ * where a helper class is used to store all the options that can be tweaked for an object
+ * when it's created. A builder class will have reasonable defaults where appropriate and only
+ * require arguments for the builder constructor when a value is always required for creation to occur
+ * (for example, the size of a buffer or the extent of an image).  Remaining parameters can be set
+ * with methods on the builder class, which return a reference to the builder object, allowing
+ * chaining of the method calls.
+ *
+ * This builder class serves as a base containing options that are common to all
+ * [VMA](https://gpuopen.com/vulkan-memory-allocator/) allocated and managed resources.
+ * For instance, the VMA create and usage flags are set here, but the image or buffer
+ * usage flags are handled in the derived builder classes specific to those types.
+ *
+ * The following is an example of how the builder pattern is used in the codebase:
+ ```cpp
+        vkb::core::ImageBuilder(VkExtent3D{grid_width, grid_height, 1})
+            .with_format(VK_FORMAT_R8G8B8A8_UNORM)
+            .with_usage(VK_IMAGE_USAGE_STORAGE_BIT | VK_IMAGE_USAGE_SAMPLED_BIT)
+            .with_vma_usage(VMA_MEMORY_USAGE_GPU_ONLY)
+            .with_sample_count(VK_SAMPLE_COUNT_1_BIT)
+            .with_mip_levels(1)
+            .with_array_layers(1)
+            .with_tiling(VK_IMAGE_TILING_OPTIMAL)
+            .with_queue_families(static_cast<uint32_t>(queue_families.size()), queue_families.data())
+            .with_sharing_mode(sharing_mode));
+ ```
+ * The actual image can be created with `build()` which returns a `vkb::core::Image` or `buildPtr` which returns a `std::unique_ptr<vkb::core::Image>`.
+ * Alternatively, the builder can be used as an argument to the `Image` constructor, which will build the image for you in place.
+ * @note The builder pattern is intended to displace the currently used `vkb::core::Image` and `vkb::core::Buffer` constructors with numerous
+ * arguments, but this is a work in progress and not currently in wide use in the codebase.
+ *
+ * @tparam BuilderType Allow the same builder base class to be used
+ * with a variety of subclasses while using casting to return the corect dervied type
+ * from the modifier methods.
+ * @tparam bindingType A flag indicating whether this is being used with the C or C++ API
+ * @tparam CreateInfoType The type of the Vulkan create info structure. Either a `VkSomethingCreateInfo`
+ * or `vk::SomethingCreateInfo` for the C or C++ API respectively.
+ */
 template <vkb::BindingType bindingType, typename BuilderType, typename CreateInfoType>
 class BuilderBase
 {
@@ -211,4 +259,5 @@ inline BuilderType &BuilderBase<bindingType, BuilderType, CreateInfoType>::with_
 	return *static_cast<BuilderType *>(this);
 }
 
+}        // namespace allocated
 }        // namespace vkb

framework/core/allocated.cpp

@@ -17,6 +17,7 @@
  */
 
 #include "allocated.h"
+#include "common/error.h"
 
 namespace vkb
 {
@@ -32,10 +33,14 @@ VmaAllocator &get_memory_allocator()
 
 void init(const VmaAllocatorCreateInfo &create_info)
 {
-	VkResult result = vmaCreateAllocator(&create_info, &get_memory_allocator());
-	if (result != VK_SUCCESS)
+	auto &allocator = get_memory_allocator();
+	if (allocator == VK_NULL_HANDLE)
 	{
-		throw VulkanException{result, "Cannot create allocator"};
+		VkResult result = vmaCreateAllocator(&create_info, &allocator);
+		if (result != VK_SUCCESS)
+		{
+			throw VulkanException{result, "Cannot create allocator"};
+		}
 	}
 }
 
@@ -52,182 +57,5 @@ void shutdown()
 	}
 }
 
-AllocatedBase::AllocatedBase(const VmaAllocationCreateInfo &alloc_create_info) :
-    alloc_create_info(alloc_create_info)
-{
-}
-
-AllocatedBase::AllocatedBase(AllocatedBase &&other) noexcept :
-    alloc_create_info(std::exchange(other.alloc_create_info, {})),
-    allocation(std::exchange(other.allocation, {})),
-    mapped_data(std::exchange(other.mapped_data, {})),
-    coherent(std::exchange(other.coherent, {})),
-    persistent(std::exchange(other.persistent, {}))
-{
-}
-
-const uint8_t *AllocatedBase::get_data() const
-{
-	return mapped_data;
-}
-
-VkDeviceMemory AllocatedBase::get_memory() const
-{
-	VmaAllocationInfo alloc_info;
-	vmaGetAllocationInfo(get_memory_allocator(), allocation, &alloc_info);
-	return alloc_info.deviceMemory;
-}
-
-void AllocatedBase::flush(VkDeviceSize offset, VkDeviceSize size)
-{
-	if (!coherent)
-	{
-		vmaFlushAllocation(get_memory_allocator(), allocation, offset, size);
-	}
-}
-
-uint8_t *AllocatedBase::map()
-{
-	if (!persistent && !mapped())
-	{
-		VK_CHECK(vmaMapMemory(get_memory_allocator(), allocation, reinterpret_cast<void **>(&mapped_data)));
-		assert(mapped_data);
-	}
-	return mapped_data;
-}
-
-void AllocatedBase::unmap()
-{
-	if (!persistent && mapped())
-	{
-		vmaUnmapMemory(get_memory_allocator(), allocation);
-		mapped_data = nullptr;
-	}
-}
-
-size_t AllocatedBase::update(const uint8_t *data, size_t size, size_t offset)
-{
-	if (persistent)
-	{
-		std::copy(data, data + size, mapped_data + offset);
-		flush();
-	}
-	else
-	{
-		map();
-		std::copy(data, data + size, mapped_data + offset);
-		flush();
-		unmap();
-	}
-	return size;
-}
-
-size_t AllocatedBase::update(void const *data, size_t size, size_t offset)
-{
-	return update(reinterpret_cast<const uint8_t *>(data), size, offset);
-}
-
-void AllocatedBase::post_create(VmaAllocationInfo const &allocation_info)
-{
-	VkMemoryPropertyFlags memory_properties;
-	vmaGetAllocationMemoryProperties(get_memory_allocator(), allocation, &memory_properties);
-	coherent    = (memory_properties & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) == VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
-	mapped_data = static_cast<uint8_t *>(allocation_info.pMappedData);
-	persistent  = mapped();
-}
-
-[[nodiscard]] VkBuffer AllocatedBase::create_buffer(VkBufferCreateInfo const &create_info)
-{
-	VkBuffer          handleResult = VK_NULL_HANDLE;
-	VmaAllocationInfo allocation_info{};
-
-	auto result = vmaCreateBuffer(
-	    get_memory_allocator(),
-	    &create_info,
-	    &alloc_create_info,
-	    &handleResult,
-	    &allocation,
-	    &allocation_info);
-
-	if (result != VK_SUCCESS)
-	{
-		throw VulkanException{result, "Cannot create Buffer"};
-	}
-	post_create(allocation_info);
-	return handleResult;
-}
-
-[[nodiscard]] VkImage AllocatedBase::create_image(VkImageCreateInfo const &create_info)
-{
-	assert(0 < create_info.mipLevels && "Images should have at least one level");
-	assert(0 < create_info.arrayLayers && "Images should have at least one layer");
-	assert(0 < create_info.usage && "Images should have at least one usage type");
-
-	VkImage           handleResult = VK_NULL_HANDLE;
-	VmaAllocationInfo allocation_info{};
-
-#if 0
-		// If the image is an attachment, prefer dedicated memory
-		constexpr VkImageUsageFlags attachment_only_flags = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSIENT_ATTACHMENT_BIT;
-		if (create_info.usage & attachment_only_flags)
-		{
-			alloc_create_info.flags |= VMA_ALLOCATION_CREATE_DEDICATED_MEMORY_BIT;
-		}
-
-		if (create_info.usage & VK_IMAGE_USAGE_TRANSIENT_ATTACHMENT_BIT)
-		{
-			alloc_create_info.preferredFlags |= VK_MEMORY_PROPERTY_LAZILY_ALLOCATED_BIT;
-		}
-#endif
-
-	auto result = vmaCreateImage(
-	    get_memory_allocator(),
-	    &create_info,
-	    &alloc_create_info,
-	    &handleResult,
-	    &allocation,
-	    &allocation_info);
-
-	if (result != VK_SUCCESS)
-	{
-		throw VulkanException{result, "Cannot create Image"};
-	}
-
-	post_create(allocation_info);
-	return handleResult;
-}
-
-void AllocatedBase::destroy_buffer(VkBuffer handle)
-{
-	if (handle != VK_NULL_HANDLE && allocation != VK_NULL_HANDLE)
-	{
-		unmap();
-		vmaDestroyBuffer(get_memory_allocator(), handle, allocation);
-		clear();
-	}
-}
-
-void AllocatedBase::destroy_image(VkImage image)
-{
-	if (image != VK_NULL_HANDLE && allocation != VK_NULL_HANDLE)
-	{
-		unmap();
-		vmaDestroyImage(get_memory_allocator(), image, allocation);
-		clear();
-	}
-}
-
-bool AllocatedBase::mapped() const
-{
-	return mapped_data != nullptr;
-}
-
-void AllocatedBase::clear()
-{
-	mapped_data       = nullptr;
-	persistent        = false;
-	alloc_create_info = {};
-}
-
 }        // namespace allocated
 }        // namespace vkb