vulkan-guide icon indicating copy to clipboard operation
vulkan-guide copied to clipboard

Invalid usage of VmaAllocation

Open Eearslya opened this issue 1 year ago • 4 comments

Starting in chapter 3, Mesh Buffers, the guide introduces users to VMA. Buffers are created for vertices and indices, as well as host-visible staging buffers.

The guide instructs users to use staging.allocation->GetMappedData() to acquire a pointer to write to the staging buffer. However, this is not valid usage. VmaAllocation is an opaque type in VMA, and all of its members and functions are only visible to the single translation unit that defines VMA_IMPLEMENTATION. This works in vkguide's case because all of the code is within vk_engine.cpp, but it's probably not something we want to be teaching new users in a tutorial scenario.

All of the mentions of GetMappedData() should probably be replaced with the public API function vmaMapMemory().

Eearslya avatar Jul 24 '24 15:07 Eearslya

While we are at it, it would be best to access the mapped data from staging.info.pMappedData. Since the allocations are created with VMA_ALLOCATION_CREATE_MAPPED_BIT, calling the mapping/unmapping functions just adds extra work (though it's not an error).

scgehin avatar Aug 15 '24 02:08 scgehin

Thanks, switched to using allocation.pMappedData. This didn't fix my issues but at least it compiles now lol.

XavierCS-dev avatar Aug 21 '24 12:08 XavierCS-dev

This was a build error for me.

void* data;
vmaMapMemory(_allocator, staging.allocation, &data);
memcpy(data, vertices.data(), vertexBufferSize);
memcpy((char*)data + vertexBufferSize, indices.data(), indexBufferSize);
vmaUnmapMemory(_allocator, staging.allocation);

removed the error

https://github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator/issues/30#issuecomment-472437571

Possibly you try to create an object of type VmaAllocation_T by value, not by pointer like VmaAllocation_T*, which is equivalent to VmaAllocation. You should always use it by pointer and treat it like an opaque pointer. Do not try to peek inside for member variables or functions.

https://gpuopen-librariesandsdks.github.io/VulkanMemoryAllocator/html/memory_mapping.html#memory_mapping_mapping_functions

The tutorial text itself should be updated also:

Once we have the buffer, we can take its mapped address with GetMappedData(), this gives us a void* pointer
we can write to. So we do 2 memcpy commands to copy both spans into it.

to

Once we have the buffer, we can use vmaMapMemory to give us a void* pointer we can write to. So we do 2
memcpy commands to copy both spans into it. vmaUnmapMemory is used here because we have to remember to
unmap the memory after we are done copying the data.

btipling avatar Nov 23 '24 04:11 btipling

Oh I see the VMA_ALLOCATION_CREATE_MAPPED_BIT is best with staging.info.pMappedData as per @scgehin

btipling avatar Dec 26 '24 23:12 btipling