Introduce a MinAllocationSize field in ImGui_ImplVulkan_InitInfo that allows users to solve #4238

[philae-ael]

A new field MinAllocationSize is introduced in ImGui_ImplVulkan_InitInfo that controls the minimum size of a GPU allocation (for usage in buffers and images).

Such a field can be used to silence some warnings arising from Vulkan's Validation Layers when Best Practices are enabled:

init_info.MinAllocationSize = 1024*1024;

The magic number 1024*1024 comes from the vulkan's validation layers source code

If MinAllocationSize is set to 0, the current behavior is preserved

Screenshots

init_info.MinAllocationSize = 0;

init_info.MinAllocationSize = 1024*1024;

[ocornut]

Merged as 4778560, thank you !

[philae-ael]

After further inspection, the code added by this PR is unsound:
This line is the culprit:

imgui/backends/imgui_impl_vulkan.cpp

Line 407 in 4778560

VkDeviceSize size = IM_MAX(v->MinAllocationSize, req.size);

The issue is that the size of the buffer and of the allocation are not the same, i thought it would be fine, no big deal, but then the allocation size is used as the size of the buffer when the buffer are in need to be resized.

imgui/backends/imgui_impl_vulkan.cpp

Line 498 in 4778560

if (rb->IndexBuffer == VK_NULL_HANDLE || rb->IndexBufferSize < index_size)

The bug was already present but never occurred because req.size is nearly always, if not always, the same as vertex_buffer_size_aligned
And the bug was not catched when I published this PR because the validation layers do not complain when vulkan is 1.0...

The fix is quite simple:

--- a/backends/imgui_impl_vulkan.cpp
+++ b/backends/imgui_impl_vulkan.cpp
@@ -395,7 +395,7 @@ static void CreateOrResizeBuffer(VkBuffer& buffer, VkDeviceMemory& buffer_memory
     if (buffer_memory != VK_NULL_HANDLE)
         vkFreeMemory(v->Device, buffer_memory, v->Allocator);
 
-    VkDeviceSize vertex_buffer_size_aligned = ((new_size - 1) / bd->BufferMemoryAlignment + 1) * bd->BufferMemoryAlignment;
+    VkDeviceSize vertex_buffer_size_aligned = ((IM_MAX(v->MinAllocationSize, new_size) - 1) / bd->BufferMemoryAlignment + 1) * bd->BufferMemoryAlignment;
     VkBufferCreateInfo buffer_info = {};
     buffer_info.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO;
     buffer_info.size = vertex_buffer_size_aligned;
@@ -407,17 +407,16 @@ static void CreateOrResizeBuffer(VkBuffer& buffer, VkDeviceMemory& buffer_memory
     VkMemoryRequirements req;
     vkGetBufferMemoryRequirements(v->Device, buffer, &req);
     bd->BufferMemoryAlignment = (bd->BufferMemoryAlignment > req.alignment) ? bd->BufferMemoryAlignment : req.alignment;
-    VkDeviceSize size = IM_MAX(v->MinAllocationSize, req.size);
     VkMemoryAllocateInfo alloc_info = {};
     alloc_info.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO;
-    alloc_info.allocationSize = size;
+    alloc_info.allocationSize = req.size;
     alloc_info.memoryTypeIndex = ImGui_ImplVulkan_MemoryType(VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, req.memoryTypeBits);
     err = vkAllocateMemory(v->Device, &alloc_info, v->Allocator, &buffer_memory);
     check_vk_result(err);
 
     err = vkBindBufferMemory(v->Device, buffer, buffer_memory, 0);
     check_vk_result(err);
-    p_buffer_size = size;
+    p_buffer_size = vertex_buffer_size_aligned;
 }
 
 static void ImGui_ImplVulkan_SetupRenderState(ImDrawData* draw_data, VkPipeline pipeline, VkCommandBuffer command_buffer, ImGui_ImplVulkanH_FrameRenderBuffers* rb, int fb_width, int fb_height)

Should i have opened an issue for this? A PR?

[ocornut]

Thank you. I have applied your fix as 82df7c8

I also realized that the vkMapMemory() were suboptimally mapping whole buffer image even when actual vertex/index data is smaller.... It was a mistake introduced by #3957 which I have fixed with 70bb6d1.