Support vmaAllocateMemoryForBuffer() with a min. alignment

[pknowles]

It'd be nice to have all the features of vmaAllocateMemoryForBuffer() including the ability to increase the required alignment like vmaAllocateMemory() provides.

I think that's currently just:

    bool requiresDedicatedAllocation = false;
    bool prefersDedicatedAllocation = false;

This could be done by adding vmaAllocateMemoryForBufferWithAlignment() to match vmaCreateBufferWithAlignment():

• vmaAllocateMemoryForBuffer() supports users who create their own VkBuffer instead of using vmaCreateBuffer()
• 25e4368 added vmaCreateBufferWithAlignment()
• To complete the symmetry, it'd be nice to have a vmaAllocateMemoryForBufferWithAlignment()

The calls could be factored, much like 9f097a6 did, so it wouldn't be much code to add.

[sawickiap]

Thank you for this suggestion. I understand that adding this function would make sense, but I don't want to add it to avoid feature creep. Function vmaAllocateMemoryForBuffer is a shortcut for calling vkGetBufferMemoryRequirements[2] followed by allocator->AllocateMemory, which can be easily done by the user's code with raising the alignment between them.

Besides this, modern Vulkan encourages to ask for memory requirements based on the CreateInfo struct without creating the buffer first, using function vkGetDeviceBufferMemoryRequirements, which makes this function unnecessary.

What is the use case where this function would be useful to you? Enforcing extra alignment is a special case needed e.g. for interop with other APIs like OpenGL, but normally I would expect the graphics driver to inspect the parameters of the buffer and return appropriate alignment required for it. If the returned alignment is too low for the declared buffer usage, that's a driver bug.

[pknowles]

Aah, good point. GetBufferMemoryRequirements() isn't so big and I could just reimplement it myself (or better yet with vkGetDeviceBufferMemoryRequirements()). Thanks 😄! At first I was thinking there might be some VMA specific code/policy that I'd be losing out on.

My exact use-case was to enforce alignment requirements from VkPhysicalDeviceAccelerationStructurePropertiesKHR and VkPhysicalDeviceClusterAccelerationStructurePropertiesNV. There are a few others. I think some alignments can be indirectly requested by specific usage flags, but the spec does refer to these requirements separately.

For me this is more of a consistency issue because the library I'm working on (vulkan_objects) directly exposes an interface of VmaAllocationCreateInfo. It would be akward to make users pick between that or a fixed alignment.

Another option might be to add a min alignment field directly to VmaAllocationCreateInfo. But I'm happy enough with this too. What do you think?

[sawickiap]

Do I understand correctly that VkPhysicalDeviceAccelerationStructurePropertiesKHR::minAccelerationStructureScratchOffsetAlignment provides extra alignment that needs to be respected for a buffer, or a section of it, to be used as a scratch for acceleration structure building, and there is no way to tell that a buffer is going to be used for acceleration structure scratch (e.g. among buffer usage flags) so that the driver can return correct alignment requirement in the native way?

I can see VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_BUILD_INPUT_READ_ONLY_BIT_KHR and VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_STORAGE_BIT_KHR, but nothing like VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_SCRATCH_BIT_KHR. Isn't that a flaw in the Vulkan spec?

Are you sure this parameter must be respected as an extra alignment when allocating a buffer on top of what's returned via vkGetBufferMemoryRequirements[2], vkGetDeviceBufferMemoryRequirements? The "Offset" part of its name indicates that it may affect only an offset within an existing buffer.

[pknowles]

For symmetry a scratch bit would be nice. That would allow the buffer memory requirements to return appropriate alignment. As it is I think it's up to the application to follow the additional minAccelerationStructureScratchOffsetAlignment. Even though it's an offset I think the spec still expects the underlying buffer to have at least the same alignment. In practice I do see validation layer errors from misaligned absolute addresses without doing something by hand.

[sawickiap]

OK, thanks for this information. In this case, I guess it makes sense to go with the heaviest solution, adding minAlignment field directly to VmaAllocationCreateInfo. What do you think about this?

[pknowles]

It'd work. Default zero won't change anything and it gives control to the app to decide, at the cost of a few extra bytes. If adding I'd document that in nearly all cases it's not required/recommended to set since there's buffer usage bits. I'm very mildly leaning towards leaving things as they are (happy to close this issue), purely for encouraging devs to use the expected path. No real objections though; both are good and I can write it if it'd help.

I think you're right about "offset" - it sounds intended for suballocating a VkBuffer, but then we're missing an explicit usage flag for the driver to request that buffer's alignment. In this case STORAGE_BIT might work in practice, but that's just by chance spec wise. I went looking for other examples in Vulkan with a grep 'Alignment;' and I haven't found any yet (I retract my previous "There are a few others" as I found usage flags for them all). The spec does say the final device address must be aligned to the offset alignment, below. I think for the moment this means the app must either over-allocate the buffer and manually align its address or manually bump the alignment before suballocating for the buffer. Tough call re. VMA. If it's a one-off/edge case I can absolutely see the argument for expecting users do a manual vkGetDeviceBufferMemoryRequirements() + vmaAllocateMemory(). Adding the extra minAlignment parameter is nice for convenience but could easily introduce confusion.

VUID-vkCmdBuildAccelerationStructuresKHR-pInfos-03710
For each element of pInfos, its scratchData.deviceAddress member must be a multiple of VkPhysicalDeviceAccelerationStructurePropertiesKHR::minAccelerationStructureScratchOffsetAlignment

Another thought would be to expose some form of GetBufferMemoryRequirements() to allow overriding and bumping the alignment before vmaAllocateMemory(), but this is really close to your original suggestion.

[sawickiap]

Thanks for the explanation.

We had a need for a custom alignment before, used for interop with other graphics APIs, as described in documentation:
https://gpuopen-librariesandsdks.github.io/VulkanMemoryAllocator/html/other_api_interop.html#other_api_interop_exporting_custom_alignment
I treated this as a special case and I was hesitant to add more features to support it.

But ray tracing is not so much an edge case, so if custom alignment is needed for it, I will add VmaAllocationCreateInfo::minAlignment soon.

[pknowles]

Sounds good to me. Thanks! 😄

[sawickiap]

I added VmaAllocationCreateInfo::minAlignment - on my fork for now:
https://github.com/sawickiap/VulkanMemoryAllocator
Please let me know if it works for you.