From 5d30b347a9980e030886310105899da654a23afc Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sun, 23 Jul 2023 04:26:30 +0200 Subject: [PATCH] vulkan: Get rid of GskVulkanMemory Make buffers and images use GskVulkanAllocation instead and use the allocator directly. Also simplify the map/unmap into a simple get_data() call now that we know we can keep everything mapped forever. While doing this, pass alignment requirements to the allocator. And copy/paste the memory type selection code from the Vulkan documentation, so we do the same thing everybody else does. --- gsk/vulkan/gskvulkanbuffer.c | 38 +++++++------- gsk/vulkan/gskvulkanbufferprivate.h | 4 +- gsk/vulkan/gskvulkandownloadop.c | 3 +- gsk/vulkan/gskvulkanimage.c | 47 ++++++++--------- gsk/vulkan/gskvulkanimageprivate.h | 3 +- gsk/vulkan/gskvulkanmemory.c | 81 +++++++---------------------- gsk/vulkan/gskvulkanmemoryprivate.h | 28 +++++----- gsk/vulkan/gskvulkanrender.c | 6 +-- gsk/vulkan/gskvulkanuploadop.c | 7 +-- 9 files changed, 79 insertions(+), 138 deletions(-) diff --git a/gsk/vulkan/gskvulkanbuffer.c b/gsk/vulkan/gskvulkanbuffer.c index 5645ee42e4..3b757f91ea 100644 --- a/gsk/vulkan/gskvulkanbuffer.c +++ b/gsk/vulkan/gskvulkanbuffer.c @@ -9,11 +9,10 @@ struct _GskVulkanBuffer { GdkVulkanContext *vulkan; - gsize size; - VkBuffer vk_buffer; - GskVulkanMemory *memory; + GskVulkanAllocator *allocator; + GskVulkanAllocation allocation; }; static GskVulkanBuffer * @@ -27,7 +26,6 @@ gsk_vulkan_buffer_new_internal (GdkVulkanContext *context, self = g_new0 (GskVulkanBuffer, 1); self->vulkan = g_object_ref (context); - self->size = size; GSK_VK_CHECK (vkCreateBuffer, gdk_vulkan_context_get_device (context), &(VkBufferCreateInfo) { @@ -43,16 +41,21 @@ gsk_vulkan_buffer_new_internal (GdkVulkanContext *context, vkGetBufferMemoryRequirements (gdk_vulkan_context_get_device (context), self->vk_buffer, &requirements); - - self->memory = gsk_vulkan_memory_new (context, - requirements.memoryTypeBits, - GSK_VULKAN_MEMORY_MAPPABLE, - requirements.size); + + self->allocator = gsk_vulkan_find_allocator (context, + requirements.memoryTypeBits, + GSK_VULKAN_MEMORY_MAPPABLE, + GSK_VULKAN_MEMORY_MAPPABLE); + gsk_vulkan_alloc (self->allocator, + requirements.size, + requirements.alignment, + &self->allocation); GSK_VK_CHECK (vkBindBufferMemory, gdk_vulkan_context_get_device (context), self->vk_buffer, - gsk_vulkan_memory_get_device_memory (self->memory), - 0); + self->allocation.vk_memory, + self->allocation.offset); + return self; } @@ -90,7 +93,7 @@ gsk_vulkan_buffer_free (GskVulkanBuffer *self) self->vk_buffer, NULL); - gsk_vulkan_memory_free (self->memory); + gsk_vulkan_free (self->allocator, &self->allocation); g_object_unref (self->vulkan); @@ -106,17 +109,12 @@ gsk_vulkan_buffer_get_buffer (GskVulkanBuffer *self) gsize gsk_vulkan_buffer_get_size (GskVulkanBuffer *self) { - return self->size; + return self->allocation.size; } guchar * -gsk_vulkan_buffer_map (GskVulkanBuffer *self) +gsk_vulkan_buffer_get_data (GskVulkanBuffer *self) { - return gsk_vulkan_memory_map (self->memory); + return self->allocation.map; } -void -gsk_vulkan_buffer_unmap (GskVulkanBuffer *self) -{ - gsk_vulkan_memory_unmap (self->memory); -} diff --git a/gsk/vulkan/gskvulkanbufferprivate.h b/gsk/vulkan/gskvulkanbufferprivate.h index 009e79da52..6f9cb392d7 100644 --- a/gsk/vulkan/gskvulkanbufferprivate.h +++ b/gsk/vulkan/gskvulkanbufferprivate.h @@ -24,9 +24,7 @@ void gsk_vulkan_buffer_free (GskVulk VkBuffer gsk_vulkan_buffer_get_buffer (GskVulkanBuffer *self); gsize gsk_vulkan_buffer_get_size (GskVulkanBuffer *self); - -guchar * gsk_vulkan_buffer_map (GskVulkanBuffer *self); -void gsk_vulkan_buffer_unmap (GskVulkanBuffer *self); +guchar * gsk_vulkan_buffer_get_data (GskVulkanBuffer *self); G_END_DECLS diff --git a/gsk/vulkan/gskvulkandownloadop.c b/gsk/vulkan/gskvulkandownloadop.c index 5d3aa9f7b7..b460972fb7 100644 --- a/gsk/vulkan/gskvulkandownloadop.c +++ b/gsk/vulkan/gskvulkandownloadop.c @@ -115,7 +115,7 @@ gsk_vulkan_download_op_finish (GskVulkanOp *op) guchar *data; gsize stride; - data = gsk_vulkan_buffer_map (self->buffer); + data = gsk_vulkan_buffer_get_data (self->buffer); stride = gsk_vulkan_image_get_width (self->image) * gdk_memory_format_bytes_per_pixel (gsk_vulkan_image_get_format (self->image)); self->func (self->user_data, @@ -124,7 +124,6 @@ gsk_vulkan_download_op_finish (GskVulkanOp *op) gsk_vulkan_image_get_width (self->image), gsk_vulkan_image_get_height (self->image), stride); - gsk_vulkan_buffer_unmap (self->buffer); g_object_unref (self->image); g_clear_pointer (&self->buffer, gsk_vulkan_buffer_free); diff --git a/gsk/vulkan/gskvulkanimage.c b/gsk/vulkan/gskvulkanimage.c index 1b92caec67..c137186a8d 100644 --- a/gsk/vulkan/gskvulkanimage.c +++ b/gsk/vulkan/gskvulkanimage.c @@ -31,7 +31,8 @@ struct _GskVulkanImage VkImageLayout vk_image_layout; VkAccessFlags vk_access; - GskVulkanMemory *memory; + GskVulkanAllocator *allocator; + GskVulkanAllocation allocation; }; G_DEFINE_TYPE (GskVulkanImage, gsk_vulkan_image, G_TYPE_OBJECT) @@ -547,15 +548,19 @@ gsk_vulkan_image_new (GdkVulkanContext *context, self->vk_image, &requirements); - self->memory = gsk_vulkan_memory_new (context, - requirements.memoryTypeBits, - memory, - requirements.size); + self->allocator = gsk_vulkan_find_allocator (context, + requirements.memoryTypeBits, + 0, + GSK_VULKAN_MEMORY_MAPPABLE); + gsk_vulkan_alloc (self->allocator, + requirements.size, + requirements.alignment, + &self->allocation); GSK_VK_CHECK (vkBindImageMemory, gdk_vulkan_context_get_device (context), self->vk_image, - gsk_vulkan_memory_get_device_memory (self->memory), - 0); + self->allocation.vk_memory, + self->allocation.offset); gsk_vulkan_image_create_view (self, vk_format); @@ -599,24 +604,19 @@ gsk_vulkan_image_can_map (GskVulkanImage *self) self->vk_image_layout != VK_IMAGE_LAYOUT_GENERAL) return FALSE; - return gsk_vulkan_memory_can_map (self->memory, TRUE); + return self->allocation.map != NULL; } guchar * -gsk_vulkan_image_try_map (GskVulkanImage *self, - gsize *out_stride) +gsk_vulkan_image_get_data (GskVulkanImage *self, + gsize *out_stride) { VkImageSubresource image_res; VkSubresourceLayout image_layout; - guchar *result; if (!gsk_vulkan_image_can_map (self)) return NULL; - result = gsk_vulkan_memory_map (self->memory); - if (result == NULL) - return NULL; - image_res.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; image_res.mipLevel = 0; image_res.arrayLayer = 0; @@ -626,13 +626,7 @@ gsk_vulkan_image_try_map (GskVulkanImage *self, *out_stride = image_layout.rowPitch; - return result + image_layout.offset; -} - -void -gsk_vulkan_image_unmap (GskVulkanImage *self) -{ - gsk_vulkan_memory_unmap (self->memory); + return self->allocation.map + image_layout.offset; } GskVulkanImage * @@ -732,10 +726,11 @@ gsk_vulkan_image_finalize (GObject *object) /* memory is NULL for for_swapchain() images, where we don't own * the VkImage */ - if (self->memory) - vkDestroyImage (device, self->vk_image, NULL); - - g_clear_pointer (&self->memory, gsk_vulkan_memory_free); + if (self->allocator) + { + vkDestroyImage (device, self->vk_image, NULL); + gsk_vulkan_free (self->allocator, &self->allocation); + } g_object_unref (self->vulkan); diff --git a/gsk/vulkan/gskvulkanimageprivate.h b/gsk/vulkan/gskvulkanimageprivate.h index 5e915fc2a0..a097bfb98b 100644 --- a/gsk/vulkan/gskvulkanimageprivate.h +++ b/gsk/vulkan/gskvulkanimageprivate.h @@ -36,9 +36,8 @@ GskVulkanImage * gsk_vulkan_image_new_for_upload (GdkVulk GdkMemoryFormat format, gsize width, gsize height); -guchar * gsk_vulkan_image_try_map (GskVulkanImage *self, +guchar * gsk_vulkan_image_get_data (GskVulkanImage *self, gsize *out_stride); -void gsk_vulkan_image_unmap (GskVulkanImage *self); gsize gsk_vulkan_image_get_width (GskVulkanImage *self); gsize gsk_vulkan_image_get_height (GskVulkanImage *self); diff --git a/gsk/vulkan/gskvulkanmemory.c b/gsk/vulkan/gskvulkanmemory.c index 74d1cfed4a..e837103e2b 100644 --- a/gsk/vulkan/gskvulkanmemory.c +++ b/gsk/vulkan/gskvulkanmemory.c @@ -5,17 +5,7 @@ #include "gskvulkanprivate.h" -struct _GskVulkanMemory -{ - GdkVulkanContext *vulkan; - - gsize size; - - GskVulkanAllocator *allocator; - GskVulkanAllocation allocation; -}; - -static GskVulkanAllocator * +GskVulkanAllocator * gsk_vulkan_allocator_get (GdkVulkanContext *context, gsize index, const VkMemoryType *type) @@ -41,19 +31,16 @@ gsk_vulkan_allocator_get (GdkVulkanContext *context, return allocators[index]; } -GskVulkanMemory * -gsk_vulkan_memory_new (GdkVulkanContext *context, - uint32_t allowed_types, - VkMemoryPropertyFlags flags, - gsize size) +/* following code found in + * https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkPhysicalDeviceMemoryProperties.html */ +GskVulkanAllocator * +gsk_vulkan_find_allocator (GdkVulkanContext *context, + uint32_t allowed_types, + VkMemoryPropertyFlags required_flags, + VkMemoryPropertyFlags desired_flags) { VkPhysicalDeviceMemoryProperties properties; - GskVulkanMemory *self; - uint32_t i; - - self = g_new0 (GskVulkanMemory, 1); - - self->size = size; + uint32_t i, found; vkGetPhysicalDeviceMemoryProperties (gdk_vulkan_context_get_physical_device (context), &properties); @@ -63,48 +50,18 @@ gsk_vulkan_memory_new (GdkVulkanContext *context, if (!(allowed_types & (1 << i))) continue; - if ((properties.memoryTypes[i].propertyFlags & flags) == flags) + if ((properties.memoryTypes[i].propertyFlags & required_flags) != required_flags) + continue; + + found = MIN (i, found); + + if ((properties.memoryTypes[i].propertyFlags & desired_flags) == desired_flags) break; } - g_assert (i < properties.memoryTypeCount); + g_assert (found < properties.memoryTypeCount); - self->allocator = gsk_vulkan_allocator_get (context, i, &properties.memoryTypes[i]); - gsk_vulkan_alloc (self->allocator, size, &self->allocation); - - return self; -} - -void -gsk_vulkan_memory_free (GskVulkanMemory *self) -{ - gsk_vulkan_free (self->allocator, &self->allocation); - - g_free (self); -} - -VkDeviceMemory -gsk_vulkan_memory_get_device_memory (GskVulkanMemory *self) -{ - return self->allocation.vk_memory; -} - -gboolean -gsk_vulkan_memory_can_map (GskVulkanMemory *self, - gboolean fast) -{ - return self->allocation.map != NULL; -} - -guchar * -gsk_vulkan_memory_map (GskVulkanMemory *self) -{ - return self->allocation.map; -} - -void -gsk_vulkan_memory_unmap (GskVulkanMemory *self) -{ + return gsk_vulkan_allocator_get (context, i, &properties.memoryTypes[i]); } /* {{{ direct allocator ***/ @@ -129,6 +86,7 @@ gsk_vulkan_direct_allocator_free_allocator (GskVulkanAllocator *allocator) static void gsk_vulkan_direct_allocator_alloc (GskVulkanAllocator *allocator, VkDeviceSize size, + VkDeviceSize alignment, GskVulkanAllocation *alloc) { GskVulkanDirectAllocator *self = (GskVulkanDirectAllocator *) allocator; @@ -236,11 +194,12 @@ gsk_vulkan_stats_allocator_free_allocator (GskVulkanAllocator *allocator) static void gsk_vulkan_stats_allocator_alloc (GskVulkanAllocator *allocator, VkDeviceSize size, + VkDeviceSize align, GskVulkanAllocation *alloc) { GskVulkanStatsAllocator *self = (GskVulkanStatsAllocator *) allocator; - gsk_vulkan_alloc (self->allocator, size, alloc); + gsk_vulkan_alloc (self->allocator, size, align, alloc); self->n_alloc++; self->n_bytes_requested += size; diff --git a/gsk/vulkan/gskvulkanmemoryprivate.h b/gsk/vulkan/gskvulkanmemoryprivate.h index 2bb319de51..edb9ad4c39 100644 --- a/gsk/vulkan/gskvulkanmemoryprivate.h +++ b/gsk/vulkan/gskvulkanmemoryprivate.h @@ -26,43 +26,41 @@ struct _GskVulkanAllocator void (* alloc) (GskVulkanAllocator *allocator, VkDeviceSize size, + VkDeviceSize alignment, GskVulkanAllocation *out_alloc); void (* free) (GskVulkanAllocator *allocator, const GskVulkanAllocation *alloc); }; static inline void gsk_vulkan_alloc (GskVulkanAllocator *allocator, - gsize size, + VkDeviceSize size, + VkDeviceSize alignment, GskVulkanAllocation *out_alloc); static inline void gsk_vulkan_free (GskVulkanAllocator *allocator, const GskVulkanAllocation *alloc); static inline void gsk_vulkan_allocator_free (GskVulkanAllocator *allocator); +GskVulkanAllocator * gsk_vulkan_allocator_get (GdkVulkanContext *context, + gsize index, + const VkMemoryType *type); +GskVulkanAllocator * gsk_vulkan_find_allocator (GdkVulkanContext *context, + uint32_t allowed_types, + VkMemoryPropertyFlags required_flags, + VkMemoryPropertyFlags desired_flags); + GskVulkanAllocator * gsk_vulkan_direct_allocator_new (VkDevice device, uint32_t vk_type_index, const VkMemoryType *vk_type); GskVulkanAllocator * gsk_vulkan_stats_allocator_new (GskVulkanAllocator *allocator); -GskVulkanMemory * gsk_vulkan_memory_new (GdkVulkanContext *context, - uint32_t allowed_types, - VkMemoryPropertyFlags properties, - gsize size); -void gsk_vulkan_memory_free (GskVulkanMemory *memory); - -VkDeviceMemory gsk_vulkan_memory_get_device_memory (GskVulkanMemory *self); - -gboolean gsk_vulkan_memory_can_map (GskVulkanMemory *self, - gboolean fast); -guchar * gsk_vulkan_memory_map (GskVulkanMemory *self); -void gsk_vulkan_memory_unmap (GskVulkanMemory *self); - static inline void gsk_vulkan_alloc (GskVulkanAllocator *allocator, VkDeviceSize size, + VkDeviceSize alignment, GskVulkanAllocation *out_alloc) { - allocator->alloc (allocator, size, out_alloc); + allocator->alloc (allocator, size, alignment, out_alloc); } static inline void diff --git a/gsk/vulkan/gskvulkanrender.c b/gsk/vulkan/gskvulkanrender.c index 8ec89c57b2..2d38696f0e 100644 --- a/gsk/vulkan/gskvulkanrender.c +++ b/gsk/vulkan/gskvulkanrender.c @@ -732,7 +732,7 @@ gsk_vulkan_render_ensure_storage_buffer (GskVulkanRender *self) sizeof (float) * 64 * 1024 * 1024); } - self->storage_buffer_memory = gsk_vulkan_buffer_map (self->storage_buffer); + self->storage_buffer_memory = gsk_vulkan_buffer_get_data (self->storage_buffer); if (gsk_vulkan_render_get_buffer_descriptor (self, self->storage_buffer) != 0) { @@ -806,7 +806,6 @@ gsk_vulkan_render_prepare_descriptor_sets (GskVulkanRender *self) if (self->storage_buffer_memory) { - gsk_vulkan_buffer_unmap (self->storage_buffer); self->storage_buffer_memory = NULL; self->storage_buffer_used = 0; } @@ -881,12 +880,11 @@ gsk_vulkan_render_collect_vertex_buffer (GskVulkanRender *self) if (self->vertex_buffer == NULL) self->vertex_buffer = gsk_vulkan_buffer_new (self->vulkan, round_up (n_bytes, VERTEX_BUFFER_SIZE_STEP)); - data = gsk_vulkan_buffer_map (self->vertex_buffer); + data = gsk_vulkan_buffer_get_data (self->vertex_buffer); for (op = self->first_op; op; op = op->next) { gsk_vulkan_op_collect_vertex_data (op, data); } - gsk_vulkan_buffer_unmap (self->vertex_buffer); } static void diff --git a/gsk/vulkan/gskvulkanuploadop.c b/gsk/vulkan/gskvulkanuploadop.c index afb0904456..de06e6ad5e 100644 --- a/gsk/vulkan/gskvulkanuploadop.c +++ b/gsk/vulkan/gskvulkanuploadop.c @@ -41,12 +41,10 @@ gsk_vulkan_upload_op_command_with_area (GskVulkanOp *op, *buffer = gsk_vulkan_buffer_new_map (gsk_vulkan_render_get_context (render), area->height * stride, GSK_VULKAN_WRITE); - data = gsk_vulkan_buffer_map (*buffer); + data = gsk_vulkan_buffer_get_data (*buffer); draw_func (op, data, stride); - gsk_vulkan_buffer_unmap (*buffer); - vkCmdPipelineBarrier (command_buffer, VK_PIPELINE_STAGE_HOST_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, @@ -112,12 +110,11 @@ gsk_vulkan_upload_op_command (GskVulkanOp *op, gsize stride; guchar *data; - data = gsk_vulkan_image_try_map (image, &stride); + data = gsk_vulkan_image_get_data (image, &stride); if (data) { draw_func (op, data, stride); - gsk_vulkan_image_unmap (image); *buffer = NULL; return op->next;